On Mon, Oct 21, 2024 at 09:24:14AM +0000, Biju Das wrote:
> Hi Laurent,
>
> > -----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.
>
> Gentle ping. Not sure You missed this??
I've sent a pull request yesterday :-)
> > > > 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
