On Sat, Jan 12, 2019 at 03:10:51AM +0200, Laurent Pinchart wrote:
> This allows nicer kerneldoc with an easy way to reference the enum and
> the values.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---
> include/drm/drm_connector.h | 108 +++++++++++++++++++++---------------
> 1 file changed, 64 insertions(+), 44 deletions(-)
>
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 00bb7a74962b..f5ce255a2cb4 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -253,6 +253,68 @@ enum drm_panel_orientation {
> DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
> };
>
> +/**
> + * enum drm_bus_flags - bus_flags info for &drm_display_info
> + *
> + * This enum defines signal polarities and clock edge information for signals on
> + * a bus as bitmask flags.
> + *
> + * The clock edge information is conveyed by two sets of symbols,
> + * DRM_BUS_FLAGS_*_DRIVE_\* and DRM_BUS_FLAGS_*_SAMPLE_\*. When this enum is
> + * used to describe a bus from the point of view of the transmitter, the
> + * \*_DRIVE_\* flags should be used. When used from the point of view of the
> + * receiver, the \*_SAMPLE_\* flags should be used. The \*_DRIVE_\* and
> + * \*_SAMPLE_\* flags alias each other, with the \*_SAMPLE_POSEDGE and
> + * \*_SAMPLE_NEGEDGE flags being equal to \*_DRIVE_NEGEDGE and \*_DRIVE_POSEDGE
> + * respectively. This simplifies code as signals are usually sampled on the
> + * opposite edge of the driving edge. Transmitters and receivers may however
> + * need to take other signal timings into account to convert between driving
> + * and sample edges.
> + *
> + * @DRM_BUS_FLAG_DE_LOW: The Data Enable signal is active low
> + * @DRM_BUS_FLAG_DE_HIGH: The Data Enable signal is active high
> + * @DRM_BUS_FLAG_PIXDATA_POSEDGE: Legacy value, do not use
> + * @DRM_BUS_FLAG_PIXDATA_NEGEDGE: Legacy value, do not use
> + * @DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE: Data is driven on the rising edge of
> + * the pixel clock
> + * @DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE: Data is driven on the falling edge of
> + * the pixel clock
> + * @DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE: Data is sampled on the rising edge of
> + * the pixel clock
> + * @DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE: Data is sampled on the falling edge of
> + * the pixel clock
> + * @DRM_BUS_FLAG_DATA_MSB_TO_LSB: Data is transmitted MSB to LSB on the bus
> + * @DRM_BUS_FLAG_DATA_LSB_TO_MSB: Data is transmitted LSB to MSB on the bus
> + * @DRM_BUS_FLAG_SYNC_POSEDGE: Legacy value, do not use
> + * @DRM_BUS_FLAG_SYNC_NEGEDGE: Legacy value, do not use
> + * @DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE: Sync signals are driven on the rising
> + * edge of the pixel clock
> + * @DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE: Sync signals are driven on the falling
> + * edge of the pixel clock
> + * @DRM_BUS_FLAG_SYNC_SAMPLE_POSEDGE: Sync signals are sampled on the rising
> + * edge of the pixel clock
> + * @DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE: Sync signals are sampled on the falling
> + * edge of the pixel clock
> + */
For bigger stuff like enum/struct I tend to lean towards the inline
comment style, gives you a lot more space for documenting things. But also
uses a lot more whitespace, so a bit a judgement call.
Anyway, I'm always happy for more structured comments:
Acked-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> +enum drm_bus_flags {
> + DRM_BUS_FLAG_DE_LOW = BIT(0),
> + DRM_BUS_FLAG_DE_HIGH = BIT(1),
> + DRM_BUS_FLAG_PIXDATA_POSEDGE = BIT(2),
> + DRM_BUS_FLAG_PIXDATA_NEGEDGE = BIT(3),
> + DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> + DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> + DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> + DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> + DRM_BUS_FLAG_DATA_MSB_TO_LSB = BIT(4),
> + DRM_BUS_FLAG_DATA_LSB_TO_MSB = BIT(5),
> + DRM_BUS_FLAG_SYNC_POSEDGE = BIT(6),
> + DRM_BUS_FLAG_SYNC_NEGEDGE = BIT(7),
> + DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE = DRM_BUS_FLAG_SYNC_POSEDGE,
> + DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE = DRM_BUS_FLAG_SYNC_NEGEDGE,
> + DRM_BUS_FLAG_SYNC_SAMPLE_POSEDGE = DRM_BUS_FLAG_SYNC_NEGEDGE,
> + DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE = DRM_BUS_FLAG_SYNC_POSEDGE,
> +};
> +
> /**
> * struct drm_display_info - runtime data about the connected sink
> *
> @@ -328,52 +390,10 @@ struct drm_display_info {
> */
> unsigned int num_bus_formats;
>
> -#define DRM_BUS_FLAG_DE_LOW (1<<0)
> -#define DRM_BUS_FLAG_DE_HIGH (1<<1)
> -
> -/*
> - * 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)
> -#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)
> -
> -/*
> - * 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)
> -#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
> -
> /**
> * @bus_flags: Additional information (like pixel signal polarity) for
> - * the pixel data on the bus, using DRM_BUS_FLAGS\_ defines.
> + * the pixel data on the bus, using &enum drm_bus_flags values
> + * DRM_BUS_FLAGS\_.
> */
> u32 bus_flags;
>
> --
> Regards,
>
> Laurent Pinchart
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
