[libcamera-devel,v2,0/6] libcamera: pipeline: rkisp1: Refresh to match upstream
mbox series

Message ID 20201001154600.2722718-1-niklas.soderlund@ragnatech.se
Headers show
Series
  • libcamera: pipeline: rkisp1: Refresh to match upstream
Related show

Message

Niklas Söderlund Oct. 1, 2020, 3:45 p.m. UTC
Hello,

This series refresh the RkISP1 pipeline handler to match what is in the
media-tree. As the upstream driver is in staging and bound to change
quiet a bit no attempt to keep the libcamera pipeline backwards
compatible have been made. To fully test this on must run a system no
older then [1] from the linux-media tree.

Running a system newer then v5.8 without this series will also fail as
there are upstream changes breaking current libcamera master.

Apart from aligning the pipeline with the upstream driver it's now
trivial to enable support for R8, RGB656 and XBGR8888 in the pipeline.
As these have been troublesome in the past due to issues now fixed
upstream every format supported by the pipeline (YUYV, NV16, NV61, NV21,
NV12, R8, RGB565 and XRGB8888) have been retested and converted using
raw2rgbpnm to validate that the captured data is good.

1. 7eba47ab7a310ed8 ("media: staging: rkisp1: cap: protect access to buf with the spin lock")  

Niklas Söderlund (6):
  libcamera: pipeline: rkisp1: Remove support for YVYU and VYUY
  libcamera: pipeline: rkisp1: Fix media bus propagation for NV{12,21}
  libcamera: pipeline: rkisp1: Add support for RGB656
  libcamera: pipeline: rkisp1: Add support for R8
  libcamera: Add support for XRGB8888 and XBGR8888
  libcamera: pipeline: rkisp1: Add support for XRGB8888

 src/libcamera/formats.cpp                     | 20 ++++++++++++++
 src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 27 +++++++++++++------
 src/libcamera/v4l2_pixelformat.cpp            |  2 ++
 3 files changed, 41 insertions(+), 8 deletions(-)

Comments

Laurent Pinchart Oct. 1, 2020, 4:48 p.m. UTC | #1
Hi Niklas,

Thank you for the patches.

On Thu, Oct 01, 2020 at 05:45:54PM +0200, Niklas Söderlund wrote:
> Hello,
> 
> This series refresh the RkISP1 pipeline handler to match what is in the
> media-tree. As the upstream driver is in staging and bound to change
> quiet a bit no attempt to keep the libcamera pipeline backwards
> compatible have been made. To fully test this on must run a system no
> older then [1] from the linux-media tree.
> 
> Running a system newer then v5.8 without this series will also fail as
> there are upstream changes breaking current libcamera master.
> 
> Apart from aligning the pipeline with the upstream driver it's now
> trivial to enable support for R8, RGB656 and XBGR8888 in the pipeline.
> As these have been troublesome in the past due to issues now fixed
> upstream every format supported by the pipeline (YUYV, NV16, NV61, NV21,
> NV12, R8, RGB565 and XRGB8888) have been retested and converted using
> raw2rgbpnm to validate that the captured data is good.
> 
> 1. 7eba47ab7a310ed8 ("media: staging: rkisp1: cap: protect access to buf with the spin lock")  
> 
> Niklas Söderlund (6):
>   libcamera: pipeline: rkisp1: Remove support for YVYU and VYUY
>   libcamera: pipeline: rkisp1: Fix media bus propagation for NV{12,21}
>   libcamera: pipeline: rkisp1: Add support for RGB656
>   libcamera: pipeline: rkisp1: Add support for R8
>   libcamera: Add support for XRGB8888 and XBGR8888
>   libcamera: pipeline: rkisp1: Add support for XRGB8888

For the whole series,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Could you however please follow up with upstream to replace
MEDIA_BUS_FMT_YUYV8_1_5X8 with MEDIA_BUS_FMT_YUYV8_1X16 ?

>  src/libcamera/formats.cpp                     | 20 ++++++++++++++
>  src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 27 +++++++++++++------
>  src/libcamera/v4l2_pixelformat.cpp            |  2 ++
>  3 files changed, 41 insertions(+), 8 deletions(-)
>
Niklas Söderlund Oct. 1, 2020, 8:26 p.m. UTC | #2
Hi Laurent,

Thanks for your feedback.

