diff mbox series

drm/i915: Fix pipe_bpp readout for BXT/GLK DSI

Message ID 20190405141349.11950-1-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Fix pipe_bpp readout for BXT/GLK DSI | expand

Commit Message

Ville Syrjälä April 5, 2019, 2:13 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The only bpc information in pipe registers for BXT/GLK DSI
is the PIPEMISC dither bpc. Let's try to use that to read
out pipe_bpp on these platforms. However, I'm not sure if
this will be correctly populated by the GOP since bspec
suggests it's only needed if dithering is actually enabled.
If not I guess we'll have to go one step further and
extract pipe_bpp from the DSI pixel format when dithering
is disabled.

Cc: Hans de Goede <hdegoede@redhat.com>
Fixes: ca0b04db14a5 ("drm/i915/dsi: Fix pipe_bpp for handling for 6 bpc pixel-formats")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/vlv_dsi.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

Comments

Imre Deak April 5, 2019, 4:23 p.m. UTC | #1 
On Fri, Apr 05, 2019 at 05:13:49PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The only bpc information in pipe registers for BXT/GLK DSI
> is the PIPEMISC dither bpc. Let's try to use that to read
> out pipe_bpp on these platforms. However, I'm not sure if
> this will be correctly populated by the GOP since bspec
> suggests it's only needed if dithering is actually enabled.
> If not I guess we'll have to go one step further and
> extract pipe_bpp from the DSI pixel format when dithering
> is disabled.
> 
> Cc: Hans de Goede <hdegoede@redhat.com>
> Fixes: ca0b04db14a5 ("drm/i915/dsi: Fix pipe_bpp for handling for 6 bpc pixel-formats")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/vlv_dsi.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c
> index 0a950c976bbb..6898541403a2 100644
> --- a/drivers/gpu/drm/i915/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/vlv_dsi.c
> @@ -256,6 +256,28 @@ static void band_gap_reset(struct drm_i915_private *dev_priv)
>  	mutex_unlock(&dev_priv->sb_lock);
>  }
>  
> +static int bdw_get_pipemisc_bpp(struct intel_crtc *crtc)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	u32 tmp;
> +
> +	tmp = I915_READ(PIPEMISC(crtc->pipe));
> +
> +	switch (tmp & PIPEMISC_DITHER_BPC_MASK) {
> +	case PIPEMISC_DITHER_6_BPC:
> +		return 18;
> +	case PIPEMISC_DITHER_8_BPC:
> +		return 24;
> +	case PIPEMISC_DITHER_10_BPC:
> +		return 30;
> +	case PIPEMISC_DITHER_12_BPC:
> +		return 36;
> +	default:
> +		MISSING_CASE(tmp);
> +		return 0;
> +	}
> +}
> +
>  static int intel_dsi_compute_config(struct intel_encoder *encoder,
>  				    struct intel_crtc_state *pipe_config,
>  				    struct drm_connector_state *conn_state)
> @@ -1082,6 +1104,8 @@ static void bxt_dsi_get_pipe_config(struct intel_encoder *encoder,
>  	bpp = mipi_dsi_pixel_format_to_bpp(
>  			pixel_format_from_register_bits(fmt));
>  
> +	pipe_config->pipe_bpp = bdw_get_pipemisc_bpp(crtc);
> +
>  	/* Enable Frame time stamo based scanline reporting */
>  	adjusted_mode->private_flags |=
>  			I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP;
> -- 
> 2.19.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Saarinen, Jani April 5, 2019, 7:23 p.m. UTC | #2 
Hi, 

> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Patchwork
> Sent: perjantai 5. huhtikuuta 2019 20.14
> To: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Fix pipe_bpp readout for
> BXT/GLK DSI
> 
> == Series Details ==
> 
> Series: drm/i915: Fix pipe_bpp readout for BXT/GLK DSI
> URL   : https://patchwork.freedesktop.org/series/59067/
> State : success
> 
> == Summary ==
> 
> CI Bug Log - changes from CI_DRM_5881 -> Patchwork_12701
> ====================================================
> 
> Summary
> -------
> 
>   **SUCCESS**
> 
>   No regressions found.
> 
>   External URL:
> https://patchwork.freedesktop.org/api/1.0/series/59067/revisions/1/mbox/
> 
> Possible new issues
> -------------------
> 
>   Here are the unknown changes that may have been introduced in
> Patchwork_12701:
> 
> ### IGT changes ###
> 
> #### Suppressed ####
> 
>   The following results come from untrusted machines, tests, or statuses.
>   They do not affect the overall result.
> 
>   * igt@gem_ctx_exec@basic:
>     - {fi-icl-u3}:        PASS -> INCOMPLETE
> 
> 
> Known issues
> ------------
> 
>   Here are the changes found in Patchwork_12701 that come from known issues:
> 
> ### IGT changes ###
> 
> #### Issues hit ####
> 
>   * igt@gem_exec_gttfill@basic:
>     - fi-icl-y:           PASS -> INCOMPLETE [fdo#110209]
> 
>   * igt@gem_exec_store@basic-bsd2:
>     - fi-bxt-dsi:         NOTRUN -> SKIP [fdo#109271] +47
> 
>   * igt@i915_pm_rpm@module-reload:
>     - fi-glk-dsi:         NOTRUN -> DMESG-FAIL [fdo#105538] / [fdo#107732]
> 
>   * igt@i915_selftest@live_hangcheck:
>     - fi-skl-iommu:       PASS -> INCOMPLETE [fdo#108602] / [fdo#108744]
>     - fi-bxt-dsi:         NOTRUN -> INCOMPLETE [fdo#103927]
> 
>   * igt@kms_force_connector_basic@force-edid:
>     - fi-glk-dsi:         NOTRUN -> SKIP [fdo#109271] +29
> 
>   * igt@kms_frontbuffer_tracking@basic:
>     - fi-glk-dsi:         NOTRUN -> FAIL [fdo#103167]
>     - fi-byt-clapper:     PASS -> FAIL [fdo#103167]
> 
>   * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
>     - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]
> 
>   * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
>     - fi-ivb-3770:        PASS -> SKIP [fdo#109271]
> 
>   * igt@runner@aborted:
>     - fi-skl-iommu:       NOTRUN -> FAIL [fdo#104108] / [fdo#108602]
> 
> 
> #### Possible fixes ####
> 
>   * igt@i915_pm_rpm@module-reload:
>     - fi-skl-6770hq:      FAIL [fdo#108511] -> PASS
> 
>   * igt@i915_selftest@live_execlists:
>     - fi-apl-guc:         INCOMPLETE [fdo#103927] / [fdo#109720] -> PASS
> 
>   * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
>     - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS
> 
> 
>   {name}: This element is suppressed. This means it is ignored when computing
>           the status of the difference (SUCCESS, WARNING, or FAILURE).
> 
>   [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
>   [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
>   [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
>   [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
>   [fdo#105538]: https://bugs.freedesktop.org/show_bug.cgi?id=105538
>   [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
>   [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
>   [fdo#107732]: https://bugs.freedesktop.org/show_bug.cgi?id=107732
>   [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
>   [fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
>   [fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
>   [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
>   [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720
>   [fdo#110209]: https://bugs.freedesktop.org/show_bug.cgi?id=110209
> 
> 
> Participating hosts (49 -> 43)
> ------------------------------
> 
>   Missing    (6): fi-ilk-m540 fi-bdw-5557u fi-hsw-peppy fi-byt-squawks fi-bsw-cyan fi-
> bdw-samus
> 
> 
> Build changes
> -------------
> 
>     * Linux: CI_DRM_5881 -> Patchwork_12701
> 
>   CI_DRM_5881: b070175c76da1440a747fd023ee6253e573055f8 @
> git://anongit.freedesktop.org/gfx-ci/linux
>   IGT_4931: 019f892e5d1a0a9643cb726c47ce2d99c14b444f @
> git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
>   Patchwork_12701: aaabc22c7f568ba125526c7a9171dab1aa7170e6 @
> git://anongit.freedesktop.org/gfx-ci/linux
> 
> 
> == Linux commits ==
> 
> aaabc22c7f56 drm/i915: Fix pipe_bpp readout for BXT/GLK DSI
Both BXT and GLK now seems to boot and test with BAT. Looks much better ;). 

> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12701/
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Jani Nikula April 8, 2019, 6:16 a.m. UTC | #3 
On Fri, 05 Apr 2019, Imre Deak <imre.deak@intel.com> wrote:
> On Fri, Apr 05, 2019 at 05:13:49PM +0300, Ville Syrjala wrote:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> 
>> The only bpc information in pipe registers for BXT/GLK DSI
>> is the PIPEMISC dither bpc. Let's try to use that to read
>> out pipe_bpp on these platforms. However, I'm not sure if
>> this will be correctly populated by the GOP since bspec
>> suggests it's only needed if dithering is actually enabled.
>> If not I guess we'll have to go one step further and
>> extract pipe_bpp from the DSI pixel format when dithering
>> is disabled.
>> 
>> Cc: Hans de Goede <hdegoede@redhat.com>
>> Fixes: ca0b04db14a5 ("drm/i915/dsi: Fix pipe_bpp for handling for 6 bpc pixel-formats")
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Reviewed-by: Imre Deak <imre.deak@intel.com>

I referenced the bug though I guess we're not 100% confident this fixes
it throughout...?

Pushed, thanks for the patch and review.

BR,
Jani.


>
>> ---
>>  drivers/gpu/drm/i915/vlv_dsi.c | 24 ++++++++++++++++++++++++
>>  1 file changed, 24 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c
>> index 0a950c976bbb..6898541403a2 100644
>> --- a/drivers/gpu/drm/i915/vlv_dsi.c
>> +++ b/drivers/gpu/drm/i915/vlv_dsi.c
>> @@ -256,6 +256,28 @@ static void band_gap_reset(struct drm_i915_private *dev_priv)
>>  	mutex_unlock(&dev_priv->sb_lock);
>>  }
>>  
>> +static int bdw_get_pipemisc_bpp(struct intel_crtc *crtc)
>> +{
>> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> +	u32 tmp;
>> +
>> +	tmp = I915_READ(PIPEMISC(crtc->pipe));
>> +
>> +	switch (tmp & PIPEMISC_DITHER_BPC_MASK) {
>> +	case PIPEMISC_DITHER_6_BPC:
>> +		return 18;
>> +	case PIPEMISC_DITHER_8_BPC:
>> +		return 24;
>> +	case PIPEMISC_DITHER_10_BPC:
>> +		return 30;
>> +	case PIPEMISC_DITHER_12_BPC:
>> +		return 36;
>> +	default:
>> +		MISSING_CASE(tmp);
>> +		return 0;
>> +	}
>> +}
>> +
>>  static int intel_dsi_compute_config(struct intel_encoder *encoder,
>>  				    struct intel_crtc_state *pipe_config,
>>  				    struct drm_connector_state *conn_state)
>> @@ -1082,6 +1104,8 @@ static void bxt_dsi_get_pipe_config(struct intel_encoder *encoder,
>>  	bpp = mipi_dsi_pixel_format_to_bpp(
>>  			pixel_format_from_register_bits(fmt));
>>  
>> +	pipe_config->pipe_bpp = bdw_get_pipemisc_bpp(crtc);
>> +
>>  	/* Enable Frame time stamo based scanline reporting */
>>  	adjusted_mode->private_flags |=
>>  			I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP;
>> -- 
>> 2.19.2
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Ville Syrjälä April 8, 2019, 12:15 p.m. UTC | #4 
On Mon, Apr 08, 2019 at 09:16:34AM +0300, Jani Nikula wrote:
> On Fri, 05 Apr 2019, Imre Deak <imre.deak@intel.com> wrote:
> > On Fri, Apr 05, 2019 at 05:13:49PM +0300, Ville Syrjala wrote:
> >> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> 
> >> The only bpc information in pipe registers for BXT/GLK DSI
> >> is the PIPEMISC dither bpc. Let's try to use that to read
> >> out pipe_bpp on these platforms. However, I'm not sure if
> >> this will be correctly populated by the GOP since bspec
> >> suggests it's only needed if dithering is actually enabled.
> >> If not I guess we'll have to go one step further and
> >> extract pipe_bpp from the DSI pixel format when dithering
> >> is disabled.
> >> 
> >> Cc: Hans de Goede <hdegoede@redhat.com>
> >> Fixes: ca0b04db14a5 ("drm/i915/dsi: Fix pipe_bpp for handling for 6 bpc pixel-formats")
> >> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Reviewed-by: Imre Deak <imre.deak@intel.com>
> 
> I referenced the bug though I guess we're not 100% confident this fixes
> it throughout...?

It should fix the state checker 100%. But I'm not entirely sure it'll
do quite the right thing for the initial readout. The whole pipe_bpp
concept is a bit of mess. Might have to try a second time to rethink
that somehow. My first attempt at rethinking it didn't achieve
sensible results.

> 
> Pushed, thanks for the patch and review.
> 
> BR,
> Jani.
> 
> 
> >
> >> ---
> >>  drivers/gpu/drm/i915/vlv_dsi.c | 24 ++++++++++++++++++++++++
> >>  1 file changed, 24 insertions(+)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c
> >> index 0a950c976bbb..6898541403a2 100644
> >> --- a/drivers/gpu/drm/i915/vlv_dsi.c
> >> +++ b/drivers/gpu/drm/i915/vlv_dsi.c
> >> @@ -256,6 +256,28 @@ static void band_gap_reset(struct drm_i915_private *dev_priv)
> >>  	mutex_unlock(&dev_priv->sb_lock);
> >>  }
> >>  
> >> +static int bdw_get_pipemisc_bpp(struct intel_crtc *crtc)
> >> +{
> >> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >> +	u32 tmp;
> >> +
> >> +	tmp = I915_READ(PIPEMISC(crtc->pipe));
> >> +
> >> +	switch (tmp & PIPEMISC_DITHER_BPC_MASK) {
> >> +	case PIPEMISC_DITHER_6_BPC:
> >> +		return 18;
> >> +	case PIPEMISC_DITHER_8_BPC:
> >> +		return 24;
> >> +	case PIPEMISC_DITHER_10_BPC:
> >> +		return 30;
> >> +	case PIPEMISC_DITHER_12_BPC:
> >> +		return 36;
> >> +	default:
> >> +		MISSING_CASE(tmp);
> >> +		return 0;
> >> +	}
> >> +}
> >> +
> >>  static int intel_dsi_compute_config(struct intel_encoder *encoder,
> >>  				    struct intel_crtc_state *pipe_config,
> >>  				    struct drm_connector_state *conn_state)
> >> @@ -1082,6 +1104,8 @@ static void bxt_dsi_get_pipe_config(struct intel_encoder *encoder,
> >>  	bpp = mipi_dsi_pixel_format_to_bpp(
> >>  			pixel_format_from_register_bits(fmt));
> >>  
> >> +	pipe_config->pipe_bpp = bdw_get_pipemisc_bpp(crtc);
> >> +
> >>  	/* Enable Frame time stamo based scanline reporting */
> >>  	adjusted_mode->private_flags |=
> >>  			I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP;
> >> -- 
> >> 2.19.2
> >> 
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c
index 0a950c976bbb..6898541403a2 100644
--- a/drivers/gpu/drm/i915/vlv_dsi.c
+++ b/drivers/gpu/drm/i915/vlv_dsi.c
@@ -256,6 +256,28 @@  static void band_gap_reset(struct drm_i915_private *dev_priv)
 	mutex_unlock(&dev_priv->sb_lock);
 }
 
+static int bdw_get_pipemisc_bpp(struct intel_crtc *crtc)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	u32 tmp;
+
+	tmp = I915_READ(PIPEMISC(crtc->pipe));
+
+	switch (tmp & PIPEMISC_DITHER_BPC_MASK) {
+	case PIPEMISC_DITHER_6_BPC:
+		return 18;
+	case PIPEMISC_DITHER_8_BPC:
+		return 24;
+	case PIPEMISC_DITHER_10_BPC:
+		return 30;
+	case PIPEMISC_DITHER_12_BPC:
+		return 36;
+	default:
+		MISSING_CASE(tmp);
+		return 0;
+	}
+}
+
 static int intel_dsi_compute_config(struct intel_encoder *encoder,
 				    struct intel_crtc_state *pipe_config,
 				    struct drm_connector_state *conn_state)
@@ -1082,6 +1104,8 @@  static void bxt_dsi_get_pipe_config(struct intel_encoder *encoder,
 	bpp = mipi_dsi_pixel_format_to_bpp(
 			pixel_format_from_register_bits(fmt));
 
+	pipe_config->pipe_bpp = bdw_get_pipemisc_bpp(crtc);
+
 	/* Enable Frame time stamo based scanline reporting */
 	adjusted_mode->private_flags |=
 			I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP;