search for: drm_dp_atomic_release_vcpi_slots

...> */ > > That's not the only reason we should fix this: forcing the driver to > track the VCPI allocations throughout a state's atomic check is > error prone, because it means that extra care has to be taken with the > order that drm_dp_atomic_find_vcpi_slots() and > drm_dp_atomic_release_vcpi_slots() are called in in order to ensure > idempotency. Currently the only driver actually using these helpers, > i915, doesn't even do this correctly: multiple ->best_encoder() checks > with i915's current implementation would not be idempotent and would > over-allocate VCPI slots...

...y is inactive we end up calculating a PBN of 0, which inadvertently causes us to have an allocation of 0. >From there, if userspace then disables the CRTC afterwards we end up accidentally attempting to free the VCPI twice: WARNING: CPU: 0 PID: 1484 at drivers/gpu/drm/drm_dp_mst_topology.c:3336 drm_dp_atomic_release_vcpi_slots+0x87/0xb0 [drm_kms_helper] RIP: 0010:drm_dp_atomic_release_vcpi_slots+0x87/0xb0 [drm_kms_helper] Call Trace: drm_atomic_helper_check_modeset+0x3f3/0xa60 [drm_kms_helper] ? drm_atomic_check_only+0x43/0x780 [drm] drm_atomic_helper_check+0x15/0x90 [drm_kms_helper] nv50_disp_atomic_check+0x83/0x1d0...

...tell us how many slots to release. */ That's not the only reason we should fix this: forcing the driver to track the VCPI allocations throughout a state's atomic check is error prone, because it means that extra care has to be taken with the order that drm_dp_atomic_find_vcpi_slots() and drm_dp_atomic_release_vcpi_slots() are called in in order to ensure idempotency. Currently the only driver actually using these helpers, i915, doesn't even do this correctly: multiple ->best_encoder() checks with i915's current implementation would not be idempotent and would over-allocate VCPI slots, something I learne...

...only reason we should fix this: forcing the driver to > > > track the VCPI allocations throughout a state's atomic check is > > > error prone, because it means that extra care has to be taken with the > > > order that drm_dp_atomic_find_vcpi_slots() and > > > drm_dp_atomic_release_vcpi_slots() are called in in order to ensure > > > idempotency. Currently the only driver actually using these helpers, > > > i915, doesn't even do this correctly: multiple ->best_encoder() checks > > > with i915's current implementation would not be idempotent and would...

This patchset does some cleaning up of the atomic VCPI helpers for MST, and converts nouveau over to using them. I would have included amdgpu in this patch as well, but at the moment moving them over to the atomic helpers is nontrivial. Cc: Daniel Vetter <daniel at ffwll.ch> Lyude Paul (5): drm/dp_mst: Add some atomic state iterator macros drm/dp_mst: Start tracking per-port VCPI

...tell us how many slots to release. */ That's not the only reason we should fix this: forcing the driver to track the VCPI allocations throughout a state's atomic check is error prone, because it means that extra care has to be taken with the order that drm_dp_atomic_find_vcpi_slots() and drm_dp_atomic_release_vcpi_slots() are called in in order to ensure idempotency. Currently the only driver actually using these helpers, i915, doesn't even do this correctly: multiple ->best_encoder() checks with i915's current implementation would not be idempotent and would over-allocate VCPI slots, something I learne...

...; [drm:drm_atomic_helper_check_modeset [drm_kms_helper]] Disabling [CONNECTOR:123:DP-5] > [drm:drm_atomic_get_private_obj_state [drm]] Added new private object 0000000025844636 state 000000009fd2899a to 000000003a13d7b8 > WARNING: CPU: 6 PID: 1070 at drivers/gpu/drm/drm_dp_mst_topology.c:3153 drm_dp_atomic_release_vcpi_slots+0xb9/0x200 [drm_kms_helper] > Modules linked in: fuse vfat fat snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic joydev iTCO_wdt i915(O) wmi_bmof intel_rapl btusb btrtl x86_pkg_temp_thermal btbcm btintel coretemp i2c_algo_bit drm_kms_helper(O) crc32_pclmul snd_hda_intel syscopyarea...

...tell us how many slots to release. */ That's not the only reason we should fix this: forcing the driver to track the VCPI allocations throughout a state's atomic check is error prone, because it means that extra care has to be taken with the order that drm_dp_atomic_find_vcpi_slots() and drm_dp_atomic_release_vcpi_slots() are called in in order to ensure idempotency. Currently the only driver actually using these helpers, i915, doesn't even do this correctly: multiple ->best_encoder() checks with i915's current implementation would not be idempotent and would over-allocate VCPI slots, something I learne...

...> */ > > That's not the only reason we should fix this: forcing the driver to > track the VCPI allocations throughout a state's atomic check is > error prone, because it means that extra care has to be taken with the > order that drm_dp_atomic_find_vcpi_slots() and > drm_dp_atomic_release_vcpi_slots() are called in in order to ensure > idempotency. Currently the only driver actually using these helpers, > i915, doesn't even do this correctly: multiple ->best_encoder() checks > with i915's current implementation would not be idempotent and would > over-allocate VCPI slots...

