Re: [PATCH 04/13] drm/i915: Simplify PIPESRC_ERLY_TPT definition

["Hogander, Jouni" <jouni.hogander@intel.com>]

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.
> 
>