| Message ID | 1565867073-24746-5-git-send-email-fabrizio.castro@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | Add dual-LVDS panel support to EK874 | expand |
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@bp.renesas.com> > > --- > 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 >
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@ideasonboard.com> > 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@bp.renesas.com> > > > > --- > > 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
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 { + DRM_LINK_DUAL_LVDS_ODD_EVEN = BIT(0), +}; + /** * 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. + */ + enum drm_link_flags link_flags; }; #endif /* __DRM_TIMINGS_H__ */
We need more information to describe dual-LVDS links, therefore introduce link_flags. Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com> --- v1->v2: * new patch include/drm/drm_timings.h | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)