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 <gre...@linuxfoundation.org>
> > Signed-off-by: Sakari Ailus <sakari.ai...@linux.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.pinch...@ideasonboard.com>
Thank you!
--
Sakari Ailus
