> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> Sent: Saturday, September 7, 2024 6:33 PM
> Subject: Re: [PATCH] media: platform: rzg2l-cru: rzg2l-video: Set AXI burst max length
>
> Hi Biju,
>
> On Sat, Sep 07, 2024 at 07:41:24AM +0000, Biju Das wrote:
> > On Saturday, September 7, 2024 1:00 AM, Laurent Pinchart wrote:
> > > On Thu, Sep 05, 2024 at 12:18:26PM +0100, Biju Das wrote:
> > > > As per the hardware manual section 35.2.3.26 'AXI Master Transfer
> > > > Setting Register for CRU Image Data;, it is mentioned that to
> > > > improve the transfer
> > >
> > > s/;/'/
> >
> > Oops, Will fix this in next version.
> >
> > > > performance of CRU, it is recommended to use AXILEN value '0xf'
> > > > for AXI burst max length setting for image data.
> > > >
> > > > Signed-off-by: Hien Huynh <hien.huynh.px@xxxxxxxxxxx>
> > > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > > > ---
> > > > .../media/platform/renesas/rzg2l-cru/rzg2l-video.c | 11 +++++++++++
> > > > 1 file changed, 11 insertions(+)
> > > >
> > > > diff --git
> > > > a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > > index 374dc084717f..d17e3eac4177 100644
> > > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > > @@ -52,6 +52,11 @@
> > > > #define AMnMBS 0x14c
> > > > #define AMnMBS_MBSTS 0x7
> > > >
> > > > +/* AXI Master Transfer Setting Register for CRU Image Data */
> > > > +#define AMnAXIATTR 0x158
> > > > +#define AMnAXIATTR_AXILEN_MASK GENMASK(3, 0)
> > > > +#define AMnAXIATTR_AXILEN (0xf)
> > > > +
> > > > /* AXI Master FIFO Pointer Register for CRU Image Data */
> > > > #define AMnFIFOPNTR 0x168
> > > > #define AMnFIFOPNTR_FIFOWPNTR GENMASK(7, 0)
> > > > @@ -278,6 +283,7 @@ static void rzg2l_cru_fill_hw_slot(struct
> > > > rzg2l_cru_dev *cru, int slot) static void
> > > > rzg2l_cru_initialize_axi(struct rzg2l_cru_dev *cru) {
> > > > unsigned int slot;
> > > > + u32 amnaxiattr;
> > > >
> > > > /*
> > > > * Set image data memory banks.
> > > > @@ -287,6 +293,11 @@ static void rzg2l_cru_initialize_axi(struct
> > > > rzg2l_cru_dev *cru)
> > > >
> > > > for (slot = 0; slot < cru->num_buf; slot++)
> > > > rzg2l_cru_fill_hw_slot(cru, slot);
> > > > +
> > > > + /* Set AXI burst max length to recommended setting */
> > > > + amnaxiattr = rzg2l_cru_read(cru, AMnAXIATTR) & ~AMnAXIATTR_AXILEN_MASK;
> > > > + amnaxiattr |= AMnAXIATTR_AXILEN;
> > > > + rzg2l_cru_write(cru, AMnAXIATTR, amnaxiattr);
> > >
> > > It would be more efficient to just write
> > >
> > > rzg2l_cru_write(cru, AMnAXIATTR, AMnAXIATTR_AXILEN);
> >
> > I thought about that. But then re-reading register description changed
> > the mind because of the below bits
> >
> > {9,8} — 01b R Reserved:
> > When read, the initial value is read. When writing, be sure to write the initial value.
> > Operation is not guaranteed if a value other than the initial value is written.
> >
> > {6,4} — 101b R Reserved:
> > When read, the initial value is read. When writing, be sure to write the initial value.
> > Operation is not guaranteed if a value other than the initial value is written.
> >
> > Also, bits {27,24}, {22,16} and {13,12} -0b :
> >
> > It is mentioned that
> > When read, the initial value is read. When writing, be sure to write the initial value.
> > operation is not guaranteed if a value other than the initial value is written.
>
> Let's keep the code as-is then. I'll fix the typo in (and reflow) the commit message myself, no need
> to resubmit.
Thanks Laurent.
Cheers,
Biju
>
> > > the hardware manual however doesn't make it clear if this is safe or
> > > not. The rest of the register is reserved, and writes as documented
> > > as ignored, but the reset value is non-zero. If it's not safe to
> > > write the reserved bits to 0,
> > >
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > >
> > > > }
> > > >
> > > > static void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru, bool
> > > > *input_is_yuv,
>
> --
> Regards,
>
> Laurent Pinchart
