From: Daniel Vetter <daniel@ffwll.ch>
To: Xinliang Liu <xinliang.liu@linaro.org>
Cc: Xinliang Liu <z.liuxinliang@hisilicon.com>,
Daniel Vetter <daniel.vetter@ffwll.ch>,
Daniel Vetter <daniel.vetter@intel.com>,
DRI Development <dri-devel@lists.freedesktop.org>,
Chen Feng <puck.chen@hisilicon.com>
Subject: Re: [PATCH 2/2] drm/hisilicon: Make it compile again
Date: Wed, 17 Aug 2016 13:11:18 +0200 [thread overview]
Message-ID: <20160817111118.GV6232@phenom.ffwll.local> (raw)
In-Reply-To: <CAGd==053Wfh5_8xTpp7Lfcg6BsPSqsNKyViPdunbr0b1KFS4+g@mail.gmail.com>
On Wed, Aug 17, 2016 at 07:02:01PM +0800, Xinliang Liu wrote:
> Hi,
>
> On 17 August 2016 at 18:17, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > I just broke the build :(
> >
> > Note that the cleanup function is a bit confused: ade_data was never
> > set as drvdata, and calling drm_crtc_cleanup directly is a bug - this
>
> Yes, this is a bug. Thanks for pointing out this.
>
> > is called indirectly through drm_mode_config_cleanup, which calls into
> > crtc->destroy, which already has the call to drm_crtc_cleanup. Which
> > means we can just nuke it.
> >
> > Aside: I have no idea why this driver depends upon ARM64. It doesn't
> > build warning-free on 32bit, but otherwise it's perfectly fine.
>
> Because this driver is written for ARM64 SoCs.
It makes compile-testing harder, the same reasons why depending upon
specific arm platforms is really annoying (but lukily that problem seems
to have stopped being one for the drm subsystem at least). Depending on
CONFIG_OF and other stuff that's really needed is perfectly fine, but imo
depending upon specific platforms and stuff really isn't.
-Daniel
>
> Thanks,
> -xinliang
>
> >
> > Cc: Xinliang Liu <z.liuxinliang@hisilicon.com>
> > Cc: Xinwei Kong <kong.kongxinwei@hisilicon.com>
> > Cc: Chen Feng <puck.chen@hisilicon.com>
> > Cc: Sean Paul <seanpaul@chromium.org>
> > Fixes: d25bcfb8c2e1 ("drm/hisilicon: Don't set drm_device->platformdev")
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> > drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 9 ++-------
> > drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 4 ++--
> > 2 files changed, 4 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> > index 91188f33b1d9..c64c82cb7e71 100644
> > --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> > +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> > @@ -991,7 +991,7 @@ static int ade_dts_parse(struct platform_device *pdev, struct ade_hw_ctx *ctx)
> >
> > static int ade_drm_init(struct platform_device *pdev)
> > {
> > - struct drm_device *drm_dev = platform_get_drvdata(dev);
> > + struct drm_device *dev = platform_get_drvdata(pdev);
> > struct ade_data *ade;
> > struct ade_hw_ctx *ctx;
> > struct ade_crtc *acrtc;
> > @@ -1052,14 +1052,9 @@ static int ade_drm_init(struct platform_device *pdev)
> >
> > static void ade_drm_cleanup(struct platform_device *pdev)
> > {
> > - struct drm_device *drm_dev = platform_get_drvdata(dev);
> > - struct ade_data *ade = platform_get_drvdata(pdev);
> > - struct drm_crtc *crtc = &ade->acrtc.base;
> > -
> > - drm_crtc_cleanup(crtc);
> > }
> >
> > const struct kirin_dc_ops ade_dc_ops = {
> > .init = ade_drm_init,
> > .cleanup = ade_drm_cleanup
> > -;
> > +};
> > diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> > index 6b0f9f6c16e1..b9b8c25da5e3 100644
> > --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> > +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> > @@ -103,7 +103,7 @@ static int kirin_drm_kms_init(struct drm_device *dev)
> > kirin_drm_mode_config_init(dev);
> >
> > /* display controller init */
> > - ret = dc_ops->init(to_platform_device(dev));
> > + ret = dc_ops->init(to_platform_device(dev->dev));
> > if (ret)
> > goto err_mode_config_cleanup;
> >
> > @@ -137,7 +137,7 @@ static int kirin_drm_kms_init(struct drm_device *dev)
> > err_unbind_all:
> > component_unbind_all(dev->dev, dev);
> > err_dc_cleanup:
> > - dc_ops->cleanup(dev);
> > + dc_ops->cleanup(to_platform_device(dev->dev));
> > err_mode_config_cleanup:
> > drm_mode_config_cleanup(dev);
> > devm_kfree(dev->dev, priv);
> > --
> > 2.8.1
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2016-08-17 11:11 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-17 10:17 [PATCH 1/2] drm/hisilicon: Allow compile-testing on arm Daniel Vetter
2016-08-17 10:17 ` [PATCH 2/2] drm/hisilicon: Make it compile again Daniel Vetter
2016-08-17 11:02 ` Xinliang Liu
2016-08-17 11:11 ` Daniel Vetter [this message]
2016-08-26 1:48 ` Xinliang Liu
2016-08-26 2:28 ` Rob Clark
2016-09-01 3:33 ` Xinliang Liu
2016-08-17 13:58 ` [PATCH 1/2] drm/hisilicon: Allow compile-testing on arm Daniel Stone
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160817111118.GV6232@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=daniel.vetter@ffwll.ch \
--cc=daniel.vetter@intel.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=puck.chen@hisilicon.com \
--cc=xinliang.liu@linaro.org \
--cc=z.liuxinliang@hisilicon.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox