[PATCH] drm/i915/display: Don't program DBUF_CTL tracker state service

[Vodapalli, Ravi Kumar]

-----Original Message-----
From: Bhadane, Dnyaneshwar <dnyaneshwar.bhadane at intel.com> 
Sent: Thursday, December 19, 2024 12:55 PM
To: Vodapalli, Ravi Kumar <ravi.kumar.vodapalli at intel.com>; intel-gfx at lists.freedesktop.org
Cc: Vivekanandan, Balasubramani <balasubramani.vivekanandan at intel.com>; Roper, Matthew D <matthew.d.roper at intel.com>; De Marchi, Lucas <lucas.demarchi at intel.com>; Sousa, Gustavo <gustavo.sousa at intel.com>; Taylor, Clinton A <clinton.a.taylor at intel.com>; Atwood, Matthew S <matthew.s.atwood at intel.com>; Kalvala, Haridhar <haridhar.kalvala at intel.com>; Chauhan, Shekhar <shekhar.chauhan at intel.com>
Subject: Re: [PATCH] drm/i915/display: Don't program DBUF_CTL tracker state service



On 18-Dec-24 11:54 PM, Vodapalli, Ravi Kumar wrote:
> Hi,
> 
> My inline comments
> 
> 
> On 12/18/2024 2:07 AM, Bhadane, Dnyaneshwar wrote:
>>> -----Original Message-----
>>> From: Vodapalli, Ravi Kumar<ravi.kumar.vodapalli at intel.com>
>>> Sent: Tuesday, December 17, 2024 11:00 PM 
>>> To:intel-gfx at lists.freedesktop.org
>>> Cc: Vivekanandan, Balasubramani
>>> <balasubramani.vivekanandan at intel.com>; Roper, Matthew D 
>>> <matthew.d.roper at intel.com>; De Marchi, Lucas 
>>> <lucas.demarchi at intel.com>; Sousa, Gustavo<gustavo.sousa at intel.com>; 
>>> Taylor, Clinton A<clinton.a.taylor at intel.com>; Atwood, Matthew S 
>>> <matthew.s.atwood at intel.com>; Bhadane, Dnyaneshwar 
>>> <dnyaneshwar.bhadane at intel.com>; Kalvala, Haridhar 
>>> <haridhar.kalvala at intel.com>; Chauhan, Shekhar 
>>> <shekhar.chauhan at intel.com>
>>> Subject: [PATCH] drm/i915/display: Don't program DBUF_CTL tracker 
>>> state service
>>>
>>> While display initialization along with MBUS credits programming 
>>> DBUF_CTL register is also programmed, as a part of it the tracker 
>>> state service field is also set to 0x8 value when default value is 
>>> other than 0x8 which are for platforms past display version 13.
>>> For remaining platforms the default value is already 0x8 so don't 
>>> program them.
>>>
>>> Bspec: 49213
>>> Signed-off-by: Ravi Kumar Vodapalli<ravi.kumar.vodapalli at intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/display/intel_display_power.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
>>> b/drivers/gpu/drm/i915/display/intel_display_power.c
>>> index 34465d56def0..cbcc4bddc01f 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
>>> @@ -1126,7 +1126,7 @@ static void gen12_dbuf_slices_config(struct 
>>> intel_display *display)  {
>>>   	enum dbuf_slice slice;
>>>
>>> -	if (display->platform.alderlake_p)
>>> +	if (DISPLAY_VER(display) >= 13)
>> Hi Ravi,
>> This new condition is not needed here since there's already a similar check in the caller function icl_display_core_init().
>> Please update the condition at the caller function and remove this code from here.
> 
> As MattR explained The function gen12_dbuf_slices_config() name 
> suggest that it is called for gen12 platform onwards and higher, any 
> changes in the code for higher platforms has to be implemented in that function.

I don't know which part of my review was unclear that you mixed it up with what you wrote above. But what I am trying to convey is that we can easily modify the caller function condition as

	if (DISPLAY_VER(display) == 12)

instead of modifying the platform check inside gen12_dbuf_slices_config()

Got it what is your view, discussed with MattR what you said seems better.

Dnyaneshwar

> Ravi Kumar V
>> Dnyaneshwar,
>>>   		return;
>>>
>>>   	for_each_dbuf_slice(display, slice)
>>> --
>>> 2.25.1
>