On Thu, Oct 08, 2020 at 01:16:06PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> When the number of potential color planes grew to 4 we stopped
> setting all unused color plane offsets to ~0xfff. The code
> still tries to do this, but actually does nothing since the
> loop limits are bogus.
>
> skl_check_main_surface() actually depends on this ~0xfff
> behaviour as it will make sure to move the main surface
> offset below the aux surface offset because the hardware
> AUX_DIST must be a non-negative value [1], and for simplicity
> it doesn't bother checking if the AUX plane is actually
> needed or not. So currently it may end up shuffling the
> main surface around based on some stale leftover AUX offset.
>
> The skl+ plane code also just blindly calculates the AUX_DIST
> whether or not the AUX plane is actually needed by the hw or
> not, and that too will now potentially use some stale AUX
> surface offset in the calculation. Would seem nicer to
> guarantee a consistent non-negative AUX_DIST always.
>
> So bring back the original ~0xfff offset behaviour for
> unused color planes. Though it doesn't seem super likely
> that this inconsistency would cause any real issues.
>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx>
> Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> Cc: Imre Deak <imre.deak@xxxxxxxxx>
> Cc: Radhakrishna Sripada <radhakrishna.sripada@xxxxxxxxx>
> Fixes: 2dfbf9d2873a ("drm/i915/tgl: Gen-12 display can decompress surfaces compressed by the media engine")
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Arg. Yes skl_check_main_surface() adjusts now the address needlessly.
The fix looks ok to me:
Reviewed-by: Imre Deak <imre.deak@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 17 +++++------------
> 1 file changed, 5 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 907e1d155443..44fd7059838f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -4104,8 +4104,7 @@ static int skl_check_ccs_aux_surface(struct intel_plane_state *plane_state)
> int skl_check_plane_surface(struct intel_plane_state *plane_state)
> {
> const struct drm_framebuffer *fb = plane_state->hw.fb;
> - int ret;
> - bool needs_aux = false;
> + int ret, i;
>
> ret = intel_plane_compute_gtt(plane_state);
> if (ret)
> @@ -4119,7 +4118,6 @@ int skl_check_plane_surface(struct intel_plane_state *plane_state)
> * it.
> */
> if (is_ccs_modifier(fb->modifier)) {
> - needs_aux = true;
> ret = skl_check_ccs_aux_surface(plane_state);
> if (ret)
> return ret;
> @@ -4127,20 +4125,15 @@ int skl_check_plane_surface(struct intel_plane_state *plane_state)
>
> if (intel_format_info_is_yuv_semiplanar(fb->format,
> fb->modifier)) {
> - needs_aux = true;
> ret = skl_check_nv12_aux_surface(plane_state);
> if (ret)
> return ret;
> }
>
> - if (!needs_aux) {
> - int i;
> -
> - for (i = 1; i < fb->format->num_planes; i++) {
> - plane_state->color_plane[i].offset = ~0xfff;
> - plane_state->color_plane[i].x = 0;
> - plane_state->color_plane[i].y = 0;
> - }
> + for (i = fb->format->num_planes; i < ARRAY_SIZE(plane_state->color_plane); i++) {
> + plane_state->color_plane[i].offset = ~0xfff;
> + plane_state->color_plane[i].x = 0;
> + plane_state->color_plane[i].y = 0;
> }
>
> ret = skl_check_main_surface(plane_state);
> --
> 2.26.2
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
