[PATCH 0/4] drm: rcar-du: Add support for R8A779H0

[PATCH 0/4] drm: rcar-du: Add support for R8A779H0

[Jacopo Mondi]

This series upports from Renesas BSP at revision rcar-5.2.0.rc18 the initial
display support for the V4M SoC (R8A779H0).

The series includes two small bugfixes and then adds support for the
V4M SoC to the R-Car DU module and R-Car DSI encoder.

Compile-tested only as I don't have a Gray Hawk board available yet.
Once the board will be available, DTS integration will follow.

Jacopo Mondi (2):
  drm: rcar-mipi-dsi: Add support for R8A779H0
  drm: rcar-du: Add support for R8A779H0

Phong Hoang (1):
  drm: ti-sn65dsi86: Check bridge connection failure

Takeshi Kihara (1):
  drm: rcar-mipi-dsi: Fix CLOCKSET1_LOCK definition

 .../display/bridge/renesas,dsi-csi2-tx.yaml    |  1 +
 .../bindings/display/renesas,du.yaml           |  1 +
 drivers/gpu/drm/bridge/ti-sn65dsi86.c          |  6 +++++-
 drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c  | 18 ++++++++++++++++++
 .../gpu/drm/renesas/rcar-du/rcar_du_group.c    | 17 ++++++++++++-----
 .../gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c    |  1 +
 .../drm/renesas/rcar-du/rcar_mipi_dsi_regs.h   |  3 +--
 7 files changed, 39 insertions(+), 8 deletions(-)

--
2.45.2

[PATCH 1/4] drm: rcar-mipi-dsi: Fix CLOCKSET1_LOCK definition

[Jacopo Mondi]

From: Takeshi Kihara <takeshi.kihara.df@renesas.com>

Version 0.51 of the Renesas R-Car Gen4 TRM reports bit 16 of the
CLOCKSET1 register of the DSI transmitter module to be a reserved
field.

Fix this by correcting the CLOCKSET1_LOCK definition to match the TRM
and remove the CLOCKSET1_LOCK_PHY definition, as the register is simply
called "lock" in the datasheet.

Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h
index f8114d11f2d1..1bf9c4717d5a 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h
@@ -141,8 +141,7 @@
 #define PHYSETUP_RSTZ			(1 << 0)
 
 #define CLOCKSET1			0x101c
-#define CLOCKSET1_LOCK_PHY		(1 << 17)
-#define CLOCKSET1_LOCK			(1 << 16)
+#define CLOCKSET1_LOCK			(1 << 17)
 #define CLOCKSET1_CLKSEL		(1 << 8)
 #define CLOCKSET1_CLKINSEL_EXTAL	(0 << 2)
 #define CLOCKSET1_CLKINSEL_DIG		(1 << 2)
-- 
2.45.2

Re: [PATCH 1/4] drm: rcar-mipi-dsi: Fix CLOCKSET1_LOCK definition

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

CC'ing Tomi.

On Wed, Jun 19, 2024 at 12:22:15PM +0200, Jacopo Mondi wrote:
> From: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> 
> Version 0.51 of the Renesas R-Car Gen4 TRM reports bit 16 of the
> CLOCKSET1 register of the DSI transmitter module to be a reserved
> field.
> 
> Fix this by correcting the CLOCKSET1_LOCK definition to match the TRM
> and remove the CLOCKSET1_LOCK_PHY definition, as the register is simply
> called "lock" in the datasheet.
> 
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h
> index f8114d11f2d1..1bf9c4717d5a 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h
> @@ -141,8 +141,7 @@
>  #define PHYSETUP_RSTZ			(1 << 0)
>  
>  #define CLOCKSET1			0x101c
> -#define CLOCKSET1_LOCK_PHY		(1 << 17)
> -#define CLOCKSET1_LOCK			(1 << 16)
> +#define CLOCKSET1_LOCK			(1 << 17)

This matches the documentation, but we should get it tested on V4H to
make sure it doesn't cause a regression. Tomi, would you be able to test
the patch ?

>  #define CLOCKSET1_CLKSEL		(1 << 8)
>  #define CLOCKSET1_CLKINSEL_EXTAL	(0 << 2)
>  #define CLOCKSET1_CLKINSEL_DIG		(1 << 2)

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/4] drm: rcar-mipi-dsi: Fix CLOCKSET1_LOCK definition

[Tomi Valkeinen]

