| Message ID | 1476082208-14480-3-git-send-email-gnuiyl@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@vger.kernel.org # 4.8 > Signed-off-by: Liu Ying <gnuiyl@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. 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? regards Philipp
Hi Philipp, 2016-10-18 0:10 GMT+08:00 Philipp Zabel <p.zabel@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@vger.kernel.org # 4.8 >> Signed-off-by: Liu Ying <gnuiyl@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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Am Dienstag, den 18.10.2016, 11:39 +0800 schrieb Liu Ying: [...] > >> @@ -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. Alright, the change you suggest above should work then. regards Philipp
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)
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@vger.kernel.org # 4.8 Signed-off-by: Liu Ying <gnuiyl@gmail.com> --- drivers/gpu/drm/imx/ipuv3-plane.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)