On TGL+ the DSS control registers are at different offsets, and there's one per pipe. Fix the offsets to fix dual link DSI for TGL+.
There would be helpers for this in the DSC code, but just do the quick fix now for DSI. Long term, we should probably move all the DSS handling into intel_vdsc.c, so exporting the helpers seems counter-productive.
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/8232 Cc: Ville Syrjala ville.syrjala@linux.intel.com Cc: stable@vger.kernel.org Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/i915/display/icl_dsi.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c index b5316715bb3b..5a17ab3f0d1a 100644 --- a/drivers/gpu/drm/i915/display/icl_dsi.c +++ b/drivers/gpu/drm/i915/display/icl_dsi.c @@ -277,9 +277,21 @@ static void configure_dual_link_mode(struct intel_encoder *encoder, { 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;
- dss_ctl1 = intel_de_read(dev_priv, 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); @@ -299,14 +311,14 @@ static void configure_dual_link_mode(struct intel_encoder *encoder,
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, RIGHT_DL_BUF_TARGET_DEPTH_MASK, + 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, dss_ctl1); + intel_de_write(dev_priv, dss_ctl1_reg, dss_ctl1); }
/* aka DSI 8X clock */
On Wed, Mar 01, 2023 at 05:14:09PM +0200, Jani Nikula wrote:
On TGL+ the DSS control registers are at different offsets, and there's one per pipe. Fix the offsets to fix dual link DSI for TGL+.
There would be helpers for this in the DSC code, but just do the quick fix now for DSI. Long term, we should probably move all the DSS handling into intel_vdsc.c, so exporting the helpers seems counter-productive.
I'm not entirely happy with intel_vdsc.c since it handles both the hardware VDSC block (which includes DSS, and so also uncompressed joiner and MSO), and also some actual DSC calculations/etc. Might be nice to have a cleaner split of some sort.
That also reminds me that MSO+dsc/joiner is probably going to fail miserably given that neither side knows about the other and both poke the DSS registers.
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/8232 Cc: Ville Syrjala ville.syrjala@linux.intel.com Cc: stable@vger.kernel.org Signed-off-by: Jani Nikula jani.nikula@intel.com
drivers/gpu/drm/i915/display/icl_dsi.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c index b5316715bb3b..5a17ab3f0d1a 100644 --- a/drivers/gpu/drm/i915/display/icl_dsi.c +++ b/drivers/gpu/drm/i915/display/icl_dsi.c @@ -277,9 +277,21 @@ static void configure_dual_link_mode(struct intel_encoder *encoder, { 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;
- dss_ctl1 = intel_de_read(dev_priv, 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);
Side note: should get rid of this rmw to make sure the thing fully configuerd the way we want...
Anyways, this seems fine for now: Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com
dss_ctl1 |= SPLITTER_ENABLE; dss_ctl1 &= ~OVERLAP_PIXELS_MASK; dss_ctl1 |= OVERLAP_PIXELS(intel_dsi->pixel_overlap); @@ -299,14 +311,14 @@ static void configure_dual_link_mode(struct intel_encoder *encoder, 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, RIGHT_DL_BUF_TARGET_DEPTH_MASK,
intel_de_rmw(dev_priv, dss_ctl2_reg, RIGHT_DL_BUF_TARGET_DEPTH_MASK, RIGHT_DL_BUF_TARGET_DEPTH(dl_buffer_depth));
- intel_de_write(dev_priv, DSS_CTL1, dss_ctl1);
- intel_de_write(dev_priv, dss_ctl1_reg, dss_ctl1);
} /* aka DSI 8X clock */ -- 2.39.1
On Wed, Mar 01, 2023 at 05:38:39PM +0200, Ville Syrjälä wrote:
On Wed, Mar 01, 2023 at 05:14:09PM +0200, Jani Nikula wrote:
On TGL+ the DSS control registers are at different offsets, and there's one per pipe. Fix the offsets to fix dual link DSI for TGL+.
There would be helpers for this in the DSC code, but just do the quick fix now for DSI. Long term, we should probably move all the DSS handling into intel_vdsc.c, so exporting the helpers seems counter-productive.
I'm not entirely happy with intel_vdsc.c since it handles both the hardware VDSC block (which includes DSS, and so also uncompressed joiner and MSO), and also some actual DSC calculations/etc. Might be nice to have a cleaner split of some sort.
That also reminds me that MSO+dsc/joiner is probably going to fail miserably given that neither side knows about the other and both poke the DSS registers.
I suppose MSO+joiner should just be rejected outright since the splitter seems to sit before the joiner in the path. We'd need them to be the other way around.
But MSO+DSC does look plausible.
On Wed, 01 Mar 2023, Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Wed, Mar 01, 2023 at 05:14:09PM +0200, Jani Nikula wrote:
On TGL+ the DSS control registers are at different offsets, and there's one per pipe. Fix the offsets to fix dual link DSI for TGL+.
There would be helpers for this in the DSC code, but just do the quick fix now for DSI. Long term, we should probably move all the DSS handling into intel_vdsc.c, so exporting the helpers seems counter-productive.
I'm not entirely happy with intel_vdsc.c since it handles both the hardware VDSC block (which includes DSS, and so also uncompressed joiner and MSO), and also some actual DSC calculations/etc. Might be nice to have a cleaner split of some sort.
That also reminds me that MSO+dsc/joiner is probably going to fail miserably given that neither side knows about the other and both poke the DSS registers.
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/8232 Cc: Ville Syrjala ville.syrjala@linux.intel.com Cc: stable@vger.kernel.org Signed-off-by: Jani Nikula jani.nikula@intel.com
drivers/gpu/drm/i915/display/icl_dsi.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c index b5316715bb3b..5a17ab3f0d1a 100644 --- a/drivers/gpu/drm/i915/display/icl_dsi.c +++ b/drivers/gpu/drm/i915/display/icl_dsi.c @@ -277,9 +277,21 @@ static void configure_dual_link_mode(struct intel_encoder *encoder, { 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;
- dss_ctl1 = intel_de_read(dev_priv, 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);
Side note: should get rid of this rmw to make sure the thing fully configuerd the way we want...
Anyways, this seems fine for now: Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com
Thanks, pushed to din.
BR, Jani.
dss_ctl1 |= SPLITTER_ENABLE; dss_ctl1 &= ~OVERLAP_PIXELS_MASK; dss_ctl1 |= OVERLAP_PIXELS(intel_dsi->pixel_overlap); @@ -299,14 +311,14 @@ static void configure_dual_link_mode(struct intel_encoder *encoder, 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, RIGHT_DL_BUF_TARGET_DEPTH_MASK,
intel_de_rmw(dev_priv, dss_ctl2_reg, RIGHT_DL_BUF_TARGET_DEPTH_MASK, RIGHT_DL_BUF_TARGET_DEPTH(dl_buffer_depth));
- intel_de_write(dev_priv, DSS_CTL1, dss_ctl1);
- intel_de_write(dev_priv, dss_ctl1_reg, dss_ctl1);
} /* aka DSI 8X clock */ -- 2.39.1
linux-stable-mirror@lists.linaro.org
-
Jani Nikula -
Ville Syrjälä