[Intel-gfx] [PATCH 4/6] drm/i915: Add PSR2 software tracking registers
Souza, Jose
jose.souza at intel.com
Thu Jun 25 21:41:05 UTC 2020
On Mon, 2020-06-15 at 19:23 +0000, Souza, Jose wrote:
> On Mon, 2020-06-15 at 19:37 +0100, Mun, Gwan-gyeong wrote:
> > On Fri, 2020-06-12 at 21:49 +0000, Mun, Gwan-gyeong wrote:
> > > On Fri, 2020-06-12 at 14:18 -0700, Souza, Jose wrote:
> > > > On Fri, 2020-06-12 at 21:57 +0100, Mun, Gwan-gyeong wrote:
> > > > > On Tue, 2020-05-26 at 15:14 -0700, José Roberto de Souza wrote:
> > > > > > This registers will be used to implement PSR2 software
> > > > > > tracking.
> > > > > >
> > > > > > BSpec: 55229
> > > > > > BSpec: 50424
> > > > > > BSpec: 50420
> > > > > > Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> > > > > > ---
> > > > > > drivers/gpu/drm/i915/i915_reg.h | 68
> > > > > > ++++++++++++++++++++++++++++++-
> > > > > > --
> > > > > > 1 file changed, 63 insertions(+), 5 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > > > > b/drivers/gpu/drm/i915/i915_reg.h
> > > > > > index e9d50fe0f375..6f547e459d30 100644
> > > > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > > > @@ -4566,6 +4566,18 @@ enum {
> > > > > > #define PSR2_SU_STATUS_MASK(frame) (0x3ff <<
> > > > > > PSR2_SU_STATUS_SHIFT(frame))
> > > > > > #define PSR2_SU_STATUS_FRAMES 8
> > > > > >
> > > > > > +#define _PSR2_MAN_TRK_CTL_A 0x60910
> > > > > > +#define _PSR2_MAN_TRK_CTL_EDP 0x6f910
> > > > > > +#define PSR2_MAN_TRK_CTL(tran) _MMIO_T
> > > > > > RANS2(tran, _PSR2_MAN_TRK_CTL_A)
> > > > > > +#define PSR2_MAN_TRK_CTL_ENABLE REG_BIT
> > > > > > (31)
> > > > > > +#define PSR2_MAN_TRK_CTL_REGION_START_ADDR_MASK REG_GEN
> > > > > > MASK(30,
> > > > > > 21)
> > > > > > +#define PSR2_MAN_TRK_CTL_REGION_START_ADDR(val) REG_FIE
> > > > > > LD_PREP(
> > > > > > PSR2_MAN_TRK_CTL_REGION_START_ADDR_MASK, val)
> > > > > > +#define PSR2_MAN_TRK_CTL_REGION_END_ADDR_MASK REG_GEN
> > > > > > MASK(20, 11)
> > > > > > +#define PSR2_MAN_TRK_CTL_REGION_END_ADDR(val) REG_FIE
> > > > > > LD_PREP(PSR2_MAN_TRK_CTL_REGION_END_ADDR_MASK, val)
> > > > > > +#define PSR2_MAN_TRK_CTL_SINGLE_FULL_FRAME REG_BIT
> > > > > > (3)
> > > > > > +#define PSR2_MAN_TRK_CTL_CONTINUOS_FULL_FRAME REG_BIT
> > > > > > (2)
> > > > > > +#define PSR2_MAN_TRK_CTL_PARTIAL_FRAME_UPDATE REG_BIT
> > > > > > (1)
> > > > > > +
> > > > > As per Bspec, it would be better that the names of bit as below.
> > > > >
> > > > > PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME
> > > > > PSR2_MAN_TRK_CTL_SF_CONTINUOS_FULL_FRAME
> > > > > PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_ENABLE
> > > >
> > > > No problem in naming like this but MAN_TRK and SF is kind of
> > > > redundant and the name was already big.
> > > > Your call.
> > > >
> > > > > > /* VGA port control */
> > > > > > #define ADPA _MMIO(0x61100)
> > > > > > #define PCH_ADPA _MMIO(0xe1100)
> > > > > > @@ -7129,7 +7141,52 @@ enum {
> > > > > > #define PLANE_COLOR_CTL(pipe, plane) \
> > > > > > _MMIO_PLANE(plane, _PLANE_COLOR_CTL_1(pipe),
> > > > > > _PLANE_COLOR_CTL_2(pipe))
> > > > > >
> > > > > > -#/* SKL new cursor registers */
> > > > > > +#define _PLANE_SEL_FETCH_BASE_1_A 0x70890
> > > > > > +#define _PLANE_SEL_FETCH_BASE_2_A 0x708B0
> > > > > > +#define _PLANE_SEL_FETCH_BASE_3_A 0x708D0
> > > > > > +#define _PLANE_SEL_FETCH_BASE_4_A 0x708F0
> > > > > > +#define _PLANE_SEL_FETCH_BASE_5_A 0x70920
> > > > > > +#define _PLANE_SEL_FETCH_BASE_6_A 0x70940
> > > > > > +#define _PLANE_SEL_FETCH_BASE_7_A 0x70960
> > > > > > +#define _PLANE_SEL_FETCH_BASE_CUR_A 0x70880
> > > > > > +#define _PLANE_SEL_FETCH_BASE_1_B 0x70990
> > > > > > +
> > > > > And as per Bspec, the prefix _SEL_FETCH_PLANE_ is better than
> > > > > _PLANE_SEL_FETCH_ .
> > > > You mean just for the "internal" ones? For PLANE_SEL_FETCH_CTL,
> > > > PLANE_SEL_FETCH_SIZE... would be better keep like this to match
> > > > other
> > > > plane register
> > > > names.
> > > Internals and externals. I also noticed your intention (match other
> > > plane related registers), but when I checked other plane related
> > > resiters, they followed bspec names. (But I am not confident on
> > > register naming policy; we always have to follow documented register
> > > names or not. )
I don't think we are that restrict as there is several registers that don't match.
Will update the internal ones, anyone searching by BSpec name will find the internal ones and reach the exported ones.
> > > > > > +#define _PLANE_SEL_FETCH_BASE_A(plane) _PICK(plane, \
> > > > > > + _PLANE_SEL_FETCH_B
> > > > > > ASE_1_A,
> > > > > > \
> > > > > > + _PLANE_SEL_FETCH_B
> > > > > > ASE_2_A,
> > > > > > \
> > > > > > + _PLANE_SEL_FETCH_B
> > > > > > ASE_3_A,
> > > > > > \
> > > > > > + _PLANE_SEL_FETCH_B
> > > > > > ASE_4_A,
> > > > > > \
> > > > > > + _PLANE_SEL_FETCH_B
> > > > > > ASE_5_A,
> > > > > > \
> > > > > > + _PLANE_SEL_FETCH_B
> > > > > > ASE_6_A,
> > > > > > \
> > > > > > + _PLANE_SEL_FETCH_B
> > > > > > ASE_7_A,
> > > > > > \
> > > > > > + _PLANE_SEL_FETCH_B
> > > > > > ASE_CUR_
> > > > > > A)
> > > > > > +#define _PLANE_SEL_FETCH_BASE_1(pipe) _PIPE(pipe,
> > > > > > _PLANE_SEL_FETCH_BASE_1_A, _PLANE_SEL_FETCH_BASE_1_A)
> >
> > It seems that indicates an wrong register name.
> > IMHO, is it your intention like this? " #define
> > _PLANE_SEL_FETCH_BASE_1(pipe) _PIPE(pipe, _PLANE_SEL_FETCH_BASE_1_A,
> > _PLANE_SEL_FETCH_BASE_1_B) "?
>
> Yes, it should be _PLANE_SEL_FETCH_BASE_1_B, thanks for catching this.
> Will send this 4 patches in a few days with the requested fixes.
>
> > > > > > +#define PLANE_SEL_FETCH_BASE(pipe, plane)
> > > > > > (_PLANE_SEL_FETCH_BASE_1(pipe) - \
> > > > > > + _PLANE_SEL_FETCH_BAS
> > > > > > E_1_A +
> > > > > > \
> > > > > > + _PLANE_SEL_FETCH_BAS
> > > > > > E_A(plan
> > > > > > e))
> > > > > > +
> > > > > > +#define _PLANE_SEL_FETCH_CTL_1_A 0x70890
> > > > > > +#define PLANE_SEL_FETCH_CTL(pipe, plane)
> > > > > > _MMIO(PLANE_SEL_FETCH_BASE(pipe, plane) + \
> > > > > > + _PLANE_SEL_FETCH
> > > > > > _CTL_1_A
> > > > > > - \
> > > > > > + _PLANE_SEL_FETCH
> > > > > > _BASE_1_
> > > > > > A)
> > > > > > +#define PLANE_SET_FETCH_CTL_ENABLE REG_BIT(31)
> > > > > > +
> > > > > > +#define _PLANE_SEL_FETCH_POS_1_A 0x70894
> > > > > > +#define PLANE_SEL_FETCH_POS(pipe, plane)
> > > > > > _MMIO(PLANE_SEL_FETCH_BASE(pipe, plane) + \
> > > > > > + _PLANE_SEL_FETCH
> > > > > > _POS_1_A
> > > > > > - \
> > > > > > + _PLANE_SEL_FETCH
> > > > > > _BASE_1_
> > > > > > A)
> > > > > > +
> > > > > > +#define _PLANE_SEL_FETCH_SIZE_1_A 0x70898
> > > > > > +#define PLANE_SEL_FETCH_SIZE(pipe, plane)
> > > > > > _MMIO(PLANE_SEL_FETCH_BASE(pipe, plane) + \
> > > > > > + _PLANE_SEL_FETC
> > > > > > H_SIZE_1
> > > > > > _A - \
> > > > > > + _PLANE_SEL_FETC
> > > > > > H_BASE_1
> > > > > > _A)
> > > > > > +
> > > > > > +#define _PLANE_SEL_FETCH_OFFSET_1_A 0x7089C
> > > > > > +#define PLANE_SEL_FETCH_OFFSET(pipe, plane)
> > > > > > _MMIO(PLANE_SEL_FETCH_BASE(pipe, plane) + \
> > > > > > + _PLANE_SEL_FE
> > > > > > TCH_OFFS
> > > > > > ET_1_A - \
> > > > > > + _PLANE_SEL_FE
> > > > > > TCH_BASE
> > > > > > _1_A)
> > > > > > +
> > > > > > +/* SKL new cursor registers */
> > > > > > #define _CUR_BUF_CFG_A 0x7017c
> > > > > > #define _CUR_BUF_CFG_B 0x7117c
> > > > > > #define CUR_BUF_CFG(pipe) _MMIO_PIPE(pipe,
> > > > > > _CUR_BUF_CFG_A,
> > > > > > _CUR_BUF_CFG_B)
> > > > > > @@ -7775,11 +7832,12 @@ enum {
> > > > > > # define CHICKEN3_DGMG_REQ_OUT_FIX_DISABLE (1 << 5)
> > > > > > # define CHICKEN3_DGMG_DONE_FIX_DISABLE (1 <<
> > > > > > 2)
> > > > > >
> > > > > > -#define CHICKEN_PAR1_1 _MMIO(0x42080)
> > > > > > +#define CHICKEN_PAR1_1 _MMIO(0x42080)
> > > > > > #define SKL_DE_COMPRESSED_HASH_MODE (1 << 15)
> > > > > > -#define DPA_MASK_VBLANK_SRD (1 << 15)
> > > > > > -#define FORCE_ARB_IDLE_PLANES (1 << 14)
> > > > > > -#define SKL_EDP_PSR_FIX_RDWRAP (1 << 3)
> > > > > > +#define DPA_MASK_VBLANK_SRD (1 << 15)
> > > > > > +#define FORCE_ARB_IDLE_PLANES (1 << 14)
> > > > > > +#define SKL_EDP_PSR_FIX_RDWRAP (1 << 3)
> > > > > > +#define IGNORE_PSR2_HW_TRACKING (1 << 1)
> > > > > >
> > > > > > #define CHICKEN_PAR2_1 _MMIO(0x42090)
> > > > > > #define KVM_CONFIG_CHANGE_NOTIFICATION_SELECT (1 << 14)
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
More information about the Intel-gfx
mailing list