Re: [PATCH 1/2] drm/i915/display/drrs: Refactor CPU transcoder DRRS check

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

We don't need to keep adding /display/ in the subject prefix.

Just drm/i915/drrs: is fine.

On Tue, 27 Feb 2024, Bhanuprakash Modem <bhanuprakash.modem@intel.com> wrote:
> Rename cpu_transcoder_has_drrs() to intel_cpu_transcoder_has_drrs()
> and make it as non-static, therefore it can be re-used.
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> Cc: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c |  9 +++++++++
>  drivers/gpu/drm/i915/display/intel_display.h |  2 ++
>  drivers/gpu/drm/i915/display/intel_dp.c      | 11 +----------
>  3 files changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 00ac65a14029..2210d3bb2e0a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2582,6 +2582,15 @@ void intel_cpu_transcoder_set_m2_n2(struct intel_crtc *crtc,
>  		      PIPE_LINK_M2(transcoder), PIPE_LINK_N2(transcoder));
>  }
>  
> +bool intel_cpu_transcoder_has_drrs(struct drm_i915_private *i915,
> +				   enum transcoder cpu_transcoder)
> +{
> +	if (HAS_DOUBLE_BUFFERED_M_N(i915))
> +		return true;
> +
> +	return intel_cpu_transcoder_has_m2_n2(i915, cpu_transcoder);
> +}

It really needs to be moved to intel_drrs.[ch], not here.

intel_display.c must cease to be a dumping ground for random stuff.

Otherwise seems like the right thing to do.

BR,
Jani.

> +
>  static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> index f4a0773f0fca..0dc39a8a517e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -484,6 +484,8 @@ void intel_cpu_transcoder_get_m1_n1(struct intel_crtc *crtc,
>  void intel_cpu_transcoder_get_m2_n2(struct intel_crtc *crtc,
>  				    enum transcoder cpu_transcoder,
>  				    struct intel_link_m_n *m_n);
> +bool intel_cpu_transcoder_has_drrs(struct drm_i915_private *i915,
> +				    enum transcoder cpu_transcoder);
>  int intel_dotclock_calculate(int link_freq, const struct intel_link_m_n *m_n);
>  int intel_crtc_dotclock(const struct intel_crtc_state *pipe_config);
>  enum intel_display_power_domain intel_port_to_power_domain(struct intel_digital_port *dig_port);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index e13121dc3a03..5c3c0449e866 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -2689,15 +2689,6 @@ intel_dp_compute_hdr_metadata_infoframe_sdp(struct intel_dp *intel_dp,
>  		intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_GAMUT_METADATA);
>  }
>  
> -static bool cpu_transcoder_has_drrs(struct drm_i915_private *i915,
> -				    enum transcoder cpu_transcoder)
> -{
> -	if (HAS_DOUBLE_BUFFERED_M_N(i915))
> -		return true;
> -
> -	return intel_cpu_transcoder_has_m2_n2(i915, cpu_transcoder);
> -}
> -
>  static bool can_enable_drrs(struct intel_connector *connector,
>  			    const struct intel_crtc_state *pipe_config,
>  			    const struct drm_display_mode *downclock_mode)
> @@ -2720,7 +2711,7 @@ static bool can_enable_drrs(struct intel_connector *connector,
>  	if (pipe_config->has_pch_encoder)
>  		return false;
>  
> -	if (!cpu_transcoder_has_drrs(i915, pipe_config->cpu_transcoder))
> +	if (!intel_cpu_transcoder_has_drrs(i915, pipe_config->cpu_transcoder))
>  		return false;
>  
>  	return downclock_mode &&

-- 
Jani Nikula, Intel