Re: [PATCH v3 22/23] camss: Use optimal clock frequency rates

From: Sakari Ailus
Date: Thu Jul 20 2017 - 11:23:22 EST

Hi Todor,

On Mon, Jul 17, 2017 at 01:33:48PM +0300, Todor Tomov wrote:
> Use standard V4L2 control to get pixel clock rate from a sensor
> linked in the media controller pipeline. Then calculate clock
> rates on CSIPHY, CSID and VFE to use the lowest possible.
>
> If the currnet pixel clock rate of the sensor cannot be read then
> use the highest possible. This case covers also the CSID test
> generator usage.
>
> If VFE is already powered on by another pipeline, check that the
> current VFE clock rate is high enough for the new pipeline.
> If not return busy error code as VFE clock rate cannot be changed
> while VFE is running.
>
> Signed-off-by: Todor Tomov <todor.tomov@xxxxxxxxxx>
> ---
> .../media/platform/qcom/camss-8x16/camss-csid.c | 324 ++++++++++++++-------
> .../media/platform/qcom/camss-8x16/camss-csid.h | 2 +-
> .../media/platform/qcom/camss-8x16/camss-csiphy.c | 112 +++++--
> .../media/platform/qcom/camss-8x16/camss-csiphy.h | 2 +-
> .../media/platform/qcom/camss-8x16/camss-ispif.c | 23 +-
> .../media/platform/qcom/camss-8x16/camss-ispif.h | 4 +-
> drivers/media/platform/qcom/camss-8x16/camss-vfe.c | 289 +++++++++++++++---
> drivers/media/platform/qcom/camss-8x16/camss-vfe.h | 2 +-
> drivers/media/platform/qcom/camss-8x16/camss.c | 51 +++-
> drivers/media/platform/qcom/camss-8x16/camss.h | 17 +-
> 10 files changed, 634 insertions(+), 192 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss-8x16/camss-csid.c b/drivers/media/platform/qcom/camss-8x16/camss-csid.c
> index 2bf3415..5c0e359 100644
> --- a/drivers/media/platform/qcom/camss-8x16/camss-csid.c
> +++ b/drivers/media/platform/qcom/camss-8x16/camss-csid.c
> @@ -68,122 +68,224 @@ static const struct {
> u8 data_type;
> u8 decode_format;
> u8 uncompr_bpp;
> + u8 spp; /* bus samples per pixel */
> } csid_input_fmts[] = {
> {
> MEDIA_BUS_FMT_UYVY8_2X8,
> MEDIA_BUS_FMT_UYVY8_2X8,
> DATA_TYPE_YUV422_8BIT,
> DECODE_FORMAT_UNCOMPRESSED_8_BIT,
> - 16
> + 8,
> + 2

If your original patch had added the comma on the previous line, this would
be simply adding a line per array entry.

> },
> {
> MEDIA_BUS_FMT_VYUY8_2X8,
> MEDIA_BUS_FMT_VYUY8_2X8,
> DATA_TYPE_YUV422_8BIT,
> DECODE_FORMAT_UNCOMPRESSED_8_BIT,
> - 16
> + 8,
> + 2
> },
> {
> MEDIA_BUS_FMT_YUYV8_2X8,
> MEDIA_BUS_FMT_YUYV8_2X8,
> DATA_TYPE_YUV422_8BIT,
> DECODE_FORMAT_UNCOMPRESSED_8_BIT,
> - 16
> + 8,
> + 2
> },
> {
> MEDIA_BUS_FMT_YVYU8_2X8,
> MEDIA_BUS_FMT_YVYU8_2X8,
> DATA_TYPE_YUV422_8BIT,
> DECODE_FORMAT_UNCOMPRESSED_8_BIT,
> - 16
> + 8,
> + 2
> },
> {
> MEDIA_BUS_FMT_SBGGR8_1X8,
> MEDIA_BUS_FMT_SBGGR8_1X8,
> DATA_TYPE_RAW_8BIT,
> DECODE_FORMAT_UNCOMPRESSED_8_BIT,
> - 8
> + 8,
> + 1
> },
> {
> MEDIA_BUS_FMT_SGBRG8_1X8,
> MEDIA_BUS_FMT_SGBRG8_1X8,
> DATA_TYPE_RAW_8BIT,
> DECODE_FORMAT_UNCOMPRESSED_8_BIT,
> - 8
> + 8,
> + 1
> },
> {
> MEDIA_BUS_FMT_SGRBG8_1X8,
> MEDIA_BUS_FMT_SGRBG8_1X8,
> DATA_TYPE_RAW_8BIT,
> DECODE_FORMAT_UNCOMPRESSED_8_BIT,
> - 8
> + 8,
> + 1
> },
> {
> MEDIA_BUS_FMT_SRGGB8_1X8,
> MEDIA_BUS_FMT_SRGGB8_1X8,
> DATA_TYPE_RAW_8BIT,
> DECODE_FORMAT_UNCOMPRESSED_8_BIT,
> - 8
> + 8,
> + 1
> },
> {
> MEDIA_BUS_FMT_SBGGR10_1X10,
> MEDIA_BUS_FMT_SBGGR10_1X10,
> DATA_TYPE_RAW_10BIT,
> DECODE_FORMAT_UNCOMPRESSED_10_BIT,
> - 10
> + 10,
> + 1
> },
> {
> MEDIA_BUS_FMT_SGBRG10_1X10,
> MEDIA_BUS_FMT_SGBRG10_1X10,
> DATA_TYPE_RAW_10BIT,
> DECODE_FORMAT_UNCOMPRESSED_10_BIT,
> - 10
> + 10,
> + 1
> },
> {
> MEDIA_BUS_FMT_SGRBG10_1X10,
> MEDIA_BUS_FMT_SGRBG10_1X10,
> DATA_TYPE_RAW_10BIT,
> DECODE_FORMAT_UNCOMPRESSED_10_BIT,
> - 10
> + 10,
> + 1
> },
> {
> MEDIA_BUS_FMT_SRGGB10_1X10,
> MEDIA_BUS_FMT_SRGGB10_1X10,
> DATA_TYPE_RAW_10BIT,
> DECODE_FORMAT_UNCOMPRESSED_10_BIT,
> - 10
> + 10,
> + 1
> },
> {
> MEDIA_BUS_FMT_SBGGR12_1X12,
> MEDIA_BUS_FMT_SBGGR12_1X12,
> DATA_TYPE_RAW_12BIT,
> DECODE_FORMAT_UNCOMPRESSED_12_BIT,
> - 12
> + 12,
> + 1
> },
> {
> MEDIA_BUS_FMT_SGBRG12_1X12,
> MEDIA_BUS_FMT_SGBRG12_1X12,
> DATA_TYPE_RAW_12BIT,
> DECODE_FORMAT_UNCOMPRESSED_12_BIT,
> - 12
> + 12,
> + 1
> },
> {
> MEDIA_BUS_FMT_SGRBG12_1X12,
> MEDIA_BUS_FMT_SGRBG12_1X12,
> DATA_TYPE_RAW_12BIT,
> DECODE_FORMAT_UNCOMPRESSED_12_BIT,
> - 12
> + 12,
> + 1
> },
> {
> MEDIA_BUS_FMT_SRGGB12_1X12,
> MEDIA_BUS_FMT_SRGGB12_1X12,
> DATA_TYPE_RAW_12BIT,
> DECODE_FORMAT_UNCOMPRESSED_12_BIT,
> - 12
> + 12,
> + 1
> }
> };
>
> /*
> + * csid_get_uncompressed - map media bus format to uncompressed media bus format
> + * @code: media bus format code
> + *
> + * Return uncompressed media bus format code
> + */
> +static u32 csid_get_uncompressed(u32 code)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(csid_input_fmts); i++)
> + if (code == csid_input_fmts[i].code)
> + break;
> +
> + return csid_input_fmts[i].uncompressed;
> +}

