Op 21-09-18 om 20:53 schreef Ville Syrjälä:
> On Fri, Sep 21, 2018 at 07:39:43PM +0200, Maarten Lankhorst wrote:
>> We configure the chroma upsampler with the same chroma siting as
>> used by the scaler for consistency, the chroma upsampler is used
>> instead of the scaler for YUV 4:2:0 on ICL's HDR planes.
>>
>> Signed-off-by: Maarten Lankhorst <[email protected]>
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h     | 22 ++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_sprite.c | 22 ++++++++++++++++++++++
>>  2 files changed, 44 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h 
>> b/drivers/gpu/drm/i915/i915_reg.h
>> index 1b59d15aaf59..b614a06b66c4 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -6560,6 +6560,19 @@ enum {
>>  #define _PLANE_AUX_DIST_2_A                 0x702c0
>>  #define _PLANE_AUX_OFFSET_1_A                       0x701c4
>>  #define _PLANE_AUX_OFFSET_2_A                       0x702c4
>> +#define _PLANE_CUS_CTL_1_A                  0x701c8
>> +#define _PLANE_CUS_CTL_2_A                  0x702c8
>> +#define  PLANE_CUS_ENABLE                   (1 << 31)
>> +#define  PLANE_CUS_PLANE_6                  (0 << 30)
>> +#define  PLANE_CUS_PLANE_7                  (1 << 30)
>> +#define  PLANE_CUS_HPHASE_SIGN_NEGATIVE             (1 << 19)
>> +#define  PLANE_CUS_HPHASE_0                 (0 << 16)
>> +#define  PLANE_CUS_HPHASE_0_25                      (1 << 16)
>> +#define  PLANE_CUS_HPHASE_0_5                       (2 << 16)
>> +#define  PLANE_CUS_VPHASE_SIGN_NEGATIVE             (1 << 15)
>> +#define  PLANE_CUS_VPHASE_0                 (0 << 12)
>> +#define  PLANE_CUS_VPHASE_0_25                      (1 << 12)
>> +#define  PLANE_CUS_VPHASE_0_5                       (2 << 12)
>>  #define _PLANE_COLOR_CTL_1_A                        0x701CC /* GLK+ */
>>  #define _PLANE_COLOR_CTL_2_A                        0x702CC /* GLK+ */
>>  #define _PLANE_COLOR_CTL_3_A                        0x703CC /* GLK+ */
>> @@ -6697,6 +6710,15 @@ enum {
>>  #define PLANE_AUX_OFFSET(pipe, plane)   \
>>      _MMIO_PLANE(plane, _PLANE_AUX_OFFSET_1(pipe), _PLANE_AUX_OFFSET_2(pipe))
>>  
>> +#define _PLANE_CUS_CTL_1_B          0x711c8
>> +#define _PLANE_CUS_CTL_2_B          0x712c8
>> +#define _PLANE_CUS_CTL_1(pipe)       \
>> +            _PIPE(pipe, _PLANE_CUS_CTL_1_A, _PLANE_CUS_CTL_1_B)
>> +#define _PLANE_CUS_CTL_2(pipe)       \
>> +            _PIPE(pipe, _PLANE_CUS_CTL_2_A, _PLANE_CUS_CTL_2_B)
>> +#define PLANE_CUS_CTL(pipe, plane)   \
>> +    _MMIO_PLANE(plane, _PLANE_CUS_CTL_1(pipe), _PLANE_CUS_CTL_2(pipe))
>> +
>>  #define _PLANE_COLOR_CTL_1_B                        0x711CC
>>  #define _PLANE_COLOR_CTL_2_B                        0x712CC
>>  #define _PLANE_COLOR_CTL_3_B                        0x713CC
>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
>> b/drivers/gpu/drm/i915/intel_sprite.c
>> index 111d72a5d5a0..c4e05b0b60bf 100644
>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>> @@ -358,6 +358,7 @@ skl_update_plane(struct intel_plane *plane,
>>      uint32_t y = plane_state->color_plane[0].y;
>>      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;
>>      unsigned long irqflags;
>>  
>>      /* Sizes are 0 based */
>> @@ -385,6 +386,27 @@ skl_update_plane(struct intel_plane *plane,
>>                    (plane_state->color_plane[1].y << 16) |
>>                    plane_state->color_plane[1].x);
>>  
>> +    if (icl_is_hdr_plane(plane)) {
>> +            u32 cus_ctl = 0;
>> +
>> +            if (linked) {
>> +                    /* Enable and use MPEG-2 chroma siting */
>> +                    cus_ctl = PLANE_CUS_ENABLE |
>> +                            PLANE_CUS_HPHASE_0 |
>> +                            PLANE_CUS_VPHASE_SIGN_NEGATIVE |
>> +                            PLANE_CUS_VPHASE_0_25;
> Or vphase=-0.5 and hphase=-0.25 maybe? That would be consistent
> with the phase we program into the normal scaler. But maybe the
> cus considers works differently?

Just following what the documentation says in PLANE_CUS_CTL..

YUV 420 Chroma Siting
Left (MPEG-2): Hphase: 0, Vphase: -0.25

>> +
>> +                    if (linked->id == PLANE_SPRITE5)
>> +                            cus_ctl |= PLANE_CUS_PLANE_7;
>> +                    else if (linked->id == PLANE_SPRITE4)
>> +                            cus_ctl |= PLANE_CUS_PLANE_6;
> PLANE_CUS_SOMETHING(plane) ?
Considered it, but the field is a bool. So it doesn't work like that.

I rather explicitly add the MISSING_CASE() and not hide the fact you can't use
an arbitrary plane in there.
>> +                    else
>> +                            MISSING_CASE(linked->id);
>> +            }
>> +
>> +            I915_WRITE_FW(PLANE_CUS_CTL(pipe, plane_id), cus_ctl);
>> +    }
>> +
>>      /* program plane scaler */
>>      if (plane_state->scaler_id >= 0) {
>>              skl_program_scaler(dev_priv, plane, crtc_state, plane_state);
>> -- 
>> 2.18.0
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> [email protected]
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx


_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to