Re: [PATCH v2 0/7] drm: rcar-du: Misc patches

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

 



Hi Tomi,

On Fri, Jan 20, 2023 at 10:50:02AM +0200, Tomi Valkeinen wrote:
> From: Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx>
> 
> Hi,
> 
> v2 of the series. Diff to v1 can be found below.
> 
> The main changes are:
> - Runtime PM support for LVDS
> - Skip rcar_du_group_setup() for gen4+
> 
>  Tomi
> 
> Koji Matsuoka (1):
>   drm: rcar-du: lvds: Fix stop sequence
> 
> Tomi Valkeinen (6):
>   drm: rcar-du: dsi: add 'select RESET_CONTROLLER'
>   drm: rcar-du: lvds: Add runtime PM
>   drm: rcar-du: lvsd: Add reset control
>   drm: rcar-du: Add quirk for H3 ES1.x pclk workaround
>   drm: rcar-du: Fix setting a reserved bit in DPLLCR
>   drm: rcar-du: Stop accessing non-existant registers on gen4

For patches 1/7 to 6/7,

Tested-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>

on D3, H3 ES2 and M3N.

I wanted to test this on H3 ES1.x too, but the boot loader on that board
is ancient and doesn't fit with my workflow anymore. I fear updating
them would be a deep rabbit hole. If I find spare time, I'll give it a
try.

