Re: [RFC/PATCH 2/2] drm: Use new DRM_BUS_FLAG_PIXDATA_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 4224 bytes --]

On Sat, Sep 22, 2018 at 03:15:04PM +0300, Laurent Pinchart wrote:
> The DRM_BUS_FLAG_PIXDATA_(POS|NEG)EDGE flags are deprecated in favour of
> the new DRM_BUS_FLAG_PIXDATA_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags. Replace
> them through the code.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  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 ++++----
>  23 files changed, 47 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> index 9b706789a341..7dc14c22f7db 100644
> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> @@ -234,7 +234,7 @@ static int dumb_vga_remove(struct platform_device *pdev)
>   */
>  static const struct drm_bridge_timings default_dac_timings = {
>  	/* Timing specifications, datasheet page 7 */
> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +	.sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
>  	.setup_time_ps = 500,
>  	.hold_time_ps = 1500,
>  };
> @@ -245,7 +245,7 @@ static const struct drm_bridge_timings default_dac_timings = {
>   */
>  static const struct drm_bridge_timings ti_ths8134_dac_timings = {
>  	/* From timing diagram, datasheet page 9 */
> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +	.sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
>  	/* From datasheet, page 12 */
>  	.setup_time_ps = 3000,
>  	/* I guess this means latched input */
> @@ -258,7 +258,7 @@ static const struct drm_bridge_timings ti_ths8134_dac_timings = {
>   */
>  static const struct drm_bridge_timings ti_ths8135_dac_timings = {
>  	/* From timing diagram, datasheet page 14 */
> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +	.sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
>  	/* From datasheet, page 16 */
>  	.setup_time_ps = 2000,
>  	.hold_time_ps = 500,

Now I'm confused. Aren't you effectively iverting the sampling edges
here?

That and the fact that everywhere else we only use the driver variants
makes me think that we should just define which way these are supposed
to be used and just have a single set of definitions.

Also, I think there's no need for these to be always "physically"
correct. This is up to interpretation by the driver, so if a bridge
driver wants to use them as meaning "sampled edge", that's fine. If
display controllers use them to mean "driven edge", that's equally
fine.

As long as we don't communicate the flags between drivers there should
be no reason for them to be confusing. Just make sure that the comments
in the interfaces clarify from which point of view these flags are to be
interpreted and we should be fine.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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