Hi, On 17/09/2025 16:32, Swamil Jain wrote: > Hi Tomi, > > On 9/16/25 17:10, Tomi Valkeinen wrote: >> Hi, >> >> On 11/09/2025 14:07, Swamil Jain wrote: >>> From: Jayesh Choudhary <[email protected]> >>> >>> TIDSS should know which VP has OLDI output to avoid calling clock >>> functions for that VP as those are controlled by oldi driver. Add a >>> property "is_ext_vp_clk" to "tidss_device" structure for that. Mark it >>> 'true' in tidss_oldi_init() and 'false' in tidss_oldi_deinit(). >>> >>> Fixes: 7246e0929945 ("drm/tidss: Add OLDI bridge support") >> >> What bug does this fix? It's just adding a new field which it sets to >> true/false... > > Please take a look: https://lore.kernel.org/all/a0489fea-8c06-4c89- > [email protected]/
There isn't a mention of any issue or fix in the intro letter nor the patch descriptions, so a fixes tag looks very odd here. Usually a Fixes tag is for a patch that does the fix. And, of course, explains what the issue is and what the fix is. If I understand this right, the fix is this from the patch 2: + if (dispc->tidss->is_ext_vp_clk[hw_videoport]) + return 0; And patch 3? Those should be probably made into a single patch that fixes the issue. Also, bridges have mode_valid callback. Would that be better in patch 3? Tomi > Should we remove the tag? > Or, else, please suggest a better way to describe the issue mentioned in > above link. > > Regards, > Swamil >> >> Tomi >> >>> Tested-by: Michael Walle <[email protected]> >>> Reviewed-by: Devarsh Thakkar <[email protected]> >>> Signed-off-by: Jayesh Choudhary <[email protected]> >>> Signed-off-by: Swamil Jain <[email protected]> >>> --- >>> drivers/gpu/drm/tidss/tidss_drv.h | 2 ++ >>> drivers/gpu/drm/tidss/tidss_oldi.c | 2 ++ >>> 2 files changed, 4 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/tidss/tidss_drv.h b/drivers/gpu/drm/ >>> tidss/tidss_drv.h >>> index 84454a4855d1..e1c1f41d8b4b 100644 >>> --- a/drivers/gpu/drm/tidss/tidss_drv.h >>> +++ b/drivers/gpu/drm/tidss/tidss_drv.h >>> @@ -24,6 +24,8 @@ struct tidss_device { >>> const struct dispc_features *feat; >>> struct dispc_device *dispc; >>> + bool is_ext_vp_clk[TIDSS_MAX_PORTS]; >>> + >>> unsigned int num_crtcs; >>> struct drm_crtc *crtcs[TIDSS_MAX_PORTS]; >>> diff --git a/drivers/gpu/drm/tidss/tidss_oldi.c b/drivers/gpu/drm/ >>> tidss/tidss_oldi.c >>> index 7688251beba2..7ecbb2c3d0a2 100644 >>> --- a/drivers/gpu/drm/tidss/tidss_oldi.c >>> +++ b/drivers/gpu/drm/tidss/tidss_oldi.c >>> @@ -430,6 +430,7 @@ void tidss_oldi_deinit(struct tidss_device *tidss) >>> for (int i = 0; i < tidss->num_oldis; i++) { >>> if (tidss->oldis[i]) { >>> drm_bridge_remove(&tidss->oldis[i]->bridge); >>> + tidss->is_ext_vp_clk[tidss->oldis[i]->parent_vp] = false; >>> tidss->oldis[i] = NULL; >>> } >>> } >>> @@ -580,6 +581,7 @@ int tidss_oldi_init(struct tidss_device *tidss) >>> oldi->bridge.timings = &default_tidss_oldi_timings; >>> tidss->oldis[tidss->num_oldis++] = oldi; >>> + tidss->is_ext_vp_clk[oldi->parent_vp] = true; >>> oldi->tidss = tidss; >>> drm_bridge_add(&oldi->bridge); >> >
