Re: [PATCH 07/19] drm/i915/icl_dsi: Move helpers to configure dsi dual link to intel_dss

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Aug 30, 2024 at 10:39:37AM +0530, Ankit Nautiyal wrote:
> Move the function to configure dss_ctl for dual_link dsi to intel_dss
> files. While at it, use struct intel_display wherever possible.
> 
> v2: Avoid modifying the code while movement. (Jani)
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/icl_dsi.c   | 57 ++----------------------
>  drivers/gpu/drm/i915/display/intel_dss.c | 50 +++++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_dss.h |  3 ++
>  3 files changed, 57 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 79e149d51cb2..ec880d1cbbee 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -44,7 +44,7 @@
>  #include "intel_de.h"
>  #include "intel_dsi.h"
>  #include "intel_dsi_vbt.h"
> -#include "intel_dss_regs.h"
> +#include "intel_dss.h"
>  #include "intel_panel.h"
>  #include "intel_vdsc.h"
>  #include "skl_scaler.h"
> @@ -274,55 +274,6 @@ static void dsi_program_swing_and_deemphasis(struct intel_encoder *encoder)
>  	}
>  }
>  
> -static void configure_dual_link_mode(struct intel_encoder *encoder,
> -				     const struct intel_crtc_state *pipe_config,
> -				     u8 dual_link, u8 pixel_overlap)
> -{
> -	struct intel_display *display = to_intel_display(encoder);
> -	i915_reg_t dss_ctl1_reg, dss_ctl2_reg;
> -	u32 dss_ctl1;
> -
> -	/* FIXME: Move all DSS handling to intel_vdsc.c */
> -	if (DISPLAY_VER(display) >= 12) {
> -		struct intel_crtc *crtc = to_intel_crtc(pipe_config->uapi.crtc);
> -
> -		dss_ctl1_reg = ICL_PIPE_DSS_CTL1(crtc->pipe);
> -		dss_ctl2_reg = ICL_PIPE_DSS_CTL2(crtc->pipe);
> -	} else {
> -		dss_ctl1_reg = DSS_CTL1;
> -		dss_ctl2_reg = DSS_CTL2;
> -	}
> -
> -	dss_ctl1 = intel_de_read(display, dss_ctl1_reg);
> -	dss_ctl1 |= SPLITTER_ENABLE;
> -	dss_ctl1 &= ~OVERLAP_PIXELS_MASK;
> -	dss_ctl1 |= OVERLAP_PIXELS(pixel_overlap);
> -
> -	if (dual_link == DSI_DUAL_LINK_FRONT_BACK) {
> -		const struct drm_display_mode *adjusted_mode =
> -					&pipe_config->hw.adjusted_mode;
> -		u16 hactive = adjusted_mode->crtc_hdisplay;
> -		u16 dl_buffer_depth;
> -
> -		dss_ctl1 &= ~DUAL_LINK_MODE_INTERLEAVE;
> -		dl_buffer_depth = hactive / 2 + pixel_overlap;
> -
> -		if (dl_buffer_depth > MAX_DL_BUFFER_TARGET_DEPTH)
> -			drm_err(display->drm,
> -				"DL buffer depth exceed max value\n");
> -
> -		dss_ctl1 &= ~LEFT_DL_BUF_TARGET_DEPTH_MASK;
> -		dss_ctl1 |= LEFT_DL_BUF_TARGET_DEPTH(dl_buffer_depth);
> -		intel_de_rmw(display, dss_ctl2_reg, RIGHT_DL_BUF_TARGET_DEPTH_MASK,
> -			     RIGHT_DL_BUF_TARGET_DEPTH(dl_buffer_depth));
> -	} else {
> -		/* Interleave */
> -		dss_ctl1 |= DUAL_LINK_MODE_INTERLEAVE;
> -	}
> -
> -	intel_de_write(display, dss_ctl1_reg, dss_ctl1);
> -}
> -
>  /* aka DSI 8X clock */
>  static int afe_clk(struct intel_encoder *encoder,
>  		   const struct intel_crtc_state *crtc_state)
> @@ -791,9 +742,9 @@ gen11_dsi_configure_transcoder(struct intel_encoder *encoder,
>  		}
>  
>  		/* configure stream splitting */
> -		configure_dual_link_mode(encoder, pipe_config,
> -					 intel_dsi->dual_link,
> -					 intel_dsi->pixel_overlap);
> +		intel_dss_dsi_dual_link_mode_configure(encoder, pipe_config,
> +						       intel_dsi->dual_link,
> +						       intel_dsi->pixel_overlap);
>  	}
>  
>  	for_each_dsi_port(port, intel_dsi->ports) {
> diff --git a/drivers/gpu/drm/i915/display/intel_dss.c b/drivers/gpu/drm/i915/display/intel_dss.c
> index 3f7f416eb3fa..969e32143983 100644
> --- a/drivers/gpu/drm/i915/display/intel_dss.c
> +++ b/drivers/gpu/drm/i915/display/intel_dss.c
> @@ -7,6 +7,7 @@
>  #include "i915_reg_defs.h"
>  #include "intel_de.h"
>  #include "intel_display_types.h"
> +#include "intel_dsi.h"
>  #include "intel_dss.h"
>  #include "intel_dss_regs.h"
>  
> @@ -87,3 +88,52 @@ void intel_dss_mso_configure(const struct intel_crtc_state *crtc_state)
>  		     SPLITTER_ENABLE | SPLITTER_CONFIGURATION_MASK |
>  		     OVERLAP_PIXELS_MASK, dss1);
>  }
> +
> +void intel_dss_dsi_dual_link_mode_configure(struct intel_encoder *encoder,
> +					    const struct intel_crtc_state *pipe_config,
> +					    u8 dual_link,
> +					    u8 pixel_overlap)
> +{
> +	struct intel_display *display = to_intel_display(encoder);
> +	i915_reg_t dss_ctl1_reg, dss_ctl2_reg;
> +	u32 dss_ctl1;
> +
> +	if (DISPLAY_VER(display) >= 12) {
> +		struct intel_crtc *crtc = to_intel_crtc(pipe_config->uapi.crtc);
> +
> +		dss_ctl1_reg = ICL_PIPE_DSS_CTL1(crtc->pipe);
> +		dss_ctl2_reg = ICL_PIPE_DSS_CTL2(crtc->pipe);
> +	} else {
> +		dss_ctl1_reg = DSS_CTL1;
> +		dss_ctl2_reg = DSS_CTL2;
> +	}
> +
> +	dss_ctl1 = intel_de_read(display, dss_ctl1_reg);
> +	dss_ctl1 |= SPLITTER_ENABLE;
> +	dss_ctl1 &= ~OVERLAP_PIXELS_MASK;
> +	dss_ctl1 |= OVERLAP_PIXELS(pixel_overlap);
> +
> +	if (dual_link == DSI_DUAL_LINK_FRONT_BACK) {
> +		const struct drm_display_mode *adjusted_mode =
> +					&pipe_config->hw.adjusted_mode;
> +		u16 hactive = adjusted_mode->crtc_hdisplay;
> +		u16 dl_buffer_depth;
> +
> +		dss_ctl1 &= ~DUAL_LINK_MODE_INTERLEAVE;
> +		dl_buffer_depth = hactive / 2 + pixel_overlap;
> +
> +		if (dl_buffer_depth > MAX_DL_BUFFER_TARGET_DEPTH)
> +			drm_err(display->drm,
> +				"DL buffer depth exceed max value\n");
> +
> +		dss_ctl1 &= ~LEFT_DL_BUF_TARGET_DEPTH_MASK;
> +		dss_ctl1 |= LEFT_DL_BUF_TARGET_DEPTH(dl_buffer_depth);
> +		intel_de_rmw(display, dss_ctl2_reg, RIGHT_DL_BUF_TARGET_DEPTH_MASK,
> +			     RIGHT_DL_BUF_TARGET_DEPTH(dl_buffer_depth));

Leaking the DSI mess outside of the DSI code is not great. The DSI
code should really just be taught to use the crtc_state properly.

> +	} else {
> +		/* Interleave */
> +		dss_ctl1 |= DUAL_LINK_MODE_INTERLEAVE;
> +	}
> +
> +	intel_de_write(display, dss_ctl1_reg, dss_ctl1);
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_dss.h b/drivers/gpu/drm/i915/display/intel_dss.h
> index d4629052979a..aa8c67c15855 100644
> --- a/drivers/gpu/drm/i915/display/intel_dss.h
> +++ b/drivers/gpu/drm/i915/display/intel_dss.h
> @@ -16,5 +16,8 @@ u8 intel_dss_mso_pipe_mask(struct intel_display *display);
>  void intel_dss_mso_get_config(struct intel_encoder *encoder,
>  			      struct intel_crtc_state *pipe_config);
>  void intel_dss_mso_configure(const struct intel_crtc_state *crtc_state);
> +void intel_dss_dsi_dual_link_mode_configure(struct intel_encoder *encoder,
> +					    const struct intel_crtc_state *pipe_config,
> +					    u8 dual_link, u8 pixel_overlap);
>  
>  #endif /* __INTEL_DSS_H__ */
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel