On Mon, 20 May 2024, Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote: > On Thu, 16 May 2024, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote: >> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> >> >> 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@xxxxxxxxxxxxxxx> > > Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx> 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. -- Jani Nikula, Intel