Re: [PATCH v10 1/4] drm/i915/lobf: No need to pass connector-state on lobf-compute-config.

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

On Thu, 05 Sep 2024, Animesh Manna <animesh.manna@intel.com> wrote:
> Connector state is not used in lobf compute config, so remove it.
>
> Fixes: 15438b325987 ("drm/i915/alpm: Add compute config for lobf")

This is refactoring, and not a fix. We don't need to have this
backported.

On the actual change, it's fine. But I do find myself thinking most of
the similar functions on the encoder->compute_config() path should 1) be
all named _compute_config, and 2) have the same parameter set as
encoder->compute_config(), needed or not. Because often you are going to
need something later, and then you end up having to plumb them all the
way the stack to the destination. It's just unnecessary to keep removing
and adding the parameters in the _compute_config() path. (And similarly
for many other encoder hooks.)

BR,
Jani.




> Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_alpm.c | 3 +--
>  drivers/gpu/drm/i915/display/intel_alpm.h | 3 +--
>  drivers/gpu/drm/i915/display/intel_dp.c   | 2 +-
>  3 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c b/drivers/gpu/drm/i915/display/intel_alpm.c
> index 186cf4833f71..f2fea356d28a 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> @@ -266,8 +266,7 @@ bool intel_alpm_compute_params(struct intel_dp *intel_dp,
>  }
>  
>  void intel_alpm_lobf_compute_config(struct intel_dp *intel_dp,
> -				    struct intel_crtc_state *crtc_state,
> -				    struct drm_connector_state *conn_state)
> +				    struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_display *display = to_intel_display(intel_dp);
>  	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.h b/drivers/gpu/drm/i915/display/intel_alpm.h
> index 8c409b10dce6..a17dfaa5248d 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.h
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.h
> @@ -17,8 +17,7 @@ void intel_alpm_init_dpcd(struct intel_dp *intel_dp);
>  bool intel_alpm_compute_params(struct intel_dp *intel_dp,
>  			       const struct intel_crtc_state *crtc_state);
>  void intel_alpm_lobf_compute_config(struct intel_dp *intel_dp,
> -				    struct intel_crtc_state *crtc_state,
> -				    struct drm_connector_state *conn_state);
> +				    struct intel_crtc_state *crtc_state);
>  void intel_alpm_configure(struct intel_dp *intel_dp,
>  			  const struct intel_crtc_state *crtc_state);
>  void intel_alpm_lobf_debugfs_add(struct intel_connector *connector);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index a1fcedfd404b..86bc6d79279f 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -3107,7 +3107,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	intel_vrr_compute_config(pipe_config, conn_state);
>  	intel_dp_compute_as_sdp(intel_dp, pipe_config);
>  	intel_psr_compute_config(intel_dp, pipe_config, conn_state);
> -	intel_alpm_lobf_compute_config(intel_dp, pipe_config, conn_state);
> +	intel_alpm_lobf_compute_config(intel_dp, pipe_config);
>  	intel_dp_drrs_compute_config(connector, pipe_config, link_bpp_x16);
>  	intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state);
>  	intel_dp_compute_hdr_metadata_infoframe_sdp(intel_dp, pipe_config, conn_state);

-- 
Jani Nikula, Intel