diff mbox series

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

Message ID 20241021222744.294371-9-gustavo.sousa@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/dmc_wl: Fixes and enablement for Xe3_LPD | expand

Commit Message

Gustavo Sousa Oct. 21, 2024, 10:27 p.m. UTC
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(-)

Comments

Luca Coelho Nov. 1, 2024, 12:58 p.m. UTC | #1
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.
Gustavo Sousa Nov. 5, 2024, 1:42 p.m. UTC | #2
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.
Luca Coelho Nov. 6, 2024, 12:23 p.m. UTC | #3
On Tue, 2024-11-05 at 10:42 -0300, Gustavo Sousa wrote:
> 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.

Yeah, but I think this idea of blocks is still valid.  I think it's
very unlikely that only certain _addresses_ and not full blocks of
addresses are affected in the HW.

For instance:

         /* DBUF_CTL_* */
-        { .start = 0x44300, .end = 0x44300 },
-        { .start = 0x44304, .end = 0x44304 },
-        { .start = 0x44f00, .end = 0x44f00 },
-        { .start = 0x44f04, .end = 0x44f04 },
-        { .start = 0x44fe8, .end = 0x44fe8 },

This probably means that _all_ the block, from at least 0x44300 till
0x44fff, needs to be protected.  What I'm trying to say is that even
though we don't access e.g. 0x44400, if we did, it would most likely
also have to be protected, because it's in the same block of addresses.

I guess this doesn't matter _that_ much, but it would be just cleaner
to know the actual ranges where the wakelocks are _potentially_ needed.

--
Cheers,
Luca.
Gustavo Sousa Nov. 6, 2024, 12:29 p.m. UTC | #4
Quoting Luca Coelho (2024-11-06 09:23:32-03:00)
>On Tue, 2024-11-05 at 10:42 -0300, Gustavo Sousa wrote:
>> 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.
>
>Yeah, but I think this idea of blocks is still valid.  I think it's
>very unlikely that only certain _addresses_ and not full blocks of
>addresses are affected in the HW.

Except that this is not about the hardware per se, this is about
registers that are touched by the *DMC* during DC states and that need
DC exit for properly accessing them from the driver. So, I think blocks
are not applicable here.

--
Gustavo Sousa

>
>For instance:
>
>         /* DBUF_CTL_* */
>-        { .start = 0x44300, .end = 0x44300 },
>-        { .start = 0x44304, .end = 0x44304 },
>-        { .start = 0x44f00, .end = 0x44f00 },
>-        { .start = 0x44f04, .end = 0x44f04 },
>-        { .start = 0x44fe8, .end = 0x44fe8 },
>
>This probably means that _all_ the block, from at least 0x44300 till
>0x44fff, needs to be protected.  What I'm trying to say is that even
>though we don't access e.g. 0x44400, if we did, it would most likely
>also have to be protected, because it's in the same block of addresses.
>
>I guess this doesn't matter _that_ much, but it would be just cleaner
>to know the actual ranges where the wakelocks are _potentially_ needed.
>
>--
>Cheers,
>Luca.
Luca Coelho Nov. 6, 2024, 12:35 p.m. UTC | #5
On Wed, 2024-11-06 at 09:29 -0300, Gustavo Sousa wrote:
> Quoting Luca Coelho (2024-11-06 09:23:32-03:00)
> > On Tue, 2024-11-05 at 10:42 -0300, Gustavo Sousa wrote:
> > > 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.
> > 
> > Yeah, but I think this idea of blocks is still valid.  I think it's
> > very unlikely that only certain _addresses_ and not full blocks of
> > addresses are affected in the HW.
> 
> Except that this is not about the hardware per se, this is about
> registers that are touched by the *DMC* during DC states and that need
> DC exit for properly accessing them from the driver. So, I think blocks
> are not applicable here.

Ah, okay, makes sense now. This could be explained in the awesome
comment you're planning to add, as discussed in the previous patch.

--
Cheers,
Luca.
diff mbox series

Patch

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 */
 
 	/* TRANS_CMTG_CTL_* */
-	{ .start = 0x6fa88, .end = 0x6fa88 },
-	{ .start = 0x6fb88, .end = 0x6fb88 },
-
-	{ .start = 0x46430, .end = 0x46430 }, /* CHICKEN_DCPR_1 */
-	{ .start = 0x46434, .end = 0x46434 }, /* CHICKEN_DCPR_2 */
-	{ .start = 0x454a0, .end = 0x454a0 }, /* CHICKEN_DCPR_4 */
-	{ .start = 0x42084, .end = 0x42084 }, /* CHICKEN_MISC_2 */
-	{ .start = 0x42088, .end = 0x42088 }, /* CHICKEN_MISC_3 */
-	{ .start = 0x46160, .end = 0x46160 }, /* CMTG_CLK_SEL */
+	{ .start = 0x6fa88 },
+	{ .start = 0x6fb88 },
+
+	{ .start = 0x46430 }, /* CHICKEN_DCPR_1 */
+	{ .start = 0x46434 }, /* CHICKEN_DCPR_2 */
+	{ .start = 0x454a0 }, /* CHICKEN_DCPR_4 */
+	{ .start = 0x42084 }, /* CHICKEN_MISC_2 */
+	{ .start = 0x42088 }, /* CHICKEN_MISC_3 */
+	{ .start = 0x46160 }, /* CMTG_CLK_SEL */
 	{ .start = 0x8f000, .end = 0x8ffff }, /* Main DMC registers */
 
 	{},
 };
 
 static struct intel_dmc_wl_range xe3lpd_dc3co_wl_ranges[] = {
-	{ .start = 0x454a0, .end = 0x454a0 }, /* CHICKEN_DCPR_4 */
+	{ .start = 0x454a0 }, /* CHICKEN_DCPR_4 */
 
-	{ .start = 0x45504, .end = 0x45504 }, /* DC_STATE_EN */
+	{ .start = 0x45504 }, /* DC_STATE_EN */
 
 	/* 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 = 0x46070, .end = 0x46070 }, /* CDCLK_PLL_ENABLE */
-	{ .start = 0x46000, .end = 0x46000 }, /* CDCLK_CTL */
-	{ .start = 0x46008, .end = 0x46008 }, /* CDCLK_SQUASH_CTL */
+	{ .start = 0x44300 },
+	{ .start = 0x44304 },
+	{ .start = 0x44f00 },
+	{ .start = 0x44f04 },
+	{ .start = 0x44fe8 },
+	{ .start = 0x45008 },
+
+	{ .start = 0x46070 }, /* CDCLK_PLL_ENABLE */
+	{ .start = 0x46000 }, /* CDCLK_CTL */
+	{ .start = 0x46008 }, /* CDCLK_SQUASH_CTL */
 	{ .start = 0x8f000, .end = 0x8ffff }, /* Main DMC registers */
 
 	/* Scanline registers */
-	{ .start = 0x70000, .end = 0x70000 },
-	{ .start = 0x70004, .end = 0x70004 },
-	{ .start = 0x70014, .end = 0x70014 },
-	{ .start = 0x70018, .end = 0x70018 },
-	{ .start = 0x71000, .end = 0x71000 },
-	{ .start = 0x71004, .end = 0x71004 },
-	{ .start = 0x71014, .end = 0x71014 },
-	{ .start = 0x71018, .end = 0x71018 },
-	{ .start = 0x72000, .end = 0x72000 },
-	{ .start = 0x72004, .end = 0x72004 },
-	{ .start = 0x72014, .end = 0x72014 },
-	{ .start = 0x72018, .end = 0x72018 },
-	{ .start = 0x73000, .end = 0x73000 },
-	{ .start = 0x73004, .end = 0x73004 },
-	{ .start = 0x73014, .end = 0x73014 },
-	{ .start = 0x73018, .end = 0x73018 },
-	{ .start = 0x7b000, .end = 0x7b000 },
-	{ .start = 0x7b004, .end = 0x7b004 },
-	{ .start = 0x7b014, .end = 0x7b014 },
-	{ .start = 0x7b018, .end = 0x7b018 },
-	{ .start = 0x7c000, .end = 0x7c000 },
-	{ .start = 0x7c004, .end = 0x7c004 },
-	{ .start = 0x7c014, .end = 0x7c014 },
-	{ .start = 0x7c018, .end = 0x7c018 },
+	{ .start = 0x70000 },
+	{ .start = 0x70004 },
+	{ .start = 0x70014 },
+	{ .start = 0x70018 },
+	{ .start = 0x71000 },
+	{ .start = 0x71004 },
+	{ .start = 0x71014 },
+	{ .start = 0x71018 },
+	{ .start = 0x72000 },
+	{ .start = 0x72004 },
+	{ .start = 0x72014 },
+	{ .start = 0x72018 },
+	{ .start = 0x73000 },
+	{ .start = 0x73004 },
+	{ .start = 0x73014 },
+	{ .start = 0x73018 },
+	{ .start = 0x7b000 },
+	{ .start = 0x7b004 },
+	{ .start = 0x7b014 },
+	{ .start = 0x7b018 },
+	{ .start = 0x7c000 },
+	{ .start = 0x7c004 },
+	{ .start = 0x7c014 },
+	{ .start = 0x7c018 },
 
 	{},
 };
@@ -181,7 +181,9 @@  static bool intel_dmc_wl_addr_in_range(u32 address,
 				       const struct intel_dmc_wl_range ranges[])
 {
 	for (int i = 0; ranges[i].start; i++) {
-		if (ranges[i].start <= address && address <= ranges[i].end)
+		u32 end = ranges[i].end ?: ranges[i].start;
+
+		if (ranges[i].start <= address && address <= end)
 			return true;
 	}