Hi Laurent, On 14/09/18 10:10, Laurent Pinchart wrote: > The official way to stop the display is to clear the display enable > (DEN) bit in the DSYSR register, but that operates at a group level and > affects the two channels in the group. To disable channels selectively, > the driver uses TV sync mode that stops display operation on the channel > and turns output signals into inputs. > > While TV sync mode is available in all DU models currently supported, > the D3 and E3 DUs don't support it. We will thus need to find an > alternative way to turn channels off. > > In the meantime, condition the switch to TV sync mode to the > availability of the feature, to avoid writing an invalid value to the > DSYSR register. (Perhaps for leading into the next sentence) When the feature is unavailable ... > The display output will turn blank as all planes are > disabled when stopping the CRTC. Otherwise your sentence sounds like it will affect existing platforms. > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx> > Tested-by: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx> But that's a minor and possibly not relevant nit so: Reviewed-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 7 ++++++- > drivers/gpu/drm/rcar-du/rcar_du_drv.c | 33 ++++++++++++++++++++++----------- > drivers/gpu/drm/rcar-du/rcar_du_drv.h | 1 + > 3 files changed, 29 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > index f827fccf6416..17741843cf51 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > @@ -643,8 +643,13 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc) > /* > * Select switch sync mode. This stops display operation and configures > * the HSYNC and VSYNC signals as inputs. > + * > + * TODO: Find another way to stop the display for DUs that don't support > + * TVM sync. > */ > - rcar_du_crtc_dsysr_clr_set(rcrtc, DSYSR_TVM_MASK, DSYSR_TVM_SWITCH); > + if (rcar_du_has(rcrtc->group->dev, RCAR_DU_FEATURE_TVM_SYNC)) > + rcar_du_crtc_dsysr_clr_set(rcrtc, DSYSR_TVM_MASK, > + DSYSR_TVM_SWITCH); > > rcar_du_group_start_stop(rcrtc->group, false); > } > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c > index 0954ecd2f943..fa0d381c2d0f 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c > @@ -36,7 +36,8 @@ static const struct rcar_du_device_info rzg1_du_r8a7743_info = { > .gen = 2, > .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK > | RCAR_DU_FEATURE_EXT_CTRL_REGS > - | RCAR_DU_FEATURE_INTERLACED, > + | RCAR_DU_FEATURE_INTERLACED > + | RCAR_DU_FEATURE_TVM_SYNC, > .channels_mask = BIT(1) | BIT(0), > .routes = { > /* > @@ -58,7 +59,8 @@ static const struct rcar_du_device_info rzg1_du_r8a7745_info = { > .gen = 2, > .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK > | RCAR_DU_FEATURE_EXT_CTRL_REGS > - | RCAR_DU_FEATURE_INTERLACED, > + | RCAR_DU_FEATURE_INTERLACED > + | RCAR_DU_FEATURE_TVM_SYNC, > .channels_mask = BIT(1) | BIT(0), > .routes = { > /* > @@ -77,7 +79,8 @@ static const struct rcar_du_device_info rzg1_du_r8a7745_info = { > > static const struct rcar_du_device_info rcar_du_r8a7779_info = { > .gen = 2, > - .features = RCAR_DU_FEATURE_INTERLACED, > + .features = RCAR_DU_FEATURE_INTERLACED > + | RCAR_DU_FEATURE_TVM_SYNC, > .channels_mask = BIT(1) | BIT(0), > .routes = { > /* > @@ -99,7 +102,8 @@ static const struct rcar_du_device_info rcar_du_r8a7790_info = { > .gen = 2, > .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK > | RCAR_DU_FEATURE_EXT_CTRL_REGS > - | RCAR_DU_FEATURE_INTERLACED, > + | RCAR_DU_FEATURE_INTERLACED > + | RCAR_DU_FEATURE_TVM_SYNC, > .quirks = RCAR_DU_QUIRK_ALIGN_128B, > .channels_mask = BIT(2) | BIT(1) | BIT(0), > .routes = { > @@ -128,7 +132,8 @@ static const struct rcar_du_device_info rcar_du_r8a7791_info = { > .gen = 2, > .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK > | RCAR_DU_FEATURE_EXT_CTRL_REGS > - | RCAR_DU_FEATURE_INTERLACED, > + | RCAR_DU_FEATURE_INTERLACED > + | RCAR_DU_FEATURE_TVM_SYNC, > .channels_mask = BIT(1) | BIT(0), > .routes = { > /* > @@ -151,7 +156,8 @@ static const struct rcar_du_device_info rcar_du_r8a7792_info = { > .gen = 2, > .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK > | RCAR_DU_FEATURE_EXT_CTRL_REGS > - | RCAR_DU_FEATURE_INTERLACED, > + | RCAR_DU_FEATURE_INTERLACED > + | RCAR_DU_FEATURE_TVM_SYNC, > .channels_mask = BIT(1) | BIT(0), > .routes = { > /* R8A7792 has two RGB outputs. */ > @@ -170,7 +176,8 @@ static const struct rcar_du_device_info rcar_du_r8a7794_info = { > .gen = 2, > .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK > | RCAR_DU_FEATURE_EXT_CTRL_REGS > - | RCAR_DU_FEATURE_INTERLACED, > + | RCAR_DU_FEATURE_INTERLACED > + | RCAR_DU_FEATURE_TVM_SYNC, > .channels_mask = BIT(1) | BIT(0), > .routes = { > /* > @@ -193,7 +200,8 @@ static const struct rcar_du_device_info rcar_du_r8a7795_info = { > .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK > | RCAR_DU_FEATURE_EXT_CTRL_REGS > | RCAR_DU_FEATURE_VSP1_SOURCE > - | RCAR_DU_FEATURE_INTERLACED, > + | RCAR_DU_FEATURE_INTERLACED > + | RCAR_DU_FEATURE_TVM_SYNC, > .channels_mask = BIT(3) | BIT(2) | BIT(1) | BIT(0), > .routes = { > /* > @@ -226,7 +234,8 @@ static const struct rcar_du_device_info rcar_du_r8a7796_info = { > .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK > | RCAR_DU_FEATURE_EXT_CTRL_REGS > | RCAR_DU_FEATURE_VSP1_SOURCE > - | RCAR_DU_FEATURE_INTERLACED, > + | RCAR_DU_FEATURE_INTERLACED > + | RCAR_DU_FEATURE_TVM_SYNC, > .channels_mask = BIT(2) | BIT(1) | BIT(0), > .routes = { > /* > @@ -255,7 +264,8 @@ static const struct rcar_du_device_info rcar_du_r8a77965_info = { > .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK > | RCAR_DU_FEATURE_EXT_CTRL_REGS > | RCAR_DU_FEATURE_VSP1_SOURCE > - | RCAR_DU_FEATURE_INTERLACED, > + | RCAR_DU_FEATURE_INTERLACED > + | RCAR_DU_FEATURE_TVM_SYNC, > .channels_mask = BIT(3) | BIT(1) | BIT(0), > .routes = { > /* > @@ -284,7 +294,8 @@ static const struct rcar_du_device_info rcar_du_r8a77970_info = { > .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK > | RCAR_DU_FEATURE_EXT_CTRL_REGS > | RCAR_DU_FEATURE_VSP1_SOURCE > - | RCAR_DU_FEATURE_INTERLACED, > + | RCAR_DU_FEATURE_INTERLACED > + | RCAR_DU_FEATURE_TVM_SYNC, > .channels_mask = BIT(0), > .routes = { > /* R8A77970 has one RGB output and one LVDS output. */ > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h > index ebba9aefba6a..143c037e2c0f 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h > @@ -27,6 +27,7 @@ struct rcar_du_device; > #define RCAR_DU_FEATURE_EXT_CTRL_REGS BIT(1) /* Has extended control registers */ > #define RCAR_DU_FEATURE_VSP1_SOURCE BIT(2) /* Has inputs from VSP1 */ > #define RCAR_DU_FEATURE_INTERLACED BIT(3) /* HW supports interlaced */ > +#define RCAR_DU_FEATURE_TVM_SYNC BIT(4) /* Has TV switch/sync modes */ > > #define RCAR_DU_QUIRK_ALIGN_128B BIT(0) /* Align pitches to 128 bytes */ > > -- Regards -- Kieran