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

[Jani Nikula <jani.nikula@linux.intel.com>]

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.


-- 
Jani Nikula, Intel