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@xxxxxxxxxxxxxxx] > > > Sent: Thursday, May 2, 2019 3:45 PM > > > To: Sharma, Shashank <shashank.sharma@xxxxxxxxx> > > > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Syrjala, Ville <ville.syrjal > > > a@xxxxxxxxx>; Lankhorst, > > > Maarten <maarten.lankhorst@xxxxxxxxx> > > > Subject: Re: [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@xxxxxxxxx> > > > > Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxx> > > > > Cc: Uma Shankar <uma.shankar@xxxxxxxxx> > > > > Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxxxx> > > > > --- > > > > 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:
smime.p7s
Description: S/MIME cryptographic signature
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
