On Wed, 2019-04-03 at 16:35 -0700, José Roberto de Souza wrote: > PSR registers are a mess, some have the full address while others just > have the additional offset from psr_mmio_base. > > psr_mmio_base is nothing more than TRANSCODER_EDP_OFFSET + 0x800 and > using it makes more difficult for people with an PSR register address > from BSpec to search the register name in i915 as also the BSpec name > don't match with the name in i915. > > The other option would be use the whole hard-coded address but this is > not future proof, so here going in the middle ground by making every > PSR register relative to transcoder(that is EDP transcoder), the only > exception is PSR_IMR/IIR that is not relative to nothing. > For the _TRANS2() macros to work it needs the address of the register > from the TRANSCODER_A, so adding it to every register together with > the register address from the EDP transcoder so it will make easy for > people searching with BSpec address also adding those with the BSpec > name. > > For Haswell all the PSR register are relative to 0x64000, so > mmio_base_adjust was added and used to take care of that. > > Also removing BDW_EDP_PSR_BASE from GVT because it is not used as > the only PSR register that GVT have is this one(SRD/PSR_CTL). > > Cc: Dhinakaran Pandiyan <dhinakaran.pandi...@intel.com> > Cc: Rodrigo Vivi <rodrigo.v...@intel.com> > Cc: Jani Nikula <jani.nik...@linux.intel.com> > Cc: Ville Syrjälä <ville.syrj...@linux.intel.com> > Signed-off-by: José Roberto de Souza <jose.so...@intel.com> > --- > drivers/gpu/drm/i915/gvt/handlers.c | 1 - > drivers/gpu/drm/i915/i915_drv.h | 5 ++- > drivers/gpu/drm/i915/i915_reg.h | 59 ++++++++++++++++++++--------- > drivers/gpu/drm/i915/intel_psr.c | 11 ++++-- > 4 files changed, 52 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gvt/handlers.c > b/drivers/gpu/drm/i915/gvt/handlers.c > index 86761b1def1e..d09b798e93cb 100644 > --- a/drivers/gpu/drm/i915/gvt/handlers.c > +++ b/drivers/gpu/drm/i915/gvt/handlers.c > @@ -2739,7 +2739,6 @@ static int init_broadwell_mmio_info(struct intel_gvt > *gvt) > MMIO_D(CHICKEN_PIPESL_1(PIPE_C), D_BDW_PLUS); > > MMIO_D(WM_MISC, D_BDW); > - MMIO_D(_MMIO(BDW_EDP_PSR_BASE), D_BDW); > > MMIO_D(_MMIO(0x6671c), D_BDW_PLUS); > MMIO_D(_MMIO(0x66c00), D_BDW_PLUS); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 8f38d03b1c4e..9ce46a7dabfd 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -501,6 +501,8 @@ struct i915_drrs { > }; > > struct i915_psr { > + /* different than zero only on HSW see _TRANS2_PSR() for more info */ > + u32 mmio_base_adjust; > struct mutex lock; > > #define I915_PSR_DEBUG_MODE_MASK 0x0f > @@ -515,6 +517,7 @@ struct i915_psr { > bool enabled; > struct intel_dp *dp; > enum pipe pipe; > + enum transcoder transcoder; > bool active; > struct work_struct work; > unsigned busy_frontbuffer_bits; > @@ -1541,8 +1544,6 @@ struct drm_i915_private { > /* MMIO base address for MIPI regs */ > u32 mipi_mmio_base; > > - u32 psr_mmio_base; > - > u32 pps_mmio_base; > > wait_queue_head_t gmbus_wait_queue; > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 18e2991b376d..4df56c118cd2 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -250,9 +250,10 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) > #define _MMIO_PIPE2(pipe, reg) _MMIO(INTEL_INFO(dev_priv)- > >pipe_offsets[pipe] - \ > INTEL_INFO(dev_priv)- > >pipe_offsets[PIPE_A] + (reg) + \ > DISPLAY_MMIO_BASE(dev_priv)) > -#define _MMIO_TRANS2(pipe, reg) _MMIO(INTEL_INFO(dev_priv)- > >trans_offsets[(pipe)] - \ > - INTEL_INFO(dev_priv)- > >trans_offsets[TRANSCODER_A] + (reg) + \ > - DISPLAY_MMIO_BASE(dev_priv)) > +#define _TRANS2(trans, reg) (INTEL_INFO(dev_priv)- > >trans_offsets[(trans)] - \ > + INTEL_INFO(dev_priv)- > >trans_offsets[TRANSCODER_A] + (reg) + \ > + DISPLAY_MMIO_BASE(dev_priv)) > +#define _MMIO_TRANS2(trans, reg) _MMIO(_TRANS2(trans, reg)) > #define _CURSOR2(pipe, reg) _MMIO(INTEL_INFO(dev_priv)- > >cursor_offsets[(pipe)] - \ > INTEL_INFO(dev_priv)- > >cursor_offsets[PIPE_A] + (reg) + \ > DISPLAY_MMIO_BASE(dev_priv)) > @@ -4210,9 +4211,15 @@ enum { > #define PIPE_MULT(trans) _MMIO_TRANS2(trans, _PIPE_MULT_A) > > /* HSW+ eDP PSR registers */ > -#define HSW_EDP_PSR_BASE 0x64800 > -#define BDW_EDP_PSR_BASE 0x6f800 > -#define EDP_PSR_CTL _MMIO(dev_priv->psr_mmio_base + > 0) > +#define HSW_EDP_PSR_BASE 0x64000 > + > +/* PSR registers on HSW is not relative to eDP transcoder */ > +#define _TRANS2_PSR(reg) (_TRANS2(dev_priv->psr.transcoder, (reg)) - > dev_priv->psr.mmio_base_adjust)
The error interrupt registers are still hardcoded for eDP, aren't they? > +#define _MMIO_TRANS2_PSR(reg) _MMIO(_TRANS2_PSR(reg)) > + > +#define _SRD_CTL_A 0x60800 > +#define _SRD_CTL_EDP 0x6F800 > +#define EDP_PSR_CTL _MMIO_TRANS2_PSR(_SRD_CTL_A) Why isn't this accepting transcoder/pipe as an argument? dev_priv- >psr.transcoder is hidden two levels below than it should be. > #define EDP_PSR_ENABLE (1 << 31) > #define BDW_PSR_SINGLE_FRAME (1 << 30) > #define EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK (1 << 29) /* SW can't > modify */ > @@ -4245,16 +4252,22 @@ enum { > #define EDP_PSR_POST_EXIT (1 << 1) > #define EDP_PSR_PRE_ENTRY (1 << 0) > > -#define EDP_PSR_AUX_CTL _MMIO(dev_priv- > >psr_mmio_base + 0x10) > +#define _SRD_AUX_CTL_A 0x60810 > +#define _SRD_AUX_CTL_EDP 0x6F810 > +#define EDP_PSR_AUX_CTL _MMIO_TRANS2_PSR(_SRD_AU > X_CTL_A) > #define EDP_PSR_AUX_CTL_TIME_OUT_MASK (3 << 26) > #define EDP_PSR_AUX_CTL_MESSAGE_SIZE_MASK (0x1f << 20) > #define EDP_PSR_AUX_CTL_PRECHARGE_2US_MASK (0xf << 16) > #define EDP_PSR_AUX_CTL_ERROR_INTERRUPT (1 << 11) > #define EDP_PSR_AUX_CTL_BIT_CLOCK_2X_MASK (0x7ff) > > -#define EDP_PSR_AUX_DATA(i) _MMIO(dev_priv->psr_mmio_base + > 0x14 + (i) * 4) /* 5 registers */ > +#define _SRD_AUX_DATA_A 0x60814 > +#define _SRD_AUX_DATA_EDP 0x6F814 > +#define EDP_PSR_AUX_DATA(i) _MMIO(_TRANS2_PSR(_SRD_AUX_DATA_ > A) + (i) + 4) /* 5 registers */ > > -#define EDP_PSR_STATUS _MMIO(dev_priv- > >psr_mmio_base + 0x40) > +#define _SRD_STATUS_A 0x60840 > +#define _SRD_STATUS_EDP 0x6F840 > +#define EDP_PSR_STATUS _MMIO_TRANS2_PSR(_SRD_ST > ATUS_A) > #define EDP_PSR_STATUS_STATE_MASK (7 << 29) > #define EDP_PSR_STATUS_STATE_SHIFT 29 > #define EDP_PSR_STATUS_STATE_IDLE (0 << 29) > @@ -4279,10 +4292,15 @@ enum { > #define EDP_PSR_STATUS_SENDING_TP1 (1 << 4) > #define EDP_PSR_STATUS_IDLE_MASK 0xf > > -#define EDP_PSR_PERF_CNT _MMIO(dev_priv->psr_mmio_base + 0x44) > +#define _SRD_PERF_CNT_A 0x60844 > +#define _SRD_PERF_CNT_EDP 0x6F844 > +#define EDP_PSR_PERF_CNT _MMIO_TRANS2_PSR(_SRD_PERF_CNT_A) > #define EDP_PSR_PERF_CNT_MASK 0xffffff > > -#define EDP_PSR_DEBUG _MMIO(dev_priv- > >psr_mmio_base + 0x60) /* PSR_MASK on SKL+ */ > +/* PSR_MASK on SKL+ */ > +#define _SRD_DEBUG_A 0x60860 > +#define _SRD_DEBUG_EDP 0x6F860 > +#define EDP_PSR_DEBUG _MMIO_TRANS2_PSR(_SRD_DE > BUG_A) > #define EDP_PSR_DEBUG_MASK_MAX_SLEEP (1 << 28) > #define EDP_PSR_DEBUG_MASK_LPSP (1 << 27) > #define EDP_PSR_DEBUG_MASK_MEMUP (1 << 26) > @@ -4290,7 +4308,9 @@ enum { > #define EDP_PSR_DEBUG_MASK_DISP_REG_WRITE (1 << 16) /* Reserved in ICL+ > */ > #define EDP_PSR_DEBUG_EXIT_ON_PIXEL_UNDERRUN (1 << 15) /* SKL+ */ > > -#define EDP_PSR2_CTL _MMIO(0x6f900) > +#define _PSR2_CTL_A 0x60900 > +#define _PSR2_CTL_EDP 0x6F900 > +#define EDP_PSR2_CTL _MMIO_TRANS2_PSR(_PSR2_CTL_A) > #define EDP_PSR2_ENABLE (1 << 31) > #define EDP_SU_TRACK_ENABLE (1 << 30) > #define EDP_Y_COORDINATE_VALID (1 << 26) /* GLK and CNL+ */ > @@ -4308,7 +4328,9 @@ enum { > #define EDP_PSR2_IDLE_FRAME_MASK 0xf > #define EDP_PSR2_IDLE_FRAME_SHIFT 0 > > -#define PSR_EVENT _MMIO(0x6F848) > +#define _PSR_EVENT_A 0x60848 > +#define _PSR_EVENT_EDP 0x6F848 > +#define PSR_EVENT _MMIO_TRANS2_PSR(_PSR_EVENT_A) > #define PSR_EVENT_PSR2_WD_TIMER_EXPIRE (1 << 17) > #define PSR_EVENT_PSR2_DISABLED (1 << 16) > #define PSR_EVENT_SU_DIRTY_FIFO_UNDERRUN (1 << 15) > @@ -4326,14 +4348,15 @@ enum { > #define PSR_EVENT_LPSP_MODE_EXIT (1 << 1) > #define PSR_EVENT_PSR_DISABLE (1 << 0) > > -#define EDP_PSR2_STATUS _MMIO(0x6f940) > +#define _PSR2_STATUS_A 0x60940 > +#define _PSR2_STATUS_EDP 0x6F940 > +#define EDP_PSR2_STATUS _MMIO_TRANS2_PSR(_PSR2_STATUS_A) > #define EDP_PSR2_STATUS_STATE_MASK (0xf << 28) > #define EDP_PSR2_STATUS_STATE_SHIFT 28 > > -#define _PSR2_SU_STATUS_0 0x6F914 > -#define _PSR2_SU_STATUS_1 0x6F918 > -#define _PSR2_SU_STATUS_2 0x6F91C > -#define _PSR2_SU_STATUS(index) _MMIO(_PICK_EVEN((index), > _PSR2_SU_STATUS_0, _PSR2_SU_STATUS_1)) > +#define _PSR2_SU_STATUS_A 0x60914 > +#define _PSR2_SU_STATUS_EDP 0x6F914 > +#define _PSR2_SU_STATUS(index) _MMIO(_TRANS2_PSR(_PSR2_SU_STATU > S_A) + (index) * 4) > #define PSR2_SU_STATUS(frame) (_PSR2_SU_STATUS((frame) / 3)) > #define PSR2_SU_STATUS_SHIFT(frame) (((frame) % 3) * 10) > #define PSR2_SU_STATUS_MASK(frame) (0x3ff << PSR2_SU_STATUS_SHIFT(frame)) > diff --git a/drivers/gpu/drm/i915/intel_psr.c > b/drivers/gpu/drm/i915/intel_psr.c > index b984e005b72e..ba88464cbbe8 100644 > --- a/drivers/gpu/drm/i915/intel_psr.c > +++ b/drivers/gpu/drm/i915/intel_psr.c > @@ -668,6 +668,14 @@ static void intel_psr_enable_locked(struct > drm_i915_private *dev_priv, > dev_priv->psr.psr2_enabled = intel_psr2_enabled(dev_priv, crtc_state); > dev_priv->psr.busy_frontbuffer_bits = 0; > dev_priv->psr.pipe = to_intel_crtc(crtc_state->base.crtc)->pipe; > + dev_priv->psr.transcoder = crtc_state->cpu_transcoder; > + > + if (IS_HASWELL(dev_priv)) { > + u32 trans_offset = INTEL_INFO(dev_priv)->trans_offsets[dev_priv- > >psr.transcoder]; > + > + WARN_ON(trans_offset < HSW_EDP_PSR_BASE); > + dev_priv->psr.mmio_base_adjust = trans_offset - > HSW_EDP_PSR_BASE; > + } > > DRM_DEBUG_KMS("Enabling PSR%s\n", > dev_priv->psr.psr2_enabled ? "2" : "1"); > @@ -1132,9 +1140,6 @@ void intel_psr_init(struct drm_i915_private *dev_priv) > if (!HAS_PSR(dev_priv)) > return; > > - dev_priv->psr_mmio_base = IS_HASWELL(dev_priv) ? > - HSW_EDP_PSR_BASE : BDW_EDP_PSR_BASE; > - > if (!dev_priv->psr.sink_support) > return; > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx