On Mon, 2021-11-08 at 13:38 -0800, José Roberto de Souza wrote: > When a plane with a multiplanar format is added to the state by > drm_atomic_add_affected_planes(), only the UV plane is > added, so a intel_atomic_get_new_plane_state() call to get the Y > plane state can return a null pointer.
I don't understand how this could happen only sometimes? Were you able to reproduce this somehow? Generally: checking linked_new_plane_state being valid pointer makes sense. I'm just wondering how and when this could happen and how that should be handled. > To fix this, intel_atomic_get_plane_state() should be called and > the return needs to be checked for errors, as it could return a > EAGAIN > as other atomic state could be holding the lock for the Y plane. > > Other issue with the patch being fixed is that the Y plane is not > being committed to hardware because the corresponded plane bit is not > set in update_planes when UV and Y planes are added to the state by > drm_atomic_add_affected_planes(). To my understanding this one is already set in intel_display.c:icl_check_nv12_planes. > > Cc: Jouni Högander <[email protected]> > Cc: Mika Kahola <[email protected]> > Fixes: 3809991ff5f4 ("drm/i915/display: Add initial selective fetch > support for biplanar formats") > Signed-off-by: José Roberto de Souza <[email protected]> > --- > drivers/gpu/drm/i915/display/intel_psr.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > b/drivers/gpu/drm/i915/display/intel_psr.c > index 9d589d471e335..a1a663f362e7d 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr.c > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > @@ -1732,13 +1732,17 @@ int intel_psr2_sel_fetch_update(struct > intel_atomic_state *state, > * same area for Y plane as well. > */ > if (linked) { > - struct intel_plane_state > *linked_new_plane_state = > - intel_atomic_get_new_plane_state(state, > linked); > - struct drm_rect *linked_sel_fetch_area = > - &linked_new_plane_state->psr2_sel_fetch_area; > + struct intel_plane_state > *linked_new_plane_state; > + struct drm_rect *linked_sel_fetch_area; > > + linked_new_plane_state = > intel_atomic_get_plane_state(state, linked); > + if (IS_ERR(linked_new_plane_state)) > + return PTR_ERR(linked_new_plane_state); > + > + linked_sel_fetch_area = > &linked_new_plane_state->psr2_sel_fetch_area; > linked_sel_fetch_area->y1 = sel_fetch_area->y1; > linked_sel_fetch_area->y2 = sel_fetch_area->y2; > + crtc_state->update_planes |= BIT(linked->id); > } > } >
