On Mon, 26 Aug 2024, Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx> 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.
Also here, I prefer splitting code movement from other changes. It's
just so much easier to review, and so much easier to figure out what
went wrong if a bisection ever lands here.
>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/icl_dsi.c | 55 ++----------------------
> drivers/gpu/drm/i915/display/intel_dss.c | 51 ++++++++++++++++++++++
> drivers/gpu/drm/i915/display/intel_dss.h | 3 ++
> 3 files changed, 58 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 5ad5011e1fee..ab59638a8c6c 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)
> -{
> - struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> - struct intel_dsi *intel_dsi = enc_to_intel_dsi(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(dev_priv) >= 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(dev_priv, dss_ctl1_reg);
> - dss_ctl1 |= SPLITTER_ENABLE;
> - dss_ctl1 &= ~OVERLAP_PIXELS_MASK;
> - dss_ctl1 |= OVERLAP_PIXELS(intel_dsi->pixel_overlap);
> -
> - if (intel_dsi->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 + intel_dsi->pixel_overlap;
> -
> - if (dl_buffer_depth > MAX_DL_BUFFER_TARGET_DEPTH)
> - drm_err(&dev_priv->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(dev_priv, 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(dev_priv, 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,7 +742,9 @@ gen11_dsi_configure_transcoder(struct intel_encoder *encoder,
> }
>
> /* configure stream splitting */
> - configure_dual_link_mode(encoder, pipe_config);
> + intel_dss_configure_dsi_dual_link_mode(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 9cb89fe656cb..f44dcac84aa4 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"
>
> @@ -85,3 +86,53 @@ void intel_dss_configure_mso(const struct intel_crtc_state *crtc_state)
> SPLITTER_ENABLE | SPLITTER_CONFIGURATION_MASK |
> OVERLAP_PIXELS_MASK, dss1);
> }
> +
> +void intel_dss_configure_dsi_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;
> +
> + 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) {
> + struct drm_i915_private *i915 = to_i915(display->drm);
You don't need that.
> + 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(&i915->drm,
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);
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_dss.h b/drivers/gpu/drm/i915/display/intel_dss.h
> index 0571ee2a19f9..0c5d652d46f5 100644
> --- a/drivers/gpu/drm/i915/display/intel_dss.h
> +++ b/drivers/gpu/drm/i915/display/intel_dss.h
> @@ -16,6 +16,9 @@ u8 intel_dss_splitter_pipe_mask(struct intel_display *display);
> void intel_dss_get_mso_config(struct intel_encoder *encoder,
> struct intel_crtc_state *pipe_config);
> void intel_dss_configure_mso(const struct intel_crtc_state *crtc_state);
> +void intel_dss_configure_dsi_dual_link_mode(struct intel_encoder *encoder,
> + const struct intel_crtc_state *pipe_config,
> + u8 dual_link, u8 pixel_overlap);
Nitpick, I think I'd name this intel_dss_dsi_dual_link_mode_configure()
or something.
>
> #endif /* __INTEL_DSS_H__ */
--
Jani Nikula, Intel
