* [PATCH] drm/i915/GLK: Properly handle plane CSC for BT2020 framebuffers
@ 2019-05-02 9:49 Shashank Sharma
2019-05-02 10:15 ` Ville Syrjälä
2019-05-02 14:08 ` ✓ Fi.CI.BAT: success for " Patchwork
0 siblings, 2 replies; 6+ messages in thread
From: Shashank Sharma @ 2019-05-02 9:49 UTC (permalink / raw)
To: intel-gfx; +Cc: Ville Syrjala, Maarten Lankhorst
Framebuffer formats P01x are supported by GLK, but the function which
handles CSC on plane color control register, still expectes the input
buffer to be REC709. This can cause inaccurate output for direct P01x
flips.
This patch checks if the color_encoding property is set to YCBCR_2020,
and enables the corresponding color conversion mode on plane CSC.
PS: renamed variable plane_color_ctl to color_ctl for 80 char stuff.
Cc: Ville Syrjala <ville.syrjala@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@intel.com>
Cc: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dd65d7c521c1..2d4d3128bf1f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3868,24 +3868,30 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
to_i915(plane_state->base.plane->dev);
const struct drm_framebuffer *fb = plane_state->base.fb;
struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
- u32 plane_color_ctl = 0;
+ u32 color_ctl = 0;
- plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
- plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state);
+ color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
+ color_ctl |= glk_plane_color_ctl_alpha(plane_state);
if (fb->format->is_yuv && !icl_is_hdr_plane(dev_priv, plane->id)) {
- if (plane_state->base.color_encoding == DRM_COLOR_YCBCR_BT709)
- plane_color_ctl |= PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
- else
- plane_color_ctl |= PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709;
+ switch (plane_state->base.color_encoding) {
+ case DRM_COLOR_YCBCR_BT709:
+ color_ctl |= PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
+ break;
+ case DRM_COLOR_YCBCR_BT2020:
+ color_ctl |= PLANE_COLOR_CSC_MODE_YUV2020_TO_RGB2020;
+ break;
+ default:
+ color_ctl |= PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709;
+ }
if (plane_state->base.color_range == DRM_COLOR_YCBCR_FULL_RANGE)
- plane_color_ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
+ color_ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
} else if (fb->format->is_yuv) {
- plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
+ color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
}
- return plane_color_ctl;
+ return color_ctl;
}
static int
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/GLK: Properly handle plane CSC for BT2020 framebuffers
2019-05-02 9:49 [PATCH] drm/i915/GLK: Properly handle plane CSC for BT2020 framebuffers Shashank Sharma
@ 2019-05-02 10:15 ` Ville Syrjälä
2019-05-02 10:40 ` Sharma, Shashank
2019-05-02 14:08 ` ✓ Fi.CI.BAT: success for " Patchwork
1 sibling, 1 reply; 6+ messages in thread
From: Ville Syrjälä @ 2019-05-02 10:15 UTC (permalink / raw)
To: Shashank Sharma; +Cc: intel-gfx, Ville Syrjala, Maarten Lankhorst
On Thu, May 02, 2019 at 03:19:42PM +0530, Shashank Sharma wrote:
> Framebuffer formats P01x are supported by GLK, but the function which
> handles CSC on plane color control register, still expectes the input
> buffer to be REC709. This can cause inaccurate output for direct P01x
> flips.
>
> This patch checks if the color_encoding property is set to YCBCR_2020,
> and enables the corresponding color conversion mode on plane CSC.
>
> PS: renamed variable plane_color_ctl to color_ctl for 80 char stuff.
>
> Cc: Ville Syrjala <ville.syrjala@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@intel.com>
> Cc: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 26 ++++++++++++++++----------
> 1 file changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index dd65d7c521c1..2d4d3128bf1f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3868,24 +3868,30 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
> to_i915(plane_state->base.plane->dev);
> const struct drm_framebuffer *fb = plane_state->base.fb;
> struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
> - u32 plane_color_ctl = 0;
> + u32 color_ctl = 0;
>
> - plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
> - plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state);
> + color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
> + color_ctl |= glk_plane_color_ctl_alpha(plane_state);
>
> if (fb->format->is_yuv && !icl_is_hdr_plane(dev_priv, plane->id)) {
> - if (plane_state->base.color_encoding == DRM_COLOR_YCBCR_BT709)
> - plane_color_ctl |= PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
> - else
> - plane_color_ctl |= PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709;
> + switch (plane_state->base.color_encoding) {
> + case DRM_COLOR_YCBCR_BT709:
> + color_ctl |= PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
> + break;
> + case DRM_COLOR_YCBCR_BT2020:
> + color_ctl |= PLANE_COLOR_CSC_MODE_YUV2020_TO_RGB2020;
> + break;
> + default:
> + color_ctl |= PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709;
> + }
This isn't going to do anything without adjusting the property supported
encodings as well.
>
> if (plane_state->base.color_range == DRM_COLOR_YCBCR_FULL_RANGE)
> - plane_color_ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
> + color_ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
> } else if (fb->format->is_yuv) {
> - plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
> + color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
> }
>
> - return plane_color_ctl;
> + return color_ctl;
> }
>
> static int
> --
> 2.17.1
>
> _______________________________________________
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/GLK: Properly handle plane CSC for BT2020 framebuffers
2019-05-02 10:15 ` Ville Syrjälä
@ 2019-05-02 10:40 ` Sharma, Shashank
2019-05-02 11:51 ` Ville Syrjälä
0 siblings, 1 reply; 6+ messages in thread
From: Sharma, Shashank @ 2019-05-02 10:40 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, Syrjala, Ville, Lankhorst, Maarten
> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> Sent: Thursday, May 2, 2019 3:45 PM
> To: Sharma, Shashank <shashank.sharma@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst,
> Maarten <maarten.lankhorst@intel.com>
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/GLK: Properly handle plane CSC for
> BT2020 framebuffers
>
> On Thu, May 02, 2019 at 03:19:42PM +0530, Shashank Sharma wrote:
> > Framebuffer formats P01x are supported by GLK, but the function which
> > handles CSC on plane color control register, still expectes the input
> > buffer to be REC709. This can cause inaccurate output for direct P01x
> > flips.
> >
> > This patch checks if the color_encoding property is set to YCBCR_2020,
> > and enables the corresponding color conversion mode on plane CSC.
> >
> > PS: renamed variable plane_color_ctl to color_ctl for 80 char stuff.
> >
> > Cc: Ville Syrjala <ville.syrjala@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@intel.com>
> > Cc: Uma Shankar <uma.shankar@intel.com>
> > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 26 ++++++++++++++++----------
> > 1 file changed, 16 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index dd65d7c521c1..2d4d3128bf1f 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3868,24 +3868,30 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state
> *crtc_state,
> > to_i915(plane_state->base.plane->dev);
> > const struct drm_framebuffer *fb = plane_state->base.fb;
> > struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
> > - u32 plane_color_ctl = 0;
> > + u32 color_ctl = 0;
> >
> > - plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
> > - plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state);
> > + color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
> > + color_ctl |= glk_plane_color_ctl_alpha(plane_state);
> >
> > if (fb->format->is_yuv && !icl_is_hdr_plane(dev_priv, plane->id)) {
> > - if (plane_state->base.color_encoding == DRM_COLOR_YCBCR_BT709)
> > - plane_color_ctl |=
> PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
> > - else
> > - plane_color_ctl |=
> PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709;
> > + switch (plane_state->base.color_encoding) {
> > + case DRM_COLOR_YCBCR_BT709:
> > + color_ctl |=
> PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
> > + break;
> > + case DRM_COLOR_YCBCR_BT2020:
> > + color_ctl |=
> PLANE_COLOR_CSC_MODE_YUV2020_TO_RGB2020;
> > + break;
> > + default:
> > + color_ctl |=
> PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709;
> > + }
>
> This isn't going to do anything without adjusting the property supported encodings as
> well.
>
I might have not understood this comment properly, but, AFAIK, if userspace sets this property well, and we set this color_ctl bit properly, driver is setting PLANE_COLOR_CSC_MODE_YUV2020_TO_RGB2020 bits in GLK color control register. As GLK has a fix function plane CSC, HW will apply a different matrix internally to convert input buffer to RGB_2020 from YCBCR_2020 (earlier this would have been YCBCR_709). So I think we should see visible changes in output. Do you think otherwise ?
- Shashank
> >
> > if (plane_state->base.color_range ==
> DRM_COLOR_YCBCR_FULL_RANGE)
> > - plane_color_ctl |=
> PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
> > + color_ctl |=
> PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
> > } else if (fb->format->is_yuv) {
> > - plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
> > + color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
> > }
> >
> > - return plane_color_ctl;
> > + return color_ctl;
> > }
> >
> > static int
> > --
> > 2.17.1
> >
> > _______________________________________________
> > 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/GLK: Properly handle plane CSC for BT2020 framebuffers
2019-05-02 10:40 ` Sharma, Shashank
@ 2019-05-02 11:51 ` Ville Syrjälä
2019-05-02 14:32 ` Lankhorst, Maarten
0 siblings, 1 reply; 6+ messages in thread
From: Ville Syrjälä @ 2019-05-02 11:51 UTC (permalink / raw)
To: Sharma, Shashank; +Cc: intel-gfx, Syrjala, Ville, Lankhorst, Maarten
On Thu, May 02, 2019 at 10:40:39AM +0000, Sharma, Shashank wrote:
>
>
> > -----Original Message-----
> > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> > Sent: Thursday, May 2, 2019 3:45 PM
> > To: Sharma, Shashank <shashank.sharma@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org; Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst,
> > Maarten <maarten.lankhorst@intel.com>
> > Subject: Re: [Intel-gfx] [PATCH] drm/i915/GLK: Properly handle plane CSC for
> > BT2020 framebuffers
> >
> > On Thu, May 02, 2019 at 03:19:42PM +0530, Shashank Sharma wrote:
> > > Framebuffer formats P01x are supported by GLK, but the function which
> > > handles CSC on plane color control register, still expectes the input
> > > buffer to be REC709. This can cause inaccurate output for direct P01x
> > > flips.
> > >
> > > This patch checks if the color_encoding property is set to YCBCR_2020,
> > > and enables the corresponding color conversion mode on plane CSC.
> > >
> > > PS: renamed variable plane_color_ctl to color_ctl for 80 char stuff.
> > >
> > > Cc: Ville Syrjala <ville.syrjala@intel.com>
> > > Cc: Maarten Lankhorst <maarten.lankhorst@intel.com>
> > > Cc: Uma Shankar <uma.shankar@intel.com>
> > > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/intel_display.c | 26 ++++++++++++++++----------
> > > 1 file changed, 16 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index dd65d7c521c1..2d4d3128bf1f 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -3868,24 +3868,30 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state
> > *crtc_state,
> > > to_i915(plane_state->base.plane->dev);
> > > const struct drm_framebuffer *fb = plane_state->base.fb;
> > > struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
> > > - u32 plane_color_ctl = 0;
> > > + u32 color_ctl = 0;
> > >
> > > - plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
> > > - plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state);
> > > + color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
> > > + color_ctl |= glk_plane_color_ctl_alpha(plane_state);
> > >
> > > if (fb->format->is_yuv && !icl_is_hdr_plane(dev_priv, plane->id)) {
> > > - if (plane_state->base.color_encoding == DRM_COLOR_YCBCR_BT709)
> > > - plane_color_ctl |=
> > PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
> > > - else
> > > - plane_color_ctl |=
> > PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709;
> > > + switch (plane_state->base.color_encoding) {
> > > + case DRM_COLOR_YCBCR_BT709:
> > > + color_ctl |=
> > PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
> > > + break;
> > > + case DRM_COLOR_YCBCR_BT2020:
> > > + color_ctl |=
> > PLANE_COLOR_CSC_MODE_YUV2020_TO_RGB2020;
> > > + break;
> > > + default:
> > > + color_ctl |=
> > PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709;
> > > + }
> >
> > This isn't going to do anything without adjusting the property supported encodings as
> > well.
> >
> I might have not understood this comment properly, but, AFAIK, if userspace sets this property well, and we set this color_ctl bit properly, driver is setting PLANE_COLOR_CSC_MODE_YUV2020_TO_RGB2020 bits in GLK color control register. As GLK has a fix function plane CSC, HW will apply a different matrix internally to convert input buffer to RGB_2020 from YCBCR_2020 (earlier this would have been YCBCR_709). So I think we should see visible changes in output. Do you think otherwise ?
The property won't accept the BT2020 value. Or if it does we have a bug
somewhere.
I guess tests would be nice. Maybe we should extend the kms_plane pixel
format tests to check different YCbCr encodings as well? Or maybe
Maarten's kms_yuv already tests this?
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915/GLK: Properly handle plane CSC for BT2020 framebuffers
2019-05-02 9:49 [PATCH] drm/i915/GLK: Properly handle plane CSC for BT2020 framebuffers Shashank Sharma
2019-05-02 10:15 ` Ville Syrjälä
@ 2019-05-02 14:08 ` Patchwork
1 sibling, 0 replies; 6+ messages in thread
From: Patchwork @ 2019-05-02 14:08 UTC (permalink / raw)
To: Sharma, Shashank; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/GLK: Properly handle plane CSC for BT2020 framebuffers
URL : https://patchwork.freedesktop.org/series/60195/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6026 -> Patchwork_12937
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/60195/revisions/1/mbox/
Known issues
------------
Here are the changes found in Patchwork_12937 that come from known issues:
### IGT changes ###
#### Possible fixes ####
* igt@amdgpu/amd_basic@userptr:
- fi-kbl-8809g: [DMESG-WARN][1] ([fdo#108965]) -> [PASS][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6026/fi-kbl-8809g/igt@amdgpu/amd_basic@userptr.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12937/fi-kbl-8809g/igt@amdgpu/amd_basic@userptr.html
* igt@gem_exec_suspend@basic-s4-devices:
- fi-blb-e6850: [INCOMPLETE][3] ([fdo#107718] / [fdo#110581]) -> [PASS][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6026/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12937/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html
* igt@i915_selftest@live_hangcheck:
- fi-skl-iommu: [INCOMPLETE][5] ([fdo#108602] / [fdo#108744] / [fdo#110581]) -> [PASS][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6026/fi-skl-iommu/igt@i915_selftest@live_hangcheck.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12937/fi-skl-iommu/igt@i915_selftest@live_hangcheck.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#103841]: https://bugs.freedesktop.org/show_bug.cgi?id=103841
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
[fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
[fdo#108965]: https://bugs.freedesktop.org/show_bug.cgi?id=108965
[fdo#110581]: https://bugs.freedesktop.org/show_bug.cgi?id=110581
Participating hosts (52 -> 45)
------------------------------
Additional (1): fi-byt-j1900
Missing (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-apl-guc fi-byt-clapper fi-bdw-samus
Build changes
-------------
* Linux: CI_DRM_6026 -> Patchwork_12937
CI_DRM_6026: 2d2d8d3b9d8896c99c88307a881120885afd2ddb @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4972: f052e49a43cc9704ea5f240df15dd9d3dfed68ab @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_12937: 0809bda74a2027f28cd5aa06bafd0d08c3b0281e @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
0809bda74a20 drm/i915/GLK: Properly handle plane CSC for BT2020 framebuffers
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12937/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/GLK: Properly handle plane CSC for BT2020 framebuffers
2019-05-02 11:51 ` Ville Syrjälä
@ 2019-05-02 14:32 ` Lankhorst, Maarten
0 siblings, 0 replies; 6+ messages in thread
From: Lankhorst, Maarten @ 2019-05-02 14:32 UTC (permalink / raw)
To: ville.syrjala, Sharma, Shashank; +Cc: intel-gfx, Syrjala, Ville
[-- Attachment #1.1: Type: text/plain, Size: 4599 bytes --]
tor 2019-05-02 klockan 14:51 +0300 skrev Ville Syrjälä:
> On Thu, May 02, 2019 at 10:40:39AM +0000, Sharma, Shashank wrote:
> >
> >
> > > -----Original Message-----
> > > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> > > Sent: Thursday, May 2, 2019 3:45 PM
> > > To: Sharma, Shashank <shashank.sharma@intel.com>
> > > Cc: intel-gfx@lists.freedesktop.org; Syrjala, Ville <ville.syrjal
> > > a@intel.com>; Lankhorst,
> > > Maarten <maarten.lankhorst@intel.com>
> > > Subject: Re: [Intel-gfx] [PATCH] drm/i915/GLK: Properly handle
> > > plane CSC for
> > > BT2020 framebuffers
> > >
> > > On Thu, May 02, 2019 at 03:19:42PM +0530, Shashank Sharma wrote:
> > > > Framebuffer formats P01x are supported by GLK, but the function
> > > > which
> > > > handles CSC on plane color control register, still expectes the
> > > > input
> > > > buffer to be REC709. This can cause inaccurate output for
> > > > direct P01x
> > > > flips.
> > > >
> > > > This patch checks if the color_encoding property is set to
> > > > YCBCR_2020,
> > > > and enables the corresponding color conversion mode on plane
> > > > CSC.
> > > >
> > > > PS: renamed variable plane_color_ctl to color_ctl for 80 char
> > > > stuff.
> > > >
> > > > Cc: Ville Syrjala <ville.syrjala@intel.com>
> > > > Cc: Maarten Lankhorst <maarten.lankhorst@intel.com>
> > > > Cc: Uma Shankar <uma.shankar@intel.com>
> > > > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/intel_display.c | 26 ++++++++++++++++--
> > > > --------
> > > > 1 file changed, 16 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > > b/drivers/gpu/drm/i915/intel_display.c
> > > > index dd65d7c521c1..2d4d3128bf1f 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > @@ -3868,24 +3868,30 @@ u32 glk_plane_color_ctl(const struct
> > > > intel_crtc_state
> > >
> > > *crtc_state,
> > > > to_i915(plane_state->base.plane->dev);
> > > > const struct drm_framebuffer *fb = plane_state-
> > > > >base.fb;
> > > > struct intel_plane *plane =
> > > > to_intel_plane(plane_state->base.plane);
> > > > - u32 plane_color_ctl = 0;
> > > > + u32 color_ctl = 0;
> > > >
> > > > - plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
> > > > - plane_color_ctl |=
> > > > glk_plane_color_ctl_alpha(plane_state);
> > > > + color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
> > > > + color_ctl |= glk_plane_color_ctl_alpha(plane_state);
> > > >
> > > > if (fb->format->is_yuv && !icl_is_hdr_plane(dev_priv,
> > > > plane->id)) {
> > > > - if (plane_state->base.color_encoding ==
> > > > DRM_COLOR_YCBCR_BT709)
> > > > - plane_color_ctl |=
> > >
> > > PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
> > > > - else
> > > > - plane_color_ctl |=
> > >
> > > PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709;
> > > > + switch (plane_state->base.color_encoding) {
> > > > + case DRM_COLOR_YCBCR_BT709:
> > > > + color_ctl |=
> > >
> > > PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
> > > > + break;
> > > > + case DRM_COLOR_YCBCR_BT2020:
> > > > + color_ctl |=
> > >
> > > PLANE_COLOR_CSC_MODE_YUV2020_TO_RGB2020;
> > > > + break;
> > > > + default:
> > > > + color_ctl |=
> > >
> > > PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709;
> > > > + }
> > >
> > > This isn't going to do anything without adjusting the property
> > > supported encodings as
> > > well.
> > >
> >
> > I might have not understood this comment properly, but, AFAIK, if
> > userspace sets this property well, and we set this color_ctl bit
> > properly, driver is setting PLANE_COLOR_CSC_MODE_YUV2020_TO_RGB2020
> > bits in GLK color control register. As GLK has a fix function plane
> > CSC, HW will apply a different matrix internally to convert input
> > buffer to RGB_2020 from YCBCR_2020 (earlier this would have been
> > YCBCR_709). So I think we should see visible changes in output. Do
> > you think otherwise ?
>
> The property won't accept the BT2020 value. Or if it does we have a
> bug
> somewhere.
>
> I guess tests would be nice. Maybe we should extend the kms_plane
> pixel
> format tests to check different YCbCr encodings as well? Or maybe
> Maarten's kms_yuv already tests this?
Not yet, unfortunately we have no way to set CSC in igt yet. :(
Best way to do so would be to add a igt_create_fb_yuv() which would a
igt_create_fb that accepts igt color encoding and range as arguments.
~Maarten
[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3282 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-05-02 14:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-02 9:49 [PATCH] drm/i915/GLK: Properly handle plane CSC for BT2020 framebuffers Shashank Sharma
2019-05-02 10:15 ` Ville Syrjälä
2019-05-02 10:40 ` Sharma, Shashank
2019-05-02 11:51 ` Ville Syrjälä
2019-05-02 14:32 ` Lankhorst, Maarten
2019-05-02 14:08 ` ✓ Fi.CI.BAT: success for " Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.