Re: [PATCH v4 7/7] v4l: Correct conflicting V4L2 subdev selection API documentation

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Sakari,

On Friday 15 June 2012 18:31:50 Sakari Ailus wrote:
> On Fri, Jun 15, 2012 at 04:14:21PM +0200, Laurent Pinchart wrote:
> > On Friday 15 June 2012 16:44:40 Sakari Ailus wrote:
> > > The API reference documents that the KEEP_CONFIG flag tells the
> > > configuration should not be propatgated by the driver whereas the
> > > interface
> > 
> > s/propatgated/propagated/
> > 
> > > documentation (dev-subdev.xml) categorically prohibited any changes to
> > > the
> > > rest of the pipeline. The latter makes no sense, since it would severely
> > > limit the usefulness of the KEEP_CONFIG flag.
> > > 
> > > Correct the documentation in dev-subddev.xml.
> > > 
> > > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> > > ---
> > > 
> > >  Documentation/DocBook/media/v4l/dev-subdev.xml |   10 +++++-----
> > >  1 files changed, 5 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/Documentation/DocBook/media/v4l/dev-subdev.xml
> > > b/Documentation/DocBook/media/v4l/dev-subdev.xml index 8c44b3f..95ebf87
> > > 100644
> > > --- a/Documentation/DocBook/media/v4l/dev-subdev.xml
> > > +++ b/Documentation/DocBook/media/v4l/dev-subdev.xml
> > > @@ -361,11 +361,11 @@
> > > 
> > >        performed by the user: the changes made will be propagated to
> > >        any subsequent stages. If this behaviour is not desired, the
> > >        user must set
> > > 
> > > -      <constant>V4L2_SUBDEV_SEL_FLAG_KEEP_CONFIG</constant> flag. This
> > > -      flag causes no propagation of the changes are allowed in any
> > > -      circumstances. This may also cause the accessed rectangle to be
> > > -      adjusted by the driver, depending on the properties of the
> > > -      underlying hardware.</para>
> > > +      <constant>V4L2_SUBDEV_SEL_FLAG_KEEP_CONFIG</constant> flag,
> > 
> > This should be V4L2_SEL_FLAG_KEEP_CONFIG.
> > 
> > > +      which tells the driver to make minimum changes to the rest of
> > > +      the subdev's configuration.
> > 
> > I'm not sure to like this. "minimum changes" is not clearly defined. Isn't
> > the point of the KEEP_CONFIG flag is to avoid propagating *any* change
> > down the pipeline inside the subdev ?
> 
> Yes, but the hardware may have restrictions that essentially makes the
> configuration static is absolutely no changes are allowed elsewhere. In
> those cases it should be possible to allow changes elsewhere.
> 
> Or do you think we should just completely disallow them? Would that work
> e.g. for the OMAP 3 ISP resizer?

I think the point of KEEP_CONFIG was to disallow changes completely, to make 
it possible to change the digital zoom factor during streaming for instance. 
The OMAP3 ISP resizer should accomodate that. If we allow changes under "some 
circumstances" applications won't be able to rely on the flag.

Tomasz, Sylwester, what's your opinion on this ?

-- 
Regards,

Laurent Pinchart