-----Original Message-----
From: Bhadane, Dnyaneshwar <dnyaneshwar.bhad...@intel.com>
Sent: Thursday, December 19, 2024 12:55 PM
To: Vodapalli, Ravi Kumar <ravi.kumar.vodapa...@intel.com>;
intel-gfx@lists.freedesktop.org
Cc: Vivekanandan, Balasubramani <balasubramani.vivekanan...@intel.com>; Roper,
Matthew D <matthew.d.ro...@intel.com>; De Marchi, Lucas
<lucas.demar...@intel.com>; Sousa, Gustavo <gustavo.so...@intel.com>; Taylor,
Clinton A <clinton.a.tay...@intel.com>; Atwood, Matthew S
<matthew.s.atw...@intel.com>; Kalvala, Haridhar <haridhar.kalv...@intel.com>;
Chauhan, Shekhar <shekhar.chau...@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.vodapa...@intel.com>
>>> Sent: Tuesday, December 17, 2024 11:00 PM
>>> To:intel-gfx@lists.freedesktop.org
>>> Cc: Vivekanandan, Balasubramani
>>> <balasubramani.vivekanan...@intel.com>; Roper, Matthew D
>>> <matthew.d.ro...@intel.com>; De Marchi, Lucas
>>> <lucas.demar...@intel.com>; Sousa, Gustavo<gustavo.so...@intel.com>;
>>> Taylor, Clinton A<clinton.a.tay...@intel.com>; Atwood, Matthew S
>>> <matthew.s.atw...@intel.com>; Bhadane, Dnyaneshwar
>>> <dnyaneshwar.bhad...@intel.com>; Kalvala, Haridhar
>>> <haridhar.kalv...@intel.com>; Chauhan, Shekhar
>>> <shekhar.chau...@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.vodapa...@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
>
