On 4/30/2019 4:09 PM, Ville Syrjälä wrote:
On Tue, Apr 30, 2019 at 10:22:40AM +0530, Sharma, Shashank wrote:
On 4/26/2019 8:07 PM, Ville Syrjälä wrote:
On Fri, Apr 26, 2019 at 06:40:11PM +0530, Sharma, Shashank wrote:
On 4/13/2019 12:00 AM, Ville Syrjala wrote:
From: Ville Syrjälä <ville.syrj...@linux.intel.com>

The pipe has a special HDR mode with higher precision when only
HDR planes are active. Let's use it.

Curiously this fixes the kms_color gamma/degamma tests when
using a HDR plane, which is always the case unless one hacks
the test to use an SDR plane. If one does hack the test to use
an SDR plane it does pass already.

I have no actual explanation how the output after the gamma
LUT can be different between the two modes. The way the tests
are written should mean that the output should be identical
between the solid color vs. the gradient. But clearly that
somehow doesn't hold true for the HDR planes in non-HDR pipe
mode. Anyways, as long as we stick to one type of plane the
test should produce sensible results now.

Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
---
    drivers/gpu/drm/i915/i915_reg.h      |  1 +
    drivers/gpu/drm/i915/intel_display.c |  7 +++++++
    drivers/gpu/drm/i915/intel_sprite.h  | 12 ++++++++----
    3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8ad2f0a03f28..90d60ecd3317 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5767,6 +5767,7 @@ enum {
    #define _PIPE_MISC_B                        0x71030
    #define   PIPEMISC_YUV420_ENABLE    (1 << 27)
    #define   PIPEMISC_YUV420_MODE_FULL_BLEND (1 << 26)
+#define   PIPEMISC_HDR_MODE            (1 << 23) /* icl+ */
    #define   PIPEMISC_OUTPUT_COLORSPACE_YUV  (1 << 11)
    #define   PIPEMISC_DITHER_BPC_MASK  (7 << 5)
    #define   PIPEMISC_DITHER_8_BPC             (0 << 5)
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 490bd49ff42a..d0dbdbd5db3f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4055,6 +4055,9 @@ static void intel_update_pipe_config(const struct 
intel_crtc_state *old_crtc_sta
                        ironlake_pfit_disable(old_crtc_state);
        }
+ if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
+               bdw_set_pipemisc(new_crtc_state);
+
        if (INTEL_GEN(dev_priv) >= 11)
                icl_set_pipe_chicken(crtc);
    }
@@ -8869,6 +8872,10 @@ static void bdw_set_pipemisc(const struct 
intel_crtc_state *crtc_state)
                val |= PIPEMISC_YUV420_ENABLE |
                        PIPEMISC_YUV420_MODE_FULL_BLEND;
+ if (INTEL_GEN(dev_priv) >= 11 &&
+           (crtc_state->active_planes & ~icl_hdr_plane_mask()) == 0)
+               val |= PIPEMISC_HDR_MODE;
+
Shouldn't we check if the content being played on plane is HDR before
enabling this bit (even though I am not sure if there is any harm in
doing that)? Or maybe check the connector->output_hdr_metadata ? Most of
the times we would be sending SDR buffers on this plane. What happens
exactly when we set this bit ? The bspec says:

"This field enables the HDR mode, allowing for higher precision output
from the HDR supporting planes and bypassing the SDR planes in blending. "
I think the bit is just misnamed (like most things with "HDR" in their
name). It's just a "gimme moar precision" bit.
Lets make this a bit more clear, may be rename the bit to
PIPEMISC_HDR_PRECISION_MODE instead?
Then it won't match the spec.

Well we are keeping HDR_MODE aren't we ? may be PIPEMISC_HDR_MODE_PRECISION if that makes us closer to spec ?

- Shashank

With that change, this patch is

Reviewed-by: Shashank Sharma <shashank.sha...@intel.com>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to