[PATCH 3/7] drm/bridge: Extend struct drm_bus_cfg with clock field

[Maxime Ripard]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20220225/c665d0ee/attachment-0001.sig>