From patchwork Fri Oct 31 22:54:33 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steve Longerbeam X-Patchwork-Id: 5207911 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 170E79F318 for ; Fri, 31 Oct 2014 23:57:46 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id E611A20176 for ; Fri, 31 Oct 2014 23:57:44 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 874BB20120 for ; Fri, 31 Oct 2014 23:57:43 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 0AC456E866; Fri, 31 Oct 2014 16:57:00 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-pd0-f169.google.com (mail-pd0-f169.google.com [209.85.192.169]) by gabe.freedesktop.org (Postfix) with ESMTP id 740026E84E for ; Fri, 31 Oct 2014 15:57:29 -0700 (PDT) Received: by mail-pd0-f169.google.com with SMTP id y10so8159049pdj.0 for ; Fri, 31 Oct 2014 15:57:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=dqoOiNBkTvupqQNS8hERbuWcjleTs8e6sk5q/k5O3hk=; b=bGXBrTEBg/k2LWtmwQ1wsWw3ACAW8g3AYrq9e6o6Jn67SdwkxapkQIEnuH1iyhK6TX KOPPsgxoAz5W2e0aHzx+95PxhO9HLUjm6HrAN09CdWRSRoFBA+FbO1YxiNkyp1hiRiYa PF42vdsB/McqSkOKlVmXB1r0FFT2tswkZEc6vGbE7a/GQ7K3QVCE2hPkMHjSiW/EEkSJ 8qNo4ygETNLMuEnl8h099KLfbWCQN+35Ro8JAuzwhKhFvYAtO2yh+/+i1FsdXiBvz7aF dUmya30p/wPCA1hW/yA1s5kU3wnzHiAARTmQbTlLWD7OWc8Zmk1tg1/Op4gWe6uOIV/s ToeA== X-Received: by 10.68.226.226 with SMTP id rv2mr27795138pbc.77.1414796249341; Fri, 31 Oct 2014 15:57:29 -0700 (PDT) Received: from mothership.mgc.mentorg.com (c-50-152-159-227.hsd1.ca.comcast.net. [50.152.159.227]) by mx.google.com with ESMTPSA id ev8sm10870656pdb.28.2014.10.31.15.57.28 for (version=TLSv1.1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 31 Oct 2014 15:57:28 -0700 (PDT) From: Steve Longerbeam X-Google-Original-From: Steve Longerbeam To: dri-devel@lists.freedesktop.org Subject: [PATCH 50/72] imx-drm: Fix separate primary plane objects Date: Fri, 31 Oct 2014 15:54:33 -0700 Message-Id: <1414796095-10107-51-git-send-email-steve_longerbeam@mentor.com> X-Mailer: git-send-email 1.7.9.5 In-Reply-To: <1414796095-10107-1-git-send-email-steve_longerbeam@mentor.com> References: <1414796095-10107-1-git-send-email-steve_longerbeam@mentor.com> X-Mailman-Approved-At: Fri, 31 Oct 2014 16:56:32 -0700 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Spam-Status: No, score=-4.7 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP drm_crtc_init() will create a primary plane object, while imx-drm also creates its own, thus two primary planes are separately created, and can cause difficult bugs to track down in the future. Fix by using drm_crtc_init_with_planes() instead of drm_crtc_init(), so that we can hand drm our own primary plane object, thus crtc->primary and ipu_crtc->plane[0].base now are the same object. To do this we need to statically declare the primary and overlay planes in the crtc driver, to avoid a chicken-before-the-egg problem, the primary plane must exist when imx_drm_add_crtc() is called but before ipu_plane_init(). Signed-off-by: Steve Longerbeam --- drivers/staging/imx-drm/imx-drm-core.c | 3 +- drivers/staging/imx-drm/imx-drm.h | 1 + drivers/staging/imx-drm/ipuv3-crtc.c | 54 ++++++++++++++++++-------------- drivers/staging/imx-drm/ipuv3-plane.c | 32 +++++-------------- drivers/staging/imx-drm/ipuv3-plane.h | 6 ++-- 5 files changed, 45 insertions(+), 51 deletions(-) diff --git a/drivers/staging/imx-drm/imx-drm-core.c b/drivers/staging/imx-drm/imx-drm-core.c index e5ec010..59200ff 100644 --- a/drivers/staging/imx-drm/imx-drm-core.c +++ b/drivers/staging/imx-drm/imx-drm-core.c @@ -342,6 +342,7 @@ err_kms: * imx_drm_add_crtc - add a new crtc */ int imx_drm_add_crtc(struct drm_device *drm, struct drm_crtc *crtc, + struct drm_plane *primary, struct imx_drm_crtc **new_crtc, const struct imx_drm_crtc_helper_funcs *imx_drm_helper_funcs, struct device_node *port) @@ -380,7 +381,7 @@ 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, + drm_crtc_init_with_planes(drm, crtc, primary, NULL, imx_drm_crtc->imx_drm_helper_funcs.crtc_funcs); return 0; diff --git a/drivers/staging/imx-drm/imx-drm.h b/drivers/staging/imx-drm/imx-drm.h index 7453ae0..3a30f16 100644 --- a/drivers/staging/imx-drm/imx-drm.h +++ b/drivers/staging/imx-drm/imx-drm.h @@ -24,6 +24,7 @@ struct imx_drm_crtc_helper_funcs { }; int imx_drm_add_crtc(struct drm_device *drm, struct drm_crtc *crtc, + struct drm_plane *primary, struct imx_drm_crtc **new_crtc, const struct imx_drm_crtc_helper_funcs *imx_helper_funcs, struct device_node *port); diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c b/drivers/staging/imx-drm/ipuv3-crtc.c index 5a60017..593261f 100644 --- a/drivers/staging/imx-drm/ipuv3-crtc.c +++ b/drivers/staging/imx-drm/ipuv3-crtc.c @@ -84,7 +84,8 @@ struct ipu_crtc { const struct ipu_channels *ch; /* plane[0] is the full plane, plane[1] is the partial plane */ - struct ipu_plane *plane[2]; + struct ipu_plane plane[2]; + bool have_overlay; /* we have a partial plane */ struct ipu_dc *dc; struct ipu_di *di; @@ -106,7 +107,7 @@ static void ipu_fb_enable(struct ipu_crtc *ipu_crtc) return; ipu_dc_enable(ipu_crtc->dc); - ipu_plane_enable(ipu_crtc->plane[0]); + ipu_plane_enable(&ipu_crtc->plane[0]); /* Start DC channel and DI after IDMAC */ ipu_dc_enable_channel(ipu_crtc->dc); ipu_di_enable(ipu_crtc->di); @@ -123,7 +124,7 @@ static void ipu_fb_disable(struct ipu_crtc *ipu_crtc) /* Stop DC channel and DI before IDMAC */ ipu_dc_disable_channel(ipu_crtc->dc); ipu_di_disable(ipu_crtc->di); - ipu_plane_disable(ipu_crtc->plane[0]); + ipu_plane_disable(&ipu_crtc->plane[0]); ipu_dc_disable(ipu_crtc->dc); ipu_di_disable_clock(ipu_crtc->di); @@ -241,7 +242,7 @@ static int ipu_crtc_mode_set(struct drm_crtc *crtc, return ret; } - return ipu_plane_mode_set(ipu_crtc->plane[0], crtc, mode, + return ipu_plane_mode_set(&ipu_crtc->plane[0], crtc, mode, crtc->primary->fb, 0, 0, mode->hdisplay, mode->vdisplay, x, y, mode->hdisplay, mode->vdisplay); @@ -267,7 +268,7 @@ static irqreturn_t ipu_irq_handler(int irq, void *dev_id) imx_drm_handle_vblank(ipu_crtc->imx_crtc); if (ipu_crtc->newfb) { - struct ipu_plane *plane = ipu_crtc->plane[0]; + struct ipu_plane *plane = &ipu_crtc->plane[0]; ipu_crtc->newfb = NULL; ipu_plane_set_base(plane, ipu_crtc->base.primary->fb, @@ -474,20 +475,27 @@ static int ipu_crtc_init(struct ipu_crtc *ipu_crtc, return ret; } - ret = imx_drm_add_crtc(drm, &ipu_crtc->base, &ipu_crtc->imx_crtc, - &ipu_crtc_helper_funcs, ipu_crtc->port); + ret = imx_drm_add_crtc(drm, &ipu_crtc->base, &ipu_crtc->plane[0].base, + &ipu_crtc->imx_crtc, &ipu_crtc_helper_funcs, + ipu_crtc->port); if (ret) { dev_err(ipu_crtc->dev, "adding crtc failed with %d.\n", ret); goto err_put_resources; } id = imx_drm_crtc_id(ipu_crtc->imx_crtc); - ipu_crtc->plane[0] = ipu_plane_init(ipu_crtc->base.dev, - ipu_crtc->ipu, - ipu_crtc->ch->dma[0], - ipu_crtc->ch->dp[0], - BIT(id), true); - ret = ipu_plane_get_resources(ipu_crtc->plane[0]); + ret = ipu_plane_init(&ipu_crtc->plane[0], drm, + ipu_crtc->ipu, + ipu_crtc->ch->dma[0], + ipu_crtc->ch->dp[0], + BIT(id), true); + if (ret) { + dev_err(ipu_crtc->dev, "init primary plane failed with %d\n", + ret); + goto err_remove_crtc; + } + + ret = ipu_plane_get_resources(&ipu_crtc->plane[0]); if (ret) { dev_err(ipu_crtc->dev, "getting plane 0 resources failed with %d.\n", ret); @@ -496,16 +504,16 @@ static int ipu_crtc_init(struct ipu_crtc *ipu_crtc, /* If this crtc is using the DP, add an overlay plane */ if (ipu_crtc->ch->dp[1] >= 0) { - ipu_crtc->plane[1] = ipu_plane_init(ipu_crtc->base.dev, - ipu_crtc->ipu, - ipu_crtc->ch->dma[1], - ipu_crtc->ch->dp[1], - BIT(id), false); - if (IS_ERR(ipu_crtc->plane[1])) - ipu_crtc->plane[1] = NULL; + ret = ipu_plane_init(&ipu_crtc->plane[1], drm, + ipu_crtc->ipu, + ipu_crtc->ch->dma[1], + ipu_crtc->ch->dp[1], + BIT(id), false); + + ipu_crtc->have_overlay = ret ? false : true; } - ipu_crtc->irq = ipu_plane_irq(ipu_crtc->plane[0]); + ipu_crtc->irq = ipu_plane_irq(&ipu_crtc->plane[0]); ret = devm_request_irq(ipu_crtc->dev, ipu_crtc->irq, ipu_irq_handler, 0, "imx_drm", ipu_crtc); if (ret < 0) { @@ -516,7 +524,7 @@ static int ipu_crtc_init(struct ipu_crtc *ipu_crtc, return 0; err_put_plane_res: - ipu_plane_put_resources(ipu_crtc->plane[0]); + ipu_plane_put_resources(&ipu_crtc->plane[0]); err_remove_crtc: imx_drm_remove_crtc(ipu_crtc->imx_crtc); err_put_resources: @@ -553,7 +561,7 @@ static void ipu_drm_unbind(struct device *dev, struct device *master, imx_drm_remove_crtc(ipu_crtc->imx_crtc); - ipu_plane_put_resources(ipu_crtc->plane[0]); + ipu_plane_put_resources(&ipu_crtc->plane[0]); ipu_put_resources(ipu_crtc); } diff --git a/drivers/staging/imx-drm/ipuv3-plane.c b/drivers/staging/imx-drm/ipuv3-plane.c index 1572c80..51a257a 100644 --- a/drivers/staging/imx-drm/ipuv3-plane.c +++ b/drivers/staging/imx-drm/ipuv3-plane.c @@ -338,13 +338,10 @@ static int ipu_disable_plane(struct drm_plane *plane) static void ipu_plane_destroy(struct drm_plane *plane) { - struct ipu_plane *ipu_plane = to_ipu_plane(plane); - DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__); ipu_disable_plane(plane); drm_plane_cleanup(plane); - kfree(ipu_plane); } static int ipu_plane_set_global_alpha(struct ipu_plane *ipu_plane, @@ -426,22 +423,15 @@ static struct drm_plane_funcs ipu_plane_funcs = { .set_property = ipu_plane_set_property, }; -struct ipu_plane *ipu_plane_init(struct drm_device *drm, struct ipu_soc *ipu, - int dma, int dp, unsigned int possible_crtcs, - bool priv) +int ipu_plane_init(struct ipu_plane *ipu_plane, struct drm_device *drm, + struct ipu_soc *ipu, int dma, int dp, + unsigned int possible_crtcs, bool priv) { - struct ipu_plane *ipu_plane; int ret; DRM_DEBUG_KMS("channel %d, dp flow %d, possible_crtcs=0x%x\n", dma, dp, possible_crtcs); - ipu_plane = kzalloc(sizeof(*ipu_plane), GFP_KERNEL); - if (!ipu_plane) { - DRM_ERROR("failed to allocate plane\n"); - return ERR_PTR(-ENOMEM); - } - ipu_plane->ipu = ipu; ipu_plane->dma = dma; ipu_plane->dp_flow = dp; @@ -452,7 +442,7 @@ struct ipu_plane *ipu_plane_init(struct drm_device *drm, struct ipu_soc *ipu, priv); if (ret) { DRM_ERROR("failed to initialize plane\n"); - goto err_free; + return ret; } /* default global alpha is enabled and completely opaque */ @@ -461,7 +451,7 @@ struct ipu_plane *ipu_plane_init(struct drm_device *drm, struct ipu_soc *ipu, /* for private planes, skip setting up properties */ if (priv) - return ipu_plane; + return 0; /* * global alpha range is 0 - 255. Bit 8 is used as a @@ -472,8 +462,7 @@ struct ipu_plane *ipu_plane_init(struct drm_device *drm, struct ipu_soc *ipu, 0, 0x1ff); if (!ipu_plane->global_alpha_prop) { DRM_ERROR("failed to create global alpha property\n"); - ret = -ENOMEM; - goto err_free; + return -ENOMEM; } /* @@ -485,8 +474,7 @@ struct ipu_plane *ipu_plane_init(struct drm_device *drm, struct ipu_soc *ipu, 0, 0x01ffffff); if (!ipu_plane->colorkey_prop) { DRM_ERROR("failed to create colorkey property\n"); - ret = -ENOMEM; - goto err_free; + return -ENOMEM; } drm_object_attach_property(&ipu_plane->base.base, @@ -495,9 +483,5 @@ struct ipu_plane *ipu_plane_init(struct drm_device *drm, struct ipu_soc *ipu, drm_object_attach_property(&ipu_plane->base.base, ipu_plane->colorkey_prop, 0); - return ipu_plane; - -err_free: - kfree(ipu_plane); - return ERR_PTR(ret); + return 0; } diff --git a/drivers/staging/imx-drm/ipuv3-plane.h b/drivers/staging/imx-drm/ipuv3-plane.h index b4daee5..1487a08 100644 --- a/drivers/staging/imx-drm/ipuv3-plane.h +++ b/drivers/staging/imx-drm/ipuv3-plane.h @@ -37,9 +37,9 @@ struct ipu_plane { bool enabled; }; -struct ipu_plane *ipu_plane_init(struct drm_device *dev, struct ipu_soc *ipu, - int dma, int dp, unsigned int possible_crtcs, - bool priv); +int ipu_plane_init(struct ipu_plane *ipu_plane, struct drm_device *drm, + struct ipu_soc *ipu, int dma, int dp, + unsigned int possible_crtcs, bool priv); /* Init IDMAC, DMFC, DP */ int ipu_plane_mode_set(struct ipu_plane *plane, struct drm_crtc *crtc,