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

[Luca Coelho <luca@coelho.fi>]

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.

--
Cheers,
Luca.