Re: [RFC] ISP lane shifter support

2011-02-24 Thread Hans Verkuil
Hi Michael,

Sorry for the delay, the last 2-3 weeks were very busy.

On Tuesday, February 22, 2011 13:36:08 Michael Jones wrote:
 Hans, can you weigh in on this?  I'm waiting to submit a patch to
 implement lane shifter support until I get a consensus what the best
 approach is.  Laurent and Sakari favor having a different format on the
 sensor output than on the CCDC input to indicate a shift.
 
 If you agree that this is a sensible approach, I will go ahead and
 submit my patch soon.

I think this is a sensible approach as well. So you can go ahead!

Regards,

Hans

 
 thanks,
 Michael
 
 On 02/11/2011 02:06 PM, Laurent Pinchart wrote:
  Hi Michael,
  
  On Friday 11 February 2011 13:07:33 Michael Jones wrote:
  On 01/27/2011 12:46 AM, Guennadi Liakhovetski wrote:
  Looking at the Data-Lane Shifter table (12.27 in my datasheet, in the
  Bridge-Lane Shifter chapter), I think, the first two columns are fixed
  by the board design, right? So, our freedom lies only in one line there
  and is a single parameter - the shift value. The output shifter (VPIN) is
  independent from this one, but not unrelated. It seems logical to me to
  relate the former one to CCDC's input pad, and the latter one to CCDC's
  output pad. AFAIU, Laurent, your implementation in what concerns pad
  configuration is: let the user configure all interfaces independently,
  and first when we have to actually activate the pipeline (start
  streaming or configure video buffers) we can verify, whether all parts
  fit together.
 
  I would like to add this lane shifter support.  Would you like me to
  implement it as Guennadi suggested- letting the user set all 3 CCDC pad
  formats arbitrarily and postpone the consistency checks to streamon time?
  
  I've discussed this with Sakari Ailus, and we would implement it with 
  different formats on the sensor output and the CCDC input. I'd like to get 
  Hans Verkuil's opinion.
  
 
 
 MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
 Registergericht: Amtsgericht Stuttgart, HRB 271090
 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
 --
 To unsubscribe from this list: send the line unsubscribe linux-media in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 
 

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] ISP lane shifter support

2011-02-22 Thread Michael Jones
Hans, can you weigh in on this?  I'm waiting to submit a patch to
implement lane shifter support until I get a consensus what the best
approach is.  Laurent and Sakari favor having a different format on the
sensor output than on the CCDC input to indicate a shift.

If you agree that this is a sensible approach, I will go ahead and
submit my patch soon.

thanks,
Michael

On 02/11/2011 02:06 PM, Laurent Pinchart wrote:
 Hi Michael,
 
 On Friday 11 February 2011 13:07:33 Michael Jones wrote:
 On 01/27/2011 12:46 AM, Guennadi Liakhovetski wrote:
 Looking at the Data-Lane Shifter table (12.27 in my datasheet, in the
 Bridge-Lane Shifter chapter), I think, the first two columns are fixed
 by the board design, right? So, our freedom lies only in one line there
 and is a single parameter - the shift value. The output shifter (VPIN) is
 independent from this one, but not unrelated. It seems logical to me to
 relate the former one to CCDC's input pad, and the latter one to CCDC's
 output pad. AFAIU, Laurent, your implementation in what concerns pad
 configuration is: let the user configure all interfaces independently,
 and first when we have to actually activate the pipeline (start
 streaming or configure video buffers) we can verify, whether all parts
 fit together.

 I would like to add this lane shifter support.  Would you like me to
 implement it as Guennadi suggested- letting the user set all 3 CCDC pad
 formats arbitrarily and postpone the consistency checks to streamon time?
 
 I've discussed this with Sakari Ailus, and we would implement it with 
 different formats on the sensor output and the CCDC input. I'd like to get 
 Hans Verkuil's opinion.
 


MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] ISP lane shifter support

