[PATCH 0/8] v4l: ti-vpe: Add support for scaling and color conversion

2013-12-12 Thread Archit Taneja
The VPE and VIP IPs in DRA7x contain common scaler and color conversion hardware
blocks. We create libraries for these components such that the vpe driver and
the vip driver(in future) can use these library funcs to configure the blocks.

There are some points for which I would like comments:

- For VPE, setting the format and colorspace for the source and destination
  queues is enough to determine how these blocks need to be configured and
  whether they need to be bypassed or not. So it didn't make sense to represent
  them as media controller entities. For VIP(driver not upstream yet), it's
  possible that there are multiple data paths which may or may not include these
  blocks. However, the current use cases don't require such flexibility. There
  may be a need to re-consider a media controller like setup once we work on the
  VIP driver. Is it a good idea in terms of user-space compatibilty if we use
  media controller framework in the future.

- These blocks will also require some custom control commands later on. For
  example, we may want to tell the scaler later on to perform bi-linear
  scaling, or perform peaking at a particular frequency.

- The current series keeps the default scaler coefficients in a header file.
  These coefficients add a lot of lines of code in the kernel. Does it make more
  sense for the user application to pass the co-efficients to the kernel using
  an ioctl? Is there any driver which currenlty does this?

The series is based on the branch:

git://linuxtv.org/media_tree.git master

Archit Taneja (8):
  v4l: ti-vpe: create a scaler block library
  v4l: ti-vpe: support loading of scaler coefficients
  v4l: ti-vpe: make vpe driver load scaler coefficients
  v4l: ti-vpe: enable basic scaler support
  v4l: ti-vpe: create a color space converter block library
  v4l: ti-vpe: Add helper to perform color conversion
  v4l: ti-vpe: enable CSC support for VPE
  v4l: ti-vpe: Add a type specifier to describe vpdma data format type

 drivers/media/platform/ti-vpe/Makefile   |2 +-
 drivers/media/platform/ti-vpe/csc.c  |  196 +
 drivers/media/platform/ti-vpe/csc.h  |   68 ++
 drivers/media/platform/ti-vpe/sc.c   |  311 +++
 drivers/media/platform/ti-vpe/sc.h   |  208 +
 drivers/media/platform/ti-vpe/sc_coeff.h | 1342 ++
 drivers/media/platform/ti-vpe/vpdma.c|   36 +-
 drivers/media/platform/ti-vpe/vpdma.h|7 +
 drivers/media/platform/ti-vpe/vpe.c  |  251 --
 drivers/media/platform/ti-vpe/vpe_regs.h |  187 -
 10 files changed, 2335 insertions(+), 273 deletions(-)
 create mode 100644 drivers/media/platform/ti-vpe/csc.c
 create mode 100644 drivers/media/platform/ti-vpe/csc.h
 create mode 100644 drivers/media/platform/ti-vpe/sc.c
 create mode 100644 drivers/media/platform/ti-vpe/sc.h
 create mode 100644 drivers/media/platform/ti-vpe/sc_coeff.h

-- 
1.8.3.2

--
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: [PATCH 0/8] v4l: ti-vpe: Add support for scaling and color conversion

2013-12-17 Thread Hans Verkuil
On 12/12/2013 09:35 AM, Archit Taneja wrote:
> The VPE and VIP IPs in DRA7x contain common scaler and color conversion 
> hardware
> blocks. We create libraries for these components such that the vpe driver and
> the vip driver(in future) can use these library funcs to configure the blocks.
> 
> There are some points for which I would like comments:
> 
> - For VPE, setting the format and colorspace for the source and destination
>   queues is enough to determine how these blocks need to be configured and
>   whether they need to be bypassed or not. So it didn't make sense to 
> represent
>   them as media controller entities. For VIP(driver not upstream yet), it's
>   possible that there are multiple data paths which may or may not include 
> these
>   blocks. However, the current use cases don't require such flexibility. There
>   may be a need to re-consider a media controller like setup once we work on 
> the
>   VIP driver. Is it a good idea in terms of user-space compatibilty if we use
>   media controller framework in the future.

As long as you don't need the mc, then there is no need to implement it.

> - These blocks will also require some custom control commands later on. For
>   example, we may want to tell the scaler later on to perform bi-linear
>   scaling, or perform peaking at a particular frequency.
> 
> - The current series keeps the default scaler coefficients in a header file.
>   These coefficients add a lot of lines of code in the kernel. Does it make 
> more
>   sense for the user application to pass the co-efficients to the kernel using
>   an ioctl? Is there any driver which currenlty does this?

I think it is good to keep it in the driver. Otherwise apps would be forced to
set up the table. It's about 11 kilobyte in memory, which isn't that bad.

> 
> The series is based on the branch:
> 
> git://linuxtv.org/media_tree.git master
> 
> Archit Taneja (8):
>   v4l: ti-vpe: create a scaler block library
>   v4l: ti-vpe: support loading of scaler coefficients
>   v4l: ti-vpe: make vpe driver load scaler coefficients
>   v4l: ti-vpe: enable basic scaler support
>   v4l: ti-vpe: create a color space converter block library
>   v4l: ti-vpe: Add helper to perform color conversion
>   v4l: ti-vpe: enable CSC support for VPE
>   v4l: ti-vpe: Add a type specifier to describe vpdma data format type
> 
>  drivers/media/platform/ti-vpe/Makefile   |2 +-
>  drivers/media/platform/ti-vpe/csc.c  |  196 +
>  drivers/media/platform/ti-vpe/csc.h  |   68 ++
>  drivers/media/platform/ti-vpe/sc.c   |  311 +++
>  drivers/media/platform/ti-vpe/sc.h   |  208 +
>  drivers/media/platform/ti-vpe/sc_coeff.h | 1342 
> ++
>  drivers/media/platform/ti-vpe/vpdma.c|   36 +-
>  drivers/media/platform/ti-vpe/vpdma.h|7 +
>  drivers/media/platform/ti-vpe/vpe.c  |  251 --
>  drivers/media/platform/ti-vpe/vpe_regs.h |  187 -
>  10 files changed, 2335 insertions(+), 273 deletions(-)
>  create mode 100644 drivers/media/platform/ti-vpe/csc.c
>  create mode 100644 drivers/media/platform/ti-vpe/csc.h
>  create mode 100644 drivers/media/platform/ti-vpe/sc.c
>  create mode 100644 drivers/media/platform/ti-vpe/sc.h
>  create mode 100644 drivers/media/platform/ti-vpe/sc_coeff.h
> 

For this patch series:

Acked-by: Hans Verkuil 

Regards,

Hans
--
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: [PATCH 0/8] v4l: ti-vpe: Add support for scaling and color conversion

2013-12-17 Thread Archit Taneja

Hi Hans,

On Tuesday 17 December 2013 01:30 PM, Hans Verkuil wrote:

On 12/12/2013 09:35 AM, Archit Taneja wrote:

The VPE and VIP IPs in DRA7x contain common scaler and color conversion hardware
blocks. We create libraries for these components such that the vpe driver and
the vip driver(in future) can use these library funcs to configure the blocks.

There are some points for which I would like comments:

- For VPE, setting the format and colorspace for the source and destination
   queues is enough to determine how these blocks need to be configured and
   whether they need to be bypassed or not. So it didn't make sense to represent
   them as media controller entities. For VIP(driver not upstream yet), it's
   possible that there are multiple data paths which may or may not include 
these
   blocks. However, the current use cases don't require such flexibility. There
   may be a need to re-consider a media controller like setup once we work on 
the
   VIP driver. Is it a good idea in terms of user-space compatibilty if we use
   media controller framework in the future.


As long as you don't need the mc, then there is no need to implement it.


The thing is that we want to use these blocks for a future capture 
hardware called VIP. A VIP port can capture multiple streams from 
different sensors in time slices, and each of those streams might be 
sharing the same hardware, but probably in different configurations. I'm 
not completely aware of media controller details, and whether it can 
help us here.


I was just wondering if it would be a problem if we later realise that 
mc might be useful for some use cases. Would implementing it then break 
previous user space applications which don't call mc ioctls?






- These blocks will also require some custom control commands later on. For
   example, we may want to tell the scaler later on to perform bi-linear
   scaling, or perform peaking at a particular frequency.

- The current series keeps the default scaler coefficients in a header file.
   These coefficients add a lot of lines of code in the kernel. Does it make 
more
   sense for the user application to pass the co-efficients to the kernel using
   an ioctl? Is there any driver which currenlty does this?


I think it is good to keep it in the driver. Otherwise apps would be forced to
set up the table. It's about 11 kilobyte in memory, which isn't that bad.


Okay.





The series is based on the branch:

git://linuxtv.org/media_tree.git master

Archit Taneja (8):
   v4l: ti-vpe: create a scaler block library
   v4l: ti-vpe: support loading of scaler coefficients
   v4l: ti-vpe: make vpe driver load scaler coefficients
   v4l: ti-vpe: enable basic scaler support
   v4l: ti-vpe: create a color space converter block library
   v4l: ti-vpe: Add helper to perform color conversion
   v4l: ti-vpe: enable CSC support for VPE
   v4l: ti-vpe: Add a type specifier to describe vpdma data format type

  drivers/media/platform/ti-vpe/Makefile   |2 +-
  drivers/media/platform/ti-vpe/csc.c  |  196 +
  drivers/media/platform/ti-vpe/csc.h  |   68 ++
  drivers/media/platform/ti-vpe/sc.c   |  311 +++
  drivers/media/platform/ti-vpe/sc.h   |  208 +
  drivers/media/platform/ti-vpe/sc_coeff.h | 1342 ++
  drivers/media/platform/ti-vpe/vpdma.c|   36 +-
  drivers/media/platform/ti-vpe/vpdma.h|7 +
  drivers/media/platform/ti-vpe/vpe.c  |  251 --
  drivers/media/platform/ti-vpe/vpe_regs.h |  187 -
  10 files changed, 2335 insertions(+), 273 deletions(-)
  create mode 100644 drivers/media/platform/ti-vpe/csc.c
  create mode 100644 drivers/media/platform/ti-vpe/csc.h
  create mode 100644 drivers/media/platform/ti-vpe/sc.c
  create mode 100644 drivers/media/platform/ti-vpe/sc.h
  create mode 100644 drivers/media/platform/ti-vpe/sc_coeff.h



For this patch series:

Acked-by: Hans Verkuil 


Thanks for the review.

Archit
--
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: [PATCH 0/8] v4l: ti-vpe: Add support for scaling and color conversion

2013-12-17 Thread Hans Verkuil
On 12/17/13 12:19, Archit Taneja wrote:
> Hi Hans,
> 
> On Tuesday 17 December 2013 01:30 PM, Hans Verkuil wrote:
>> On 12/12/2013 09:35 AM, Archit Taneja wrote:
>>> The VPE and VIP IPs in DRA7x contain common scaler and color
>>> conversion hardware blocks. We create libraries for these
>>> components such that the vpe driver and the vip driver(in future)
>>> can use these library funcs to configure the blocks.
>>> 
>>> There are some points for which I would like comments:
>>> 
>>> - For VPE, setting the format and colorspace for the source and
>>> destination queues is enough to determine how these blocks need
>>> to be configured and whether they need to be bypassed or not. So
>>> it didn't make sense to represent them as media controller
>>> entities. For VIP(driver not upstream yet), it's possible that
>>> there are multiple data paths which may or may not include these 
>>> blocks. However, the current use cases don't require such
>>> flexibility. There may be a need to re-consider a media
>>> controller like setup once we work on the VIP driver. Is it a
>>> good idea in terms of user-space compatibilty if we use media
>>> controller framework in the future.
>> 
>> As long as you don't need the mc, then there is no need to
>> implement it.
> 
> The thing is that we want to use these blocks for a future capture
> hardware called VIP. A VIP port can capture multiple streams from
> different sensors in time slices, and each of those streams might be
> sharing the same hardware, but probably in different configurations.
> I'm not completely aware of media controller details, and whether it
> can help us here.
> 
> I was just wondering if it would be a problem if we later realise
> that mc might be useful for some use cases. Would implementing it
> then break previous user space applications which don't call mc
> ioctls?

My understanding is that in the current vpe driver the mc won't be
needed, so there is no point adding it. When implementing the vip
capture driver the mc might be needed, but that should not require
the vpe to add the mc API as well. It's a decision that has to be
made per driver.

When you start work on the vip driver it is probably a good idea to
talk to Laurent and myself first to see whether the mc is needed or
not.

If you have a block diagram of the video hardware that you can share,
then that will be quite useful.

Regards,

Hans
--
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: [PATCH 0/8] v4l: ti-vpe: Add support for scaling and color conversion

2013-12-17 Thread Archit Taneja

Hi,

On Tuesday 17 December 2013 05:19 PM, Hans Verkuil wrote:

On 12/17/13 12:19, Archit Taneja wrote:

Hi Hans,

On Tuesday 17 December 2013 01:30 PM, Hans Verkuil wrote:

On 12/12/2013 09:35 AM, Archit Taneja wrote:

The VPE and VIP IPs in DRA7x contain common scaler and color
conversion hardware blocks. We create libraries for these
components such that the vpe driver and the vip driver(in future)
can use these library funcs to configure the blocks.

There are some points for which I would like comments:

- For VPE, setting the format and colorspace for the source and
destination queues is enough to determine how these blocks need
to be configured and whether they need to be bypassed or not. So
it didn't make sense to represent them as media controller
entities. For VIP(driver not upstream yet), it's possible that
there are multiple data paths which may or may not include these
blocks. However, the current use cases don't require such
flexibility. There may be a need to re-consider a media
controller like setup once we work on the VIP driver. Is it a
good idea in terms of user-space compatibilty if we use media
controller framework in the future.


As long as you don't need the mc, then there is no need to
implement it.


The thing is that we want to use these blocks for a future capture
hardware called VIP. A VIP port can capture multiple streams from
different sensors in time slices, and each of those streams might be
sharing the same hardware, but probably in different configurations.
I'm not completely aware of media controller details, and whether it
can help us here.

I was just wondering if it would be a problem if we later realise
that mc might be useful for some use cases. Would implementing it
then break previous user space applications which don't call mc
ioctls?


My understanding is that in the current vpe driver the mc won't be
needed, so there is no point adding it. When implementing the vip
capture driver the mc might be needed, but that should not require
the vpe to add the mc API as well. It's a decision that has to be
made per driver.


That's right, vpe doesn't need mc. It might be needed for vip. The 
reason I brought it up now is because some of the blocks like SC/CSC are 
replicated in VIP hardware, and I created them in a 'library' sort of 
way in this series. If vip driver uses mc, these blocks might need to 
become media entities.




When you start work on the vip driver it is probably a good idea to
talk to Laurent and myself first to see whether the mc is needed or
not.


Thanks, that'll be quite useful. I'll look up some mc documentation and 
drivers using mc myself as well.




If you have a block diagram of the video hardware that you can share,
then that will be quite useful.


Thanks for the clarification. I don't think the DRA7x documentation is 
public yet. I'll try to look for a block diagram(or create one) and 
share it with the list.


Archit
--
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