[v2,10/11] drm/i915: Improve PSR2 CTL macros

Submitted by José Roberto de Souza on Nov. 30, 2018, 2:25 a.m.

Details

Message ID 20181130022525.25676-10-jose.souza@intel.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in DRI devel

Not browsing as part of any series.

Commit Message

José Roberto de Souza Nov. 30, 2018, 2:25 a.m.
- Reusing the EDP_PSR2_FRAME_BEFORE_SU_SHIFT in EDP_PSR2_FRAME_BEFORE_SU
- Removing unused EDP_PSR2_FRAME_BEFORE_SU_MASK
- Adding EDP_PSR2_FRAME_BEFORE_SU_MAX
- Adding EDP_PSR2_IDLE_FRAME()
- Adding EDP_PSR2_IDLE_FRAME_MAX

In the next patch the new macros will be used.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d3ef97915455..9e46da5032c0 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4216,10 +4216,11 @@  enum {
 #define   EDP_PSR2_TP2_TIME_50us	(3 << 8)
 #define   EDP_PSR2_TP2_TIME_MASK	(3 << 8)
 #define   EDP_PSR2_FRAME_BEFORE_SU_SHIFT 4
-#define   EDP_PSR2_FRAME_BEFORE_SU_MASK	(0xf << 4)
-#define   EDP_PSR2_FRAME_BEFORE_SU(a)	((a) << 4)
-#define   EDP_PSR2_IDLE_FRAME_MASK	0xf
+#define   EDP_PSR2_FRAME_BEFORE_SU_MAX	0xf
+#define   EDP_PSR2_FRAME_BEFORE_SU(a)	((a) << EDP_PSR2_FRAME_BEFORE_SU_SHIFT)
 #define   EDP_PSR2_IDLE_FRAME_SHIFT	0
+#define   EDP_PSR2_IDLE_FRAME_MAX	0xf
+#define   EDP_PSR2_IDLE_FRAME(a)	((a) << EDP_PSR2_IDLE_FRAME_SHIFT)
 
 #define _PSR_EVENT_TRANS_A			0x60848
 #define _PSR_EVENT_TRANS_B			0x61848

Comments

On Thu, 2018-11-29 at 18:25 -0800, José Roberto de Souza wrote:
> - Reusing the EDP_PSR2_FRAME_BEFORE_SU_SHIFT in
> EDP_PSR2_FRAME_BEFORE_SU
> - Removing unused EDP_PSR2_FRAME_BEFORE_SU_MASK
> - Adding EDP_PSR2_FRAME_BEFORE_SU_MAX
> - Adding EDP_PSR2_IDLE_FRAME()
> - Adding EDP_PSR2_IDLE_FRAME_MAX
> 
> In the next patch the new macros will be used.
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index d3ef97915455..9e46da5032c0 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4216,10 +4216,11 @@ enum {
>  #define   EDP_PSR2_TP2_TIME_50us	(3 << 8)
>  #define   EDP_PSR2_TP2_TIME_MASK	(3 << 8)
>  #define   EDP_PSR2_FRAME_BEFORE_SU_SHIFT 4
> -#define   EDP_PSR2_FRAME_BEFORE_SU_MASK	(0xf << 4)
_MASKs are useful when we want to read back the register for debugging.
So, I'm not fully convinced this is an improvement.

Rodrigo, your thoughts on this?


> -#define   EDP_PSR2_FRAME_BEFORE_SU(a)	((a) << 4)
> -#define   EDP_PSR2_IDLE_FRAME_MASK	0xf
> +#define   EDP_PSR2_FRAME_BEFORE_SU_MAX	0xf
> +#define   EDP_PSR2_FRAME_BEFORE_SU(a)	((a) <<
> EDP_PSR2_FRAME_BEFORE_SU_SHIFT)
>  #define   EDP_PSR2_IDLE_FRAME_SHIFT	0
> +#define   EDP_PSR2_IDLE_FRAME_MAX	0xf
Not sure if this is better than re-using _MASK here. I'm sure there are
places in the driver where we already do that.

> +#define   EDP_PSR2_IDLE_FRAME(a)	((a) <<
> EDP_PSR2_IDLE_FRAME_SHIFT)
>  
>  #define _PSR_EVENT_TRANS_A			0x60848
>  #define _PSR_EVENT_TRANS_B			0x61848
On Mon, Dec 03, 2018 at 03:03:52PM -0800, Dhinakaran Pandiyan wrote:
> On Thu, 2018-11-29 at 18:25 -0800, José Roberto de Souza wrote:
> > - Reusing the EDP_PSR2_FRAME_BEFORE_SU_SHIFT in
> > EDP_PSR2_FRAME_BEFORE_SU
> > - Removing unused EDP_PSR2_FRAME_BEFORE_SU_MASK
> > - Adding EDP_PSR2_FRAME_BEFORE_SU_MAX
> > - Adding EDP_PSR2_IDLE_FRAME()
> > - Adding EDP_PSR2_IDLE_FRAME_MAX
> > 
> > In the next patch the new macros will be used.
> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index d3ef97915455..9e46da5032c0 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4216,10 +4216,11 @@ enum {
> >  #define   EDP_PSR2_TP2_TIME_50us	(3 << 8)
> >  #define   EDP_PSR2_TP2_TIME_MASK	(3 << 8)
> >  #define   EDP_PSR2_FRAME_BEFORE_SU_SHIFT 4
> > -#define   EDP_PSR2_FRAME_BEFORE_SU_MASK	(0xf << 4)
> _MASKs are useful when we want to read back the register for debugging.
> So, I'm not fully convinced this is an improvement.
> 
> Rodrigo, your thoughts on this?

I agree with DK.

But I also don't mind removing if we aren't using.

> 
> 
> > -#define   EDP_PSR2_FRAME_BEFORE_SU(a)	((a) << 4)
> > -#define   EDP_PSR2_IDLE_FRAME_MASK	0xf
> > +#define   EDP_PSR2_FRAME_BEFORE_SU_MAX	0xf
> > +#define   EDP_PSR2_FRAME_BEFORE_SU(a)	((a) <<
> > EDP_PSR2_FRAME_BEFORE_SU_SHIFT)
> >  #define   EDP_PSR2_IDLE_FRAME_SHIFT	0
> > +#define   EDP_PSR2_IDLE_FRAME_MAX	0xf
> Not sure if this is better than re-using _MASK here. I'm sure there are
> places in the driver where we already do that.

I think it depends on the use actually...

> 
> > +#define   EDP_PSR2_IDLE_FRAME(a)	((a) <<
> > EDP_PSR2_IDLE_FRAME_SHIFT)
> >  
> >  #define _PSR_EVENT_TRANS_A			0x60848
> >  #define _PSR_EVENT_TRANS_B			0x61848
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx