Re: [Intel-gfx] [PATCH 1/6] drm/i915: Make a copy of the ggtt view for slave plane

[Chris Wilson <chris@chris-wilson.co.uk>]

Quoting Ville Syrjala (2020-01-10 18:32:23)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> intel_prepare_plane_fb() will always pin plane_state->hw.fb whenever
> it is present. We copy that from the master plane to the slave plane,
> but we fail to copy the corresponding ggtt view. Thus when it comes time
> to pin the slave plane's fb we use some stale ggtt view left over from
> the last time the plane was used as a non-slave plane. If that previous
> use involved 90/270 degree rotation or remapping we'll try to shuffle
> the pages of the new fb around accordingingly. However the new
> fb may be backed by a bo with less pages than what the ggtt view
> rotation/remapped info requires, and so we we trip a GEM_BUG().
> 
> Steps to reproduce on icl:
> 1. plane 1: whatever
>    plane 6: largish !NV12 fb + 90 degree rotation
> 2. plane 1: smallish NV12 fb
>    plane 6: make invisible so it gets slaved to plane 1
> 3. GEM_BUG()
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Closes: https://gitlab.freedesktop.org/drm/intel/issues/951
> Fixes: 1f594b209fe1 ("drm/i915: Remove special case slave handling during hw programming, v3.")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 59c375879186..fafb67689dee 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -12366,6 +12366,7 @@ static int icl_check_nv12_planes(struct intel_crtc_state *crtc_state)
>                 /* Copy parameters to slave plane */
>                 linked_state->ctl = plane_state->ctl | PLANE_CTL_YUV420_Y_PLANE;
>                 linked_state->color_ctl = plane_state->color_ctl;
> +               linked_state->view = plane_state->view;
>                 memcpy(linked_state->color_plane, plane_state->color_plane,
>                        sizeof(linked_state->color_plane));

So this bit is just copying across the results of
intel_plane_compute_gtt()?

What happens for equivalent of intel_plane_needs_remap()?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx