| Message ID | 1540397893-3301-3-git-send-email-uma.shankar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enable Plane Input CSC for ICL | expand |
On Wed, Oct 24, 2018 at 09:48:13PM +0530, Uma Shankar wrote: > Plane input CSC needs to be enabled to convert frambuffers from > YUV to RGB. This is needed for bottom 3 planes on ICL, rest of > the planes have hardcoded conversion and taken care by the legacy > code. Just to confirm, we're specifically talking YUV444(packed) -> RGB here, right? So for formats like NV12 (YUV 420), the chroma upsampler first merges/scales the Y and UV planes to produce YUV444(packed) data which then has this input CSC matrix applied to convert it to RGB? And then after that point there's a separate set of userspace-programmable degamma/CSC/gamma set of steps, which is what your other series "[RFC v5 0/8] Add Plane Color Properties" adds, right? And then after that comes the pipe-level degamma/CSC/gamma... Is there somewhere in the bspec that talks about this input CSC in more detail? I don't see any mention on the general YUV programming or plane capability pages, and the register details themselves are pretty sparse, so it's hard to fully review these for correctness. Based on your code here, it appears the INPUT_CSC follows the same general layout and expected coefficient values as the pipe CSC, but I'm not sure where in the spec to confirm that. Matt > > This patch defines the co-efficient values for YUV to RGB conversion > in BT709 and BT601 formats. It programs the coefficients and enables > the plane input csc unit in hardware. > > Note: This is currently untested and floated to get an early feedback > on the design and implementation for this feature. In parallel, > I will test this on actual ICL hardware and confirm with planar > formats. > > v2: Addressed Maarten's and Ville's review comments and added the > coefficients in a 2D array instead of independent Macros. > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > --- > drivers/gpu/drm/i915/intel_color.c | 49 ++++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_display.c | 24 +++++++++++++----- > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > 3 files changed, 69 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c > index 5127da2..de733cf 100644 > --- a/drivers/gpu/drm/i915/intel_color.c > +++ b/drivers/gpu/drm/i915/intel_color.c > @@ -57,6 +57,11 @@ > #define CSC_RGB_TO_YUV_RV_GV 0xbce89ad8 > #define CSC_RGB_TO_YUV_BV 0x1e080000 > > +/* Preoffset values for YUV to RGB Conversion */ > +#define PREOFF_YUV_TO_RGB_HI 0x800 > +#define PREOFF_YUV_TO_RGB_ME 0xF00 > +#define PREOFF_YUV_TO_RGB_LO 0x800 > + > /* > * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point > * format). This macro takes the coefficient we want transformed and the > @@ -643,6 +648,50 @@ int intel_color_check(struct drm_crtc *crtc, > return -EINVAL; > } > > +void icl_program_input_csc_coeff(const struct intel_crtc_state *crtc_state, > + const struct intel_plane_state *plane_state) > +{ > + struct drm_i915_private *dev_priv = > + to_i915(plane_state->base.plane->dev); > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); > + enum pipe pipe = crtc->pipe; > + struct intel_plane *intel_plane = > + to_intel_plane(plane_state->base.plane); > + enum plane_id plane = intel_plane->id; > + > + static const u32 input_csc_matrix[][6] = { > + /* BT.601 full range YCbCr -> full range RGB */ > + [DRM_COLOR_YCBCR_BT601] = { > + 0x7AF87800, 0x0, 0x8B287800, > + 0x9AC0, 0x7800, 0x7DD8, > + }, > + /* BT.709 full range YCbCr -> full range RGB */ > + [DRM_COLOR_YCBCR_BT709] = { > + 0x7C987800, 0x0, 0x9EF87800, > + 0xABF8, 0x7800, 0x7ED8, > + }, > + }; > + const u32 *csc = input_csc_matrix[plane_state->base.color_encoding]; > + > + I915_WRITE(PLANE_INPUT_CSC_RY_GY(pipe, plane), csc[0]); > + I915_WRITE(PLANE_INPUT_CSC_BY(pipe, plane), csc[1]); > + I915_WRITE(PLANE_INPUT_CSC_RU_GU(pipe, plane), csc[2]); > + I915_WRITE(PLANE_INPUT_CSC_BU(pipe, plane), csc[3]); > + I915_WRITE(PLANE_INPUT_CSC_RV_GV(pipe, plane), csc[4]); > + I915_WRITE(PLANE_INPUT_CSC_BV(pipe, plane), csc[5]); > + > + I915_WRITE(PLANE_INPUT_CSC_PREOFF_HI(pipe, plane), > + PREOFF_YUV_TO_RGB_HI); > + I915_WRITE(PLANE_INPUT_CSC_PREOFF_ME(pipe, plane), > + PREOFF_YUV_TO_RGB_ME); > + I915_WRITE(PLANE_INPUT_CSC_PREOFF_LO(pipe, plane), > + PREOFF_YUV_TO_RGB_LO); > + > + I915_WRITE(PLANE_INPUT_CSC_POSTOFF_HI(pipe, plane), 0x0); > + I915_WRITE(PLANE_INPUT_CSC_POSTOFF_ME(pipe, plane), 0x0); > + I915_WRITE(PLANE_INPUT_CSC_POSTOFF_LO(pipe, plane), 0x0); > +} > + > void intel_color_init(struct drm_crtc *crtc) > { > struct drm_i915_private *dev_priv = to_i915(crtc->dev); > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index fe045ab..be65419 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3666,6 +3666,8 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state, > struct drm_i915_private *dev_priv = > 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; > > if (INTEL_GEN(dev_priv) < 11) { > @@ -3676,13 +3678,23 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state, > plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state); > > if (fb->format->is_yuv) { > - 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; > + if (!icl_is_hdr_plane(plane)) { > + 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; > > - if (plane_state->base.color_range == DRM_COLOR_YCBCR_FULL_RANGE) > - plane_color_ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE; > + if (plane_state->base.color_range == > + DRM_COLOR_YCBCR_FULL_RANGE) > + plane_color_ctl |= > + PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE; > + } else { > + icl_program_input_csc_coeff(crtc_state, plane_state); > + plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE; > + } > } > > return plane_color_ctl; > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index db24308..bd9e946 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -2285,6 +2285,8 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_ > int intel_color_check(struct drm_crtc *crtc, struct drm_crtc_state *state); > void intel_color_set_csc(struct drm_crtc_state *crtc_state); > void intel_color_load_luts(struct drm_crtc_state *crtc_state); > +void icl_program_input_csc_coeff(const struct intel_crtc_state *crtc_state, > + const struct intel_plane_state *plane_state); > > /* intel_lspcon.c */ > bool lspcon_init(struct intel_digital_port *intel_dig_port); > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Op 24-10-18 om 18:18 schreef Uma Shankar: > Plane input CSC needs to be enabled to convert frambuffers from > YUV to RGB. This is needed for bottom 3 planes on ICL, rest of > the planes have hardcoded conversion and taken care by the legacy > code. > > This patch defines the co-efficient values for YUV to RGB conversion > in BT709 and BT601 formats. It programs the coefficients and enables > the plane input csc unit in hardware. > > Note: This is currently untested and floated to get an early feedback > on the design and implementation for this feature. In parallel, > I will test this on actual ICL hardware and confirm with planar > formats. > > v2: Addressed Maarten's and Ville's review comments and added the > coefficients in a 2D array instead of independent Macros. > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > --- > drivers/gpu/drm/i915/intel_color.c | 49 ++++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_display.c | 24 +++++++++++++----- > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > 3 files changed, 69 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c > index 5127da2..de733cf 100644 > --- a/drivers/gpu/drm/i915/intel_color.c > +++ b/drivers/gpu/drm/i915/intel_color.c > @@ -57,6 +57,11 @@ > #define CSC_RGB_TO_YUV_RV_GV 0xbce89ad8 > #define CSC_RGB_TO_YUV_BV 0x1e080000 > > +/* Preoffset values for YUV to RGB Conversion */ > +#define PREOFF_YUV_TO_RGB_HI 0x800 > +#define PREOFF_YUV_TO_RGB_ME 0xF00 > +#define PREOFF_YUV_TO_RGB_LO 0x800 > + > /* > * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point > * format). This macro takes the coefficient we want transformed and the > @@ -643,6 +648,50 @@ int intel_color_check(struct drm_crtc *crtc, > return -EINVAL; > } > > +void icl_program_input_csc_coeff(const struct intel_crtc_state *crtc_state, > + const struct intel_plane_state *plane_state) > +{ > + struct drm_i915_private *dev_priv = > + to_i915(plane_state->base.plane->dev); > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); > + enum pipe pipe = crtc->pipe; > + struct intel_plane *intel_plane = > + to_intel_plane(plane_state->base.plane); > + enum plane_id plane = intel_plane->id; > + > + static const u32 input_csc_matrix[][6] = { > + /* BT.601 full range YCbCr -> full range RGB */ > + [DRM_COLOR_YCBCR_BT601] = { > + 0x7AF87800, 0x0, 0x8B287800, > + 0x9AC0, 0x7800, 0x7DD8, > + }, Maybe each component separately? uint16 [][9] Also BY/BU/BV are programmed in bits 31-16 according to bspec. :) Seems that bug was already there in the original submission? Otherwise looking much better! And once more, I still miss any form of limited/full range handling? > + /* BT.709 full range YCbCr -> full range RGB */ > + [DRM_COLOR_YCBCR_BT709] = { > + 0x7C987800, 0x0, 0x9EF87800, > + 0xABF8, 0x7800, 0x7ED8, > + }, > + }; > + const u32 *csc = input_csc_matrix[plane_state->base.color_encoding]; > + > + I915_WRITE(PLANE_INPUT_CSC_RY_GY(pipe, plane), csc[0]); > + I915_WRITE(PLANE_INPUT_CSC_BY(pipe, plane), csc[1]); > + I915_WRITE(PLANE_INPUT_CSC_RU_GU(pipe, plane), csc[2]); > + I915_WRITE(PLANE_INPUT_CSC_BU(pipe, plane), csc[3]); > + I915_WRITE(PLANE_INPUT_CSC_RV_GV(pipe, plane), csc[4]); > + I915_WRITE(PLANE_INPUT_CSC_BV(pipe, plane), csc[5]); > + > + I915_WRITE(PLANE_INPUT_CSC_PREOFF_HI(pipe, plane), > + PREOFF_YUV_TO_RGB_HI); > + I915_WRITE(PLANE_INPUT_CSC_PREOFF_ME(pipe, plane), > + PREOFF_YUV_TO_RGB_ME); > + I915_WRITE(PLANE_INPUT_CSC_PREOFF_LO(pipe, plane), > + PREOFF_YUV_TO_RGB_LO); > + > + I915_WRITE(PLANE_INPUT_CSC_POSTOFF_HI(pipe, plane), 0x0); > + I915_WRITE(PLANE_INPUT_CSC_POSTOFF_ME(pipe, plane), 0x0); > + I915_WRITE(PLANE_INPUT_CSC_POSTOFF_LO(pipe, plane), 0x0); > +} > + > void intel_color_init(struct drm_crtc *crtc) > { > struct drm_i915_private *dev_priv = to_i915(crtc->dev); > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index fe045ab..be65419 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3666,6 +3666,8 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state, > struct drm_i915_private *dev_priv = > 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; > > if (INTEL_GEN(dev_priv) < 11) { > @@ -3676,13 +3678,23 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state, > plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state); > > if (fb->format->is_yuv) { > + if (!icl_is_hdr_plane(plane)) { > + 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; > > + if (plane_state->base.color_range == > + DRM_COLOR_YCBCR_FULL_RANGE) > + plane_color_ctl |= > + PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE; > + } else { > + icl_program_input_csc_coeff(crtc_state, plane_state); > + plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE; > + } > } > > return plane_color_ctl; > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index db24308..bd9e946 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -2285,6 +2285,8 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_ > int intel_color_check(struct drm_crtc *crtc, struct drm_crtc_state *state); > void intel_color_set_csc(struct drm_crtc_state *crtc_state); > void intel_color_load_luts(struct drm_crtc_state *crtc_state); > +void icl_program_input_csc_coeff(const struct intel_crtc_state *crtc_state, > + const struct intel_plane_state *plane_state); > > /* intel_lspcon.c */ > bool lspcon_init(struct intel_digital_port *intel_dig_port);
>-----Original Message----- >From: Roper, Matthew D >Sent: Thursday, October 25, 2018 4:52 AM >To: Shankar, Uma <uma.shankar@intel.com> >Cc: intel-gfx@lists.freedesktop.org; Syrjala, Ville <ville.syrjala@intel.com>; >Lankhorst, Maarten <maarten.lankhorst@intel.com> >Subject: Re: [Intel-gfx] [v2 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to >RGB Conversion > >On Wed, Oct 24, 2018 at 09:48:13PM +0530, Uma Shankar wrote: >> Plane input CSC needs to be enabled to convert frambuffers from YUV to >> RGB. This is needed for bottom 3 planes on ICL, rest of the planes >> have hardcoded conversion and taken care by the legacy code. > >Just to confirm, we're specifically talking YUV444(packed) -> RGB here, right? So >for formats like NV12 (YUV 420), the chroma upsampler first merges/scales the Y >and UV planes to produce YUV444(packed) data which then has this input CSC >matrix applied to convert it to RGB? And then after that point there's a separate >set of userspace-programmable degamma/CSC/gamma set of steps, which is >what your other series "[RFC v5 0/8] Add Plane Color Properties" adds, right? And >then after that comes the pipe-level degamma/CSC/gamma... This is correct, all the planar formats first should be upsampled to YUV444 and then the general YUV->RGB conversion will happen using the Plane Input CSC hardware block. Once this conversion is done, later the other plane CSC hardware (after de-gamma) can be used to convert the colorspace if required (SRGB to BT2020 etc). >Is there somewhere in the bspec that talks about this input CSC in more detail? I >don't see any mention on the general YUV programming or plane capability >pages, and the register details themselves are pretty sparse, so it's hard to fully >review these for correctness. Based on your code here, it appears the >INPUT_CSC follows the same general layout and expected coefficient values as >the pipe CSC, but I'm not sure where in the spec to confirm that. Unfortunately not much details are available in bspec, this is based on my understanding of the hardware pipeline taking reference from few DCN document for HDR. But, in general what you said is correct and this is how input csc hardware block is supposed to work. Though it's a multipurpose programmable unit, but YUV444->RGB conversion is its primary usecase in a real world scenario. Regards, Uma Shankar > >Matt > >> >> This patch defines the co-efficient values for YUV to RGB conversion >> in BT709 and BT601 formats. It programs the coefficients and enables >> the plane input csc unit in hardware. >> >> Note: This is currently untested and floated to get an early feedback >> on the design and implementation for this feature. In parallel, I will >> test this on actual ICL hardware and confirm with planar formats. >> >> v2: Addressed Maarten's and Ville's review comments and added the >> coefficients in a 2D array instead of independent Macros. >> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com> >> --- >> drivers/gpu/drm/i915/intel_color.c | 49 >++++++++++++++++++++++++++++++++++++ >> drivers/gpu/drm/i915/intel_display.c | 24 +++++++++++++----- >> drivers/gpu/drm/i915/intel_drv.h | 2 ++ >> 3 files changed, 69 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_color.c >> b/drivers/gpu/drm/i915/intel_color.c >> index 5127da2..de733cf 100644 >> --- a/drivers/gpu/drm/i915/intel_color.c >> +++ b/drivers/gpu/drm/i915/intel_color.c >> @@ -57,6 +57,11 @@ >> #define CSC_RGB_TO_YUV_RV_GV 0xbce89ad8 #define >CSC_RGB_TO_YUV_BV >> 0x1e080000 >> >> +/* Preoffset values for YUV to RGB Conversion */ >> +#define PREOFF_YUV_TO_RGB_HI 0x800 >> +#define PREOFF_YUV_TO_RGB_ME 0xF00 >> +#define PREOFF_YUV_TO_RGB_LO 0x800 >> + >> /* >> * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point >> * format). This macro takes the coefficient we want transformed and >> the @@ -643,6 +648,50 @@ int intel_color_check(struct drm_crtc *crtc, >> return -EINVAL; >> } >> >> +void icl_program_input_csc_coeff(const struct intel_crtc_state *crtc_state, >> + const struct intel_plane_state *plane_state) { >> + struct drm_i915_private *dev_priv = >> + to_i915(plane_state->base.plane->dev); >> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); >> + enum pipe pipe = crtc->pipe; >> + struct intel_plane *intel_plane = >> + to_intel_plane(plane_state->base.plane); >> + enum plane_id plane = intel_plane->id; >> + >> + static const u32 input_csc_matrix[][6] = { >> + /* BT.601 full range YCbCr -> full range RGB */ >> + [DRM_COLOR_YCBCR_BT601] = { >> + 0x7AF87800, 0x0, 0x8B287800, >> + 0x9AC0, 0x7800, 0x7DD8, >> + }, >> + /* BT.709 full range YCbCr -> full range RGB */ >> + [DRM_COLOR_YCBCR_BT709] = { >> + 0x7C987800, 0x0, 0x9EF87800, >> + 0xABF8, 0x7800, 0x7ED8, >> + }, >> + }; >> + const u32 *csc = input_csc_matrix[plane_state->base.color_encoding]; >> + >> + I915_WRITE(PLANE_INPUT_CSC_RY_GY(pipe, plane), csc[0]); >> + I915_WRITE(PLANE_INPUT_CSC_BY(pipe, plane), csc[1]); >> + I915_WRITE(PLANE_INPUT_CSC_RU_GU(pipe, plane), csc[2]); >> + I915_WRITE(PLANE_INPUT_CSC_BU(pipe, plane), csc[3]); >> + I915_WRITE(PLANE_INPUT_CSC_RV_GV(pipe, plane), csc[4]); >> + I915_WRITE(PLANE_INPUT_CSC_BV(pipe, plane), csc[5]); >> + >> + I915_WRITE(PLANE_INPUT_CSC_PREOFF_HI(pipe, plane), >> + PREOFF_YUV_TO_RGB_HI); >> + I915_WRITE(PLANE_INPUT_CSC_PREOFF_ME(pipe, plane), >> + PREOFF_YUV_TO_RGB_ME); >> + I915_WRITE(PLANE_INPUT_CSC_PREOFF_LO(pipe, plane), >> + PREOFF_YUV_TO_RGB_LO); >> + >> + I915_WRITE(PLANE_INPUT_CSC_POSTOFF_HI(pipe, plane), 0x0); >> + I915_WRITE(PLANE_INPUT_CSC_POSTOFF_ME(pipe, plane), 0x0); >> + I915_WRITE(PLANE_INPUT_CSC_POSTOFF_LO(pipe, plane), 0x0); } >> + >> void intel_color_init(struct drm_crtc *crtc) { >> struct drm_i915_private *dev_priv = to_i915(crtc->dev); diff --git >> a/drivers/gpu/drm/i915/intel_display.c >> b/drivers/gpu/drm/i915/intel_display.c >> index fe045ab..be65419 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -3666,6 +3666,8 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state >*crtc_state, >> struct drm_i915_private *dev_priv = >> 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; >> >> if (INTEL_GEN(dev_priv) < 11) { >> @@ -3676,13 +3678,23 @@ u32 glk_plane_color_ctl(const struct >intel_crtc_state *crtc_state, >> plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state); >> >> if (fb->format->is_yuv) { >> - 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; >> + if (!icl_is_hdr_plane(plane)) { >> + 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; >> >> - if (plane_state->base.color_range == >DRM_COLOR_YCBCR_FULL_RANGE) >> - plane_color_ctl |= >PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE; >> + if (plane_state->base.color_range == >> + DRM_COLOR_YCBCR_FULL_RANGE) >> + plane_color_ctl |= >> + > PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE; >> + } else { >> + icl_program_input_csc_coeff(crtc_state, plane_state); >> + plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE; >> + } >> } >> >> return plane_color_ctl; >> diff --git a/drivers/gpu/drm/i915/intel_drv.h >> b/drivers/gpu/drm/i915/intel_drv.h >> index db24308..bd9e946 100644 >> --- a/drivers/gpu/drm/i915/intel_drv.h >> +++ b/drivers/gpu/drm/i915/intel_drv.h >> @@ -2285,6 +2285,8 @@ int intel_plane_atomic_check_with_state(const >> struct intel_crtc_state *old_crtc_ int intel_color_check(struct >> drm_crtc *crtc, struct drm_crtc_state *state); void >> intel_color_set_csc(struct drm_crtc_state *crtc_state); void >> intel_color_load_luts(struct drm_crtc_state *crtc_state); >> +void icl_program_input_csc_coeff(const struct intel_crtc_state *crtc_state, >> + const struct intel_plane_state *plane_state); >> >> /* intel_lspcon.c */ >> bool lspcon_init(struct intel_digital_port *intel_dig_port); >> -- >> 1.9.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > >-- >Matt Roper >Graphics Software Engineer >IoTG Platform Enabling & Development >Intel Corporation >(916) 356-2795
>-----Original Message----- >From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com] >Sent: Thursday, October 25, 2018 3:17 PM >To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org >Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten ><maarten.lankhorst@intel.com> >Subject: Re: [Intel-gfx] [v2 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to >RGB Conversion > >Op 24-10-18 om 18:18 schreef Uma Shankar: >> Plane input CSC needs to be enabled to convert frambuffers from YUV to >> RGB. This is needed for bottom 3 planes on ICL, rest of the planes >> have hardcoded conversion and taken care by the legacy code. >> >> This patch defines the co-efficient values for YUV to RGB conversion >> in BT709 and BT601 formats. It programs the coefficients and enables >> the plane input csc unit in hardware. >> >> Note: This is currently untested and floated to get an early feedback >> on the design and implementation for this feature. In parallel, I will >> test this on actual ICL hardware and confirm with planar formats. >> >> v2: Addressed Maarten's and Ville's review comments and added the >> coefficients in a 2D array instead of independent Macros. >> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com> >> --- >> drivers/gpu/drm/i915/intel_color.c | 49 >++++++++++++++++++++++++++++++++++++ >> drivers/gpu/drm/i915/intel_display.c | 24 +++++++++++++----- >> drivers/gpu/drm/i915/intel_drv.h | 2 ++ >> 3 files changed, 69 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_color.c >> b/drivers/gpu/drm/i915/intel_color.c >> index 5127da2..de733cf 100644 >> --- a/drivers/gpu/drm/i915/intel_color.c >> +++ b/drivers/gpu/drm/i915/intel_color.c >> @@ -57,6 +57,11 @@ >> #define CSC_RGB_TO_YUV_RV_GV 0xbce89ad8 #define >CSC_RGB_TO_YUV_BV >> 0x1e080000 >> >> +/* Preoffset values for YUV to RGB Conversion */ >> +#define PREOFF_YUV_TO_RGB_HI 0x800 >> +#define PREOFF_YUV_TO_RGB_ME 0xF00 >> +#define PREOFF_YUV_TO_RGB_LO 0x800 >> + >> /* >> * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point >> * format). This macro takes the coefficient we want transformed and >> the @@ -643,6 +648,50 @@ int intel_color_check(struct drm_crtc *crtc, >> return -EINVAL; >> } >> >> +void icl_program_input_csc_coeff(const struct intel_crtc_state *crtc_state, >> + const struct intel_plane_state *plane_state) { >> + struct drm_i915_private *dev_priv = >> + to_i915(plane_state->base.plane->dev); >> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); >> + enum pipe pipe = crtc->pipe; >> + struct intel_plane *intel_plane = >> + to_intel_plane(plane_state->base.plane); >> + enum plane_id plane = intel_plane->id; >> + >> + static const u32 input_csc_matrix[][6] = { >> + /* BT.601 full range YCbCr -> full range RGB */ >> + [DRM_COLOR_YCBCR_BT601] = { >> + 0x7AF87800, 0x0, 0x8B287800, >> + 0x9AC0, 0x7800, 0x7DD8, >> + }, >Maybe each component separately? uint16 [][9] Sure, I will do that. >Also BY/BU/BV are programmed in bits 31-16 according to bspec. :) Seems that >bug was already there in the original submission? Yes, thanks for spotting this. Will correct it. > >Otherwise looking much better! > >And once more, I still miss any form of limited/full range handling? My plan is to add limited range and other colorspace in a later patch since we need to calculate the exact coefficients for those cases. If its ok, I can re-submit the next version just limiting to full range as of now. Regards, Uma Shankar >> + /* BT.709 full range YCbCr -> full range RGB */ >> + [DRM_COLOR_YCBCR_BT709] = { >> + 0x7C987800, 0x0, 0x9EF87800, >> + 0xABF8, 0x7800, 0x7ED8, >> + }, >> + }; >> + const u32 *csc = input_csc_matrix[plane_state->base.color_encoding]; >> + >> + I915_WRITE(PLANE_INPUT_CSC_RY_GY(pipe, plane), csc[0]); >> + I915_WRITE(PLANE_INPUT_CSC_BY(pipe, plane), csc[1]); >> + I915_WRITE(PLANE_INPUT_CSC_RU_GU(pipe, plane), csc[2]); >> + I915_WRITE(PLANE_INPUT_CSC_BU(pipe, plane), csc[3]); >> + I915_WRITE(PLANE_INPUT_CSC_RV_GV(pipe, plane), csc[4]); >> + I915_WRITE(PLANE_INPUT_CSC_BV(pipe, plane), csc[5]); >> + >> + I915_WRITE(PLANE_INPUT_CSC_PREOFF_HI(pipe, plane), >> + PREOFF_YUV_TO_RGB_HI); >> + I915_WRITE(PLANE_INPUT_CSC_PREOFF_ME(pipe, plane), >> + PREOFF_YUV_TO_RGB_ME); >> + I915_WRITE(PLANE_INPUT_CSC_PREOFF_LO(pipe, plane), >> + PREOFF_YUV_TO_RGB_LO); >> + >> + I915_WRITE(PLANE_INPUT_CSC_POSTOFF_HI(pipe, plane), 0x0); >> + I915_WRITE(PLANE_INPUT_CSC_POSTOFF_ME(pipe, plane), 0x0); >> + I915_WRITE(PLANE_INPUT_CSC_POSTOFF_LO(pipe, plane), 0x0); } >> + >> void intel_color_init(struct drm_crtc *crtc) { >> struct drm_i915_private *dev_priv = to_i915(crtc->dev); diff --git >> a/drivers/gpu/drm/i915/intel_display.c >> b/drivers/gpu/drm/i915/intel_display.c >> index fe045ab..be65419 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -3666,6 +3666,8 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state >*crtc_state, >> struct drm_i915_private *dev_priv = >> 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; >> >> if (INTEL_GEN(dev_priv) < 11) { >> @@ -3676,13 +3678,23 @@ u32 glk_plane_color_ctl(const struct >intel_crtc_state *crtc_state, >> plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state); >> >> if (fb->format->is_yuv) { >> + if (!icl_is_hdr_plane(plane)) { >> + 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; >> >> + if (plane_state->base.color_range == >> + DRM_COLOR_YCBCR_FULL_RANGE) >> + plane_color_ctl |= >> + > PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE; >> + } else { >> + icl_program_input_csc_coeff(crtc_state, plane_state); >> + plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE; >> + } >> } >> >> return plane_color_ctl; >> diff --git a/drivers/gpu/drm/i915/intel_drv.h >> b/drivers/gpu/drm/i915/intel_drv.h >> index db24308..bd9e946 100644 >> --- a/drivers/gpu/drm/i915/intel_drv.h >> +++ b/drivers/gpu/drm/i915/intel_drv.h >> @@ -2285,6 +2285,8 @@ int intel_plane_atomic_check_with_state(const >> struct intel_crtc_state *old_crtc_ int intel_color_check(struct >> drm_crtc *crtc, struct drm_crtc_state *state); void >> intel_color_set_csc(struct drm_crtc_state *crtc_state); void >> intel_color_load_luts(struct drm_crtc_state *crtc_state); >> +void icl_program_input_csc_coeff(const struct intel_crtc_state *crtc_state, >> + const struct intel_plane_state *plane_state); >> >> /* intel_lspcon.c */ >> bool lspcon_init(struct intel_digital_port *intel_dig_port); >
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c index 5127da2..de733cf 100644 --- a/drivers/gpu/drm/i915/intel_color.c +++ b/drivers/gpu/drm/i915/intel_color.c @@ -57,6 +57,11 @@ #define CSC_RGB_TO_YUV_RV_GV 0xbce89ad8 #define CSC_RGB_TO_YUV_BV 0x1e080000 +/* Preoffset values for YUV to RGB Conversion */ +#define PREOFF_YUV_TO_RGB_HI 0x800 +#define PREOFF_YUV_TO_RGB_ME 0xF00 +#define PREOFF_YUV_TO_RGB_LO 0x800 + /* * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point * format). This macro takes the coefficient we want transformed and the @@ -643,6 +648,50 @@ int intel_color_check(struct drm_crtc *crtc, return -EINVAL; } +void icl_program_input_csc_coeff(const struct intel_crtc_state *crtc_state, + const struct intel_plane_state *plane_state) +{ + struct drm_i915_private *dev_priv = + to_i915(plane_state->base.plane->dev); + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); + enum pipe pipe = crtc->pipe; + struct intel_plane *intel_plane = + to_intel_plane(plane_state->base.plane); + enum plane_id plane = intel_plane->id; + + static const u32 input_csc_matrix[][6] = { + /* BT.601 full range YCbCr -> full range RGB */ + [DRM_COLOR_YCBCR_BT601] = { + 0x7AF87800, 0x0, 0x8B287800, + 0x9AC0, 0x7800, 0x7DD8, + }, + /* BT.709 full range YCbCr -> full range RGB */ + [DRM_COLOR_YCBCR_BT709] = { + 0x7C987800, 0x0, 0x9EF87800, + 0xABF8, 0x7800, 0x7ED8, + }, + }; + const u32 *csc = input_csc_matrix[plane_state->base.color_encoding]; + + I915_WRITE(PLANE_INPUT_CSC_RY_GY(pipe, plane), csc[0]); + I915_WRITE(PLANE_INPUT_CSC_BY(pipe, plane), csc[1]); + I915_WRITE(PLANE_INPUT_CSC_RU_GU(pipe, plane), csc[2]); + I915_WRITE(PLANE_INPUT_CSC_BU(pipe, plane), csc[3]); + I915_WRITE(PLANE_INPUT_CSC_RV_GV(pipe, plane), csc[4]); + I915_WRITE(PLANE_INPUT_CSC_BV(pipe, plane), csc[5]); + + I915_WRITE(PLANE_INPUT_CSC_PREOFF_HI(pipe, plane), + PREOFF_YUV_TO_RGB_HI); + I915_WRITE(PLANE_INPUT_CSC_PREOFF_ME(pipe, plane), + PREOFF_YUV_TO_RGB_ME); + I915_WRITE(PLANE_INPUT_CSC_PREOFF_LO(pipe, plane), + PREOFF_YUV_TO_RGB_LO); + + I915_WRITE(PLANE_INPUT_CSC_POSTOFF_HI(pipe, plane), 0x0); + I915_WRITE(PLANE_INPUT_CSC_POSTOFF_ME(pipe, plane), 0x0); + I915_WRITE(PLANE_INPUT_CSC_POSTOFF_LO(pipe, plane), 0x0); +} + void intel_color_init(struct drm_crtc *crtc) { struct drm_i915_private *dev_priv = to_i915(crtc->dev); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index fe045ab..be65419 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3666,6 +3666,8 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state, struct drm_i915_private *dev_priv = 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; if (INTEL_GEN(dev_priv) < 11) { @@ -3676,13 +3678,23 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state, plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state); if (fb->format->is_yuv) { - 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; + if (!icl_is_hdr_plane(plane)) { + 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; - if (plane_state->base.color_range == DRM_COLOR_YCBCR_FULL_RANGE) - plane_color_ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE; + if (plane_state->base.color_range == + DRM_COLOR_YCBCR_FULL_RANGE) + plane_color_ctl |= + PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE; + } else { + icl_program_input_csc_coeff(crtc_state, plane_state); + plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE; + } } return plane_color_ctl; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index db24308..bd9e946 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -2285,6 +2285,8 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_ int intel_color_check(struct drm_crtc *crtc, struct drm_crtc_state *state); void intel_color_set_csc(struct drm_crtc_state *crtc_state); void intel_color_load_luts(struct drm_crtc_state *crtc_state); +void icl_program_input_csc_coeff(const struct intel_crtc_state *crtc_state, + const struct intel_plane_state *plane_state); /* intel_lspcon.c */ bool lspcon_init(struct intel_digital_port *intel_dig_port);
Plane input CSC needs to be enabled to convert frambuffers from YUV to RGB. This is needed for bottom 3 planes on ICL, rest of the planes have hardcoded conversion and taken care by the legacy code. This patch defines the co-efficient values for YUV to RGB conversion in BT709 and BT601 formats. It programs the coefficients and enables the plane input csc unit in hardware. Note: This is currently untested and floated to get an early feedback on the design and implementation for this feature. In parallel, I will test this on actual ICL hardware and confirm with planar formats. v2: Addressed Maarten's and Ville's review comments and added the coefficients in a 2D array instead of independent Macros. Signed-off-by: Uma Shankar <uma.shankar@intel.com> --- drivers/gpu/drm/i915/intel_color.c | 49 ++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_display.c | 24 +++++++++++++----- drivers/gpu/drm/i915/intel_drv.h | 2 ++ 3 files changed, 69 insertions(+), 6 deletions(-)