Hi Jayesh, Thanks for the patches! On 13-Jun-24 14:08, Jayesh Choudhary wrote: > Currently, mode_valid is defined only in drm_connector_helper_funcs. > When the bridge is attached with the 'DRM_BRIDGE_ATTACH_NO_CONNECTOR' > flag, the connector is not initialized, and so is the mode_valid > hook under connector helper funcs. > It also returns MODE_OK for all modes without actually checking the > modes. > So move the mode_valid hook to drm_bridge_funcs with proper clock > checks for maximum and minimum pixel clock supported by the bridge. > > Signed-off-by: Jayesh Choudhary <j-choudhary@xxxxxx> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> > Acked-by: Sui Jingfeng <sui.jingfeng@xxxxxxxxx> > --- > drivers/gpu/drm/bridge/sii902x.c | 32 +++++++++++++++++++++++--------- > 1 file changed, 23 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c > index 2fbeda9025bf..6a6055a4ccf9 100644 > --- a/drivers/gpu/drm/bridge/sii902x.c > +++ b/drivers/gpu/drm/bridge/sii902x.c > @@ -163,6 +163,14 @@ > > #define SII902X_AUDIO_PORT_INDEX 3 > > +/* > + * The maximum resolution supported by the HDMI bridge is 1080p@60Hz > + * and 1920x1200 requiring a pixel clock of 165MHz and the minimum > + * resolution supported is 480p@60Hz requiring a pixel clock of 25MHz > + */ > +#define SII902X_MIN_PIXEL_CLOCK_KHZ 25000 > +#define SII902X_MAX_PIXEL_CLOCK_KHZ 165000 > + > struct sii902x { > struct i2c_client *i2c; > struct regmap *regmap; > @@ -310,17 +318,8 @@ static int sii902x_get_modes(struct drm_connector *connector) > return num; > } > > -static enum drm_mode_status sii902x_mode_valid(struct drm_connector *connector, > - struct drm_display_mode *mode) > -{ > - /* TODO: check mode */ > - > - return MODE_OK; > -} > - > static const struct drm_connector_helper_funcs sii902x_connector_helper_funcs = { > .get_modes = sii902x_get_modes, > - .mode_valid = sii902x_mode_valid, > }; > > static void sii902x_bridge_disable(struct drm_bridge *bridge) > @@ -504,6 +503,20 @@ static int sii902x_bridge_atomic_check(struct drm_bridge *bridge, > return 0; > } > > +static enum drm_mode_status > +sii902x_bridge_mode_valid(struct drm_bridge *bridge, > + const struct drm_display_info *info, > + const struct drm_display_mode *mode) > +{ > + if (mode->clock < SII902X_MIN_PIXEL_CLOCK_KHZ) > + return MODE_CLOCK_LOW; > + > + if (mode->clock > SII902X_MAX_PIXEL_CLOCK_KHZ) > + return MODE_CLOCK_HIGH; > + > + return MODE_OK; > +} > + > static const struct drm_bridge_funcs sii902x_bridge_funcs = { > .attach = sii902x_bridge_attach, > .mode_set = sii902x_bridge_mode_set, > @@ -516,6 +529,7 @@ static const struct drm_bridge_funcs sii902x_bridge_funcs = { > .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, > .atomic_get_input_bus_fmts = sii902x_bridge_atomic_get_input_bus_fmts, > .atomic_check = sii902x_bridge_atomic_check, > + .mode_valid = sii902x_bridge_mode_valid, > }; > > static int sii902x_mute(struct sii902x *sii902x, bool mute) Reviewed-by: Aradhya Bhatia <a-bhatia1@xxxxxx> -- Regards Aradhya