* [PATCH] drm/i915/dsi: fix DSS CTL register offsets for TGL+
@ 2023-03-01 15:14 Jani Nikula
2023-03-01 15:38 ` Ville Syrjälä
0 siblings, 1 reply; 4+ messages in thread
From: Jani Nikula @ 2023-03-01 15:14 UTC (permalink / raw)
To: intel-gfx; +Cc: Jani Nikula, Ville Syrjala, stable
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 */
--
2.39.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915/dsi: fix DSS CTL register offsets for TGL+
2023-03-01 15:14 [PATCH] drm/i915/dsi: fix DSS CTL register offsets for TGL+ Jani Nikula
@ 2023-03-01 15:38 ` Ville Syrjälä
2023-03-01 16:00 ` [Intel-gfx] " Ville Syrjälä
2023-03-06 16:25 ` Jani Nikula
0 siblings, 2 replies; 4+ messages in thread
From: Ville Syrjälä @ 2023-03-01 15:38 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx, stable
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));
> } 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 */
> --
> 2.39.1
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/dsi: fix DSS CTL register offsets for TGL+
2023-03-01 15:38 ` Ville Syrjälä
@ 2023-03-01 16:00 ` Ville Syrjälä
2023-03-06 16:25 ` Jani Nikula
1 sibling, 0 replies; 4+ messages in thread
From: Ville Syrjälä @ 2023-03-01 16:00 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx, stable
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.
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915/dsi: fix DSS CTL register offsets for TGL+
2023-03-01 15:38 ` Ville Syrjälä
2023-03-01 16:00 ` [Intel-gfx] " Ville Syrjälä
@ 2023-03-06 16:25 ` Jani Nikula
1 sibling, 0 replies; 4+ messages in thread
From: Jani Nikula @ 2023-03-06 16:25 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, stable
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));
>> } 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 */
>> --
>> 2.39.1
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-03-06 16:27 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-01 15:14 [PATCH] drm/i915/dsi: fix DSS CTL register offsets for TGL+ Jani Nikula
2023-03-01 15:38 ` Ville Syrjälä
2023-03-01 16:00 ` [Intel-gfx] " Ville Syrjälä
2023-03-06 16:25 ` Jani Nikula
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).