* Re: [v8 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion
2018-11-01 18:33 ` [v8 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion Uma Shankar
@ 2018-11-01 18:29 ` Ville Syrjälä
2018-11-01 18:42 ` Shankar, Uma
0 siblings, 1 reply; 5+ messages in thread
From: Ville Syrjälä @ 2018-11-01 18:29 UTC (permalink / raw)
To: Uma Shankar; +Cc: intel-gfx, ville.syrjala, maarten.lankhorst
On Fri, Nov 02, 2018 at 12:03:12AM +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 expecpted.
>
> 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.
>
> v7: Refactored the code, move csc coefficient programming function to
> intel_sprite.c and made it static as per Ville's review comment.
>
> v8: Addressed Ville's review comment. Called the coefficient programming
> from within the skl_program_plane and used I915_WRITE_FW instead of
> I915_WRITE.
>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>
> ---
> drivers/gpu/drm/i915/intel_display.c | 17 ++++--
> drivers/gpu/drm/i915/intel_sprite.c | 108 +++++++++++++++++++++++++++++++++++
> 2 files changed, 120 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index fe045ab..daec1fd 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) {
> @@ -3675,14 +3676,20 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
> plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
> 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;
> + if (fb->format->is_yuv && !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;
> + 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;
> + plane_color_ctl |=
> + PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
These formatting changes to the SDR plane path don't seem to belong
in this patch. Just adds noise making it hard to see what is actually
changed.
> + } else if (fb->format->is_yuv) {
> + plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
> }
>
> return plane_color_ctl;
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index c604f6b..0b9cf29 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -40,6 +40,7 @@
> #include "intel_frontbuffer.h"
> #include <drm/i915_drm.h>
> #include "i915_drv.h"
> +#include <drm/drm_color_mgmt.h>
>
> int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
> int usecs)
> @@ -361,6 +362,109 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state)
> ((crtc_w + 1) << 16)|(crtc_h + 1));
> }
>
> +/* 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
> +
> +#define ROFF(x) (((x) & 0xffff) << 16)
> +#define GOFF(x) (((x) & 0xffff) << 0)
> +#define BOFF(x) (((x) & 0xffff) << 16)
> +
> +static 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);
s/intel_plane/plane/
> + enum plane_id plane = intel_plane->id;
s/plane/plane_id/
I am trying to make the driver use consistnet names for things.
Let's not go against that process when adding new code.
> +
> + 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_FW(PLANE_INPUT_CSC_COEFF(pipe, plane, 0), ROFF(csc[0]) |
> + GOFF(csc[1]));
Some alignment issues here. I guess missing a reindent after
adding the _FW().
> + I915_WRITE_FW(PLANE_INPUT_CSC_COEFF(pipe, plane, 1), BOFF(csc[2]));
> + I915_WRITE_FW(PLANE_INPUT_CSC_COEFF(pipe, plane, 2), ROFF(csc[3]) |
> + GOFF(csc[4]));
> + I915_WRITE_FW(PLANE_INPUT_CSC_COEFF(pipe, plane, 3), BOFF(csc[5]));
> + I915_WRITE_FW(PLANE_INPUT_CSC_COEFF(pipe, plane, 4), ROFF(csc[6]) |
> + GOFF(csc[7]));
> + I915_WRITE_FW(PLANE_INPUT_CSC_COEFF(pipe, plane, 5), BOFF(csc[8]));
> +
> + I915_WRITE_FW(PLANE_INPUT_CSC_PREOFF(pipe, plane, 0),
> + PREOFF_YUV_TO_RGB_HI);
> + I915_WRITE_FW(PLANE_INPUT_CSC_PREOFF(pipe, plane, 1),
> + PREOFF_YUV_TO_RGB_ME);
> + I915_WRITE_FW(PLANE_INPUT_CSC_PREOFF(pipe, plane, 2),
> + PREOFF_YUV_TO_RGB_LO);
> + I915_WRITE_FW(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 0), 0x0);
> + I915_WRITE_FW(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 1), 0x0);
> + I915_WRITE_FW(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 2), 0x0);
> +}
> +
> static void
> skl_program_plane(struct intel_plane *plane,
> const struct intel_crtc_state *crtc_state,
> @@ -381,6 +485,7 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state)
> uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16;
> uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
> struct intel_plane *linked = plane_state->linked_plane;
> + const struct drm_framebuffer *fb = plane_state->base.fb;
> unsigned long irqflags;
> u32 keymsk = 0, keymax = 0;
>
> @@ -394,6 +499,9 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state)
> I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id),
> plane_state->color_ctl);
>
> + if (fb->format->is_yuv && icl_is_hdr_plane(plane))
> + icl_program_input_csc_coeff(crtc_state, plane_state);
> +
> if (key->flags) {
> I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value);
>
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
* [v8 0/2] Enable Plane Input CSC for ICL
@ 2018-11-01 18:33 Uma Shankar
2018-11-01 18:33 ` [v8 1/2] drm/i915/icl: Define Plane Input CSC Coefficient Registers Uma Shankar
2018-11-01 18:33 ` [v8 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion Uma Shankar
0 siblings, 2 replies; 5+ messages in thread
From: Uma Shankar @ 2018-11-01 18:33 UTC (permalink / raw)
To: intel-gfx; +Cc: ville.syrjala, maarten.lankhorst
This patch series enables plane input csc feature for
ICL. This is needed for YUV to RGB conversion on bottom
3 planes on ICL, other planes are handled in the legacy
way using fixed function hardware.
This series enables color conversion for Full Range YUV data,
limited range handling will be done as a separate patch.
v2: Separated the patch into 2 parts as per Maarten's comments.
Addressed Ville and Maarten's review comment.
v3: Redesigned the register macro definition as per Matt's comment.
Addressed Maarten's review comment.
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.
v7: Refactored the code, move csc coefficient programming function to
intel_sprite.c and made it static as per Ville's review comment.
v8: Addressed Ville's review comment. Called the coefficient programming
from within the skl_program_plane and used I915_WRITE_FW instead of
I915_WRITE.
This has been verified and tested by Maarten and the change is working
as expected.
Uma Shankar (2):
drm/i915/icl: Define Plane Input CSC Coefficient Registers
drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion
drivers/gpu/drm/i915/i915_reg.h | 50 ++++++++++++++++
drivers/gpu/drm/i915/intel_display.c | 17 ++++--
drivers/gpu/drm/i915/intel_sprite.c | 108 +++++++++++++++++++++++++++++++++++
include/drm/drm_color_mgmt.h | 9 +++
4 files changed, 179 insertions(+), 5 deletions(-)
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
* [v8 1/2] drm/i915/icl: Define Plane Input CSC Coefficient Registers
2018-11-01 18:33 [v8 0/2] Enable Plane Input CSC for ICL Uma Shankar
@ 2018-11-01 18:33 ` Uma Shankar
2018-11-01 18:33 ` [v8 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion Uma Shankar
1 sibling, 0 replies; 5+ messages in thread
From: Uma Shankar @ 2018-11-01 18:33 UTC (permalink / raw)
To: intel-gfx; +Cc: ville.syrjala, maarten.lankhorst
Defined the plane input csc coefficient registers and macros.
6 registers are used to program a total of 9 coefficients,
added macros to define each of them for all the planes
supporting the feature on pipes. On ICL, bottom 3 planes
have this capability.
v2: Segregated the register macro definition as separate patch
as per Maarten's suggestion.
v3: Removed a redundant 3rd Pipe register definition and
simplified the equally spaced register definition by adding an
offset as per Matt's comment.
v4: No Change
v5: Renamed the register Macro as per Matt's suggestion.
v6: No Change
v7: No Change
v8: No Change
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 50 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 69eb573..87c275c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6569,6 +6569,7 @@ enum {
#define _PLANE_COLOR_CTL_3_A 0x703CC /* GLK+ */
#define PLANE_COLOR_PIPE_GAMMA_ENABLE (1 << 30) /* Pre-ICL */
#define PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE (1 << 28)
+#define PLANE_COLOR_INPUT_CSC_ENABLE (1 << 20) /* ICL+ */
#define PLANE_COLOR_PIPE_CSC_ENABLE (1 << 23) /* Pre-ICL */
#define PLANE_COLOR_CSC_MODE_BYPASS (0 << 17)
#define PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709 (1 << 17)
@@ -6585,6 +6586,55 @@ enum {
#define _PLANE_NV12_BUF_CFG_1_A 0x70278
#define _PLANE_NV12_BUF_CFG_2_A 0x70378
+/* Input CSC Register Definitions */
+#define _PLANE_INPUT_CSC_RY_GY_1_A 0x701E0
+#define _PLANE_INPUT_CSC_RY_GY_2_A 0x702E0
+
+#define _PLANE_INPUT_CSC_RY_GY_1_B 0x711E0
+#define _PLANE_INPUT_CSC_RY_GY_2_B 0x712E0
+
+#define _PLANE_INPUT_CSC_RY_GY_1(pipe) \
+ _PIPE(pipe, _PLANE_INPUT_CSC_RY_GY_1_A, \
+ _PLANE_INPUT_CSC_RY_GY_1_B)
+#define _PLANE_INPUT_CSC_RY_GY_2(pipe) \
+ _PIPE(pipe, _PLANE_INPUT_CSC_RY_GY_2_A, \
+ _PLANE_INPUT_CSC_RY_GY_2_B)
+
+#define PLANE_INPUT_CSC_COEFF(pipe, plane, index) \
+ _MMIO_PLANE(plane, _PLANE_INPUT_CSC_RY_GY_1(pipe) + (index) * 4, \
+ _PLANE_INPUT_CSC_RY_GY_2(pipe) + (index) * 4)
+
+#define _PLANE_INPUT_CSC_PREOFF_HI_1_A 0x701F8
+#define _PLANE_INPUT_CSC_PREOFF_HI_2_A 0x702F8
+
+#define _PLANE_INPUT_CSC_PREOFF_HI_1_B 0x711F8
+#define _PLANE_INPUT_CSC_PREOFF_HI_2_B 0x712F8
+
+#define _PLANE_INPUT_CSC_PREOFF_HI_1(pipe) \
+ _PIPE(pipe, _PLANE_INPUT_CSC_PREOFF_HI_1_A, \
+ _PLANE_INPUT_CSC_PREOFF_HI_1_B)
+#define _PLANE_INPUT_CSC_PREOFF_HI_2(pipe) \
+ _PIPE(pipe, _PLANE_INPUT_CSC_PREOFF_HI_2_A, \
+ _PLANE_INPUT_CSC_PREOFF_HI_2_B)
+#define PLANE_INPUT_CSC_PREOFF(pipe, plane, index) \
+ _MMIO_PLANE(plane, _PLANE_INPUT_CSC_PREOFF_HI_1(pipe) + (index) * 4, \
+ _PLANE_INPUT_CSC_PREOFF_HI_2(pipe) + (index) * 4)
+
+#define _PLANE_INPUT_CSC_POSTOFF_HI_1_A 0x70204
+#define _PLANE_INPUT_CSC_POSTOFF_HI_2_A 0x70304
+
+#define _PLANE_INPUT_CSC_POSTOFF_HI_1_B 0x71204
+#define _PLANE_INPUT_CSC_POSTOFF_HI_2_B 0x71304
+
+#define _PLANE_INPUT_CSC_POSTOFF_HI_1(pipe) \
+ _PIPE(pipe, _PLANE_INPUT_CSC_POSTOFF_HI_1_A, \
+ _PLANE_INPUT_CSC_POSTOFF_HI_1_B)
+#define _PLANE_INPUT_CSC_POSTOFF_HI_2(pipe) \
+ _PIPE(pipe, _PLANE_INPUT_CSC_POSTOFF_HI_2_A, \
+ _PLANE_INPUT_CSC_POSTOFF_HI_2_B)
+#define PLANE_INPUT_CSC_POSTOFF(pipe, plane, index) \
+ _MMIO_PLANE(plane, _PLANE_INPUT_CSC_POSTOFF_HI_1(pipe) + (index) * 4, \
+ _PLANE_INPUT_CSC_POSTOFF_HI_2(pipe) + (index) * 4)
#define _PLANE_CTL_1_B 0x71180
#define _PLANE_CTL_2_B 0x71280
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [v8 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion
2018-11-01 18:33 [v8 0/2] Enable Plane Input CSC for ICL Uma Shankar
2018-11-01 18:33 ` [v8 1/2] drm/i915/icl: Define Plane Input CSC Coefficient Registers Uma Shankar
@ 2018-11-01 18:33 ` Uma Shankar
2018-11-01 18:29 ` Ville Syrjälä
1 sibling, 1 reply; 5+ messages in thread
From: Uma Shankar @ 2018-11-01 18:33 UTC (permalink / raw)
To: intel-gfx; +Cc: ville.syrjala, maarten.lankhorst
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 expecpted.
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.
v7: Refactored the code, move csc coefficient programming function to
intel_sprite.c and made it static as per Ville's review comment.
v8: Addressed Ville's review comment. Called the coefficient programming
from within the skl_program_plane and used I915_WRITE_FW instead of
I915_WRITE.
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 17 ++++--
drivers/gpu/drm/i915/intel_sprite.c | 108 +++++++++++++++++++++++++++++++++++
2 files changed, 120 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fe045ab..daec1fd 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) {
@@ -3675,14 +3676,20 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
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;
+ if (fb->format->is_yuv && !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;
+ 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;
+ plane_color_ctl |=
+ PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
+ } else if (fb->format->is_yuv) {
+ plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
}
return plane_color_ctl;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index c604f6b..0b9cf29 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -40,6 +40,7 @@
#include "intel_frontbuffer.h"
#include <drm/i915_drm.h>
#include "i915_drv.h"
+#include <drm/drm_color_mgmt.h>
int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
int usecs)
@@ -361,6 +362,109 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state)
((crtc_w + 1) << 16)|(crtc_h + 1));
}
+/* 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
+
+#define ROFF(x) (((x) & 0xffff) << 16)
+#define GOFF(x) (((x) & 0xffff) << 0)
+#define BOFF(x) (((x) & 0xffff) << 16)
+
+static 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_FW(PLANE_INPUT_CSC_COEFF(pipe, plane, 0), ROFF(csc[0]) |
+ GOFF(csc[1]));
+ I915_WRITE_FW(PLANE_INPUT_CSC_COEFF(pipe, plane, 1), BOFF(csc[2]));
+ I915_WRITE_FW(PLANE_INPUT_CSC_COEFF(pipe, plane, 2), ROFF(csc[3]) |
+ GOFF(csc[4]));
+ I915_WRITE_FW(PLANE_INPUT_CSC_COEFF(pipe, plane, 3), BOFF(csc[5]));
+ I915_WRITE_FW(PLANE_INPUT_CSC_COEFF(pipe, plane, 4), ROFF(csc[6]) |
+ GOFF(csc[7]));
+ I915_WRITE_FW(PLANE_INPUT_CSC_COEFF(pipe, plane, 5), BOFF(csc[8]));
+
+ I915_WRITE_FW(PLANE_INPUT_CSC_PREOFF(pipe, plane, 0),
+ PREOFF_YUV_TO_RGB_HI);
+ I915_WRITE_FW(PLANE_INPUT_CSC_PREOFF(pipe, plane, 1),
+ PREOFF_YUV_TO_RGB_ME);
+ I915_WRITE_FW(PLANE_INPUT_CSC_PREOFF(pipe, plane, 2),
+ PREOFF_YUV_TO_RGB_LO);
+ I915_WRITE_FW(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 0), 0x0);
+ I915_WRITE_FW(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 1), 0x0);
+ I915_WRITE_FW(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 2), 0x0);
+}
+
static void
skl_program_plane(struct intel_plane *plane,
const struct intel_crtc_state *crtc_state,
@@ -381,6 +485,7 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state)
uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16;
uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
struct intel_plane *linked = plane_state->linked_plane;
+ const struct drm_framebuffer *fb = plane_state->base.fb;
unsigned long irqflags;
u32 keymsk = 0, keymax = 0;
@@ -394,6 +499,9 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state)
I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id),
plane_state->color_ctl);
+ if (fb->format->is_yuv && icl_is_hdr_plane(plane))
+ icl_program_input_csc_coeff(crtc_state, plane_state);
+
if (key->flags) {
I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value);
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [v8 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion
2018-11-01 18:29 ` Ville Syrjälä
@ 2018-11-01 18:42 ` Shankar, Uma
0 siblings, 0 replies; 5+ messages in thread
From: Shankar, Uma @ 2018-11-01 18:42 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, Syrjala, Ville, Lankhorst, Maarten
>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>Sent: Friday, November 2, 2018 12:00 AM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Syrjala, Ville <ville.syrjala@intel.com>;
>Lankhorst, Maarten <maarten.lankhorst@intel.com>
>Subject: Re: [Intel-gfx] [v8 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to
>RGB Conversion
>
>On Fri, Nov 02, 2018 at 12:03:12AM +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 expecpted.
>>
>> 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.
>>
>> v7: Refactored the code, move csc coefficient programming function to
>> intel_sprite.c and made it static as per Ville's review comment.
>>
>> v8: Addressed Ville's review comment. Called the coefficient
>> programming from within the skl_program_plane and used I915_WRITE_FW
>> instead of I915_WRITE.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
>> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>
>> ---
>> drivers/gpu/drm/i915/intel_display.c | 17 ++++--
>> drivers/gpu/drm/i915/intel_sprite.c | 108
>> +++++++++++++++++++++++++++++++++++
>> 2 files changed, 120 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index fe045ab..daec1fd 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) {
>> @@ -3675,14 +3676,20 @@ u32 glk_plane_color_ctl(const struct
>intel_crtc_state *crtc_state,
>> plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
>> 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;
>> + if (fb->format->is_yuv && !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;
>> + 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;
>> + plane_color_ctl |=
>> + PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
>
>These formatting changes to the SDR plane path don't seem to belong in this
>patch. Just adds noise making it hard to see what is actually changed.
It was going beyond 80 characters so thought of correcting it, but yes they don't
belong to the current change here. Will drop these from the patch.
>> + } else if (fb->format->is_yuv) {
>> + plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
>> }
>>
>> return plane_color_ctl;
>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
>> b/drivers/gpu/drm/i915/intel_sprite.c
>> index c604f6b..0b9cf29 100644
>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>> @@ -40,6 +40,7 @@
>> #include "intel_frontbuffer.h"
>> #include <drm/i915_drm.h>
>> #include "i915_drv.h"
>> +#include <drm/drm_color_mgmt.h>
>>
>> int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
>> int usecs)
>> @@ -361,6 +362,109 @@ int intel_plane_check_src_coordinates(struct
>intel_plane_state *plane_state)
>> ((crtc_w + 1) << 16)|(crtc_h + 1)); }
>>
>> +/* 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
>> +
>> +#define ROFF(x) (((x) & 0xffff) << 16)
>> +#define GOFF(x) (((x) & 0xffff) << 0)
>> +#define BOFF(x) (((x) & 0xffff) << 16)
>> +
>> +static 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);
>
>s/intel_plane/plane/
>
>> + enum plane_id plane = intel_plane->id;
>
>s/plane/plane_id/
>
>I am trying to make the driver use consistnet names for things.
>Let's not go against that process when adding new code.
Ok Sure, will keep this in mind for future changes as well.
>> +
>> + 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_FW(PLANE_INPUT_CSC_COEFF(pipe, plane, 0), ROFF(csc[0])
>|
>> + GOFF(csc[1]));
>
>Some alignment issues here. I guess missing a reindent after adding the _FW().
Yeah, will rectify this.
>> + I915_WRITE_FW(PLANE_INPUT_CSC_COEFF(pipe, plane, 1),
>BOFF(csc[2]));
>> + I915_WRITE_FW(PLANE_INPUT_CSC_COEFF(pipe, plane, 2), ROFF(csc[3])
>|
>> + GOFF(csc[4]));
>> + I915_WRITE_FW(PLANE_INPUT_CSC_COEFF(pipe, plane, 3),
>BOFF(csc[5]));
>> + I915_WRITE_FW(PLANE_INPUT_CSC_COEFF(pipe, plane, 4), ROFF(csc[6])
>|
>> + GOFF(csc[7]));
>> + I915_WRITE_FW(PLANE_INPUT_CSC_COEFF(pipe, plane, 5),
>BOFF(csc[8]));
>> +
>> + I915_WRITE_FW(PLANE_INPUT_CSC_PREOFF(pipe, plane, 0),
>> + PREOFF_YUV_TO_RGB_HI);
>> + I915_WRITE_FW(PLANE_INPUT_CSC_PREOFF(pipe, plane, 1),
>> + PREOFF_YUV_TO_RGB_ME);
>> + I915_WRITE_FW(PLANE_INPUT_CSC_PREOFF(pipe, plane, 2),
>> + PREOFF_YUV_TO_RGB_LO);
>> + I915_WRITE_FW(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 0), 0x0);
>> + I915_WRITE_FW(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 1), 0x0);
>> + I915_WRITE_FW(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 2), 0x0); }
>> +
>> static void
>> skl_program_plane(struct intel_plane *plane,
>> const struct intel_crtc_state *crtc_state, @@ -381,6 +485,7
>@@
>> int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state)
>> uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16;
>> uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
>> struct intel_plane *linked = plane_state->linked_plane;
>> + const struct drm_framebuffer *fb = plane_state->base.fb;
>> unsigned long irqflags;
>> u32 keymsk = 0, keymax = 0;
>>
>> @@ -394,6 +499,9 @@ int intel_plane_check_src_coordinates(struct
>intel_plane_state *plane_state)
>> I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id),
>> plane_state->color_ctl);
>>
>> + if (fb->format->is_yuv && icl_is_hdr_plane(plane))
>> + icl_program_input_csc_coeff(crtc_state, plane_state);
>> +
>> if (key->flags) {
>> I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key-
>>min_value);
>>
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>--
>Ville Syrjälä
>Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-11-01 18:43 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-01 18:33 [v8 0/2] Enable Plane Input CSC for ICL Uma Shankar
2018-11-01 18:33 ` [v8 1/2] drm/i915/icl: Define Plane Input CSC Coefficient Registers Uma Shankar
2018-11-01 18:33 ` [v8 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion Uma Shankar
2018-11-01 18:29 ` Ville Syrjälä
2018-11-01 18:42 ` Shankar, Uma
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.