2011-02-02 Thread Michael Jones
Hi Guennadi, Laurent,

On 01/27/2011 12:46 AM, Guennadi Liakhovetski wrote:

snip


 I didn't realize the video port can further shift the data.  Where can I
 find this in the TRM?

 VPIN field of the CCDC_FMTCFG register.
 
 This only plays a role, if cam_d is set to 10 bits raw in 
 CCDC_SYN_MODE.DATSIZ, right?
 
I didn't understand from the TRM that this is the case.  I also didn't
see that VPIN is only relevant with parallel data.  Is it so?

snip

 It could be, yes. The other option is to modify the format at the CCDC 
 input. 
 I agree that both options have drawbacks.

 Hans, Guennadi, any opinion on this ?
 
 Looking at the Data-Lane Shifter table (12.27 in my datasheet, in the 
 Bridge-Lane Shifter chapter), I think, the first two columns are fixed 
 by the board design, right? So, our freedom lies only in one line there 
 and is a single parameter - the shift value. The output shifter (VPIN) is 
 independent from this one, but not unrelated. It seems logical to me to 
 relate the former one to CCDC's input pad, and the latter one to CCDC's 
 output pad. AFAIU, Laurent, your implementation in what concerns pad 
 configuration is: let the user configure all interfaces independently, and 
 first when we have to actually activate the pipeline (start streaming or 
 configure video buffers) we can verify, whether all parts fit together. 
 So, why don't we stay consistent and do the same here? Give the user both 
 parameters and see how clever they were in the end;) I also think, if we 
 later decide to add some consistency checks, we can always do it.
 
Are you proposing having different formats on each end of the link
between the sensor and the CCDC?  Or do you agree with my favored
approach that the lane shift value be determined by the difference
between the CCDC input format and the CCDC output format(s)?

I assume the VPIN value will always just select the upper 10 bits of the
format which the CCDC is outputting on its other output pad.

I understand that Laurent is out until Monday, so I'll wait for him to
weigh in on this again before moving forward.

 Thanks
 Guennadi


thanks,
Michael

MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] ISP lane shifter support

2011-01-26 Thread Guennadi Liakhovetski
On Tue, 25 Jan 2011, Laurent Pinchart wrote:

 Hi Michael,
 
 On Tuesday 25 January 2011 10:10:50 Michael Jones wrote:
  On 01/24/2011 08:45 PM, Laurent Pinchart wrote:
   On Monday 24 January 2011 15:16:28 Michael Jones wrote:
   On 01/24/2011 02:57 PM, Laurent Pinchart wrote:
   snip
   
   As the lane shifter is located at the CCDC input, it might be easier
   to implement support for this using the CCDC input format.
   ispvideo.c would need to validate the pipeline when the output of
   the entity connected to the CCDC input (parallel sensor, CCP2 or
   CSI2) is configured with a format that can be shifted to the format
   at the CCDC input.
   
   This crossed my mind, but it seems illogical to have a link with a
   different format at each of its ends.
   
   I agree in theory, but it might be problematic for the CCDC. Right now
   the CCDC can write to memory or send the data to the preview engine,
   but not both at the same time. That's something that I'd like to
   change in the future. What happens if the user then sets different
   widths on the output pads ?
   
   Shouldn't we prohibit the user from doing this in ccdc_[try/set]_format
   in the first place? By prohibit, I mean shouldn't we be sure that the
   pixel format on pad 1 is always the same as on pad 2?
   
   Yes we should (although we could have a larger width on the memory write
   port, as the video port can further shift the data).
  
  Doesn't this conflict with your comment below that we shouldn't silently
  change pad 1 when setting pad 2?  How can we ensure that they're always
  the same if a change in one doesn't result in a change in the other?
  See my example below.
 
 Yes it does, and that's why I'm not too sure yet how this should be 
 implemented.
 
  I didn't realize the video port can further shift the data.  Where can I
  find this in the TRM?
 
 VPIN field of the CCDC_FMTCFG register.

