Re: [RFC/PATCH 0/2] Clarify display info PIXDATA bus flags

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Stefan,

On Monday, 24 September 2018 14:54:36 EET Stefan Agner wrote:
> On 22.09.2018 14:15, Laurent Pinchart wrote:
> > Hello,
> > 
> > This patch series attemps at clarifying usage of the
> > DRM_BUS_FLAG_PIXDATA_(POS|NEG)EDGE flags. It results from a discussion
> > on the mailing list available at [1].
> > 
> > The problem being discussed was confusion around how the
> > DRM_BUS_FLAG_PIXDATA_POSEDGE and DRM_BUS_FLAG_PIXDATA_NEGEDGE flags
> > could be interpreted (and are interpreted now by drivers). Patch 1/2
> > introduces new, more explicit flags, and explains the rationale. Patch
> > 2/2 then updates the drivers to use the new flags.
> 
> IMHO, the meaning was quite clearly stated... I am not sure whether this
> added clarity is worth the churn.

It is stated in a comment, but it's not clear from the macro names, and I had 
to constantly refer back to the comments to make sure which side the flags 
applied to when reviewing related patches (or writing related code). This 
series started as "scratch your own itch" patches, but I think it has more 
potential than that. By making the drive and sample sides apparent in the API, 
we let drivers focus on their own side. Of course there's still the implied 
assumption that sampling on one edge requires driving on the other edge, but I 
would like to introduce a helper function that computes the driving edge based 
on all sampling parameters (edge, setup/hold time) and frequency.

> But I am ok with it if others think it's necessary.
> 
> Btw, if we change this for DRM_BUS_FLAG*, we probably should also do the
> equal change for DISPLAY_FLAGS_PIXDATA_[NEG|POS]EDGE. Since displays are
> always on the sample side, it probably has higher chance to get mixed
> up.

That's a good point. I'd like to focus on the DRM side first to see how that 
goes, and then address the display timings flags. It would be really nice if 
we could merge both...

> > [1] https://www.spinics.net/lists/arm-kernel/msg677079.html
> > 
> > Laurent Pinchart (2):
> >   drm: Clarify definition of the DRM_BUS_FLAG_PIXDATA_* macros
> >   drm: Use new DRM_BUS_FLAG_PIXDATA_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags
> >  
> >  drivers/gpu/drm/bridge/dumb-vga-dac.c                |  6 +++---
> >  drivers/gpu/drm/drm_modes.c                          |  8 ++++----
> >  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c           |  2 +-
> >  drivers/gpu/drm/imx/ipuv3-crtc.c                     |  2 +-
> >  drivers/gpu/drm/mxsfb/mxsfb_crtc.c                   |  6 +++---
> >  drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c    |  2 +-
> >  .../drm/omapdrm/displays/panel-lgphilips-lb035q02.c  |  2 +-
> >  .../gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c  |  2 +-
> >  .../drm/omapdrm/displays/panel-sharp-ls037v7dw01.c   |  2 +-
> >  .../gpu/drm/omapdrm/displays/panel-sony-acx565akm.c  |  2 +-
> >  .../gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c  |  2 +-
> >  .../gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c  |  2 +-
> >  drivers/gpu/drm/omapdrm/dss/dsi.c                    |  2 +-
> >  drivers/gpu/drm/omapdrm/dss/sdi.c                    |  2 +-
> >  drivers/gpu/drm/omapdrm/omap_encoder.c               |  4 ++--
> >  drivers/gpu/drm/panel/panel-arm-versatile.c          |  4 ++--
> >  drivers/gpu/drm/panel/panel-ilitek-ili9322.c         |  4 ++--
> >  drivers/gpu/drm/panel/panel-seiko-43wvf1g.c          |  2 +-
> >  drivers/gpu/drm/panel/panel-simple.c                 | 20 +++++++--------
> >  drivers/gpu/drm/pl111/pl111_display.c                |  2 +-
> >  drivers/gpu/drm/sun4i/sun4i_tcon.c                   |  4 ++--
> >  drivers/gpu/drm/tve200/tve200_display.c              |  3 ++-
> >  include/drm/drm_bridge.h                             |  8 ++++----
> >  include/drm/drm_connector.h                          | 20 +++++++++++++--
> >  24 files changed, 65 insertions(+), 48 deletions(-)

-- 
Regards,

Laurent Pinchart



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