On Fri, Nov 06, 2015 at 11:05:48AM +0100, Philipp Zabel wrote: > Am Mittwoch, den 04.11.2015, 18:15 +0800 schrieb Liu Ying: > > Since we are using ipu_plane_init() to add one primary plane for each > > IPU CRTC, it's unnecessary to create the safe one by using the helper > > create_primary_plane(). > > > > Furthermore, the safe one is attached to crtc->primary, which actually > > carries a framebuffer(crtc->primary->fb) created by the fbdev helper to > > build up the fbcon. Instead, the one created by ipu_plane_init() is > > dangling, but it is the one actually does ipu_plane_mode_set() for the > > fbcon. This may causes the mismatch bewteen ipu_plane->enabled(true) and > > ipu_plane->base.fb(NULL). Thus, it brings a NULL pointer dereference > > issue in ipu_plane_mode_set() when we try to additionally touch the > > IDMAC channel of the ipu_plane. This issue could be reproduced by > > running the drm modetest with command line 'modetest -P 19:1024x768@XR24' > > on the i.MX6Q SabreSD platform(single LVDS display). This patch binds > > the plane created by ipu_plane_init() with crtc->primary and removes the > > safe one to address this issue. > > > > Signed-off-by: Liu Ying <Ying.Liu@xxxxxxxxxxxxx> > > --- > > drivers/gpu/drm/imx/imx-drm-core.c | 3 ++- > > drivers/gpu/drm/imx/ipuv3-crtc.c | 6 ++++++ > > 2 files changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c > > index 6faa735..08eceeb 100644 > > --- a/drivers/gpu/drm/imx/imx-drm-core.c > > +++ b/drivers/gpu/drm/imx/imx-drm-core.c > > @@ -373,7 +373,8 @@ int imx_drm_add_crtc(struct drm_device *drm, struct drm_crtc *crtc, > > drm_crtc_helper_add(crtc, > > imx_drm_crtc->imx_drm_helper_funcs.crtc_helper_funcs); > > > > - drm_crtc_init(drm, crtc, > > + /* The related primary plane will be created in ipu_plane_init(). */ > > + drm_crtc_init_with_planes(drm, crtc, NULL, NULL, > > imx_drm_crtc->imx_drm_helper_funcs.crtc_funcs); > > We rather should generate the plane first and add the primary_plane > parameter to imx_drm_add_crtc than calling drm_crtc_init_with_planes > without planes. Yes, that is the regular way. I thought about that, though I took the easy approach here. The question is that we currently generate the crtc pipe number in imx_drm_add_crtc() first, and then pass it to ipu_plane_init(). Do you think it would be good to add OF alias id for IPU, pass the id to ipu_crtc_init() via struct ipu_soc and determine the pipe number in ipu_crtc_init() by using the IPU id and pdata->di? This way, ipu_plane_init() may know the pipe number in the first place. Or, any other suggestions? Regards, Liu Ying > > regards > Philipp > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