This only plays a role, if cam_d is set to 10 bits raw in 
CCDC_SYN_MODE.DATSIZ, right?

   Downside: this suggests that set_fmt on pad 2 could change the fmt on
   pad 1, which may be unexpected. But that does at least reflect the
   reality of the hardware, right?
   
   I don't think it would be a good idea to silently change formats on pad 1
   when setting the format on pad 2. Applications don't expect that. That's
   why I've proposed changing the format on pad 0 instead. I agree that it
   would be better to have the same format on the sensor output and on CCDC
   pad 0 though.
  
  I don't understand how we can change the pixel format on pad 1 without
  also changing it on pad 2.  Let me take a simple example:
  0. Default state: all 3 CCDC pads have SGRBG10.
  1. Sensor delivers Y10, so I set CCDC pad 0 to Y10. CCDC then changes
  format of pad 12 to Y10 also.
  2. I want 8-bit data written to memory, so I set Y8 on pad 1 to use the
  shifter. Pad 0 stays Y10, but pad 2 can no longer get Y10, so (?) it
  must be changed to Y8.  And I have to allow the change on pad 1 to be
  able to use the shifter at all.
  
  I agree applications may not expect this behavior.  They may _expect_
  that they can get Y10 to the video port and Y8 to memory, but they
  can't.  Isn't this just what we pay for the simplicity of building the
  lane shifter into the CCDC subdev rather than creating its own subdev?
 
 It could be, yes. The other option is to modify the format at the CCDC input. 
 I agree that both options have drawbacks.
 
 Hans, Guennadi, any opinion on this ?

Looking at the Data-Lane Shifter table (12.27 in my datasheet, in the 
Bridge-Lane Shifter chapter), I think, the first two columns are fixed 
by the board design, right? So, our freedom lies only in one line there 
and is a single parameter - the shift value. The output shifter (VPIN) is 
independent from this one, but not unrelated. It seems logical to me to 
relate the former one to CCDC's input pad, and the latter one to CCDC's 
output pad. AFAIU, Laurent, your implementation in what concerns pad 
configuration is: let the user configure all interfaces independently, and 
first when we have to actually activate the pipeline (start streaming or 
configure video buffers) we can verify, whether all parts fit together. 
So, why don't we stay consistent and do the same here? Give the user both 
parameters and see how clever they were in the end;) I also think, if we 
later decide to add some consistency checks, we can always do it.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] ISP lane shifter support

2011-01-25 Thread Michael Jones
Hi Laurent,

On 01/24/2011 08:45 PM, Laurent Pinchart wrote:
 Hi Michael,
 
 On Monday 24 January 2011 15:16:28 Michael Jones wrote:
 On 01/24/2011 02:57 PM, Laurent Pinchart wrote:
 snip

 As the lane shifter is located at the CCDC input, it might be easier to
 implement support for this using the CCDC input format. ispvideo.c
 would need to validate the pipeline when the output of the entity
 connected to the CCDC input (parallel sensor, CCP2 or CSI2) is
 configured with a format that can be shifted to the format at the CCDC
 input.

 This crossed my mind, but it seems illogical to have a link with a
 different format at each of its ends.

 I agree in theory, but it might be problematic for the CCDC. Right now
 the CCDC can write to memory or send the data to the preview engine, but
 not both at the same time. That's something that I'd like to change in
 the future. What happens if the user then sets different widths on the
 output pads ?

 Shouldn't we prohibit the user from doing this in ccdc_[try/set]_format
 in the first place? By prohibit, I mean shouldn't we be sure that the
 pixel format on pad 1 is always the same as on pad 2?
 
 Yes we should (although we could have a larger width on the memory write 
 port, 
 as the video port can further shift the data).

Doesn't this conflict with your comment below that we shouldn't silently
change pad 1 when setting pad 2?  How can we ensure that they're always
the same if a change in one doesn't result in a change in the other?
See my example below.

