diff mbox series

[v2,3/4] drm: rcar-du: fix DSI enable & disable sequence

Message ID 20220822130513.119029-4-tomi.valkeinen@ideasonboard.com (mailing list archive)
State New, archived
Headers show
Series drm: rcar-du: DSI fixes | expand

Commit Message

Tomi Valkeinen Aug. 22, 2022, 1:05 p.m. UTC 
From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

The rcar crtc depends on the clock provided from the rcar DSI bridge.
When the DSI bridge is disabled, the clock is stopped, which causes the
crtc disable to timeout.

Also, while I have no issue with the enable, the documentation suggests
to enable the DSI before the crtc so that the crtc has its clock enabled
at enable time. This is also not done by the current driver.

To fix this, we need to keep the DSI bridge enabled until the crtc has
disabled itself, and enable the DSI bridge before crtc enables itself.

Add functions (rcar_mipi_dsi_atomic_early_enable and
rcar_mipi_dsi_atomic_late_disable) to the rcar DSI bridge driver which
the rcar driver can use to enable/disable the DSI clock when needed.
This is similar to what is already done with the rcar LVDS bridge.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c    | 26 +++++++++++++++++++
 drivers/gpu/drm/rcar-du/rcar_du_drv.h     |  2 ++
 drivers/gpu/drm/rcar-du/rcar_du_encoder.c |  4 +++
 drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c   | 25 +++++++++++++-----
 drivers/gpu/drm/rcar-du/rcar_mipi_dsi.h   | 31 +++++++++++++++++++++++
 5 files changed, 82 insertions(+), 6 deletions(-)
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_mipi_dsi.h

Comments

Laurent Pinchart Aug. 22, 2022, 1:40 p.m. UTC | #1 
Hi Tomi,

Thank you for the patch.

On Mon, Aug 22, 2022 at 04:05:11PM +0300, Tomi Valkeinen wrote:
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> The rcar crtc depends on the clock provided from the rcar DSI bridge.
> When the DSI bridge is disabled, the clock is stopped, which causes the
> crtc disable to timeout.
> 
> Also, while I have no issue with the enable, the documentation suggests
> to enable the DSI before the crtc so that the crtc has its clock enabled
> at enable time. This is also not done by the current driver.
> 
> To fix this, we need to keep the DSI bridge enabled until the crtc has
> disabled itself, and enable the DSI bridge before crtc enables itself.
> 
> Add functions (rcar_mipi_dsi_atomic_early_enable and
> rcar_mipi_dsi_atomic_late_disable) to the rcar DSI bridge driver which

