Hi Neil, On 22-May-23 13:35, neil.armstrong@xxxxxxxxxx wrote: > On 17/05/2023 07:48, Aradhya Bhatia wrote: >> Hi Neil, >> >> On 16-May-23 12:54, Neil Armstrong wrote: >>> On 09/05/2023 11:30, Aradhya Bhatia wrote: >>>> From: Nikhil Devshatwar <nikhil.nd@xxxxxx> >>>> >>>> input_bus_flags are specified in drm_bridge_timings (legacy) as well >>>> as drm_bridge_state->input_bus_cfg.flags >>>> >>>> The flags from the timings will be deprecated. Bridges are supposed >>>> to validate and set the bridge state flags from atomic_check. >>>> >>>> Signed-off-by: Nikhil Devshatwar <nikhil.nd@xxxxxx> >>>> [a-bhatia1: replace timings in cdns_mhdp_platform_info by >>>> input_bus_flags] >>>> Signed-off-by: Aradhya Bhatia <a-bhatia1@xxxxxx> >>>> --- >>>> >>>> Notes: >>>> >>>> changes from v5: >>>> * removed the wrongly addded return statement in tfp410 driver. >>>> * replaced the timings field in cdns_mhdp_platform_info by >>>> input_bus_flags field, in order to get rid of bridge->timings >>>> altogether. >>>> >>>> drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 11 >>>> ++++++++--- >>>> drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h | 2 +- >>>> drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-j721e.c | 9 ++++----- >>>> drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-j721e.h | 2 +- >>>> 4 files changed, 14 insertions(+), 10 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c >>>> b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c >>>> index 623e4235c94f..a677b1267525 100644 >>>> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c >>>> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c >>>> @@ -2189,6 +2189,13 @@ static int cdns_mhdp_atomic_check(struct >>>> drm_bridge *bridge, >>>> return -EINVAL; >>>> } >>>> + /* >>>> + * There might be flags negotiation supported in future. >>>> + * Set the bus flags in atomic_check statically for now. >>>> + */ >>>> + if (mhdp->info) >>>> + bridge_state->input_bus_cfg.flags = >>>> *mhdp->info->input_bus_flags; >>>> + >>>> mutex_unlock(&mhdp->link_mutex); >>>> return 0; >>>> } >>>> @@ -2554,8 +2561,6 @@ static int cdns_mhdp_probe(struct >>>> platform_device *pdev) >>>> mhdp->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID | >>>> DRM_BRIDGE_OP_HPD; >>>> mhdp->bridge.type = DRM_MODE_CONNECTOR_DisplayPort; >>>> - if (mhdp->info) >>>> - mhdp->bridge.timings = mhdp->info->timings; >>> >>> Won't this cause a breakage because at this point in time >>> bridge.timings->input_bus_flags >>> seems to be still used by tidss right ? >>> >> >> tidss was using the bridge.timings->input_bus_flags before the 7th >> patch[1] in this series. >> >> After the patch, it only relies on bridge_state and display_info for bus >> flags and formats. > > OK thanks, then: > > Reviewed-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx> > > if you resend a new version, please add this info in the commit message. Thank you! Will do so. Regards Aradhya > > Neil > >> >> >> Regards >> Aradhya >> >> [1]: https://lore.kernel.org/all/20230509093036.3303-8-a-bhatia1@xxxxxx/ >> >> >>> >>>> ret = phy_init(mhdp->phy); >>>> if (ret) { >>>> @@ -2642,7 +2647,7 @@ static const struct of_device_id mhdp_ids[] = { >>>> #ifdef CONFIG_DRM_CDNS_MHDP8546_J721E >>>> { .compatible = "ti,j721e-mhdp8546", >>>> .data = &(const struct cdns_mhdp_platform_info) { >>>> - .timings = &mhdp_ti_j721e_bridge_timings, >>>> + .input_bus_flags = &mhdp_ti_j721e_bridge_input_bus_flags, >>>> .ops = &mhdp_ti_j721e_ops, >>>> }, >>>> }, >>>> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h >>>> b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h >>>> index bedddd510d17..bad2fc0c7306 100644 >>>> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h >>>> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h >>>> @@ -336,7 +336,7 @@ struct cdns_mhdp_bridge_state { >>>> }; >>>> struct cdns_mhdp_platform_info { >>>> - const struct drm_bridge_timings *timings; >>>> + const u32 *input_bus_flags; >>>> const struct mhdp_platform_ops *ops; >>>> }; >>>> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-j721e.c >>>> b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-j721e.c >>>> index dfe1b59514f7..12d04be4e242 100644 >>>> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-j721e.c >>>> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-j721e.c >>>> @@ -71,8 +71,7 @@ const struct mhdp_platform_ops mhdp_ti_j721e_ops = { >>>> .disable = cdns_mhdp_j721e_disable, >>>> }; >>>> -const struct drm_bridge_timings mhdp_ti_j721e_bridge_timings = { >>>> - .input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE | >>>> - DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE | >>>> - DRM_BUS_FLAG_DE_HIGH, >>>> -}; >>>> +const u32 >>>> +mhdp_ti_j721e_bridge_input_bus_flags = >>>> DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE | >>>> + DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE | >>>> + DRM_BUS_FLAG_DE_HIGH; >>>> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-j721e.h >>>> b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-j721e.h >>>> index 97d20d115a24..5ddca07a4255 100644 >>>> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-j721e.h >>>> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-j721e.h >>>> @@ -14,6 +14,6 @@ >>>> struct mhdp_platform_ops; >>>> extern const struct mhdp_platform_ops mhdp_ti_j721e_ops; >>>> -extern const struct drm_bridge_timings mhdp_ti_j721e_bridge_timings; >>>> +extern const u32 mhdp_ti_j721e_bridge_input_bus_flags; >>>> #endif /* !CDNS_MHDP8546_J721E_H */ >>> >