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

[Stefan Agner]

On 24.09.2018 13:48, Thierry Reding wrote:
> 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 at 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?

The meaning of the flag has been defined differently for the
sampling_edge case, see current comment in include/drm/drm_bridge.h.

This is correct. It essentially fixes the flags such that they can be
interpreted by the driver with the usual assumption to drive on opposite
edge. It is essentially the same as my patch did, but using the new
flag:
https://patchwork.kernel.org/patch/10589233/

So far, no driver used sampling_edge, so we can change safely at this
point.

--
Stefan

> 
> 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