| Message ID | 1540992931-26022-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 31, 2018 at 07:05:31PM +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. > > 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. > > This has been verified and tested by Maarten and the change is working > as expected. > > v2: Addressed Maarten's and Ville's review comments and added the > coefficients in a 2D array instead of independent Macros. > > v3: Added individual coefficient matrix (9 values) instead of 6 > register values as per Maarten's comment. Also addresed a shift > issue with B channel coefficient. > > v4: Added support for Limited Range Color Handling > > v5: Fixed Matt and Maarten's review comments. > > v6: Added human readable matrix values for YUV to RGB Conversion along > with just the bspec register values, as per Matt's suggestion. > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > --- > drivers/gpu/drm/i915/intel_color.c | 103 +++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_display.c | 23 ++++++-- > drivers/gpu/drm/i915/intel_drv.h | 2 + > 3 files changed, 122 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c > index 5127da2..6dc9075 100644 > --- a/drivers/gpu/drm/i915/intel_color.c > +++ b/drivers/gpu/drm/i915/intel_color.c > @@ -57,6 +57,15 @@ > #define CSC_RGB_TO_YUV_RV_GV 0xbce89ad8 > #define CSC_RGB_TO_YUV_BV 0x1e080000 > > +#define ROFF(x) (((x) & 0xffff) << 16) > +#define GOFF(x) (((x) & 0xffff) << 0) > +#define BOFF(x) (((x) & 0xffff) << 16) > + > +/* Preoffset values for YUV to RGB Conversion */ > +#define PREOFF_YUV_TO_RGB_HI 0x1800 > +#define PREOFF_YUV_TO_RGB_ME 0x1F00 > +#define PREOFF_YUV_TO_RGB_LO 0x1800 > + > /* > * 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 +652,100 @@ 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 u16 input_csc_matrix[][9] = { > + /* > + * BT.601 full range YCbCr -> full range RGB > + * The matrix required is : > + * [1.000, 0.000, 1.371, > + * 1.000, -0.336, -0.698, > + * 1.000, 1.732, 0.0000] > + */ > + [DRM_COLOR_YCBCR_BT601] = { > + 0x7AF8, 0x7800, 0x0, > + 0x8B28, 0x7800, 0x9AC0, > + 0x0, 0x7800, 0x7DD8, > + }, > + /* > + * BT.709 full range YCbCr -> full range RGB > + * The matrix required is : > + * [1.000, 0.000, 1.574, > + * 1.000, -0.187, -0.468, > + * 1.000, 1.855, 0.0000] > + */ > + [DRM_COLOR_YCBCR_BT709] = { > + 0x7C98, 0x7800, 0x0, > + 0x9EF8, 0x7800, 0xABF8, > + 0x0, 0x7800, 0x7ED8, > + }, > + }; > + > + /* Matrix for Limited Range to Full Range Conversion */ > + static const u16 input_csc_matrix_lr[][9] = { > + /* > + * BT.601 Limted range YCbCr -> full range RGB > + * The matrix required is : > + * [1.164384, 0.000, 1.596370, > + * 1.138393, -0.382500, -0.794598, > + * 1.138393, 1.971696, 0.0000] > + */ > + [DRM_COLOR_YCBCR_BT601] = { > + 0x7CC8, 0x7950, 0x0, > + 0x8CB8, 0x7918, 0x9C40, > + 0x0, 0x7918, 0x7FC8, > + }, > + /* > + * BT.709 Limited range YCbCr -> full range RGB > + * The matrix required is : > + * [1.164, 0.000, 1.833671, > + * 1.138393, -0.213249, -0.532909, > + * 1.138393, 2.112402, 0.0000] > + */ > + [DRM_COLOR_YCBCR_BT709] = { > + 0x7EA8, 0x7950, 0x0, > + 0x8888, 0x7918, 0xADA8, > + 0x0, 0x7918, 0x6870, > + }, > + }; > + const u16 *csc; > + > + if (plane_state->base.color_range == DRM_COLOR_YCBCR_FULL_RANGE) > + csc = input_csc_matrix[plane_state->base.color_encoding]; > + else > + csc = input_csc_matrix_lr[plane_state->base.color_encoding]; > + > + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 0), ROFF(csc[0]) | > + GOFF(csc[1])); > + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 1), BOFF(csc[2])); > + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 2), ROFF(csc[3]) | > + GOFF(csc[4])); > + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 3), BOFF(csc[5])); > + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 4), ROFF(csc[6]) | > + GOFF(csc[7])); > + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 5), BOFF(csc[8])); > + > + I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 0), > + PREOFF_YUV_TO_RGB_HI); > + I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 1), > + PREOFF_YUV_TO_RGB_ME); > + I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 2), > + PREOFF_YUV_TO_RGB_LO); > + > + I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 0), 0x0); > + I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 1), 0x0); > + I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 2), 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..d16a064 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3666,6 +3666,7 @@ 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 +3677,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); Wrong place. This needs to be .in update_plane(). > + plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE; > + } I'd probably write this whole thing as something like: if (is_hdr_plane() && is_yuv) { ctl |= INPUT_CSC_ENABLE; } else if (is_yuv) { ... } as that avoids the deep nesting you now have. > } > > 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
On Wed, Oct 31, 2018 at 07:05:31PM +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. > > 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. > > This has been verified and tested by Maarten and the change is working > as expected. > > v2: Addressed Maarten's and Ville's review comments and added the > coefficients in a 2D array instead of independent Macros. > > v3: Added individual coefficient matrix (9 values) instead of 6 > register values as per Maarten's comment. Also addresed a shift > issue with B channel coefficient. > > v4: Added support for Limited Range Color Handling > > v5: Fixed Matt and Maarten's review comments. > > v6: Added human readable matrix values for YUV to RGB Conversion along > with just the bspec register values, as per Matt's suggestion. > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > --- > drivers/gpu/drm/i915/intel_color.c | 103 +++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_display.c | 23 ++++++-- > drivers/gpu/drm/i915/intel_drv.h | 2 + > 3 files changed, 122 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c > index 5127da2..6dc9075 100644 > --- a/drivers/gpu/drm/i915/intel_color.c > +++ b/drivers/gpu/drm/i915/intel_color.c > @@ -57,6 +57,15 @@ > #define CSC_RGB_TO_YUV_RV_GV 0xbce89ad8 > #define CSC_RGB_TO_YUV_BV 0x1e080000 > > +#define ROFF(x) (((x) & 0xffff) << 16) > +#define GOFF(x) (((x) & 0xffff) << 0) > +#define BOFF(x) (((x) & 0xffff) << 16) > + > +/* Preoffset values for YUV to RGB Conversion */ > +#define PREOFF_YUV_TO_RGB_HI 0x1800 > +#define PREOFF_YUV_TO_RGB_ME 0x1F00 > +#define PREOFF_YUV_TO_RGB_LO 0x1800 > + > /* > * 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 +652,100 @@ 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 u16 input_csc_matrix[][9] = { > + /* > + * BT.601 full range YCbCr -> full range RGB > + * The matrix required is : > + * [1.000, 0.000, 1.371, > + * 1.000, -0.336, -0.698, > + * 1.000, 1.732, 0.0000] > + */ > + [DRM_COLOR_YCBCR_BT601] = { > + 0x7AF8, 0x7800, 0x0, > + 0x8B28, 0x7800, 0x9AC0, > + 0x0, 0x7800, 0x7DD8, > + }, > + /* > + * BT.709 full range YCbCr -> full range RGB > + * The matrix required is : > + * [1.000, 0.000, 1.574, > + * 1.000, -0.187, -0.468, > + * 1.000, 1.855, 0.0000] > + */ > + [DRM_COLOR_YCBCR_BT709] = { > + 0x7C98, 0x7800, 0x0, > + 0x9EF8, 0x7800, 0xABF8, > + 0x0, 0x7800, 0x7ED8, > + }, > + }; > + > + /* Matrix for Limited Range to Full Range Conversion */ > + static const u16 input_csc_matrix_lr[][9] = { > + /* > + * BT.601 Limted range YCbCr -> full range RGB > + * The matrix required is : > + * [1.164384, 0.000, 1.596370, > + * 1.138393, -0.382500, -0.794598, > + * 1.138393, 1.971696, 0.0000] > + */ > + [DRM_COLOR_YCBCR_BT601] = { > + 0x7CC8, 0x7950, 0x0, > + 0x8CB8, 0x7918, 0x9C40, > + 0x0, 0x7918, 0x7FC8, > + }, > + /* > + * BT.709 Limited range YCbCr -> full range RGB > + * The matrix required is : > + * [1.164, 0.000, 1.833671, > + * 1.138393, -0.213249, -0.532909, > + * 1.138393, 2.112402, 0.0000] > + */ > + [DRM_COLOR_YCBCR_BT709] = { > + 0x7EA8, 0x7950, 0x0, > + 0x8888, 0x7918, 0xADA8, > + 0x0, 0x7918, 0x6870, > + }, > + }; > + const u16 *csc; > + > + if (plane_state->base.color_range == DRM_COLOR_YCBCR_FULL_RANGE) > + csc = input_csc_matrix[plane_state->base.color_encoding]; > + else > + csc = input_csc_matrix_lr[plane_state->base.color_encoding]; > + > + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 0), ROFF(csc[0]) | > + GOFF(csc[1])); > + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 1), BOFF(csc[2])); > + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 2), ROFF(csc[3]) | > + GOFF(csc[4])); > + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 3), BOFF(csc[5])); > + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 4), ROFF(csc[6]) | > + GOFF(csc[7])); > + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 5), BOFF(csc[8])); > + > + I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 0), > + PREOFF_YUV_TO_RGB_HI); > + I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 1), > + PREOFF_YUV_TO_RGB_ME); > + I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 2), > + PREOFF_YUV_TO_RGB_LO); > + > + I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 0), 0x0); > + I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 1), 0x0); > + I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 2), 0x0); > +} Oh and please move this function into intel_sprite.c. That way it can be static. > + > 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..d16a064 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3666,6 +3666,7 @@ 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 +3677,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
On Wed, Oct 31, 2018 at 07:05:31PM +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. > > 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. > > This has been verified and tested by Maarten and the change is working > as expected. > > v2: Addressed Maarten's and Ville's review comments and added the > coefficients in a 2D array instead of independent Macros. > > v3: Added individual coefficient matrix (9 values) instead of 6 > register values as per Maarten's comment. Also addresed a shift > issue with B channel coefficient. > > v4: Added support for Limited Range Color Handling > > v5: Fixed Matt and Maarten's review comments. > > v6: Added human readable matrix values for YUV to RGB Conversion along > with just the bspec register values, as per Matt's suggestion. > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > --- > drivers/gpu/drm/i915/intel_color.c | 103 +++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_display.c | 23 ++++++-- > drivers/gpu/drm/i915/intel_drv.h | 2 + > 3 files changed, 122 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c > index 5127da2..6dc9075 100644 > --- a/drivers/gpu/drm/i915/intel_color.c > +++ b/drivers/gpu/drm/i915/intel_color.c > @@ -57,6 +57,15 @@ > #define CSC_RGB_TO_YUV_RV_GV 0xbce89ad8 > #define CSC_RGB_TO_YUV_BV 0x1e080000 > > +#define ROFF(x) (((x) & 0xffff) << 16) > +#define GOFF(x) (((x) & 0xffff) << 0) > +#define BOFF(x) (((x) & 0xffff) << 16) > + > +/* Preoffset values for YUV to RGB Conversion */ > +#define PREOFF_YUV_TO_RGB_HI 0x1800 > +#define PREOFF_YUV_TO_RGB_ME 0x1F00 > +#define PREOFF_YUV_TO_RGB_LO 0x1800 > + > /* > * 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 +652,100 @@ 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 u16 input_csc_matrix[][9] = { > + /* > + * BT.601 full range YCbCr -> full range RGB > + * The matrix required is : > + * [1.000, 0.000, 1.371, > + * 1.000, -0.336, -0.698, > + * 1.000, 1.732, 0.0000] > + */ > + [DRM_COLOR_YCBCR_BT601] = { > + 0x7AF8, 0x7800, 0x0, > + 0x8B28, 0x7800, 0x9AC0, > + 0x0, 0x7800, 0x7DD8, > + }, > + /* > + * BT.709 full range YCbCr -> full range RGB > + * The matrix required is : > + * [1.000, 0.000, 1.574, > + * 1.000, -0.187, -0.468, > + * 1.000, 1.855, 0.0000] > + */ > + [DRM_COLOR_YCBCR_BT709] = { > + 0x7C98, 0x7800, 0x0, > + 0x9EF8, 0x7800, 0xABF8, > + 0x0, 0x7800, 0x7ED8, > + }, > + }; > + > + /* Matrix for Limited Range to Full Range Conversion */ > + static const u16 input_csc_matrix_lr[][9] = { > + /* > + * BT.601 Limted range YCbCr -> full range RGB > + * The matrix required is : > + * [1.164384, 0.000, 1.596370, > + * 1.138393, -0.382500, -0.794598, > + * 1.138393, 1.971696, 0.0000] > + */ > + [DRM_COLOR_YCBCR_BT601] = { > + 0x7CC8, 0x7950, 0x0, > + 0x8CB8, 0x7918, 0x9C40, > + 0x0, 0x7918, 0x7FC8, > + }, > + /* > + * BT.709 Limited range YCbCr -> full range RGB > + * The matrix required is : > + * [1.164, 0.000, 1.833671, > + * 1.138393, -0.213249, -0.532909, > + * 1.138393, 2.112402, 0.0000] > + */ Did you scale the BT.709 values slightly differently than the BT.601 values above? The non-0/1 values don't seem to be exactly full*255/224 or full*255/219, but they're still close enough though that the hex representation comes out the same for all of the values except for the 0xADA8 (which I get as 0xADA0). So double-check that, but otherwise the patch is, Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > + [DRM_COLOR_YCBCR_BT709] = { > + 0x7EA8, 0x7950, 0x0, > + 0x8888, 0x7918, 0xADA8, > + 0x0, 0x7918, 0x6870, > + }, > + }; > + const u16 *csc; > + > + if (plane_state->base.color_range == DRM_COLOR_YCBCR_FULL_RANGE) > + csc = input_csc_matrix[plane_state->base.color_encoding]; > + else > + csc = input_csc_matrix_lr[plane_state->base.color_encoding]; > + > + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 0), ROFF(csc[0]) | > + GOFF(csc[1])); > + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 1), BOFF(csc[2])); > + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 2), ROFF(csc[3]) | > + GOFF(csc[4])); > + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 3), BOFF(csc[5])); > + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 4), ROFF(csc[6]) | > + GOFF(csc[7])); > + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 5), BOFF(csc[8])); > + > + I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 0), > + PREOFF_YUV_TO_RGB_HI); > + I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 1), > + PREOFF_YUV_TO_RGB_ME); > + I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 2), > + PREOFF_YUV_TO_RGB_LO); > + > + I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 0), 0x0); > + I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 1), 0x0); > + I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 2), 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..d16a064 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3666,6 +3666,7 @@ 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 +3677,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 >
>-----Original Message----- >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] >Sent: Wednesday, October 31, 2018 7:32 PM >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] [v6 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to >RGB Conversion > >On Wed, Oct 31, 2018 at 07:05:31PM +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. >> >> 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. >> >> This has been verified and tested by Maarten and the change is working >> as expected. >> >> v2: Addressed Maarten's and Ville's review comments and added the >> coefficients in a 2D array instead of independent Macros. >> >> v3: Added individual coefficient matrix (9 values) instead of 6 >> register values as per Maarten's comment. Also addresed a shift issue >> with B channel coefficient. >> >> v4: Added support for Limited Range Color Handling >> >> v5: Fixed Matt and Maarten's review comments. >> >> v6: Added human readable matrix values for YUV to RGB Conversion along >> with just the bspec register values, as per Matt's suggestion. >> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com> >> --- >> drivers/gpu/drm/i915/intel_color.c | 103 >+++++++++++++++++++++++++++++++++++ >> drivers/gpu/drm/i915/intel_display.c | 23 ++++++-- >> drivers/gpu/drm/i915/intel_drv.h | 2 + >> 3 files changed, 122 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_color.c >> b/drivers/gpu/drm/i915/intel_color.c >> index 5127da2..6dc9075 100644 >> --- a/drivers/gpu/drm/i915/intel_color.c >> +++ b/drivers/gpu/drm/i915/intel_color.c >> @@ -57,6 +57,15 @@ >> #define CSC_RGB_TO_YUV_RV_GV 0xbce89ad8 #define >CSC_RGB_TO_YUV_BV >> 0x1e080000 >> >> +#define ROFF(x) (((x) & 0xffff) << 16) >> +#define GOFF(x) (((x) & 0xffff) << 0) >> +#define BOFF(x) (((x) & 0xffff) << 16) >> + >> +/* Preoffset values for YUV to RGB Conversion */ >> +#define PREOFF_YUV_TO_RGB_HI 0x1800 >> +#define PREOFF_YUV_TO_RGB_ME 0x1F00 >> +#define PREOFF_YUV_TO_RGB_LO 0x1800 >> + >> /* >> * 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 +652,100 @@ 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 u16 input_csc_matrix[][9] = { >> + /* >> + * BT.601 full range YCbCr -> full range RGB >> + * The matrix required is : >> + * [1.000, 0.000, 1.371, >> + * 1.000, -0.336, -0.698, >> + * 1.000, 1.732, 0.0000] >> + */ >> + [DRM_COLOR_YCBCR_BT601] = { >> + 0x7AF8, 0x7800, 0x0, >> + 0x8B28, 0x7800, 0x9AC0, >> + 0x0, 0x7800, 0x7DD8, >> + }, >> + /* >> + * BT.709 full range YCbCr -> full range RGB >> + * The matrix required is : >> + * [1.000, 0.000, 1.574, >> + * 1.000, -0.187, -0.468, >> + * 1.000, 1.855, 0.0000] >> + */ >> + [DRM_COLOR_YCBCR_BT709] = { >> + 0x7C98, 0x7800, 0x0, >> + 0x9EF8, 0x7800, 0xABF8, >> + 0x0, 0x7800, 0x7ED8, >> + }, >> + }; >> + >> + /* Matrix for Limited Range to Full Range Conversion */ >> + static const u16 input_csc_matrix_lr[][9] = { >> + /* >> + * BT.601 Limted range YCbCr -> full range RGB >> + * The matrix required is : >> + * [1.164384, 0.000, 1.596370, >> + * 1.138393, -0.382500, -0.794598, >> + * 1.138393, 1.971696, 0.0000] >> + */ >> + [DRM_COLOR_YCBCR_BT601] = { >> + 0x7CC8, 0x7950, 0x0, >> + 0x8CB8, 0x7918, 0x9C40, >> + 0x0, 0x7918, 0x7FC8, >> + }, >> + /* >> + * BT.709 Limited range YCbCr -> full range RGB >> + * The matrix required is : >> + * [1.164, 0.000, 1.833671, >> + * 1.138393, -0.213249, -0.532909, >> + * 1.138393, 2.112402, 0.0000] >> + */ >> + [DRM_COLOR_YCBCR_BT709] = { >> + 0x7EA8, 0x7950, 0x0, >> + 0x8888, 0x7918, 0xADA8, >> + 0x0, 0x7918, 0x6870, >> + }, >> + }; >> + const u16 *csc; >> + >> + if (plane_state->base.color_range == >DRM_COLOR_YCBCR_FULL_RANGE) >> + csc = input_csc_matrix[plane_state->base.color_encoding]; >> + else >> + csc = input_csc_matrix_lr[plane_state->base.color_encoding]; >> + >> + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 0), ROFF(csc[0]) | >> + GOFF(csc[1])); >> + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 1), BOFF(csc[2])); >> + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 2), ROFF(csc[3]) | >> + GOFF(csc[4])); >> + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 3), BOFF(csc[5])); >> + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 4), ROFF(csc[6]) | >> + GOFF(csc[7])); >> + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 5), BOFF(csc[8])); >> + >> + I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 0), >> + PREOFF_YUV_TO_RGB_HI); >> + I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 1), >> + PREOFF_YUV_TO_RGB_ME); >> + I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 2), >> + PREOFF_YUV_TO_RGB_LO); >> + >> + I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 0), 0x0); >> + I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 1), 0x0); >> + I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 2), 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..d16a064 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -3666,6 +3666,7 @@ 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 +3677,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); > >Wrong place. This needs to be .in update_plane(). Ok, will update this. >> + plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE; >> + } > >I'd probably write this whole thing as something like: > >if (is_hdr_plane() && is_yuv) { > ctl |= INPUT_CSC_ENABLE; >} else if (is_yuv) { > ... >} > >as that avoids the deep nesting you now have. Yes, this is better. Will update the patch. Thanks.. > >> } >> >> 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 > >-- >Ville Syrjälä >Intel
>-----Original Message----- >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] >Sent: Wednesday, October 31, 2018 7:36 PM >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] [v6 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to >RGB Conversion > >On Wed, Oct 31, 2018 at 07:05:31PM +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. >> >> 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. >> >> This has been verified and tested by Maarten and the change is working >> as expected. >> >> v2: Addressed Maarten's and Ville's review comments and added the >> coefficients in a 2D array instead of independent Macros. >> >> v3: Added individual coefficient matrix (9 values) instead of 6 >> register values as per Maarten's comment. Also addresed a shift issue >> with B channel coefficient. >> >> v4: Added support for Limited Range Color Handling >> >> v5: Fixed Matt and Maarten's review comments. >> >> v6: Added human readable matrix values for YUV to RGB Conversion along >> with just the bspec register values, as per Matt's suggestion. >> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com> >> --- >> drivers/gpu/drm/i915/intel_color.c | 103 >+++++++++++++++++++++++++++++++++++ >> drivers/gpu/drm/i915/intel_display.c | 23 ++++++-- >> drivers/gpu/drm/i915/intel_drv.h | 2 + >> 3 files changed, 122 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_color.c >> b/drivers/gpu/drm/i915/intel_color.c >> index 5127da2..6dc9075 100644 >> --- a/drivers/gpu/drm/i915/intel_color.c >> +++ b/drivers/gpu/drm/i915/intel_color.c >> @@ -57,6 +57,15 @@ >> #define CSC_RGB_TO_YUV_RV_GV 0xbce89ad8 #define >CSC_RGB_TO_YUV_BV >> 0x1e080000 >> >> +#define ROFF(x) (((x) & 0xffff) << 16) >> +#define GOFF(x) (((x) & 0xffff) << 0) >> +#define BOFF(x) (((x) & 0xffff) << 16) >> + >> +/* Preoffset values for YUV to RGB Conversion */ >> +#define PREOFF_YUV_TO_RGB_HI 0x1800 >> +#define PREOFF_YUV_TO_RGB_ME 0x1F00 >> +#define PREOFF_YUV_TO_RGB_LO 0x1800 >> + >> /* >> * 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 +652,100 @@ 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 u16 input_csc_matrix[][9] = { >> + /* >> + * BT.601 full range YCbCr -> full range RGB >> + * The matrix required is : >> + * [1.000, 0.000, 1.371, >> + * 1.000, -0.336, -0.698, >> + * 1.000, 1.732, 0.0000] >> + */ >> + [DRM_COLOR_YCBCR_BT601] = { >> + 0x7AF8, 0x7800, 0x0, >> + 0x8B28, 0x7800, 0x9AC0, >> + 0x0, 0x7800, 0x7DD8, >> + }, >> + /* >> + * BT.709 full range YCbCr -> full range RGB >> + * The matrix required is : >> + * [1.000, 0.000, 1.574, >> + * 1.000, -0.187, -0.468, >> + * 1.000, 1.855, 0.0000] >> + */ >> + [DRM_COLOR_YCBCR_BT709] = { >> + 0x7C98, 0x7800, 0x0, >> + 0x9EF8, 0x7800, 0xABF8, >> + 0x0, 0x7800, 0x7ED8, >> + }, >> + }; >> + >> + /* Matrix for Limited Range to Full Range Conversion */ >> + static const u16 input_csc_matrix_lr[][9] = { >> + /* >> + * BT.601 Limted range YCbCr -> full range RGB >> + * The matrix required is : >> + * [1.164384, 0.000, 1.596370, >> + * 1.138393, -0.382500, -0.794598, >> + * 1.138393, 1.971696, 0.0000] >> + */ >> + [DRM_COLOR_YCBCR_BT601] = { >> + 0x7CC8, 0x7950, 0x0, >> + 0x8CB8, 0x7918, 0x9C40, >> + 0x0, 0x7918, 0x7FC8, >> + }, >> + /* >> + * BT.709 Limited range YCbCr -> full range RGB >> + * The matrix required is : >> + * [1.164, 0.000, 1.833671, >> + * 1.138393, -0.213249, -0.532909, >> + * 1.138393, 2.112402, 0.0000] >> + */ >> + [DRM_COLOR_YCBCR_BT709] = { >> + 0x7EA8, 0x7950, 0x0, >> + 0x8888, 0x7918, 0xADA8, >> + 0x0, 0x7918, 0x6870, >> + }, >> + }; >> + const u16 *csc; >> + >> + if (plane_state->base.color_range == >DRM_COLOR_YCBCR_FULL_RANGE) >> + csc = input_csc_matrix[plane_state->base.color_encoding]; >> + else >> + csc = input_csc_matrix_lr[plane_state->base.color_encoding]; >> + >> + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 0), ROFF(csc[0]) | >> + GOFF(csc[1])); >> + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 1), BOFF(csc[2])); >> + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 2), ROFF(csc[3]) | >> + GOFF(csc[4])); >> + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 3), BOFF(csc[5])); >> + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 4), ROFF(csc[6]) | >> + GOFF(csc[7])); >> + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 5), BOFF(csc[8])); >> + >> + I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 0), >> + PREOFF_YUV_TO_RGB_HI); >> + I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 1), >> + PREOFF_YUV_TO_RGB_ME); >> + I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 2), >> + PREOFF_YUV_TO_RGB_LO); >> + >> + I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 0), 0x0); >> + I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 1), 0x0); >> + I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 2), 0x0); } > >Oh and please move this function into intel_sprite.c. That way it can be static. We were trying to consolidate all the color stuff and the helper functions in intel_color.c. Since its currently called from one place, it can be made static. Will update the patch. Thanks. >> + >> 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..d16a064 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -3666,6 +3666,7 @@ 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 +3677,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 > >-- >Ville Syrjälä >Intel
>-----Original Message----- >From: Roper, Matthew D >Sent: Wednesday, October 31, 2018 10:32 PM >To: Shankar, Uma <uma.shankar@intel.com> >Cc: intel-gfx@lists.freedesktop.org; Lankhorst, Maarten ><maarten.lankhorst@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>; Sharma, >Shashank <shashank.sharma@intel.com> >Subject: Re: [v6 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to RGB >Conversion > >On Wed, Oct 31, 2018 at 07:05:31PM +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. >> >> 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. >> >> This has been verified and tested by Maarten and the change is working >> as expected. >> >> v2: Addressed Maarten's and Ville's review comments and added the >> coefficients in a 2D array instead of independent Macros. >> >> v3: Added individual coefficient matrix (9 values) instead of 6 >> register values as per Maarten's comment. Also addresed a shift issue >> with B channel coefficient. >> >> v4: Added support for Limited Range Color Handling >> >> v5: Fixed Matt and Maarten's review comments. >> >> v6: Added human readable matrix values for YUV to RGB Conversion along >> with just the bspec register values, as per Matt's suggestion. >> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com> >> --- >> drivers/gpu/drm/i915/intel_color.c | 103 >+++++++++++++++++++++++++++++++++++ >> drivers/gpu/drm/i915/intel_display.c | 23 ++++++-- >> drivers/gpu/drm/i915/intel_drv.h | 2 + >> 3 files changed, 122 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_color.c >> b/drivers/gpu/drm/i915/intel_color.c >> index 5127da2..6dc9075 100644 >> --- a/drivers/gpu/drm/i915/intel_color.c >> +++ b/drivers/gpu/drm/i915/intel_color.c >> @@ -57,6 +57,15 @@ >> #define CSC_RGB_TO_YUV_RV_GV 0xbce89ad8 #define >CSC_RGB_TO_YUV_BV >> 0x1e080000 >> >> +#define ROFF(x) (((x) & 0xffff) << 16) >> +#define GOFF(x) (((x) & 0xffff) << 0) >> +#define BOFF(x) (((x) & 0xffff) << 16) >> + >> +/* Preoffset values for YUV to RGB Conversion */ >> +#define PREOFF_YUV_TO_RGB_HI 0x1800 >> +#define PREOFF_YUV_TO_RGB_ME 0x1F00 >> +#define PREOFF_YUV_TO_RGB_LO 0x1800 >> + >> /* >> * 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 +652,100 @@ 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 u16 input_csc_matrix[][9] = { >> + /* >> + * BT.601 full range YCbCr -> full range RGB >> + * The matrix required is : >> + * [1.000, 0.000, 1.371, >> + * 1.000, -0.336, -0.698, >> + * 1.000, 1.732, 0.0000] >> + */ >> + [DRM_COLOR_YCBCR_BT601] = { >> + 0x7AF8, 0x7800, 0x0, >> + 0x8B28, 0x7800, 0x9AC0, >> + 0x0, 0x7800, 0x7DD8, >> + }, >> + /* >> + * BT.709 full range YCbCr -> full range RGB >> + * The matrix required is : >> + * [1.000, 0.000, 1.574, >> + * 1.000, -0.187, -0.468, >> + * 1.000, 1.855, 0.0000] >> + */ >> + [DRM_COLOR_YCBCR_BT709] = { >> + 0x7C98, 0x7800, 0x0, >> + 0x9EF8, 0x7800, 0xABF8, >> + 0x0, 0x7800, 0x7ED8, >> + }, >> + }; >> + >> + /* Matrix for Limited Range to Full Range Conversion */ >> + static const u16 input_csc_matrix_lr[][9] = { >> + /* >> + * BT.601 Limted range YCbCr -> full range RGB >> + * The matrix required is : >> + * [1.164384, 0.000, 1.596370, >> + * 1.138393, -0.382500, -0.794598, >> + * 1.138393, 1.971696, 0.0000] >> + */ >> + [DRM_COLOR_YCBCR_BT601] = { >> + 0x7CC8, 0x7950, 0x0, >> + 0x8CB8, 0x7918, 0x9C40, >> + 0x0, 0x7918, 0x7FC8, >> + }, >> + /* >> + * BT.709 Limited range YCbCr -> full range RGB >> + * The matrix required is : >> + * [1.164, 0.000, 1.833671, >> + * 1.138393, -0.213249, -0.532909, >> + * 1.138393, 2.112402, 0.0000] >> + */ > >Did you scale the BT.709 values slightly differently than the BT.601 values above? >The non-0/1 values don't seem to be exactly full*255/224 or full*255/219, but >they're still close enough though that the hex representation comes out the same >for all of the values except for the >0xADA8 (which I get as 0xADA0). So double-check that, but otherwise the patch >is, > >Reviewed-by: Matt Roper <matthew.d.roper@intel.com> As mentioned in earlier reply, This is due to the higher precision (-0.18732427293) which I took for conversion. So the values will match. Thanks a lot Matt for all your comment and review. Regards, Uma Shankar >> + [DRM_COLOR_YCBCR_BT709] = { >> + 0x7EA8, 0x7950, 0x0, >> + 0x8888, 0x7918, 0xADA8, >> + 0x0, 0x7918, 0x6870, >> + }, >> + }; >> + const u16 *csc; >> + >> + if (plane_state->base.color_range == >DRM_COLOR_YCBCR_FULL_RANGE) >> + csc = input_csc_matrix[plane_state->base.color_encoding]; >> + else >> + csc = input_csc_matrix_lr[plane_state->base.color_encoding]; >> + >> + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 0), ROFF(csc[0]) | >> + GOFF(csc[1])); >> + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 1), BOFF(csc[2])); >> + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 2), ROFF(csc[3]) | >> + GOFF(csc[4])); >> + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 3), BOFF(csc[5])); >> + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 4), ROFF(csc[6]) | >> + GOFF(csc[7])); >> + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 5), BOFF(csc[8])); >> + >> + I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 0), >> + PREOFF_YUV_TO_RGB_HI); >> + I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 1), >> + PREOFF_YUV_TO_RGB_ME); >> + I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 2), >> + PREOFF_YUV_TO_RGB_LO); >> + >> + I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 0), 0x0); >> + I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 1), 0x0); >> + I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 2), 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..d16a064 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -3666,6 +3666,7 @@ 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 +3677,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 >> > >-- >Matt Roper >Graphics Software Engineer >IoTG Platform Enabling & Development >Intel Corporation >(916) 356-2795
On Thu, Nov 01, 2018 at 06:22:08AM +0000, Shankar, Uma wrote: > > > >-----Original Message----- > >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > >Sent: Wednesday, October 31, 2018 7:36 PM > >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] [v6 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to > >RGB Conversion > > > >On Wed, Oct 31, 2018 at 07:05:31PM +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. > >> > >> 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. > >> > >> This has been verified and tested by Maarten and the change is working > >> as expected. > >> > >> v2: Addressed Maarten's and Ville's review comments and added the > >> coefficients in a 2D array instead of independent Macros. > >> > >> v3: Added individual coefficient matrix (9 values) instead of 6 > >> register values as per Maarten's comment. Also addresed a shift issue > >> with B channel coefficient. > >> > >> v4: Added support for Limited Range Color Handling > >> > >> v5: Fixed Matt and Maarten's review comments. > >> > >> v6: Added human readable matrix values for YUV to RGB Conversion along > >> with just the bspec register values, as per Matt's suggestion. > >> > >> Signed-off-by: Uma Shankar <uma.shankar@intel.com> > >> --- > >> drivers/gpu/drm/i915/intel_color.c | 103 > >+++++++++++++++++++++++++++++++++++ > >> drivers/gpu/drm/i915/intel_display.c | 23 ++++++-- > >> drivers/gpu/drm/i915/intel_drv.h | 2 + > >> 3 files changed, 122 insertions(+), 6 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_color.c > >> b/drivers/gpu/drm/i915/intel_color.c > >> index 5127da2..6dc9075 100644 > >> --- a/drivers/gpu/drm/i915/intel_color.c > >> +++ b/drivers/gpu/drm/i915/intel_color.c > >> @@ -57,6 +57,15 @@ > >> #define CSC_RGB_TO_YUV_RV_GV 0xbce89ad8 #define > >CSC_RGB_TO_YUV_BV > >> 0x1e080000 > >> > >> +#define ROFF(x) (((x) & 0xffff) << 16) > >> +#define GOFF(x) (((x) & 0xffff) << 0) > >> +#define BOFF(x) (((x) & 0xffff) << 16) > >> + > >> +/* Preoffset values for YUV to RGB Conversion */ > >> +#define PREOFF_YUV_TO_RGB_HI 0x1800 > >> +#define PREOFF_YUV_TO_RGB_ME 0x1F00 > >> +#define PREOFF_YUV_TO_RGB_LO 0x1800 > >> + > >> /* > >> * 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 +652,100 @@ 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 u16 input_csc_matrix[][9] = { > >> + /* > >> + * BT.601 full range YCbCr -> full range RGB > >> + * The matrix required is : > >> + * [1.000, 0.000, 1.371, > >> + * 1.000, -0.336, -0.698, > >> + * 1.000, 1.732, 0.0000] > >> + */ > >> + [DRM_COLOR_YCBCR_BT601] = { > >> + 0x7AF8, 0x7800, 0x0, > >> + 0x8B28, 0x7800, 0x9AC0, > >> + 0x0, 0x7800, 0x7DD8, > >> + }, > >> + /* > >> + * BT.709 full range YCbCr -> full range RGB > >> + * The matrix required is : > >> + * [1.000, 0.000, 1.574, > >> + * 1.000, -0.187, -0.468, > >> + * 1.000, 1.855, 0.0000] > >> + */ > >> + [DRM_COLOR_YCBCR_BT709] = { > >> + 0x7C98, 0x7800, 0x0, > >> + 0x9EF8, 0x7800, 0xABF8, > >> + 0x0, 0x7800, 0x7ED8, > >> + }, > >> + }; > >> + > >> + /* Matrix for Limited Range to Full Range Conversion */ > >> + static const u16 input_csc_matrix_lr[][9] = { > >> + /* > >> + * BT.601 Limted range YCbCr -> full range RGB > >> + * The matrix required is : > >> + * [1.164384, 0.000, 1.596370, > >> + * 1.138393, -0.382500, -0.794598, > >> + * 1.138393, 1.971696, 0.0000] > >> + */ > >> + [DRM_COLOR_YCBCR_BT601] = { > >> + 0x7CC8, 0x7950, 0x0, > >> + 0x8CB8, 0x7918, 0x9C40, > >> + 0x0, 0x7918, 0x7FC8, > >> + }, > >> + /* > >> + * BT.709 Limited range YCbCr -> full range RGB > >> + * The matrix required is : > >> + * [1.164, 0.000, 1.833671, > >> + * 1.138393, -0.213249, -0.532909, > >> + * 1.138393, 2.112402, 0.0000] > >> + */ > >> + [DRM_COLOR_YCBCR_BT709] = { > >> + 0x7EA8, 0x7950, 0x0, > >> + 0x8888, 0x7918, 0xADA8, > >> + 0x0, 0x7918, 0x6870, > >> + }, > >> + }; > >> + const u16 *csc; > >> + > >> + if (plane_state->base.color_range == > >DRM_COLOR_YCBCR_FULL_RANGE) > >> + csc = input_csc_matrix[plane_state->base.color_encoding]; > >> + else > >> + csc = input_csc_matrix_lr[plane_state->base.color_encoding]; > >> + > >> + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 0), ROFF(csc[0]) | > >> + GOFF(csc[1])); > >> + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 1), BOFF(csc[2])); > >> + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 2), ROFF(csc[3]) | > >> + GOFF(csc[4])); > >> + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 3), BOFF(csc[5])); > >> + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 4), ROFF(csc[6]) | > >> + GOFF(csc[7])); > >> + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 5), BOFF(csc[8])); > >> + > >> + I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 0), > >> + PREOFF_YUV_TO_RGB_HI); > >> + I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 1), > >> + PREOFF_YUV_TO_RGB_ME); > >> + I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 2), > >> + PREOFF_YUV_TO_RGB_LO); > >> + > >> + I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 0), 0x0); > >> + I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 1), 0x0); > >> + I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 2), 0x0); } > > > >Oh and please move this function into intel_sprite.c. That way it can be static. > > We were trying to consolidate all the color stuff and the helper functions in > intel_color.c. I prefer organizing based on hardware block. If there is other use for the same matrices and whatnot those can of course be put into intel_color.c, but the plane programming stuff should be in the plane code IMO. > Since its currently called from one place, it can be made static. > Will update the patch. Thanks. > > >> + > >> 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..d16a064 100644 > >> --- a/drivers/gpu/drm/i915/intel_display.c > >> +++ b/drivers/gpu/drm/i915/intel_display.c > >> @@ -3666,6 +3666,7 @@ 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 +3677,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 > > > >-- > >Ville Syrjälä > >Intel
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c index 5127da2..6dc9075 100644 --- a/drivers/gpu/drm/i915/intel_color.c +++ b/drivers/gpu/drm/i915/intel_color.c @@ -57,6 +57,15 @@ #define CSC_RGB_TO_YUV_RV_GV 0xbce89ad8 #define CSC_RGB_TO_YUV_BV 0x1e080000 +#define ROFF(x) (((x) & 0xffff) << 16) +#define GOFF(x) (((x) & 0xffff) << 0) +#define BOFF(x) (((x) & 0xffff) << 16) + +/* Preoffset values for YUV to RGB Conversion */ +#define PREOFF_YUV_TO_RGB_HI 0x1800 +#define PREOFF_YUV_TO_RGB_ME 0x1F00 +#define PREOFF_YUV_TO_RGB_LO 0x1800 + /* * 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 +652,100 @@ 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 u16 input_csc_matrix[][9] = { + /* + * BT.601 full range YCbCr -> full range RGB + * The matrix required is : + * [1.000, 0.000, 1.371, + * 1.000, -0.336, -0.698, + * 1.000, 1.732, 0.0000] + */ + [DRM_COLOR_YCBCR_BT601] = { + 0x7AF8, 0x7800, 0x0, + 0x8B28, 0x7800, 0x9AC0, + 0x0, 0x7800, 0x7DD8, + }, + /* + * BT.709 full range YCbCr -> full range RGB + * The matrix required is : + * [1.000, 0.000, 1.574, + * 1.000, -0.187, -0.468, + * 1.000, 1.855, 0.0000] + */ + [DRM_COLOR_YCBCR_BT709] = { + 0x7C98, 0x7800, 0x0, + 0x9EF8, 0x7800, 0xABF8, + 0x0, 0x7800, 0x7ED8, + }, + }; + + /* Matrix for Limited Range to Full Range Conversion */ + static const u16 input_csc_matrix_lr[][9] = { + /* + * BT.601 Limted range YCbCr -> full range RGB + * The matrix required is : + * [1.164384, 0.000, 1.596370, + * 1.138393, -0.382500, -0.794598, + * 1.138393, 1.971696, 0.0000] + */ + [DRM_COLOR_YCBCR_BT601] = { + 0x7CC8, 0x7950, 0x0, + 0x8CB8, 0x7918, 0x9C40, + 0x0, 0x7918, 0x7FC8, + }, + /* + * BT.709 Limited range YCbCr -> full range RGB + * The matrix required is : + * [1.164, 0.000, 1.833671, + * 1.138393, -0.213249, -0.532909, + * 1.138393, 2.112402, 0.0000] + */ + [DRM_COLOR_YCBCR_BT709] = { + 0x7EA8, 0x7950, 0x0, + 0x8888, 0x7918, 0xADA8, + 0x0, 0x7918, 0x6870, + }, + }; + const u16 *csc; + + if (plane_state->base.color_range == DRM_COLOR_YCBCR_FULL_RANGE) + csc = input_csc_matrix[plane_state->base.color_encoding]; + else + csc = input_csc_matrix_lr[plane_state->base.color_encoding]; + + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 0), ROFF(csc[0]) | + GOFF(csc[1])); + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 1), BOFF(csc[2])); + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 2), ROFF(csc[3]) | + GOFF(csc[4])); + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 3), BOFF(csc[5])); + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 4), ROFF(csc[6]) | + GOFF(csc[7])); + I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 5), BOFF(csc[8])); + + I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 0), + PREOFF_YUV_TO_RGB_HI); + I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 1), + PREOFF_YUV_TO_RGB_ME); + I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 2), + PREOFF_YUV_TO_RGB_LO); + + I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 0), 0x0); + I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 1), 0x0); + I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 2), 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..d16a064 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3666,6 +3666,7 @@ 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 +3677,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. This has been verified and tested by Maarten and the change is working as expected. v2: Addressed Maarten's and Ville's review comments and added the coefficients in a 2D array instead of independent Macros. v3: Added individual coefficient matrix (9 values) instead of 6 register values as per Maarten's comment. Also addresed a shift issue with B channel coefficient. v4: Added support for Limited Range Color Handling v5: Fixed Matt and Maarten's review comments. v6: Added human readable matrix values for YUV to RGB Conversion along with just the bspec register values, as per Matt's suggestion. Signed-off-by: Uma Shankar <uma.shankar@intel.com> --- drivers/gpu/drm/i915/intel_color.c | 103 +++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_display.c | 23 ++++++-- drivers/gpu/drm/i915/intel_drv.h | 2 + 3 files changed, 122 insertions(+), 6 deletions(-)