| Message ID | 20220219002844.362157-4-marex@denx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/bridge: Add support for selecting DSI host HS clock from DSI bridge | expand |
Hi, On Sat, Feb 19, 2022 at 01:28:40AM +0100, Marek Vasut wrote: > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > index 1701c2128a5cb..32455cf28f0bc 100644 > --- a/include/drm/drm_atomic.h > +++ b/include/drm/drm_atomic.h > @@ -1077,6 +1077,11 @@ struct drm_bus_cfg { > * @flags: DRM_BUS_* flags used on this bus > */ > u32 flags; > + > + /** > + * @clock: Clock frequency in kHz used on this bus > + */ > + u32 clock; > }; This is fairly vague. You were mentioning DSI: is it the pixel clock? The HS clock rate? With or without counting the lanes? What about the burst mode: would it be the lane or pixel rate? It would be just as confusing for HDMI: is it the the TMDS character rate? The TMDS bit rate ? TMDS Clock rate? Maxime
On 2/24/22 16:19, Maxime Ripard wrote: > Hi, Hi, > On Sat, Feb 19, 2022 at 01:28:40AM +0100, Marek Vasut wrote: >> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h >> index 1701c2128a5cb..32455cf28f0bc 100644 >> --- a/include/drm/drm_atomic.h >> +++ b/include/drm/drm_atomic.h >> @@ -1077,6 +1077,11 @@ struct drm_bus_cfg { >> * @flags: DRM_BUS_* flags used on this bus >> */ >> u32 flags; >> + >> + /** >> + * @clock: Clock frequency in kHz used on this bus >> + */ >> + u32 clock; >> }; > > This is fairly vague. You were mentioning DSI: is it the pixel clock? DSI HS clock is the one I need. I hope we can flesh out what exactly should be in here. > The HS clock rate? Yes > With or without counting the lanes? What about the Without > burst mode: would it be the lane or pixel rate? Still the HS clock rate. > It would be just as confusing for HDMI: is it the the TMDS character > rate? The TMDS bit rate ? TMDS Clock rate? For HDMI I would expect 148.5 MHz here , and if HDMI needs additional extras, they might have to be added to struct drm_bus_cfg as extra fields ?
On Thu, Feb 24, 2022 at 09:07:19PM +0100, Marek Vasut wrote: > On 2/24/22 16:19, Maxime Ripard wrote: > > On Sat, Feb 19, 2022 at 01:28:40AM +0100, Marek Vasut wrote: > > > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > > > index 1701c2128a5cb..32455cf28f0bc 100644 > > > --- a/include/drm/drm_atomic.h > > > +++ b/include/drm/drm_atomic.h > > > @@ -1077,6 +1077,11 @@ struct drm_bus_cfg { > > > * @flags: DRM_BUS_* flags used on this bus > > > */ > > > u32 flags; > > > + > > > + /** > > > + * @clock: Clock frequency in kHz used on this bus > > > + */ > > > + u32 clock; > > > }; > > > > This is fairly vague. You were mentioning DSI: is it the pixel clock? > > DSI HS clock is the one I need. > > I hope we can flesh out what exactly should be in here. > > > The HS clock rate? > > Yes > > > With or without counting the lanes? What about the > > Without > > > burst mode: would it be the lane or pixel rate? > > Still the HS clock rate. > > > It would be just as confusing for HDMI: is it the the TMDS character > > rate? The TMDS bit rate ? TMDS Clock rate? > > For HDMI I would expect 148.5 MHz here , and if HDMI needs additional > extras, they might have to be added to struct drm_bus_cfg as extra fields ? The thing is: you're patching some core code here. Whatever you come up with needs to be properly defined, documented, and should apply to all the display interfaces we support. It cannot be an after-thought. Even for DSI, I don't think that the HS clock is something that is desirable: how does it interacts with virtual channels? burst mode or not? The pixel clock is a better choice I think for this, since this is abstract enough to apply to all the interfaces, and the devices can easily compute whatever they want to based on the pixel clock as well. If you *really* need the HS clock itself, then the struct mipi_dsi_device feels like a better abstraction. Which raises the question: why can't you use hs_rate? Maxime
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index a069c50cc7d6b..6a5981b82499a 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -859,7 +859,9 @@ static int select_bus_fmt_recursive(struct drm_bridge *first_bridge, */ if (cur_state) { cur_state->input_bus_cfg.format = MEDIA_BUS_FMT_FIXED; + cur_state->input_bus_cfg.clock = 0; cur_state->output_bus_cfg.format = out_bus_cfg->format; + cur_state->output_bus_cfg.clock = out_bus_cfg->clock; } return 0; @@ -911,7 +913,9 @@ static int select_bus_fmt_recursive(struct drm_bridge *first_bridge, if (first_bridge == cur_bridge) { cur_state->input_bus_cfg.format = in_bus_cfgs[0].format; + cur_state->input_bus_cfg.clock = in_bus_cfgs[0].clock; cur_state->output_bus_cfg.format = out_bus_cfg->format; + cur_state->output_bus_cfg.clock = out_bus_cfg->clock; kfree(in_bus_cfgs); return 0; } @@ -926,7 +930,9 @@ static int select_bus_fmt_recursive(struct drm_bridge *first_bridge, if (!ret) { cur_state->input_bus_cfg.format = in_bus_cfgs[i].format; + cur_state->input_bus_cfg.clock = in_bus_cfgs[i].clock; cur_state->output_bus_cfg.format = out_bus_cfg->format; + cur_state->output_bus_cfg.clock = out_bus_cfg->clock; } kfree(in_bus_cfgs); diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index 1701c2128a5cb..32455cf28f0bc 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -1077,6 +1077,11 @@ struct drm_bus_cfg { * @flags: DRM_BUS_* flags used on this bus */ u32 flags; + + /** + * @clock: Clock frequency in kHz used on this bus + */ + u32 clock; }; /**
Extend struct drm_bus_cfg with a clock field. This makes it possible for an upstream bridge (further from scanout engine) to indicate to a downstream bridge which frequency it expects on a link. This is particularly useful in case of DSI bridges which derive their own internal clock from the DSI HS clock. Signed-off-by: Marek Vasut <marex@denx.de> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> Cc: Maxime Ripard <maxime@cerno.tech> Cc: Neil Armstrong <narmstrong@baylibre.com> Cc: Sam Ravnborg <sam@ravnborg.org> --- drivers/gpu/drm/drm_bridge.c | 6 ++++++ include/drm/drm_atomic.h | 5 +++++ 2 files changed, 11 insertions(+)