Re: [GIT PATCHES FOR 2.6.34] Support for vpfe-capture on DM365

Re: [GIT PATCHES FOR 2.6.34] Support for vpfe-capture on DM365

[Mauro Carvalho Chehab]

Muralidharan Karicheri wrote:
> Mauro,
> 
> Please pull from the following:-
> 
> The following changes since commit 84b74782ace1ae091c1b0e14ae2ee9bb720532ba:
>   Douglas Schilling Landgraf (1):
>         V4L/DVB: Fix logic for Leadtek winfast tv usbii deluxe
> 
> are available in the git repository at:
> 
>   git://linuxtv.org/mkaricheri/vpfe-vpbe-video.git for_upstream
> 
> Murali Karicheri (6):
>       DaVinci - Adding platform & board changes for vpfe capture on DM365
>       V4L - vpfe capture - header files for ISIF driver
>       V4L - vpfe capture - source for ISIF driver on DM365

Hmm...
+static int isif_get_params(void __user *params)
+{
+       /* only raw module parameters can be set through the IOCTL */
+       if (isif_cfg.if_type != VPFE_RAW_BAYER)
+               return -EINVAL;
+
+       if (copy_to_user(params,
+                       &isif_cfg.bayer.config_params,
+                       sizeof(isif_cfg.bayer.config_params))) {

+/* Parameter operations */
+static int isif_set_params(void __user *params)
+{
+       struct isif_config_params_raw *isif_raw_params;
+       int ret = -EINVAL;
+
+       /* only raw module parameters can be set through the IOCTL */
+       if (isif_cfg.if_type != VPFE_RAW_BAYER)
+               return ret;
+
+       isif_raw_params = kzalloc(sizeof(*isif_raw_params), GFP_KERNEL);
+       if (NULL == isif_raw_params)
+               return -ENOMEM;
+
+       ret = copy_from_user(isif_raw_params,


It seems that you're defining some undocumented new userspace API here.

Cheers,
Mauro

Re: [GIT PATCHES FOR 2.6.34] Support for vpfe-capture on DM365

[Muralidharan Karicheri]

Mauro,


On Fri, Feb 5, 2010 at 4:52 PM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> Muralidharan Karicheri wrote:
>> Mauro,
>>
>> Please pull from the following:-
>>
>> The following changes since commit 84b74782ace1ae091c1b0e14ae2ee9bb720532ba:
>>   Douglas Schilling Landgraf (1):
>>         V4L/DVB: Fix logic for Leadtek winfast tv usbii deluxe
>>
>> are available in the git repository at:
>>
>>   git://linuxtv.org/mkaricheri/vpfe-vpbe-video.git for_upstream
>>
>> Murali Karicheri (6):
>>       DaVinci - Adding platform & board changes for vpfe capture on DM365
>>       V4L - vpfe capture - header files for ISIF driver
>>       V4L - vpfe capture - source for ISIF driver on DM365
>
> Hmm...
> +static int isif_get_params(void __user *params)
> +{
> +       /* only raw module parameters can be set through the IOCTL */
> +       if (isif_cfg.if_type != VPFE_RAW_BAYER)
> +               return -EINVAL;
> +
> +       if (copy_to_user(params,
> +                       &isif_cfg.bayer.config_params,
> +                       sizeof(isif_cfg.bayer.config_params))) {
>
> +/* Parameter operations */
> +static int isif_set_params(void __user *params)
> +{
> +       struct isif_config_params_raw *isif_raw_params;
> +       int ret = -EINVAL;
> +
> +       /* only raw module parameters can be set through the IOCTL */
> +       if (isif_cfg.if_type != VPFE_RAW_BAYER)
> +               return ret;
> +
> +       isif_raw_params = kzalloc(sizeof(*isif_raw_params), GFP_KERNEL);
> +       if (NULL == isif_raw_params)
> +               return -ENOMEM;
> +
> +       ret = copy_from_user(isif_raw_params,
>
>
> It seems that you're defining some undocumented new userspace API here.
>
Yes. This supports an experimental, but necessary API that configures
the ISIF (Image sensor Interface) image tuning parameters from
User Space. These parameters are used when converting Bayer RGB image
to UYVY format when capturing from sensors such as MT9T031.
For SoCs like TI's DMxxx series, the user needs to have full control
over these parameters to get desired image quality.
This had been discussed during the initial version of vpfe-capture
driver discussion and it was decided to keep them as experimental. So
no
documentation is provided at this time. The ioctls are defined in the
vpfe_capture.h header file and the user space structures are under
ccdc.h and isif.h under include/media/davinci and are marked as
experimental.  This was also discussed  in this mailing list before
and the decision taken at that time was to do it properly as part of
media soc framework. In this framework, isif and other similar SoC
hardware IPs will have a device node to configure these parameters
and therefore will have to be re-worked once media soc framework is
available.  Until then vpfe-capture users need to have a way to
configure the ISIF or such hardware IPs.

Regards,

Murali
> Cheers,
> Mauro
>



-- 
Murali Karicheri
mkaricheri@gmail.com

Re: [GIT PATCHES FOR 2.6.34] Support for vpfe-capture on DM365

[Mauro Carvalho Chehab]

Muralidharan Karicheri wrote:
> Mauro,
>> +       ret = copy_from_user(isif_raw_params,

>> It seems that you're defining some undocumented new userspace API here.
>>
> Yes. This supports an experimental, but necessary API that configures
> the ISIF (Image sensor Interface) image tuning parameters from
> User Space. These parameters are used when converting Bayer RGB image
> to UYVY format when capturing from sensors such as MT9T031.
> For SoCs like TI's DMxxx series, the user needs to have full control
> over these parameters to get desired image quality.
> This had been discussed during the initial version of vpfe-capture
> driver discussion and it was decided to keep them as experimental. So
> no
> documentation is provided at this time. The ioctls are defined in the
> vpfe_capture.h header file and the user space structures are under
> ccdc.h and isif.h under include/media/davinci and are marked as
> experimental.  This was also discussed  in this mailing list before
> and the decision taken at that time was to do it properly as part of
> media soc framework. In this framework, isif and other similar SoC
> hardware IPs will have a device node to configure these parameters
> and therefore will have to be re-worked once media soc framework is
> available.  Until then vpfe-capture users need to have a way to
> configure the ISIF or such hardware IPs.

So, it is not an experimental but a temporary API. We shouldn't add any
driver with temporary API's under /drivers, since API's are forever.
So, or submit it with the proper media soc framework or add the driver
at /staging.

Cheers,
Mauro