Re: [Intel-gfx] [PATCH] drm/i915/display: Prevent double YUV range correction on HDR planes
On Wed, Dec 16, 2020 at 01:46:58AM -0500, Andres Calderon Jaramillo wrote:
> On Tue, Dec 15, 2020 at 3:13 PM Ville Syrjälä
> wrote:
> >
> > On Tue, Dec 15, 2020 at 03:06:30PM -0500, Andres Calderon Jaramillo wrote:
> > > On Tue, Dec 15, 2020 at 1:01 PM Ville Syrjälä
> > > wrote:
> > > >
> > > > On Mon, Dec 14, 2020 at 10:57:03PM +, Shankar, Uma wrote:
> > > > >
> > > > >
> > > > > > -Original Message-
> > > > > > From: andrescj via sendgmr
> > > > > >
> > > > > > On Behalf Of Andres Calderon Jaramillo
> > > > > > Sent: Tuesday, December 15, 2020 3:50 AM
> > > > > > To: intel-gfx@lists.freedesktop.org
> > > > > > Cc: Shankar, Uma ; Venkatesh Reddy, Sushma
> > > > > > ; seanp...@chromium.org; Andres
> > > > > > Calderon Jaramillo
> > > > > > Subject: [PATCH] drm/i915/display: Prevent double YUV range
> > > > > > correction on HDR
> > > > > > planes
> > > > > >
> > > > > > From: Andres Calderon Jaramillo
> > > > > >
> > > > > > Prevent the ICL HDR plane pipeline from performing YUV color range
> > > > > > correction
> > > > > > twice when the input is in limited range.
> > > > > >
> > > > > > Before this patch the following could happen: user space gives us a
> > > > > > YUV buffer in
> > > > > > limited range; per the pipeline in [1], the plane would first go
> > > > > > through a "YUV
> > > > > > Range correct" stage that expands the range; the plane would then
> > > > > > go through
> > > > > > the "Input CSC" stage which would also expand the range because
> > > > > > icl_program_input_csc() would use a matrix and an offset that
> > > > > > assume limited-
> > > > > > range input; this would ultimately cause dark and light colors to
> > > > > > appear darker
> > > > > > and lighter than they should respectively.
> > > > > >
> > > > > > This is an issue because if a buffer switches between being scanned
> > > > > > out and
> > > > > > being composited with the GPU, the user will see a color difference.
> > > > > > If this switching happens quickly and frequently, the user will
> > > > > > perceive this as a
> > > > > > flickering.
> > > > > >
> > > > > > [1]
> > > > > > https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-icllp-
> > > > > > vol12-displayengine_0.pdf#page=281
> > > > >
> > > > > Change looks good to me, double conversion should not be done.
> > > > > Plane input csc coefficients take care of the limited range.
> > > >
> > > > Might make sense to actually use the hw range correction instead.
> > > > Would avoid having to keep around two sets of coefficients.
> > > >
> > > > Also the question now becomes: How can our tests be passing if
> > > > we're doing the range correction twice?
> > > >
> > >
> > > I also considered just removing the limited-range matrix/offsets from
> > > icl_program_input_csc(). However, I figured that since the "Input CSC"
> > > stage must happen regardless of range correction, maybe it would be a
> > > gain to disable the "YUV Range Correction" stage. However, I'm not
> > > familiar with the hardware details, so I don't know for sure. I don't
> > > feel strongly either way; let me know what you'd prefer.
> >
> > IIRC we use the fixed function range compression + full range csc
> > approach on chv as well. Wouldn't hurt to do the same thing on
> > icl+ IMO.
>
> Sounds good! Thanks for the review.
>
> >
> > >
> > > I'm also curious about the testing. How do the tests work? I assume
> > > they are in Intel's CI and not open sourced? Do they use the DRM
> > > writeback connector (I didn't think this was implemented for i915
> > > yet)?
> >
> > Tests are here:
> > git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
> >
> > In particular kms_plane/pixel-format tests should rather be failing I
> > think. Unless we've relaxed things a bit too much to get the crcs to
> > match when the results are close enough.
> >
>
> Nice. I took a look at the tests you referenced. It seems that we may
> have just gotten unlucky with the color choice, but I'm not sure why
> (I kind of expected to still get a color difference with the current
> colors).
Hmm. I guess we need to think a bit more about the colors used.
> I tried changing the color in [1] to {0.0f, 0.2f, 0.2f}, and
> I got "CRC mismatches" failures for multiple formats including NV12,
> C8, and YUYV. Interestingly, I had tried something like this multiple
> times, and I couldn't get the CRC mismatch at first. I don't know what
> changed. Maybe the mode was in a bad state and when I rebooted the
> device, it was good.
>
> Note, with that color change and with this patch, I still get "CRC
> mismatches" failures but only for C8. I'm unfamiliar with that format,
> but it seems like a different problem.
We render the image as RGB332 for C8 so it can only have 2^3 or 2^2
levels. Whereas for the 8bpc reference image we only chop off the
results to 5 msbs with the LUT, which gets us 2^5 levels. So the
choice of color would have to be constrained to fit the RGB332
limits to get the
Re: [Intel-gfx] [PATCH] drm/i915/display: Prevent double YUV range correction on HDR planes
On Tue, Dec 15, 2020 at 3:13 PM Ville Syrjälä
wrote:
>
> On Tue, Dec 15, 2020 at 03:06:30PM -0500, Andres Calderon Jaramillo wrote:
> > On Tue, Dec 15, 2020 at 1:01 PM Ville Syrjälä
> > wrote:
> > >
> > > On Mon, Dec 14, 2020 at 10:57:03PM +, Shankar, Uma wrote:
> > > >
> > > >
> > > > > -Original Message-
> > > > > From: andrescj via sendgmr
> > > > >
> > > > > On Behalf Of Andres Calderon Jaramillo
> > > > > Sent: Tuesday, December 15, 2020 3:50 AM
> > > > > To: intel-gfx@lists.freedesktop.org
> > > > > Cc: Shankar, Uma ; Venkatesh Reddy, Sushma
> > > > > ; seanp...@chromium.org; Andres
> > > > > Calderon Jaramillo
> > > > > Subject: [PATCH] drm/i915/display: Prevent double YUV range
> > > > > correction on HDR
> > > > > planes
> > > > >
> > > > > From: Andres Calderon Jaramillo
> > > > >
> > > > > Prevent the ICL HDR plane pipeline from performing YUV color range
> > > > > correction
> > > > > twice when the input is in limited range.
> > > > >
> > > > > Before this patch the following could happen: user space gives us a
> > > > > YUV buffer in
> > > > > limited range; per the pipeline in [1], the plane would first go
> > > > > through a "YUV
> > > > > Range correct" stage that expands the range; the plane would then go
> > > > > through
> > > > > the "Input CSC" stage which would also expand the range because
> > > > > icl_program_input_csc() would use a matrix and an offset that assume
> > > > > limited-
> > > > > range input; this would ultimately cause dark and light colors to
> > > > > appear darker
> > > > > and lighter than they should respectively.
> > > > >
> > > > > This is an issue because if a buffer switches between being scanned
> > > > > out and
> > > > > being composited with the GPU, the user will see a color difference.
> > > > > If this switching happens quickly and frequently, the user will
> > > > > perceive this as a
> > > > > flickering.
> > > > >
> > > > > [1]
> > > > > https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-icllp-
> > > > > vol12-displayengine_0.pdf#page=281
> > > >
> > > > Change looks good to me, double conversion should not be done.
> > > > Plane input csc coefficients take care of the limited range.
> > >
> > > Might make sense to actually use the hw range correction instead.
> > > Would avoid having to keep around two sets of coefficients.
> > >
> > > Also the question now becomes: How can our tests be passing if
> > > we're doing the range correction twice?
> > >
> >
> > I also considered just removing the limited-range matrix/offsets from
> > icl_program_input_csc(). However, I figured that since the "Input CSC"
> > stage must happen regardless of range correction, maybe it would be a
> > gain to disable the "YUV Range Correction" stage. However, I'm not
> > familiar with the hardware details, so I don't know for sure. I don't
> > feel strongly either way; let me know what you'd prefer.
>
> IIRC we use the fixed function range compression + full range csc
> approach on chv as well. Wouldn't hurt to do the same thing on
> icl+ IMO.
Sounds good! Thanks for the review.
>
> >
> > I'm also curious about the testing. How do the tests work? I assume
> > they are in Intel's CI and not open sourced? Do they use the DRM
> > writeback connector (I didn't think this was implemented for i915
> > yet)?
>
> Tests are here:
> git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
>
> In particular kms_plane/pixel-format tests should rather be failing I
> think. Unless we've relaxed things a bit too much to get the crcs to
> match when the results are close enough.
>
Nice. I took a look at the tests you referenced. It seems that we may
have just gotten unlucky with the color choice, but I'm not sure why
(I kind of expected to still get a color difference with the current
colors). I tried changing the color in [1] to {0.0f, 0.2f, 0.2f}, and
I got "CRC mismatches" failures for multiple formats including NV12,
C8, and YUYV. Interestingly, I had tried something like this multiple
times, and I couldn't get the CRC mismatch at first. I don't know what
changed. Maybe the mode was in a bad state and when I rebooted the
device, it was good.
Note, with that color change and with this patch, I still get "CRC
mismatches" failures but only for C8. I'm unfamiliar with that format,
but it seems like a different problem.
[1]
https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/igt-gpu-tools/tests/kms_plane.c;l=383;drc=c8a9aa95e2c5c8d9d39f4f9388a7d602a2e64311
> >
> > > >
> > > > Reviewed-by: Uma Shankar
> > > >
> > > > > Signed-off-by: Andres Calderon Jaramillo
> > > > > ---
> > > > > drivers/gpu/drm/i915/display/intel_display.c | 7 +++
> > > > > 1 file changed, 7 insertions(+)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > index 761be8deaa9b..aeea344b06ad 100644
> > > > > ---
Re: [Intel-gfx] [PATCH] drm/i915/display: Prevent double YUV range correction on HDR planes
On Tue, Dec 15, 2020 at 1:01 PM Ville Syrjälä
wrote:
>
> On Mon, Dec 14, 2020 at 10:57:03PM +, Shankar, Uma wrote:
> >
> >
> > > -Original Message-
> > > From: andrescj via sendgmr
> > >
> > > On Behalf Of Andres Calderon Jaramillo
> > > Sent: Tuesday, December 15, 2020 3:50 AM
> > > To: intel-gfx@lists.freedesktop.org
> > > Cc: Shankar, Uma ; Venkatesh Reddy, Sushma
> > > ; seanp...@chromium.org; Andres
> > > Calderon Jaramillo
> > > Subject: [PATCH] drm/i915/display: Prevent double YUV range correction on
> > > HDR
> > > planes
> > >
> > > From: Andres Calderon Jaramillo
> > >
> > > Prevent the ICL HDR plane pipeline from performing YUV color range
> > > correction
> > > twice when the input is in limited range.
> > >
> > > Before this patch the following could happen: user space gives us a YUV
> > > buffer in
> > > limited range; per the pipeline in [1], the plane would first go through
> > > a "YUV
> > > Range correct" stage that expands the range; the plane would then go
> > > through
> > > the "Input CSC" stage which would also expand the range because
> > > icl_program_input_csc() would use a matrix and an offset that assume
> > > limited-
> > > range input; this would ultimately cause dark and light colors to appear
> > > darker
> > > and lighter than they should respectively.
> > >
> > > This is an issue because if a buffer switches between being scanned out
> > > and
> > > being composited with the GPU, the user will see a color difference.
> > > If this switching happens quickly and frequently, the user will perceive
> > > this as a
> > > flickering.
> > >
> > > [1]
> > > https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-icllp-
> > > vol12-displayengine_0.pdf#page=281
> >
> > Change looks good to me, double conversion should not be done.
> > Plane input csc coefficients take care of the limited range.
>
> Might make sense to actually use the hw range correction instead.
> Would avoid having to keep around two sets of coefficients.
>
> Also the question now becomes: How can our tests be passing if
> we're doing the range correction twice?
>
I also considered just removing the limited-range matrix/offsets from
icl_program_input_csc(). However, I figured that since the "Input CSC"
stage must happen regardless of range correction, maybe it would be a
gain to disable the "YUV Range Correction" stage. However, I'm not
familiar with the hardware details, so I don't know for sure. I don't
feel strongly either way; let me know what you'd prefer.
I'm also curious about the testing. How do the tests work? I assume
they are in Intel's CI and not open sourced? Do they use the DRM
writeback connector (I didn't think this was implemented for i915
yet)?
> >
> > Reviewed-by: Uma Shankar
> >
> > > Signed-off-by: Andres Calderon Jaramillo
> > > ---
> > > drivers/gpu/drm/i915/display/intel_display.c | 7 +++
> > > 1 file changed, 7 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > index 761be8deaa9b..aeea344b06ad 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -4811,6 +4811,13 @@ u32 glk_plane_color_ctl(const struct
> > > intel_crtc_state
> > > *crtc_state,
> > > plane_color_ctl |=
> > > PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
> > > } else if (fb->format->is_yuv) {
> > > plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
> > > +
> > > + /*
> > > +* Disable the YUV range correction stage because the input
> > > CSC
> > > +* stage already takes care of range conversion by using
> > > separate
> > > +* matrices and offsets depending on the color range.
> > > +*/
> > > + plane_color_ctl |=
> > > PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
> > > }
> > >
> > > return plane_color_ctl;
> > > --
> > > 2.29.2.684.gfbc64c5ab5-goog
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/display: Prevent double YUV range correction on HDR planes
From: Andres Calderon Jaramillo
Prevent the ICL HDR plane pipeline from performing YUV color range
correction twice when the input is in limited range.
Before this patch the following could happen: user space gives us a YUV
buffer in limited range; per the pipeline in [1], the plane would first
go through a "YUV Range correct" stage that expands the range; the plane
would then go through the "Input CSC" stage which would also expand the
range because icl_program_input_csc() would use a matrix and an offset
that assume limited-range input; this would ultimately cause dark and
light colors to appear darker and lighter than they should respectively.
This is an issue because if a buffer switches between being scanned out
and being composited with the GPU, the user will see a color difference.
If this switching happens quickly and frequently, the user will perceive
this as a flickering.
[1]
https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-icllp-vol12-displayengine_0.pdf#page=281
Signed-off-by: Andres Calderon Jaramillo
---
drivers/gpu/drm/i915/display/intel_display.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 761be8deaa9b..aeea344b06ad 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -4811,6 +4811,13 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state
*crtc_state,
plane_color_ctl |=
PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
} else if (fb->format->is_yuv) {
plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
+
+ /*
+* Disable the YUV range correction stage because the input CSC
+* stage already takes care of range conversion by using
separate
+* matrices and offsets depending on the color range.
+*/
+ plane_color_ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
}
return plane_color_ctl;
--
2.29.2.684.gfbc64c5ab5-goog
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/display: Prevent double YUV range correction on HDR planes
On Tue, Dec 15, 2020 at 03:06:30PM -0500, Andres Calderon Jaramillo wrote:
> On Tue, Dec 15, 2020 at 1:01 PM Ville Syrjälä
> wrote:
> >
> > On Mon, Dec 14, 2020 at 10:57:03PM +, Shankar, Uma wrote:
> > >
> > >
> > > > -Original Message-
> > > > From: andrescj via sendgmr
> > > >
> > > > On Behalf Of Andres Calderon Jaramillo
> > > > Sent: Tuesday, December 15, 2020 3:50 AM
> > > > To: intel-gfx@lists.freedesktop.org
> > > > Cc: Shankar, Uma ; Venkatesh Reddy, Sushma
> > > > ; seanp...@chromium.org; Andres
> > > > Calderon Jaramillo
> > > > Subject: [PATCH] drm/i915/display: Prevent double YUV range correction
> > > > on HDR
> > > > planes
> > > >
> > > > From: Andres Calderon Jaramillo
> > > >
> > > > Prevent the ICL HDR plane pipeline from performing YUV color range
> > > > correction
> > > > twice when the input is in limited range.
> > > >
> > > > Before this patch the following could happen: user space gives us a YUV
> > > > buffer in
> > > > limited range; per the pipeline in [1], the plane would first go
> > > > through a "YUV
> > > > Range correct" stage that expands the range; the plane would then go
> > > > through
> > > > the "Input CSC" stage which would also expand the range because
> > > > icl_program_input_csc() would use a matrix and an offset that assume
> > > > limited-
> > > > range input; this would ultimately cause dark and light colors to
> > > > appear darker
> > > > and lighter than they should respectively.
> > > >
> > > > This is an issue because if a buffer switches between being scanned out
> > > > and
> > > > being composited with the GPU, the user will see a color difference.
> > > > If this switching happens quickly and frequently, the user will
> > > > perceive this as a
> > > > flickering.
> > > >
> > > > [1]
> > > > https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-icllp-
> > > > vol12-displayengine_0.pdf#page=281
> > >
> > > Change looks good to me, double conversion should not be done.
> > > Plane input csc coefficients take care of the limited range.
> >
> > Might make sense to actually use the hw range correction instead.
> > Would avoid having to keep around two sets of coefficients.
> >
> > Also the question now becomes: How can our tests be passing if
> > we're doing the range correction twice?
> >
>
> I also considered just removing the limited-range matrix/offsets from
> icl_program_input_csc(). However, I figured that since the "Input CSC"
> stage must happen regardless of range correction, maybe it would be a
> gain to disable the "YUV Range Correction" stage. However, I'm not
> familiar with the hardware details, so I don't know for sure. I don't
> feel strongly either way; let me know what you'd prefer.
IIRC we use the fixed function range compression + full range csc
approach on chv as well. Wouldn't hurt to do the same thing on
icl+ IMO.
>
> I'm also curious about the testing. How do the tests work? I assume
> they are in Intel's CI and not open sourced? Do they use the DRM
> writeback connector (I didn't think this was implemented for i915
> yet)?
Tests are here:
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
In particular kms_plane/pixel-format tests should rather be failing I
think. Unless we've relaxed things a bit too much to get the crcs to
match when the results are close enough.
>
> > >
> > > Reviewed-by: Uma Shankar
> > >
> > > > Signed-off-by: Andres Calderon Jaramillo
> > > > ---
> > > > drivers/gpu/drm/i915/display/intel_display.c | 7 +++
> > > > 1 file changed, 7 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > > index 761be8deaa9b..aeea344b06ad 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > @@ -4811,6 +4811,13 @@ u32 glk_plane_color_ctl(const struct
> > > > intel_crtc_state
> > > > *crtc_state,
> > > > plane_color_ctl |=
> > > > PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
> > > > } else if (fb->format->is_yuv) {
> > > > plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
> > > > +
> > > > + /*
> > > > +* Disable the YUV range correction stage because the input
> > > > CSC
> > > > +* stage already takes care of range conversion by using
> > > > separate
> > > > +* matrices and offsets depending on the color range.
> > > > +*/
> > > > + plane_color_ctl |=
> > > > PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
> > > > }
> > > >
> > > > return plane_color_ctl;
> > > > --
> > > > 2.29.2.684.gfbc64c5ab5-goog
> > >
> > > ___
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Ville Syrjälä
> > Intel
--
Ville Syrjälä
Intel
Re: [Intel-gfx] [PATCH] drm/i915/display: Prevent double YUV range correction on HDR planes
On Mon, Dec 14, 2020 at 10:57:03PM +, Shankar, Uma wrote:
>
>
> > -Original Message-
> > From: andrescj via sendgmr
> >
> > On Behalf Of Andres Calderon Jaramillo
> > Sent: Tuesday, December 15, 2020 3:50 AM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Shankar, Uma ; Venkatesh Reddy, Sushma
> > ; seanp...@chromium.org; Andres
> > Calderon Jaramillo
> > Subject: [PATCH] drm/i915/display: Prevent double YUV range correction on
> > HDR
> > planes
> >
> > From: Andres Calderon Jaramillo
> >
> > Prevent the ICL HDR plane pipeline from performing YUV color range
> > correction
> > twice when the input is in limited range.
> >
> > Before this patch the following could happen: user space gives us a YUV
> > buffer in
> > limited range; per the pipeline in [1], the plane would first go through a
> > "YUV
> > Range correct" stage that expands the range; the plane would then go through
> > the "Input CSC" stage which would also expand the range because
> > icl_program_input_csc() would use a matrix and an offset that assume
> > limited-
> > range input; this would ultimately cause dark and light colors to appear
> > darker
> > and lighter than they should respectively.
> >
> > This is an issue because if a buffer switches between being scanned out and
> > being composited with the GPU, the user will see a color difference.
> > If this switching happens quickly and frequently, the user will perceive
> > this as a
> > flickering.
> >
> > [1]
> > https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-icllp-
> > vol12-displayengine_0.pdf#page=281
>
> Change looks good to me, double conversion should not be done.
> Plane input csc coefficients take care of the limited range.
Might make sense to actually use the hw range correction instead.
Would avoid having to keep around two sets of coefficients.
Also the question now becomes: How can our tests be passing if
we're doing the range correction twice?
>
> Reviewed-by: Uma Shankar
>
> > Signed-off-by: Andres Calderon Jaramillo
> > ---
> > drivers/gpu/drm/i915/display/intel_display.c | 7 +++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 761be8deaa9b..aeea344b06ad 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -4811,6 +4811,13 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state
> > *crtc_state,
> > plane_color_ctl |=
> > PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
> > } else if (fb->format->is_yuv) {
> > plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
> > +
> > + /*
> > +* Disable the YUV range correction stage because the input CSC
> > +* stage already takes care of range conversion by using
> > separate
> > +* matrices and offsets depending on the color range.
> > +*/
> > + plane_color_ctl |=
> > PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
> > }
> >
> > return plane_color_ctl;
> > --
> > 2.29.2.684.gfbc64c5ab5-goog
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/display: Prevent double YUV range correction on HDR planes
> -Original Message-
> From: andrescj via sendgmr
> On Behalf Of Andres Calderon Jaramillo
> Sent: Tuesday, December 15, 2020 3:50 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: Shankar, Uma ; Venkatesh Reddy, Sushma
> ; seanp...@chromium.org; Andres
> Calderon Jaramillo
> Subject: [PATCH] drm/i915/display: Prevent double YUV range correction on HDR
> planes
>
> From: Andres Calderon Jaramillo
>
> Prevent the ICL HDR plane pipeline from performing YUV color range correction
> twice when the input is in limited range.
>
> Before this patch the following could happen: user space gives us a YUV
> buffer in
> limited range; per the pipeline in [1], the plane would first go through a
> "YUV
> Range correct" stage that expands the range; the plane would then go through
> the "Input CSC" stage which would also expand the range because
> icl_program_input_csc() would use a matrix and an offset that assume limited-
> range input; this would ultimately cause dark and light colors to appear
> darker
> and lighter than they should respectively.
>
> This is an issue because if a buffer switches between being scanned out and
> being composited with the GPU, the user will see a color difference.
> If this switching happens quickly and frequently, the user will perceive this
> as a
> flickering.
>
> [1] https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-icllp-
> vol12-displayengine_0.pdf#page=281
Change looks good to me, double conversion should not be done.
Plane input csc coefficients take care of the limited range.
Reviewed-by: Uma Shankar
> Signed-off-by: Andres Calderon Jaramillo
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 761be8deaa9b..aeea344b06ad 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -4811,6 +4811,13 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state
> *crtc_state,
> plane_color_ctl |=
> PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
> } else if (fb->format->is_yuv) {
> plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
> +
> + /*
> + * Disable the YUV range correction stage because the input CSC
> + * stage already takes care of range conversion by using
> separate
> + * matrices and offsets depending on the color range.
> + */
> + plane_color_ctl |=
> PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
> }
>
> return plane_color_ctl;
> --
> 2.29.2.684.gfbc64c5ab5-goog
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
