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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
