Re: [Intel-gfx] [RFC 1/4] drm/i915/quirks: Add quirk for 480MHz CDCLK step

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

On Mon, 02 Jan 2023, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Mon, 02 Jan 2023, Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com> wrote:
>> A new CDCLK step of 480MHz has been added on SKUs that has a
>> RPL-U device id. This is done to support 120Hz displays with
>> more efficiency.
>>
>> RPL-U device ids are currently added within the RPL-P sub
>> platform. It seems to be an overkill to add a separate sub
>> platform just to support this change. Therefore, quirks
>> are a good way to achieve the same.
>
> The thing is, this part is *not* a quirk. It's basic enabling for RPL-U.
>
> If you start conflating quirks and basic enabling to avoid overkill,
> you're eventually going to end up in all kinds of trouble with
> maintenance.

Please start off with adding RPL-U as a subplatform, and let's worry
about the CDCLK after that.

BR,
Jani.

>
>
> BR,
> Jani.
>
>>
>> BSpec: 55409
>>
>> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_quirks.c | 14 ++++++++++++++
>>  drivers/gpu/drm/i915/display/intel_quirks.h |  1 +
>>  2 files changed, 15 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_quirks.c b/drivers/gpu/drm/i915/display/intel_quirks.c
>> index 6e48d3bcdfec..0a30499835b3 100644
>> --- a/drivers/gpu/drm/i915/display/intel_quirks.c
>> +++ b/drivers/gpu/drm/i915/display/intel_quirks.c
>> @@ -65,6 +65,16 @@ static void quirk_no_pps_backlight_power_hook(struct drm_i915_private *i915)
>>  	drm_info(&i915->drm, "Applying no pps backlight power quirk\n");
>>  }
>>  
>> +/*
>> + * A new step of 480MHz has been added on SKUs that have a RPL-U device id.
>> + * This particular step is to better support 120Hz panels.
>> + */
>> +static void quirk_480mhz_cdclk_step_hook(struct drm_i915_private *i915)
>> +{
>> +	intel_set_quirk(i915, QUIRK_480MHZ_CDCLK_STEP);
>> +	drm_info(&i915->drm, "Applying 480MHz CDCLK step quirk\n");
>> +}
>> +
>>  struct intel_quirk {
>>  	int device;
>>  	int subsystem_vendor;
>> @@ -199,6 +209,10 @@ static struct intel_quirk intel_quirks[] = {
>>  	/* ECS Liva Q2 */
>>  	{ 0x3185, 0x1019, 0xa94d, quirk_increase_ddi_disabled_time },
>>  	{ 0x3184, 0x1019, 0xa94d, quirk_increase_ddi_disabled_time },
>> +	/* RPL-U */
>> +	{ 0xA7A1, PCI_ANY_ID, PCI_ANY_ID, quirk_480mhz_cdclk_step_hook },
>> +	{ 0xA721, PCI_ANY_ID, PCI_ANY_ID, quirk_480mhz_cdclk_step_hook },
>> +	{ 0xA7A9, PCI_ANY_ID, PCI_ANY_ID, quirk_480mhz_cdclk_step_hook },
>>  };
>>  
>>  void intel_init_quirks(struct drm_i915_private *i915)
>> diff --git a/drivers/gpu/drm/i915/display/intel_quirks.h b/drivers/gpu/drm/i915/display/intel_quirks.h
>> index 10a4d163149f..71e05684f5f4 100644
>> --- a/drivers/gpu/drm/i915/display/intel_quirks.h
>> +++ b/drivers/gpu/drm/i915/display/intel_quirks.h
>> @@ -17,6 +17,7 @@ enum intel_quirk_id {
>>  	QUIRK_INVERT_BRIGHTNESS,
>>  	QUIRK_LVDS_SSC_DISABLE,
>>  	QUIRK_NO_PPS_BACKLIGHT_POWER_HOOK,
>> +	QUIRK_480MHZ_CDCLK_STEP,
>>  };
>>  
>>  void intel_init_quirks(struct drm_i915_private *i915);

-- 
Jani Nikula, Intel Open Source Graphics Center