I didn't realize the video port can further shift the data.  Where can I
find this in the TRM?

 
 Downside: this suggests that set_fmt on pad 2 could change the fmt on pad 1,
 which may be unexpected. But that does at least reflect the reality of the
 hardware, right?
 
 I don't think it would be a good idea to silently change formats on pad 1 
 when 
 setting the format on pad 2. Applications don't expect that. That's why I've 
 proposed changing the format on pad 0 instead. I agree that it would be 
 better 
 to have the same format on the sensor output and on CCDC pad 0 though.
 

I don't understand how we can change the pixel format on pad 1 without
also changing it on pad 2.  Let me take a simple example:
0. Default state: all 3 CCDC pads have SGRBG10.
1. Sensor delivers Y10, so I set CCDC pad 0 to Y10. CCDC then changes
format of pad 12 to Y10 also.
2. I want 8-bit data written to memory, so I set Y8 on pad 1 to use the
shifter. Pad 0 stays Y10, but pad 2 can no longer get Y10, so (?) it
must be changed to Y8.  And I have to allow the change on pad 1 to be
able to use the shifter at all.

I agree applications may not expect this behavior.  They may _expect_
that they can get Y10 to the video port and Y8 to memory, but they
can't.  Isn't this just what we pay for the simplicity of building the
lane shifter into the CCDC subdev rather than creating its own subdev?

-Michael

MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] ISP lane shifter support

2011-01-25 Thread Laurent Pinchart
Hi Michael,

On Tuesday 25 January 2011 10:10:50 Michael Jones wrote:
 On 01/24/2011 08:45 PM, Laurent Pinchart wrote:
  On Monday 24 January 2011 15:16:28 Michael Jones wrote:
  On 01/24/2011 02:57 PM, Laurent Pinchart wrote:
  snip
  
  As the lane shifter is located at the CCDC input, it might be easier
  to implement support for this using the CCDC input format.
  ispvideo.c would need to validate the pipeline when the output of
  the entity connected to the CCDC input (parallel sensor, CCP2 or
  CSI2) is configured with a format that can be shifted to the format
  at the CCDC input.
  
  This crossed my mind, but it seems illogical to have a link with a
  different format at each of its ends.
  
  I agree in theory, but it might be problematic for the CCDC. Right now
  the CCDC can write to memory or send the data to the preview engine,
  but not both at the same time. That's something that I'd like to
  change in the future. What happens if the user then sets different
  widths on the output pads ?
  
  Shouldn't we prohibit the user from doing this in ccdc_[try/set]_format
  in the first place? By prohibit, I mean shouldn't we be sure that the
  pixel format on pad 1 is always the same as on pad 2?
  
  Yes we should (although we could have a larger width on the memory write
  port, as the video port can further shift the data).
 
 Doesn't this conflict with your comment below that we shouldn't silently
 change pad 1 when setting pad 2?  How can we ensure that they're always
 the same if a change in one doesn't result in a change in the other?
 See my example below.

Yes it does, and that's why I'm not too sure yet how this should be 
implemented.

 I didn't realize the video port can further shift the data.  Where can I
 find this in the TRM?

VPIN field of the CCDC_FMTCFG register.

  Downside: this suggests that set_fmt on pad 2 could change the fmt on
  pad 1, which may be unexpected. But that does at least reflect the
  reality of the hardware, right?
  
  I don't think it would be a good idea to silently change formats on pad 1
  when setting the format on pad 2. Applications don't expect that. That's
  why I've proposed changing the format on pad 0 instead. I agree that it
  would be better to have the same format on the sensor output and on CCDC
  pad 0 though.
 
 I don't understand how we can change the pixel format on pad 1 without
 also changing it on pad 2.  Let me take a simple example:
 0. Default state: all 3 CCDC pads have SGRBG10.
 1. Sensor delivers Y10, so I set CCDC pad 0 to Y10. CCDC then changes
 format of pad 12 to Y10 also.
 2. I want 8-bit data written to memory, so I set Y8 on pad 1 to use the
 shifter. Pad 0 stays Y10, but pad 2 can no longer get Y10, so (?) it
 must be changed to Y8.  And I have to allow the change on pad 1 to be
 able to use the shifter at all.
 
 I agree applications may not expect this behavior.  They may _expect_
 that they can get Y10 to the video port and Y8 to memory, but they
 can't.  Isn't this just what we pay for the simplicity of building the
 lane shifter into the CCDC subdev rather than creating its own subdev?

