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

[Hans de Goede <hdegoede@redhat.com>]

Hi Ville,

On 4/20/22 16:03, Ville Syrjälä wrote:
> 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.

Thank you pushing this out, will you (or someone else from Intel)
also take care of getting this on its way to 5.18-rc# ?

Regards,

Hans



> 
>>  #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
>