Re: [PATCH] media: rkisp1: Allow higher input resolution

[Paul Elder]

On Mon, Feb 19, 2024 at 02:33:21PM +0530, Umang Jain wrote:
> Hi All,
> 
> On 19/02/24 7:39 am, Adam Ford wrote:
> > On Sun, Feb 18, 2024 at 3:02 PM Ondřej Jirman <megi@xxxxxx> wrote:
> > > On Sun, Feb 18, 2024 at 10:59:08PM +0200, Laurent Pinchart wrote:
> > > > Hi Ondrej,
> > > > 
> > > > (CC'ing Paul and Umang)
> > > > 
> > > > Thank you for the patch.
> > > > 
> > > > On Sat, Feb 17, 2024 at 07:51:58PM +0100, Ondřej Jirman wrote:
> > > > > From: Ondrej Jirman <megi@xxxxxx>
> > > > > 
> > > > > In BSP driver, it is allowed, and it works in practice. Tested on
> > > > > Pinephone Pro/RK3399 with IMX258 at full res.
> > > > Paul, Umang, do I recall correctly that you have a similar change ?
> > > > Could you review and test this (especially on the i.MX8MP) ?
> > > It's also a limit from the datasheet, so the change should not be that
> > > controversial:
> > > 
> > >    https://megous.com/dl/tmp/d2b333043ecebaf3.png
> > > 
> > > (so that it doesn't sound like I just copied the BSP values)
> > > 
> >  From what I see in the i.MX8M Plus reference manual, it has a max
> > resolution of 4096x3072, so it might be necessary to move this off
> 
> This is what I (and I assume Paul too) have been working with on i.MX8M
> Plus. So it's the known and tested value of max ISP input from out side.
> > from a #define into a structure that varies by product family.

Yes, this is what needs to be done. Here's what I have in my notes:

- The RK3399 TRM says 4416x3312 max input and output on main path, with
  1920x1080 max output on self path.
- The PX30 datasheet [1] says 3264x2448 max input and output on main
  path, with 1920x1080 max output on self path.
- The RK3288 documentation [2] (under "Camera Interface and Image
  Processor") says 4416x3312 max input and output on main path, with
  1920x1080 max output on self path.
- The i.MX8MP reference manual (the open one) [3] (in table 13-1) says
  4096x3072 max resolution in single ISP mode

The i.MX8M Plus seems to indeed be limited to 4096x3072, but the TPG is
capable of generating 4416x3312, and the ISP works fine in bypass (and
therefore raw) mode, so technically it has different maximum sizes
depending on the format which makes this more exciting.

In any case, the PX30 (assuming the datasheet is correct) only supports
up to 3264x2448, so the existing #define is incorrect anyway.

I don't have a PX30 nor an RK3288 so I can't test those, and I haven't
set up my OV64A40 yet which (I've heard) can be used to test even bigger
resolutions.


Paul

[1] https://opensource.rock-chips.com/images/8/87/Rockchip_PX30_Datasheet_V1.4-20191227.pdf
[2] https://opensource.rock-chips.com/images/4/49/Rockchip_RK3288_Datasheet_V2.7-20191227.pdf
[3] (requires login) https://www.nxp.com/products/processors-and-microcontrollers/arm-processors/i-mx-applications-processors/i-mx-8-applications-processors/i-mx-8m-plus-arm-cortex-a53-machine-learning-vision-multimedia-and-industrial-iot:IMX8MPLUS


> 
> Yeah!
> > 
> > adam
> > > regards,
> > >          o.
> > > 
> > > > > Signed-off-by: Ondrej Jirman <megi@xxxxxx>
> > > > > ---
> > > > >   drivers/media/platform/rockchip/rkisp1/rkisp1-common.h | 4 ++--
> > > > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> > > > > index 4b6b28c05b89..74098ddbeeb3 100644
> > > > > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> > > > > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> > > > > @@ -33,8 +33,8 @@ struct dentry;
> > > > >   #define RKISP1_ISP_SD_SINK                 BIT(1)
> > > > > 
> > > > >   /* min and max values for the widths and heights of the entities */
> > > > > -#define RKISP1_ISP_MAX_WIDTH                       4032
> > > > > -#define RKISP1_ISP_MAX_HEIGHT                      3024
> > > > > +#define RKISP1_ISP_MAX_WIDTH                       4416
> > > > > +#define RKISP1_ISP_MAX_HEIGHT                      3312
> > > > >   #define RKISP1_ISP_MIN_WIDTH                       32
> > > > >   #define RKISP1_ISP_MIN_HEIGHT                      32
> > > > > 
> > > > --
> > > > Regards,
> > > > 
> > > > Laurent Pinchart
>