Re: [PATCH] [media] davinci: vpfe: Add documentation
Hi Rob, On Thursday 16 August 2012 06:40 PM, Rob Landley wrote: > On 07/11/2012 10:39 AM, Manjunath Hadli wrote: >> Add documentation on the Davinci VPFE driver. Document the subdevs, >> and private IOTCLs the driver implements >> >> Signed-off-by: Manjunath Hadli >> Signed-off-by: Lad, Prabhakar > > I saw the comment on the 8th, is there another version of this > documentation coming...? > I was waiting for comments from Sakari/Laurent, If they are happy from what Manju has proposed, depending on the outcome of the discussion I'll soon post another version soon. Thanks and Regards, --Prabhakar > Rob > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [media] davinci: vpfe: Add documentation
On 07/11/2012 10:39 AM, Manjunath Hadli wrote: > Add documentation on the Davinci VPFE driver. Document the subdevs, > and private IOTCLs the driver implements > > Signed-off-by: Manjunath Hadli > Signed-off-by: Lad, Prabhakar I saw the comment on the 8th, is there another version of this documentation coming...? Rob -- GNU/Linux isn't: Linux=GPLv2, GNU=GPLv3+, they can't share code. Either it's "mere aggregation", or a license violation. Pick one. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [media] davinci: vpfe: Add documentation
On 07/11/2012 10:39 AM, Manjunath Hadli wrote: Add documentation on the Davinci VPFE driver. Document the subdevs, and private IOTCLs the driver implements Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Signed-off-by: Lad, Prabhakar prabhakar@ti.com I saw the comment on the 8th, is there another version of this documentation coming...? Rob -- GNU/Linux isn't: Linux=GPLv2, GNU=GPLv3+, they can't share code. Either it's mere aggregation, or a license violation. Pick one. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [media] davinci: vpfe: Add documentation
Hi Rob, On Thursday 16 August 2012 06:40 PM, Rob Landley wrote: On 07/11/2012 10:39 AM, Manjunath Hadli wrote: Add documentation on the Davinci VPFE driver. Document the subdevs, and private IOTCLs the driver implements Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Signed-off-by: Lad, Prabhakar prabhakar@ti.com I saw the comment on the 8th, is there another version of this documentation coming...? I was waiting for comments from Sakari/Laurent, If they are happy from what Manju has proposed, depending on the outcome of the discussion I'll soon post another version soon. Thanks and Regards, --Prabhakar Rob -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [media] davinci: vpfe: Add documentation
Hi Manjunath,
On Tuesday 31 July 2012 13:15:27 Manju wrote:
> On Friday 27 July 2012 04:19 PM, Laurent Pinchart wrote:
> > On Friday 27 July 2012 05:49:24 Hadli, Manjunath wrote:
> >> On Thu, Jul 26, 2012 at 05:55:31, Laurent Pinchart wrote:
> >>> On Tuesday 17 July 2012 10:43:54 Hadli, Manjunath wrote:
> On Sun, Jul 15, 2012 at 18:16:25, Laurent Pinchart wrote:
> > On Wednesday 11 July 2012 21:09:26 Manjunath Hadli wrote:
> >> Add documentation on the Davinci VPFE driver. Document the subdevs,
> >> and private IOTCLs the driver implements
> >>
> >> Signed-off-by: Manjunath Hadli
> >> Signed-off-by: Lad, Prabhakar
> >>>
> >>> [snip]
> >>>
> >> +Private IOCTLs
> >> +==
> >> +
> >> +The Davinci Video processing Front End (VPFE) driver supports
> >> standard V4L2
> >> +IOCTLs and controls where possible and practical. Much of the
> >> functions provided
> >> +by the VPFE, however, does not fall under the standard IOCTLs.
> >> +
> >> +In general, there is a private ioctl for configuring each of the
> >> blocks
> >> +containing hardware-dependent functions.
> >> +
> >> +The following private IOCTLs are supported:
> >> +
> >> +1: IOCTL: PREV_S_PARAM/PREV_G_PARAM
> >> +Description:
> >> + Sets/Gets the parameters required by the previewer module
> >> +Parameter:
> >> + /**
> >> + * struct prev_module_param- structure to configure preview
> >> modules
> >> + * @version: Version of the preview module
> >
> > Who is responsible for filling this field, the application or the
> > driver ?
>
> The application is responsible for filling this info. He would
> enumerate the capabilities first and set them using S_PARAM/G_PARAM.
> >>>
> >>> And what's the point of the application setting the version field ? How
> >>> does the driver use it ?
> >>
> >> The version may not be required. Will remove it.
> >>
> >> + * @len: Length of the module config structure
> >> + * @module_id: Module id
> >> + * @param: pointer to module config parameter.
> >
> > What is module_id for ? What does param point to ?
>
> There are a lot of tiny modules in the previewer/resizer which are
> enumerated as individual modules. The param points to the parameter set
> that the module expects to be set.
> >>>
> >>> Why don't you implement something similar to
> >>> VPFE_CMD_S_CCDC_RAW_PARAMS/VPFE_CMD_G_CCDC_RAW_PARAMS instead ?
> >>
> >> I feel if we implement direct IOCTLS there might be many of them. To make
> >> sure than independent of the number of internal modules present, having
> >> the
> >> same IOCTL used for all modules is a good idea.
> >
> > You can set several parameters using a single ioctl, much like
> > VPFE_CMD_S_CCDC_RAW_PARAMS does. You don't need one ioctl per parameter.
> >
> > PREV_ENUM_CAP, PREV_[GS]_PARAM and PREV_[GS]_CONFIG are essentially
> > reinventing V4L2 controls, and I don't think that's a good idea.
>
> Ok. I looked into this, and found that the structure needed to pass
> all the parameters is going to be huge. just to avoid a big structure
> from the user space, I propose:
>
> Having a union of structures and a parameter identifying the structure.
>
> In that way, we will remove the enumeration and all the other
> things except for a SET and GET, much like the CCDC_RAW_PARAMS
> like you suggested. So essentially we will have only 2 IOCTLS for setting
> the private params/configs and remove the rest. I hope that was your
> point and this proposal will solve it?
What about something like the following structure, from the OMAP3 ISP driver ?
struct omap3isp_prev_update_config {
__u32 update;
__u32 flag;
__u32 shading_shift;
struct omap3isp_prev_luma __user *luma;
struct omap3isp_prev_hmed __user *hmed;
struct omap3isp_prev_cfa __user *cfa;
struct omap3isp_prev_csup __user *csup;
struct omap3isp_prev_wbal __user *wbal;
struct omap3isp_prev_blkadj __user *blkadj;
struct omap3isp_prev_rgbtorgb __user *rgb2rgb;
struct omap3isp_prev_csc __user *csc;
struct omap3isp_prev_yclimit __user *yclimit;
struct omap3isp_prev_dcor __user *dcor;
struct omap3isp_prev_nf __user *nf;
struct omap3isp_prev_gtables __user *gamma;
};
I'll probably have more comments when I'll see the complete list of parameters
you need to expose.
> >> + */
> >> + struct prev_module_param {
> >> + char version[IMP_MAX_NAME_SIZE];
> >
> > Is there a need to express the version as a string instead of an
> > integer ?
>
> It could be integer. It is generally a fixed point num, and easy to
> read
> it as a string than an integer. Can I keep it as a string?
> >>>
> >>> Let's first decide whether a version field is
Re: [PATCH] [media] davinci: vpfe: Add documentation
Hi Manjunath,
On Tuesday 31 July 2012 13:15:27 Manju wrote:
On Friday 27 July 2012 04:19 PM, Laurent Pinchart wrote:
On Friday 27 July 2012 05:49:24 Hadli, Manjunath wrote:
On Thu, Jul 26, 2012 at 05:55:31, Laurent Pinchart wrote:
On Tuesday 17 July 2012 10:43:54 Hadli, Manjunath wrote:
On Sun, Jul 15, 2012 at 18:16:25, Laurent Pinchart wrote:
On Wednesday 11 July 2012 21:09:26 Manjunath Hadli wrote:
Add documentation on the Davinci VPFE driver. Document the subdevs,
and private IOTCLs the driver implements
Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com
Signed-off-by: Lad, Prabhakar prabhakar@ti.com
[snip]
+Private IOCTLs
+==
+
+The Davinci Video processing Front End (VPFE) driver supports
standard V4L2
+IOCTLs and controls where possible and practical. Much of the
functions provided
+by the VPFE, however, does not fall under the standard IOCTLs.
+
+In general, there is a private ioctl for configuring each of the
blocks
+containing hardware-dependent functions.
+
+The following private IOCTLs are supported:
+
+1: IOCTL: PREV_S_PARAM/PREV_G_PARAM
+Description:
+ Sets/Gets the parameters required by the previewer module
+Parameter:
+ /**
+ * struct prev_module_param- structure to configure preview
modules
+ * @version: Version of the preview module
Who is responsible for filling this field, the application or the
driver ?
The application is responsible for filling this info. He would
enumerate the capabilities first and set them using S_PARAM/G_PARAM.
And what's the point of the application setting the version field ? How
does the driver use it ?
The version may not be required. Will remove it.
+ * @len: Length of the module config structure
+ * @module_id: Module id
+ * @param: pointer to module config parameter.
What is module_id for ? What does param point to ?
There are a lot of tiny modules in the previewer/resizer which are
enumerated as individual modules. The param points to the parameter set
that the module expects to be set.
Why don't you implement something similar to
VPFE_CMD_S_CCDC_RAW_PARAMS/VPFE_CMD_G_CCDC_RAW_PARAMS instead ?
I feel if we implement direct IOCTLS there might be many of them. To make
sure than independent of the number of internal modules present, having
the
same IOCTL used for all modules is a good idea.
You can set several parameters using a single ioctl, much like
VPFE_CMD_S_CCDC_RAW_PARAMS does. You don't need one ioctl per parameter.
PREV_ENUM_CAP, PREV_[GS]_PARAM and PREV_[GS]_CONFIG are essentially
reinventing V4L2 controls, and I don't think that's a good idea.
Ok. I looked into this, and found that the structure needed to pass
all the parameters is going to be huge. just to avoid a big structure
from the user space, I propose:
Having a union of structures and a parameter identifying the structure.
In that way, we will remove the enumeration and all the other
things except for a SET and GET, much like the CCDC_RAW_PARAMS
like you suggested. So essentially we will have only 2 IOCTLS for setting
the private params/configs and remove the rest. I hope that was your
point and this proposal will solve it?
What about something like the following structure, from the OMAP3 ISP driver ?
struct omap3isp_prev_update_config {
__u32 update;
__u32 flag;
__u32 shading_shift;
struct omap3isp_prev_luma __user *luma;
struct omap3isp_prev_hmed __user *hmed;
struct omap3isp_prev_cfa __user *cfa;
struct omap3isp_prev_csup __user *csup;
struct omap3isp_prev_wbal __user *wbal;
struct omap3isp_prev_blkadj __user *blkadj;
struct omap3isp_prev_rgbtorgb __user *rgb2rgb;
struct omap3isp_prev_csc __user *csc;
struct omap3isp_prev_yclimit __user *yclimit;
struct omap3isp_prev_dcor __user *dcor;
struct omap3isp_prev_nf __user *nf;
struct omap3isp_prev_gtables __user *gamma;
};
I'll probably have more comments when I'll see the complete list of parameters
you need to expose.
+ */
+ struct prev_module_param {
+ char version[IMP_MAX_NAME_SIZE];
Is there a need to express the version as a string instead of an
integer ?
It could be integer. It is generally a fixed point num, and easy to
read
it as a string than an integer. Can I keep it as a string?
Let's first decide whether a version field is needed at all :-)
Will remove.
+ unsigned short len;
+ unsigned short module_id;
+ void *param;
+ };
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at
Re: [PATCH] [media] davinci: vpfe: Add documentation
Hi Laurent,
On Friday 27 July 2012 04:19 PM, Laurent Pinchart wrote:
Hi Manjunath,
On Friday 27 July 2012 05:49:24 Hadli, Manjunath wrote:
On Thu, Jul 26, 2012 at 05:55:31, Laurent Pinchart wrote:
On Tuesday 17 July 2012 10:43:54 Hadli, Manjunath wrote:
On Sun, Jul 15, 2012 at 18:16:25, Laurent Pinchart wrote:
On Wednesday 11 July 2012 21:09:26 Manjunath Hadli wrote:
Add documentation on the Davinci VPFE driver. Document the subdevs,
and private IOTCLs the driver implements
Signed-off-by: Manjunath Hadli
Signed-off-by: Lad, Prabhakar
[snip]
+Private IOCTLs
+==
+
+The Davinci Video processing Front End (VPFE) driver supports
standard V4L2
+IOCTLs and controls where possible and practical. Much of the
functions provided
+by the VPFE, however, does not fall under the standard IOCTLs.
+
+In general, there is a private ioctl for configuring each of the
blocks
+containing hardware-dependent functions.
+
+The following private IOCTLs are supported:
+
+1: IOCTL: PREV_S_PARAM/PREV_G_PARAM
+Description:
+ Sets/Gets the parameters required by the previewer module
+Parameter:
+ /**
+* struct prev_module_param- structure to configure preview
modules
+* @version: Version of the preview module
Who is responsible for filling this field, the application or the
driver ?
The application is responsible for filling this info. He would enumerate
the capabilities first and set them using S_PARAM/G_PARAM.
And what's the point of the application setting the version field ? How
does the driver use it ?
The version may not be required. Will remove it.
+* @len: Length of the module config structure
+* @module_id: Module id
+* @param: pointer to module config parameter.
What is module_id for ? What does param point to ?
There are a lot of tiny modules in the previewer/resizer which are
enumerated as individual modules. The param points to the parameter set
that the module expects to be set.
Why don't you implement something similar to
VPFE_CMD_S_CCDC_RAW_PARAMS/VPFE_CMD_G_CCDC_RAW_PARAMS instead ?
I feel if we implement direct IOCTLS there might be many of them. To make
sure than independent of the number of internal modules present, having the
same IOCTL used for all modules is a good idea.
You can set several parameters using a single ioctl, much like
VPFE_CMD_S_CCDC_RAW_PARAMS does. You don't need one ioctl per parameter.
PREV_ENUM_CAP, PREV_[GS]_PARAM and PREV_[GS]_CONFIG are essentially
reinventing V4L2 controls, and I don't think that's a good idea.
Ok. I looked into this, and found that the structure needed to pass
all the parameters is going to be huge. just to avoid a big structure
from the user space, I propose:
Having a union of structures and a parameter identifying the structure.
In that way, we will remove the enumeration and all the other
things except for a SET and GET, much like the CCDC_RAW_PARAMS
like you suggested. So essentially we will have only 2 IOCTLS for setting
the private params/configs and remove the rest. I hope that was your
point and this proposal will solve it?
+*/
+ struct prev_module_param {
+ char version[IMP_MAX_NAME_SIZE];
Is there a need to express the version as a string instead of an
integer ?
It could be integer. It is generally a fixed point num, and easy to read
it as a string than an integer. Can I keep it as a string?
Let's first decide whether a version field is needed at all :-)
Will remove.
+ unsigned short len;
+ unsigned short module_id;
+ void *param;
+ };
+
+2: IOCTL: PREV_S_CONFIG/PREV_G_CONFIG
+Description:
+ Sets/Gets the configuration required by the previewer channel
+Parameter:
+ /**
+* struct prev_channel_config - structure for configuring the
previewer
channel
+* @len: Length of the user configuration
+* @config: pointer to either single shot config or continuous
+*/
+ struct prev_channel_config {
+ unsigned short len;
+ void *config;
+ };
What's the difference between parameters and configuration ? What does
config point to ?
Config is setting which is required for a subdev to function based on
what it is set for (single shot/continuous.) common to all platforms.
Parameters are the settings for individual small sub-ips which might be
slightly different from one platform to another. Config points to
prev_single_shot_config or prev_continuous_config currently defined in
linux/dm3656ipipe.h. I think we will move it to a common location.
Why don't you implement something similar to
VPFE_CMD_S_CCDC_RAW_PARAMS/VPFE_CMD_G_CCDC_RAW_PARAMS here as well (same
for the resizer configuration ioctls) ?
Ditto.
+
+3: IOCTL: PREV_ENUM_CAP
+Description:
+ Queries the modules available in the image processor for preview
the
+ input image.
+Parameter:
+ /**
+* struct prev_cap - structure to
Re: [PATCH] [media] davinci: vpfe: Add documentation
Hi Laurent,
On Friday 27 July 2012 04:19 PM, Laurent Pinchart wrote:
Hi Manjunath,
On Friday 27 July 2012 05:49:24 Hadli, Manjunath wrote:
On Thu, Jul 26, 2012 at 05:55:31, Laurent Pinchart wrote:
On Tuesday 17 July 2012 10:43:54 Hadli, Manjunath wrote:
On Sun, Jul 15, 2012 at 18:16:25, Laurent Pinchart wrote:
On Wednesday 11 July 2012 21:09:26 Manjunath Hadli wrote:
Add documentation on the Davinci VPFE driver. Document the subdevs,
and private IOTCLs the driver implements
Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com
Signed-off-by: Lad, Prabhakar prabhakar@ti.com
[snip]
+Private IOCTLs
+==
+
+The Davinci Video processing Front End (VPFE) driver supports
standard V4L2
+IOCTLs and controls where possible and practical. Much of the
functions provided
+by the VPFE, however, does not fall under the standard IOCTLs.
+
+In general, there is a private ioctl for configuring each of the
blocks
+containing hardware-dependent functions.
+
+The following private IOCTLs are supported:
+
+1: IOCTL: PREV_S_PARAM/PREV_G_PARAM
+Description:
+ Sets/Gets the parameters required by the previewer module
+Parameter:
+ /**
+* struct prev_module_param- structure to configure preview
modules
+* @version: Version of the preview module
Who is responsible for filling this field, the application or the
driver ?
The application is responsible for filling this info. He would enumerate
the capabilities first and set them using S_PARAM/G_PARAM.
And what's the point of the application setting the version field ? How
does the driver use it ?
The version may not be required. Will remove it.
+* @len: Length of the module config structure
+* @module_id: Module id
+* @param: pointer to module config parameter.
What is module_id for ? What does param point to ?
There are a lot of tiny modules in the previewer/resizer which are
enumerated as individual modules. The param points to the parameter set
that the module expects to be set.
Why don't you implement something similar to
VPFE_CMD_S_CCDC_RAW_PARAMS/VPFE_CMD_G_CCDC_RAW_PARAMS instead ?
I feel if we implement direct IOCTLS there might be many of them. To make
sure than independent of the number of internal modules present, having the
same IOCTL used for all modules is a good idea.
You can set several parameters using a single ioctl, much like
VPFE_CMD_S_CCDC_RAW_PARAMS does. You don't need one ioctl per parameter.
PREV_ENUM_CAP, PREV_[GS]_PARAM and PREV_[GS]_CONFIG are essentially
reinventing V4L2 controls, and I don't think that's a good idea.
Ok. I looked into this, and found that the structure needed to pass
all the parameters is going to be huge. just to avoid a big structure
from the user space, I propose:
Having a union of structures and a parameter identifying the structure.
In that way, we will remove the enumeration and all the other
things except for a SET and GET, much like the CCDC_RAW_PARAMS
like you suggested. So essentially we will have only 2 IOCTLS for setting
the private params/configs and remove the rest. I hope that was your
point and this proposal will solve it?
+*/
+ struct prev_module_param {
+ char version[IMP_MAX_NAME_SIZE];
Is there a need to express the version as a string instead of an
integer ?
It could be integer. It is generally a fixed point num, and easy to read
it as a string than an integer. Can I keep it as a string?
Let's first decide whether a version field is needed at all :-)
Will remove.
+ unsigned short len;
+ unsigned short module_id;
+ void *param;
+ };
+
+2: IOCTL: PREV_S_CONFIG/PREV_G_CONFIG
+Description:
+ Sets/Gets the configuration required by the previewer channel
+Parameter:
+ /**
+* struct prev_channel_config - structure for configuring the
previewer
channel
+* @len: Length of the user configuration
+* @config: pointer to either single shot config or continuous
+*/
+ struct prev_channel_config {
+ unsigned short len;
+ void *config;
+ };
What's the difference between parameters and configuration ? What does
config point to ?
Config is setting which is required for a subdev to function based on
what it is set for (single shot/continuous.) common to all platforms.
Parameters are the settings for individual small sub-ips which might be
slightly different from one platform to another. Config points to
prev_single_shot_config or prev_continuous_config currently defined in
linux/dm3656ipipe.h. I think we will move it to a common location.
Why don't you implement something similar to
VPFE_CMD_S_CCDC_RAW_PARAMS/VPFE_CMD_G_CCDC_RAW_PARAMS here as well (same
for the resizer configuration ioctls) ?
Ditto.
+
+3: IOCTL: PREV_ENUM_CAP
+Description:
+ Queries the modules available in the image processor for preview
the
+ input image.
+Parameter:
+ /**
+
Re: [PATCH] [media] davinci: vpfe: Add documentation
Hi Manjunath,
On Friday 27 July 2012 05:49:24 Hadli, Manjunath wrote:
> On Thu, Jul 26, 2012 at 05:55:31, Laurent Pinchart wrote:
> > On Tuesday 17 July 2012 10:43:54 Hadli, Manjunath wrote:
> > > On Sun, Jul 15, 2012 at 18:16:25, Laurent Pinchart wrote:
> > > > On Wednesday 11 July 2012 21:09:26 Manjunath Hadli wrote:
> > > > > Add documentation on the Davinci VPFE driver. Document the subdevs,
> > > > > and private IOTCLs the driver implements
> > > > >
> > > > > Signed-off-by: Manjunath Hadli
> > > > > Signed-off-by: Lad, Prabhakar
> >
> > [snip]
> >
> > > > > +Private IOCTLs
> > > > > +==
> > > > > +
> > > > > +The Davinci Video processing Front End (VPFE) driver supports
> > > > > standard V4L2
> > > > > +IOCTLs and controls where possible and practical. Much of the
> > > > > functions provided
> > > > > +by the VPFE, however, does not fall under the standard IOCTLs.
> > > > > +
> > > > > +In general, there is a private ioctl for configuring each of the
> > > > > blocks
> > > > > +containing hardware-dependent functions.
> > > > > +
> > > > > +The following private IOCTLs are supported:
> > > > > +
> > > > > +1: IOCTL: PREV_S_PARAM/PREV_G_PARAM
> > > > > +Description:
> > > > > + Sets/Gets the parameters required by the previewer module
> > > > > +Parameter:
> > > > > + /**
> > > > > + * struct prev_module_param- structure to configure preview
> > > > > modules
> > > > > + * @version: Version of the preview module
> > > >
> > > > Who is responsible for filling this field, the application or the
> > > > driver ?
> > >
> > > The application is responsible for filling this info. He would enumerate
> > > the capabilities first and set them using S_PARAM/G_PARAM.
> >
> > And what's the point of the application setting the version field ? How
> > does the driver use it ?
>
> The version may not be required. Will remove it.
>
> > > > > + * @len: Length of the module config structure
> > > > > + * @module_id: Module id
> > > > > + * @param: pointer to module config parameter.
> > > >
> > > > What is module_id for ? What does param point to ?
> > >
> > > There are a lot of tiny modules in the previewer/resizer which are
> > > enumerated as individual modules. The param points to the parameter set
> > > that the module expects to be set.
> >
> > Why don't you implement something similar to
> > VPFE_CMD_S_CCDC_RAW_PARAMS/VPFE_CMD_G_CCDC_RAW_PARAMS instead ?
>
> I feel if we implement direct IOCTLS there might be many of them. To make
> sure than independent of the number of internal modules present, having the
> same IOCTL used for all modules is a good idea.
You can set several parameters using a single ioctl, much like
VPFE_CMD_S_CCDC_RAW_PARAMS does. You don't need one ioctl per parameter.
PREV_ENUM_CAP, PREV_[GS]_PARAM and PREV_[GS]_CONFIG are essentially
reinventing V4L2 controls, and I don't think that's a good idea.
> > > > > + */
> > > > > + struct prev_module_param {
> > > > > + char version[IMP_MAX_NAME_SIZE];
> > > >
> > > > Is there a need to express the version as a string instead of an
> > > > integer ?
> > >
> > > It could be integer. It is generally a fixed point num, and easy to read
> > > it as a string than an integer. Can I keep it as a string?
> >
> > Let's first decide whether a version field is needed at all :-)
>
> Will remove.
>
> > > > > + unsigned short len;
> > > > > + unsigned short module_id;
> > > > > + void *param;
> > > > > + };
> > > > > +
> > > > > +2: IOCTL: PREV_S_CONFIG/PREV_G_CONFIG
> > > > > +Description:
> > > > > + Sets/Gets the configuration required by the previewer channel
> > > > > +Parameter:
> > > > > + /**
> > > > > + * struct prev_channel_config - structure for configuring the
> > > > > previewer
> > > > > channel
> > > > > + * @len: Length of the user configuration
> > > > > + * @config: pointer to either single shot config or continuous
> > > > > + */
> > > > > + struct prev_channel_config {
> > > > > + unsigned short len;
> > > > > + void *config;
> > > > > + };
> > > >
> > > > What's the difference between parameters and configuration ? What does
> > > > config point to ?
> > >
> > > Config is setting which is required for a subdev to function based on
> > > what it is set for (single shot/continuous.) common to all platforms.
> > > Parameters are the settings for individual small sub-ips which might be
> > > slightly different from one platform to another. Config points to
> > > prev_single_shot_config or prev_continuous_config currently defined in
> > > linux/dm3656ipipe.h. I think we will move it to a common location.
> >
> > Why don't you implement something similar to
> > VPFE_CMD_S_CCDC_RAW_PARAMS/VPFE_CMD_G_CCDC_RAW_PARAMS here as well (same
> > for the resizer configuration ioctls) ?
>
> Ditto.
>
> > > > > +
> > > > > +3: IOCTL: PREV_ENUM_CAP
> > > > >
Re: [PATCH] [media] davinci: vpfe: Add documentation
Hi Manjunath,
On Friday 27 July 2012 05:49:24 Hadli, Manjunath wrote:
On Thu, Jul 26, 2012 at 05:55:31, Laurent Pinchart wrote:
On Tuesday 17 July 2012 10:43:54 Hadli, Manjunath wrote:
On Sun, Jul 15, 2012 at 18:16:25, Laurent Pinchart wrote:
On Wednesday 11 July 2012 21:09:26 Manjunath Hadli wrote:
Add documentation on the Davinci VPFE driver. Document the subdevs,
and private IOTCLs the driver implements
Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com
Signed-off-by: Lad, Prabhakar prabhakar@ti.com
[snip]
+Private IOCTLs
+==
+
+The Davinci Video processing Front End (VPFE) driver supports
standard V4L2
+IOCTLs and controls where possible and practical. Much of the
functions provided
+by the VPFE, however, does not fall under the standard IOCTLs.
+
+In general, there is a private ioctl for configuring each of the
blocks
+containing hardware-dependent functions.
+
+The following private IOCTLs are supported:
+
+1: IOCTL: PREV_S_PARAM/PREV_G_PARAM
+Description:
+ Sets/Gets the parameters required by the previewer module
+Parameter:
+ /**
+ * struct prev_module_param- structure to configure preview
modules
+ * @version: Version of the preview module
Who is responsible for filling this field, the application or the
driver ?
The application is responsible for filling this info. He would enumerate
the capabilities first and set them using S_PARAM/G_PARAM.
And what's the point of the application setting the version field ? How
does the driver use it ?
The version may not be required. Will remove it.
+ * @len: Length of the module config structure
+ * @module_id: Module id
+ * @param: pointer to module config parameter.
What is module_id for ? What does param point to ?
There are a lot of tiny modules in the previewer/resizer which are
enumerated as individual modules. The param points to the parameter set
that the module expects to be set.
Why don't you implement something similar to
VPFE_CMD_S_CCDC_RAW_PARAMS/VPFE_CMD_G_CCDC_RAW_PARAMS instead ?
I feel if we implement direct IOCTLS there might be many of them. To make
sure than independent of the number of internal modules present, having the
same IOCTL used for all modules is a good idea.
You can set several parameters using a single ioctl, much like
VPFE_CMD_S_CCDC_RAW_PARAMS does. You don't need one ioctl per parameter.
PREV_ENUM_CAP, PREV_[GS]_PARAM and PREV_[GS]_CONFIG are essentially
reinventing V4L2 controls, and I don't think that's a good idea.
+ */
+ struct prev_module_param {
+ char version[IMP_MAX_NAME_SIZE];
Is there a need to express the version as a string instead of an
integer ?
It could be integer. It is generally a fixed point num, and easy to read
it as a string than an integer. Can I keep it as a string?
Let's first decide whether a version field is needed at all :-)
Will remove.
+ unsigned short len;
+ unsigned short module_id;
+ void *param;
+ };
+
+2: IOCTL: PREV_S_CONFIG/PREV_G_CONFIG
+Description:
+ Sets/Gets the configuration required by the previewer channel
+Parameter:
+ /**
+ * struct prev_channel_config - structure for configuring the
previewer
channel
+ * @len: Length of the user configuration
+ * @config: pointer to either single shot config or continuous
+ */
+ struct prev_channel_config {
+ unsigned short len;
+ void *config;
+ };
What's the difference between parameters and configuration ? What does
config point to ?
Config is setting which is required for a subdev to function based on
what it is set for (single shot/continuous.) common to all platforms.
Parameters are the settings for individual small sub-ips which might be
slightly different from one platform to another. Config points to
prev_single_shot_config or prev_continuous_config currently defined in
linux/dm3656ipipe.h. I think we will move it to a common location.
Why don't you implement something similar to
VPFE_CMD_S_CCDC_RAW_PARAMS/VPFE_CMD_G_CCDC_RAW_PARAMS here as well (same
for the resizer configuration ioctls) ?
Ditto.
+
+3: IOCTL: PREV_ENUM_CAP
+Description:
+ Queries the modules available in the image processor for preview
the
+ input image.
+Parameter:
+ /**
+ * struct prev_cap - structure to enumerate capabilities of
previewer
+ * @index: application use this to iterate over the available
modules
+ * @version: version of the preview module
+
RE: [PATCH] [media] davinci: vpfe: Add documentation
Laurent,
Thank you for your comments.
On Thu, Jul 26, 2012 at 05:55:31, Laurent Pinchart wrote:
> Hi Manjunath,
>
> On Tuesday 17 July 2012 10:43:54 Hadli, Manjunath wrote:
> > On Sun, Jul 15, 2012 at 18:16:25, Laurent Pinchart wrote:
> > > On Wednesday 11 July 2012 21:09:26 Manjunath Hadli wrote:
> > > > Add documentation on the Davinci VPFE driver. Document the subdevs,
> > > > and private IOTCLs the driver implements
> > > >
> > > > Signed-off-by: Manjunath Hadli
> > > > Signed-off-by: Lad, Prabhakar
>
> [snip]
>
> > > > +Private IOCTLs
> > > > +==
> > > > +
> > > > +The Davinci Video processing Front End (VPFE) driver supports standard
> > > > V4L2 +IOCTLs and controls where possible and practical. Much of the
> > > > functions provided
> > > > +by the VPFE, however, does not fall under the standard IOCTLs.
> > > > +
> > > > +In general, there is a private ioctl for configuring each of the blocks
> > > > +containing hardware-dependent functions.
> > > > +
> > > > +The following private IOCTLs are supported:
> > > > +
> > > > +1: IOCTL: PREV_S_PARAM/PREV_G_PARAM
> > > > +Description:
> > > > + Sets/Gets the parameters required by the previewer module
> > > > +Parameter:
> > > > + /**
> > > > +* struct prev_module_param- structure to configure preview
> > > > modules
> > > > +* @version: Version of the preview module
> > >
> > > Who is responsible for filling this field, the application or the driver ?
> >
> > The application is responsible for filling this info. He would enumerate the
> > capabilities first and set them using S_PARAM/G_PARAM.
>
> And what's the point of the application setting the version field ? How does
> the driver use it ?
The version may not be required. Will remove it.
>
> > > > +* @len: Length of the module config structure
> > > > +* @module_id: Module id
> > > > +* @param: pointer to module config parameter.
> > >
> > > What is module_id for ? What does param point to ?
> >
> > There are a lot of tiny modules in the previewer/resizer which are
> > enumerated as individual modules. The param points to the parameter set
> > that the module expects to be set.
>
> Why don't you implement something similar to
> VPFE_CMD_S_CCDC_RAW_PARAMS/VPFE_CMD_G_CCDC_RAW_PARAMS instead ?
I feel if we implement direct IOCTLS there might be many of them. To make sure
than independent of the number of internal modules present, having the same
IOCTL used for all modules is a good idea.
>
> > > > +*/
> > > > + struct prev_module_param {
> > > > + char version[IMP_MAX_NAME_SIZE];
> > >
> > > Is there a need to express the version as a string instead of an integer ?
> >
> > It could be integer. It is generally a fixed point num, and easy to read it
> > as a string than an integer. Can I keep it as a string?
>
> Let's first decide whether a version field is needed at all :-)
Will remove.
>
> > > > + unsigned short len;
> > > > + unsigned short module_id;
> > > > + void *param;
> > > > + };
> > > > +
> > > > +2: IOCTL: PREV_S_CONFIG/PREV_G_CONFIG
> > > > +Description:
> > > > + Sets/Gets the configuration required by the previewer channel
> > > > +Parameter:
> > > > + /**
> > > > +* struct prev_channel_config - structure for configuring the
> > > > previewer
> > > > channel
> > > > +* @len: Length of the user configuration
> > > > +* @config: pointer to either single shot config or continuous
> > > > +*/
> > > > + struct prev_channel_config {
> > > > + unsigned short len;
> > > > + void *config;
> > > > + };
> > >
> > > What's the difference between parameters and configuration ? What does
> > > config point to ?
> >
> > Config is setting which is required for a subdev to function based on what
> > it is set for (single shot/continuous.) common to all platforms. Parameters
> > are the settings for individual small sub-ips which might be slightly
> > different from one platform to another. Config points to
> > prev_single_shot_config or prev_continuous_config currently defined in
> > linux/dm3656ipipe.h. I think we will move it to a common location.
>
> Why don't you implement something similar to
> VPFE_CMD_S_CCDC_RAW_PARAMS/VPFE_CMD_G_CCDC_RAW_PARAMS here as well (same for
> the resizer configuration ioctls) ?
>
Ditto.
> > > > +
> > > > +3: IOCTL: PREV_ENUM_CAP
> > > > +Description:
> > > > + Queries the modules available in the image processor for
> > > > preview the
> > > > + input image.
> > > > +Parameter:
> > > > + /**
> > > > +* struct prev_cap - structure to enumerate capabilities of
> > > > previewer
> > > > +* @index: application use this to iterate over the available
> > > > modules
> > > > +* @version: version of the preview module
> > > > +* @module_id: module id
> > > > +*
RE: [PATCH] [media] davinci: vpfe: Add documentation
Laurent,
Thank you for your comments.
On Thu, Jul 26, 2012 at 05:55:31, Laurent Pinchart wrote:
Hi Manjunath,
On Tuesday 17 July 2012 10:43:54 Hadli, Manjunath wrote:
On Sun, Jul 15, 2012 at 18:16:25, Laurent Pinchart wrote:
On Wednesday 11 July 2012 21:09:26 Manjunath Hadli wrote:
Add documentation on the Davinci VPFE driver. Document the subdevs,
and private IOTCLs the driver implements
Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com
Signed-off-by: Lad, Prabhakar prabhakar@ti.com
[snip]
+Private IOCTLs
+==
+
+The Davinci Video processing Front End (VPFE) driver supports standard
V4L2 +IOCTLs and controls where possible and practical. Much of the
functions provided
+by the VPFE, however, does not fall under the standard IOCTLs.
+
+In general, there is a private ioctl for configuring each of the blocks
+containing hardware-dependent functions.
+
+The following private IOCTLs are supported:
+
+1: IOCTL: PREV_S_PARAM/PREV_G_PARAM
+Description:
+ Sets/Gets the parameters required by the previewer module
+Parameter:
+ /**
+* struct prev_module_param- structure to configure preview
modules
+* @version: Version of the preview module
Who is responsible for filling this field, the application or the driver ?
The application is responsible for filling this info. He would enumerate the
capabilities first and set them using S_PARAM/G_PARAM.
And what's the point of the application setting the version field ? How does
the driver use it ?
The version may not be required. Will remove it.
+* @len: Length of the module config structure
+* @module_id: Module id
+* @param: pointer to module config parameter.
What is module_id for ? What does param point to ?
There are a lot of tiny modules in the previewer/resizer which are
enumerated as individual modules. The param points to the parameter set
that the module expects to be set.
Why don't you implement something similar to
VPFE_CMD_S_CCDC_RAW_PARAMS/VPFE_CMD_G_CCDC_RAW_PARAMS instead ?
I feel if we implement direct IOCTLS there might be many of them. To make sure
than independent of the number of internal modules present, having the same
IOCTL used for all modules is a good idea.
+*/
+ struct prev_module_param {
+ char version[IMP_MAX_NAME_SIZE];
Is there a need to express the version as a string instead of an integer ?
It could be integer. It is generally a fixed point num, and easy to read it
as a string than an integer. Can I keep it as a string?
Let's first decide whether a version field is needed at all :-)
Will remove.
+ unsigned short len;
+ unsigned short module_id;
+ void *param;
+ };
+
+2: IOCTL: PREV_S_CONFIG/PREV_G_CONFIG
+Description:
+ Sets/Gets the configuration required by the previewer channel
+Parameter:
+ /**
+* struct prev_channel_config - structure for configuring the
previewer
channel
+* @len: Length of the user configuration
+* @config: pointer to either single shot config or continuous
+*/
+ struct prev_channel_config {
+ unsigned short len;
+ void *config;
+ };
What's the difference between parameters and configuration ? What does
config point to ?
Config is setting which is required for a subdev to function based on what
it is set for (single shot/continuous.) common to all platforms. Parameters
are the settings for individual small sub-ips which might be slightly
different from one platform to another. Config points to
prev_single_shot_config or prev_continuous_config currently defined in
linux/dm3656ipipe.h. I think we will move it to a common location.
Why don't you implement something similar to
VPFE_CMD_S_CCDC_RAW_PARAMS/VPFE_CMD_G_CCDC_RAW_PARAMS here as well (same for
the resizer configuration ioctls) ?
Ditto.
+
+3: IOCTL: PREV_ENUM_CAP
+Description:
+ Queries the modules available in the image processor for
preview the
+ input image.
+Parameter:
+ /**
+* struct prev_cap - structure to enumerate capabilities of
previewer
+* @index: application use this to iterate over the available
modules
+* @version: version of the preview module
+* @module_id: module id
+* @control: control operation allowed in continuous mode? 1 -
allowed, 0
- not allowed
+* @path: path on which the module is sitting
+* @module_name: module name
+*/
+ struct prev_cap {
+ unsigned short index;
+ char
Re: [PATCH] [media] davinci: vpfe: Add documentation
Hi Manjunath,
On Tuesday 17 July 2012 10:43:54 Hadli, Manjunath wrote:
> On Sun, Jul 15, 2012 at 18:16:25, Laurent Pinchart wrote:
> > On Wednesday 11 July 2012 21:09:26 Manjunath Hadli wrote:
> > > Add documentation on the Davinci VPFE driver. Document the subdevs,
> > > and private IOTCLs the driver implements
> > >
> > > Signed-off-by: Manjunath Hadli
> > > Signed-off-by: Lad, Prabhakar
[snip]
> > > +Private IOCTLs
> > > +==
> > > +
> > > +The Davinci Video processing Front End (VPFE) driver supports standard
> > > V4L2 +IOCTLs and controls where possible and practical. Much of the
> > > functions provided
> > > +by the VPFE, however, does not fall under the standard IOCTLs.
> > > +
> > > +In general, there is a private ioctl for configuring each of the blocks
> > > +containing hardware-dependent functions.
> > > +
> > > +The following private IOCTLs are supported:
> > > +
> > > +1: IOCTL: PREV_S_PARAM/PREV_G_PARAM
> > > +Description:
> > > + Sets/Gets the parameters required by the previewer module
> > > +Parameter:
> > > + /**
> > > + * struct prev_module_param- structure to configure preview modules
> > > + * @version: Version of the preview module
> >
> > Who is responsible for filling this field, the application or the driver ?
>
> The application is responsible for filling this info. He would enumerate the
> capabilities first and set them using S_PARAM/G_PARAM.
And what's the point of the application setting the version field ? How does
the driver use it ?
> > > + * @len: Length of the module config structure
> > > + * @module_id: Module id
> > > + * @param: pointer to module config parameter.
> >
> > What is module_id for ? What does param point to ?
>
> There are a lot of tiny modules in the previewer/resizer which are
> enumerated as individual modules. The param points to the parameter set
> that the module expects to be set.
Why don't you implement something similar to
VPFE_CMD_S_CCDC_RAW_PARAMS/VPFE_CMD_G_CCDC_RAW_PARAMS instead ?
> > > + */
> > > + struct prev_module_param {
> > > + char version[IMP_MAX_NAME_SIZE];
> >
> > Is there a need to express the version as a string instead of an integer ?
>
> It could be integer. It is generally a fixed point num, and easy to read it
> as a string than an integer. Can I keep it as a string?
Let's first decide whether a version field is needed at all :-)
> > > + unsigned short len;
> > > + unsigned short module_id;
> > > + void *param;
> > > + };
> > > +
> > > +2: IOCTL: PREV_S_CONFIG/PREV_G_CONFIG
> > > +Description:
> > > + Sets/Gets the configuration required by the previewer channel
> > > +Parameter:
> > > + /**
> > > + * struct prev_channel_config - structure for configuring the
> > > previewer
> > > channel
> > > + * @len: Length of the user configuration
> > > + * @config: pointer to either single shot config or continuous
> > > + */
> > > + struct prev_channel_config {
> > > + unsigned short len;
> > > + void *config;
> > > + };
> >
> > What's the difference between parameters and configuration ? What does
> > config point to ?
>
> Config is setting which is required for a subdev to function based on what
> it is set for (single shot/continuous.) common to all platforms. Parameters
> are the settings for individual small sub-ips which might be slightly
> different from one platform to another. Config points to
> prev_single_shot_config or prev_continuous_config currently defined in
> linux/dm3656ipipe.h. I think we will move it to a common location.
Why don't you implement something similar to
VPFE_CMD_S_CCDC_RAW_PARAMS/VPFE_CMD_G_CCDC_RAW_PARAMS here as well (same for
the resizer configuration ioctls) ?
> > > +
> > > +3: IOCTL: PREV_ENUM_CAP
> > > +Description:
> > > + Queries the modules available in the image processor for preview the
> > > + input image.
> > > +Parameter:
> > > + /**
> > > + * struct prev_cap - structure to enumerate capabilities of previewer
> > > + * @index: application use this to iterate over the available modules
> > > + * @version: version of the preview module
> > > + * @module_id: module id
> > > + * @control: control operation allowed in continuous mode? 1 -
> > > allowed, 0
> > > - not allowed
> > > + * @path: path on which the module is sitting
> > > + * @module_name: module name
> > > + */
> > > + struct prev_cap {
> > > + unsigned short index;
> > > + char version[IMP_MAX_NAME_SIZE];
> > > + unsigned short module_id;
> > > + char control;
> > > + enum imp_data_paths path;
> > > + char module_name[IMP_MAX_NAME_SIZE];
> > > + };
> >
> > Enumerating internal modules is exactly what the MC API was designed for.
> > Why do you reimplement that using private ioctls ?
>
> The number of these sub-Ips are quite a few in DM365 and Dm355, having a lot
> of them In a way that may be bewildering to the end-user to be able to
> connect them quickly and properly. But
Re: [PATCH] [media] davinci: vpfe: Add documentation
Hi Manjunath, Please ignore the previous reply, I've hit the sent button too soon by mistake. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [media] davinci: vpfe: Add documentation
Hi Manjunath,
On Tuesday 17 July 2012 10:43:54 Hadli, Manjunath wrote:
> On Sun, Jul 15, 2012 at 18:16:25, Laurent Pinchart wrote:
> > On Wednesday 11 July 2012 21:09:26 Manjunath Hadli wrote:
> > > Add documentation on the Davinci VPFE driver. Document the subdevs,
> > > and private IOTCLs the driver implements
> > >
> > > Signed-off-by: Manjunath Hadli
> > > Signed-off-by: Lad, Prabhakar
[snip]
> > > + DAVINCI CCDC
> > > + DAVINCI PREVIEWER
> > > + DAVINCI RESIZER
> >
> > the DM36x VPFE documentation doesn't split the hardware in CCDC, PREVIEWER
> > and RESIZER modules, but in ISIF, IPIPEIF and IPIPE. Why don't you use
> > those names ? It looks like you're introducing an abstraction layer on
> > top of the existing driver. Why is that needed, why don't you just port
> > the driver to the MC API instead ?
>
> Please see below my comment for "enumerating internal modules".
>
> > > + DAVINCI AEW
> > > + DAVINCI AF
> > > +
> > > +Each possible link in the VPFE is modeled by a link in the Media
> > > controller +interface. For an example program see [1].
> > > +
> > > +
> > > +Private IOCTLs
> > > +==
> > > +
> > > +The Davinci Video processing Front End (VPFE) driver supports standard
> > > V4L2 +IOCTLs and controls where possible and practical. Much of the
> > > functions provided
> > > +by the VPFE, however, does not fall under the standard IOCTLs.
> > > +
> > > +In general, there is a private ioctl for configuring each of the blocks
> > > +containing hardware-dependent functions.
> > > +
> > > +The following private IOCTLs are supported:
> > > +
> > > +1: IOCTL: PREV_S_PARAM/PREV_G_PARAM
> > > +Description:
> > > + Sets/Gets the parameters required by the previewer module
> > > +Parameter:
> > > + /**
> > > + * struct prev_module_param- structure to configure preview modules
> > > + * @version: Version of the preview module
> >
> > Who is responsible for filling this field, the application or the driver ?
>
> The application is responsible for filling this info. He would enumerate the
> capabilities first and set them using S_PARAM/G_PARAM.
>
> > > + * @len: Length of the module config structure
> > > + * @module_id: Module id
> > > + * @param: pointer to module config parameter.
> >
> > What is module_id for ? What does param point to ?
>
> There are a lot of tiny modules in the previewer/resizer which are
> enumerated as individual modules. The param points to the parameter set
> that the module expects to be set.
>
> > > + */
> > > + struct prev_module_param {
> > > + char version[IMP_MAX_NAME_SIZE];
> >
> > Is there a need to express the version as a string instead of an integer ?
>
> It could be integer. It is generally a fixed point num, and easy to read it
> as a string than an integer. Can I keep it as a string?
>
> > > + unsigned short len;
> > > + unsigned short module_id;
> > > + void *param;
> > > + };
> > > +
> > > +2: IOCTL: PREV_S_CONFIG/PREV_G_CONFIG
> > > +Description:
> > > + Sets/Gets the configuration required by the previewer channel
> > > +Parameter:
> > > + /**
> > > + * struct prev_channel_config - structure for configuring the
> > > previewer
> > > channel
> > > + * @len: Length of the user configuration
> > > + * @config: pointer to either single shot config or continuous
> > > + */
> > > + struct prev_channel_config {
> > > + unsigned short len;
> > > + void *config;
> > > + };
> >
> > What's the difference between parameters and configuration ? What does
> > config point to ?
>
> Config is setting which is required for a subdev to function based on what
> it is set for (single shot/continuous.) common to all platforms. Parameters
> are the settings for individual small sub-ips which might be slightly
> different from one platform to another. Config points to
> prev_single_shot_config or prev_continuous_config currently defined in
> linux/dm3656ipipe.h. I think we will move it to a common location.
> > > +
> > > +3: IOCTL: PREV_ENUM_CAP
> > > +Description:
> > > + Queries the modules available in the image processor for preview the
> > > + input image.
> > > +Parameter:
> > > + /**
> > > + * struct prev_cap - structure to enumerate capabilities of previewer
> > > + * @index: application use this to iterate over the available modules
> > > + * @version: version of the preview module
> > > + * @module_id: module id
> > > + * @control: control operation allowed in continuous mode? 1 -
> > > allowed, 0
> > > - not allowed
> > > + * @path: path on which the module is sitting
> > > + * @module_name: module name
> > > + */
> > > + struct prev_cap {
> > > + unsigned short index;
> > > + char version[IMP_MAX_NAME_SIZE];
> > > + unsigned short module_id;
> > > + char control;
> > > + enum imp_data_paths path;
> > > + char module_name[IMP_MAX_NAME_SIZE];
> > > + };
> >
> > Enumerating internal modules is exactly what the MC API was designed
Re: [PATCH] [media] davinci: vpfe: Add documentation
Hi Manjunath,
On Tuesday 17 July 2012 10:43:54 Hadli, Manjunath wrote:
On Sun, Jul 15, 2012 at 18:16:25, Laurent Pinchart wrote:
On Wednesday 11 July 2012 21:09:26 Manjunath Hadli wrote:
Add documentation on the Davinci VPFE driver. Document the subdevs,
and private IOTCLs the driver implements
Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com
Signed-off-by: Lad, Prabhakar prabhakar@ti.com
[snip]
+ DAVINCI CCDC
+ DAVINCI PREVIEWER
+ DAVINCI RESIZER
the DM36x VPFE documentation doesn't split the hardware in CCDC, PREVIEWER
and RESIZER modules, but in ISIF, IPIPEIF and IPIPE. Why don't you use
those names ? It looks like you're introducing an abstraction layer on
top of the existing driver. Why is that needed, why don't you just port
the driver to the MC API instead ?
Please see below my comment for enumerating internal modules.
+ DAVINCI AEW
+ DAVINCI AF
+
+Each possible link in the VPFE is modeled by a link in the Media
controller +interface. For an example program see [1].
+
+
+Private IOCTLs
+==
+
+The Davinci Video processing Front End (VPFE) driver supports standard
V4L2 +IOCTLs and controls where possible and practical. Much of the
functions provided
+by the VPFE, however, does not fall under the standard IOCTLs.
+
+In general, there is a private ioctl for configuring each of the blocks
+containing hardware-dependent functions.
+
+The following private IOCTLs are supported:
+
+1: IOCTL: PREV_S_PARAM/PREV_G_PARAM
+Description:
+ Sets/Gets the parameters required by the previewer module
+Parameter:
+ /**
+ * struct prev_module_param- structure to configure preview modules
+ * @version: Version of the preview module
Who is responsible for filling this field, the application or the driver ?
The application is responsible for filling this info. He would enumerate the
capabilities first and set them using S_PARAM/G_PARAM.
+ * @len: Length of the module config structure
+ * @module_id: Module id
+ * @param: pointer to module config parameter.
What is module_id for ? What does param point to ?
There are a lot of tiny modules in the previewer/resizer which are
enumerated as individual modules. The param points to the parameter set
that the module expects to be set.
+ */
+ struct prev_module_param {
+ char version[IMP_MAX_NAME_SIZE];
Is there a need to express the version as a string instead of an integer ?
It could be integer. It is generally a fixed point num, and easy to read it
as a string than an integer. Can I keep it as a string?
+ unsigned short len;
+ unsigned short module_id;
+ void *param;
+ };
+
+2: IOCTL: PREV_S_CONFIG/PREV_G_CONFIG
+Description:
+ Sets/Gets the configuration required by the previewer channel
+Parameter:
+ /**
+ * struct prev_channel_config - structure for configuring the
previewer
channel
+ * @len: Length of the user configuration
+ * @config: pointer to either single shot config or continuous
+ */
+ struct prev_channel_config {
+ unsigned short len;
+ void *config;
+ };
What's the difference between parameters and configuration ? What does
config point to ?
Config is setting which is required for a subdev to function based on what
it is set for (single shot/continuous.) common to all platforms. Parameters
are the settings for individual small sub-ips which might be slightly
different from one platform to another. Config points to
prev_single_shot_config or prev_continuous_config currently defined in
linux/dm3656ipipe.h. I think we will move it to a common location.
+
+3: IOCTL: PREV_ENUM_CAP
+Description:
+ Queries the modules available in the image processor for preview the
+ input image.
+Parameter:
+ /**
+ * struct prev_cap - structure to enumerate capabilities of previewer
+ * @index: application use this to iterate over the available modules
+ * @version: version of the preview module
+ * @module_id: module id
+ * @control: control operation allowed in continuous mode? 1 -
allowed, 0
- not allowed
+ * @path: path on which the module is sitting
+ * @module_name: module name
+ */
+ struct prev_cap {
+ unsigned short index;
+ char version[IMP_MAX_NAME_SIZE];
+ unsigned short module_id;
+ char control;
+ enum imp_data_paths path;
+ char module_name[IMP_MAX_NAME_SIZE];
+ };
Enumerating internal modules is exactly what the MC API was designed for.
Why do you reimplement that using private ioctls ?
The number of these sub-Ips are quite a few in DM365 and Dm355, having a lot
of them In a way that may be bewildering to the end-user to be able to
connect them quickly and properly. But overall, these are nothing
Re: [PATCH] [media] davinci: vpfe: Add documentation
Hi Manjunath, Please ignore the previous reply, I've hit the sent button too soon by mistake. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [media] davinci: vpfe: Add documentation
Hi Manjunath,
On Tuesday 17 July 2012 10:43:54 Hadli, Manjunath wrote:
On Sun, Jul 15, 2012 at 18:16:25, Laurent Pinchart wrote:
On Wednesday 11 July 2012 21:09:26 Manjunath Hadli wrote:
Add documentation on the Davinci VPFE driver. Document the subdevs,
and private IOTCLs the driver implements
Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com
Signed-off-by: Lad, Prabhakar prabhakar@ti.com
[snip]
+Private IOCTLs
+==
+
+The Davinci Video processing Front End (VPFE) driver supports standard
V4L2 +IOCTLs and controls where possible and practical. Much of the
functions provided
+by the VPFE, however, does not fall under the standard IOCTLs.
+
+In general, there is a private ioctl for configuring each of the blocks
+containing hardware-dependent functions.
+
+The following private IOCTLs are supported:
+
+1: IOCTL: PREV_S_PARAM/PREV_G_PARAM
+Description:
+ Sets/Gets the parameters required by the previewer module
+Parameter:
+ /**
+ * struct prev_module_param- structure to configure preview modules
+ * @version: Version of the preview module
Who is responsible for filling this field, the application or the driver ?
The application is responsible for filling this info. He would enumerate the
capabilities first and set them using S_PARAM/G_PARAM.
And what's the point of the application setting the version field ? How does
the driver use it ?
+ * @len: Length of the module config structure
+ * @module_id: Module id
+ * @param: pointer to module config parameter.
What is module_id for ? What does param point to ?
There are a lot of tiny modules in the previewer/resizer which are
enumerated as individual modules. The param points to the parameter set
that the module expects to be set.
Why don't you implement something similar to
VPFE_CMD_S_CCDC_RAW_PARAMS/VPFE_CMD_G_CCDC_RAW_PARAMS instead ?
+ */
+ struct prev_module_param {
+ char version[IMP_MAX_NAME_SIZE];
Is there a need to express the version as a string instead of an integer ?
It could be integer. It is generally a fixed point num, and easy to read it
as a string than an integer. Can I keep it as a string?
Let's first decide whether a version field is needed at all :-)
+ unsigned short len;
+ unsigned short module_id;
+ void *param;
+ };
+
+2: IOCTL: PREV_S_CONFIG/PREV_G_CONFIG
+Description:
+ Sets/Gets the configuration required by the previewer channel
+Parameter:
+ /**
+ * struct prev_channel_config - structure for configuring the
previewer
channel
+ * @len: Length of the user configuration
+ * @config: pointer to either single shot config or continuous
+ */
+ struct prev_channel_config {
+ unsigned short len;
+ void *config;
+ };
What's the difference between parameters and configuration ? What does
config point to ?
Config is setting which is required for a subdev to function based on what
it is set for (single shot/continuous.) common to all platforms. Parameters
are the settings for individual small sub-ips which might be slightly
different from one platform to another. Config points to
prev_single_shot_config or prev_continuous_config currently defined in
linux/dm3656ipipe.h. I think we will move it to a common location.
Why don't you implement something similar to
VPFE_CMD_S_CCDC_RAW_PARAMS/VPFE_CMD_G_CCDC_RAW_PARAMS here as well (same for
the resizer configuration ioctls) ?
+
+3: IOCTL: PREV_ENUM_CAP
+Description:
+ Queries the modules available in the image processor for preview the
+ input image.
+Parameter:
+ /**
+ * struct prev_cap - structure to enumerate capabilities of previewer
+ * @index: application use this to iterate over the available modules
+ * @version: version of the preview module
+ * @module_id: module id
+ * @control: control operation allowed in continuous mode? 1 -
allowed, 0
- not allowed
+ * @path: path on which the module is sitting
+ * @module_name: module name
+ */
+ struct prev_cap {
+ unsigned short index;
+ char version[IMP_MAX_NAME_SIZE];
+ unsigned short module_id;
+ char control;
+ enum imp_data_paths path;
+ char module_name[IMP_MAX_NAME_SIZE];
+ };
Enumerating internal modules is exactly what the MC API was designed for.
Why do you reimplement that using private ioctls ?
The number of these sub-Ips are quite a few in DM365 and Dm355, having a lot
of them In a way that may be bewildering to the end-user to be able to
connect them quickly and properly. But overall, these are nothing but
exposed subips of what we call as CCDC,Previewer and Resizer.It Made a lot
of logical sense to keep it that way, give a default configuration for
everything, and if at all the user wants the fine
RE: [PATCH] [media] davinci: vpfe: Add documentation
Hi Laurent,
Thank you for your comments.
On Sun, Jul 15, 2012 at 18:16:25, Laurent Pinchart wrote:
> Hi Manjunath,
>
> Thanks for the patch.
>
> On Wednesday 11 July 2012 21:09:26 Manjunath Hadli wrote:
> > Add documentation on the Davinci VPFE driver. Document the subdevs,
> > and private IOTCLs the driver implements
> >
> > Signed-off-by: Manjunath Hadli
> > Signed-off-by: Lad, Prabhakar
> > ---
> > Documentation/video4linux/davinci-vpfe-mc.txt | 263
> > + 1 files changed, 263 insertions(+), 0
> > deletions(-)
> > create mode 100644 Documentation/video4linux/davinci-vpfe-mc.txt
> >
> > diff --git a/Documentation/video4linux/davinci-vpfe-mc.txt
> > b/Documentation/video4linux/davinci-vpfe-mc.txt new file mode 100644
> > index 000..968194f
> > --- /dev/null
> > +++ b/Documentation/video4linux/davinci-vpfe-mc.txt
> > @@ -0,0 +1,263 @@
> > +Davinci Video processing Front End (VPFE) driver
> > +
> > +Copyright (C) 2012 Texas Instruments Inc
> > +
> > +Contacts: Manjunath Hadli
> > +
> > +Introduction
> > +
> > +
> > +This file documents the Texas Instruments Davinci Video processing Front
> > End
> > +(VPFE) driver located under drivers/media/video/davinci. The original
> > driver
> > +exists for Davinci VPFE, which is now being changed to Media Controller
> > +Framework.
> > +
> > +Currently the driver has been successfully used on the following version of
> > Davinci:
> > +
> > + DM365/DM368
>
> Does the driver still support the DM644x ?
Yes. The driver supports DM6446. We will add the Dm6446x patches on these.
>
> > +The driver implements V4L2, Media controller and v4l2_subdev interfaces.
> > +Sensor, lens and flash drivers using the v4l2_subdev interface in the
> > kernel
> > +are supported.
> > +
> > +
> > +Split to subdevs
> > +
> > +
> > +The Davinic VPFE is split into V4L2 subdevs, each of the blocks inside the
>
> s/Davinic/Davinci/
OK. Thanks.
>
> > VPFE
> > +having one subdev to represent it. Each of the subdevs provide a V4L2
> > subdev
> > +interface to userspace.
> > +
> > + DAVINCI CCDC
> > + DAVINCI PREVIEWER
> > + DAVINCI RESIZER
>
> the DM36x VPFE documentation doesn't split the hardware in CCDC, PREVIEWER
> and
> RESIZER modules, but in ISIF, IPIPEIF and IPIPE. Why don't you use those
> names
> ? It looks like you're introducing an abstraction layer on top of the
> existing
> driver. Why is that needed, why don't you just port the driver to the MC API
> instead ?
Please see below my comment for "enumerating internal modules".
>
> > + DAVINCI AEW
> > + DAVINCI AF
> > +
> > +Each possible link in the VPFE is modeled by a link in the Media controller
> > +interface. For an example program see [1].
> > +
> > +
> > +Private IOCTLs
> > +==
> > +
> > +The Davinci Video processing Front End (VPFE) driver supports standard V4L2
> > +IOCTLs and controls where possible and practical. Much of the functions
> > provided
> > +by the VPFE, however, does not fall under the standard IOCTLs.
> > +
> > +In general, there is a private ioctl for configuring each of the blocks
> > +containing hardware-dependent functions.
> > +
> > +The following private IOCTLs are supported:
> > +
> > +1: IOCTL: PREV_S_PARAM/PREV_G_PARAM
> > +Description:
> > + Sets/Gets the parameters required by the previewer module
> > +Parameter:
> > + /**
> > +* struct prev_module_param- structure to configure preview modules
> > +* @version: Version of the preview module
>
> Who is responsible for filling this field, the application or the driver ?
The application is responsible for filling this info. He would enumerate the
capabilities first and set them using S_PARAM/G_PARAM.
>
> > +* @len: Length of the module config structure
> > +* @module_id: Module id
> > +* @param: pointer to module config parameter.
>
> What is module_id for ? What does param point to ?
There are a lot of tiny modules in the previewer/resizer which are enumerated
as individual modules. The param points to the parameter set that the module
expects to be set.
>
> > +*/
> > + struct prev_module_param {
> > + char version[IMP_MAX_NAME_SIZE];
>
> Is there a need to express the version as a string instead of an integer ?
It could be integer. It is generally a fixed point num, and easy to read it
as a string than an integer. Can I keep it as a string?
>
> > + unsigned short len;
> > + unsigned short module_id;
> > + void *param;
> > + };
> > +
> > +2: IOCTL: PREV_S_CONFIG/PREV_G_CONFIG
> > +Description:
> > + Sets/Gets the configuration required by the previewer channel
> > +Parameter:
> > + /**
> > +* struct prev_channel_config - structure for configuring the previewer
> > channel
> > +* @len: Length of the user configuration
> > +* @config: pointer to either single shot config or continuous
> > +*/
> > + struct prev_channel_config {
> > +
RE: [PATCH] [media] davinci: vpfe: Add documentation
Hi Laurent,
Thank you for your comments.
On Sun, Jul 15, 2012 at 18:16:25, Laurent Pinchart wrote:
Hi Manjunath,
Thanks for the patch.
On Wednesday 11 July 2012 21:09:26 Manjunath Hadli wrote:
Add documentation on the Davinci VPFE driver. Document the subdevs,
and private IOTCLs the driver implements
Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com
Signed-off-by: Lad, Prabhakar prabhakar@ti.com
---
Documentation/video4linux/davinci-vpfe-mc.txt | 263
+ 1 files changed, 263 insertions(+), 0
deletions(-)
create mode 100644 Documentation/video4linux/davinci-vpfe-mc.txt
diff --git a/Documentation/video4linux/davinci-vpfe-mc.txt
b/Documentation/video4linux/davinci-vpfe-mc.txt new file mode 100644
index 000..968194f
--- /dev/null
+++ b/Documentation/video4linux/davinci-vpfe-mc.txt
@@ -0,0 +1,263 @@
+Davinci Video processing Front End (VPFE) driver
+
+Copyright (C) 2012 Texas Instruments Inc
+
+Contacts: Manjunath Hadli manjunath.ha...@ti.com
+
+Introduction
+
+
+This file documents the Texas Instruments Davinci Video processing Front
End
+(VPFE) driver located under drivers/media/video/davinci. The original
driver
+exists for Davinci VPFE, which is now being changed to Media Controller
+Framework.
+
+Currently the driver has been successfully used on the following version of
Davinci:
+
+ DM365/DM368
Does the driver still support the DM644x ?
Yes. The driver supports DM6446. We will add the Dm6446x patches on these.
+The driver implements V4L2, Media controller and v4l2_subdev interfaces.
+Sensor, lens and flash drivers using the v4l2_subdev interface in the
kernel
+are supported.
+
+
+Split to subdevs
+
+
+The Davinic VPFE is split into V4L2 subdevs, each of the blocks inside the
s/Davinic/Davinci/
OK. Thanks.
VPFE
+having one subdev to represent it. Each of the subdevs provide a V4L2
subdev
+interface to userspace.
+
+ DAVINCI CCDC
+ DAVINCI PREVIEWER
+ DAVINCI RESIZER
the DM36x VPFE documentation doesn't split the hardware in CCDC, PREVIEWER
and
RESIZER modules, but in ISIF, IPIPEIF and IPIPE. Why don't you use those
names
? It looks like you're introducing an abstraction layer on top of the
existing
driver. Why is that needed, why don't you just port the driver to the MC API
instead ?
Please see below my comment for enumerating internal modules.
+ DAVINCI AEW
+ DAVINCI AF
+
+Each possible link in the VPFE is modeled by a link in the Media controller
+interface. For an example program see [1].
+
+
+Private IOCTLs
+==
+
+The Davinci Video processing Front End (VPFE) driver supports standard V4L2
+IOCTLs and controls where possible and practical. Much of the functions
provided
+by the VPFE, however, does not fall under the standard IOCTLs.
+
+In general, there is a private ioctl for configuring each of the blocks
+containing hardware-dependent functions.
+
+The following private IOCTLs are supported:
+
+1: IOCTL: PREV_S_PARAM/PREV_G_PARAM
+Description:
+ Sets/Gets the parameters required by the previewer module
+Parameter:
+ /**
+* struct prev_module_param- structure to configure preview modules
+* @version: Version of the preview module
Who is responsible for filling this field, the application or the driver ?
The application is responsible for filling this info. He would enumerate the
capabilities first and set them using S_PARAM/G_PARAM.
+* @len: Length of the module config structure
+* @module_id: Module id
+* @param: pointer to module config parameter.
What is module_id for ? What does param point to ?
There are a lot of tiny modules in the previewer/resizer which are enumerated
as individual modules. The param points to the parameter set that the module
expects to be set.
+*/
+ struct prev_module_param {
+ char version[IMP_MAX_NAME_SIZE];
Is there a need to express the version as a string instead of an integer ?
It could be integer. It is generally a fixed point num, and easy to read it
as a string than an integer. Can I keep it as a string?
+ unsigned short len;
+ unsigned short module_id;
+ void *param;
+ };
+
+2: IOCTL: PREV_S_CONFIG/PREV_G_CONFIG
+Description:
+ Sets/Gets the configuration required by the previewer channel
+Parameter:
+ /**
+* struct prev_channel_config - structure for configuring the previewer
channel
+* @len: Length of the user configuration
+* @config: pointer to either single shot config or continuous
+*/
+ struct prev_channel_config {
+ unsigned short len;
+ void *config;
+ };
What's the difference between parameters and configuration ? What does config
point to ?
Config is setting which is
Re: [PATCH] [media] davinci: vpfe: Add documentation
Hi Manjunath,
Thanks for the patch.
On Wednesday 11 July 2012 21:09:26 Manjunath Hadli wrote:
> Add documentation on the Davinci VPFE driver. Document the subdevs,
> and private IOTCLs the driver implements
>
> Signed-off-by: Manjunath Hadli
> Signed-off-by: Lad, Prabhakar
> ---
> Documentation/video4linux/davinci-vpfe-mc.txt | 263
> + 1 files changed, 263 insertions(+), 0
> deletions(-)
> create mode 100644 Documentation/video4linux/davinci-vpfe-mc.txt
>
> diff --git a/Documentation/video4linux/davinci-vpfe-mc.txt
> b/Documentation/video4linux/davinci-vpfe-mc.txt new file mode 100644
> index 000..968194f
> --- /dev/null
> +++ b/Documentation/video4linux/davinci-vpfe-mc.txt
> @@ -0,0 +1,263 @@
> +Davinci Video processing Front End (VPFE) driver
> +
> +Copyright (C) 2012 Texas Instruments Inc
> +
> +Contacts: Manjunath Hadli
> +
> +Introduction
> +
> +
> +This file documents the Texas Instruments Davinci Video processing Front
> End
> +(VPFE) driver located under drivers/media/video/davinci. The original
> driver
> +exists for Davinci VPFE, which is now being changed to Media Controller
> +Framework.
> +
> +Currently the driver has been successfully used on the following version of
> Davinci:
> +
> + DM365/DM368
Does the driver still support the DM644x ?
> +The driver implements V4L2, Media controller and v4l2_subdev interfaces.
> +Sensor, lens and flash drivers using the v4l2_subdev interface in the
> kernel
> +are supported.
> +
> +
> +Split to subdevs
> +
> +
> +The Davinic VPFE is split into V4L2 subdevs, each of the blocks inside the
s/Davinic/Davinci/
> VPFE
> +having one subdev to represent it. Each of the subdevs provide a V4L2
> subdev
> +interface to userspace.
> +
> + DAVINCI CCDC
> + DAVINCI PREVIEWER
> + DAVINCI RESIZER
the DM36x VPFE documentation doesn't split the hardware in CCDC, PREVIEWER and
RESIZER modules, but in ISIF, IPIPEIF and IPIPE. Why don't you use those names
? It looks like you're introducing an abstraction layer on top of the existing
driver. Why is that needed, why don't you just port the driver to the MC API
instead ?
> + DAVINCI AEW
> + DAVINCI AF
> +
> +Each possible link in the VPFE is modeled by a link in the Media controller
> +interface. For an example program see [1].
> +
> +
> +Private IOCTLs
> +==
> +
> +The Davinci Video processing Front End (VPFE) driver supports standard V4L2
> +IOCTLs and controls where possible and practical. Much of the functions
> provided
> +by the VPFE, however, does not fall under the standard IOCTLs.
> +
> +In general, there is a private ioctl for configuring each of the blocks
> +containing hardware-dependent functions.
> +
> +The following private IOCTLs are supported:
> +
> +1: IOCTL: PREV_S_PARAM/PREV_G_PARAM
> +Description:
> + Sets/Gets the parameters required by the previewer module
> +Parameter:
> + /**
> + * struct prev_module_param- structure to configure preview modules
> + * @version: Version of the preview module
Who is responsible for filling this field, the application or the driver ?
> + * @len: Length of the module config structure
> + * @module_id: Module id
> + * @param: pointer to module config parameter.
What is module_id for ? What does param point to ?
> + */
> + struct prev_module_param {
> + char version[IMP_MAX_NAME_SIZE];
Is there a need to express the version as a string instead of an integer ?
> + unsigned short len;
> + unsigned short module_id;
> + void *param;
> + };
> +
> +2: IOCTL: PREV_S_CONFIG/PREV_G_CONFIG
> +Description:
> + Sets/Gets the configuration required by the previewer channel
> +Parameter:
> + /**
> + * struct prev_channel_config - structure for configuring the previewer
> channel
> + * @len: Length of the user configuration
> + * @config: pointer to either single shot config or continuous
> + */
> + struct prev_channel_config {
> + unsigned short len;
> + void *config;
> + };
What's the difference between parameters and configuration ? What does config
point to ?
> +
> +3: IOCTL: PREV_ENUM_CAP
> +Description:
> + Queries the modules available in the image processor for preview the
> + input image.
> +Parameter:
> + /**
> + * struct prev_cap - structure to enumerate capabilities of previewer
> + * @index: application use this to iterate over the available modules
> + * @version: version of the preview module
> + * @module_id: module id
> + * @control: control operation allowed in continuous mode? 1 - allowed, > 0
> - not allowed
> + * @path: path on which the module is sitting
> + * @module_name: module name
> + */
> + struct prev_cap {
> + unsigned short index;
> + char version[IMP_MAX_NAME_SIZE];
> + unsigned short module_id;
>
Re: [PATCH] [media] davinci: vpfe: Add documentation
Hi Manjunath,
Thanks for the patch.
On Wednesday 11 July 2012 21:09:26 Manjunath Hadli wrote:
Add documentation on the Davinci VPFE driver. Document the subdevs,
and private IOTCLs the driver implements
Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com
Signed-off-by: Lad, Prabhakar prabhakar@ti.com
---
Documentation/video4linux/davinci-vpfe-mc.txt | 263
+ 1 files changed, 263 insertions(+), 0
deletions(-)
create mode 100644 Documentation/video4linux/davinci-vpfe-mc.txt
diff --git a/Documentation/video4linux/davinci-vpfe-mc.txt
b/Documentation/video4linux/davinci-vpfe-mc.txt new file mode 100644
index 000..968194f
--- /dev/null
+++ b/Documentation/video4linux/davinci-vpfe-mc.txt
@@ -0,0 +1,263 @@
+Davinci Video processing Front End (VPFE) driver
+
+Copyright (C) 2012 Texas Instruments Inc
+
+Contacts: Manjunath Hadli manjunath.ha...@ti.com
+
+Introduction
+
+
+This file documents the Texas Instruments Davinci Video processing Front
End
+(VPFE) driver located under drivers/media/video/davinci. The original
driver
+exists for Davinci VPFE, which is now being changed to Media Controller
+Framework.
+
+Currently the driver has been successfully used on the following version of
Davinci:
+
+ DM365/DM368
Does the driver still support the DM644x ?
+The driver implements V4L2, Media controller and v4l2_subdev interfaces.
+Sensor, lens and flash drivers using the v4l2_subdev interface in the
kernel
+are supported.
+
+
+Split to subdevs
+
+
+The Davinic VPFE is split into V4L2 subdevs, each of the blocks inside the
s/Davinic/Davinci/
VPFE
+having one subdev to represent it. Each of the subdevs provide a V4L2
subdev
+interface to userspace.
+
+ DAVINCI CCDC
+ DAVINCI PREVIEWER
+ DAVINCI RESIZER
the DM36x VPFE documentation doesn't split the hardware in CCDC, PREVIEWER and
RESIZER modules, but in ISIF, IPIPEIF and IPIPE. Why don't you use those names
? It looks like you're introducing an abstraction layer on top of the existing
driver. Why is that needed, why don't you just port the driver to the MC API
instead ?
+ DAVINCI AEW
+ DAVINCI AF
+
+Each possible link in the VPFE is modeled by a link in the Media controller
+interface. For an example program see [1].
+
+
+Private IOCTLs
+==
+
+The Davinci Video processing Front End (VPFE) driver supports standard V4L2
+IOCTLs and controls where possible and practical. Much of the functions
provided
+by the VPFE, however, does not fall under the standard IOCTLs.
+
+In general, there is a private ioctl for configuring each of the blocks
+containing hardware-dependent functions.
+
+The following private IOCTLs are supported:
+
+1: IOCTL: PREV_S_PARAM/PREV_G_PARAM
+Description:
+ Sets/Gets the parameters required by the previewer module
+Parameter:
+ /**
+ * struct prev_module_param- structure to configure preview modules
+ * @version: Version of the preview module
Who is responsible for filling this field, the application or the driver ?
+ * @len: Length of the module config structure
+ * @module_id: Module id
+ * @param: pointer to module config parameter.
What is module_id for ? What does param point to ?
+ */
+ struct prev_module_param {
+ char version[IMP_MAX_NAME_SIZE];
Is there a need to express the version as a string instead of an integer ?
+ unsigned short len;
+ unsigned short module_id;
+ void *param;
+ };
+
+2: IOCTL: PREV_S_CONFIG/PREV_G_CONFIG
+Description:
+ Sets/Gets the configuration required by the previewer channel
+Parameter:
+ /**
+ * struct prev_channel_config - structure for configuring the previewer
channel
+ * @len: Length of the user configuration
+ * @config: pointer to either single shot config or continuous
+ */
+ struct prev_channel_config {
+ unsigned short len;
+ void *config;
+ };
What's the difference between parameters and configuration ? What does config
point to ?
+
+3: IOCTL: PREV_ENUM_CAP
+Description:
+ Queries the modules available in the image processor for preview the
+ input image.
+Parameter:
+ /**
+ * struct prev_cap - structure to enumerate capabilities of previewer
+ * @index: application use this to iterate over the available modules
+ * @version: version of the preview module
+ * @module_id: module id
+ * @control: control operation allowed in continuous mode? 1 - allowed, 0
- not allowed
+ * @path: path on which the module is sitting
+ * @module_name: module name
+ */
+ struct prev_cap {
+ unsigned short index;
+ char version[IMP_MAX_NAME_SIZE];
+ unsigned short module_id;
+ char control;
+ enum imp_data_paths
