[PATCH v3 2/2] drm: renesas: rz-du: Set DCS maximum return packet size

[Hugo Villeneuve]

On Wed, 4 Jun 2025 12:08:49 +0000
Chris Brandt <Chris.Brandt at renesas.com> wrote:

Hi Chris,

> Hi Hugo,
> 
> Sorry, one more thing....
> 
> > +	/*
> > +	 * The default value of 1 will result in long read commands payload
> > +	 * not being saved to memory. Set to the DMA buffer size.
> > +	 */
> 
> The comment is a bit wordy.
> 
> You just need to say:
> 
> /* Set read buffer size */

Like I said, this is related to something that is not obvious, so that
is why I put a lenghty description.

> Or...no comment at all. It's pretty obvious what the code is doing because you are writing
> RZG2L_DCS_BUF_SIZE to a register.

Agreed, I will remove the part that says "Set to the DMA buffer size".

Hugo.


> -----Original Message-----
> From: Chris Brandt 
> Sent: Wednesday, June 4, 2025 7:54 AM
> To: Hugo Villeneuve <hugo at hugovil.com>; Biju Das <biju.das.jz at bp.renesas.com>; maarten.lankhorst at linux.intel.com; mripard at kernel.org; tzimmermann at suse.de; airlied at gmail.com; simona at ffwll.ch
> Cc: dri-devel at lists.freedesktop.org; linux-renesas-soc at vger.kernel.org; linux-kernel at vger.kernel.org; Hugo Villeneuve <hvilleneuve at dimonoff.com>
> Subject: RE: [PATCH v3 2/2] drm: renesas: rz-du: Set DCS maximum return packet size
> 
> Hi Hugo,
> 
> I'm fine with the code, but maybe it should go in a different location.
> 
> Since it's a register setup, it should probably go in rzg2l_mipi_dsi_startup() with the others.
> 
> Additionally, since it is required to make rzg2l_mipi_dsi_host_transfer() operate properly, my suggestion is to add this to your previous patch instead of making it separate.
> Otherwise, it's like you are submitting one patch with a known bug, then immediately fixing it with a second patch.
> 
> This also would prevent the merge conflict with my patch that also modifies rzg2l_mipi_dsi_atomic_enable().
> 
> Chris
> 
> 
> -----Original Message-----
> From: Hugo Villeneuve <hugo at hugovil.com>
> Sent: Thursday, May 22, 2025 10:39 AM
> To: Biju Das <biju.das.jz at bp.renesas.com>; maarten.lankhorst at linux.intel.com; mripard at kernel.org; tzimmermann at suse.de; airlied at gmail.com; simona at ffwll.ch
> Cc: dri-devel at lists.freedesktop.org; linux-renesas-soc at vger.kernel.org; linux-kernel at vger.kernel.org; hugo at hugovil.com; Hugo Villeneuve <hvilleneuve at dimonoff.com>; Chris Brandt <Chris.Brandt at renesas.com>
> Subject: [PATCH v3 2/2] drm: renesas: rz-du: Set DCS maximum return packet size
> 
> From: Hugo Villeneuve <hvilleneuve at dimonoff.com>
> 
> The default value of 1 will result in long read commands payload not being saved to memory.
> 
> Fix by setting this value to the DMA buffer size.
> 
> Cc: Biju Das <biju.das.jz at bp.renesas.com>
> Cc: Chris Brandt <chris.brandt at renesas.com>
> Signed-off-by: Hugo Villeneuve <hvilleneuve at dimonoff.com>
> ---
>  drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c      | 10 ++++++++++
>  drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi_regs.h |  4 ++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c b/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c
> index a048d473db00b..745aae63af9d8 100644
> --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c
> +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c
> @@ -549,6 +549,7 @@ static void rzg2l_mipi_dsi_atomic_enable(struct drm_bridge *bridge,
>  	const struct drm_display_mode *mode;
>  	struct drm_connector *connector;
>  	struct drm_crtc *crtc;
> +	u32 value;
>  	int ret;
>  
>  	connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder); @@ -561,6 +562,15 @@ static void rzg2l_mipi_dsi_atomic_enable(struct drm_bridge *bridge,
>  
>  	rzg2l_mipi_dsi_set_display_timing(dsi, mode);
>  
> +	/*
> +	 * The default value of 1 will result in long read commands payload
> +	 * not being saved to memory. Set to the DMA buffer size.
> +	 */
> +	value = rzg2l_mipi_dsi_link_read(dsi, DSISETR);
> +	value &= ~DSISETR_MRPSZ;
> +	value |= FIELD_PREP(DSISETR_MRPSZ, RZG2L_DCS_BUF_SIZE);
> +	rzg2l_mipi_dsi_link_write(dsi, DSISETR, value);
> +
>  	ret = rzg2l_mipi_dsi_start_hs_clock(dsi);
>  	if (ret < 0)
>  		goto err_stop;
> diff --git a/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi_regs.h b/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi_regs.h
> index 0e432b04188d0..26d8a37ee6351 100644
> --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi_regs.h
> +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi_regs.h
> @@ -81,6 +81,10 @@
>  #define RSTSR_SWRSTLP			(1 << 1)
>  #define RSTSR_SWRSTHS			(1 << 0)
>  
> +/* DSI Set Register */
> +#define DSISETR				0x120
> +#define DSISETR_MRPSZ			GENMASK(15, 0)
> +
>  /* Rx Result Save Slot 0 Register */
>  #define RXRSS0R				0x240
>  #define RXRSS0R_RXPKTDFAIL		BIT(28)
> --
> 2.39.5
> 
> 


-- 
Hugo Villeneuve