[Intel-gfx] [PATCH 2/5] drm/i915: Fix plane YCbCr->RGB conversion for GLK
Sharma, Shashank
shashank.sharma at intel.com
Thu Jun 15 13:28:45 UTC 2017
Regards
Shashank
On 6/15/2017 6:12 PM, Ville Syrjälä wrote:
> On Thu, Jun 15, 2017 at 05:57:21PM +0530, Sharma, Shashank wrote:
>> Regards
>> Shashank
>> On 6/9/2017 2:03 AM, ville.syrjala at linux.intel.com wrote:
>>> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>>
>>> On GLK the plane CSC controls moved into the COLOR_CTL register.
>>> Update the code to progam the YCbCr->RGB CSC mode correctly when
>>> faced with an YCbCr framebuffer.
>>>
>>> The spec is rather confusing as it calls the mode "YUV601 to RGB709".
>>> I'm going to assume that just means it's going to use the YCbCr->RGB
>>> matrix as specified in BT.601 and doesn't actually change the gamut.
>>>
>>> Cc: Jyri Sarha <jsarha at ti.com>
>>> Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira at intel.com>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>> ---
>>> drivers/gpu/drm/i915/i915_reg.h | 5 +++++
>>> drivers/gpu/drm/i915/intel_display.c | 19 ++++++++++++++++---
>>> drivers/gpu/drm/i915/intel_drv.h | 2 ++
>>> drivers/gpu/drm/i915/intel_sprite.c | 13 +++++--------
>>> 4 files changed, 28 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>> index ce7c0dc79cf7..f4f51e7a3e83 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -5924,6 +5924,11 @@ enum {
>>> #define _PLANE_COLOR_CTL_3_A 0x703CC /* GLK+ */
>>> #define PLANE_COLOR_PIPE_GAMMA_ENABLE (1 << 30)
>>> #define PLANE_COLOR_PIPE_CSC_ENABLE (1 << 23)
>>> +#define PLANE_COLOR_CSC_MODE_BYPASS (0 << 17)
>>> +#define PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709 (1 << 17)
>>> +#define PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709 (2 << 17)
>>> +#define PLANE_COLOR_CSC_MODE_YUV2020_TO_RGB2020 (3 << 17)
>>> +#define PLANE_COLOR_CSC_MODE_RGB709_TO_RGB2020 (4 << 17)
>>> #define PLANE_COLOR_PLANE_GAMMA_DISABLE (1 << 13)
>>> #define _PLANE_BUF_CFG_1_A 0x7027c
>>> #define _PLANE_BUF_CFG_2_A 0x7037c
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>> index 25390dd8e08e..e2aaaf60a969 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -3335,6 +3335,21 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
>>> return plane_ctl;
>>> }
>>>
>>> +u32 glk_color_ctl(const struct intel_plane_state *plane_state)
>>> +{
>>> + const struct drm_framebuffer *fb = plane_state->base.fb;
>>> + u32 color_ctl;
>>> +
>>> + color_ctl = PLANE_COLOR_PIPE_GAMMA_ENABLE |
>>> + PLANE_COLOR_PIPE_CSC_ENABLE |
>>> + PLANE_COLOR_PLANE_GAMMA_DISABLE;
>>> +
>>> + if (intel_format_is_yuv(fb->format->format))
>>> + color_ctl |= PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709;
>> why not YUV709_TO_RGB709 ?
> To match the other platforms.
I am slightly confused. Dint we just upgrade our CSC matrix from stone
age, to REC_709, in patch 4 ?
Shouldn't we be consistent here ? or am I missing something basic :-) ?
- Shashank
>>> +
>>> + return color_ctl;
>>> +}
>>> +
>> You might want to have a look on this series from Dhanya too:
>>
>> https://patchwork.freedesktop.org/series/17259/
> I think we have everything we need from there.
>
>> - Shashank
>>
>>> static void skylake_update_primary_plane(struct intel_plane *plane,
>>> const struct intel_crtc_state *crtc_state,
>>> const struct intel_plane_state *plane_state)
>>> @@ -3374,9 +3389,7 @@ static void skylake_update_primary_plane(struct intel_plane *plane,
>>>
>>> if (IS_GEMINILAKE(dev_priv)) {
>>> I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id),
>>> - PLANE_COLOR_PIPE_GAMMA_ENABLE |
>>> - PLANE_COLOR_PIPE_CSC_ENABLE |
>>> - PLANE_COLOR_PLANE_GAMMA_DISABLE);
>>> + glk_color_ctl(plane_state));
>>> }
>>>
>>> I915_WRITE_FW(PLANE_CTL(pipe, plane_id), plane_ctl);
>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>>> index 83dd40905821..e13e714b1176 100644
>>> --- a/drivers/gpu/drm/i915/intel_drv.h
>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>>> @@ -1484,6 +1484,7 @@ static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state)
>>>
>>> u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
>>> const struct intel_plane_state *plane_state);
>>> +u32 glk_color_ctl(const struct intel_plane_state *plane_state);
>>> u32 skl_plane_stride(const struct drm_framebuffer *fb, int plane,
>>> unsigned int rotation);
>>> int skl_check_plane_surface(struct intel_plane_state *plane_state);
>>> @@ -1888,6 +1889,7 @@ bool intel_sdvo_init(struct drm_i915_private *dev_priv,
>>>
>>>
>>> /* intel_sprite.c */
>>> +bool intel_format_is_yuv(u32 format);
>>> int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
>>> int usecs);
>>> struct intel_plane *intel_sprite_plane_create(struct drm_i915_private *dev_priv,
>>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
>>> index 2ce3b3c6ffa8..cc07157f2eb6 100644
>>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>>> @@ -40,8 +40,7 @@
>>> #include <drm/i915_drm.h>
>>> #include "i915_drv.h"
>>>
>>> -static bool
>>> -format_is_yuv(uint32_t format)
>>> +bool intel_format_is_yuv(u32 format)
>>> {
>>> switch (format) {
>>> case DRM_FORMAT_YUYV:
>>> @@ -264,9 +263,7 @@ skl_update_plane(struct intel_plane *plane,
>>>
>>> if (IS_GEMINILAKE(dev_priv)) {
>>> I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id),
>>> - PLANE_COLOR_PIPE_GAMMA_ENABLE |
>>> - PLANE_COLOR_PIPE_CSC_ENABLE |
>>> - PLANE_COLOR_PLANE_GAMMA_DISABLE);
>>> + glk_color_ctl(plane_state));
>>> }
>>>
>>> if (key->flags) {
>>> @@ -333,7 +330,7 @@ chv_update_csc(const struct intel_plane_state *plane_state)
>>> enum plane_id plane_id = plane->id;
>>>
>>> /* Seems RGB data bypasses the CSC always */
>>> - if (!format_is_yuv(fb->format->format))
>>> + if (!intel_format_is_yuv(fb->format->format))
>>> return;
>>>
>>> /*
>>> @@ -375,7 +372,7 @@ vlv_update_clrc(const struct intel_plane_state *plane_state)
>>> enum plane_id plane_id = plane->id;
>>> int con, bri, sh_sin, sh_cos;
>>>
>>> - if (format_is_yuv(fb->format->format)) {
>>> + if (intel_format_is_yuv(fb->format->format)) {
>>> /*
>>> * expand limited range to full range.
>>> * contrast is applied first, then brightness
>>> @@ -933,7 +930,7 @@ intel_check_sprite_plane(struct intel_plane *plane,
>>> src_y = src->y1 >> 16;
>>> src_h = drm_rect_height(src) >> 16;
>>>
>>> - if (format_is_yuv(fb->format->format)) {
>>> + if (intel_format_is_yuv(fb->format->format)) {
>>> src_x &= ~1;
>>> src_w &= ~1;
>>>
More information about the Intel-gfx
mailing list