It could be, yes. The other option is to modify the format at the CCDC input. 
I agree that both options have drawbacks.

Hans, Guennadi, any opinion on this ?

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] ISP lane shifter support

2011-01-24 Thread Michael Jones
Hi Laurent,

Thanks for the feedback.

On 01/24/2011 01:10 AM, Laurent Pinchart wrote:
 Hi Michael,
 
 On Friday 21 January 2011 09:40:21 Michael Jones wrote:
 Hi all,

 In the OMAP ISP driver, I'm interested in being able to choose between
 8-bit and 12-bit formats when I have a 12-bit sensor attached.  At the
 moment it looks like it's only possible to define this statically with
 data_lane_shift in the board definition.  But with the ISP's lane
 shifter, it should be possible for the application to ask for 8-bits
 although it has a 12-bit sensor attached.
 
 That's right. This would be an interesting feature for the driver. It's also 
 possible to implement this at the input of the video port (but only when 
 forwarding data to the preview engine).

True, but I do also want the feature available for data written
directly to memory.

 
 Has anybody already begun implementing this functionality?
 
 Not that I know of.
 
 One approach that comes to mind is to create a subdev for the
 bridge/lane shifter in front of the CCDC, but this also seems a bit
 overkill.  Otherwise, perhaps consider the lane shifter a part of the
 CCDC and put the code in there?
 
 I would keep the code in isp.c, and call it from ccdc_configure(). It should 
 just be a matter of adding an argument to the function.

It seems unnecessary to add an arg to ccdc_configure (that's what I
understood you to mean). It gets isp_ccdc_device, which has all the
necessary info (pixel format in, which output is active, pixel format
out).  Seems like I could implement it entirely within
isp_configure_bridge(). And of course some changes in
ccdc_[try/set]_format().  This is what I will try to do.

 
 Then ccdc_try_format() would have to check whether the sink pad has a pixel
 format which is shiftable to the requested pixel format on the source pad. 
 A problem with this might be if there are architectures which have a CCDC
 but no shifter.
 
 The CCDC module already calls isp_configure_bridge(), so I don't think it's 
 an 
 issue for now. Let's fix the code when (and if) we start using the driver on 
 a 
 platform without a lane shifter.

Agreed.

 
 Are there other approaches I'm not considering?  Or problems I'm
 overlooking?
 
 As the lane shifter is located at the CCDC input, it might be easier to 
 implement support for this using the CCDC input format. ispvideo.c would need 
 to validate the pipeline when the output of the entity connected to the CCDC 
 input (parallel sensor, CCP2 or CSI2) is configured with a format that can be 
 shifted to the format at the CCDC input.

This crossed my mind, but it seems illogical to have a link with a
different format at each of its ends.  For instance, I think it is a
sensible assumption that media-ctl automatically sets the format at the
remote end of a link if you're setting an output pad's format. This is
when I thought a subdev of its own would be more logically consistent
with the media controller framework (although overkill).

 
 Also- it looks like the CCDC now supports writing 12-bit bayer
 data to memory.  Is that true?
 
 That's correct. It will support 8-bit grey data soon (patches have been 
 submitted internally already).
 

Michael

MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] ISP lane shifter support

2011-01-24 Thread Laurent Pinchart
Hi Michael,

