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

[Louis Chauvet <louis.chauvet@bootlin.com>]

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?

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?

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)
>>>
>>

-- 
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com