>  drivers/gpu/drm/rcar-du/Kconfig         |  2 +
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c  | 39 +++++++----
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c   | 49 ++++++++++++++
>  drivers/gpu/drm/rcar-du/rcar_du_drv.h   |  2 +
>  drivers/gpu/drm/rcar-du/rcar_du_group.c | 11 +++-
>  drivers/gpu/drm/rcar-du/rcar_du_regs.h  |  8 +--
>  drivers/gpu/drm/rcar-du/rcar_lvds.c     | 86 +++++++++++++++++++++++--
>  7 files changed, 169 insertions(+), 28 deletions(-)
> 
> Interdiff against v1:
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index 56b23333993c..008e172ed43b 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -249,15 +249,24 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc)
>  			dpllcr |= DPLLCR_PLCS1
>  			       |  DPLLCR_INCS_DOTCLKIN1;
>  		} else {
> -			dpllcr |= DPLLCR_PLCS0
> +			dpllcr |= DPLLCR_PLCS0_PLL
>  			       |  DPLLCR_INCS_DOTCLKIN0;
> +
>  			/*
> -			 * On H3 ES1.x, in addition to setting bit 21 (PLCS0),
> -			 * also bit 20 has to be set to select PLL1 as the
> -			 * clock source.
> +			 * On ES2.x we have a single mux controlled via bit 21,
> +			 * which selects between DCLKIN source (bit 21 = 0) and
> +			 * a PLL source (bit 21 = 1), where the PLL is always
> +			 * PLL1.
> +			 *
> +			 * On ES1.x we have an additional mux, controlled
> +			 * via bit 20, for choosing between PLL0 (bit 20 = 0)
> +			 * and PLL1 (bit 20 = 1). We always want to use PLL1,
> +			 * so on ES1.x, in addition to setting bit 21, we need
> +			 * to set the bit 20.
>  			 */
> +
>  			if (rcdu->info->quirks & RCAR_DU_QUIRK_H3_ES1_PLL)
> -				dpllcr |= DPLLCR_PLCS0_H3ES1X_PLL1_SEL;
> +				dpllcr |= DPLLCR_PLCS0_H3ES1X_PLL1;
>  		}
>  
>  		rcar_du_group_write(rcrtc->group, DPLLCR, dpllcr);
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> index d689f2510081..82f9718ff500 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -710,10 +710,10 @@ static void rcar_du_shutdown(struct platform_device *pdev)
>  
>  static int rcar_du_probe(struct platform_device *pdev)
>  {
> +	const struct soc_device_attribute *soc_attr;
>  	struct rcar_du_device *rcdu;
>  	unsigned int mask;
>  	int ret;
> -	const struct soc_device_attribute *soc_attr;
>  
>  	if (drm_firmware_drivers_only())
>  		return -ENODEV;
> @@ -726,13 +726,12 @@ static int rcar_du_probe(struct platform_device *pdev)
>  
>  	rcdu->dev = &pdev->dev;
>  
> +	rcdu->info = of_device_get_match_data(rcdu->dev);
> +
>  	soc_attr = soc_device_match(rcar_du_soc_table);
>  	if (soc_attr)
>  		rcdu->info = soc_attr->data;
>  
> -	if (!rcdu->info)
> -		rcdu->info = of_device_get_match_data(rcdu->dev);
> -
>  	platform_set_drvdata(pdev, rcdu);
>  
>  	/* I/O resources */
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> index c236e2aa8a01..c2209d427bb7 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> @@ -194,10 +194,13 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
>   */
>  int rcar_du_group_get(struct rcar_du_group *rgrp)
>  {
> +	struct rcar_du_device *rcdu = rgrp->dev;
> +
>  	if (rgrp->use_count)
>  		goto done;
>  
> -	rcar_du_group_setup(rgrp);
> +	if (rcdu->info->gen < 4)
> +		rcar_du_group_setup(rgrp);
>  
>  done:
>  	rgrp->use_count++;
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_regs.h b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> index 94d913f66c8f..789ae9285108 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> @@ -283,13 +283,8 @@
>  #define DPLLCR			0x20044
>  #define DPLLCR_CODE		(0x95 << 24)
>  #define DPLLCR_PLCS1		(1 << 23)
> -/*
> - * PLCS0 is bit 21, but H3 ES1.x requires bit 20 to be set as well. As bit 20
> - * isn't implemented by other SoC in the Gen3 family it can safely be set
> - * unconditionally.
> - */
> -#define DPLLCR_PLCS0		(1 << 21)
> -#define DPLLCR_PLCS0_H3ES1X_PLL1_SEL	(1 << 20)
> +#define DPLLCR_PLCS0_PLL	(1 << 21)
> +#define DPLLCR_PLCS0_H3ES1X_PLL1	(1 << 20)
>  #define DPLLCR_CLKE		(1 << 18)
>  #define DPLLCR_FDPLL(n)		((n) << 12)
>  #define DPLLCR_N(n)		((n) << 5)
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index 01800cef1c33..8fa5f7400179 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -16,6 +16,7 @@
>  #include <linux/of_device.h>
>  #include <linux/of_graph.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/reset.h>
>  #include <linux/slab.h>
>  #include <linux/sys_soc.h>
> @@ -82,14 +83,14 @@ struct rcar_lvds {
>  #define bridge_to_rcar_lvds(b) \
>  	container_of(b, struct rcar_lvds, bridge)
>  
> -static void rcar_lvds_write(struct rcar_lvds *lvds, u32 reg, u32 data)
> +static u32 rcar_lvds_read(struct rcar_lvds *lvds, u32 reg)
>  {
> -	iowrite32(data, lvds->mmio + reg);
> +	return ioread32(lvds->mmio + reg);
>  }
>  
> -static u32 rcar_lvds_read(struct rcar_lvds *lvds, u32 reg)
> +static void rcar_lvds_write(struct rcar_lvds *lvds, u32 reg, u32 data)
>  {
> -	return ioread32(lvds->mmio + reg);
> +	iowrite32(data, lvds->mmio + reg);
>  }
>  
>  /* -----------------------------------------------------------------------------
> @@ -323,10 +324,8 @@ int rcar_lvds_pclk_enable(struct drm_bridge *bridge, unsigned long freq)
>  
>  	dev_dbg(lvds->dev, "enabling LVDS PLL, freq=%luHz\n", freq);
>  
> -	reset_control_deassert(lvds->rstc);
> -
> -	ret = clk_prepare_enable(lvds->clocks.mod);
> -	if (ret < 0)
> +	ret = pm_runtime_resume_and_get(lvds->dev);
> +	if (ret)
>  		return ret;
>  
>  	__rcar_lvds_pll_setup_d3_e3(lvds, freq, true);
> @@ -346,9 +345,7 @@ void rcar_lvds_pclk_disable(struct drm_bridge *bridge)
>  
>  	rcar_lvds_write(lvds, LVDPLLCR, 0);
>  
> -	clk_disable_unprepare(lvds->clocks.mod);
> -
> -	reset_control_assert(lvds->rstc);
> +	pm_runtime_put(lvds->dev);
>  }
>  EXPORT_SYMBOL_GPL(rcar_lvds_pclk_disable);
>  
> @@ -407,10 +404,8 @@ static void __rcar_lvds_atomic_enable(struct drm_bridge *bridge,
>  	u32 lvdcr0;
>  	int ret;
>  
> -	reset_control_deassert(lvds->rstc);
> -
> -	ret = clk_prepare_enable(lvds->clocks.mod);
> -	if (ret < 0)
> +	ret = pm_runtime_resume_and_get(lvds->dev);
> +	if (ret)
>  		return;
>  
>  	/* Enable the companion LVDS encoder in dual-link mode. */
> @@ -576,6 +571,7 @@ static void rcar_lvds_atomic_disable(struct drm_bridge *bridge,
>  		rcar_lvds_write(lvds, LVDCR0, lvdcr0);
>  	}
>  
> +	rcar_lvds_write(lvds, LVDCR0, 0);
>  	rcar_lvds_write(lvds, LVDCR1, 0);
>  	rcar_lvds_write(lvds, LVDPLLCR, 0);
>  
> @@ -584,8 +580,7 @@ static void rcar_lvds_atomic_disable(struct drm_bridge *bridge,
>  		lvds->companion->funcs->atomic_disable(lvds->companion,
>  						       old_bridge_state);
>  
> -	clk_disable_unprepare(lvds->clocks.mod);
> -	reset_control_assert(lvds->rstc);
> +	pm_runtime_put(lvds->dev);
>  }
>  
>  static bool rcar_lvds_mode_fixup(struct drm_bridge *bridge,
> @@ -878,11 +873,11 @@ static int rcar_lvds_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		return ret;
>  
> -	lvds->rstc = devm_reset_control_get(&pdev->dev, NULL);
> -	if (IS_ERR(lvds->rstc)) {
> -		dev_err(&pdev->dev, "failed to get cpg reset\n");
> -		return PTR_ERR(lvds->rstc);
> -	}
> +	lvds->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
> +	if (IS_ERR(lvds->rstc))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(lvds->rstc),
> +				     "failed to get cpg reset\n");
> +	pm_runtime_enable(&pdev->dev);
>  
>  	drm_bridge_add(&lvds->bridge);
>  
> @@ -895,6 +890,8 @@ static int rcar_lvds_remove(struct platform_device *pdev)
>  
>  	drm_bridge_remove(&lvds->bridge);
>  
> +	pm_runtime_disable(&pdev->dev);
> +
>  	return 0;
>  }
>  
> @@ -953,11 +950,48 @@ static const struct of_device_id rcar_lvds_of_table[] = {
>  
>  MODULE_DEVICE_TABLE(of, rcar_lvds_of_table);
>  
> +static int rcar_lvds_runtime_suspend(struct device *dev)
> +{
> +	struct rcar_lvds *lvds = dev_get_drvdata(dev);
> +
> +	clk_disable_unprepare(lvds->clocks.mod);
> +
> +	reset_control_assert(lvds->rstc);
> +
> +	return 0;
> +}
> +
> +static int rcar_lvds_runtime_resume(struct device *dev)
> +{
> +	struct rcar_lvds *lvds = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = reset_control_deassert(lvds->rstc);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_prepare_enable(lvds->clocks.mod);
> +	if (ret < 0)
> +		goto err_reset_assert;
> +
> +	return 0;
> +
> +err_reset_assert:
> +	reset_control_assert(lvds->rstc);
> +
> +	return ret;
> +}
> +
> +static const struct dev_pm_ops rcar_lvds_pm_ops = {
> +	SET_RUNTIME_PM_OPS(rcar_lvds_runtime_suspend, rcar_lvds_runtime_resume, NULL)
> +};
> +
>  static struct platform_driver rcar_lvds_platform_driver = {
>  	.probe		= rcar_lvds_probe,
>  	.remove		= rcar_lvds_remove,
>  	.driver		= {
>  		.name	= "rcar-lvds",
> +		.pm	= &rcar_lvds_pm_ops,
>  		.of_match_table = rcar_lvds_of_table,
>  	},
>  };

-- 
Regards,

Laurent Pinchart



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux