[Intel-gfx] [PATCH v2 21/21] drm/i915/mtl: Do not update GV point, mask value

[Matt Roper]

On Thu, Aug 18, 2022 at 04:42:02PM -0700, Radhakrishna Sripada wrote:
> No need to update mask value/restrict because
> "Pcode only wants to use GV bandwidth value, not the mask value."
> for Display version greater than 14.

While the code changes might be correct, I can't decipher what the
commit message here is trying to tell us.  I'm not sure what the source
or context of the quote is, but the description in the commit message
should be more focused on why it's correct for our driver to skip these
operations.  I assume it has something to do with the new pm_demand
interfaces we'll be using to program this information into the hardware
in a future series?  If so, maybe this patch (with a modified commit
message) is better suited for inclusion in that future series where the
context makes more sense.

> 
> Bspec: 646365

This page number seems to be incorrect too.


Matt

> Cc: Matt Roper <matthew.d.roper at intel.com>
> Original Author: Caz Yokoyama
> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index d09e9e5f4481..47869fe964ba 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3924,6 +3924,14 @@ void intel_sagv_pre_plane_update(struct intel_atomic_state *state)
>  {
>  	struct drm_i915_private *i915 = to_i915(state->base.dev);
>  
> +	/*
> +	 * No need to update mask value/restrict because
> +	 * "Pcode only wants to use GV bandwidth value, not the mask value."
> +	 * for DISPLAY_VER() >= 14.
> +	 */
> +	if (DISPLAY_VER(i915) >= 14)
> +		return;
> +
>  	/*
>  	 * Just return if we can't control SAGV or don't have it.
>  	 * This is different from situation when we have SAGV but just can't
> @@ -3944,6 +3952,16 @@ void intel_sagv_post_plane_update(struct intel_atomic_state *state)
>  {
>  	struct drm_i915_private *i915 = to_i915(state->base.dev);
>  
> +	/*
> +	 * No need to update mask value/restrict because
> +	 * "Pcode only wants to use GV bandwidth value, not the mask value."
> +	 * for DISPLAY_VER() >= 14.
> +	 *
> +	 * GV bandwidth will be set by intel_pmdemand_post_plane_update()
> +	 */
> +	if (DISPLAY_VER(i915) >= 14)
> +		return;
> +
>  	/*
>  	 * Just return if we can't control SAGV or don't have it.
>  	 * This is different from situation when we have SAGV but just can't
> -- 
> 2.25.1
> 

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation