Re: [Intel-gfx] [PATCH v5 4/9] drm/i915: Eliminate IS_MTL_GRAPHICS_STEP

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 31 Aug 2023, Matt Roper <matthew.d.roper@intel.com> wrote:
> On Thu, Aug 31, 2023 at 07:16:55PM +0300, Jani Nikula wrote:
>> On Mon, 21 Aug 2023, Matt Roper <matthew.d.roper@intel.com> wrote:
>> > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
>> > index a408ec2d3958..4566c95da1ca 100644
>> > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
>> > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
>> > @@ -20,6 +20,7 @@
>> >  #include "skl_scaler.h"
>> >  #include "skl_universal_plane.h"
>> >  #include "skl_watermark.h"
>> > +#include "gt/intel_gt.h"
>> >  #include "pxp/intel_pxp.h"
>> >  
>> >  static const u32 skl_plane_formats[] = {
>> > @@ -2169,8 +2170,8 @@ static bool skl_plane_has_rc_ccs(struct drm_i915_private *i915,
>> >  				 enum pipe pipe, enum plane_id plane_id)
>> >  {
>> >  	/* Wa_14017240301 */
>> > -	if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) ||
>> > -	    IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0))
>> > +	if (IS_GFX_GT_IP_STEP(to_gt(i915), IP_VER(12, 70), STEP_A0, STEP_B0) ||
>> > +	    IS_GFX_GT_IP_STEP(to_gt(i915), IP_VER(12, 71), STEP_A0, STEP_B0))
>> >  		return false;
>> 
>> This seems to be the only user of IS_GFX_GT_IP_STEP() under display/,
>> and it kind of seems wrong to have display code check for GT
>> versions. Is there a clean way to move this out of display?
>
> If I remember correctly, this one literally is tied to the graphics IP
> rather than the display IP.  There's something busted with how the
> graphics GT is trying to generate compressed buffers that causes them to
> not decompress properly in the display controller (although GT<->GT
> compression/decompression is okay since both sides are broken in the
> same way).  So the workaround is to not advertise our display planes as
> having support for compressed buffers when the GT is A-step, because we
> know they're going to show up in the wrong format.  That still allows
> compression to be used for the non-display use cases, but avoids
> possible display corruption.
>
> Honestly the simplest solution might be to just go ahead and delete this
> workaround since it's only relevant to pre-production hardware.  I know
> our general policy has always been to hang on to workarounds for
> pre-production steppings in the driver until the n+1 platform/IP is
> pretty far along, but in this case it looks like our CI machines are
> already on B0 GT stepping, and even if some internal people are still
> working with older boards, this is still kind of a corner case that
> probably won't impact most usage.

Thanks for the explanation. I'd ack just nuking the check.

BR,
Jani.

>
>
> Matt
>
>> 
>> BR,
>> Jani.
>> 
>> 
>> -- 
>> Jani Nikula, Intel Open Source Graphics Center

-- 
Jani Nikula, Intel Open Source Graphics Center