Re: [PATCH 4/4] drm: xlnx: zynqmp_dpsub: Set live video in format

[Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>]

On 19/01/2024 07:54, Klymenko, Anatoliy wrote:

>>> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h
>>> b/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h
>>> index f92a006d5070..926e07c255bb 100644
>>> --- a/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h
>>> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h
>>> @@ -165,10 +165,10 @@
>>>    #define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_10               0x2
>>>    #define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_12               0x3
>>>    #define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_MASK
>> GENMASK(2, 0)
>>> -#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB               0x0
>>> -#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV444    0x1
>>> -#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV422    0x2
>>> -#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YONLY     0x3
>>> +#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB               0x00
>>> +#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV444    0x10
>>> +#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV422    0x20
>>> +#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YONLY     0x30
>>
>> What's this about? Were these wrong before? Sounds like a separate patch
>> needed for these.
>>
> 
> It is an embedded bit shift that corresponds to DPSUB live video / gfx format register layout. Original values are technically correct but would require extra bit shifts to operate with. The current patch is the first instance of actual use of those defines. Do you think it's worth to factor those changes out into a separate patch?

The value for the defines should then be something like (0x3 << 4), to 
make it clearer that it's shifted to the right position.

  Tomi