| Message ID | 20210510142915.941460-1-l.stach@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/imx: ipuv3-plane: use drm managed resources | expand |
Hi Lucas, On Mon, 2021-05-10 at 16:29 +0200, Lucas Stach wrote: > The conversion to drm managed resources introduced two bugs: the plane is now > always initialized with the linear-only list, while the list with the Vivante > GPU modifiers should have been used when the PRG/PRE engines are present. This > masked another issue, as ipu_plane_format_mod_supported() is now called before > the private plane data is set up, so if a non-linear modifier is supplied in > the plane modifier list, we run into a NULL pointer dereference checking for > the PRG presence. To fix this just remove the check from this function, as we > know that it will only be called with a non-linear modifier, if the plane init > code has already determined that the PRG/PRE is present. > > Fixes: 699e7e543f1a ("drm/imx: ipuv3-plane: use drm managed resources") > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> Thank you, I've rebased and applied this patch on top of imx-drm/next. regards Philipp
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c index fa5009705365..8b6c137bf0fc 100644 --- a/drivers/gpu/drm/imx/ipuv3-plane.c +++ b/drivers/gpu/drm/imx/ipuv3-plane.c @@ -320,10 +320,11 @@ static bool ipu_plane_format_mod_supported(struct drm_plane *plane, if (modifier == DRM_FORMAT_MOD_LINEAR) return true; - /* without a PRG there are no supported modifiers */ - if (!ipu_prg_present(ipu)) - return false; - + /* + * Without a PRG the possible modifiers list only includes the linear + * modifier, so we always take the early return from this function and + * only end up here if the PRG is present. + */ return ipu_prg_format_supported(ipu, format, modifier); } @@ -835,6 +836,9 @@ struct ipu_plane *ipu_plane_init(struct drm_device *dev, struct ipu_soc *ipu, DRM_DEBUG_KMS("channel %d, dp flow %d, possible_crtcs=0x%x\n", dma, dp, possible_crtcs); + if (ipu_prg_present(ipu)) + modifiers = pre_format_modifiers; + ipu_plane = drmm_universal_plane_alloc(dev, struct ipu_plane, base, possible_crtcs, &ipu_plane_funcs, ipu_plane_formats, @@ -850,9 +854,6 @@ struct ipu_plane *ipu_plane_init(struct drm_device *dev, struct ipu_soc *ipu, ipu_plane->dma = dma; ipu_plane->dp_flow = dp; - if (ipu_prg_present(ipu)) - modifiers = pre_format_modifiers; - drm_plane_helper_add(&ipu_plane->base, &ipu_plane_helper_funcs); if (dp == IPU_DP_FLOW_SYNC_BG || dp == IPU_DP_FLOW_SYNC_FG)
The conversion to drm managed resources introduced two bugs: the plane is now always initialized with the linear-only list, while the list with the Vivante GPU modifiers should have been used when the PRG/PRE engines are present. This masked another issue, as ipu_plane_format_mod_supported() is now called before the private plane data is set up, so if a non-linear modifier is supplied in the plane modifier list, we run into a NULL pointer dereference checking for the PRG presence. To fix this just remove the check from this function, as we know that it will only be called with a non-linear modifier, if the plane init code has already determined that the PRG/PRE is present. Fixes: 699e7e543f1a ("drm/imx: ipuv3-plane: use drm managed resources") Signed-off-by: Lucas Stach <l.stach@pengutronix.de> --- drivers/gpu/drm/imx/ipuv3-plane.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)