On 2020-10-01 19:48:34 +0300, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patches.
> 
> On Thu, Oct 01, 2020 at 05:45:54PM +0200, Niklas Söderlund wrote:
> > Hello,
> > 
> > This series refresh the RkISP1 pipeline handler to match what is in the
> > media-tree. As the upstream driver is in staging and bound to change
> > quiet a bit no attempt to keep the libcamera pipeline backwards
> > compatible have been made. To fully test this on must run a system no
> > older then [1] from the linux-media tree.
> > 
> > Running a system newer then v5.8 without this series will also fail as
> > there are upstream changes breaking current libcamera master.
> > 
> > Apart from aligning the pipeline with the upstream driver it's now
> > trivial to enable support for R8, RGB656 and XBGR8888 in the pipeline.
> > As these have been troublesome in the past due to issues now fixed
> > upstream every format supported by the pipeline (YUYV, NV16, NV61, NV21,
> > NV12, R8, RGB565 and XRGB8888) have been retested and converted using
> > raw2rgbpnm to validate that the captured data is good.
> > 
> > 1. 7eba47ab7a310ed8 ("media: staging: rkisp1: cap: protect access to buf with the spin lock")  
> > 
> > Niklas Söderlund (6):
> >   libcamera: pipeline: rkisp1: Remove support for YVYU and VYUY
> >   libcamera: pipeline: rkisp1: Fix media bus propagation for NV{12,21}
> >   libcamera: pipeline: rkisp1: Add support for RGB656
> >   libcamera: pipeline: rkisp1: Add support for R8
> >   libcamera: Add support for XRGB8888 and XBGR8888
> >   libcamera: pipeline: rkisp1: Add support for XRGB8888
> 
> For the whole series,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks.

> 
> Could you however please follow up with upstream to replace
> MEDIA_BUS_FMT_YUYV8_1_5X8 with MEDIA_BUS_FMT_YUYV8_1X16 ?

I will follow up on this yes.

> 
> >  src/libcamera/formats.cpp                     | 20 ++++++++++++++
> >  src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 27 +++++++++++++------
> >  src/libcamera/v4l2_pixelformat.cpp            |  2 ++
> >  3 files changed, 41 insertions(+), 8 deletions(-)
> > 
> 
> -- 
> Regards,
> 
> Laurent Pinchart
Jacopo Mondi Oct. 2, 2020, 2:12 p.m. UTC | #3
Hi Niklas,

On Thu, Oct 01, 2020 at 07:48:34PM +0300, Laurent Pinchart wrote:
> Hi Niklas,
>
> Thank you for the patches.
>
> On Thu, Oct 01, 2020 at 05:45:54PM +0200, Niklas Söderlund wrote:
> > Hello,
> >
> > This series refresh the RkISP1 pipeline handler to match what is in the
> > media-tree. As the upstream driver is in staging and bound to change
> > quiet a bit no attempt to keep the libcamera pipeline backwards
> > compatible have been made. To fully test this on must run a system no
> > older then [1] from the linux-media tree.
> >
> > Running a system newer then v5.8 without this series will also fail as
> > there are upstream changes breaking current libcamera master.
> >
> > Apart from aligning the pipeline with the upstream driver it's now
> > trivial to enable support for R8, RGB656 and XBGR8888 in the pipeline.
> > As these have been troublesome in the past due to issues now fixed
> > upstream every format supported by the pipeline (YUYV, NV16, NV61, NV21,
> > NV12, R8, RGB565 and XRGB8888) have been retested and converted using
> > raw2rgbpnm to validate that the captured data is good.
> >
> > 1. 7eba47ab7a310ed8 ("media: staging: rkisp1: cap: protect access to buf with the spin lock")
> >
> > Niklas Söderlund (6):
> >   libcamera: pipeline: rkisp1: Remove support for YVYU and VYUY
> >   libcamera: pipeline: rkisp1: Fix media bus propagation for NV{12,21}
> >   libcamera: pipeline: rkisp1: Add support for RGB656
> >   libcamera: pipeline: rkisp1: Add support for R8
> >   libcamera: Add support for XRGB8888 and XBGR8888
> >   libcamera: pipeline: rkisp1: Add support for XRGB8888
>
> For the whole series,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

With the small comments clarified, for the series:

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

>
> Could you however please follow up with upstream to replace
> MEDIA_BUS_FMT_YUYV8_1_5X8 with MEDIA_BUS_FMT_YUYV8_1X16 ?
>
> >  src/libcamera/formats.cpp                     | 20 ++++++++++++++
> >  src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 27 +++++++++++++------
> >  src/libcamera/v4l2_pixelformat.cpp            |  2 ++
> >  3 files changed, 41 insertions(+), 8 deletions(-)
> >
>
> --
> Regards,
>
> Laurent Pinchart
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Niklas Söderlund Oct. 2, 2020, 2:52 p.m. UTC | #4
Hi Jacopo,

