[PATCH 3/3] drm/imx: ipuv3-plane: Access old u/vbo properly in ->atomic_check for YU12/YV12

[Liu Ying]

Hi Philipp,

2016-10-18 0:10 GMT+08:00 Philipp Zabel <p.zabel at pengutronix.de>:
> Hi Liu,
>
> Am Montag, den 10.10.2016, 14:50 +0800 schrieb Liu Ying:
>> Before accessing the u/v offset(aka, u/vbo for IPUv3) of the old plane state's
>> relevant fb, we should make sure the fb is in YU12 or YV12 pixel format(which
>> are the two YUV pixel formats we support only), otherwise, we are likely to
>> trigger BUG_ON() in drm_plane_state_to_u/vbo() since the fb's pixel format is
>> probably not YU12 or YV12.
>>
>> Link: https://bugs.freedesktop.org/show_bug.cgi?id=98150
>> Fixes: c6c1f9bc798b ("drm/imx: Add active plane reconfiguration support")
>> Cc: stable at vger.kernel.org # 4.8
>> Signed-off-by: Liu Ying <gnuiyl at gmail.com>
>> ---
>>  drivers/gpu/drm/imx/ipuv3-plane.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
>> index e33110e..a691892 100644
>> --- a/drivers/gpu/drm/imx/ipuv3-plane.c
>> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
>> @@ -359,7 +359,7 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
>>               if ((ubo > 0xfffff8) || (vbo > 0xfffff8))
>>                       return -EINVAL;
>>
>> -             if (old_fb) {
>> +             if (old_fb && old_fb->pixel_format == fb->pixel_format) {
>>                       old_ubo = drm_plane_state_to_ubo(old_state);
>>                       old_vbo = drm_plane_state_to_vbo(old_state);
>>                       if (ubo != old_ubo || vbo != old_vbo)
>
> thank you for the patches. I have applied patches 1 and 2, but with this
> change UBO/VBO changes are ignored when switching from YU12 to YV12.

Good catch.  Does this change look okay, then?

-               if (old_fb) {
+               if (old_fb &&
+                   (old_fb->pixel_format == DRM_FORMAT_YUV420 ||
+                    old_fb->pixel_format == DRM_FORMAT_YVU420)) {

>
> Shouldn't we rather set crtc_state->mode_changed = true if either
> (fb->pixel_format != old_fb->pixel_format) for any pixel format or
> (ubo != old_ubo || vbo != old_vbo) for YUV formats, instead of returning
> -EINVAL?

I thought about this and determined that this could be done
in an additional patch later.
It would be good if we change as little as we can in a patch
with stable Cced.

Regards,
Liu Ying

>
> regards
> Philipp
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Best Regards,
Liu Ying