Sensor driver support with CCS

Sensor driver support with CCS

[Umang Jain]

Hello everyone,

Past few weeks I have been working to support IMX519 sensor with CCS
driver. The highlight is that I can capture frames with CCS driver now,
with a few workarounds. This thread summarises the issues that we faced
and act as placeholder for more open and structured discussion.

- First open question is regarding the upstream support of a sensor
  driver with CCS. How should we term that a sensor is 'mainline-supported'
  if it is getting supported by CCS. How can one use a sensor out of the box
  with mainline kernel, when supported by CCS?

- Continued from the previous discusion point, the more specific question is
  about the sensor-specific static data that is composed of
  manfacturer-specific-registers (MSRs) and (maybe) overridden
  sensor-read-only-regs which are created as part of ccs-tools [1].
  This becomes part of firmware? which needs to live /lib/firmware and
  then picked up by CCS driver.

  Where can they be centralized ? is linux-firmware an option? [2].

- It was noticed that with current version of CCS - I was only able to
  get 1/3 buffer filled. This was due to the fact that LINE_LENGTH_PCK
  and FRAME_LENGTH_LINES registers are not getting updated to permissible
  values in / before ccs_start_streaming() starts. Hence they are stuck
  with values from probe time.

These are top issues we faced while trying to support IMX519. This thread
will act as placeholder to have more discussions in the open and/or help
other sensors that can be supported with CCS.

[1]: https://github.com/MIPI-Alliance/ccs-tools
[2]: https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/

thanks,
uajain

Re: Sensor driver support with CCS

[Sakari Ailus]

Hi Umang,

On Wed, Oct 18, 2023 at 12:04:46PM +0530, Umang Jain wrote:
> Hello everyone,
> 
> Past few weeks I have been working to support IMX519 sensor with CCS
> driver. The highlight is that I can capture frames with CCS driver now,
> with a few workarounds. This thread summarises the issues that we faced
> and act as placeholder for more open and structured discussion.
> 
> - First open question is regarding the upstream support of a sensor
>   driver with CCS. How should we term that a sensor is 'mainline-supported'
>   if it is getting supported by CCS. How can one use a sensor out of the box
>   with mainline kernel, when supported by CCS?

If the sensor is fully compliant, it should "just work", but then there's
the question of CCS static data if the sensor needs it.

For devices that aren't fully compliant this is a more complicated
question. Some devices might work with some parameters only, in particular
horizontal blanking is a sensitive value. The approach here should be to
set minimum and maximum to the same value to force horizontal blanking
value that is known to work. The if rules could be used for this as well.

> 
> - Continued from the previous discusion point, the more specific question is
>   about the sensor-specific static data that is composed of
>   manfacturer-specific-registers (MSRs) and (maybe) overridden
>   sensor-read-only-regs which are created as part of ccs-tools [1].
>   This becomes part of firmware? which needs to live /lib/firmware and
>   then picked up by CCS driver.
> 
>   Where can they be centralized ? is linux-firmware an option? [2].

I'd favour this.

> 
> - It was noticed that with current version of CCS - I was only able to
>   get 1/3 buffer filled. This was due to the fact that LINE_LENGTH_PCK
>   and FRAME_LENGTH_LINES registers are not getting updated to permissible
>   values in / before ccs_start_streaming() starts. Hence they are stuck
>   with values from probe time.

If you're not changing vertical or horizontal blanking, these are likely
the correct values from driver perspective. If these values do not work
however, then you most likely have an issue with sensor provided limit and
capability information. This should be fixed using CCS static data.

> These are top issues we faced while trying to support IMX519. This thread
> will act as placeholder to have more discussions in the open and/or help
> other sensors that can be supported with CCS.

I'm happy to see you using the CCS driver on a sensor I haven't used
before. The above issues, while they prevent meaningfully using the sensor
right now, they also seem easy to resolve.

Additionally --- if the sensor does not provide meaningful values for its
vendor and model, we could use the device's compatible string as the basis
for the firmware file name. I can post a patch for this.

-- 
Kind regards,

Sakari Ailus

Re: Sensor driver support with CCS

[Laurent Pinchart]

On Wed, Oct 18, 2023 at 09:27:59AM +0000, Sakari Ailus wrote:
> On Wed, Oct 18, 2023 at 12:04:46PM +0530, Umang Jain wrote:
> > Hello everyone,
> > 
> > Past few weeks I have been working to support IMX519 sensor with CCS
> > driver. The highlight is that I can capture frames with CCS driver now,
> > with a few workarounds. This thread summarises the issues that we faced
> > and act as placeholder for more open and structured discussion.
> > 
> > - First open question is regarding the upstream support of a sensor
> >   driver with CCS. How should we term that a sensor is 'mainline-supported'
> >   if it is getting supported by CCS. How can one use a sensor out of the box
> >   with mainline kernel, when supported by CCS?
> 
> If the sensor is fully compliant, it should "just work", but then there's
> the question of CCS static data if the sensor needs it.
> 
> For devices that aren't fully compliant this is a more complicated
> question. Some devices might work with some parameters only, in particular
> horizontal blanking is a sensitive value. The approach here should be to
> set minimum and maximum to the same value to force horizontal blanking
> value that is known to work. The if rules could be used for this as well.
> 
> > 
> > - Continued from the previous discusion point, the more specific question is
> >   about the sensor-specific static data that is composed of
> >   manfacturer-specific-registers (MSRs) and (maybe) overridden
> >   sensor-read-only-regs which are created as part of ccs-tools [1].
> >   This becomes part of firmware? which needs to live /lib/firmware and
> >   then picked up by CCS driver.
> > 
> >   Where can they be centralized ? is linux-firmware an option? [2].
> 
> I'd favour this.

