Hi Hugo,

> Also, by default, most panels are configured to send a maximum of 1 byte, so 
> it is not absolutely required by default and not really a bug.

Thanks. I found that I needed to call mipi_dsi_set_maximum_return_packet_size() 
first in my panel driver, then I could receive multiple bytes.

Chris


-----Original Message-----
From: Hugo Villeneuve <h...@hugovil.com> 
Sent: Wednesday, June 4, 2025 9:35 AM
To: Chris Brandt <chris.bra...@renesas.com>
Cc: Biju Das <biju.das...@bp.renesas.com>; maarten.lankho...@linux.intel.com; 
mrip...@kernel.org; tzimmerm...@suse.de; airl...@gmail.com; sim...@ffwll.ch; 
dri-devel@lists.freedesktop.org; linux-renesas-...@vger.kernel.org; 
linux-ker...@vger.kernel.org; Hugo Villeneuve <hvillene...@dimonoff.com>
Subject: Re: [PATCH v3 2/2] drm: renesas: rz-du: Set DCS maximum return packet 
size

On Wed, 4 Jun 2025 11:54:28 +0000
Chris Brandt <chris.bra...@renesas.com> wrote:

Hi Chris,

> 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.

It makes sense, I will move it there.

 
> 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.

I made it a separate patch to clearly show why this is needed, because it took 
me a lot of time to figure this out, and I didn't want that knowledge to be 
lost :)

Also, by default, most panels are configured to send a maximum of 1 byte, so it 
is not absolutely required by default and not really a bug.

But sure I can merge it, anyway my comment will clearly indicate why it is 
needed.

> This also would prevent the merge conflict with my patch that also modifies 
> rzg2l_mipi_dsi_atomic_enable().

Ok.

Hugo.


> Chris
> 
> 
> -----Original Message-----
> From: Hugo Villeneuve <h...@hugovil.com>
> Sent: Thursday, May 22, 2025 10:39 AM
> To: Biju Das <biju.das...@bp.renesas.com>; 
> maarten.lankho...@linux.intel.com; mrip...@kernel.org; 
> tzimmerm...@suse.de; airl...@gmail.com; sim...@ffwll.ch
> Cc: dri-devel@lists.freedesktop.org; 
> linux-renesas-...@vger.kernel.org; linux-ker...@vger.kernel.org; 
> h...@hugovil.com; Hugo Villeneuve <hvillene...@dimonoff.com>; Chris 
> Brandt <chris.bra...@renesas.com>
> Subject: [PATCH v3 2/2] drm: renesas: rz-du: Set DCS maximum return 
> packet size
> 
> From: Hugo Villeneuve <hvillene...@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...@bp.renesas.com>
> Cc: Chris Brandt <chris.bra...@renesas.com>
> Signed-off-by: Hugo Villeneuve <hvillene...@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

Reply via email to