| Message ID | 20240516135622.3498-5-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/i915: Plane register cleanups | expand |
On Thu, 16 May 2024, Ville Syrjala <ville.syrjala@linux.intel.com> wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > PIPESRC_ERLY_TPT is a pipe register, and it lives in the 0x70000 range. > so using _MMIO_TRANS2() for it is not really correct. Also since this > is a pipe register, and not present on CHV, the registers will be > equally spaced out, so we can use the simpler _MMIO_PIPE() instead > of _MMIO_PIPE2(). > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/display/intel_cursor.c | 2 +- > drivers/gpu/drm/i915/display/intel_psr.c | 2 +- > drivers/gpu/drm/i915/display/intel_psr_regs.h | 4 ++-- > 3 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c > index b44809899502..7983cbaf83f7 100644 > --- a/drivers/gpu/drm/i915/display/intel_cursor.c > +++ b/drivers/gpu/drm/i915/display/intel_cursor.c > @@ -525,7 +525,7 @@ static void wa_16021440873(struct intel_plane *plane, > > intel_de_write_fw(dev_priv, SEL_FETCH_CUR_CTL(pipe), ctl); > > - intel_de_write(dev_priv, PIPE_SRCSZ_ERLY_TPT(dev_priv, pipe), > + intel_de_write(dev_priv, PIPE_SRCSZ_ERLY_TPT(pipe), > PIPESRC_HEIGHT(et_y_position)); > } > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c > index df0d14a5023f..d49e869f6be2 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr.c > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > @@ -2381,7 +2381,7 @@ void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_st > if (!crtc_state->enable_psr2_su_region_et) > return; > > - intel_de_write(dev_priv, PIPE_SRCSZ_ERLY_TPT(dev_priv, crtc->pipe), > + intel_de_write(dev_priv, PIPE_SRCSZ_ERLY_TPT(crtc->pipe), > crtc_state->pipe_srcsz_early_tpt); > } > > diff --git a/drivers/gpu/drm/i915/display/intel_psr_regs.h b/drivers/gpu/drm/i915/display/intel_psr_regs.h > index e14cb48f2614..47e3a2e2977c 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr_regs.h > +++ b/drivers/gpu/drm/i915/display/intel_psr_regs.h > @@ -248,8 +248,8 @@ > > /* PSR2 Early transport */ > #define _PIPE_SRCSZ_ERLY_TPT_A 0x70074 > - > -#define PIPE_SRCSZ_ERLY_TPT(dev_priv, trans) _MMIO_TRANS2(dev_priv, trans, _PIPE_SRCSZ_ERLY_TPT_A) > +#define _PIPE_SRCSZ_ERLY_TPT_B 0x71074 > +#define PIPE_SRCSZ_ERLY_TPT(pipe) _MMIO_PIPE((pipe), _PIPE_SRCSZ_ERLY_TPT_A, _PIPE_SRCSZ_ERLY_TPT_B) > > #define _SEL_FETCH_PLANE_BASE_1_A 0x70890 > #define _SEL_FETCH_PLANE_BASE_2_A 0x708B0
On Mon, 20 May 2024, Jani Nikula <jani.nikula@linux.intel.com> wrote: > On Thu, 16 May 2024, Ville Syrjala <ville.syrjala@linux.intel.com> wrote: >> From: Ville Syrjälä <ville.syrjala@linux.intel.com> >> >> PIPESRC_ERLY_TPT is a pipe register, and it lives in the 0x70000 range. >> so using _MMIO_TRANS2() for it is not really correct. Also since this >> is a pipe register, and not present on CHV, the registers will be >> equally spaced out, so we can use the simpler _MMIO_PIPE() instead >> of _MMIO_PIPE2(). >> >> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Reviewed-by: Jani Nikula <jani.nikula@intel.com> Side note, while reviewing this I found this monstrosity: static bool intel_psr2_sel_fetch_config_valid(struct intel_dp *intel_dp, struct intel_crtc_state *crtc_state) { struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); if (!dev_priv->display.params.enable_psr2_sel_fetch && intel_dp->psr.debug != I915_PSR_DEBUG_ENABLE_SEL_FETCH) { drm_dbg_kms(&dev_priv->drm, "PSR2 sel fetch not enabled, disabled by parameter\n"); return false; } if (crtc_state->uapi.async_flip) { drm_dbg_kms(&dev_priv->drm, "PSR2 sel fetch not enabled, async flip enabled\n"); return false; } return crtc_state->enable_psr2_sel_fetch = true; ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ } Judging by name, a predicate function to check if config is valid, actually modifies the config in what looks like a typoed return statement. Ugh. BR, Jani.
On Mon, 2024-05-20 at 12:37 +0300, Jani Nikula wrote: > On Mon, 20 May 2024, Jani Nikula <jani.nikula@linux.intel.com> wrote: > > On Thu, 16 May 2024, Ville Syrjala <ville.syrjala@linux.intel.com> > > wrote: > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > PIPESRC_ERLY_TPT is a pipe register, and it lives in the 0x70000 > > > range. > > > so using _MMIO_TRANS2() for it is not really correct. Also since > > > this > > > is a pipe register, and not present on CHV, the registers will be > > > equally spaced out, so we can use the simpler _MMIO_PIPE() > > > instead > > > of _MMIO_PIPE2(). > > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Reviewed-by: Jani Nikula <jani.nikula@intel.com> > > Side note, while reviewing this I found this monstrosity: > > static bool intel_psr2_sel_fetch_config_valid(struct intel_dp > *intel_dp, > struct intel_crtc_state > *crtc_state) > { > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > > if (!dev_priv->display.params.enable_psr2_sel_fetch && > intel_dp->psr.debug != I915_PSR_DEBUG_ENABLE_SEL_FETCH) { > drm_dbg_kms(&dev_priv->drm, > "PSR2 sel fetch not enabled, disabled by > parameter\n"); > return false; > } > > if (crtc_state->uapi.async_flip) { > drm_dbg_kms(&dev_priv->drm, > "PSR2 sel fetch not enabled, async flip > enabled\n"); > return false; > } > > return crtc_state->enable_psr2_sel_fetch = true; > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > } > > Judging by name, a predicate function to check if config is valid, > actually modifies the config in what looks like a typoed return > statement. Ugh. Yes, I have inhaled this already enough that it begun to look like normal. BR, Jouni Högander > > BR, > Jani. > >
diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c index b44809899502..7983cbaf83f7 100644 --- a/drivers/gpu/drm/i915/display/intel_cursor.c +++ b/drivers/gpu/drm/i915/display/intel_cursor.c @@ -525,7 +525,7 @@ static void wa_16021440873(struct intel_plane *plane, intel_de_write_fw(dev_priv, SEL_FETCH_CUR_CTL(pipe), ctl); - intel_de_write(dev_priv, PIPE_SRCSZ_ERLY_TPT(dev_priv, pipe), + intel_de_write(dev_priv, PIPE_SRCSZ_ERLY_TPT(pipe), PIPESRC_HEIGHT(et_y_position)); } diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index df0d14a5023f..d49e869f6be2 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -2381,7 +2381,7 @@ void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_st if (!crtc_state->enable_psr2_su_region_et) return; - intel_de_write(dev_priv, PIPE_SRCSZ_ERLY_TPT(dev_priv, crtc->pipe), + intel_de_write(dev_priv, PIPE_SRCSZ_ERLY_TPT(crtc->pipe), crtc_state->pipe_srcsz_early_tpt); } diff --git a/drivers/gpu/drm/i915/display/intel_psr_regs.h b/drivers/gpu/drm/i915/display/intel_psr_regs.h index e14cb48f2614..47e3a2e2977c 100644 --- a/drivers/gpu/drm/i915/display/intel_psr_regs.h +++ b/drivers/gpu/drm/i915/display/intel_psr_regs.h @@ -248,8 +248,8 @@ /* PSR2 Early transport */ #define _PIPE_SRCSZ_ERLY_TPT_A 0x70074 - -#define PIPE_SRCSZ_ERLY_TPT(dev_priv, trans) _MMIO_TRANS2(dev_priv, trans, _PIPE_SRCSZ_ERLY_TPT_A) +#define _PIPE_SRCSZ_ERLY_TPT_B 0x71074 +#define PIPE_SRCSZ_ERLY_TPT(pipe) _MMIO_PIPE((pipe), _PIPE_SRCSZ_ERLY_TPT_A, _PIPE_SRCSZ_ERLY_TPT_B) #define _SEL_FETCH_PLANE_BASE_1_A 0x70890 #define _SEL_FETCH_PLANE_BASE_2_A 0x708B0