Re: [PATCH 2/4] drm/rcar-du: dsi: Remove fixed PPI lane count setup

[Marek Vasut <marek.vasut@mailbox.org>]

On 8/13/25 9:34 AM, Tomi Valkeinen wrote:

Hi,

>>>>> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h b/
>>>>> drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h
>>>>> index b3e57217ae63..cefa7e92b5b8 100644
>>>>> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h
>>>>> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h
>>>>> @@ -80,10 +80,7 @@
>>>>>     * PHY-Protocol Interface (PPI) Registers
>>>>>     */
>>>>>    #define PPISETR                0x700
>>>>> -#define PPISETR_DLEN_0            (0x1 << 0)
>>>>> -#define PPISETR_DLEN_1            (0x3 << 0)
>>>>> -#define PPISETR_DLEN_2            (0x7 << 0)
>>>>> -#define PPISETR_DLEN_3            (0xf << 0)
>>>>> +#define PPISETR_DLEN_MASK        (0xf << 0)
>>>>>    #define PPISETR_CLEN            BIT(8)
>>>>
>>>> Looks fine, but do you know what the TXSETR register does? It also has
>>>> LANECNT, but I don't see the driver touching that register at all.
>>>> TXSETR:LANECNT default value is 3 (4 lanes), which matches with the old
>>>> hardcoded behavior for PPISETR... So I wonder if that register should
>>>> also be set?
>>>
>>> Ah, never mind, I now saw the patch 3 =). But should it be before patch
>>> 2? Hmm, I guess that ordering is no better. Should they be combined into
>>> "support 1,2,3 datalanes" patch?
>> I think each patch fixes slighly different issue, even if the issues are
>> related. I tried to keep the issue description in each patch commit
>> message for posterity. I can squash them if you think that's better, I
>> don't mind either way.
> 
> I was thinking about this the user's or backporting point of view.
> Neither of the commits (clearly) say that they add support for 1/2/3
> lane modes.

The 1/2/3 lane mode was already implemented in the driver, except it was 
broken.

> You say they "fix", but they're not quite fixes either. The
> patch 3 could be considered a fix, but at the moment it just writes the
> default value to the register, so no point in marking it as a fix to be
> backported.

3/4 does write the DSI lane count into TXSETR , not the default value.

> So... I don't have a strong opinion, but I think a single patch that
> adds support to 1, 2,3 lanes makes most sense.

Lemme send a single patch with Fixes tag then. The combined patch does 
not look too great though.