linux-firmware could host the binary files, but how about the YAML
sources ?

> > - It was noticed that with current version of CCS - I was only able to
> >   get 1/3 buffer filled. This was due to the fact that LINE_LENGTH_PCK
> >   and FRAME_LENGTH_LINES registers are not getting updated to permissible
> >   values in / before ccs_start_streaming() starts. Hence they are stuck
> >   with values from probe time.
> 
> If you're not changing vertical or horizontal blanking, these are likely
> the correct values from driver perspective. If these values do not work
> however, then you most likely have an issue with sensor provided limit and
> capability information. This should be fixed using CCS static data.

I may have understood Umang incorrectly, but I thought he meant that the
driver doesn't program those registers if the controls are not changed
by userspace, and the register power-up default values are not right.
Umang, is my understanding wrong ?

> > These are top issues we faced while trying to support IMX519. This thread
> > will act as placeholder to have more discussions in the open and/or help
> > other sensors that can be supported with CCS.
> 
> I'm happy to see you using the CCS driver on a sensor I haven't used
> before. The above issues, while they prevent meaningfully using the sensor
> right now, they also seem easy to resolve.
> 
> Additionally --- if the sensor does not provide meaningful values for its
> vendor and model, we could use the device's compatible string as the basis
> for the firmware file name. I can post a patch for this.

-- 
Regards,

Laurent Pinchart

Re: Sensor driver support with CCS

[Umang Jain]

Hi Laurent,

On 10/18/23 4:02 PM, Laurent Pinchart wrote:
> On Wed, Oct 18, 2023 at 09:27:59AM +0000, Sakari Ailus wrote:
>> On Wed, Oct 18, 2023 at 12:04:46PM +0530, Umang Jain wrote:
>>> Hello everyone,
>>>
>>> Past few weeks I have been working to support IMX519 sensor with CCS
>>> driver. The highlight is that I can capture frames with CCS driver now,
>>> with a few workarounds. This thread summarises the issues that we faced
>>> and act as placeholder for more open and structured discussion.
>>>
>>> - First open question is regarding the upstream support of a sensor
>>>    driver with CCS. How should we term that a sensor is 'mainline-supported'
>>>    if it is getting supported by CCS. How can one use a sensor out of the box
>>>    with mainline kernel, when supported by CCS?
>> If the sensor is fully compliant, it should "just work", but then there's
>> the question of CCS static data if the sensor needs it.
>>
>> For devices that aren't fully compliant this is a more complicated
>> question. Some devices might work with some parameters only, in particular
>> horizontal blanking is a sensitive value. The approach here should be to
>> set minimum and maximum to the same value to force horizontal blanking
>> value that is known to work. The if rules could be used for this as well.
>>
>>> - Continued from the previous discusion point, the more specific question is
>>>    about the sensor-specific static data that is composed of
>>>    manfacturer-specific-registers (MSRs) and (maybe) overridden
>>>    sensor-read-only-regs which are created as part of ccs-tools [1].
>>>    This becomes part of firmware? which needs to live /lib/firmware and
>>>    then picked up by CCS driver.
>>>
>>>    Where can they be centralized ? is linux-firmware an option? [2].
>> I'd favour this.
> linux-firmware could host the binary files, but how about the YAML
> sources ?
>
>>> - It was noticed that with current version of CCS - I was only able to
>>>    get 1/3 buffer filled. This was due to the fact that LINE_LENGTH_PCK
>>>    and FRAME_LENGTH_LINES registers are not getting updated to permissible
>>>    values in / before ccs_start_streaming() starts. Hence they are stuck
>>>    with values from probe time.
>> If you're not changing vertical or horizontal blanking, these are likely
>> the correct values from driver perspective. If these values do not work
>> however, then you most likely have an issue with sensor provided limit and
>> capability information. This should be fixed using CCS static data.
> I may have understood Umang incorrectly, but I thought he meant that the
> driver doesn't program those registers if the controls are not changed
> by userspace, and the register power-up default values are not right.
> Umang, is my understanding wrong ?

Tested everything again, so.. it does seem invoke through ccs_set_ctrl() 
before start streaming but write with the same values as defaults, so it 
appeared to me nothing changed before stream start to me.

So I can say, it wasn't a issue with the values, but the units implied. 
LINE_LENGTH_PCK wasn't written correctly in terms of units. Fixed that 
as part of
[PATCH] media: css: Write LINE_LENGTH_PCK correctly on linux-media.


>
>>> These are top issues we faced while trying to support IMX519. This thread
>>> will act as placeholder to have more discussions in the open and/or help
>>> other sensors that can be supported with CCS.
>> I'm happy to see you using the CCS driver on a sensor I haven't used
>> before. The above issues, while they prevent meaningfully using the sensor
>> right now, they also seem easy to resolve.
>>
>> Additionally --- if the sensor does not provide meaningful values for its
>> vendor and model, we could use the device's compatible string as the basis
>> for the firmware file name. I can post a patch for this.