On 19/06/2024 22:29, Laurent Pinchart wrote:
> Hi Jacopo,
> 
> Thank you for the patch.
> 
> CC'ing Tomi.
> 
> On Wed, Jun 19, 2024 at 12:22:15PM +0200, Jacopo Mondi wrote:
>> From: Takeshi Kihara <takeshi.kihara.df@renesas.com>
>>
>> Version 0.51 of the Renesas R-Car Gen4 TRM reports bit 16 of the
>> CLOCKSET1 register of the DSI transmitter module to be a reserved
>> field.
>>
>> Fix this by correcting the CLOCKSET1_LOCK definition to match the TRM
>> and remove the CLOCKSET1_LOCK_PHY definition, as the register is simply
>> called "lock" in the datasheet.
>>
>> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
>> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>> ---
>>   drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h
>> index f8114d11f2d1..1bf9c4717d5a 100644
>> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h
>> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h
>> @@ -141,8 +141,7 @@
>>   #define PHYSETUP_RSTZ			(1 << 0)
>>   
>>   #define CLOCKSET1			0x101c
>> -#define CLOCKSET1_LOCK_PHY		(1 << 17)
>> -#define CLOCKSET1_LOCK			(1 << 16)
>> +#define CLOCKSET1_LOCK			(1 << 17)
> 
> This matches the documentation, but we should get it tested on V4H to
> make sure it doesn't cause a regression. Tomi, would you be able to test
> the patch ?

Works for me.

Tested-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

  Tomi

[PATCH 2/4] drm: ti-sn65dsi86: Check bridge connection failure

[Jacopo Mondi]

From: Phong Hoang <phong.hoang.wz@renesas.com>

Add a check to the register access function when attaching a bridge
device.

