Hi Laurent, Thank you for the feedback! I think we need to come a conclusion on here: https://patchwork.kernel.org/patch/11095547/ before taking the comments on this patch any further. Thanks, Fab > From: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > Sent: 15 August 2019 13:00 > Subject: Re: [PATCH v2 4/9] drm/timings: Add link flags > > Hi Fabrizio, > > Thank you for the patch. > > On Thu, Aug 15, 2019 at 12:04:28PM +0100, Fabrizio Castro wrote: > > We need more information to describe dual-LVDS links, therefore > > introduce link_flags. > > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@xxxxxxxxxxxxxx> > > > > --- > > v1->v2: > > * new patch > > > > include/drm/drm_timings.h | 26 ++++++++++++++++++++++++++ > > 1 file changed, 26 insertions(+) > > > > diff --git a/include/drm/drm_timings.h b/include/drm/drm_timings.h > > index 4af8814..58fbf1b 100644 > > --- a/include/drm/drm_timings.h > > +++ b/include/drm/drm_timings.h > > @@ -1,4 +1,6 @@ > > /* > > + * Copyright (C) 2019 Renesas Electronics Corporation > > + * > > * Permission to use, copy, modify, distribute, and sell this software and its > > * documentation for any purpose is hereby granted without fee, provided that > > * the above copyright notice appear in all copies and that both that copyright > > @@ -21,6 +23,24 @@ > > #ifndef __DRM_TIMINGS_H__ > > #define __DRM_TIMINGS_H__ > > > > +#include <linux/bits.h> > > + > > +/** > > + * enum drm_link_flags - link_flags for &drm_timings > > + * > > + * This enum defines the details of the link. > > + * > > + * @DRM_LINK_DUAL_LVDS_ODD_EVEN: Dual-LVDS link, with odd pixels (1,3,5, > > + * etc.) coming through the first port, and > > + * even pixels (0,2,4,etc.) coming through > > + * the second port. If not specified for a > > + * dual-LVDS panel, it is assumed that even > > + * pixels are coming through the first port > > + */ > > +enum drm_link_flags { > > The text will be easier to read if you inline it here. > > /** > * @DRM_LINK_DUAL_LVDS_ODD_EVEN: Dual-LVDS link, with odd pixels (1,3,5, > * etc.) coming through the first port, and even pixels (0,2,4,etc.) > ... > > > + DRM_LINK_DUAL_LVDS_ODD_EVEN = BIT(0), > > I would remove the dual_link field and add a DRM_LINK_DUAL_LVDS (or > alternatively two flags, dual lvds odd-even and dual lvds even-odd). > > > +}; > > + > > /** > > * struct drm_timings - timing information > > */ > > @@ -55,6 +75,12 @@ struct drm_timings { > > * and odd-numbered pixels are received on separate links. > > */ > > bool dual_link; > > + /** > > + * @link_flags > > + * > > + * Provides detailed information about the link. > > I think this calls for a bit more information than "detailed > information". What information do you want to store in this field ? > > > + */ > > + enum drm_link_flags link_flags; > > }; > > > > #endif /* __DRM_TIMINGS_H__ */ > > -- > > 2.7.4 > > > > -- > Regards, > > Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel