All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Mark yao <mark.yao@rock-chips.com>
Cc: linux-rockchip@lists.infradead.org,
	dri-devel@lists.freedesktop.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/2] drm/rockchip: create the fake mmap offset during gem creation
Date: Mon, 16 Nov 2015 17:21:15 +0100	[thread overview]
Message-ID: <20151116162115.GB16848@phenom.ffwll.local> (raw)
In-Reply-To: <56418C07.60208@rock-chips.com>

On Tue, Nov 10, 2015 at 02:17:43PM +0800, Mark yao wrote:
> Hi Heiko
>      I don't think this patch is needed for rockchip drm, since rockchip drm
> only use kms.
> I saw the discussion about ("drm/exynos: create a fake mmap offset with gem
> creation"),
> Inki said that: "This patch makes drm_gem_create_mmap_offset to be called
> even in case of
> not using dumb* interfaces. I.e., exynos_drm_gem_create_ioctl ->
> exynos_drm_gem_mmap"
> 
> So I think this patch doesn't make sense for rockchip drm.
> 
> 
> On 2015年11月10日 03:50, Heiko Stuebner wrote:
> >Similarly to the equivalent exynos-fix in commit 48cf53f4343a

This exynos patch and your rockchip patch look really strange - the point
_is_ to only create fake mmap offset when someone actually tries to mmap a
bo. drm_gem_create_mmap_offset internally (and in a race-free manner)
checks whether it's been called already, so it's perfectly safe to call
mutliple times.

Imo drop this (and probably revert 48cf53f4343a as unjustified).
-Daniel

> >("drm/exynos: create a fake mmap offset with gem creation") the rockchip
> >drm driver also calls the drm_gem_create_mmap_offset fake-offset creation
> >on every invocation of rockchip_gem_dumb_map_offset.
> >
> >Move that to the creation of the gem to only create that offset once.
> >
> >Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> >---
> >  drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> >index 4ab2f20..e50d113 100644
> >--- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> >+++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> >@@ -126,8 +126,14 @@ struct rockchip_gem_object *
> >  	if (ret)
> >  		goto err_free_rk_obj;
> >+	ret = drm_gem_create_mmap_offset(obj);
> >+	if (ret)
> >+		goto err_free_buf;
> >+
> >  	return rk_obj;
> >+err_free_buf:
> >+	rockchip_gem_free_buf(rk_obj);
> >  err_free_rk_obj:
> >  	kfree(rk_obj);
> >  	return ERR_PTR(ret);
> >@@ -196,7 +202,6 @@ int rockchip_gem_dumb_map_offset(struct drm_file *file_priv,
> >  				 uint64_t *offset)
> >  {
> >  	struct drm_gem_object *obj;
> >-	int ret;
> >  	obj = drm_gem_object_lookup(dev, file_priv, handle);
> >  	if (!obj) {
> >@@ -204,14 +209,9 @@ int rockchip_gem_dumb_map_offset(struct drm_file *file_priv,
> >  		return -EINVAL;
> >  	}
> >-	ret = drm_gem_create_mmap_offset(obj);
> >-	if (ret)
> >-		goto out;
> >-
> >  	*offset = drm_vma_node_offset_addr(&obj->vma_node);
> >  	DRM_DEBUG_KMS("offset = 0x%llx\n", *offset);
> >-out:
> >  	drm_gem_object_unreference_unlocked(obj);
> >  	return 0;
> 
> 
> -- 
> Mark Yao
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://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
http://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID (diff)
From: daniel@ffwll.ch (Daniel Vetter)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/2] drm/rockchip: create the fake mmap offset during gem creation
Date: Mon, 16 Nov 2015 17:21:15 +0100	[thread overview]
Message-ID: <20151116162115.GB16848@phenom.ffwll.local> (raw)
In-Reply-To: <56418C07.60208@rock-chips.com>

On Tue, Nov 10, 2015 at 02:17:43PM +0800, Mark yao wrote:
> Hi Heiko
>      I don't think this patch is needed for rockchip drm, since rockchip drm
> only use kms.
> I saw the discussion about ("drm/exynos: create a fake mmap offset with gem
> creation"),
> Inki said that: "This patch makes drm_gem_create_mmap_offset to be called
> even in case of
> not using dumb* interfaces. I.e., exynos_drm_gem_create_ioctl ->
> exynos_drm_gem_mmap"
> 
> So I think this patch doesn't make sense for rockchip drm.
> 
> 
> On 2015?11?10? 03:50, Heiko Stuebner wrote:
> >Similarly to the equivalent exynos-fix in commit 48cf53f4343a

This exynos patch and your rockchip patch look really strange - the point
_is_ to only create fake mmap offset when someone actually tries to mmap a
bo. drm_gem_create_mmap_offset internally (and in a race-free manner)
checks whether it's been called already, so it's perfectly safe to call
mutliple times.

Imo drop this (and probably revert 48cf53f4343a as unjustified).
-Daniel

> >("drm/exynos: create a fake mmap offset with gem creation") the rockchip
> >drm driver also calls the drm_gem_create_mmap_offset fake-offset creation
> >on every invocation of rockchip_gem_dumb_map_offset.
> >
> >Move that to the creation of the gem to only create that offset once.
> >
> >Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> >---
> >  drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> >index 4ab2f20..e50d113 100644
> >--- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> >+++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> >@@ -126,8 +126,14 @@ struct rockchip_gem_object *
> >  	if (ret)
> >  		goto err_free_rk_obj;
> >+	ret = drm_gem_create_mmap_offset(obj);
> >+	if (ret)
> >+		goto err_free_buf;
> >+
> >  	return rk_obj;
> >+err_free_buf:
> >+	rockchip_gem_free_buf(rk_obj);
> >  err_free_rk_obj:
> >  	kfree(rk_obj);
> >  	return ERR_PTR(ret);
> >@@ -196,7 +202,6 @@ int rockchip_gem_dumb_map_offset(struct drm_file *file_priv,
> >  				 uint64_t *offset)
> >  {
> >  	struct drm_gem_object *obj;
> >-	int ret;
> >  	obj = drm_gem_object_lookup(dev, file_priv, handle);
> >  	if (!obj) {
> >@@ -204,14 +209,9 @@ int rockchip_gem_dumb_map_offset(struct drm_file *file_priv,
> >  		return -EINVAL;
> >  	}
> >-	ret = drm_gem_create_mmap_offset(obj);
> >-	if (ret)
> >-		goto out;
> >-
> >  	*offset = drm_vma_node_offset_addr(&obj->vma_node);
> >  	DRM_DEBUG_KMS("offset = 0x%llx\n", *offset);
> >-out:
> >  	drm_gem_object_unreference_unlocked(obj);
> >  	return 0;
> 
> 
> -- 
> ?ark Yao
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

  parent reply	other threads:[~2015-11-16 16:21 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-09 19:50 [PATCH 1/2] drm/rockchip: create the fake mmap offset during gem creation Heiko Stuebner
2015-11-09 19:50 ` Heiko Stuebner
2015-11-09 19:52 ` [PATCH 2/2] drm/rockchip: unset pgoff when mmap'ing gems Heiko Stuebner
2015-11-09 19:52   ` Heiko Stuebner
2015-11-10  5:53   ` Mark yao
2015-11-10  5:53     ` Mark yao
2015-11-10  6:17 ` [PATCH 1/2] drm/rockchip: create the fake mmap offset during gem creation Mark yao
2015-11-10  6:17   ` Mark yao
2015-11-10  6:46   ` Heiko Stuebner
2015-11-10  6:46     ` Heiko Stuebner
2015-11-10  9:26     ` Mark yao
2015-11-10  9:26       ` Mark yao
2015-11-10  9:28       ` Heiko Stuebner
2015-11-10  9:28         ` Heiko Stuebner
2015-11-10  9:38         ` Mark yao
2015-11-10  9:38           ` Mark yao
2015-11-16 16:21   ` Daniel Vetter [this message]
2015-11-16 16:21     ` Daniel Vetter

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=20151116162115.GB16848@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=mark.yao@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.