Re: [PATCH v2 10/16] drm: rcar-du: Don't use TV sync mode when not supported by the hardware

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux