| Message ID | 20240524073305.107293-2-j-choudhary@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add mode_valid hook for sii902x bridge | expand |
Hi Jayesh, On Fri, May 24, 2024 at 01:03:04PM +0530, Jayesh Choudhary wrote: > Currently, mode_valid hook returns all mode as valid and it is > defined only in drm_connector_helper_funcs. With the introduction of > 'DRM_BRIDGE_ATTACH_NO_CONNECTOR', connector is not initialized in > bridge_attach call for cases when the encoder has this flag enabled. > So add the mode_valid hook in drm_bridge_funcs as well with proper > clock checks for maximum and minimum pixel clock supported by the > bridge. > > Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com> > --- > drivers/gpu/drm/bridge/sii902x.c | 38 ++++++++++++++++++++++++++++++-- > 1 file changed, 36 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c > index 2fbeda9025bf..ef7c3ab3386c 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,12 +318,26 @@ static int sii902x_get_modes(struct drm_connector *connector) > return num; > } > > +static enum > +drm_mode_status sii902x_validate(struct sii902x *sii902x, > + 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 enum drm_mode_status sii902x_mode_valid(struct drm_connector *connector, > struct drm_display_mode *mode) > { > - /* TODO: check mode */ > + struct sii902x *sii902x = connector_to_sii902x(connector); > + const struct drm_display_mode *mod = mode; > > - return MODE_OK; > + return sii902x_validate(sii902x, mod); > } > > static const struct drm_connector_helper_funcs sii902x_connector_helper_funcs = { > @@ -499,11 +521,22 @@ static int sii902x_bridge_atomic_check(struct drm_bridge *bridge, > * There might be flags negotiation supported in future but > * set the bus flags in atomic_check statically for now. > */ > + > bridge_state->input_bus_cfg.flags = bridge->timings->input_bus_flags; If you spin a v2 then drop the above change as you delete the line again in the next patch. Sam
Hello Sam, On 24/05/24 13:48, Sam Ravnborg wrote: > Hi Jayesh, > > On Fri, May 24, 2024 at 01:03:04PM +0530, Jayesh Choudhary wrote: >> Currently, mode_valid hook returns all mode as valid and it is >> defined only in drm_connector_helper_funcs. With the introduction of >> 'DRM_BRIDGE_ATTACH_NO_CONNECTOR', connector is not initialized in >> bridge_attach call for cases when the encoder has this flag enabled. >> So add the mode_valid hook in drm_bridge_funcs as well with proper >> clock checks for maximum and minimum pixel clock supported by the >> bridge. >> >> Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com> >> --- >> drivers/gpu/drm/bridge/sii902x.c | 38 ++++++++++++++++++++++++++++++-- >> 1 file changed, 36 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c >> index 2fbeda9025bf..ef7c3ab3386c 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,12 +318,26 @@ static int sii902x_get_modes(struct drm_connector *connector) >> return num; >> } >> >> +static enum >> +drm_mode_status sii902x_validate(struct sii902x *sii902x, >> + 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 enum drm_mode_status sii902x_mode_valid(struct drm_connector *connector, >> struct drm_display_mode *mode) >> { >> - /* TODO: check mode */ >> + struct sii902x *sii902x = connector_to_sii902x(connector); >> + const struct drm_display_mode *mod = mode; >> >> - return MODE_OK; >> + return sii902x_validate(sii902x, mod); >> } >> >> static const struct drm_connector_helper_funcs sii902x_connector_helper_funcs = { >> @@ -499,11 +521,22 @@ static int sii902x_bridge_atomic_check(struct drm_bridge *bridge, >> * There might be flags negotiation supported in future but >> * set the bus flags in atomic_check statically for now. >> */ >> + >> bridge_state->input_bus_cfg.flags = bridge->timings->input_bus_flags; > > If you spin a v2 then drop the above change as you delete the line again > in the next patch. > > Sam While splitting the patches, I must have messed up. I will quickly send v3 with this fixed. Thanks, Jayesh
On Fri, May 24, 2024 at 01:03:04PM +0530, Jayesh Choudhary wrote: > Currently, mode_valid hook returns all mode as valid and it is > defined only in drm_connector_helper_funcs. With the introduction of > 'DRM_BRIDGE_ATTACH_NO_CONNECTOR', connector is not initialized in > bridge_attach call for cases when the encoder has this flag enabled. > So add the mode_valid hook in drm_bridge_funcs as well with proper > clock checks for maximum and minimum pixel clock supported by the > bridge. > > Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com> > --- > drivers/gpu/drm/bridge/sii902x.c | 38 ++++++++++++++++++++++++++++++-- > 1 file changed, 36 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c > index 2fbeda9025bf..ef7c3ab3386c 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,12 +318,26 @@ static int sii902x_get_modes(struct drm_connector *connector) > return num; > } > > +static enum > +drm_mode_status sii902x_validate(struct sii902x *sii902x, > + 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 enum drm_mode_status sii902x_mode_valid(struct drm_connector *connector, > struct drm_display_mode *mode) > { > - /* TODO: check mode */ > + struct sii902x *sii902x = connector_to_sii902x(connector); > + const struct drm_display_mode *mod = mode; > > - return MODE_OK; > + return sii902x_validate(sii902x, mod); If you have a mode_valid for the bridge, I don't think you need to perform the same check for the connector. > } > > static const struct drm_connector_helper_funcs sii902x_connector_helper_funcs = { > @@ -499,11 +521,22 @@ static int sii902x_bridge_atomic_check(struct drm_bridge *bridge, > * There might be flags negotiation supported in future but > * set the bus flags in atomic_check statically for now. > */ > + > bridge_state->input_bus_cfg.flags = bridge->timings->input_bus_flags; > > 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) > +{ > + struct sii902x *sii902x = bridge_to_sii902x(bridge); > + > + return sii902x_validate(sii902x, mode); > +} > + > static const struct drm_bridge_funcs sii902x_bridge_funcs = { > .attach = sii902x_bridge_attach, > .mode_set = sii902x_bridge_mode_set, > @@ -516,6 +549,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) > -- > 2.25.1 >
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c index 2fbeda9025bf..ef7c3ab3386c 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,12 +318,26 @@ static int sii902x_get_modes(struct drm_connector *connector) return num; } +static enum +drm_mode_status sii902x_validate(struct sii902x *sii902x, + 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 enum drm_mode_status sii902x_mode_valid(struct drm_connector *connector, struct drm_display_mode *mode) { - /* TODO: check mode */ + struct sii902x *sii902x = connector_to_sii902x(connector); + const struct drm_display_mode *mod = mode; - return MODE_OK; + return sii902x_validate(sii902x, mod); } static const struct drm_connector_helper_funcs sii902x_connector_helper_funcs = { @@ -499,11 +521,22 @@ static int sii902x_bridge_atomic_check(struct drm_bridge *bridge, * There might be flags negotiation supported in future but * set the bus flags in atomic_check statically for now. */ + bridge_state->input_bus_cfg.flags = bridge->timings->input_bus_flags; 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) +{ + struct sii902x *sii902x = bridge_to_sii902x(bridge); + + return sii902x_validate(sii902x, mode); +} + static const struct drm_bridge_funcs sii902x_bridge_funcs = { .attach = sii902x_bridge_attach, .mode_set = sii902x_bridge_mode_set, @@ -516,6 +549,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)
Currently, mode_valid hook returns all mode as valid and it is defined only in drm_connector_helper_funcs. With the introduction of 'DRM_BRIDGE_ATTACH_NO_CONNECTOR', connector is not initialized in bridge_attach call for cases when the encoder has this flag enabled. So add the mode_valid hook in drm_bridge_funcs as well with proper clock checks for maximum and minimum pixel clock supported by the bridge. Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com> --- drivers/gpu/drm/bridge/sii902x.c | 38 ++++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-)