* [PATCH v2 0/2] Add mode_valid hook for sii902x bridge @ 2024-05-24 7:33 Jayesh Choudhary 2024-05-24 7:33 ` [PATCH v2 1/2] drm/bridge: sii902x: Fix mode_valid hook Jayesh Choudhary 2024-05-24 7:33 ` [PATCH v2 2/2] drm/bridge: Add pixel clock check in atomic_check Jayesh Choudhary 0 siblings, 2 replies; 6+ messages in thread From: Jayesh Choudhary @ 2024-05-24 7:33 UTC (permalink / raw) To: linux-kernel, andrzej.hajda, neil.armstrong, rfoss, Laurent.pinchart, mripard, dri-devel, j-choudhary Cc: jonas, jernej.skrabec, maarten.lankhorst, tzimmermann, airlied, daniel, a-bhatia1 Currently, there are no check to validate the modes in the bridge. Add pixel clock check to validate the modes that the bridge can support. Also add the mode_valid hook in drm_bridge_funcs structure to take care of the case when the encoder attaches the bridge chain with the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag in which case, the connector is not initialized in the bridge's attach call. Also add this check to the atomic_check call as suggested by Maxime in v1 patch. Changelog v1->v2: - Add KHZ suffix in the macros to be more clear - Add the hook for drm_bridge_funcs as well - Add check in atomic_check dunction call (in a separate patch) v1 patch: <https://lore.kernel.org/all/20240408081435.216927-1-j-choudhary@ti.com/> Jayesh Choudhary (2): drm/bridge: sii902x: Fix mode_valid hook drm/bridge: Add pixel clock check in atomic_check drivers/gpu/drm/bridge/sii902x.c | 43 ++++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] drm/bridge: sii902x: Fix mode_valid hook 2024-05-24 7:33 [PATCH v2 0/2] Add mode_valid hook for sii902x bridge Jayesh Choudhary @ 2024-05-24 7:33 ` Jayesh Choudhary 2024-05-24 8:18 ` Sam Ravnborg 2024-05-24 9:38 ` Dmitry Baryshkov 2024-05-24 7:33 ` [PATCH v2 2/2] drm/bridge: Add pixel clock check in atomic_check Jayesh Choudhary 1 sibling, 2 replies; 6+ messages in thread From: Jayesh Choudhary @ 2024-05-24 7:33 UTC (permalink / raw) To: linux-kernel, andrzej.hajda, neil.armstrong, rfoss, Laurent.pinchart, mripard, dri-devel, j-choudhary Cc: jonas, jernej.skrabec, maarten.lankhorst, tzimmermann, airlied, daniel, a-bhatia1 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; 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] drm/bridge: sii902x: Fix mode_valid hook 2024-05-24 7:33 ` [PATCH v2 1/2] drm/bridge: sii902x: Fix mode_valid hook Jayesh Choudhary @ 2024-05-24 8:18 ` Sam Ravnborg 2024-05-24 9:21 ` Jayesh Choudhary 2024-05-24 9:38 ` Dmitry Baryshkov 1 sibling, 1 reply; 6+ messages in thread From: Sam Ravnborg @ 2024-05-24 8:18 UTC (permalink / raw) To: Jayesh Choudhary Cc: linux-kernel, andrzej.hajda, neil.armstrong, rfoss, Laurent.pinchart, mripard, dri-devel, jonas, jernej.skrabec, maarten.lankhorst, tzimmermann, airlied, daniel, a-bhatia1 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] drm/bridge: sii902x: Fix mode_valid hook 2024-05-24 8:18 ` Sam Ravnborg @ 2024-05-24 9:21 ` Jayesh Choudhary 0 siblings, 0 replies; 6+ messages in thread From: Jayesh Choudhary @ 2024-05-24 9:21 UTC (permalink / raw) To: Sam Ravnborg Cc: linux-kernel, andrzej.hajda, neil.armstrong, rfoss, Laurent.pinchart, mripard, dri-devel, jonas, jernej.skrabec, maarten.lankhorst, tzimmermann, airlied, daniel, a-bhatia1 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] drm/bridge: sii902x: Fix mode_valid hook 2024-05-24 7:33 ` [PATCH v2 1/2] drm/bridge: sii902x: Fix mode_valid hook Jayesh Choudhary 2024-05-24 8:18 ` Sam Ravnborg @ 2024-05-24 9:38 ` Dmitry Baryshkov 1 sibling, 0 replies; 6+ messages in thread From: Dmitry Baryshkov @ 2024-05-24 9:38 UTC (permalink / raw) To: Jayesh Choudhary Cc: linux-kernel, andrzej.hajda, neil.armstrong, rfoss, Laurent.pinchart, mripard, dri-devel, jonas, jernej.skrabec, maarten.lankhorst, tzimmermann, airlied, daniel, a-bhatia1 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 > -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] drm/bridge: Add pixel clock check in atomic_check 2024-05-24 7:33 [PATCH v2 0/2] Add mode_valid hook for sii902x bridge Jayesh Choudhary 2024-05-24 7:33 ` [PATCH v2 1/2] drm/bridge: sii902x: Fix mode_valid hook Jayesh Choudhary @ 2024-05-24 7:33 ` Jayesh Choudhary 1 sibling, 0 replies; 6+ messages in thread From: Jayesh Choudhary @ 2024-05-24 7:33 UTC (permalink / raw) To: linux-kernel, andrzej.hajda, neil.armstrong, rfoss, Laurent.pinchart, mripard, dri-devel, j-choudhary Cc: jonas, jernej.skrabec, maarten.lankhorst, tzimmermann, airlied, daniel, a-bhatia1 Check the pixel clock for the mode in atomic_check and ensure that it is within the range supported by the bridge. Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com> --- drivers/gpu/drm/bridge/sii902x.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c index ef7c3ab3386c..a5b7ee1e3bbe 100644 --- a/drivers/gpu/drm/bridge/sii902x.c +++ b/drivers/gpu/drm/bridge/sii902x.c @@ -517,11 +517,16 @@ static int sii902x_bridge_atomic_check(struct drm_bridge *bridge, struct drm_crtc_state *crtc_state, struct drm_connector_state *conn_state) { + if (crtc_state->mode.clock < SII902X_MIN_PIXEL_CLOCK_KHZ) + return MODE_CLOCK_LOW; + + if (crtc_state->mode.clock > SII902X_MAX_PIXEL_CLOCK_KHZ) + return MODE_CLOCK_HIGH; + /* * 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; -- 2.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-05-24 9:38 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-05-24 7:33 [PATCH v2 0/2] Add mode_valid hook for sii902x bridge Jayesh Choudhary 2024-05-24 7:33 ` [PATCH v2 1/2] drm/bridge: sii902x: Fix mode_valid hook Jayesh Choudhary 2024-05-24 8:18 ` Sam Ravnborg 2024-05-24 9:21 ` Jayesh Choudhary 2024-05-24 9:38 ` Dmitry Baryshkov 2024-05-24 7:33 ` [PATCH v2 2/2] drm/bridge: Add pixel clock check in atomic_check Jayesh Choudhary
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox