-----Original Message-----
From: Bhadane, Dnyaneshwar <dnyaneshwar.bhadane@xxxxxxxxx>
Sent: Thursday, December 19, 2024 12:55 PM
To: Vodapalli, Ravi Kumar <ravi.kumar.vodapalli@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: Vivekanandan, Balasubramani <balasubramani.vivekanandan@xxxxxxxxx>; Roper, Matthew D <matthew.d.roper@xxxxxxxxx>; De Marchi, Lucas <lucas.demarchi@xxxxxxxxx>; Sousa, Gustavo <gustavo.sousa@xxxxxxxxx>; Taylor, Clinton A <clinton.a.taylor@xxxxxxxxx>; Atwood, Matthew S <matthew.s.atwood@xxxxxxxxx>; Kalvala, Haridhar <haridhar.kalvala@xxxxxxxxx>; Chauhan, Shekhar <shekhar.chauhan@xxxxxxxxx>
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@xxxxxxxxx>
>>> Sent: Tuesday, December 17, 2024 11:00 PM
>>> To:intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>>> Cc: Vivekanandan, Balasubramani
>>> <balasubramani.vivekanandan@xxxxxxxxx>; Roper, Matthew D
>>> <matthew.d.roper@xxxxxxxxx>; De Marchi, Lucas
>>> <lucas.demarchi@xxxxxxxxx>; Sousa, Gustavo<gustavo.sousa@xxxxxxxxx>;
>>> Taylor, Clinton A<clinton.a.taylor@xxxxxxxxx>; Atwood, Matthew S
>>> <matthew.s.atwood@xxxxxxxxx>; Bhadane, Dnyaneshwar
>>> <dnyaneshwar.bhadane@xxxxxxxxx>; Kalvala, Haridhar
>>> <haridhar.kalvala@xxxxxxxxx>; Chauhan, Shekhar
>>> <shekhar.chauhan@xxxxxxxxx>
>>> 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@xxxxxxxxx>
>>> ---
>>> 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
>