Signed-off-by: Phong Hoang <phong.hoang.wz@renesas.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 84698a0b27a8..b7df53577987 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -696,6 +696,7 @@ static struct ti_sn65dsi86 *bridge_to_ti_sn65dsi86(struct drm_bridge *bridge)
 
 static int ti_sn_attach_host(struct auxiliary_device *adev, struct ti_sn65dsi86 *pdata)
 {
+	int ret;
 	int val;
 	struct mipi_dsi_host *host;
 	struct mipi_dsi_device *dsi;
@@ -720,8 +721,11 @@ static int ti_sn_attach_host(struct auxiliary_device *adev, struct ti_sn65dsi86
 
 	/* check if continuous dsi clock is required or not */
 	pm_runtime_get_sync(dev);
-	regmap_read(pdata->regmap, SN_DPPLL_SRC_REG, &val);
+	ret = regmap_read(pdata->regmap, SN_DPPLL_SRC_REG, &val);
 	pm_runtime_put_autosuspend(dev);
+	if (ret)
+		return ret;
+
 	if (!(val & DPPLL_CLK_SRC_DSICLK))
 		dsi->mode_flags |= MIPI_DSI_CLOCK_NON_CONTINUOUS;
 
-- 
2.45.2

Re: [PATCH 2/4] drm: ti-sn65dsi86: Check bridge connection failure

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Wed, Jun 19, 2024 at 12:22:16PM +0200, Jacopo Mondi wrote:
> From: Phong Hoang <phong.hoang.wz@renesas.com>
> 
> Add a check to the register access function when attaching a bridge
> device.
> 
> Signed-off-by: Phong Hoang <phong.hoang.wz@renesas.com>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

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

> ---
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index 84698a0b27a8..b7df53577987 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -696,6 +696,7 @@ static struct ti_sn65dsi86 *bridge_to_ti_sn65dsi86(struct drm_bridge *bridge)
>  
>  static int ti_sn_attach_host(struct auxiliary_device *adev, struct ti_sn65dsi86 *pdata)
>  {
> +	int ret;
>  	int val;
>  	struct mipi_dsi_host *host;
>  	struct mipi_dsi_device *dsi;
> @@ -720,8 +721,11 @@ static int ti_sn_attach_host(struct auxiliary_device *adev, struct ti_sn65dsi86
>  
>  	/* check if continuous dsi clock is required or not */
>  	pm_runtime_get_sync(dev);
> -	regmap_read(pdata->regmap, SN_DPPLL_SRC_REG, &val);
> +	ret = regmap_read(pdata->regmap, SN_DPPLL_SRC_REG, &val);
>  	pm_runtime_put_autosuspend(dev);
> +	if (ret)
> +		return ret;
> +
>  	if (!(val & DPPLL_CLK_SRC_DSICLK))
>  		dsi->mode_flags |= MIPI_DSI_CLOCK_NON_CONTINUOUS;
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/4] drm: ti-sn65dsi86: Check bridge connection failure

[Tomi Valkeinen]

On 19/06/2024 22:32, Laurent Pinchart wrote:
> Hi Jacopo,
> 
> Thank you for the patch.
> 
> On Wed, Jun 19, 2024 at 12:22:16PM +0200, Jacopo Mondi wrote:
>> From: Phong Hoang <phong.hoang.wz@renesas.com>
>>
>> Add a check to the register access function when attaching a bridge
>> device.

I think the desc is missing the "why". I'm guessing it's the first 
register access to the IC, and thus verifies that it is accessible.

  Tomi

>>
>> Signed-off-by: Phong Hoang <phong.hoang.wz@renesas.com>
>> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
>> ---
>>   drivers/gpu/drm/bridge/ti-sn65dsi86.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>> index 84698a0b27a8..b7df53577987 100644
>> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>> @@ -696,6 +696,7 @@ static struct ti_sn65dsi86 *bridge_to_ti_sn65dsi86(struct drm_bridge *bridge)
>>   
>>   static int ti_sn_attach_host(struct auxiliary_device *adev, struct ti_sn65dsi86 *pdata)
>>   {
>> +	int ret;
>>   	int val;
>>   	struct mipi_dsi_host *host;
>>   	struct mipi_dsi_device *dsi;
>> @@ -720,8 +721,11 @@ static int ti_sn_attach_host(struct auxiliary_device *adev, struct ti_sn65dsi86
>>   
>>   	/* check if continuous dsi clock is required or not */
>>   	pm_runtime_get_sync(dev);
>> -	regmap_read(pdata->regmap, SN_DPPLL_SRC_REG, &val);
>> +	ret = regmap_read(pdata->regmap, SN_DPPLL_SRC_REG, &val);
>>   	pm_runtime_put_autosuspend(dev);
>> +	if (ret)
>> +		return ret;
>> +
>>   	if (!(val & DPPLL_CLK_SRC_DSICLK))
>>   		dsi->mode_flags |= MIPI_DSI_CLOCK_NON_CONTINUOUS;
>>   
> 

Re: [PATCH 2/4] drm: ti-sn65dsi86: Check bridge connection failure

[Laurent Pinchart]

On Thu, Jun 20, 2024 at 09:43:05AM +0300, Tomi Valkeinen wrote:
> On 19/06/2024 22:32, Laurent Pinchart wrote:
> > Hi Jacopo,
> > 
> > Thank you for the patch.
> > 
> > On Wed, Jun 19, 2024 at 12:22:16PM +0200, Jacopo Mondi wrote:
> >> From: Phong Hoang <phong.hoang.wz@renesas.com>
> >>
> >> Add a check to the register access function when attaching a bridge
> >> device.
> 
> I think the desc is missing the "why". I'm guessing it's the first 
> register access to the IC, and thus verifies that it is accessible.

Isn't it a good idea in general to always check if I2C reads succeeded ?

> >>
> >> Signed-off-by: Phong Hoang <phong.hoang.wz@renesas.com>
> >> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > 
> > Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > 
> >> ---
> >>   drivers/gpu/drm/bridge/ti-sn65dsi86.c | 6 +++++-
> >>   1 file changed, 5 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> >> index 84698a0b27a8..b7df53577987 100644
> >> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> >> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> >> @@ -696,6 +696,7 @@ static struct ti_sn65dsi86 *bridge_to_ti_sn65dsi86(struct drm_bridge *bridge)
> >>   
> >>   static int ti_sn_attach_host(struct auxiliary_device *adev, struct ti_sn65dsi86 *pdata)
> >>   {
> >> +	int ret;
> >>   	int val;
> >>   	struct mipi_dsi_host *host;
> >>   	struct mipi_dsi_device *dsi;
> >> @@ -720,8 +721,11 @@ static int ti_sn_attach_host(struct auxiliary_device *adev, struct ti_sn65dsi86
> >>   
> >>   	/* check if continuous dsi clock is required or not */
> >>   	pm_runtime_get_sync(dev);
> >> -	regmap_read(pdata->regmap, SN_DPPLL_SRC_REG, &val);
> >> +	ret = regmap_read(pdata->regmap, SN_DPPLL_SRC_REG, &val);
> >>   	pm_runtime_put_autosuspend(dev);
> >> +	if (ret)
> >> +		return ret;
> >> +
> >>   	if (!(val & DPPLL_CLK_SRC_DSICLK))
> >>   		dsi->mode_flags |= MIPI_DSI_CLOCK_NON_CONTINUOUS;
> >>   

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/4] drm: ti-sn65dsi86: Check bridge connection failure

[Tomi Valkeinen]

On 20/06/2024 13:42, Laurent Pinchart wrote:
> On Thu, Jun 20, 2024 at 09:43:05AM +0300, Tomi Valkeinen wrote:
>> On 19/06/2024 22:32, Laurent Pinchart wrote:
>>> Hi Jacopo,
>>>
>>> Thank you for the patch.
>>>
>>> On Wed, Jun 19, 2024 at 12:22:16PM +0200, Jacopo Mondi wrote:
>>>> From: Phong Hoang <phong.hoang.wz@renesas.com>
>>>>
>>>> Add a check to the register access function when attaching a bridge
>>>> device.
>>
>> I think the desc is missing the "why". I'm guessing it's the first
>> register access to the IC, and thus verifies that it is accessible.
> 
> Isn't it a good idea in general to always check if I2C reads succeeded ?

It is. But if there are tens of other i2c accesses for which the return 
value is ignored, the question remains: why this single one was 
specifically fixed?

  Tomi

> 
>>>>
>>>> Signed-off-by: Phong Hoang <phong.hoang.wz@renesas.com>
>>>> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>>>
>>> Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>>>
>>>> ---
>>>>    drivers/gpu/drm/bridge/ti-sn65dsi86.c | 6 +++++-
>>>>    1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>>>> index 84698a0b27a8..b7df53577987 100644
>>>> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>>>> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>>>> @@ -696,6 +696,7 @@ static struct ti_sn65dsi86 *bridge_to_ti_sn65dsi86(struct drm_bridge *bridge)
>>>>    
>>>>    static int ti_sn_attach_host(struct auxiliary_device *adev, struct ti_sn65dsi86 *pdata)
>>>>    {
>>>> +	int ret;
>>>>    	int val;
>>>>    	struct mipi_dsi_host *host;
>>>>    	struct mipi_dsi_device *dsi;
>>>> @@ -720,8 +721,11 @@ static int ti_sn_attach_host(struct auxiliary_device *adev, struct ti_sn65dsi86
>>>>    
>>>>    	/* check if continuous dsi clock is required or not */
>>>>    	pm_runtime_get_sync(dev);
>>>> -	regmap_read(pdata->regmap, SN_DPPLL_SRC_REG, &val);
>>>> +	ret = regmap_read(pdata->regmap, SN_DPPLL_SRC_REG, &val);
>>>>    	pm_runtime_put_autosuspend(dev);
>>>> +	if (ret)
>>>> +		return ret;
>>>> +
>>>>    	if (!(val & DPPLL_CLK_SRC_DSICLK))
>>>>    		dsi->mode_flags |= MIPI_DSI_CLOCK_NON_CONTINUOUS;
>>>>    
> 

[PATCH 3/4] drm: rcar-mipi-dsi: Add support for R8A779H0

[Jacopo Mondi]

Add support for R-Car R8A779H0 V4M which has the same characteristics
of the already supported R-Car V4H R8A779G0.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

---
BSP patch:
https://github.com/renesas-rcar/linux-bsp/commit/61b876a8fa2c5d0f8049ecf29f24e3dd73ba9f8b
---
 .../devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml  | 1 +
 drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c                  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml b/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
index d33026f85e19..c167795c63f6 100644
--- a/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
@@ -19,6 +19,7 @@ properties:
     enum:
       - renesas,r8a779a0-dsi-csi2-tx    # for V3U
       - renesas,r8a779g0-dsi-csi2-tx    # for V4H
+      - renesas,r8a779h0-dsi-csi2-tx    # for V4M
 
   reg:
     maxItems: 1
diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
index 2dba7c5ffd2c..d5800ef14a2f 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
@@ -1081,6 +1081,7 @@ static const struct rcar_mipi_dsi_device_info v4h_data = {
 static const struct of_device_id rcar_mipi_dsi_of_table[] = {
 	{ .compatible = "renesas,r8a779a0-dsi-csi2-tx", .data = &v3u_data },
 	{ .compatible = "renesas,r8a779g0-dsi-csi2-tx", .data = &v4h_data },
+	{ .compatible = "renesas,r8a779h0-dsi-csi2-tx", .data = &v4h_data },
 	{ }
 };
 
-- 
2.45.2

Re: [PATCH 3/4] drm: rcar-mipi-dsi: Add support for R8A779H0

[Krzysztof Kozlowski]

On 19/06/2024 12:22, Jacopo Mondi wrote:
> Add support for R-Car R8A779H0 V4M which has the same characteristics
> of the already supported R-Car V4H R8A779G0.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> 

Please run scripts/checkpatch.pl and fix reported warnings. Then please
run `scripts/checkpatch.pl --strict` and (probably) fix more warnings.
Some warnings can be ignored, especially from --strict run, but the code
here looks like it needs a fix. Feel free to get in touch if the warning
is not clear.

Bindings are separate patches.

<form letter>
Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.

Tools like b4 or scripts/get_maintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, instead use mainline), work on fork of kernel
(don't, instead use mainline) or you ignore some maintainers (really
don't). Just use b4 and everything should be fine, although remember
about `b4 prep --auto-to-cc` if you added new patches to the patchset.

You missed at least devicetree list (maybe more), so this won't be
tested by automated tooling. Performing review on untested code might be
a waste of time, thus I will skip this patch entirely till you follow
the process allowing the patch to be tested.

Please kindly resend and include all necessary To/Cc entries.
</form letter>

Best regards,
Krzysztof

Re: [PATCH 3/4] drm: rcar-mipi-dsi: Add support for R8A779H0

[Jacopo Mondi]

Hi Krzysztof

On Wed, Jun 19, 2024 at 01:07:24PM GMT, Krzysztof Kozlowski wrote:
> On 19/06/2024 12:22, Jacopo Mondi wrote:
> > Add support for R-Car R8A779H0 V4M which has the same characteristics
> > of the already supported R-Car V4H R8A779G0.
> >
> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> >
>
> Please run scripts/checkpatch.pl and fix reported warnings. Then please
> run `scripts/checkpatch.pl --strict` and (probably) fix more warnings.
> Some warnings can be ignored, especially from --strict run, but the code
> here looks like it needs a fix. Feel free to get in touch if the warning
> is not clear.
>

The only report I got was

WARNING: DT binding docs and includes should be a separate patch. See: Documentation/devicetree/bindings/submitting-patches.rst

which for such trivial addition seems overkill, and I presume the
patch will go through geert or drm anyway

The other thing I got

CHECK: Prefer using the BIT macro
#30: FILE: drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h:144:

makes sense, but the rest of the file already uses (1 << x) so I kept
it that way for consistency (honestly I would have changed all defines
to use BIT() but that would make upporting from BSP quite more
tedious).

Is there any other warning I have missed ?



> Bindings are separate patches.
>
> <form letter>
> Please use scripts/get_maintainers.pl to get a list of necessary people
> and lists to CC. It might happen, that command when run on an older
> kernel, gives you outdated entries. Therefore please be sure you base
> your patches on recent Linux kernel.
>
> Tools like b4 or scripts/get_maintainer.pl provide you proper list of
> people, so fix your workflow. Tools might also fail if you work on some
> ancient tree (don't, instead use mainline), work on fork of kernel
> (don't, instead use mainline) or you ignore some maintainers (really
> don't). Just use b4 and everything should be fine, although remember
> about `b4 prep --auto-to-cc` if you added new patches to the patchset.
>
> You missed at least devicetree list (maybe more), so this won't be
> tested by automated tooling. Performing review on untested code might be
> a waste of time, thus I will skip this patch entirely till you follow
> the process allowing the patch to be tested.
>
> Please kindly resend and include all necessary To/Cc entries.
> </form letter>
>
> Best regards,
> Krzysztof
>

Re: [PATCH 3/4] drm: rcar-mipi-dsi: Add support for R8A779H0

[Krzysztof Kozlowski]

On 19/06/2024 14:00, Jacopo Mondi wrote:
> Hi Krzysztof
> 
> On Wed, Jun 19, 2024 at 01:07:24PM GMT, Krzysztof Kozlowski wrote:
>> On 19/06/2024 12:22, Jacopo Mondi wrote:
>>> Add support for R-Car R8A779H0 V4M which has the same characteristics
>>> of the already supported R-Car V4H R8A779G0.
>>>
>>> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>>>
>>
>> Please run scripts/checkpatch.pl and fix reported warnings. Then please
>> run `scripts/checkpatch.pl --strict` and (probably) fix more warnings.
>> Some warnings can be ignored, especially from --strict run, but the code
>> here looks like it needs a fix. Feel free to get in touch if the warning
>> is not clear.
>>
> 
> The only report I got was
> 
> WARNING: DT binding docs and includes should be a separate patch. See: Documentation/devicetree/bindings/submitting-patches.rst
> 
> which for such trivial addition seems overkill, and I presume the
> patch will go through geert or drm anyway

That's still the preference. Having patches, even when trivial, tested
via automation as well.

Best regards,
Krzysztof

[PATCH 4/4] drm: rcar-du: Add support for R8A779H0

[Jacopo Mondi]

Add support for R-Car R8A779H0 V4M which has similar characteristics
as the already supported R-Car V4H R8A779G0, but with a single output
channel.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

---
BSP patch
https://github.com/renesas-rcar/linux-bsp/commit/f2fc3314dab2052240653c1a31ba3d7c7190038e
---
---
 .../bindings/display/renesas,du.yaml           |  1 +
 drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c  | 18 ++++++++++++++++++
 .../gpu/drm/renesas/rcar-du/rcar_du_group.c    | 17 ++++++++++++-----
 3 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/renesas,du.yaml b/Documentation/devicetree/bindings/display/renesas,du.yaml
index c5b9e6812bce..d369953f16f7 100644
--- a/Documentation/devicetree/bindings/display/renesas,du.yaml
+++ b/Documentation/devicetree/bindings/display/renesas,du.yaml
@@ -41,6 +41,7 @@ properties:
       - renesas,du-r8a77995 # for R-Car D3 compatible DU
       - renesas,du-r8a779a0 # for R-Car V3U compatible DU
       - renesas,du-r8a779g0 # for R-Car V4H compatible DU
+      - renesas,du-r8a779h0 # for R-Car V4M compatible DU
 
   reg:
     maxItems: 1
diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
index dee530e4c8b2..a1d174b0b00b 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
@@ -545,6 +545,23 @@ static const struct rcar_du_device_info rcar_du_r8a779g0_info = {
 	.dsi_clk_mask =  BIT(1) | BIT(0),
 };
 
+static const struct rcar_du_device_info rcar_du_r8a779h0_info = {
+	.gen = 4,
+	.features = RCAR_DU_FEATURE_CRTC_IRQ
+		  | RCAR_DU_FEATURE_VSP1_SOURCE
+		  | RCAR_DU_FEATURE_NO_BLENDING,
+	.channels_mask = BIT(0),
+	.routes = {
+		/* R8A779H0 has a single MIPI DSI output. */
+		[RCAR_DU_OUTPUT_DSI0] = {
+			.possible_crtcs = BIT(0),
+			.port = 0,
+		},
+	},
+	.num_rpf = 5,
+	.dsi_clk_mask = BIT(0),
+};
+
 static const struct of_device_id rcar_du_of_table[] = {
 	{ .compatible = "renesas,du-r8a7742", .data = &rcar_du_r8a7790_info },
 	{ .compatible = "renesas,du-r8a7743", .data = &rzg1_du_r8a7743_info },
@@ -571,6 +588,7 @@ static const struct of_device_id rcar_du_of_table[] = {
 	{ .compatible = "renesas,du-r8a77995", .data = &rcar_du_r8a7799x_info },
 	{ .compatible = "renesas,du-r8a779a0", .data = &rcar_du_r8a779a0_info },
 	{ .compatible = "renesas,du-r8a779g0", .data = &rcar_du_r8a779g0_info },
+	{ .compatible = "renesas,du-r8a779h0", .data = &rcar_du_r8a779h0_info },
 	{ }
 };
 
diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
index 2ccd2581f544..361e1d01b817 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
@@ -111,6 +111,8 @@ static void rcar_du_group_setup_didsr(struct rcar_du_group *rgrp)
 		/*
 		 * On Gen3 dot clocks are setup through per-group registers,
 		 * only available when the group has two channels.
+		 *
+		 * R-Car V4M (R8A779H0) has only one channel, index is == 0.
 		 */
 		rcrtc = &rcdu->crtcs[rgrp->index * 2];
 		num_crtcs = rgrp->num_crtcs;
@@ -185,11 +187,16 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
 		dorcr |= DORCR_PG1T | DORCR_DK1S | DORCR_PG1D_DS1;
 	rcar_du_group_write(rgrp, DORCR, dorcr);
 
-	/* Apply planes to CRTCs association. */
-	mutex_lock(&rgrp->lock);
-	rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
-			    rgrp->dptsr_planes);
-	mutex_unlock(&rgrp->lock);
+	/*
+	 * Apply planes to CRTCs association, skip for V4M which has a single
+	 * channel.
+	 */
+	if (rcdu->info->gen < 4 || rgrp->num_crtcs > 1) {
+		mutex_lock(&rgrp->lock);
+		rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
+				    rgrp->dptsr_planes);
+		mutex_unlock(&rgrp->lock);
+	}
 }
 
 /*
-- 
2.45.2

Re: [PATCH 4/4] drm: rcar-du: Add support for R8A779H0

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Wed, Jun 19, 2024 at 12:22:18PM +0200, Jacopo Mondi wrote:
> Add support for R-Car R8A779H0 V4M which has similar characteristics
> as the already supported R-Car V4H R8A779G0, but with a single output
> channel.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> 
> ---
> BSP patch
> https://github.com/renesas-rcar/linux-bsp/commit/f2fc3314dab2052240653c1a31ba3d7c7190038e
> ---
> ---
>  .../bindings/display/renesas,du.yaml           |  1 +
>  drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c  | 18 ++++++++++++++++++
>  .../gpu/drm/renesas/rcar-du/rcar_du_group.c    | 17 ++++++++++++-----
>  3 files changed, 31 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/renesas,du.yaml b/Documentation/devicetree/bindings/display/renesas,du.yaml
> index c5b9e6812bce..d369953f16f7 100644
> --- a/Documentation/devicetree/bindings/display/renesas,du.yaml
> +++ b/Documentation/devicetree/bindings/display/renesas,du.yaml
> @@ -41,6 +41,7 @@ properties:
>        - renesas,du-r8a77995 # for R-Car D3 compatible DU
>        - renesas,du-r8a779a0 # for R-Car V3U compatible DU
>        - renesas,du-r8a779g0 # for R-Car V4H compatible DU
> +      - renesas,du-r8a779h0 # for R-Car V4M compatible DU
>  
>    reg:
>      maxItems: 1

This should be split to a separate patch.

You need to add a conditional validation rule below to address the
clocks, interrupts, ports, ...

> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
> index dee530e4c8b2..a1d174b0b00b 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
> @@ -545,6 +545,23 @@ static const struct rcar_du_device_info rcar_du_r8a779g0_info = {
>  	.dsi_clk_mask =  BIT(1) | BIT(0),
>  };
>  
> +static const struct rcar_du_device_info rcar_du_r8a779h0_info = {
> +	.gen = 4,
> +	.features = RCAR_DU_FEATURE_CRTC_IRQ
> +		  | RCAR_DU_FEATURE_VSP1_SOURCE
> +		  | RCAR_DU_FEATURE_NO_BLENDING,
> +	.channels_mask = BIT(0),
> +	.routes = {
> +		/* R8A779H0 has a single MIPI DSI output. */
> +		[RCAR_DU_OUTPUT_DSI0] = {
> +			.possible_crtcs = BIT(0),
> +			.port = 0,
> +		},
> +	},
> +	.num_rpf = 5,
> +	.dsi_clk_mask = BIT(0),
> +};

This looks good.

> +
>  static const struct of_device_id rcar_du_of_table[] = {
>  	{ .compatible = "renesas,du-r8a7742", .data = &rcar_du_r8a7790_info },
>  	{ .compatible = "renesas,du-r8a7743", .data = &rzg1_du_r8a7743_info },
> @@ -571,6 +588,7 @@ static const struct of_device_id rcar_du_of_table[] = {
>  	{ .compatible = "renesas,du-r8a77995", .data = &rcar_du_r8a7799x_info },
>  	{ .compatible = "renesas,du-r8a779a0", .data = &rcar_du_r8a779a0_info },
>  	{ .compatible = "renesas,du-r8a779g0", .data = &rcar_du_r8a779g0_info },
> +	{ .compatible = "renesas,du-r8a779h0", .data = &rcar_du_r8a779h0_info },
>  	{ }
>  };
>  
> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> index 2ccd2581f544..361e1d01b817 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> @@ -111,6 +111,8 @@ static void rcar_du_group_setup_didsr(struct rcar_du_group *rgrp)
>  		/*
>  		 * On Gen3 dot clocks are setup through per-group registers,
>  		 * only available when the group has two channels.
> +		 *
> +		 * R-Car V4M (R8A779H0) has only one channel, index is == 0.

Is it relevant here ?

>  		 */
>  		rcrtc = &rcdu->crtcs[rgrp->index * 2];
>  		num_crtcs = rgrp->num_crtcs;
> @@ -185,11 +187,16 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
>  		dorcr |= DORCR_PG1T | DORCR_DK1S | DORCR_PG1D_DS1;
>  	rcar_du_group_write(rgrp, DORCR, dorcr);
>  
> -	/* Apply planes to CRTCs association. */
> -	mutex_lock(&rgrp->lock);
> -	rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
> -			    rgrp->dptsr_planes);
> -	mutex_unlock(&rgrp->lock);
> +	/*
> +	 * Apply planes to CRTCs association, skip for V4M which has a single
> +	 * channel.

" and doesn't implement the DPTSR register."

I'm pretty sure writing it is still harmless, but...

> +	 */
> +	if (rcdu->info->gen < 4 || rgrp->num_crtcs > 1) {
> +		mutex_lock(&rgrp->lock);
> +		rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
> +				    rgrp->dptsr_planes);
> +		mutex_unlock(&rgrp->lock);
> +	}
>  }
>  
>  /*

-- 
Regards,

Laurent Pinchart

Re: [PATCH 4/4] drm: rcar-du: Add support for R8A779H0

[Geert Uytterhoeven]

Hi Laurent, Jacopo,

On Wed, Jun 19, 2024 at 9:46 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Wed, Jun 19, 2024 at 12:22:18PM +0200, Jacopo Mondi wrote:
> > Add support for R-Car R8A779H0 V4M which has similar characteristics
> > as the already supported R-Car V4H R8A779G0, but with a single output
> > channel.
> >
> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

> > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> > @@ -185,11 +187,16 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
> >               dorcr |= DORCR_PG1T | DORCR_DK1S | DORCR_PG1D_DS1;
> >       rcar_du_group_write(rgrp, DORCR, dorcr);
> >
> > -     /* Apply planes to CRTCs association. */
> > -     mutex_lock(&rgrp->lock);
> > -     rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
> > -                         rgrp->dptsr_planes);
> > -     mutex_unlock(&rgrp->lock);
> > +     /*
> > +      * Apply planes to CRTCs association, skip for V4M which has a single
> > +      * channel.
>
> " and doesn't implement the DPTSR register."
>
> I'm pretty sure writing it is still harmless, but...
>
> > +      */
> > +     if (rcdu->info->gen < 4 || rgrp->num_crtcs > 1) {

Looking at the R-Car Gen3 docs, this check seems to be wrong, and the
lack of a check might have been an issue before?

Seems like the register (per pair) is only present if the second CRTC
of a CRTC pair is present, so R-Car V3M and V3H (single CRTC) do not
have DPTSR at all, and M3-W (triple CRTC) does not have it on the
second pair.  M3-N does have both, as it lacks the first CRTC of
second pair, but does have the second CRTC of the second pair.

> > +             mutex_lock(&rgrp->lock);
> > +             rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
> > +                                 rgrp->dptsr_planes);
> > +             mutex_unlock(&rgrp->lock);
> > +     }
> >  }

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 4/4] drm: rcar-du: Add support for R8A779H0

[Jacopo Mondi]

Hi Geert

On Thu, Jun 20, 2024 at 02:48:49PM GMT, Geert Uytterhoeven wrote:
> Hi Laurent, Jacopo,
>
> On Wed, Jun 19, 2024 at 9:46 PM Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> > On Wed, Jun 19, 2024 at 12:22:18PM +0200, Jacopo Mondi wrote:
> > > Add support for R-Car R8A779H0 V4M which has similar characteristics
> > > as the already supported R-Car V4H R8A779G0, but with a single output
> > > channel.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>
> > > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> > > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> > > @@ -185,11 +187,16 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
> > >               dorcr |= DORCR_PG1T | DORCR_DK1S | DORCR_PG1D_DS1;
> > >       rcar_du_group_write(rgrp, DORCR, dorcr);
> > >
> > > -     /* Apply planes to CRTCs association. */
> > > -     mutex_lock(&rgrp->lock);
> > > -     rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
> > > -                         rgrp->dptsr_planes);
> > > -     mutex_unlock(&rgrp->lock);
> > > +     /*
> > > +      * Apply planes to CRTCs association, skip for V4M which has a single
> > > +      * channel.
> >
> > " and doesn't implement the DPTSR register."
> >
> > I'm pretty sure writing it is still harmless, but...
> >
> > > +      */
> > > +     if (rcdu->info->gen < 4 || rgrp->num_crtcs > 1) {
>
> Looking at the R-Car Gen3 docs, this check seems to be wrong, and the
> lack of a check might have been an issue before?


Not sure I got from your comment what part is wrong.

Reading below it seems you're suggesting that writes to DPTSR should
be skipped for some Gen3 boards as well ?

>
> Seems like the register (per pair) is only present if the second CRTC
> of a CRTC pair is present, so R-Car V3M and V3H (single CRTC) do not
> have DPTSR at all, and M3-W (triple CRTC) does not have it on the
> second pair.  M3-N does have both, as it lacks the first CRTC of
> second pair, but does have the second CRTC of the second pair.
>

/o\

So far however, all Gen3 SoCs you mentioned seem to work with DPTSR
being written and the BSP [1] only actually skips it for V4M.

What would you suggesting in this case ? Addressing gen3 as well ?
That's something that would require testing on all the above boards
thought.

Thanks
  j


[1] https://github.com/renesas-rcar/linux-bsp/commit/f2fc3314dab2052240653c1a31ba3d7c7190038e#diff-8bce6f4032dc891042e2561163754f49723ac119ae63df2425cc3487b432ee1cR206
> > > +             mutex_lock(&rgrp->lock);
> > > +             rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
> > > +                                 rgrp->dptsr_planes);
> > > +             mutex_unlock(&rgrp->lock);
> > > +     }
> > >  }
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

