Re: [PATCH 4/5] drm/i915: Add PSR2 selective update status registers and bits definitions

From: "Souza, Jose" <jose.souza@intel.com>
To: "intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
Cc: "Vivi, Rodrigo" <rodrigo.vivi@intel.com>
Subject: Re: [PATCH 4/5] drm/i915: Add PSR2 selective update status registers and bits definitions
Date: Tue, 11 Dec 2018 14:20:07 +0000	[thread overview]
Message-ID: <e64e70f6cdfb0c3ed3b822e6cdf2088a838bd6cf.camel@intel.com> (raw)
In-Reply-To: <97444c13d54364c806f475c18d8317c65b06aeca.camel@intel.com>


[-- Attachment #1.1: Type: text/plain, Size: 2030 bytes --]

On Mon, 2018-12-10 at 23:51 -0800, Dhinakaran Pandiyan wrote:
> On Tue, 2018-12-04 at 15:00 -0800, José Roberto de Souza wrote:
> > This register contains how many blocks was sent in the past
> > selective
> > updates.
> > Those registers are not kept set all the times but pulling it after
> > flip
> > can show that the expected values are set for the current frame and
> > the
> > previous ones too.
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 0a7d60509ca7..7d634f34ca7d 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4248,6 +4248,12 @@ enum {
> >  #define EDP_PSR2_STATUS_STATE_MASK     (0xf << 28)
> >  #define EDP_PSR2_STATUS_STATE_SHIFT    28
> >  
> > +#define EDP_PSR2_SU_STATUS					_MMIO(0
> > x6f914)
> > +#define EDP_PSR2_SU_STATUS2					
> > _MMIO(0
> > x6F918)
> > +#define EDP_PSR2_SU_STATUS3					
> > _MMIO(0
> > x6F91C)
> > +#define  EDP_PSR2_SU_STATUS_NUM_SU_BLOCKS_IN_FRAME_SHIFT(i)	
> > ((i) *
> > 10)
> > +#define  EDP_PSR2_SU_STATUS_NUM_SU_BLOCKS_IN_FRAME_MASK(i)	(0x3FF
> > << ((i) * 10))
> How about moving the MMIO selection logic to the macros? 
> 
> #define PSR2_SU_HISTORY 8
> #define _PSR2_SU_STATUS_0 0x6f914
> #define _PSR2_SU_STATUS_1 0x6f918
> #define _PSR2_SU_STATUS(dword) _MMIO(_PICK_EVEN((dword),\
> _PSR2_SU_STATUS_0, _PSR2_SU_STATUS_1))
> #define PSR2_SU_SHIFT(frame) ((frame) % 3) * 10
> #define PSR2_SU_MASK(frame)  (0x3ff << PSR2_SU_SHIFT(frame))
> #define PSR2_SU_BLOCKS(frame) _PSR2_SU_STATUS((frame) / 3)

Looks better

> 
> 
> > +
> >  /* VGA port control */
> >  #define ADPA			_MMIO(0x61100)
> >  #define PCH_ADPA                _MMIO(0xe1100)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx