On Mon, Sep 24, 2018 at 01:59:25PM +0200, Stefan Agner wrote:
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@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.
I was just wondering because the above clearly changes the value in .sampling_edge, but if it isn't currently used it doesn't really matter.
One potential problem I see here is that the kerneldoc for sampling_edge says that it should reuse the flags from the DRM connector. Now if the DRM connector specifies these from a drive perspective and the bridge interprets them from a sample perspective, this is obviously going to be a problem. But then, the only places where these are used seems to be in the VGA bridge driver, so I'm assuming that these are from a sampling perspective and should be interpreted that way.
So I think that's all that we need to define. If a driver specifies the values in some structure, then they should be interpreted from the driver's perspective. If a display driver uses the values provided by a bridge driver it needs to interpret them from a sampling perspective as well.
The issue I see with multiple definitions is that it doesn't actually solve the problem. If a bridge driver specifies the flags from a sampling perspective and the display driver interprets them as drive flags, there will still be confusion, right?
Thierry