Re: [PATCH 4/4] drm: rcar-du: Add support for R8A779H0

[Geert Uytterhoeven]

Hi Jacopo,

On Thu, Jun 20, 2024 at 6:48 PM Jacopo Mondi
<jacopo.mondi@ideasonboard.com> wrote:
> On Thu, Jun 20, 2024 at 02:48:49PM GMT, Geert Uytterhoeven wrote:
> > On Wed, Jun 19, 2024 at 9:46 PM Laurent Pinchart
> > <laurent.pinchart@ideasonboard.com> wrote:
> > > On Wed, Jun 19, 2024 at 12:22:18PM +0200, Jacopo Mondi wrote:
> > > > Add support for R-Car R8A779H0 V4M which has similar characteristics
> > > > as the already supported R-Car V4H R8A779G0, but with a single output
> > > > channel.
> > > >
> > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> >
> > > > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> > > > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> > > > @@ -185,11 +187,16 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
> > > >               dorcr |= DORCR_PG1T | DORCR_DK1S | DORCR_PG1D_DS1;
> > > >       rcar_du_group_write(rgrp, DORCR, dorcr);
> > > >
> > > > -     /* Apply planes to CRTCs association. */
> > > > -     mutex_lock(&rgrp->lock);
> > > > -     rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
> > > > -                         rgrp->dptsr_planes);
> > > > -     mutex_unlock(&rgrp->lock);
> > > > +     /*
> > > > +      * Apply planes to CRTCs association, skip for V4M which has a single
> > > > +      * channel.
> > >
> > > " and doesn't implement the DPTSR register."
> > >
> > > I'm pretty sure writing it is still harmless, but...
> > >
> > > > +      */
> > > > +     if (rcdu->info->gen < 4 || rgrp->num_crtcs > 1) {
> >
> > Looking at the R-Car Gen3 docs, this check seems to be wrong, and the
> > lack of a check might have been an issue before?
>
> Not sure I got from your comment what part is wrong.
>
> Reading below it seems you're suggesting that writes to DPTSR should
> be skipped for some Gen3 boards as well ?

Indeed.

> > Seems like the register (per pair) is only present if the second CRTC
> > of a CRTC pair is present, so R-Car V3M and V3H (single CRTC) do not
> > have DPTSR at all, and M3-W (triple CRTC) does not have it on the
> > second pair.  M3-N does have both, as it lacks the first CRTC of
> > second pair, but does have the second CRTC of the second pair.
> >
>
> /o\
>
> So far however, all Gen3 SoCs you mentioned seem to work with DPTSR
> being written and the BSP [1] only actually skips it for V4M.

I don't doubt it works, I was just reading the documentation.
Many nonexistent registers can be written zero to without ill effects...

> What would you suggesting in this case ? Addressing gen3 as well ?
> That's something that would require testing on all the above boards
> thought.

Ah, what if we could do without all this pesky testing? ;-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds