All of lore.kernel.org
 help / color / mirror / Atom feed
From: jeffy <jeffy.chen@rock-chips.com>
To: linux-kernel@vger.kernel.org, briannorris@chromium.org,
	dianders@chromium.org, dri-devel@lists.freedesktop.org,
	tfiga@chromium.org, linux-rockchip@lists.infradead.org,
	zyw@rock-chips.com, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 8/9] drm/rockchip: gem: Don't alloc/free gem buf before drm dev registered
Date: Wed, 05 Apr 2017 09:28:17 +0800	[thread overview]
Message-ID: <58E44831.900@rock-chips.com> (raw)
In-Reply-To: <20170403074101.f3ykqpdphklwummz@phenom.ffwll.local>

Hi Daniel,

On 04/03/2017 03:41 PM, Daniel Vetter wrote:
> On Sat, Apr 01, 2017 at 07:35:28PM +0800, Jeffy Chen wrote:
>> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
>> ---
>>
>> Changes in v2: None
>
> Wut? How is this even possible? If you haven't registered the driver yet,
> there's no way for userspace to call allocation functions. Anything else
> is a driver bug that should be handled by fixing the driver load sequence.
> The only thing I can imagine is that you init the fbdev stuff too early,
> in that case pls fix that. Not apply this duct-tape here.
after unbind drm drivers, userspace may still have the chance to call 
gem ioctls before we call drm_dev_register again. and we can add a 
sanity check here to avoid invalid access.

i'll try to make a better commit message in next version soon, thanx for 
reply.
>
> Thanks, Daniel
>
>>
>>   drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
>> index df9e570..2bf8024 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
>> @@ -184,6 +184,9 @@ static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj,
>>   	struct drm_device *drm = obj->dev;
>>   	struct rockchip_drm_private *private = drm->dev_private;
>>
>> +	if (!drm->registered)
>> +		return -ENODEV;
>> +
>>   	if (private->domain)
>>   		return rockchip_gem_alloc_iommu(rk_obj, alloc_kmap);
>>   	else
>> @@ -208,6 +211,11 @@ static void rockchip_gem_free_dma(struct rockchip_gem_object *rk_obj)
>>
>>   static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj)
>>   {
>> +	struct drm_device *drm = rk_obj->base.dev;
>> +
>> +	if (!drm->registered)
>> +		return;
>> +
>>   	if (rk_obj->pages)
>>   		rockchip_gem_free_iommu(rk_obj);
>>   	else
>> --
>> 2.1.4
>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>

WARNING: multiple messages have this Message-ID (diff)
From: jeffy.chen@rock-chips.com (jeffy)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 8/9] drm/rockchip: gem: Don't alloc/free gem buf before drm dev registered
Date: Wed, 05 Apr 2017 09:28:17 +0800	[thread overview]
Message-ID: <58E44831.900@rock-chips.com> (raw)
In-Reply-To: <20170403074101.f3ykqpdphklwummz@phenom.ffwll.local>

Hi Daniel,

On 04/03/2017 03:41 PM, Daniel Vetter wrote:
> On Sat, Apr 01, 2017 at 07:35:28PM +0800, Jeffy Chen wrote:
>> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
>> ---
>>
>> Changes in v2: None
>
> Wut? How is this even possible? If you haven't registered the driver yet,
> there's no way for userspace to call allocation functions. Anything else
> is a driver bug that should be handled by fixing the driver load sequence.
> The only thing I can imagine is that you init the fbdev stuff too early,
> in that case pls fix that. Not apply this duct-tape here.
after unbind drm drivers, userspace may still have the chance to call 
gem ioctls before we call drm_dev_register again. and we can add a 
sanity check here to avoid invalid access.

