[Intel-gfx] [PATCH] drm/i915/tgl: Assume future platforms will inherit TGL's SFC capability

[Matt Roper]

On Thu, Dec 26, 2019 at 02:23:49PM -0800, Lucas De Marchi wrote:
> On Tue, Dec 24, 2019 at 03:15:21PM -0800, Matt Roper wrote:
> > Our usual i915 convention is to assume that future platforms will follow
> > the same behavior as the latest platform of today.  The VDBOX/SFC
> > capabilities described here don't seem like something that should be
> > specific to TGL, so let's future-proof by making the test apply to all
> > gen12+ platforms.
> > 
> > Cc: Lucas De Marchi <lucas.demarchi at intel.com>
> > Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_device_info.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> > index 1acb5db77431..bb709a08bd3c 100644
> > --- a/drivers/gpu/drm/i915/intel_device_info.c
> > +++ b/drivers/gpu/drm/i915/intel_device_info.c
> > @@ -1093,7 +1093,7 @@ void intel_device_info_init_mmio(struct drm_i915_private *dev_priv)
> > 		 * hooked up to an SFC (Scaler & Format Converter) unit.
> > 		 * In TGL each VDBOX has access to an SFC.
> > 		 */
> > -		if (IS_TIGERLAKE(dev_priv) || logical_vdbox++ % 2 == 0)
> > +		if (INTEL_GEN(dev_priv) >= 12 || logical_vdbox++ % 2 == 0)
> > 			RUNTIME_INFO(dev_priv)->vdbox_sfc_access |= BIT(i);
> 
> but why are we even doing this instead of initiliazing them at compile
> time on the device_info? If it's fused off, then whatever is set in
> vdbox_sfc_access bit shouldn't matter... or if the code making use of
> this doesn't check for engine availability, then this part of the
> function could just disable the bit of whatever is fused off, regardless
> if it's ice lake, tiger lake or whatever.

I'm not sure; it looks like among other things we send this bitmask
directly to the GuC, so I'm not really comfortable making the assumption
that all users of the mask will pay attention to whether the engine is
fused off or not, even if that turns out to be true for the i915 usage.

I could switch this to being initialized statically and then modified
here if an engine is fused off.  In that case should this move out of
RUNTIME_INFO() and back to INTEL_INFO()?  Honestly I've never really
understood why we have those separated given that we still ignore the
const and modify INTEL_INFO at runtime in several places.


Matt

> 
> Lucas De Marchi
> 
> 
> > 	}
> > 	DRM_DEBUG_DRIVER("vdbox enable: %04x, instances: %04lx\n",
> > -- 
> > 2.23.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795