On Monday 24 January 2011 14:47:04 Michael Jones wrote:
 On 01/24/2011 01:10 AM, Laurent Pinchart wrote:
  On Friday 21 January 2011 09:40:21 Michael Jones wrote:
  Hi all,
  
  In the OMAP ISP driver, I'm interested in being able to choose between
  8-bit and 12-bit formats when I have a 12-bit sensor attached.  At the
  moment it looks like it's only possible to define this statically with
  data_lane_shift in the board definition.  But with the ISP's lane
  shifter, it should be possible for the application to ask for 8-bits
  although it has a 12-bit sensor attached.
  
  That's right. This would be an interesting feature for the driver. It's
  also possible to implement this at the input of the video port (but only
  when forwarding data to the preview engine).
 
 True, but I do also want the feature available for data written
 directly to memory.
 
  Has anybody already begun implementing this functionality?
  
  Not that I know of.
  
  One approach that comes to mind is to create a subdev for the
  bridge/lane shifter in front of the CCDC, but this also seems a bit
  overkill.  Otherwise, perhaps consider the lane shifter a part of the
  CCDC and put the code in there?
  
  I would keep the code in isp.c, and call it from ccdc_configure(). It
  should just be a matter of adding an argument to the function.
 
 It seems unnecessary to add an arg to ccdc_configure (that's what I
 understood you to mean). It gets isp_ccdc_device, which has all the
 necessary info (pixel format in, which output is active, pixel format
 out).  Seems like I could implement it entirely within
 isp_configure_bridge(). And of course some changes in
 ccdc_[try/set]_format().  This is what I will try to do.
 
  Then ccdc_try_format() would have to check whether the sink pad has a
  pixel format which is shiftable to the requested pixel format on the
  source pad. A problem with this might be if there are architectures
  which have a CCDC but no shifter.
  
  The CCDC module already calls isp_configure_bridge(), so I don't think
  it's an issue for now. Let's fix the code when (and if) we start using
  the driver on a platform without a lane shifter.
 
 Agreed.
 
  Are there other approaches I'm not considering?  Or problems I'm
  overlooking?
  
  As the lane shifter is located at the CCDC input, it might be easier to
  implement support for this using the CCDC input format. ispvideo.c would
  need to validate the pipeline when the output of the entity connected to
  the CCDC input (parallel sensor, CCP2 or CSI2) is configured with a
  format that can be shifted to the format at the CCDC input.
 
 This crossed my mind, but it seems illogical to have a link with a
 different format at each of its ends.

I agree in theory, but it might be problematic for the CCDC. Right now the 
CCDC can write to memory or send the data to the preview engine, but not both 
at the same time. That's something that I'd like to change in the future. What 
happens if the user then sets differents widths on the output pads ?

 For instance, I think it is a sensible assumption that media-ctl
 automatically sets the format at the remote end of a link if you're setting
 an output pad's format. This is when I thought a subdev of its own would be
 more logically consistent with the media controller framework (although
 overkill).

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] ISP lane shifter support

2011-01-24 Thread Michael Jones
Hi Laurent,

On 01/24/2011 02:57 PM, Laurent Pinchart wrote:
snip

 As the lane shifter is located at the CCDC input, it might be easier to
 implement support for this using the CCDC input format. ispvideo.c would
 need to validate the pipeline when the output of the entity connected to
 the CCDC input (parallel sensor, CCP2 or CSI2) is configured with a
 format that can be shifted to the format at the CCDC input.

 This crossed my mind, but it seems illogical to have a link with a
 different format at each of its ends.
 
 I agree in theory, but it might be problematic for the CCDC. Right now the 
 CCDC can write to memory or send the data to the preview engine, but not both 
 at the same time. That's something that I'd like to change in the future. 
 What 
 happens if the user then sets different widths on the output pads ?
 

