Re: [PATCH v2 4/5] media: staging: rkisp1: cap: support uv swapped plane formats

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 4/6/20 8:56 AM, Dafna Hirschfeld wrote:
> 
> 
> On 4/5/20 8:16 PM, Laurent Pinchart wrote:
>> Hi Dafna,
>>
>> Thank you for the patch.
>>
>> On Thu, Apr 02, 2020 at 09:04:18PM +0200, Dafna Hirschfeld wrote:
>>> Plane formats with the u and v planes swapped can be
>>> supported by changing the address of the cb and cr in
>>> the buffer.
>>>
>>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@xxxxxxxxxxxxx>
>>> Acked-by: Helen Koike <helen.koike@xxxxxxxxxxxxx>
>>> ---
>>>   drivers/staging/media/rkisp1/rkisp1-capture.c | 17 +++++++++++++++++
>>>   1 file changed, 17 insertions(+)
>>>
>>> diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c
>>> index fa2849209433..2d274e8f565b 100644
>>> --- a/drivers/staging/media/rkisp1/rkisp1-capture.c
>>> +++ b/drivers/staging/media/rkisp1/rkisp1-capture.c
>>> @@ -41,6 +41,10 @@
>>>       (((write_format) == RKISP1_MI_CTRL_MP_WRITE_YUV_SPLA) ||    \
>>>        ((write_format) == RKISP1_MI_CTRL_SP_WRITE_SPLA))
>>>   +#define RKISP1_IS_PLANAR(write_format)                    \
>>> +    (((write_format) == RKISP1_MI_CTRL_SP_WRITE_PLA) ||        \
>>> +     ((write_format) == RKISP1_MI_CTRL_MP_WRITE_YUV_PLA_OR_RAW8))
>>> +
>>>   enum rkisp1_plane {
>>>       RKISP1_PLANE_Y    = 0,
>>>       RKISP1_PLANE_CB    = 1,
>>> @@ -788,6 +792,19 @@ static void rkisp1_vb2_buf_queue(struct vb2_buffer *vb)
>>>               rkisp1_pixfmt_comp_size(pixm, RKISP1_PLANE_CB);
>>>       }
>>>   +    /*
>>> +     * uv swap can be supported for plane formats by switching
>>> +     * the address of cb and cr
>>> +     */
>>> +    if (RKISP1_IS_PLANAR(cap->pix.cfg->write_format) &&
>>
>> As commented on patch 3/5, could this be checked from the data in
>> v4l2_format_info ?
> yes
>>
>>> +        cap->pix.cfg->uv_swap) {
>>> +        ispbuf->buff_addr[RKISP1_PLANE_CR] =
>>> +            ispbuf->buff_addr[RKISP1_PLANE_CB];
>>> +        ispbuf->buff_addr[RKISP1_PLANE_CB] =
>>> +            ispbuf->buff_addr[RKISP1_PLANE_CR] +
>>> +            rkisp1_pixfmt_comp_size(pixm, RKISP1_PLANE_CR);

Actually this is wrong if pixm->num_planes != 1, since they are different buffers.

>>
>> How about
>>
>>         swap(ispbuf->buff_addr[RKISP1_PLANE_CR],
>>              ispbuf->buff_addr[RKISP1_PLANE_CB]);
>>
>> ?
> This also works, theoretically if there was a format where the Cb, Cr planes
> are not equal size then a swap will not work.

If you check rkisp1_fill_pixfmt(), you'll see that they are equal size.
hdiv and vdiv applies to both.

Thank you Laurent for the review and thank you Dafna for working on this.

Regards,
Helen

> 
> Thanks,
> Dafna
>>
>>> +    }
>>> +
>>>       spin_lock_irqsave(&cap->buf.lock, flags);
>>>         /*
>>
> 



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux