RE: [PATCH] media: platform: rzg2l-cru: rzg2l-video: Set AXI burst max length

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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




[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux