Re: [Intel-gfx] [PATCH] drm/i915: Fix DISP_POS_Y and DISP_HEIGHT defines

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Apr 18, 2022 at 05:09:36PM +0200, Hans de Goede wrote:
> Commit 428cb15d5b00 ("drm/i915: Clean up pre-skl primary plane registers")
> introduced DISP_POS_Y and DISP_HEIGHT defines but accidentally set these
> their masks to REG_GENMASK(31, 0) instead of REG_GENMASK(31, 16).
> 
> This breaks the primary display pane on at least pineview machines, fix
> the mask to fix the primary display pane only showing black.
> 
> Tested on an Acer One AO532h with an Intel N450 SoC.
> 
> Fixes: 428cb15d5b00 ("drm/i915: Clean up pre-skl primary plane registers")
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Note this fixes a regression in 5.18-rc# and I'm not entirely sure what
> the procedure is here. Once I get a Reviewed-by or Acked-by and I push
> this to drm-intel-next (where it also is necessary), should I then also
> push it to drm-intel-fixes or will the current drm-intel-fixes
> maintainer pick it up?
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 51f46fe45c72..5f1f38684d65 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4352,12 +4352,12 @@
>  #define _DSPAADDR				0x70184
>  #define _DSPASTRIDE				0x70188
>  #define _DSPAPOS				0x7018C /* reserved */
> -#define   DISP_POS_Y_MASK		REG_GENMASK(31, 0)
> +#define   DISP_POS_Y_MASK		REG_GENMASK(31, 16)

Doh. I guess I only tested it on plane A where the plane gets its size
from PIPESRC instead. And looks like the failure mode is such that
the likes of kms_plane/pixel-formats still gets consistent looking CRCs
even with the misconfigured plane size :/

Thanks for the fix. Pushed to drm-intel-next.

>  #define   DISP_POS_Y(y)			REG_FIELD_PREP(DISP_POS_Y_MASK, (y))
>  #define   DISP_POS_X_MASK		REG_GENMASK(15, 0)
>  #define   DISP_POS_X(x)			REG_FIELD_PREP(DISP_POS_X_MASK, (x))
>  #define _DSPASIZE				0x70190
> -#define   DISP_HEIGHT_MASK		REG_GENMASK(31, 0)
> +#define   DISP_HEIGHT_MASK		REG_GENMASK(31, 16)
>  #define   DISP_HEIGHT(h)		REG_FIELD_PREP(DISP_HEIGHT_MASK, (h))
>  #define   DISP_WIDTH_MASK		REG_GENMASK(15, 0)
>  #define   DISP_WIDTH(w)			REG_FIELD_PREP(DISP_WIDTH_MASK, (w))
> -- 
> 2.35.1

-- 
Ville Syrjälä
Intel