i'll try to make a better commit message in next version soon, thanx for 
reply.
>
> Thanks, Daniel
>
>>
>>   drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
>> index df9e570..2bf8024 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
>> @@ -184,6 +184,9 @@ static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj,
>>   	struct drm_device *drm = obj->dev;
>>   	struct rockchip_drm_private *private = drm->dev_private;
>>
>> +	if (!drm->registered)
>> +		return -ENODEV;
>> +
>>   	if (private->domain)
>>   		return rockchip_gem_alloc_iommu(rk_obj, alloc_kmap);
>>   	else
>> @@ -208,6 +211,11 @@ static void rockchip_gem_free_dma(struct rockchip_gem_object *rk_obj)
>>
>>   static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj)
>>   {
>> +	struct drm_device *drm = rk_obj->base.dev;
>> +
>> +	if (!drm->registered)
>> +		return;
>> +
>>   	if (rk_obj->pages)
>>   		rockchip_gem_free_iommu(rk_obj);
>>   	else
>> --
>> 2.1.4
>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>

  reply	other threads:[~2017-04-05  1:28 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-01 11:35 [PATCH v2 0/9] drm: rockchip: Fix rockchip drm unbind crash error Jeffy Chen
2017-04-01 11:35 ` Jeffy Chen
2017-04-01 11:35 ` [PATCH v2 1/9] drm: bridge: analogix: Detach panel when unbinding analogix dp Jeffy Chen
2017-04-01 11:35 ` [PATCH v2 2/9] drm: bridge: analogix: Unregister dp aux when unbinding Jeffy Chen
2017-04-01 11:35 ` [PATCH v2 3/9] drm: bridge: analogix: Destroy connector " Jeffy Chen
2017-04-01 11:35 ` [PATCH v2 4/9] drm/rockchip: cdn-dp: Don't try to release firmware when not loaded Jeffy Chen
2017-04-01 11:35   ` Jeffy Chen
2017-04-01 11:35 ` [PATCH v2 5/9] drm/rockchip: vop: Enable pm domain when resetting vop Jeffy Chen
2017-04-01 11:35   ` Jeffy Chen
2017-04-01 11:35   ` Jeffy Chen
2017-04-04 19:42   ` Sean Paul
2017-04-04 19:42     ` Sean Paul
2017-04-04 19:42     ` Sean Paul
2017-04-05 10:23     ` jeffy
2017-04-05 10:23       ` jeffy
2017-04-01 11:35 ` [PATCH v2 6/9] drm/rockchip: Reoder unload sequence Jeffy Chen
2017-04-01 11:35   ` Jeffy Chen
2017-04-04 19:44   ` Sean Paul
2017-04-04 19:44     ` Sean Paul
2017-04-04 19:44     ` Sean Paul
2017-04-05 10:20     ` jeffy
2017-04-05 10:20       ` jeffy
2017-04-05 10:20       ` jeffy
2017-04-01 11:35 ` [PATCH v2 7/9] drm/rockchip: Force disable all crtc when unload Jeffy Chen
2017-04-01 11:35   ` Jeffy Chen
2017-04-03  7:58   ` Daniel Vetter
2017-04-03  7:58     ` Daniel Vetter
2017-04-03  7:58     ` Daniel Vetter
2017-04-05 10:25     ` jeffy
2017-04-05 10:25       ` jeffy
2017-04-01 11:35 ` [PATCH v2 8/9] drm/rockchip: gem: Don't alloc/free gem buf before drm dev registered Jeffy Chen
2017-04-01 11:35   ` Jeffy Chen
2017-04-03  7:41   ` Daniel Vetter
2017-04-03  7:41     ` Daniel Vetter
2017-04-03  7:41     ` Daniel Vetter
2017-04-05  1:28     ` jeffy [this message]
2017-04-05  1:28       ` jeffy
2017-04-01 11:35 ` [PATCH v2 9/9] drm/rockchip: cdn-dp: Don't unregister audio dev when unbinding Jeffy Chen
2017-04-01 11:35   ` Jeffy Chen

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=58E44831.900@rock-chips.com \
    --to=jeffy.chen@rock-chips.com \
    --cc=briannorris@chromium.org \
    --cc=dianders@chromium.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=tfiga@chromium.org \
    --cc=zyw@rock-chips.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.