Hi Jacopo,
Thanks for your work.
On 2018-05-11 11:59:40 +0200, Jacopo Mondi wrote:
> The crop_scale routine uses the crop rectangle memebers to configure the
> VIN clipping rectangle. When crop is not configured all its fields are
> 0s, and setting the clipping rectangle vertical and horizontal extensions
> to (0 - 1) causes the registers to be written with an incorrect
> 0xffffffff value.
This is an interesting find and a clear bug in my code. But I can't see
how crop ever could be 0. When s_fmt is called it always resets the crop
and compose width's to the requested format size.
I'm curious how you found this bug, I tried to reproduce it but could
not. This is indeed something we should fix! But I think the proper fix
is not allowing crop to be 0 and not treating the symptom in
rvin_crop_scale_comp().
>
> Fix this by using the actual format width and height when no crop
> rectangle has been programmed.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx>
> ---
> drivers/media/platform/rcar-vin/rcar-dma.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> index b41ba9a..ea7a120 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -579,22 +579,25 @@ static void rvin_crop_scale_comp_gen2(struct rvin_dev *vin)
>
> void rvin_crop_scale_comp(struct rvin_dev *vin)
> {
> - /* Set Start/End Pixel/Line Pre-Clip */
> + u32 width = vin->crop.width ? vin->crop.left + vin->crop.width :
> + vin->format.width;
> + u32 height = vin->crop.height ? vin->crop.top + vin->crop.height :
> + vin->format.height;
> +
> + /* Set Start/End Pixel/Line Pre-Clip if crop is configured. */
> rvin_write(vin, vin->crop.left, VNSPPRC_REG);
> - rvin_write(vin, vin->crop.left + vin->crop.width - 1, VNEPPRC_REG);
> + rvin_write(vin, width - 1, VNEPPRC_REG);
>
> switch (vin->format.field) {
> case V4L2_FIELD_INTERLACED:
> case V4L2_FIELD_INTERLACED_TB:
> case V4L2_FIELD_INTERLACED_BT:
> rvin_write(vin, vin->crop.top / 2, VNSLPRC_REG);
> - rvin_write(vin, (vin->crop.top + vin->crop.height) / 2 - 1,
> - VNELPRC_REG);
> + rvin_write(vin, height / 2 - 1, VNELPRC_REG);
> break;
> default:
> rvin_write(vin, vin->crop.top, VNSLPRC_REG);
> - rvin_write(vin, vin->crop.top + vin->crop.height - 1,
> - VNELPRC_REG);
> + rvin_write(vin, height - 1, VNELPRC_REG);
> break;
> }
>
> --
> 2.7.4
>
--
Regards,
Niklas Söderlund
