Re: [PATCH 21/23] drm/xe/xe3p_xpc: Skip compression tuning on platforms without flatccs

[Shekhar Chauhan <shekhar.chauhan@intel.com>]

On 10/14/2025 23:44, Matt Roper wrote:
> On Tue, Oct 14, 2025 at 01:39:39PM +0530, Shekhar Chauhan wrote:
>> On 10/14/2025 8:54, Lucas De Marchi wrote:
>>> From: Matt Roper <matthew.d.roper@intel.com>
>>>
>>> The compression overfetch tuning settings only apply to platforms that
>>> support FlatCCS.  In Xe3p_XPC (and any future IPs that also lack
>>> compression) some of the registers being adjusted by this tuning will
>>> not exist or may have been repurposed for something else, so we should
>>> take care not to try to program them.
>>>
>>> Note that our xe_rtp_match_has_flatccs() function will also return false
>>> on platforms that do have FlatCCS in the hardware design, but have
>>> compression manually disabled in the BIOS.  On such platforms the
>>> registers still exist (and it would be fine to continue programming
>>> them), but they would have no effect, so skipping that tuning is also
>>> safe.
>>>
>>> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> The patch in itself looks fine to me, but, the patch doesn't specifically
>> play only around Xe3p_XPC. Maybe we can move this patch out of the current
>> series, or if we wish to keep it here, can we instead drop the xe3p_xpc from
>> the prefix? Thoughts?
> Aside from PVC (which is Xe1 and not relevant to these tuning settings),
> Xe3p_XPC is the only IP we have without FlatCCS in the design, so at the
> moment this patch is specific to Xe3p_XPC.  As a general architecture
> though, if we eventually wind up with other IPs down the road that also
> lack FlatCCS, then these tuning settings will also not apply to them for
> the same reasons.
>
>
> Matt

Okay, makes sense, I'm still skeptical on whether this should be named 
with the prefix Xe3p_XPC if we expect it to carry it's effect to other 
future platforms as well. For that reason, I believe, the commit title 
can be 'drm/xe: Skip compression tuning on platforms without flatccs'. 
This in itself engulfs Xe3p_XPC (and any other future platforms). But, 
consider this feedback as only a nitpick.

Since the changes already look good,

Reviewed-by: Shekhar Chauhan <shekhar.chauhan@intel.com>

>
>>> ---
>>>    drivers/gpu/drm/xe/xe_rtp.c    |  6 ++++++
>>>    drivers/gpu/drm/xe/xe_rtp.h    | 10 ++++++++++
>>>    drivers/gpu/drm/xe/xe_tuning.c |  9 ++++++---
>>>    3 files changed, 22 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_rtp.c b/drivers/gpu/drm/xe/xe_rtp.c
>>> index b5f430d59f805..22a879270c1ec 100644
>>> --- a/drivers/gpu/drm/xe/xe_rtp.c
>>> +++ b/drivers/gpu/drm/xe/xe_rtp.c
>>> @@ -376,3 +376,9 @@ bool xe_rtp_match_gt_has_discontiguous_dss_groups(const struct xe_gt *gt,
>>>    {
>>>    	return xe_gt_has_discontiguous_dss_groups(gt);
>>>    }
>>> +
>>> +bool xe_rtp_match_has_flat_ccs(const struct xe_gt *gt,
>>> +			      const struct xe_hw_engine *hwe)
>>> +{
>>> +	return gt_to_xe(gt)->info.has_flat_ccs;
>>> +}
>>> diff --git a/drivers/gpu/drm/xe/xe_rtp.h b/drivers/gpu/drm/xe/xe_rtp.h
>>> index ac12ddf6cde6d..92f0d16110dd5 100644
>>> --- a/drivers/gpu/drm/xe/xe_rtp.h
>>> +++ b/drivers/gpu/drm/xe/xe_rtp.h
>>> @@ -483,4 +483,14 @@ bool xe_rtp_match_psmi_enabled(const struct xe_gt *gt,
>>>    bool xe_rtp_match_gt_has_discontiguous_dss_groups(const struct xe_gt *gt,
>>>    						  const struct xe_hw_engine *hwe);
>>> +/**
>>> + * xe_rtp_match_has_flat_ccs - Match when platform has FlatCCS compression
>>> + * @gt: GT structure
>>> + * @hwe: Engine instance
>>> + *
>>> + * Returns: true if platform has FlatCCS compression, false otherwise
>>> + */
>>> +bool xe_rtp_match_has_flat_ccs(const struct xe_gt *gt,
>>> +			       const struct xe_hw_engine *hwe);
>>> +
>>>    #endif
>>> diff --git a/drivers/gpu/drm/xe/xe_tuning.c b/drivers/gpu/drm/xe/xe_tuning.c
>>> index fd58ea5e78bf6..7c140d8cb1e07 100644
>>> --- a/drivers/gpu/drm/xe/xe_tuning.c
>>> +++ b/drivers/gpu/drm/xe/xe_tuning.c
>>> @@ -40,7 +40,8 @@ static const struct xe_rtp_entry_sr gt_tunings[] = {
>>>    				   REG_FIELD_PREP(L3_PWM_TIMER_INIT_VAL_MASK, 0x7f)))
>>>    	},
>>>    	{ XE_RTP_NAME("Tuning: Compression Overfetch"),
>>> -	  XE_RTP_RULES(GRAPHICS_VERSION_RANGE(2001, XE_RTP_END_VERSION_UNDEFINED)),
>>> +	  XE_RTP_RULES(GRAPHICS_VERSION_RANGE(2001, XE_RTP_END_VERSION_UNDEFINED),
>>> +		       FUNC(xe_rtp_match_has_flat_ccs)),
>>>    	  XE_RTP_ACTIONS(CLR(CCCHKNREG1, ENCOMPPERFFIX),
>>>    			 SET(CCCHKNREG1, L3CMPCTRL))
>>>    	},
>>> @@ -58,12 +59,14 @@ static const struct xe_rtp_entry_sr gt_tunings[] = {
>>>    	  XE_RTP_ACTIONS(SET(XE2LPM_L3SQCREG3, COMPPWOVERFETCHEN))
>>>    	},
>>>    	{ XE_RTP_NAME("Tuning: L2 Overfetch Compressible Only"),
>>> -	  XE_RTP_RULES(GRAPHICS_VERSION_RANGE(2001, XE_RTP_END_VERSION_UNDEFINED)),
>>> +	  XE_RTP_RULES(GRAPHICS_VERSION_RANGE(2001, XE_RTP_END_VERSION_UNDEFINED),
>>> +		       FUNC(xe_rtp_match_has_flat_ccs)),
>>>    	  XE_RTP_ACTIONS(SET(L3SQCREG2,
>>>    			     COMPMEMRD256BOVRFETCHEN))
>>>    	},
>>>    	{ XE_RTP_NAME("Tuning: L2 Overfetch Compressible Only - media"),
>>> -	  XE_RTP_RULES(MEDIA_VERSION_RANGE(2000, XE_RTP_END_VERSION_UNDEFINED)),
>>> +	  XE_RTP_RULES(MEDIA_VERSION_RANGE(2000, XE_RTP_END_VERSION_UNDEFINED),
>>> +		       FUNC(xe_rtp_match_has_flat_ccs)),
>>>    	  XE_RTP_ACTIONS(SET(XE2LPM_L3SQCREG2,
>>>    			     COMPMEMRD256BOVRFETCHEN))
>>>    	},
>>>