Re: [RESEND 02/10] drm/i915/display: Introduce HAS_CASF for sharpness support

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 9/22/2025 4:40 PM, Jani Nikula wrote:
> On Fri, 19 Sep 2025, Nemesa Garg <nemesa.garg@intel.com> wrote:
>> Add HAS_CASF macro to check whether platform supports
>> the content adaptive sharpness capability or not.
>>
>> v2: Update commit message[Ankit]
>> v3: Remove \n from middle[Jani]
>>
>> Signed-off-by: Nemesa Garg <nemesa.garg@intel.com>
>> Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_crtc_state_dump.c | 7 +++++++
>>   drivers/gpu/drm/i915/display/intel_display_device.h  | 1 +
>>   2 files changed, 8 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
>> index 0c7f91046996..bc6a041cec13 100644
>> --- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
>> +++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
>> @@ -373,6 +373,13 @@ void intel_crtc_state_dump(const struct intel_crtc_state *pipe_config,
>>   
>>   	intel_vdsc_state_dump(&p, 0, pipe_config);
>>   
>> +	if (HAS_CASF(display)) {
>> +		drm_printf(&p, "sharpness strength: %d, sharpness tap size: %d, sharpness enable: %d\n",
>> +			   pipe_config->hw.casf_params.strength,
>> +			   pipe_config->hw.casf_params.win_size,
>> +			   pipe_config->hw.casf_params.casf_enable);
>> +	}
> This breaks bisect, because it's referring to things that aren't there yet:
>
> ../drivers/gpu/drm/i915/display/intel_crtc_state_dump.c: In function ‘intel_crtc_state_dump’:
> ../drivers/gpu/drm/i915/display/intel_crtc_state_dump.c:378:43: error: ‘const struct <anonymous>’ has no member named ‘casf_params’
>    378 |                            pipe_config->hw.casf_params.strength,
>        |                                           ^
> ../drivers/gpu/drm/i915/display/intel_crtc_state_dump.c:379:43: error: ‘const struct <anonymous>’ has no member named ‘casf_params’
>    379 |                            pipe_config->hw.casf_params.win_size,
>        |                                           ^
> ../drivers/gpu/drm/i915/display/intel_crtc_state_dump.c:380:43: error: ‘const struct <anonymous>’ has no member named ‘casf_params’
>    380 |                            pipe_config->hw.casf_params.casf_enable);
>        |                                           ^
>
This is strange, the patch where this was RB'd was not having this piece 
of code [1].

Seems like things were moved after the patches were RB'd, without 
mention in history :(.

This indeed needs to be re-looked.

[1] https://patchwork.freedesktop.org/patch/644360/?series=138754&rev=11


Regards,

Ankit

> I thought about simply reordering the patches in the series, but the
> patch that adds casf_params depends on HAS_CASF() introduced here, and
> there are other similar issues all over the place. I can't merge this,
> sorry.
>
> Every commit must compile, no exceptions.
>
> Checking this is as simple as:
>
> $ git rebase -i drm-tip/drm-tip -x make
>
> with whatever remote and build command you use.
>
> And yes, I obviously do this before merging anything, also no
> exceptions.
>
> I'm more than a little annoyed that there have been umpteen versions of
> this series, I've been incessantly pestered about gathering acks and
> getting this merged, and there are still basic things like this that
> nobody looked at or cared about when I'm on the verge of merging.
>
>
> BR,
> Jani.
>
>
>
>> +
>>   dump_planes:
>>   	if (!state)
>>   		return;
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
>> index 1f091fbcd0ec..157aa0b8f36e 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_device.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
>> @@ -144,6 +144,7 @@ struct intel_display_platforms {
>>   #define HAS_ASYNC_FLIPS(__display)	(DISPLAY_VER(__display) >= 5)
>>   #define HAS_AS_SDP(__display)		(DISPLAY_VER(__display) >= 13)
>>   #define HAS_BIGJOINER(__display)	(DISPLAY_VER(__display) >= 11 && HAS_DSC(__display))
>> +#define HAS_CASF(__display)		(DISPLAY_VER(__display) >= 20)
>>   #define HAS_CDCLK_CRAWL(__display)	(DISPLAY_INFO(__display)->has_cdclk_crawl)
>>   #define HAS_CDCLK_SQUASH(__display)	(DISPLAY_INFO(__display)->has_cdclk_squash)
>>   #define HAS_CMRR(__display)		(DISPLAY_VER(__display) >= 20)