Shouldn't we prohibit the user from doing this in ccdc_[try/set]_format
in the first place? By prohibit, I mean shouldn't we be sure that the
pixel format on pad 1 is always the same as on pad 2?  Downside: this
suggests that set_fmt on pad 2 could change the fmt on pad 1, which may
be unexpected. But that does at least reflect the reality of the
hardware, right?

snip

Michael

MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] ISP lane shifter support

2011-01-24 Thread Laurent Pinchart
Hi Michael,

On Monday 24 January 2011 15:16:28 Michael Jones wrote:
 On 01/24/2011 02:57 PM, Laurent Pinchart wrote:
 snip
 
  As the lane shifter is located at the CCDC input, it might be easier to
  implement support for this using the CCDC input format. ispvideo.c
  would need to validate the pipeline when the output of the entity
  connected to the CCDC input (parallel sensor, CCP2 or CSI2) is
  configured with a format that can be shifted to the format at the CCDC
  input.
  
  This crossed my mind, but it seems illogical to have a link with a
  different format at each of its ends.
  
  I agree in theory, but it might be problematic for the CCDC. Right now
  the CCDC can write to memory or send the data to the preview engine, but
  not both at the same time. That's something that I'd like to change in
  the future. What happens if the user then sets different widths on the
  output pads ?
 
 Shouldn't we prohibit the user from doing this in ccdc_[try/set]_format
 in the first place? By prohibit, I mean shouldn't we be sure that the
 pixel format on pad 1 is always the same as on pad 2?

Yes we should (although we could have a larger width on the memory write port, 
as the video port can further shift the data).

 Downside: this suggests that set_fmt on pad 2 could change the fmt on pad 1,
 which may be unexpected. But that does at least reflect the reality of the
 hardware, right?

I don't think it would be a good idea to silently change formats on pad 1 when 
setting the format on pad 2. Applications don't expect that. That's why I've 
proposed changing the format on pad 0 instead. I agree that it would be better 
to have the same format on the sensor output and on CCDC pad 0 though.

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] ISP lane shifter support

2011-01-23 Thread Laurent Pinchart
Hi Michael,

On Friday 21 January 2011 09:40:21 Michael Jones wrote:
 Hi all,
 
 In the OMAP ISP driver, I'm interested in being able to choose between
 8-bit and 12-bit formats when I have a 12-bit sensor attached.  At the
 moment it looks like it's only possible to define this statically with
 data_lane_shift in the board definition.  But with the ISP's lane
 shifter, it should be possible for the application to ask for 8-bits
 although it has a 12-bit sensor attached.

That's right. This would be an interesting feature for the driver. It's also 
possible to implement this at the input of the video port (but only when 
forwarding data to the preview engine).

 Has anybody already begun implementing this functionality?

Not that I know of.

 One approach that comes to mind is to create a subdev for the
 bridge/lane shifter in front of the CCDC, but this also seems a bit
 overkill.  Otherwise, perhaps consider the lane shifter a part of the
 CCDC and put the code in there?

I would keep the code in isp.c, and call it from ccdc_configure(). It should 
just be a matter of adding an argument to the function.

 Then ccdc_try_format() would have to check whether the sink pad has a pixel
 format which is shiftable to the requested pixel format on the source pad. 
 A problem with this might be if there are architectures which have a CCDC
 but no shifter.

The CCDC module already calls isp_configure_bridge(), so I don't think it's an 
issue for now. Let's fix the code when (and if) we start using the driver on a 
platform without a lane shifter.

 Are there other approaches I'm not considering?  Or problems I'm
 overlooking?

As the lane shifter is located at the CCDC input, it might be easier to 
implement support for this using the CCDC input format. ispvideo.c would need 
to validate the pipeline when the output of the entity connected to the CCDC 
input (parallel sensor, CCP2 or CSI2) is configured with a format that can be 
shifted to the format at the CCDC input.

 Also- it looks like the CCDC now supports writing 12-bit bayer
 data to memory.  Is that true?

That's correct. It will support 8-bit grey data soon (patches have been 
submitted internally already).

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html