[PATCH v3 0/3] Decouple max_pclk check from constant display feats

Jayesh Choudhary posted 3 patches 3 months, 1 week ago
There is a newer version of this series
drivers/gpu/drm/tidss/tidss_dispc.c | 77 +++++++++++------------------
drivers/gpu/drm/tidss/tidss_dispc.h |  1 -
drivers/gpu/drm/tidss/tidss_drv.h   |  4 ++
drivers/gpu/drm/tidss/tidss_oldi.c  | 26 ++++++++++
4 files changed, 59 insertions(+), 49 deletions(-)
[PATCH v3 0/3] Decouple max_pclk check from constant display feats
Posted by Jayesh Choudhary 3 months, 1 week ago
In an effort to make the existing compatibles more usable, we are
removing the max_pclk_khz form dispc_features structure and doing the
correspondig checks using "curr_max_pclk[]".

Changes are fully backwards compatible.

After integration of OLDI support[0], we need additional patches in
oldi to identify the VP that has OLDI. We have to do this since
OLDI driver owns the VP clock (its serial clock) and we cannot perform
clock operations on those VP clock from tidss driver. This issue was
also reported upstream when DSI fixes[1] had some clock related calls
in tidss driver. When "clk_round_rate()" is called, ideally it should
have gone to "sci_clk_determine_rate()" to query DM but it doesn't since
clock is owned by OLDI not tidss.

So add a member is_oldi_vp[] in tidss_device structure to identify this
and avoid performing clock operations for VP if it has OLDI panel.
For the same checks in OLDI driver, atomic_check() hook is added to its
bridge_funcs.
In the atomic_check() chain, first the bridge_atomic_check() is called
and then crtc_atomic_check() is called. So mode clock is first checked
in oldi driver and then skipped in tidss driver.

Had the tidss_oldi structure been exposed to tidss_dispc.c, we could
have directly checked VP type in dispc but since the structure is defined
in tidss_oldi.c , we have to add additional member to tidss_device
structure.

[0]: https://lore.kernel.org/all/20250528122544.817829-1-aradhya.bhatia@linux.dev/
[1]: https://lore.kernel.org/all/DA6TT575Z82D.3MPK8HG5GRL8U@kernel.org/

Changelog v2->v3:
- Add changes for OLDI
- Rename max_pclk as it is misleading
- Change commit message to make it more appropriate
- Drop unnecessary zero initialization

v2 patch link:
https://lore.kernel.org/all/20250618100509.20386-1-j-choudhary@ti.com/

Changelog v1->v2:
- Rebase it on linux-next after OLDI support series as all of its
  patches are reviewed and tested and it touches one of the functions
  used.
  
v1 patch link:
https://lore.kernel.org/all/20250618075804.139844-1-j-choudhary@ti.com/

Jayesh Choudhary (3):
  drm/tidss: oldi: Add property to identify OLDI supported VP
  drm/tidss: Remove max_pclk_khz from tidss display features
  drm/tidss: oldi: Add atomic_check hook for oldi bridge

 drivers/gpu/drm/tidss/tidss_dispc.c | 77 +++++++++++------------------
 drivers/gpu/drm/tidss/tidss_dispc.h |  1 -
 drivers/gpu/drm/tidss/tidss_drv.h   |  4 ++
 drivers/gpu/drm/tidss/tidss_oldi.c  | 26 ++++++++++
 4 files changed, 59 insertions(+), 49 deletions(-)

-- 
2.34.1
Re: [PATCH v3 0/3] Decouple max_pclk check from constant display feats
Posted by Devarsh Thakkar 3 months, 1 week ago
On 01/07/25 15:25, Jayesh Choudhary wrote:
> In an effort to make the existing compatibles more usable, we are
> removing the max_pclk_khz form dispc_features structure and doing the
> correspondig checks using "curr_max_pclk[]".
> 
> Changes are fully backwards compatible.
> 
> After integration of OLDI support[0], we need additional patches in
> oldi to identify the VP that has OLDI. We have to do this since
> OLDI driver owns the VP clock (its serial clock) and we cannot perform
> clock operations on those VP clock from tidss driver. This issue was
> also reported upstream when DSI fixes[1] had some clock related calls
> in tidss driver. When "clk_round_rate()" is called, ideally it should
> have gone to "sci_clk_determine_rate()" to query DM but it doesn't since
> clock is owned by OLDI not tidss.
> 

