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

Nautiyal, Ankit K ankit.k.nautiyal at intel.com
Mon Sep 2 04:51:05 UTC 2024


On 8/30/2024 4:55 PM, Ville Syrjälä wrote:
> 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 at 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.

I do agree. Perhaps have a separate structure for DSS in 
intel_crtc_state with relevant bits, which would be computed in 
compute_config and then we write it one time.

Can we have a separate patch series to fix this and mso part, and just 
have DSS, joiner things separated from VDSC as a first step?

Regards,

Ankit


>
>> +	} 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


More information about the Intel-gfx mailing list