linux-kernel - Re: [PATCH 1/1] staging: ipu3-imgu: Move the UAPI header from include under include/uapi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 13 Apr 2021 17:53:15 +0300
From:   Sakari Ailus <sakari.ailus@...ux.intel.com>
To:     Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc:     linux-media@...r.kernel.org, Greg KH <gregkh@...uxfoundation.org>,
        Mitali Borkar <mitaliborkar810@...il.com>,
        bingbu.cao@...el.com, tian.shu.qiu@...el.com, mchehab@...nel.org,
        linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org,
        outreachy-kernel@...glegroups.com, mitali_s@...iitr.ac.in
Subject: Re: [PATCH 1/1] staging: ipu3-imgu: Move the UAPI header from
 include under include/uapi

Hi Laurent,

On Tue, Apr 13, 2021 at 04:45:17PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Mon, Apr 12, 2021 at 02:11:20PM +0300, Sakari Ailus wrote:
> > The header defines the user space interface but may be mistaken as
> > kernel-only header due to its location. Add "uapi" directory under
> > driver's include directory and move the header there.
> > 
> > Suggested-by: Greg KH <gregkh@...uxfoundation.org>
> > Signed-off-by: Sakari Ailus <sakari.ailus@...ux.intel.com>
> > ---
> >  Documentation/admin-guide/media/ipu3.rst      | 35 ++++++++++---------
> >  .../media/v4l/pixfmt-meta-intel-ipu3.rst      |  2 +-
> >  .../ipu3/include/{ => uapi}/intel-ipu3.h      |  0
> >  drivers/staging/media/ipu3/ipu3-abi.h         |  2 +-
> >  4 files changed, 20 insertions(+), 19 deletions(-)
> >  rename drivers/staging/media/ipu3/include/{ => uapi}/intel-ipu3.h (100%)
> > 
> > diff --git a/Documentation/admin-guide/media/ipu3.rst b/Documentation/admin-guide/media/ipu3.rst
> > index f59697c7b374..d6454f637ff4 100644
> > --- a/Documentation/admin-guide/media/ipu3.rst
> > +++ b/Documentation/admin-guide/media/ipu3.rst
> > @@ -234,22 +234,23 @@ The IPU3 ImgU pipelines can be configured using the Media Controller, defined at
> >  Running mode and firmware binary selection
> >  ------------------------------------------
> >  
> > -ImgU works based on firmware, currently the ImgU firmware support run 2 pipes in
> > -time-sharing with single input frame data. Each pipe can run at certain mode -
> > -"VIDEO" or "STILL", "VIDEO" mode is commonly used for video frames capture, and
> > -"STILL" is used for still frame capture. However, you can also select "VIDEO" to
> > -capture still frames if you want to capture images with less system load and
> > -power. For "STILL" mode, ImgU will try to use smaller BDS factor and output
> > -larger bayer frame for further YUV processing than "VIDEO" mode to get high
> > -quality images. Besides, "STILL" mode need XNR3 to do noise reduction, hence
> > -"STILL" mode will need more power and memory bandwidth than "VIDEO" mode. TNR
> > -will be enabled in "VIDEO" mode and bypassed by "STILL" mode. ImgU is running at
> > -“VIDEO” mode by default, the user can use v4l2 control V4L2_CID_INTEL_IPU3_MODE
> > -(currently defined in drivers/staging/media/ipu3/include/intel-ipu3.h) to query
> > -and set the running mode. For user, there is no difference for buffer queueing
> > -between the "VIDEO" and "STILL" mode, mandatory input and main output node
> > -should be enabled and buffers need be queued, the statistics and the view-finder
> > -queues are optional.
> > +ImgU works based on firmware, currently the ImgU firmware support run 2 pipes
> > +in time-sharing with single input frame data. Each pipe can run at certain mode
> > +- "VIDEO" or "STILL", "VIDEO" mode is commonly used for video frames capture,
> > +and "STILL" is used for still frame capture. However, you can also select
> > +"VIDEO" to capture still frames if you want to capture images with less system
> > +load and power. For "STILL" mode, ImgU will try to use smaller BDS factor and
> > +output larger bayer frame for further YUV processing than "VIDEO" mode to get
> > +high quality images. Besides, "STILL" mode need XNR3 to do noise reduction,
> > +hence "STILL" mode will need more power and memory bandwidth than "VIDEO" mode.
> > +TNR will be enabled in "VIDEO" mode and bypassed by "STILL" mode. ImgU is
> > +running at “VIDEO” mode by default, the user can use v4l2 control
> > +V4L2_CID_INTEL_IPU3_MODE (currently defined in
> > +drivers/staging/media/ipu3/include/uapi/intel-ipu3.h) to query and set the
> > +running mode. For user, there is no difference for buffer queueing between the
> > +"VIDEO" and "STILL" mode, mandatory input and main output node should be
> > +enabled and buffers need be queued, the statistics and the view-finder queues
> > +are optional.
> 
> The reflow of the whole paragraph is a bit painful to review.

Yes. The only difference there is still the addition of "/uapi". :-)

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

Thank you!

-- 
Sakari Ailus