Re: [Intel-gfx] [v5 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion
>-Original Message- >From: Roper, Matthew D >Sent: Wednesday, October 31, 2018 10:11 PM >To: Shankar, Uma >Cc: intel-gfx@lists.freedesktop.org; Syrjala, Ville ; >Lankhorst, Maarten >Subject: Re: [Intel-gfx] [v5 2/2] drm/i915/icl: Enable Plane Input CSC for YUV >to >RGB Conversion > >On Wed, Oct 31, 2018 at 05:34:19AM -0700, Shankar, Uma wrote: >> >> >> >-Original Message- >> >From: Roper, Matthew D >> >Sent: Tuesday, October 30, 2018 4:59 AM >> >To: Shankar, Uma >> >Cc: intel-gfx@lists.freedesktop.org; Syrjala, Ville >> >; Lankhorst, Maarten >> > >> >Subject: Re: [Intel-gfx] [v5 2/2] drm/i915/icl: Enable Plane Input >> >CSC for YUV to RGB Conversion >> > >> >On Fri, Oct 26, 2018 at 03:31:57PM +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. >> >> >> >> 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. >> >> >> >> 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. >> >> >> >> Signed-off-by: Uma Shankar >> >> --- >> >> drivers/gpu/drm/i915/intel_color.c | 79 >> > >> >> drivers/gpu/drm/i915/intel_display.c | 23 --- >> >> drivers/gpu/drm/i915/intel_drv.h | 2 + >> >> 3 files changed, 98 insertions(+), 6 deletions(-) >> >> >> >> diff --git a/drivers/gpu/drm/i915/intel_color.c >> >> b/drivers/gpu/drm/i915/intel_color.c >> >> index 5127da2..681cd13 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 >> >> 0x1e08 >> >> >> >> +#define ROFF(x) (((x) & 0x) << 16) >> >> +#define GOFF(x) (((x) & 0x) << 0) >> >> +#define BOFF(x) (((x) & 0x) << 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,76 @@ 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] = { >> > >> >Can you add comments to these indicating the human-readable values >> >they translate to? >> >> Sure Matt, will add tha
Re: [Intel-gfx] [v5 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion
On Wed, Oct 31, 2018 at 05:34:19AM -0700, Shankar, Uma wrote: > > > >-Original Message- > >From: Roper, Matthew D > >Sent: Tuesday, October 30, 2018 4:59 AM > >To: Shankar, Uma > >Cc: intel-gfx@lists.freedesktop.org; Syrjala, Ville > >; > >Lankhorst, Maarten > >Subject: Re: [Intel-gfx] [v5 2/2] drm/i915/icl: Enable Plane Input CSC for > >YUV to > >RGB Conversion > > > >On Fri, Oct 26, 2018 at 03:31:57PM +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. > >> > >> 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. > >> > >> 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. > >> > >> Signed-off-by: Uma Shankar > >> --- > >> drivers/gpu/drm/i915/intel_color.c | 79 > > > >> drivers/gpu/drm/i915/intel_display.c | 23 --- > >> drivers/gpu/drm/i915/intel_drv.h | 2 + > >> 3 files changed, 98 insertions(+), 6 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_color.c > >> b/drivers/gpu/drm/i915/intel_color.c > >> index 5127da2..681cd13 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 > >> 0x1e08 > >> > >> +#define ROFF(x) (((x) & 0x) << 16) > >> +#define GOFF(x) (((x) & 0x) << 0) > >> +#define BOFF(x) (((x) & 0x) << 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,76 @@ 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] = { > > > >Can you add comments to these indicating the human-readable values they > >translate to? > > Sure Matt, will add that. > > >> + /* BT.601 full range YCbCr -> full range RGB */ > >> + [DRM_COLOR_YCBCR_BT601] = { > >> + 0x7AF8, 0x7800, 0x0, > >> + 0x8B28, 0x7800, 0x9AC0, > >> + 0x0, 0x7800, 0x7DD8, > >> + }, > >> + /* BT.709 full range YCbCr -> full range RGB */ > >> + [DRM_COLOR_YCBCR_BT709] = { > >> + 0x7C98, 0x7800, 0x0, > >> + 0x9EF8, 0x7800, 0xABF8, > >> + 0x0, 0x7800, 0x7ED8, > >> + }, > &
Re: [Intel-gfx] [v5 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion
>-Original Message- >From: Maarten Lankhorst [mailto:maarten.lankho...@linux.intel.com] >Sent: Tuesday, October 30, 2018 4:25 PM >To: Shankar, Uma ; intel-gfx@lists.freedesktop.org >Cc: Syrjala, Ville ; Lankhorst, Maarten > >Subject: Re: [Intel-gfx] [v5 2/2] drm/i915/icl: Enable Plane Input CSC for YUV >to >RGB Conversion > >Op 26-10-18 om 12:01 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. >> >> 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. >> >> Signed-off-by: Uma Shankar >> --- >> drivers/gpu/drm/i915/intel_color.c | 79 > >> drivers/gpu/drm/i915/intel_display.c | 23 --- >> drivers/gpu/drm/i915/intel_drv.h | 2 + >> 3 files changed, 98 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_color.c >> b/drivers/gpu/drm/i915/intel_color.c >> index 5127da2..681cd13 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 >> 0x1e08 >> >> +#define ROFF(x) (((x) & 0x) << 16) >> +#define GOFF(x) (((x) & 0x) << 0) >> +#define BOFF(x) (((x) & 0x) << 16) >> + >> +/* Preoffset values for YUV to RGB Conversion */ >> +#define PREOFF_YUV_TO_RGB_HI0x1800 >> +#define PREOFF_YUV_TO_RGB_ME0x1F00 >> +#define PREOFF_YUV_TO_RGB_LO0x1800 >> + >> /* >> * 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,76 @@ 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 */ >> +[DRM_COLOR_YCBCR_BT601] = { >> +0x7AF8, 0x7800, 0x0, >> +0x8B28, 0x7800, 0x9AC0, >> +0x0, 0x7800, 0x7DD8, >> +}, >> +/* BT.709 full range YCbCr -> full range RGB */ >> +[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 */ >> +[DRM_COLOR_YCBCR_BT601] = { >> +0x7CC8, 0x7950, 0x0, >> +0x8CB8, 0x7918, 0x9C40, >> +0x0, 0x7918, 0x7FC8, >> +}, >> +/* BT.709 Limited range YCbCr -> full range RGB */ >> +[DRM_COLOR_YCBCR_BT709] = { >> +0x7EA8, 0x7950, 0x0,
Re: [Intel-gfx] [v5 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion
>-Original Message- >From: Roper, Matthew D >Sent: Tuesday, October 30, 2018 4:59 AM >To: Shankar, Uma >Cc: intel-gfx@lists.freedesktop.org; Syrjala, Ville ; >Lankhorst, Maarten >Subject: Re: [Intel-gfx] [v5 2/2] drm/i915/icl: Enable Plane Input CSC for YUV >to >RGB Conversion > >On Fri, Oct 26, 2018 at 03:31:57PM +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. >> >> 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. >> >> 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. >> >> Signed-off-by: Uma Shankar >> --- >> drivers/gpu/drm/i915/intel_color.c | 79 > >> drivers/gpu/drm/i915/intel_display.c | 23 --- >> drivers/gpu/drm/i915/intel_drv.h | 2 + >> 3 files changed, 98 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_color.c >> b/drivers/gpu/drm/i915/intel_color.c >> index 5127da2..681cd13 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 >> 0x1e08 >> >> +#define ROFF(x) (((x) & 0x) << 16) >> +#define GOFF(x) (((x) & 0x) << 0) >> +#define BOFF(x) (((x) & 0x) << 16) >> + >> +/* Preoffset values for YUV to RGB Conversion */ >> +#define PREOFF_YUV_TO_RGB_HI0x1800 >> +#define PREOFF_YUV_TO_RGB_ME0x1F00 >> +#define PREOFF_YUV_TO_RGB_LO0x1800 >> + >> /* >> * 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,76 @@ 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] = { > >Can you add comments to these indicating the human-readable values they >translate to? Sure Matt, will add that. >> +/* BT.601 full range YCbCr -> full range RGB */ >> +[DRM_COLOR_YCBCR_BT601] = { >> +0x7AF8, 0x7800, 0x0, >> +0x8B28, 0x7800, 0x9AC0, >> +0x0, 0x7800, 0x7DD8, >> +}, >> +/* BT.709 full range YCbCr -> full range RGB */ >> +[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 */ >> +[DRM_COLOR_YCBCR_BT601] = { >> +0x7CC8, 0x7950, 0x0, >> +0x8CB8, 0x7918, 0x9C40, >> +0x0, 0x7918, 0x7FC8, > >Are these obtained by scaling the first row (Y-based) by 256/219 and the other >two rows (Cb and Cr) b
Re: [Intel-gfx] [v5 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion
Op 26-10-18 om 12:01 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. > > 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. > > Signed-off-by: Uma Shankar > --- > drivers/gpu/drm/i915/intel_color.c | 79 > > drivers/gpu/drm/i915/intel_display.c | 23 --- > drivers/gpu/drm/i915/intel_drv.h | 2 + > 3 files changed, 98 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_color.c > b/drivers/gpu/drm/i915/intel_color.c > index 5127da2..681cd13 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 0x1e08 > > +#define ROFF(x) (((x) & 0x) << 16) > +#define GOFF(x) (((x) & 0x) << 0) > +#define BOFF(x) (((x) & 0x) << 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,76 @@ 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 */ > + [DRM_COLOR_YCBCR_BT601] = { > + 0x7AF8, 0x7800, 0x0, > + 0x8B28, 0x7800, 0x9AC0, > + 0x0, 0x7800, 0x7DD8, > + }, > + /* BT.709 full range YCbCr -> full range RGB */ > + [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 */ > + [DRM_COLOR_YCBCR_BT601] = { > + 0x7CC8, 0x7950, 0x0, > + 0x8CB8, 0x7918, 0x9C40, > + 0x0, 0x7918, 0x7FC8, > + }, > + /* BT.709 Limited range YCbCr -> full range RGB */ > + [DRM_COLOR_YCBCR_BT709] = { > + 0x7EA8, 0x7950, 0x0, > + 0x, 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); > +
Re: [Intel-gfx] [v5 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion
On Fri, Oct 26, 2018 at 03:31:57PM +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. > > 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. > > 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. > > Signed-off-by: Uma Shankar > --- > drivers/gpu/drm/i915/intel_color.c | 79 > > drivers/gpu/drm/i915/intel_display.c | 23 --- > drivers/gpu/drm/i915/intel_drv.h | 2 + > 3 files changed, 98 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_color.c > b/drivers/gpu/drm/i915/intel_color.c > index 5127da2..681cd13 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 0x1e08 > > +#define ROFF(x) (((x) & 0x) << 16) > +#define GOFF(x) (((x) & 0x) << 0) > +#define BOFF(x) (((x) & 0x) << 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,76 @@ 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] = { Can you add comments to these indicating the human-readable values they translate to? > + /* BT.601 full range YCbCr -> full range RGB */ > + [DRM_COLOR_YCBCR_BT601] = { > + 0x7AF8, 0x7800, 0x0, > + 0x8B28, 0x7800, 0x9AC0, > + 0x0, 0x7800, 0x7DD8, > + }, > + /* BT.709 full range YCbCr -> full range RGB */ > + [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 */ > + [DRM_COLOR_YCBCR_BT601] = { > + 0x7CC8, 0x7950, 0x0, > + 0x8CB8, 0x7918, 0x9C40, > + 0x0, 0x7918, 0x7FC8, Are these obtained by scaling the first row (Y-based) by 256/219 and the other two rows (Cb and Cr) by 256/224? If so, it looks like you've always rounded down, whereas in some cases rounding up gives you a closer value (and matches how the bspec seems to have chosen the full range encodings for their example). [ 0x7CD0, 0x7958,0x0 ] [ 0x8CC0, 0x7928, 0x9C48 ] [0x0, 0x7928, 0x7FD8 ] Our encodings of the 1.0 value on the second two rows seems to deviate slightly more for some reason; not sure why that is. For completeness, here's how I came up with 0x7928: 1 * 256/224 = 1.142857143 Sign bit = 0 Exponent bits = 0b111 Mantissa bits = round(1.142857143 << 8) = round(292.571428571) = 293 = 0b100100101 Reserved bits = 0b000 Result = 0111 1001 0010 1000 = 0x7928 If you did floor() instead of round() for the mantissa, you'd get 292, which would translate to 0x7920 instead. > + }, > + /* BT.709 Limited range YCbCr -> full range
[Intel-gfx] [v5 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion
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. 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. Signed-off-by: Uma Shankar --- drivers/gpu/drm/i915/intel_color.c | 79 drivers/gpu/drm/i915/intel_display.c | 23 --- drivers/gpu/drm/i915/intel_drv.h | 2 + 3 files changed, 98 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c index 5127da2..681cd13 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 0x1e08 +#define ROFF(x) (((x) & 0x) << 16) +#define GOFF(x) (((x) & 0x) << 0) +#define BOFF(x) (((x) & 0x) << 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,76 @@ 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 */ + [DRM_COLOR_YCBCR_BT601] = { + 0x7AF8, 0x7800, 0x0, + 0x8B28, 0x7800, 0x9AC0, + 0x0, 0x7800, 0x7DD8, + }, + /* BT.709 full range YCbCr -> full range RGB */ + [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 */ + [DRM_COLOR_YCBCR_BT601] = { + 0x7CC8, 0x7950, 0x0, + 0x8CB8, 0x7918, 0x9C40, + 0x0, 0x7918, 0x7FC8, + }, + /* BT.709 Limited range YCbCr -> full range RGB */ + [DRM_COLOR_YCBCR_BT709] = { + 0x7EA8, 0x7950, 0x0, + 0x, 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), +