diff mbox series

drm/i915/icl: avoid unclaimed PLANE_NV12_BUF_CFG register

Message ID 20180801004614.22149-1-paulo.r.zanoni@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/icl: avoid unclaimed PLANE_NV12_BUF_CFG register | expand

Commit Message

Zanoni, Paulo R Aug. 1, 2018, 12:46 a.m. UTC 
We don't have proper watermark NV12 support on ICL due to differences
in how it should be implemented. In commit 234059da0f33
("drm/i915/icl: NV12 y-plane ddb is not in same plane") we avoided
writing the non-existent PLANE_NV12_BUF_CFG registers but we forgot to
also avoid them on the hardware state readout. While the code is still
not correct, at least now we can avoid unclaimed register error
messages when dealing with RGB formats, which makes CI happier.

Also add some FIXME comments in order to make it even more clear that
there's still work to do.

References: commit 234059da0f33 ("drm/i915/icl: NV12 y-plane ddb is
 not in same plane")
Cc: Mahesh Kumar <mahesh1.kumar@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Kumar, Mahesh Aug. 1, 2018, 2:59 p.m. UTC | #1 
Hi,

Reviewed-by: Mahesh Kumar <mahesh1.kumar@intel.com>

-Mahesh
On 8/1/2018 6:16 AM, Paulo Zanoni wrote:
> We don't have proper watermark NV12 support on ICL due to differences
> in how it should be implemented. In commit 234059da0f33
> ("drm/i915/icl: NV12 y-plane ddb is not in same plane") we avoided
> writing the non-existent PLANE_NV12_BUF_CFG registers but we forgot to
> also avoid them on the hardware state readout. While the code is still
> not correct, at least now we can avoid unclaimed register error
> messages when dealing with RGB formats, which makes CI happier.
>
> Also add some FIXME comments in order to make it even more clear that
> there's still work to do.
>
> References: commit 234059da0f33 ("drm/i915/icl: NV12 y-plane ddb is
>   not in same plane")
> Cc: Mahesh Kumar <mahesh1.kumar@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_pm.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 2531eb75bdce..04cef1369e8c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3909,7 +3909,12 @@ skl_ddb_get_hw_plane_state(struct drm_i915_private *dev_priv,
>   				      val & PLANE_CTL_ALPHA_MASK);
>   
>   	val = I915_READ(PLANE_BUF_CFG(pipe, plane_id));
> -	val2 = I915_READ(PLANE_NV12_BUF_CFG(pipe, plane_id));
> +	/*
> +	 * FIXME: add proper NV12 support for ICL. Avoid reading unclaimed
> +	 * registers for now.
> +	 */
> +	if (INTEL_GEN(dev_priv) < 11)
> +		val2 = I915_READ(PLANE_NV12_BUF_CFG(pipe, plane_id));
>   
>   	if (fourcc == DRM_FORMAT_NV12) {
>   		skl_ddb_entry_init_from_hw(dev_priv,
> @@ -4977,6 +4982,7 @@ static void skl_write_plane_wm(struct intel_crtc *intel_crtc,
>   
>   	skl_ddb_entry_write(dev_priv, PLANE_BUF_CFG(pipe, plane_id),
>   			    &ddb->plane[pipe][plane_id]);
> +	/* FIXME: add proper NV12 support for ICL. */
>   	if (INTEL_GEN(dev_priv) >= 11)
>   		return skl_ddb_entry_write(dev_priv,
>   					   PLANE_BUF_CFG(pipe, plane_id),
<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <p>Hi,</p>
    <p>Reviewed-by: Mahesh Kumar <a class="moz-txt-link-rfc2396E"
        href="mailto:mahesh1.kumar@intel.com">&lt;mahesh1.kumar@intel.com&gt;</a>
      <br>
    </p>
    -Mahesh<br>
    <div class="moz-cite-prefix">On 8/1/2018 6:16 AM, Paulo Zanoni
      wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:20180801004614.22149-1-paulo.r.zanoni@intel.com">
      <pre wrap="">We don't have proper watermark NV12 support on ICL due to differences
in how it should be implemented. In commit 234059da0f33
("drm/i915/icl: NV12 y-plane ddb is not in same plane") we avoided
writing the non-existent PLANE_NV12_BUF_CFG registers but we forgot to
also avoid them on the hardware state readout. While the code is still
not correct, at least now we can avoid unclaimed register error
messages when dealing with RGB formats, which makes CI happier.

Also add some FIXME comments in order to make it even more clear that
there's still work to do.

References: commit 234059da0f33 ("drm/i915/icl: NV12 y-plane ddb is
 not in same plane")
Cc: Mahesh Kumar <a class="moz-txt-link-rfc2396E" href="mailto:mahesh1.kumar@intel.com">&lt;mahesh1.kumar@intel.com&gt;</a>
Signed-off-by: Paulo Zanoni <a class="moz-txt-link-rfc2396E" href="mailto:paulo.r.zanoni@intel.com">&lt;paulo.r.zanoni@intel.com&gt;</a>
---
 drivers/gpu/drm/i915/intel_pm.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2531eb75bdce..04cef1369e8c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3909,7 +3909,12 @@ skl_ddb_get_hw_plane_state(struct drm_i915_private *dev_priv,
 				      val &amp; PLANE_CTL_ALPHA_MASK);
 
 	val = I915_READ(PLANE_BUF_CFG(pipe, plane_id));
-	val2 = I915_READ(PLANE_NV12_BUF_CFG(pipe, plane_id));
+	/*
+	 * FIXME: add proper NV12 support for ICL. Avoid reading unclaimed
+	 * registers for now.
+	 */
+	if (INTEL_GEN(dev_priv) &lt; 11)
+		val2 = I915_READ(PLANE_NV12_BUF_CFG(pipe, plane_id));
 
 	if (fourcc == DRM_FORMAT_NV12) {
 		skl_ddb_entry_init_from_hw(dev_priv,
@@ -4977,6 +4982,7 @@ static void skl_write_plane_wm(struct intel_crtc *intel_crtc,
 
 	skl_ddb_entry_write(dev_priv, PLANE_BUF_CFG(pipe, plane_id),
 			    &amp;ddb-&gt;plane[pipe][plane_id]);
+	/* FIXME: add proper NV12 support for ICL. */
 	if (INTEL_GEN(dev_priv) &gt;= 11)
 		return skl_ddb_entry_write(dev_priv,
 					   PLANE_BUF_CFG(pipe, plane_id),
</pre>
    </blockquote>
    <br>
  </body>
</html>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2531eb75bdce..04cef1369e8c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3909,7 +3909,12 @@  skl_ddb_get_hw_plane_state(struct drm_i915_private *dev_priv,
 				      val & PLANE_CTL_ALPHA_MASK);
 
 	val = I915_READ(PLANE_BUF_CFG(pipe, plane_id));
-	val2 = I915_READ(PLANE_NV12_BUF_CFG(pipe, plane_id));
+	/*
+	 * FIXME: add proper NV12 support for ICL. Avoid reading unclaimed
+	 * registers for now.
+	 */
+	if (INTEL_GEN(dev_priv) < 11)
+		val2 = I915_READ(PLANE_NV12_BUF_CFG(pipe, plane_id));
 
 	if (fourcc == DRM_FORMAT_NV12) {
 		skl_ddb_entry_init_from_hw(dev_priv,
@@ -4977,6 +4982,7 @@  static void skl_write_plane_wm(struct intel_crtc *intel_crtc,
 
 	skl_ddb_entry_write(dev_priv, PLANE_BUF_CFG(pipe, plane_id),
 			    &ddb->plane[pipe][plane_id]);
+	/* FIXME: add proper NV12 support for ICL. */
 	if (INTEL_GEN(dev_priv) >= 11)
 		return skl_ddb_entry_write(dev_priv,
 					   PLANE_BUF_CFG(pipe, plane_id),