I think my suggestion of returning an array entry (a struct) was a good
one. :-)

> +
> +/*
> + * csid_get_data_type - map media bus format to data type
> + * @code: media bus format code
> + *
> + * Return data type code
> + */
> +static u8 csid_get_data_type(u32 code)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(csid_input_fmts); i++)
> + if (code == csid_input_fmts[i].code)
> + break;
> +
> + return csid_input_fmts[i].data_type;
> +}
> +
> +/*
> + * csid_get_decode_format - map media bus format to decode format
> + * @code: media bus format code
> + *
> + * Return decode format code
> + */
> +static u8 csid_get_decode_format(u32 code)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(csid_input_fmts); i++)
> + if (code == csid_input_fmts[i].code)
> + break;
> +
> + return csid_input_fmts[i].decode_format;
> +}
> +
> +/*
> + * csid_get_bpp - map media bus format to bits per pixel
> + * @code: media bus format code
> + *
> + * Return number of bits per pixel
> + */
> +static u8 csid_get_bpp(u32 code)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(csid_input_fmts); i++)
> + if (code == csid_input_fmts[i].uncompressed)
> + break;
> +
> + return csid_input_fmts[i].uncompr_bpp;
> +}
> +
> +/*
> + * csid_get_spp - map media bus format to bus samples per pixel
> + * @code: media bus format code
> + *
> + * Return number of bus samples per pixel
> + */
> +static u8 csid_get_spp(u32 code)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(csid_input_fmts); i++)
> + if (code == csid_input_fmts[i].uncompressed)
> + break;
> +
> + return csid_input_fmts[i].spp;
> +}
> +
> +/*
> * csid_isr - CSID module interrupt handler
> * @irq: Interrupt line
> * @dev: CSID device

--
Regards,

Sakari Ailus
e-mail: sakari.ailus@xxxxxx XMPP: sailus@xxxxxxxxxxxxxx