| Message ID | 20180922121504.31220-3-laurent.pinchart+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Clarify display info PIXDATA bus flags | expand |
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
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. -- 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
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
On 24.09.2018 14:13, Thierry Reding wrote: > 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. Until struct drm_bridge_timings has been introduced, *everything* was driving perspective. Display timings (DISPLAY_FLAGS_PIXDATA_[POS|NEG]EDGE) and bus flags... struct drm_bridge_timings reuses the bus flags, and with that we have a somewhat conflicting definition: - At the #define site for the flags we document that they are drive perspective - sampling_edge reuses the same flag, but changes its meaning according to kerneldoc This is not ideal, and my patch as well as this patch series is an attempt to clear things up. > > 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? Having the drive/sample situation encoded in the flag avoids confusion... We assume that by default we drive/sample at the opposite edge, and that is how the flags are defined. What confusion do you still see? My attempt to clear things up was just using the definition we always had: https://lkml.org/lkml/2018/9/12/911 I see Laurents point, but I am not sure if its worth the churn. Especially since we also use driving view for DISPLAY_FLAGS_PIXDATA_[POS|NEG]EDGE (see include/video/display_timing.h). Maybe we should just restate the fact that pixel clock is driving view in kerneldoc of struct drm_bridge_timings... -- Stefan
Hello, On Monday, 24 September 2018 15:34:54 EET Stefan Agner wrote: > On 24.09.2018 14:13, Thierry Reding wrote: > > 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. Your concern was correct, the patch changes the value, and as Stefan explained it is safe as no driver consumes these values. I will however update the commit message to make this explicit. > > 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. > > Until struct drm_bridge_timings has been introduced, *everything* was > driving perspective. Display timings > (DISPLAY_FLAGS_PIXDATA_[POS|NEG]EDGE) and bus flags... > > struct drm_bridge_timings reuses the bus flags, and with that we have a > somewhat conflicting definition: > > - At the #define site for the flags we document that they are drive > perspective > - sampling_edge reuses the same flag, but changes its meaning according > to kerneldoc > > This is not ideal, and my patch as well as this patch series is an > attempt to clear things up. > > > 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? > > Having the drive/sample situation encoded in the flag avoids > confusion... > > We assume that by default we drive/sample at the opposite edge, and that > is how the flags are defined. > > What confusion do you still see? > > > My attempt to clear things up was just using the definition we always > had: > https://lkml.org/lkml/2018/9/12/911 > > I see Laurents point, but I am not sure if its worth the churn. I'll handle the churn :-) > Especially since we also use driving view for > DISPLAY_FLAGS_PIXDATA_[POS|NEG]EDGE (see > include/video/display_timing.h). I will try to have a look at that after handling the DRM side. > Maybe we should just restate the fact that pixel clock is driving view > in kerneldoc of struct drm_bridge_timings... I would prefer avoiding that. I think bridges should be as simple as possible and just expose their own parameters, from their own point of view. The complexity of deciding on which edge to drive should be on the source (display controller) side, based on the sink's (bridge) intrinsic parameters (sampling edge, setup/hold time), on the bus clock and on the source's intrinsic parameters (internal signal delays). The computation should happen in a helper function, and it should then be as simple as the following pseudo-code for display controller drivers. source_timings.pixdata_delay_ps = 8000; driving_edge = drm_timings_driving_edge(bridge, mode, &source_timings);
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, diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 02db9ac82d7a..475bdc8a6420 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -662,17 +662,17 @@ EXPORT_SYMBOL_GPL(drm_display_mode_to_videomode); * @bus_flags: information about pixelclk, sync and DE polarity will be stored * here * - * Sets DRM_BUS_FLAG_DE_(LOW|HIGH), DRM_BUS_FLAG_PIXDATA_(POS|NEG)EDGE and - * DISPLAY_FLAGS_SYNC_(POS|NEG)EDGE in @bus_flags according to DISPLAY_FLAGS + * Sets DRM_BUS_FLAG_DE_(LOW|HIGH), DRM_BUS_FLAG_PIXDATA_DRIVE_(POS|NEG)EDGE + * and DISPLAY_FLAGS_SYNC_(POS|NEG)EDGE in @bus_flags according to DISPLAY_FLAGS * found in @vm */ void drm_bus_flags_from_videomode(const struct videomode *vm, u32 *bus_flags) { *bus_flags = 0; if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE) - *bus_flags |= DRM_BUS_FLAG_PIXDATA_POSEDGE; + *bus_flags |= DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE; if (vm->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE) - *bus_flags |= DRM_BUS_FLAG_PIXDATA_NEGEDGE; + *bus_flags |= DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE; if (vm->flags & DISPLAY_FLAGS_SYNC_POSEDGE) *bus_flags |= DRM_BUS_FLAG_SYNC_POSEDGE; diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c index 0e3752437e44..ba0d00f1a8b1 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c @@ -99,7 +99,7 @@ static void fsl_dcu_drm_crtc_mode_set_nofb(struct drm_crtc *crtc) vsw = mode->vsync_end - mode->vsync_start; /* INV_PXCK as default (most display sample data on rising edge) */ - if (!(con->display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_POSEDGE)) + if (!(con->display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)) pol |= DCU_SYN_POL_INV_PXCK; if (mode->flags & DRM_MODE_FLAG_NHSYNC) diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c index 7d4b710b837a..82c513fc0ae3 100644 --- a/drivers/gpu/drm/imx/ipuv3-crtc.c +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c @@ -277,7 +277,7 @@ static void ipu_crtc_mode_set_nofb(struct drm_crtc *crtc) sig_cfg.enable_pol = !(imx_crtc_state->bus_flags & DRM_BUS_FLAG_DE_LOW); /* Default to driving pixel data on negative clock edges */ sig_cfg.clk_pol = !!(imx_crtc_state->bus_flags & - DRM_BUS_FLAG_PIXDATA_POSEDGE); + DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE); sig_cfg.bus_format = imx_crtc_state->bus_format; sig_cfg.v_to_h_sync = 0; sig_cfg.hsync_pin = imx_crtc_state->di_hsync_pin; diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c index 0abe77675b76..bbbf4b3df949 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c @@ -242,12 +242,12 @@ static void mxsfb_crtc_mode_set_nofb(struct mxsfb_drm_private *mxsfb) if (!(bus_flags & DRM_BUS_FLAG_DE_LOW)) vdctrl0 |= VDCTRL0_ENABLE_ACT_HIGH; /* - * DRM_BUS_FLAG_PIXDATA_ defines are controller centric, + * DRM_BUS_FLAG_PIXDATA_DRIVE_ defines are controller centric, * controllers VDCTRL0_DOTCLK is display centric. * Drive on positive edge -> display samples on falling edge - * DRM_BUS_FLAG_PIXDATA_POSEDGE -> VDCTRL0_DOTCLK_ACT_FALLING + * DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE -> VDCTRL0_DOTCLK_ACT_FALLING */ - if (bus_flags & DRM_BUS_FLAG_PIXDATA_POSEDGE) + if (bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE) vdctrl0 |= VDCTRL0_DOTCLK_ACT_FALLING; writel(vdctrl0, mxsfb->base + LCDC_VDCTRL0); diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c b/drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c index f1a748353279..0a8132c2352a 100644 --- a/drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c +++ b/drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c @@ -112,7 +112,7 @@ static int tfp410_probe(struct platform_device *pdev) dssdev->owner = THIS_MODULE; dssdev->of_ports = BIT(1) | BIT(0); dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_SYNC_POSEDGE - | DRM_BUS_FLAG_PIXDATA_POSEDGE; + | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE; dssdev->next = omapdss_of_find_connected_device(pdev->dev.of_node, 1); if (IS_ERR(dssdev->next)) { diff --git a/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c b/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c index f6ef8ff964dd..b468ef158bc8 100644 --- a/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c +++ b/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c @@ -230,7 +230,7 @@ static int lb035q02_panel_spi_probe(struct spi_device *spi) * DATA needs to be driven on the FALLING edge */ dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_SYNC_NEGEDGE - | DRM_BUS_FLAG_PIXDATA_POSEDGE; + | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE; omapdss_display_init(dssdev); omapdss_device_register(dssdev); diff --git a/drivers/gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c b/drivers/gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c index f445de6369f7..09796105796d 100644 --- a/drivers/gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c +++ b/drivers/gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c @@ -219,7 +219,7 @@ static int nec_8048_probe(struct spi_device *spi) dssdev->owner = THIS_MODULE; dssdev->of_ports = BIT(0); dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_SYNC_POSEDGE - | DRM_BUS_FLAG_PIXDATA_POSEDGE; + | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE; omapdss_display_init(dssdev); omapdss_device_register(dssdev); diff --git a/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c b/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c index 64b1369cb274..63e6db2b9a69 100644 --- a/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c +++ b/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c @@ -228,7 +228,7 @@ static int sharp_ls_probe(struct platform_device *pdev) * DATA needs to be driven on the FALLING edge */ dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_SYNC_NEGEDGE - | DRM_BUS_FLAG_PIXDATA_POSEDGE; + | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE; omapdss_display_init(dssdev); omapdss_device_register(dssdev); diff --git a/drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c b/drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c index e04663856b31..0fb04ce024b1 100644 --- a/drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c +++ b/drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c @@ -742,7 +742,7 @@ static int acx565akm_probe(struct spi_device *spi) dssdev->owner = THIS_MODULE; dssdev->of_ports = BIT(0); dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_SYNC_NEGEDGE - | DRM_BUS_FLAG_PIXDATA_POSEDGE; + | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE; omapdss_display_init(dssdev); omapdss_device_register(dssdev); diff --git a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c index 7ddc8c574a61..66a1a2d456b4 100644 --- a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c +++ b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c @@ -352,7 +352,7 @@ static int td028ttec1_panel_probe(struct spi_device *spi) * SYNC needs to be driven on the FALLING edge */ dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_SYNC_POSEDGE - | DRM_BUS_FLAG_PIXDATA_NEGEDGE; + | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE; omapdss_display_init(dssdev); omapdss_device_register(dssdev); diff --git a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c index 8440fcb744d9..f5608e0e655e 100644 --- a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c +++ b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c @@ -450,7 +450,7 @@ static int tpo_td043_probe(struct spi_device *spi) * SYNC needs to be driven on the FALLING edge */ dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_SYNC_POSEDGE - | DRM_BUS_FLAG_PIXDATA_NEGEDGE; + | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE; omapdss_display_init(dssdev); omapdss_device_register(dssdev); diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c index 394c129cfb3b..06952e0cc3cc 100644 --- a/drivers/gpu/drm/omapdrm/dss/dsi.c +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c @@ -5135,7 +5135,7 @@ static int dsi_init_output(struct dsi_data *dsi) out->ops = &dsi_ops; out->owner = THIS_MODULE; out->of_ports = BIT(0); - out->bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE + out->bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE | DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_SYNC_NEGEDGE; diff --git a/drivers/gpu/drm/omapdrm/dss/sdi.c b/drivers/gpu/drm/omapdrm/dss/sdi.c index b2fe2387037a..656e5838b40a 100644 --- a/drivers/gpu/drm/omapdrm/dss/sdi.c +++ b/drivers/gpu/drm/omapdrm/dss/sdi.c @@ -294,7 +294,7 @@ static int sdi_init_output(struct sdi_device *sdi) out->of_ports = BIT(1); out->ops = &sdi_ops; out->owner = THIS_MODULE; - out->bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE /* 15.5.9.1.2 */ + out->bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE /* 15.5.9.1.2 */ | DRM_BUS_FLAG_SYNC_POSEDGE; out->next = omapdss_of_find_connected_device(out->dev->of_node, 1); diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c index 452e625f6ce3..e7e833da0ee3 100644 --- a/drivers/gpu/drm/omapdrm/omap_encoder.c +++ b/drivers/gpu/drm/omapdrm/omap_encoder.c @@ -88,9 +88,9 @@ static void omap_encoder_mode_set(struct drm_encoder *encoder, if (!(vm.flags & (DISPLAY_FLAGS_PIXDATA_POSEDGE | DISPLAY_FLAGS_PIXDATA_NEGEDGE))) { - if (bus_flags & DRM_BUS_FLAG_PIXDATA_POSEDGE) + if (bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE) vm.flags |= DISPLAY_FLAGS_PIXDATA_POSEDGE; - else if (bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE) + else if (bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE) vm.flags |= DISPLAY_FLAGS_PIXDATA_NEGEDGE; } diff --git a/drivers/gpu/drm/panel/panel-arm-versatile.c b/drivers/gpu/drm/panel/panel-arm-versatile.c index b428c4678106..078fa2c0eef8 100644 --- a/drivers/gpu/drm/panel/panel-arm-versatile.c +++ b/drivers/gpu/drm/panel/panel-arm-versatile.c @@ -191,7 +191,7 @@ static const struct versatile_panel_type versatile_panels[] = { .vrefresh = 390, .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC, }, - .bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE, + .bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE, }, /* * Sanyo ALR252RGT 240x320 portrait display found on the @@ -215,7 +215,7 @@ static const struct versatile_panel_type versatile_panels[] = { .vrefresh = 116, .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC, }, - .bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE, + .bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE, .ib2 = true, }, }; diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9322.c b/drivers/gpu/drm/panel/panel-ilitek-ili9322.c index bd38bf4f1ba6..35497ff08391 100644 --- a/drivers/gpu/drm/panel/panel-ilitek-ili9322.c +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9322.c @@ -412,11 +412,11 @@ static int ili9322_init(struct drm_panel *panel, struct ili9322 *ili) if (ili->conf->dclk_active_high) { reg = ILI9322_POL_DCLK; connector->display_info.bus_flags |= - DRM_BUS_FLAG_PIXDATA_POSEDGE; + DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE; } else { reg = 0; connector->display_info.bus_flags |= - DRM_BUS_FLAG_PIXDATA_NEGEDGE; + DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE; } if (ili->conf->de_active_high) { reg |= ILI9322_POL_DE; diff --git a/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c b/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c index 75f925390551..198ab13f165d 100644 --- a/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c +++ b/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c @@ -331,7 +331,7 @@ static const struct seiko_panel_desc seiko_43wvf1g = { .height = 57, }, .bus_format = MEDIA_BUS_FMT_RGB888_1X24, - .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_NEGEDGE, + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE, }; static const struct of_device_id platform_of_match[] = { diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index 97964f7f2ace..49e851d06eb2 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -929,7 +929,7 @@ static const struct panel_desc dataimage_scf0700c48ggu18 = { .height = 91, }, .bus_format = MEDIA_BUS_FMT_RGB888_1X24, - .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE, + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE, }; static const struct display_timing dlc_dlc0700yzg_1_timing = { @@ -984,7 +984,7 @@ static const struct panel_desc edt_et057090dhu = { .height = 86, }, .bus_format = MEDIA_BUS_FMT_RGB666_1X18, - .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_NEGEDGE, + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE, }; static const struct drm_display_mode edt_etm0700g0dh6_mode = { @@ -1010,7 +1010,7 @@ static const struct panel_desc edt_etm0700g0dh6 = { .height = 91, }, .bus_format = MEDIA_BUS_FMT_RGB666_1X18, - .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_NEGEDGE, + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE, }; static const struct panel_desc edt_etm0700g0bdh6 = { @@ -1022,7 +1022,7 @@ static const struct panel_desc edt_etm0700g0bdh6 = { .height = 91, }, .bus_format = MEDIA_BUS_FMT_RGB666_1X18, - .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE, + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE, }; static const struct drm_display_mode foxlink_fl500wvr00_a0t_mode = { @@ -1176,7 +1176,7 @@ static const struct panel_desc innolux_at043tn24 = { .height = 54, }, .bus_format = MEDIA_BUS_FMT_RGB888_1X24, - .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE, + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE, }; static const struct drm_display_mode innolux_at070tn92_mode = { @@ -1658,7 +1658,7 @@ static const struct panel_desc nec_nl4827hc19_05b = { .height = 54, }, .bus_format = MEDIA_BUS_FMT_RGB888_1X24, - .bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE, + .bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE, }; static const struct drm_display_mode netron_dy_e231732_mode = { @@ -1707,7 +1707,7 @@ static const struct panel_desc newhaven_nhd_43_480272ef_atxl = { .height = 54, }, .bus_format = MEDIA_BUS_FMT_RGB888_1X24, - .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE | + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE | DRM_BUS_FLAG_SYNC_POSEDGE, }; @@ -1869,7 +1869,7 @@ static const struct panel_desc ortustech_com43h4m85ulc = { .height = 93, }, .bus_format = MEDIA_BUS_FMT_RGB888_1X24, - .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE, + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE, }; static const struct drm_display_mode qd43003c0_40_mode = { @@ -2214,7 +2214,7 @@ static const struct panel_desc toshiba_lt089ac29000 = { .height = 116, }, .bus_format = MEDIA_BUS_FMT_RGB888_1X24, - .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE, + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE, }; static const struct drm_display_mode tpk_f07a_0102_mode = { @@ -2237,7 +2237,7 @@ static const struct panel_desc tpk_f07a_0102 = { .width = 152, .height = 91, }, - .bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE, + .bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE, }; static const struct drm_display_mode tpk_f10a_0102_mode = { diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c index 754f6b25f265..0c5d391f0a8f 100644 --- a/drivers/gpu/drm/pl111/pl111_display.c +++ b/drivers/gpu/drm/pl111/pl111_display.c @@ -188,7 +188,7 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe, tim2 |= TIM2_IOE; if (connector->display_info.bus_flags & - DRM_BUS_FLAG_PIXDATA_NEGEDGE) + DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE) tim2 |= TIM2_IPC; } diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index c78cd35a1294..88dea125e76a 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -560,10 +560,10 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon, struct drm_connector *connector = panel->connector; struct drm_display_info display_info = connector->display_info; - if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_POSEDGE) + if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE) clk_set_phase(tcon->dclk, 240); - if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE) + if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE) clk_set_phase(tcon->dclk, 0); } diff --git a/drivers/gpu/drm/tve200/tve200_display.c b/drivers/gpu/drm/tve200/tve200_display.c index e8723a2412a6..d775d10dbe6a 100644 --- a/drivers/gpu/drm/tve200/tve200_display.c +++ b/drivers/gpu/drm/tve200/tve200_display.c @@ -149,7 +149,8 @@ static void tve200_display_enable(struct drm_simple_display_pipe *pipe, /* Vsync IRQ at start of Vsync at first */ ctrl1 |= TVE200_VSTSTYPE_VSYNC; - if (connector->display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE) + if (connector->display_info.bus_flags & + DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE) ctrl1 |= TVE200_CTRL_TVCLKP; if ((mode->hdisplay == 352 && mode->vdisplay == 240) || /* SIF(525) */ diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index bd850747ce54..8d12486181e6 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -246,10 +246,10 @@ struct drm_bridge_timings { /** * @sampling_edge: * - * Tells whether the bridge samples the digital input signal - * from the display engine on the positive or negative edge of the - * clock, this should reuse the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE - * bitwise flags from the DRM connector (bit 2 and 3 valid). + * Tells whether the bridge samples the digital input signals from the + * display engine on the positive or negative edge of the clock. This + * should use the DRM_BUS_FLAG_PIXDATA_SAMPLE_[POS|NEG]EDGE bitwise + * flags from the DRM connector (bit 2 and 3 valid). */ u32 sampling_edge; /**
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(-)