Hi Daniel, On Friday, 11 January 2019 11:23:10 EET Daniel Vetter wrote: > On Fri, Jan 11, 2019 at 05:51:06AM +0200, Laurent Pinchart wrote: > > From: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx> > > > > The DRM_BUS_FLAG_PIXDATA_POSEDGE and DRM_BUS_FLAG_PIXDATA_NEGEDGE macros > > and their DRM_BUS_FLAG_SYNC_* counterparts define on which pixel clock > > edge data and sync signals are driven. They are however used in some > > drivers to define on which pixel clock edge data and sync signals are > > sampled, which should usually (but not always) be the opposite edge of > > the driving edge. This creates confusion. > > > > Create four new macros for both PIXDATA and SYNC that explicitly state > > the driving and sampling edge in their name to remove the confusion. The > > driving macros are defined as the opposite of the sampling macros to > > made code simpler based on the assumption that the driving and sampling > > edges are opposite. > > > > Signed-off-by: Laurent Pinchart > > <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx> > > Acked-by: Linus Walleij <linus.walleij@xxxxxxxxxx> > > --- > > Changes since v1: > > > > - Address the DRM_BUS_FLAG_SYNC_* flags > > - Rebase on top of drm-next > > --- > > > > include/drm/drm_connector.h | 36 ++++++++++++++++++++++++++++++++---- > > 1 file changed, 32 insertions(+), 4 deletions(-) > > > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > > index 9be2181b3ed7..00bb7a74962b 100644 > > --- a/include/drm/drm_connector.h > > +++ b/include/drm/drm_connector.h > > @@ -330,19 +330,47 @@ struct drm_display_info { > > > > #define DRM_BUS_FLAG_DE_LOW (1<<0) > > #define DRM_BUS_FLAG_DE_HIGH (1<<1) > > > > -/* drive data on pos. edge */ > > + > > +/* > > + * Don't use those two flags directly, use the > > DRM_BUS_FLAG_PIXDATA_DRIVE_* + * and DRM_BUS_FLAG_PIXDATA_SAMPLE_* > > variants to qualify the flags explicitly. + * The > > DRM_BUS_FLAG_PIXDATA_SAMPLE_* flags are defined as the opposite of the + > > * DRM_BUS_FLAG_PIXDATA_DRIVE_* flags to make code simpler, as signals are > > + * usually to be sampled on the opposite edge of the driving edge. + */ > > > > #define DRM_BUS_FLAG_PIXDATA_POSEDGE (1<<2) > > > > -/* drive data on neg. edge */ > > > > #define DRM_BUS_FLAG_PIXDATA_NEGEDGE (1<<3) > > > > + > > +/* Drive data on rising edge */ > > +#define DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE DRM_BUS_FLAG_PIXDATA_POSEDGE > > +/* Drive data on falling edge */ > > +#define DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE DRM_BUS_FLAG_PIXDATA_NEGEDGE > > +/* Sample data on rising edge */ > > +#define DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE DRM_BUS_FLAG_PIXDATA_NEGEDGE > > +/* Sample data on falling edge */ > > +#define DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE DRM_BUS_FLAG_PIXDATA_POSEDGE > > + > > > > /* data is transmitted MSB to LSB on the bus */ > > #define DRM_BUS_FLAG_DATA_MSB_TO_LSB (1<<4) > > /* data is transmitted LSB to MSB on the bus */ > > #define DRM_BUS_FLAG_DATA_LSB_TO_MSB (1<<5) > > > > -/* drive sync on pos. edge */ > > + > > +/* > > + * Similarly to the DRM_BUS_FLAG_PIXDATA_* flags, don't use these two > > flags + * directly, use one of the DRM_BUS_FLAG_SYNC_(DRIVE|SAMPLE)_* > > instead. + */ > > > > #define DRM_BUS_FLAG_SYNC_POSEDGE (1<<6) > > > > -/* drive sync on neg. edge */ > > > > #define DRM_BUS_FLAG_SYNC_NEGEDGE (1<<7) > > > > +/* Drive sync on rising edge */ > > +#define DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE DRM_BUS_FLAG_SYNC_POSEDGE > > +/* Drive sync on falling edge */ > > +#define DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE DRM_BUS_FLAG_SYNC_NEGEDGE > > +/* Sample sync on rising edge */ > > +#define DRM_BUS_FLAG_SYNC_SAMPLE_POSEDGE DRM_BUS_FLAG_SYNC_NEGEDGE > > +/* Sample sync on falling edge */ > > +#define DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE DRM_BUS_FLAG_SYNC_POSEDGE > > Since these are internal I recommend you convert them over to an enum and > up this to full kerneldoc comments. That way you can have lots more pretty > formatting and linking, and an easy way to give others a public link to > full docs. The patch is in your mailbox. > > + > > /** > > * @bus_flags: Additional information (like pixel signal polarity) for > > * the pixel data on the bus, using DRM_BUS_FLAGS\_ defines. -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel