lkml.org 
[lkml]   [2024]   [Jan]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
SubjectRe: [PATCH v2 1/4] drm/mediatek: dsi: Use GENMASK() for register mask definitions
From
Il 26/12/23 11:46, Fei Shao ha scritto:
> Hi Angelo,
>
> On Wed, Dec 20, 2023 at 9:57 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Change magic numerical masks with usage of the GENMASK() macro
>> to improve readability.
>>
>> This commit brings no functional changes.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> ---
>> drivers/gpu/drm/mediatek/mtk_dsi.c | 46 ++++++++++++++++--------------
>> 1 file changed, 24 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
>> index a2fdfc8ddb15..23d2c5be8dbb 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
>> @@ -58,18 +58,18 @@
>>
>> #define DSI_TXRX_CTRL 0x18
>> #define VC_NUM BIT(1)
>> -#define LANE_NUM (0xf << 2)
>> +#define LANE_NUM GENMASK(5, 2)
>> #define DIS_EOT BIT(6)
>> #define NULL_EN BIT(7)
>> #define TE_FREERUN BIT(8)
>> #define EXT_TE_EN BIT(9)
>> #define EXT_TE_EDGE BIT(10)
>> -#define MAX_RTN_SIZE (0xf << 12)
>> +#define MAX_RTN_SIZE GENMASK(15, 12)
>> #define HSTX_CKLP_EN BIT(16)
>>
>> #define DSI_PSCTRL 0x1c
>> -#define DSI_PS_WC 0x3fff
>> -#define DSI_PS_SEL (3 << 16)
>> +#define DSI_PS_WC GENMASK(14, 0)
>> +#define DSI_PS_SEL GENMASK(19, 16)
>
> GENMASK(17, 16)

That was intended, and depends on the SoC - I should effectively advertise that
in the commit description and I will.

As per the datasheets:

MT8192 - DSI_PS_SEL[18:16]
MT8195 - DSI_PS_SEL[19:16]

..on SoCs that don't have those additional bits, the higher ones are reserved
(unused), so it is safe to have this as 19, 16.

>>
>> #define PACKED_PS_16BIT_RGB565 (0 << 16)
>> #define LOOSELY_PS_18BIT_RGB666 (1 << 16)
>> #define PACKED_PS_18BIT_RGB666 (2 << 16)
>> @@ -109,26 +109,27 @@
>> #define LD0_WAKEUP_EN BIT(2)
>>
>> #define DSI_PHY_TIMECON0 0x110
>> -#define LPX (0xff << 0)
>> -#define HS_PREP (0xff << 8)
>> -#define HS_ZERO (0xff << 16)
>> -#define HS_TRAIL (0xff << 24)
>> +#define LPX GENMASK(7, 0)
>> +#define HS_PREP GENMASK(15, 8)
>> +#define HS_ZERO GENMASK(23, 16)
>> +#define HS_TRAIL GENMASK(31, 24)
>>
>> #define DSI_PHY_TIMECON1 0x114
>> -#define TA_GO (0xff << 0)
>> -#define TA_SURE (0xff << 8)
>> -#define TA_GET (0xff << 16)
>> -#define DA_HS_EXIT (0xff << 24)
>> +#define TA_GO GENMASK(7, 0)
>> +#define TA_SURE GENMASK(15, 8)
>> +#define TA_GET GENMASK(23, 16)
>> +#define DA_HS_EXIT GENMASK(31, 24)
>>
>> #define DSI_PHY_TIMECON2 0x118
>> -#define CONT_DET (0xff << 0)
>> -#define CLK_ZERO (0xff << 16)
>> -#define CLK_TRAIL (0xff << 24)
>> +#define CONT_DET GENMASK(7, 0)
>> +#define DA_HS_SYNC GENMASK(15, 8)
>
> This is new, so please introduce it in a separate patch if intended.
>

I wouldn't really be comfortable doing so: this would mean that I'd
have to first write a constant and then fix that in a later patch...

P.S.: Sorry for working again on this one whole month after .... :-)

Regards,
Angelo

> The rest looks good to me.
>
> Regards,
> Fei
>
>
>>
>> +#define CLK_ZERO GENMASK(23, 16)
>> +#define CLK_TRAIL GENMASK(31, 24)
>>
>> #define DSI_PHY_TIMECON3 0x11c
>> -#define CLK_HS_PREP (0xff << 0)
>> -#define CLK_HS_POST (0xff << 8)
>> -#define CLK_HS_EXIT (0xff << 16)
>> +#define CLK_HS_PREP GENMASK(7, 0)
>> +#define CLK_HS_POST GENMASK(15, 8)
>> +#define CLK_HS_EXIT GENMASK(23, 16)
>>
>> #define DSI_VM_CMD_CON 0x130
>> #define VM_CMD_EN BIT(0)
>> @@ -138,13 +139,14 @@
>> #define FORCE_COMMIT BIT(0)
>> #define BYPASS_SHADOW BIT(1)
>>
>> -#define CONFIG (0xff << 0)
>> +/* CMDQ related bits */
>> +#define CONFIG GENMASK(7, 0)
>> #define SHORT_PACKET 0
>> #define LONG_PACKET 2
>> #define BTA BIT(2)
>> -#define DATA_ID (0xff << 8)
>> -#define DATA_0 (0xff << 16)
>> -#define DATA_1 (0xff << 24)
>> +#define DATA_ID GENMASK(15, 8)
>> +#define DATA_0 GENMASK(23, 16)
>> +#define DATA_1 GENMASK(31, 24)
>>
>> #define NS_TO_CYCLE(n, c) ((n) / (c) + (((n) % (c)) ? 1 : 0))
>>
>> --
>> 2.43.0
>>
>>

\
 
 \ /
  Last update: 2024-05-27 14:41    [W:0.045 / U:0.396 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site