As series is fixing above issue (abnormal behaviour while calling 
clk_round_rate from tidss for VP clock being used by OLDI), can we add 
"Fixes tag" for the patches?

Regards
Devarsh
Re: [PATCH v3 0/3] Decouple max_pclk check from constant display feats
Posted by Jayesh Choudhary 3 months, 1 week ago
Hello Devarsh,

On 01/07/25 19:00, Devarsh Thakkar wrote:
> On 01/07/25 15:25, Jayesh Choudhary wrote:
>> In an effort to make the existing compatibles more usable, we are
>> removing the max_pclk_khz form dispc_features structure and doing the
>> correspondig checks using "curr_max_pclk[]".
>>
>> Changes are fully backwards compatible.
>>
>> After integration of OLDI support[0], we need additional patches in
>> oldi to identify the VP that has OLDI. We have to do this since
>> OLDI driver owns the VP clock (its serial clock) and we cannot perform
>> clock operations on those VP clock from tidss driver. This issue was
>> also reported upstream when DSI fixes[1] had some clock related calls
>> in tidss driver. When "clk_round_rate()" is called, ideally it should
>> have gone to "sci_clk_determine_rate()" to query DM but it doesn't since
>> clock is owned by OLDI not tidss.
>>
> 
> As series is fixing above issue (abnormal behaviour while calling 
> clk_round_rate from tidss for VP clock being used by OLDI), can we add 
> "Fixes tag" for the patches?


This seems like a preemptive fix. So I was not sure what to add.

If it should be added then which commit?
7246e0929945 ("drm/tidss: Add OLDI bridge support") ?

Warm Regards,
Jayesh

> 
> Regards
> Devarsh
Re: [PATCH v3 0/3] Decouple max_pclk check from constant display feats
Posted by Devarsh Thakkar 3 months, 1 week ago
On 02/07/25 12:31, Jayesh Choudhary wrote:
> Hello Devarsh,
> 
> On 01/07/25 19:00, Devarsh Thakkar wrote:
>> On 01/07/25 15:25, Jayesh Choudhary wrote:
>>> In an effort to make the existing compatibles more usable, we are
>>> removing the max_pclk_khz form dispc_features structure and doing the
>>> correspondig checks using "curr_max_pclk[]".
>>>
>>> Changes are fully backwards compatible.
>>>
>>> After integration of OLDI support[0], we need additional patches in
>>> oldi to identify the VP that has OLDI. We have to do this since
>>> OLDI driver owns the VP clock (its serial clock) and we cannot perform
>>> clock operations on those VP clock from tidss driver. This issue was
>>> also reported upstream when DSI fixes[1] had some clock related calls
>>> in tidss driver. When "clk_round_rate()" is called, ideally it should
>>> have gone to "sci_clk_determine_rate()" to query DM but it doesn't since
>>> clock is owned by OLDI not tidss.
>>>
>>
>> As series is fixing above issue (abnormal behaviour while calling 
>> clk_round_rate from tidss for VP clock being used by OLDI), can we add 
>> "Fixes tag" for the patches?
> 
> 
> This seems like a preemptive fix. So I was not sure what to add.
> 
> If it should be added then which commit?
> 7246e0929945 ("drm/tidss: Add OLDI bridge support") ?
> 

Yes, this looks good. I think good to add it as a bug fix since 
otherwise it causes an abnormal behavior while setting up the VP clock 
from tidss.

Regards
Devarsh


> Warm Regards,
> Jayesh
> 
>>
>> Regards
>> Devarsh