On 2020-10-02 16:12:53 +0200, Jacopo Mondi wrote:
> Hi Niklas,
> 
> On Thu, Oct 01, 2020 at 07:48:34PM +0300, Laurent Pinchart wrote:
> > Hi Niklas,
> >
> > Thank you for the patches.
> >
> > On Thu, Oct 01, 2020 at 05:45:54PM +0200, Niklas Söderlund wrote:
> > > Hello,
> > >
> > > This series refresh the RkISP1 pipeline handler to match what is in the
> > > media-tree. As the upstream driver is in staging and bound to change
> > > quiet a bit no attempt to keep the libcamera pipeline backwards
> > > compatible have been made. To fully test this on must run a system no
> > > older then [1] from the linux-media tree.
> > >
> > > Running a system newer then v5.8 without this series will also fail as
> > > there are upstream changes breaking current libcamera master.
> > >
> > > Apart from aligning the pipeline with the upstream driver it's now
> > > trivial to enable support for R8, RGB656 and XBGR8888 in the pipeline.
> > > As these have been troublesome in the past due to issues now fixed
> > > upstream every format supported by the pipeline (YUYV, NV16, NV61, NV21,
> > > NV12, R8, RGB565 and XRGB8888) have been retested and converted using
> > > raw2rgbpnm to validate that the captured data is good.
> > >
> > > 1. 7eba47ab7a310ed8 ("media: staging: rkisp1: cap: protect access to buf with the spin lock")
> > >
> > > Niklas Söderlund (6):
> > >   libcamera: pipeline: rkisp1: Remove support for YVYU and VYUY
> > >   libcamera: pipeline: rkisp1: Fix media bus propagation for NV{12,21}
> > >   libcamera: pipeline: rkisp1: Add support for RGB656
> > >   libcamera: pipeline: rkisp1: Add support for R8
> > >   libcamera: Add support for XRGB8888 and XBGR8888
> > >   libcamera: pipeline: rkisp1: Add support for XRGB8888
> >
> > For the whole series,
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> With the small comments clarified, for the series:
> 
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks for reviewing this series, unfortunately the series is already 
merged so I can't record your effort :-(

> 
> Thanks
>   j
> 
> >
> > Could you however please follow up with upstream to replace
> > MEDIA_BUS_FMT_YUYV8_1_5X8 with MEDIA_BUS_FMT_YUYV8_1X16 ?
> >
> > >  src/libcamera/formats.cpp                     | 20 ++++++++++++++
> > >  src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 27 +++++++++++++------
> > >  src/libcamera/v4l2_pixelformat.cpp            |  2 ++
> > >  3 files changed, 41 insertions(+), 8 deletions(-)
> > >
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel@lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
Jacopo Mondi Oct. 2, 2020, 3:03 p.m. UTC | #5
Hi Niklas,

On Fri, Oct 02, 2020 at 04:52:01PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> On 2020-10-02 16:12:53 +0200, Jacopo Mondi wrote:
> > Hi Niklas,
> >
> > On Thu, Oct 01, 2020 at 07:48:34PM +0300, Laurent Pinchart wrote:
> > > Hi Niklas,
> > >
> > > Thank you for the patches.
> > >
> > > On Thu, Oct 01, 2020 at 05:45:54PM +0200, Niklas Söderlund wrote:
> > > > Hello,
> > > >
> > > > This series refresh the RkISP1 pipeline handler to match what is in the
> > > > media-tree. As the upstream driver is in staging and bound to change
> > > > quiet a bit no attempt to keep the libcamera pipeline backwards
> > > > compatible have been made. To fully test this on must run a system no
> > > > older then [1] from the linux-media tree.
> > > >
> > > > Running a system newer then v5.8 without this series will also fail as
> > > > there are upstream changes breaking current libcamera master.
> > > >
> > > > Apart from aligning the pipeline with the upstream driver it's now
> > > > trivial to enable support for R8, RGB656 and XBGR8888 in the pipeline.
> > > > As these have been troublesome in the past due to issues now fixed
> > > > upstream every format supported by the pipeline (YUYV, NV16, NV61, NV21,
> > > > NV12, R8, RGB565 and XRGB8888) have been retested and converted using
> > > > raw2rgbpnm to validate that the captured data is good.
> > > >
> > > > 1. 7eba47ab7a310ed8 ("media: staging: rkisp1: cap: protect access to buf with the spin lock")
> > > >
> > > > Niklas Söderlund (6):
> > > >   libcamera: pipeline: rkisp1: Remove support for YVYU and VYUY
> > > >   libcamera: pipeline: rkisp1: Fix media bus propagation for NV{12,21}
> > > >   libcamera: pipeline: rkisp1: Add support for RGB656
> > > >   libcamera: pipeline: rkisp1: Add support for R8
> > > >   libcamera: Add support for XRGB8888 and XBGR8888
> > > >   libcamera: pipeline: rkisp1: Add support for XRGB8888
> > >
> > > For the whole series,
> > >
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > With the small comments clarified, for the series:
> >
> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
>
> Thanks for reviewing this series, unfortunately the series is already
> merged so I can't record your effort :-(
>

Please revert and re-push with my tags

:p

Sorry for being late to the party

> >
> > Thanks
> >   j
> >
> > >
> > > Could you however please follow up with upstream to replace
> > > MEDIA_BUS_FMT_YUYV8_1_5X8 with MEDIA_BUS_FMT_YUYV8_1X16 ?
> > >
> > > >  src/libcamera/formats.cpp                     | 20 ++++++++++++++
> > > >  src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 27 +++++++++++++------
> > > >  src/libcamera/v4l2_pixelformat.cpp            |  2 ++
> > > >  3 files changed, 41 insertions(+), 8 deletions(-)
> > > >
> > >
> > > --
> > > Regards,
> > >
> > > Laurent Pinchart
> > > _______________________________________________
> > > libcamera-devel mailing list
> > > libcamera-devel@lists.libcamera.org
> > > https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards,
> Niklas Söderlund