Re: [PATCH 4/4] drm/tidss: Fix sampling edge configuration

[Swamil Jain <s-jain1@ti.com>]

On 8/11/25 15:26, Louis Chauvet wrote:
> 
> 
> Le 08/08/2025 à 18:26, Swamil Jain a écrit :
>>
>>
>> On 8/8/25 19:16, devarsh wrote:
>>> Hi Louis,
>>>
>>> Thanks for the patch.
>>>
>>> On 30/07/25 22:32, Louis Chauvet wrote:
>>>> As stated in the AM62x Technical Reference Manual (SPRUIV7B), the data
>>>> sampling edge needs to be configured in two distinct registers: one 
>>>> in the
>>>> TIDSS IP and another in the memory-mapped control register modules.
>>>
>>> I don't think AM62x is thee only one which requires this and on the
>>> contrary not all SoCs require this extra setting. We had been waiting on
>>> confirmations from hardware team and very recently they gave a list of
>>> SoCs which require this, as per that I think we need to limit this to
>>> AM62x and AM62A per current supported SoCs.
>>>
>>> Swamil,
>>> Please confirm on this and share if any additional details required 
>>> here.
>>>
>>
>> Yeah Devarsh, as you mentioned, this is valid for AM62X, AM62A and
>> AM62P. We would have upstreamed this feature, but there are some
>> corrections in Technical Reference Manual for these SoCs regarding
>> programming CTRL_MMR_DPI_CLK_CTRL register fields, we are in loop with
>> H/W team, waiting for their official confirmation regarding this issue.
>>
>> Thanks Louis for working on this patch, but we should wait for H/W
>> team's confirmation.
> 
> Hello all,
> 
> Thanks for the feedback. I was not aware of this current work.
> Do you plan to send the fix yourself? Should I wait your HW team 
> feedback and send a v2?
> 
Hi Louis, H/W team confirmed that, CTRL_MMR_DPI0_CLK_CTRL.bit[8] should 
be programmed same as DSS_VP1_POL_FREQ.bit[14](IPC) and 
CTRL_MMR_DPI0_CLK_CTRL.bit[9] should be programmed same as 
DSS_VP1_POL_FREQ.bit[16](RF). Please continue with you patches.

> I also have a very similar patch ready for u-boot (depending on the same 
> DT modifications), do you plan to fix u-boot too?
> 
Please fix u-boot also.

Thanks and regards,
Swamil.

> Thanks,
> Louis Chauvet
> 
> 
>> Regards,
>> Swamil.
>>
>>> Regards
>>> Devarsh
>>>
>>>    Since
>>>> the latter is not within the same address range, a phandle to a syscon
>>>> device is used to access the regmap.
>>>>
>>>> Fixes: 32a1795f57ee ("drm/tidss: New driver for TI Keystone platform 
>>>> Display SubSystem")
>>>> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
>>>>
>>>> ---
>>>>
>>>> Cc: stable@vger.kernel.org
>>>> ---
>>>>    drivers/gpu/drm/tidss/tidss_dispc.c | 14 ++++++++++++++
>>>>    1 file changed, 14 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c 
>>>> b/drivers/gpu/drm/tidss/tidss_dispc.c
>>>> index 
>>>> c0277fa36425ee1f966dccecf2b69a2d01794899..65ca7629a2e75437023bf58f8a1bddc24db5e3da 100644
>>>> --- a/drivers/gpu/drm/tidss/tidss_dispc.c
>>>> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c
>>>> @@ -498,6 +498,7 @@ struct dispc_device {
>>>>        const struct dispc_features *feat;
>>>>        struct clk *fclk;
>>>> +    struct regmap *clk_ctrl;
>>>>        bool is_enabled;
>>>> @@ -1267,6 +1268,11 @@ void dispc_vp_enable(struct dispc_device 
>>>> *dispc, u32 hw_videoport,
>>>>                   FLD_VAL(mode->vdisplay - 1, 27, 16));
>>>>        VP_REG_FLD_MOD(dispc, hw_videoport, DISPC_VP_CONTROL, 1, 0, 0);
>>>> +
>>>> +    if (dispc->clk_ctrl) {
>>>> +        regmap_update_bits(dispc->clk_ctrl, 0, 0x100, ipc ? 0x100 : 
>>>> 0x000);
>>>> +        regmap_update_bits(dispc->clk_ctrl, 0, 0x200, rf ? 0x200 : 
>>>> 0x000);
>>>> +    }
>>>>    }
>>>>    void dispc_vp_disable(struct dispc_device *dispc, u32 hw_videoport)
>>>> @@ -3012,6 +3018,14 @@ int dispc_init(struct tidss_device *tidss)
>>>>        dispc_init_errata(dispc);
>>>> +    dispc->clk_ctrl = 
>>>> syscon_regmap_lookup_by_phandle_optional(tidss->dev->of_node,
>>>> +                                   "ti,clk-ctrl");
>>>> +    if (IS_ERR(dispc->clk_ctrl)) {
>>>> +        r = dev_err_probe(dispc->dev, PTR_ERR(dispc->clk_ctrl),
>>>> +                  "DISPC: syscon_regmap_lookup_by_phandle failed.\n");
>>>> +        return r;
>>>> +    }
>>>> +
>>>>        dispc->fourccs = devm_kcalloc(dev, 
>>>> ARRAY_SIZE(dispc_color_formats),
>>>>                          sizeof(*dispc->fourccs), GFP_KERNEL);
>>>>        if (!dispc->fourccs)
>>>>
>>>
>