The function names don't match the code. With this fixed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> the rcar driver can use to enable/disable the DSI clock when needed.
> This is similar to what is already done with the rcar LVDS bridge.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c    | 26 +++++++++++++++++++
>  drivers/gpu/drm/rcar-du/rcar_du_drv.h     |  2 ++
>  drivers/gpu/drm/rcar-du/rcar_du_encoder.c |  4 +++
>  drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c   | 25 +++++++++++++-----
>  drivers/gpu/drm/rcar-du/rcar_mipi_dsi.h   | 31 +++++++++++++++++++++++
>  5 files changed, 82 insertions(+), 6 deletions(-)
>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_mipi_dsi.h
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index fd3b94649a01..31e33270e6db 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -29,6 +29,7 @@
>  #include "rcar_du_regs.h"
>  #include "rcar_du_vsp.h"
>  #include "rcar_lvds.h"
> +#include "rcar_mipi_dsi.h"
>  
>  static u32 rcar_du_crtc_read(struct rcar_du_crtc *rcrtc, u32 reg)
>  {
> @@ -747,6 +748,18 @@ static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc,
>  		rcar_lvds_clk_enable(bridge, mode->clock * 1000);
>  	}
>  
> +	/*
> +	 * Similarly to LVDS, on V3U the dot clock is provided by the DSI
> +	 * encoder, and we need to enable the DSI clocks before enabling the CRTC.
> +	 */
> +	if ((rcdu->info->dsi_clk_mask & BIT(rcrtc->index)) &&
> +	    (rstate->outputs &
> +	     (BIT(RCAR_DU_OUTPUT_DSI0) | BIT(RCAR_DU_OUTPUT_DSI1)))) {
> +		struct drm_bridge *bridge = rcdu->dsi[rcrtc->index];
> +
> +		rcar_mipi_dsi_pclk_enable(bridge, state);
> +	}
> +
>  	rcar_du_crtc_start(rcrtc);
>  
>  	/*
> @@ -780,6 +793,19 @@ static void rcar_du_crtc_atomic_disable(struct drm_crtc *crtc,
>  		rcar_lvds_clk_disable(bridge);
>  	}
>  
> +	if ((rcdu->info->dsi_clk_mask & BIT(rcrtc->index)) &&
> +	    (rstate->outputs &
> +	     (BIT(RCAR_DU_OUTPUT_DSI0) | BIT(RCAR_DU_OUTPUT_DSI1)))) {
> +		struct drm_bridge *bridge = rcdu->dsi[rcrtc->index];
> +
> +		/*
> +		 * Disable the DSI clock output, see
> +		 * rcar_du_crtc_atomic_enable().
> +		 */
> +
> +		rcar_mipi_dsi_pclk_disable(bridge);
> +	}
> +
>  	spin_lock_irq(&crtc->dev->event_lock);
>  	if (crtc->state->event) {
>  		drm_crtc_send_vblank_event(crtc, crtc->state->event);
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> index bfad7775d9a1..9367c2f59431 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> @@ -91,6 +91,7 @@ struct rcar_du_device_info {
>  #define RCAR_DU_MAX_GROUPS		DIV_ROUND_UP(RCAR_DU_MAX_CRTCS, 2)
>  #define RCAR_DU_MAX_VSPS		4
>  #define RCAR_DU_MAX_LVDS		2
> +#define RCAR_DU_MAX_DSI			2
>  
>  struct rcar_du_device {
>  	struct device *dev;
> @@ -107,6 +108,7 @@ struct rcar_du_device {
>  	struct platform_device *cmms[RCAR_DU_MAX_CRTCS];
>  	struct rcar_du_vsp vsps[RCAR_DU_MAX_VSPS];
>  	struct drm_bridge *lvds[RCAR_DU_MAX_LVDS];
> +	struct drm_bridge *dsi[RCAR_DU_MAX_DSI];
>  
>  	struct {
>  		struct drm_property *colorkey;
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> index 60d6be78323b..ac93e08e8af4 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> @@ -84,6 +84,10 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
>  		if (output == RCAR_DU_OUTPUT_LVDS0 ||
>  		    output == RCAR_DU_OUTPUT_LVDS1)
>  			rcdu->lvds[output - RCAR_DU_OUTPUT_LVDS0] = bridge;
> +
> +		if (output == RCAR_DU_OUTPUT_DSI0 ||
> +		    output == RCAR_DU_OUTPUT_DSI1)
> +			rcdu->dsi[output - RCAR_DU_OUTPUT_DSI0] = bridge;
>  	}
>  
>  	/*
> diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> index 6a10a35f1122..b8fabf821a12 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> @@ -598,7 +598,22 @@ static int rcar_mipi_dsi_attach(struct drm_bridge *bridge,
>  static void rcar_mipi_dsi_atomic_enable(struct drm_bridge *bridge,
>  					struct drm_bridge_state *old_bridge_state)
>  {
> -	struct drm_atomic_state *state = old_bridge_state->base.state;
> +	struct rcar_mipi_dsi *dsi = bridge_to_rcar_mipi_dsi(bridge);
> +
> +	rcar_mipi_dsi_start_video(dsi);
> +}
> +
> +static void rcar_mipi_dsi_atomic_disable(struct drm_bridge *bridge,
> +					 struct drm_bridge_state *old_bridge_state)
> +{
> +	struct rcar_mipi_dsi *dsi = bridge_to_rcar_mipi_dsi(bridge);
> +
> +	rcar_mipi_dsi_stop_video(dsi);
> +}
> +
> +void rcar_mipi_dsi_pclk_enable(struct drm_bridge *bridge,
> +			       struct drm_atomic_state *state)
> +{
>  	struct rcar_mipi_dsi *dsi = bridge_to_rcar_mipi_dsi(bridge);
>  	const struct drm_display_mode *mode;
>  	struct drm_connector *connector;
> @@ -626,8 +641,6 @@ static void rcar_mipi_dsi_atomic_enable(struct drm_bridge *bridge,
>  	if (ret < 0)
>  		goto err_dsi_start_hs;
>  
> -	rcar_mipi_dsi_start_video(dsi);
> -
>  	return;
>  
>  err_dsi_start_hs:
> @@ -635,16 +648,16 @@ static void rcar_mipi_dsi_atomic_enable(struct drm_bridge *bridge,
>  err_dsi_startup:
>  	rcar_mipi_dsi_clk_disable(dsi);
>  }
> +EXPORT_SYMBOL_GPL(rcar_mipi_dsi_pclk_enable);
>  
> -static void rcar_mipi_dsi_atomic_disable(struct drm_bridge *bridge,
> -					 struct drm_bridge_state *old_bridge_state)
> +void rcar_mipi_dsi_pclk_disable(struct drm_bridge *bridge)
>  {
>  	struct rcar_mipi_dsi *dsi = bridge_to_rcar_mipi_dsi(bridge);
>  
> -	rcar_mipi_dsi_stop_video(dsi);
>  	rcar_mipi_dsi_shutdown(dsi);
>  	rcar_mipi_dsi_clk_disable(dsi);
>  }
> +EXPORT_SYMBOL_GPL(rcar_mipi_dsi_pclk_disable);
>  
>  static enum drm_mode_status
>  rcar_mipi_dsi_bridge_mode_valid(struct drm_bridge *bridge,
> diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.h b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.h
> new file mode 100644
> index 000000000000..ff759fc5edc7
> --- /dev/null
> +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.h
> @@ -0,0 +1,31 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * R-Car DSI Encoder
> + *
> + * Copyright (C) 2022 Renesas Electronics Corporation
> + *
> + * Contact: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> + */
> +
> +#ifndef __RCAR_MIPI_DSI_H__
> +#define __RCAR_MIPI_DSI_H__
> +
> +struct drm_atomic_state;
> +struct drm_bridge;
> +
> +#if IS_ENABLED(CONFIG_DRM_RCAR_MIPI_DSI)
> +void rcar_mipi_dsi_pclk_enable(struct drm_bridge *bridge,
> +			       struct drm_atomic_state *state);
> +void rcar_mipi_dsi_pclk_disable(struct drm_bridge *bridge);
> +#else
> +static inline void rcar_mipi_dsi_pclk_enable(struct drm_bridge *bridge,
> +					     struct drm_atomic_state *state)
> +{
> +}
> +
> +void rcar_mipi_dsi_pclk_disable(struct drm_bridge *bridge)
> +{
> +}
> +#endif /* CONFIG_DRM_RCAR_MIPI_DSI */
> +
> +#endif /* __RCAR_MIPI_DSI_H__ */
kernel test robot Aug. 22, 2022, 7:51 p.m. UTC | #2 
Hi Tomi,

I love your patch! Perhaps something to improve:

[auto build test WARNING on pinchartl-media/drm/du/next]
[also build test WARNING on linus/master v6.0-rc2 next-20220822]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Tomi-Valkeinen/drm-rcar-du-DSI-fixes/20220822-210756
base:   git://linuxtv.org/pinchartl/media.git drm/du/next
config: ia64-allyesconfig
compiler: ia64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/d037472981c443d699c022aa91c5335f686d82ad
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Tomi-Valkeinen/drm-rcar-du-DSI-fixes/20220822-210756
        git checkout d037472981c443d699c022aa91c5335f686d82ad
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/gpu/drm/rcar-du/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c:615:6: warning: no previous prototype for 'rcar_mipi_dsi_pclk_enable' [-Wmissing-prototypes]
     615 | void rcar_mipi_dsi_pclk_enable(struct drm_bridge *bridge,
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c:654:6: warning: no previous prototype for 'rcar_mipi_dsi_pclk_disable' [-Wmissing-prototypes]
     654 | void rcar_mipi_dsi_pclk_disable(struct drm_bridge *bridge)
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~
   {standard input}: Assembler messages:
   {standard input}:2025: Error: Register number out of range 0..5
   {standard input}:2029: Error: Register number out of range 0..5
   {standard input}:2274: Error: Register number out of range 0..2
   {standard input}:2274: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 38
   {standard input}:2270: Warning: This is the location of the conflicting usage
   {standard input}:2275: Error: Register number out of range 0..2
   {standard input}:2275: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 38
   {standard input}:2270: Warning: This is the location of the conflicting usage
   {standard input}:2275: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 38
   {standard input}:2274: Warning: This is the location of the conflicting usage
   {standard input}:2276: Error: Register number out of range 0..2
   {standard input}:2276: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 38
   {standard input}:2270: Warning: This is the location of the conflicting usage
   {standard input}:2276: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 38
   {standard input}:2274: Warning: This is the location of the conflicting usage
   {standard input}:2276: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 38
   {standard input}:2275: Warning: This is the location of the conflicting usage
   {standard input}:2279: Error: Register number out of range 0..2
   {standard input}:2280: Error: Register number out of range 0..2
   {standard input}:2280: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 38
   {standard input}:2279: Warning: This is the location of the conflicting usage


vim +/rcar_mipi_dsi_pclk_enable +615 drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c

   614	
 > 615	void rcar_mipi_dsi_pclk_enable(struct drm_bridge *bridge,
   616				       struct drm_atomic_state *state)
   617	{
   618		struct rcar_mipi_dsi *dsi = bridge_to_rcar_mipi_dsi(bridge);
   619		const struct drm_display_mode *mode;
   620		struct drm_connector *connector;
   621		struct drm_crtc *crtc;
   622		int ret;
   623	
   624		connector = drm_atomic_get_new_connector_for_encoder(state,
   625								     bridge->encoder);
   626		crtc = drm_atomic_get_new_connector_state(state, connector)->crtc;
   627		mode = &drm_atomic_get_new_crtc_state(state, crtc)->adjusted_mode;
   628	
   629		ret = rcar_mipi_dsi_clk_enable(dsi);
   630		if (ret < 0) {
   631			dev_err(dsi->dev, "failed to enable DSI clocks\n");
   632			return;
   633		}
   634	
   635		ret = rcar_mipi_dsi_startup(dsi, mode);
   636		if (ret < 0)
   637			goto err_dsi_startup;
   638	
   639		rcar_mipi_dsi_set_display_timing(dsi, mode);
   640	
   641		ret = rcar_mipi_dsi_start_hs_clock(dsi);
   642		if (ret < 0)
   643			goto err_dsi_start_hs;
   644	
   645		return;
   646	
   647	err_dsi_start_hs:
   648		rcar_mipi_dsi_shutdown(dsi);
   649	err_dsi_startup:
   650		rcar_mipi_dsi_clk_disable(dsi);
   651	}
   652	EXPORT_SYMBOL_GPL(rcar_mipi_dsi_pclk_enable);
   653	
 > 654	void rcar_mipi_dsi_pclk_disable(struct drm_bridge *bridge)
   655	{
   656		struct rcar_mipi_dsi *dsi = bridge_to_rcar_mipi_dsi(bridge);
   657	
   658		rcar_mipi_dsi_shutdown(dsi);
   659		rcar_mipi_dsi_clk_disable(dsi);
   660	}
   661	EXPORT_SYMBOL_GPL(rcar_mipi_dsi_pclk_disable);
   662
kernel test robot Aug. 23, 2022, 3:08 a.m. UTC | #3 
Hi Tomi,

I love your patch! Perhaps something to improve:

[auto build test WARNING on pinchartl-media/drm/du/next]
[also build test WARNING on linus/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Tomi-Valkeinen/drm-rcar-du-DSI-fixes/20220822-210756
base:   git://linuxtv.org/pinchartl/media.git drm/du/next
config: riscv-randconfig-r042-20220821 (https://download.01.org/0day-ci/archive/20220823/202208231031.hFwNDAuL-lkp@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project abce7acebd4c06c977bc4bd79170697f1122bc5e)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/d037472981c443d699c022aa91c5335f686d82ad
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Tomi-Valkeinen/drm-rcar-du-DSI-fixes/20220822-210756
        git checkout d037472981c443d699c022aa91c5335f686d82ad
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/gpu/drm/rcar-du/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c:10:
   In file included from include/linux/io.h:13:
   In file included from arch/riscv/include/asm/io.h:136:
   include/asm-generic/io.h:464:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:477:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:36:51: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
                                                     ^
   In file included from drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c:10:
   In file included from include/linux/io.h:13:
   In file included from arch/riscv/include/asm/io.h:136:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:34:51: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
                                                     ^
   In file included from drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c:10:
   In file included from include/linux/io.h:13:
   In file included from arch/riscv/include/asm/io.h:136:
   include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:1024:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
                                                     ~~~~~~~~~~ ^
>> drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c:615:6: warning: no previous prototype for function 'rcar_mipi_dsi_pclk_enable' [-Wmissing-prototypes]
   void rcar_mipi_dsi_pclk_enable(struct drm_bridge *bridge,
        ^
   drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c:615:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void rcar_mipi_dsi_pclk_enable(struct drm_bridge *bridge,
   ^
   static 
>> drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c:654:6: warning: no previous prototype for function 'rcar_mipi_dsi_pclk_disable' [-Wmissing-prototypes]
   void rcar_mipi_dsi_pclk_disable(struct drm_bridge *bridge)
        ^
   drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c:654:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void rcar_mipi_dsi_pclk_disable(struct drm_bridge *bridge)
   ^
   static 
   9 warnings generated.


vim +/rcar_mipi_dsi_pclk_enable +615 drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c

   614	
 > 615	void rcar_mipi_dsi_pclk_enable(struct drm_bridge *bridge,
   616				       struct drm_atomic_state *state)
   617	{
   618		struct rcar_mipi_dsi *dsi = bridge_to_rcar_mipi_dsi(bridge);
   619		const struct drm_display_mode *mode;
   620		struct drm_connector *connector;
   621		struct drm_crtc *crtc;
   622		int ret;
   623	
   624		connector = drm_atomic_get_new_connector_for_encoder(state,
   625								     bridge->encoder);
   626		crtc = drm_atomic_get_new_connector_state(state, connector)->crtc;
   627		mode = &drm_atomic_get_new_crtc_state(state, crtc)->adjusted_mode;
   628	
   629		ret = rcar_mipi_dsi_clk_enable(dsi);
   630		if (ret < 0) {
   631			dev_err(dsi->dev, "failed to enable DSI clocks\n");
   632			return;
   633		}
   634	
   635		ret = rcar_mipi_dsi_startup(dsi, mode);
   636		if (ret < 0)
   637			goto err_dsi_startup;
   638	
   639		rcar_mipi_dsi_set_display_timing(dsi, mode);
   640	
   641		ret = rcar_mipi_dsi_start_hs_clock(dsi);
   642		if (ret < 0)
   643			goto err_dsi_start_hs;
   644	
   645		return;
   646	
   647	err_dsi_start_hs:
   648		rcar_mipi_dsi_shutdown(dsi);
   649	err_dsi_startup:
   650		rcar_mipi_dsi_clk_disable(dsi);
   651	}
   652	EXPORT_SYMBOL_GPL(rcar_mipi_dsi_pclk_enable);
   653	
 > 654	void rcar_mipi_dsi_pclk_disable(struct drm_bridge *bridge)
   655	{
   656		struct rcar_mipi_dsi *dsi = bridge_to_rcar_mipi_dsi(bridge);
   657	
   658		rcar_mipi_dsi_shutdown(dsi);
   659		rcar_mipi_dsi_clk_disable(dsi);
   660	}
   661	EXPORT_SYMBOL_GPL(rcar_mipi_dsi_pclk_disable);
   662
kernel test robot Aug. 23, 2022, 7:08 p.m. UTC | #4 
Hi Tomi,

I love your patch! Perhaps something to improve:

[auto build test WARNING on pinchartl-media/drm/du/next]
[also build test WARNING on linus/master v6.0-rc2 next-20220823]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Tomi-Valkeinen/drm-rcar-du-DSI-fixes/20220822-210756
base:   git://linuxtv.org/pinchartl/media.git drm/du/next
config: arm-defconfig (https://download.01.org/0day-ci/archive/20220824/202208240201.ss8E6NY0-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/d037472981c443d699c022aa91c5335f686d82ad
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Tomi-Valkeinen/drm-rcar-du-DSI-fixes/20220822-210756
        git checkout d037472981c443d699c022aa91c5335f686d82ad
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/gpu/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from drivers/gpu/drm/rcar-du/rcar_du_crtc.c:34:
>> drivers/gpu/drm/rcar-du/rcar_mipi_dsi.h:26:6: warning: no previous prototype for 'rcar_mipi_dsi_pclk_disable' [-Wmissing-prototypes]
      26 | void rcar_mipi_dsi_pclk_disable(struct drm_bridge *bridge)
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~


vim +/rcar_mipi_dsi_pclk_disable +26 drivers/gpu/drm/rcar-du/rcar_mipi_dsi.h

    25	
  > 26	void rcar_mipi_dsi_pclk_disable(struct drm_bridge *bridge)
    27	{
    28	}
    29	#endif /* CONFIG_DRM_RCAR_MIPI_DSI */
    30
diff mbox series

Patch

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index fd3b94649a01..31e33270e6db 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -29,6 +29,7 @@ 
 #include "rcar_du_regs.h"
 #include "rcar_du_vsp.h"
 #include "rcar_lvds.h"
+#include "rcar_mipi_dsi.h"
 
 static u32 rcar_du_crtc_read(struct rcar_du_crtc *rcrtc, u32 reg)
 {
@@ -747,6 +748,18 @@  static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc,
 		rcar_lvds_clk_enable(bridge, mode->clock * 1000);
 	}
 
+	/*
+	 * Similarly to LVDS, on V3U the dot clock is provided by the DSI
+	 * encoder, and we need to enable the DSI clocks before enabling the CRTC.
+	 */
+	if ((rcdu->info->dsi_clk_mask & BIT(rcrtc->index)) &&
+	    (rstate->outputs &
+	     (BIT(RCAR_DU_OUTPUT_DSI0) | BIT(RCAR_DU_OUTPUT_DSI1)))) {
+		struct drm_bridge *bridge = rcdu->dsi[rcrtc->index];
+
+		rcar_mipi_dsi_pclk_enable(bridge, state);
+	}
+
 	rcar_du_crtc_start(rcrtc);
 
 	/*
@@ -780,6 +793,19 @@  static void rcar_du_crtc_atomic_disable(struct drm_crtc *crtc,
 		rcar_lvds_clk_disable(bridge);
 	}
 
+	if ((rcdu->info->dsi_clk_mask & BIT(rcrtc->index)) &&
+	    (rstate->outputs &
+	     (BIT(RCAR_DU_OUTPUT_DSI0) | BIT(RCAR_DU_OUTPUT_DSI1)))) {
+		struct drm_bridge *bridge = rcdu->dsi[rcrtc->index];
+
+		/*
+		 * Disable the DSI clock output, see
+		 * rcar_du_crtc_atomic_enable().
+		 */
+
+		rcar_mipi_dsi_pclk_disable(bridge);
+	}
+
 	spin_lock_irq(&crtc->dev->event_lock);
 	if (crtc->state->event) {
 		drm_crtc_send_vblank_event(crtc, crtc->state->event);
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
index bfad7775d9a1..9367c2f59431 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
@@ -91,6 +91,7 @@  struct rcar_du_device_info {
 #define RCAR_DU_MAX_GROUPS		DIV_ROUND_UP(RCAR_DU_MAX_CRTCS, 2)
 #define RCAR_DU_MAX_VSPS		4
 #define RCAR_DU_MAX_LVDS		2
+#define RCAR_DU_MAX_DSI			2
 
 struct rcar_du_device {
 	struct device *dev;
@@ -107,6 +108,7 @@  struct rcar_du_device {
 	struct platform_device *cmms[RCAR_DU_MAX_CRTCS];
 	struct rcar_du_vsp vsps[RCAR_DU_MAX_VSPS];
 	struct drm_bridge *lvds[RCAR_DU_MAX_LVDS];
+	struct drm_bridge *dsi[RCAR_DU_MAX_DSI];
 
 	struct {
 		struct drm_property *colorkey;
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
index 60d6be78323b..ac93e08e8af4 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
@@ -84,6 +84,10 @@  int rcar_du_encoder_init(struct rcar_du_device *rcdu,
 		if (output == RCAR_DU_OUTPUT_LVDS0 ||
 		    output == RCAR_DU_OUTPUT_LVDS1)
 			rcdu->lvds[output - RCAR_DU_OUTPUT_LVDS0] = bridge;
+
+		if (output == RCAR_DU_OUTPUT_DSI0 ||
+		    output == RCAR_DU_OUTPUT_DSI1)
+			rcdu->dsi[output - RCAR_DU_OUTPUT_DSI0] = bridge;
 	}
 
 	/*
diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
index 6a10a35f1122..b8fabf821a12 100644
--- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
+++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
@@ -598,7 +598,22 @@  static int rcar_mipi_dsi_attach(struct drm_bridge *bridge,
 static void rcar_mipi_dsi_atomic_enable(struct drm_bridge *bridge,
 					struct drm_bridge_state *old_bridge_state)
 {
-	struct drm_atomic_state *state = old_bridge_state->base.state;
+	struct rcar_mipi_dsi *dsi = bridge_to_rcar_mipi_dsi(bridge);
+
+	rcar_mipi_dsi_start_video(dsi);
+}
+
+static void rcar_mipi_dsi_atomic_disable(struct drm_bridge *bridge,
+					 struct drm_bridge_state *old_bridge_state)
+{
+	struct rcar_mipi_dsi *dsi = bridge_to_rcar_mipi_dsi(bridge);
+
+	rcar_mipi_dsi_stop_video(dsi);
+}
+
+void rcar_mipi_dsi_pclk_enable(struct drm_bridge *bridge,
+			       struct drm_atomic_state *state)
+{
 	struct rcar_mipi_dsi *dsi = bridge_to_rcar_mipi_dsi(bridge);
 	const struct drm_display_mode *mode;
 	struct drm_connector *connector;
@@ -626,8 +641,6 @@  static void rcar_mipi_dsi_atomic_enable(struct drm_bridge *bridge,
 	if (ret < 0)
 		goto err_dsi_start_hs;
 
-	rcar_mipi_dsi_start_video(dsi);
-
 	return;
 
 err_dsi_start_hs:
@@ -635,16 +648,16 @@  static void rcar_mipi_dsi_atomic_enable(struct drm_bridge *bridge,
 err_dsi_startup:
 	rcar_mipi_dsi_clk_disable(dsi);
 }
+EXPORT_SYMBOL_GPL(rcar_mipi_dsi_pclk_enable);
 
-static void rcar_mipi_dsi_atomic_disable(struct drm_bridge *bridge,
-					 struct drm_bridge_state *old_bridge_state)
+void rcar_mipi_dsi_pclk_disable(struct drm_bridge *bridge)
 {
 	struct rcar_mipi_dsi *dsi = bridge_to_rcar_mipi_dsi(bridge);
 
-	rcar_mipi_dsi_stop_video(dsi);
 	rcar_mipi_dsi_shutdown(dsi);
 	rcar_mipi_dsi_clk_disable(dsi);
 }
+EXPORT_SYMBOL_GPL(rcar_mipi_dsi_pclk_disable);
 
 static enum drm_mode_status
 rcar_mipi_dsi_bridge_mode_valid(struct drm_bridge *bridge,
diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.h b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.h
new file mode 100644
index 000000000000..ff759fc5edc7
--- /dev/null
+++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.h
@@ -0,0 +1,31 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * R-Car DSI Encoder
+ *
+ * Copyright (C) 2022 Renesas Electronics Corporation
+ *
+ * Contact: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
+ */
+
+#ifndef __RCAR_MIPI_DSI_H__
+#define __RCAR_MIPI_DSI_H__
+
+struct drm_atomic_state;
+struct drm_bridge;
+
+#if IS_ENABLED(CONFIG_DRM_RCAR_MIPI_DSI)
+void rcar_mipi_dsi_pclk_enable(struct drm_bridge *bridge,
+			       struct drm_atomic_state *state);
+void rcar_mipi_dsi_pclk_disable(struct drm_bridge *bridge);
+#else
+static inline void rcar_mipi_dsi_pclk_enable(struct drm_bridge *bridge,
+					     struct drm_atomic_state *state)
+{
+}
+
+void rcar_mipi_dsi_pclk_disable(struct drm_bridge *bridge)
+{
+}
+#endif /* CONFIG_DRM_RCAR_MIPI_DSI */
+
+#endif /* __RCAR_MIPI_DSI_H__ */