Re: [PATCH v3 1/7] drm: Add DSI bus infrastructure

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2956 bytes --]

On Mon, Nov 18, 2013 at 11:59:23AM +0000, Bert Kenward wrote:
> On 11/18/2013 11:22, Thierry Reding wrote:
> > On Thu, Nov 14, 2013 at 03:04:19PM +0000, Bert Kenward wrote:
> > > #define DSI_WINDOW_VFP  (1 << 0)
> > > #define DSI_WINDOW_ACT  (1 << 1)
> > > #define DSI_WINDOW_VBP  (1 << 2)
> > > #define DSI_WINDOW_VSY  (1 << 3)
> > >
> > > /**
> > >  * struct dsi_msg - DSI command message
> > >  * @channel: virtual channel to send the message to
> > >  * @type: data ID of the message
> > >  * @lp_mode: send in LP mode if non-zero
> > 
> > Perhaps a flags field would be more flexible here. I can easily imagine
> > other I can imagine that other flags may be needed eventually.
> 
> Agreed. "TE synchronised" would be one such extra flag, for supporting command mode updates.
> 
> > >  * @window: video period when transfer is allowed - bitmask of
> > DSI_WINDOW_*
> > 
> > I'm not sure if this is the right interface. What will happen for
> > instance if the hardware doesn't support any of the bits in that mask?
> > Perhaps a slightly better approach might be to expose the capabilities
> > of the DSI host, so that the DSI core knows up front which windows can
> > be used.
> 
> Exposing the capabilities seems like the smart thing to do, certainly
> - but you'd still need a way to specify which of those capabilities
> you want to use for each transfer.

Yes. I think we'll still need to have that. It's just that for some
transfers it doesn't matter during which window they are executed.
Although I guess in those cases the caller could just specify all bits
to signal that it doesn't care.

> I'd suggest that hardware would ignore bits that it couldn't support -
> in the limit, hardware that has no way to choose when to send a
> command during video would ignore this completely. I realise that
> could well cause confusion when trying to work out why a particularly
> display is misbehaving when you think you're sending commands at the
> right time.

I think at the very least if there's no match between the requested set
of windows and the ones that a particular DSI host supports, then the
driver should report an error.

The reason why I thought that exposing the capabilities might be useful
is that the caller could be smart about how to transfer a message, or
perhaps use different messages to the same effect. But perhaps that's
not even possible and maybe not worth considering.

A smart thing to do in my opinion will be to not try to overengineer
this from the beginning. That's why I'm thinking of splitting out the
whose dsi_msg support in a first step, so that the bus infrastructure
can be merged without having discussed all possible cases. And even so
dsi_msg doesn't have to be perfect from the start. It's an in-kernel
API, therefore can change easily if needed. The less we require of it
now the easier it will be to extend when the need arises.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel