Re: [PATCH v3 17/23] camss: vfe: Add interface for scaling
Hi Todor,
Todor Tomov wrote:
> Hi Sakari,
>
> Thank you for review.
>
> On 20.07.2017 18:20, Sakari Ailus wrote:
>> Hi Todor,
>>
>> On Mon, Jul 17, 2017 at 01:33:43PM +0300, Todor Tomov wrote:
>>> Add compose selection ioctls to handle scaling configuration.
>>>
>>> Signed-off-by: Todor Tomov
>>> ---
>>> drivers/media/platform/qcom/camss-8x16/camss-vfe.c | 189
>>> -
>>> drivers/media/platform/qcom/camss-8x16/camss-vfe.h | 1 +
>>> 2 files changed, 188 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/media/platform/qcom/camss-8x16/camss-vfe.c
>>> b/drivers/media/platform/qcom/camss-8x16/camss-vfe.c
>>> index 327f158..8ec6ce7 100644
>>> --- a/drivers/media/platform/qcom/camss-8x16/camss-vfe.c
>>> +++ b/drivers/media/platform/qcom/camss-8x16/camss-vfe.c
>>> @@ -211,6 +211,8 @@
>>> #define CAMIF_TIMEOUT_SLEEP_US 1000
>>> #define CAMIF_TIMEOUT_ALL_US 100
>>>
>>> +#define SCALER_RATIO_MAX 16
>>> +
>>> static const u32 vfe_formats[] = {
>>> MEDIA_BUS_FMT_UYVY8_2X8,
>>> MEDIA_BUS_FMT_VYUY8_2X8,
>>> @@ -1905,6 +1907,25 @@ __vfe_get_format(struct vfe_line *line,
>>> return &line->fmt[pad];
>>> }
>>>
>>> +/*
>>> + * __vfe_get_compose - Get pointer to compose selection structure
>>> + * @line: VFE line
>>> + * @cfg: V4L2 subdev pad configuration
>>> + * @which: TRY or ACTIVE format
>>> + *
>>> + * Return pointer to TRY or ACTIVE compose rectangle structure
>>> + */
>>> +static struct v4l2_rect *
>>> +__vfe_get_compose(struct vfe_line *line,
>>> + struct v4l2_subdev_pad_config *cfg,
>>> + enum v4l2_subdev_format_whence which)
>>> +{
>>> + if (which == V4L2_SUBDEV_FORMAT_TRY)
>>> + return v4l2_subdev_get_try_compose(&line->subdev, cfg,
>>> + MSM_VFE_PAD_SINK);
>>> +
>>> + return &line->compose;
>>> +}
>>>
>>> /*
>>> * vfe_try_format - Handle try format by pad subdev method
>>> @@ -1951,7 +1972,14 @@ static void vfe_try_format(struct vfe_line *line,
>>> *fmt = *__vfe_get_format(line, cfg, MSM_VFE_PAD_SINK,
>>> which);
>>>
>>> - if (line->id == VFE_LINE_PIX)
>>> + if (line->id == VFE_LINE_PIX) {
>>> + struct v4l2_rect *rect;
>>> +
>>> + rect = __vfe_get_compose(line, cfg, which);
>>> +
>>> + fmt->width = rect->width;
>>> + fmt->height = rect->height;
>>> +
>>> switch (fmt->code) {
>>> case MEDIA_BUS_FMT_YUYV8_2X8:
>>> if (code == MEDIA_BUS_FMT_YUYV8_1_5X8)
>>> @@ -1979,6 +2007,7 @@ static void vfe_try_format(struct vfe_line *line,
>>> fmt->code = MEDIA_BUS_FMT_VYUY8_2X8;
>>> break;
>>> }
>>> + }
>>>
>>> break;
>>> }
>>> @@ -1987,6 +2016,50 @@ static void vfe_try_format(struct vfe_line *line,
>>> }
>>>
>>> /*
>>> + * vfe_try_compose - Handle try compose selection by pad subdev method
>>> + * @line: VFE line
>>> + * @cfg: V4L2 subdev pad configuration
>>> + * @rect: pointer to v4l2 rect structure
>>> + * @which: wanted subdev format
>>> + */
>>> +static void vfe_try_compose(struct vfe_line *line,
>>> + struct v4l2_subdev_pad_config *cfg,
>>> + struct v4l2_rect *rect,
>>> + enum v4l2_subdev_format_whence which)
>>> +{
>>> + struct v4l2_mbus_framefmt *fmt;
>>> +
>>> + rect->width = rect->width - rect->left;
>>> + rect->left = 0;
>>
>> This is the compose rectangle i.e. left and top should be zero (unless it's
>> about composing on e.g. a frame buffer). No need to decrement from width;
>> similarly for height below.
>
> Yes, it is not composing, but does the user know that? If left and top are
> set, it makes sense to keep the rectangle size unchanged I think - actually
> decrement width and height (and then clear left and top).
The API documentation tells these values are zero. If the user specifies
non-zero values then I don't think that they should have an effect. This
behaviour is in line with other drivers AFAIK.
>
>>
>>> + rect->height = rect->height - rect->top;
>>> + rect->top = 0;
>>> +
>>> + fmt = __vfe_get_format(line, cfg, MSM_VFE_PAD_SINK, which);
>>> +
>>> + if (rect->width > fmt->width)
>>> + rect->width = fmt->width;
>>> +
>>> + if (rect->height > fmt->height)
>>> + rect->height = fmt->height;
>>> +
>>> + if (fmt->width > rect->width * SCALER_RATIO_MAX)
>>> + rect->width = (fmt->width + SCALER_RATIO_MAX - 1) /
>>> + SCALER_RATIO_MAX;
>>> +
>>> + rect->width &= ~0x1;
>>> +
>>> + if (fmt->height > rect->height * SCALER_RATIO_MAX)
>>> + rect->height = (fmt->height + SCALER_RATIO_MAX - 1) /
>>> + SCALER_RATIO_MAX;
>>>
Re: [PATCH v3 17/23] camss: vfe: Add interface for scaling
Hi Sakari,
Thank you for review.
On 20.07.2017 18:20, Sakari Ailus wrote:
> Hi Todor,
>
> On Mon, Jul 17, 2017 at 01:33:43PM +0300, Todor Tomov wrote:
>> Add compose selection ioctls to handle scaling configuration.
>>
>> Signed-off-by: Todor Tomov
>> ---
>> drivers/media/platform/qcom/camss-8x16/camss-vfe.c | 189
>> -
>> drivers/media/platform/qcom/camss-8x16/camss-vfe.h | 1 +
>> 2 files changed, 188 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/camss-8x16/camss-vfe.c
>> b/drivers/media/platform/qcom/camss-8x16/camss-vfe.c
>> index 327f158..8ec6ce7 100644
>> --- a/drivers/media/platform/qcom/camss-8x16/camss-vfe.c
>> +++ b/drivers/media/platform/qcom/camss-8x16/camss-vfe.c
>> @@ -211,6 +211,8 @@
>> #define CAMIF_TIMEOUT_SLEEP_US 1000
>> #define CAMIF_TIMEOUT_ALL_US 100
>>
>> +#define SCALER_RATIO_MAX 16
>> +
>> static const u32 vfe_formats[] = {
>> MEDIA_BUS_FMT_UYVY8_2X8,
>> MEDIA_BUS_FMT_VYUY8_2X8,
>> @@ -1905,6 +1907,25 @@ __vfe_get_format(struct vfe_line *line,
>> return &line->fmt[pad];
>> }
>>
>> +/*
>> + * __vfe_get_compose - Get pointer to compose selection structure
>> + * @line: VFE line
>> + * @cfg: V4L2 subdev pad configuration
>> + * @which: TRY or ACTIVE format
>> + *
>> + * Return pointer to TRY or ACTIVE compose rectangle structure
>> + */
>> +static struct v4l2_rect *
>> +__vfe_get_compose(struct vfe_line *line,
>> + struct v4l2_subdev_pad_config *cfg,
>> + enum v4l2_subdev_format_whence which)
>> +{
>> +if (which == V4L2_SUBDEV_FORMAT_TRY)
>> +return v4l2_subdev_get_try_compose(&line->subdev, cfg,
>> + MSM_VFE_PAD_SINK);
>> +
>> +return &line->compose;
>> +}
>>
>> /*
>> * vfe_try_format - Handle try format by pad subdev method
>> @@ -1951,7 +1972,14 @@ static void vfe_try_format(struct vfe_line *line,
>> *fmt = *__vfe_get_format(line, cfg, MSM_VFE_PAD_SINK,
>> which);
>>
>> -if (line->id == VFE_LINE_PIX)
>> +if (line->id == VFE_LINE_PIX) {
>> +struct v4l2_rect *rect;
>> +
>> +rect = __vfe_get_compose(line, cfg, which);
>> +
>> +fmt->width = rect->width;
>> +fmt->height = rect->height;
>> +
>> switch (fmt->code) {
>> case MEDIA_BUS_FMT_YUYV8_2X8:
>> if (code == MEDIA_BUS_FMT_YUYV8_1_5X8)
>> @@ -1979,6 +2007,7 @@ static void vfe_try_format(struct vfe_line *line,
>> fmt->code = MEDIA_BUS_FMT_VYUY8_2X8;
>> break;
>> }
>> +}
>>
>> break;
>> }
>> @@ -1987,6 +2016,50 @@ static void vfe_try_format(struct vfe_line *line,
>> }
>>
>> /*
>> + * vfe_try_compose - Handle try compose selection by pad subdev method
>> + * @line: VFE line
>> + * @cfg: V4L2 subdev pad configuration
>> + * @rect: pointer to v4l2 rect structure
>> + * @which: wanted subdev format
>> + */
>> +static void vfe_try_compose(struct vfe_line *line,
>> +struct v4l2_subdev_pad_config *cfg,
>> +struct v4l2_rect *rect,
>> +enum v4l2_subdev_format_whence which)
>> +{
>> +struct v4l2_mbus_framefmt *fmt;
>> +
>> +rect->width = rect->width - rect->left;
>> +rect->left = 0;
>
> This is the compose rectangle i.e. left and top should be zero (unless it's
> about composing on e.g. a frame buffer). No need to decrement from width;
> similarly for height below.
Yes, it is not composing, but does the user know that? If left and top are
set, it makes sense to keep the rectangle size unchanged I think - actually
decrement width and height (and then clear left and top).
>
>> +rect->height = rect->height - rect->top;
>> +rect->top = 0;
>> +
>> +fmt = __vfe_get_format(line, cfg, MSM_VFE_PAD_SINK, which);
>> +
>> +if (rect->width > fmt->width)
>> +rect->width = fmt->width;
>> +
>> +if (rect->height > fmt->height)
>> +rect->height = fmt->height;
>> +
>> +if (fmt->width > rect->width * SCALER_RATIO_MAX)
>> +rect->width = (fmt->width + SCALER_RATIO_MAX - 1) /
>> +SCALER_RATIO_MAX;
>> +
>> +rect->width &= ~0x1;
>> +
>> +if (fmt->height > rect->height * SCALER_RATIO_MAX)
>> +rect->height = (fmt->height + SCALER_RATIO_MAX - 1) /
>> +SCALER_RATIO_MAX;
>> +
>> +if (rect->width < 16)
>> +rect->width = 16;
>> +
>> +if (rect->height < 4)
>> +rect->height = 4;
>> +}
>> +
--
Best regards,
Todor Tomov
Re: [PATCH v3 17/23] camss: vfe: Add interface for scaling
Hi Todor,
On Mon, Jul 17, 2017 at 01:33:43PM +0300, Todor Tomov wrote:
> Add compose selection ioctls to handle scaling configuration.
>
> Signed-off-by: Todor Tomov
> ---
> drivers/media/platform/qcom/camss-8x16/camss-vfe.c | 189
> -
> drivers/media/platform/qcom/camss-8x16/camss-vfe.h | 1 +
> 2 files changed, 188 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss-8x16/camss-vfe.c
> b/drivers/media/platform/qcom/camss-8x16/camss-vfe.c
> index 327f158..8ec6ce7 100644
> --- a/drivers/media/platform/qcom/camss-8x16/camss-vfe.c
> +++ b/drivers/media/platform/qcom/camss-8x16/camss-vfe.c
> @@ -211,6 +211,8 @@
> #define CAMIF_TIMEOUT_SLEEP_US 1000
> #define CAMIF_TIMEOUT_ALL_US 100
>
> +#define SCALER_RATIO_MAX 16
> +
> static const u32 vfe_formats[] = {
> MEDIA_BUS_FMT_UYVY8_2X8,
> MEDIA_BUS_FMT_VYUY8_2X8,
> @@ -1905,6 +1907,25 @@ __vfe_get_format(struct vfe_line *line,
> return &line->fmt[pad];
> }
>
> +/*
> + * __vfe_get_compose - Get pointer to compose selection structure
> + * @line: VFE line
> + * @cfg: V4L2 subdev pad configuration
> + * @which: TRY or ACTIVE format
> + *
> + * Return pointer to TRY or ACTIVE compose rectangle structure
> + */
> +static struct v4l2_rect *
> +__vfe_get_compose(struct vfe_line *line,
> + struct v4l2_subdev_pad_config *cfg,
> + enum v4l2_subdev_format_whence which)
> +{
> + if (which == V4L2_SUBDEV_FORMAT_TRY)
> + return v4l2_subdev_get_try_compose(&line->subdev, cfg,
> +MSM_VFE_PAD_SINK);
> +
> + return &line->compose;
> +}
>
> /*
> * vfe_try_format - Handle try format by pad subdev method
> @@ -1951,7 +1972,14 @@ static void vfe_try_format(struct vfe_line *line,
> *fmt = *__vfe_get_format(line, cfg, MSM_VFE_PAD_SINK,
>which);
>
> - if (line->id == VFE_LINE_PIX)
> + if (line->id == VFE_LINE_PIX) {
> + struct v4l2_rect *rect;
> +
> + rect = __vfe_get_compose(line, cfg, which);
> +
> + fmt->width = rect->width;
> + fmt->height = rect->height;
> +
> switch (fmt->code) {
> case MEDIA_BUS_FMT_YUYV8_2X8:
> if (code == MEDIA_BUS_FMT_YUYV8_1_5X8)
> @@ -1979,6 +2007,7 @@ static void vfe_try_format(struct vfe_line *line,
> fmt->code = MEDIA_BUS_FMT_VYUY8_2X8;
> break;
> }
> + }
>
> break;
> }
> @@ -1987,6 +2016,50 @@ static void vfe_try_format(struct vfe_line *line,
> }
>
> /*
> + * vfe_try_compose - Handle try compose selection by pad subdev method
> + * @line: VFE line
> + * @cfg: V4L2 subdev pad configuration
> + * @rect: pointer to v4l2 rect structure
> + * @which: wanted subdev format
> + */
> +static void vfe_try_compose(struct vfe_line *line,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_rect *rect,
> + enum v4l2_subdev_format_whence which)
> +{
> + struct v4l2_mbus_framefmt *fmt;
> +
> + rect->width = rect->width - rect->left;
> + rect->left = 0;
This is the compose rectangle i.e. left and top should be zero (unless it's
about composing on e.g. a frame buffer). No need to decrement from width;
similarly for height below.
> + rect->height = rect->height - rect->top;
> + rect->top = 0;
> +
> + fmt = __vfe_get_format(line, cfg, MSM_VFE_PAD_SINK, which);
> +
> + if (rect->width > fmt->width)
> + rect->width = fmt->width;
> +
> + if (rect->height > fmt->height)
> + rect->height = fmt->height;
> +
> + if (fmt->width > rect->width * SCALER_RATIO_MAX)
> + rect->width = (fmt->width + SCALER_RATIO_MAX - 1) /
> + SCALER_RATIO_MAX;
> +
> + rect->width &= ~0x1;
> +
> + if (fmt->height > rect->height * SCALER_RATIO_MAX)
> + rect->height = (fmt->height + SCALER_RATIO_MAX - 1) /
> + SCALER_RATIO_MAX;
> +
> + if (rect->width < 16)
> + rect->width = 16;
> +
> + if (rect->height < 4)
> + rect->height = 4;
> +}
> +
> +/*
> * vfe_enum_mbus_code - Handle pixel format enumeration
> * @sd: VFE V4L2 subdevice
> * @cfg: V4L2 subdev pad configuration
> @@ -2081,6 +2154,10 @@ static int vfe_get_format(struct v4l2_subdev *sd,
> return 0;
> }
>
> +static int vfe_set_selection(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_selection *sel);
> +
> /*
> * vfe_set_format - Handle set format by pads subdev method
> * @sd: VFE V4L2 subdevice
> @@ -2103,20
