Re: [PATCH 08/13] drm/i915/dmc_wl: Allow simpler syntax for single reg in range tables

[Gustavo Sousa <gustavo.sousa@intel.com>]

Quoting Luca Coelho (2024-11-01 09:58:33-03:00)
>On Mon, 2024-10-21 at 19:27 -0300, Gustavo Sousa wrote:
>> Allow simpler syntax for defining entries for single registers in range
>> tables. That makes them easier to type as well as to read, allowing one
>> to quickly tell whether a range actually refers to a single register or
>> a "true range".
>> 
>> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_dmc_wl.c | 118 ++++++++++----------
>>  1 file changed, 60 insertions(+), 58 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
>> index 8bf2f32be859..6992ce654e75 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
>> @@ -54,82 +54,82 @@ static struct intel_dmc_wl_range lnl_wl_range[] = {
>>  };
>>  
>>  static struct intel_dmc_wl_range xe3lpd_dc5_dc6_wl_ranges[] = {
>> -        { .start = 0x45500, .end = 0x45500 }, /* DC_STATE_SEL */
>> +        { .start = 0x45500 }, /* DC_STATE_SEL */
>>          { .start = 0x457a0, .end = 0x457b0 }, /* DC*_RESIDENCY_COUNTER */
>> -        { .start = 0x45504, .end = 0x45504 }, /* DC_STATE_EN */
>> +        { .start = 0x45504 }, /* DC_STATE_EN */
>>          { .start = 0x45400, .end = 0x4540c }, /* PWR_WELL_CTL_* */
>> -        { .start = 0x454f0, .end = 0x454f0 }, /* RETENTION_CTRL */
>> +        { .start = 0x454f0 }, /* RETENTION_CTRL */
>>  
>>          /* DBUF_CTL_* */
>> -        { .start = 0x44300, .end = 0x44300 },
>> -        { .start = 0x44304, .end = 0x44304 },
>> -        { .start = 0x44f00, .end = 0x44f00 },
>> -        { .start = 0x44f04, .end = 0x44f04 },
>> -        { .start = 0x44fe8, .end = 0x44fe8 },
>> -        { .start = 0x45008, .end = 0x45008 },
>> +        { .start = 0x44300 },
>> +        { .start = 0x44304 },
>> +        { .start = 0x44f00 },
>> +        { .start = 0x44f04 },
>> +        { .start = 0x44fe8 },
>> +        { .start = 0x45008 },
>>  
>> -        { .start = 0x46070, .end = 0x46070 }, /* CDCLK_PLL_ENABLE */
>> -        { .start = 0x46000, .end = 0x46000 }, /* CDCLK_CTL */
>> -        { .start = 0x46008, .end = 0x46008 }, /* CDCLK_SQUASH_CTL */
>> +        { .start = 0x46070 }, /* CDCLK_PLL_ENABLE */
>> +        { .start = 0x46000 }, /* CDCLK_CTL */
>> +        { .start = 0x46008 }, /* CDCLK_SQUASH_CTL */
>
>Many of these are probably actually ranges.  I'm not a HW guy, but
>these are probably blocks that need the wakelock and it just happens
>that many of those addresses are actually not used, but would need a
>wakelock if they were used?
>
>IOW, e.g. all these DBUF_CTL registers are probably in the same range
>that needs wakelocks (i.e. 0x44300-0x46fff)? Do we really need to
>define many of these individually?
>
>This is related to the previous patch as well, but I decided to comment
>it here because it becomes clearer.

Maybe my reply on the previous patch clarifies this? I.e., these
offset or offset ranges represent offsets that the DMC touches when on
specific DC states.

--
Gustavo Sousa

>
>--
>Cheers,
>Luca.