Message-ID: <02920114-b86b-47ad-945d-1905b9117518@ti.com>
Date: Tue, 12 Aug 2025 23:40:35 +0530
From: Swamil Jain <s-jain1@...com>
To: Louis Chauvet <louis.chauvet@...tlin.com>, devarsh <devarsht@...com>,
        "Jyri Sarha" <jyri.sarha@....fi>,
        Tomi Valkeinen
	<tomi.valkeinen@...asonboard.com>,
        Maarten Lankhorst
	<maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <mripard@...nel.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
        Rob Herring <robh@...nel.org>,
        Krzysztof
 Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>, Sam
 Ravnborg <sam@...nborg.org>,
        Benoit Parrot <bparrot@...com>, Lee Jones
	<lee@...nel.org>,
        Nishanth Menon <nm@...com>, Vignesh Raghavendra
	<vigneshr@...com>,
        Tero Kristo <kristo@...nel.org>
CC: <thomas.petazzoni@...tlin.com>, Jyri Sarha <jsarha@...com>,
        Tomi Valkeinen
	<tomi.valkeinen@...com>,
        <dri-devel@...ts.freedesktop.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>,
        <stable@...r.kernel.org>
Subject: Re: [PATCH 4/4] drm/tidss: Fix sampling edge configuration



On 8/12/25 23:32, Swamil Jain wrote:
> 
> 
> 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.
> 
Please go ahead and send v2.
We are working with the documentation team to get the Technical 
Reference Manual updated in parallel.

Regards,
Swamil.

>> 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@...tlin.com>
>>>>>
>>>>> ---
>>>>>
>>>>> Cc: stable@...r.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)
>>>>>
>>>>
>>
> 

Powered by blists - more mailing lists