...tell us how many slots to release. */ That's not the only reason we should fix this: forcing the driver to track the VCPI allocations throughout a state's atomic check is error prone, because it means that extra care has to be taken with the order that drm_dp_atomic_find_vcpi_slots() and drm_dp_atomic_release_vcpi_slots() are called in in order to ensure idempotency. Currently the only driver actually using these helpers, i915, doesn't even do this correctly: multiple ->best_encoder() checks with i915's current implementation would not be idempotent and would over-allocate VCPI slots, something I learne...

...> */ > > That's not the only reason we should fix this: forcing the driver to > track the VCPI allocations throughout a state's atomic check is > error prone, because it means that extra care has to be taken with the > order that drm_dp_atomic_find_vcpi_slots() and > drm_dp_atomic_release_vcpi_slots() are called in in order to ensure > idempotency. Currently the only driver actually using these helpers, > i915, doesn't even do this correctly: multiple ->best_encoder() checks > with i915's current implementation would not be idempotent and would > over-allocate VCPI slots...

This patchset does some cleaning up of the atomic VCPI helpers for MST, and converts nouveau over to using them. I would have included amdgpu in this patch as well, but at the moment moving them over to the atomic helpers is nontrivial. Cc: Daniel Vetter <daniel at ffwll.ch> Lyude Paul (5): drm/dp_mst: Add some atomic state iterator macros drm/dp_mst: Start tracking per-port VCPI

...only reason we should fix this: forcing the driver to > > > track the VCPI allocations throughout a state's atomic check is > > > error prone, because it means that extra care has to be taken with the > > > order that drm_dp_atomic_find_vcpi_slots() and > > > drm_dp_atomic_release_vcpi_slots() are called in in order to ensure > > > idempotency. Currently the only driver actually using these helpers, > > > i915, doesn't even do this correctly: multiple ->best_encoder() checks > > > with i915's current implementation would not be idempotent and would...

...d fix this: forcing the driver to > > > > track the VCPI allocations throughout a state's atomic check is > > > > error prone, because it means that extra care has to be taken with the > > > > order that drm_dp_atomic_find_vcpi_slots() and > > > > drm_dp_atomic_release_vcpi_slots() are called in in order to ensure > > > > idempotency. Currently the only driver actually using these helpers, > > > > i915, doesn't even do this correctly: multiple ->best_encoder() checks > > > > with i915's current implementation would not be idemp...

...That's not the only reason we should fix this: forcing the driver to > > track the VCPI allocations throughout a state's atomic check is > > error prone, because it means that extra care has to be taken with the > > order that drm_dp_atomic_find_vcpi_slots() and > > drm_dp_atomic_release_vcpi_slots() are called in in order to ensure > > idempotency. Currently the only driver actually using these helpers, > > i915, doesn't even do this correctly: multiple ->best_encoder() checks > > with i915's current implementation would not be idempotent and would > > over...

...tell us how many slots to release. */ That's not the only reason we should fix this: forcing the driver to track the VCPI allocations throughout a state's atomic check is error prone, because it means that extra care has to be taken with the order that drm_dp_atomic_find_vcpi_slots() and drm_dp_atomic_release_vcpi_slots() are called in in order to ensure idempotency. Currently the only driver actually using these helpers, i915, doesn't even do this correctly: multiple ->best_encoder() checks with i915's current implementation would not be idempotent and would over-allocate VCPI slots, something I learne...

This patchset does some cleaning up of the atomic VCPI helpers for MST, and converts nouveau over to using them. I would have included amdgpu in this patch as well, but at the moment moving them over to the atomic helpers is nontrivial. Cc: Daniel Vetter <daniel at ffwll.ch> Lyude Paul (5): drm/dp_mst: Add some atomic state iterator macros drm/dp_mst: Start tracking per-port VCPI

...gt; > > track the VCPI allocations throughout a state's atomic check is > > > > > error prone, because it means that extra care has to be taken with > > > > > the > > > > > order that drm_dp_atomic_find_vcpi_slots() and > > > > > drm_dp_atomic_release_vcpi_slots() are called in in order to ensure > > > > > idempotency. Currently the only driver actually using these helpers, > > > > > i915, doesn't even do this correctly: multiple ->best_encoder() > > > > > checks > > > > > with i915's c...

This patchset does some cleaning up of the atomic VCPI helpers for MST, and converts nouveau over to using them. I would have included amdgpu in this patch as well, but at the moment moving them over to the atomic helpers is nontrivial. [sorry for still not adding your R-Bs! I ended up needing to make a lot of changes to make it so we do actually kref() each port in the atomic state] Cc: Daniel

...tell us how many slots to release. */ That's not the only reason we should fix this: forcing the driver to track the VCPI allocations throughout a state's atomic check is error prone, because it means that extra care has to be taken with the order that drm_dp_atomic_find_vcpi_slots() and drm_dp_atomic_release_vcpi_slots() are called in in order to ensure idempotency. Currently the only driver actually using these helpers, i915, doesn't even do this correctly: multiple ->best_encoder() checks with i915's current implementation would not be idempotent and would over-allocate VCPI slots, something I learne...