dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] drm/i915: Set I915_BO_ALLOC_USER for framebuffer
@ 2023-03-06 10:28 Nirmoy Das
  2023-03-06 10:28 ` [PATCH 2/3] drm/i915: Add a helper func for gem obj mmap Nirmoy Das
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Nirmoy Das @ 2023-03-06 10:28 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel, Nirmoy Das

Framebuffer is exposed to userspace so set I915_BO_ALLOC_USER
flag for it. This also make sure that ttm allocates offset
for lmem objects.

Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dpt.c           | 4 +++-
 drivers/gpu/drm/i915/display/intel_fbdev.c         | 3 ++-
 drivers/gpu/drm/i915/display/intel_plane_initial.c | 3 ++-
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c b/drivers/gpu/drm/i915/display/intel_dpt.c
index ad1a37b515fb..2e6238881860 100644
--- a/drivers/gpu/drm/i915/display/intel_dpt.c
+++ b/drivers/gpu/drm/i915/display/intel_dpt.c
@@ -254,7 +254,9 @@ intel_dpt_create(struct intel_framebuffer *fb)
 
 	size = round_up(size * sizeof(gen8_pte_t), I915_GTT_PAGE_SIZE);
 
-	dpt_obj = i915_gem_object_create_lmem(i915, size, I915_BO_ALLOC_CONTIGUOUS);
+	dpt_obj = i915_gem_object_create_lmem(i915, size,
+					      I915_BO_ALLOC_CONTIGUOUS |
+					      I915_BO_ALLOC_USER);
 	if (IS_ERR(dpt_obj) && i915_ggtt_has_aperture(to_gt(i915)->ggtt))
 		dpt_obj = i915_gem_object_create_stolen(i915, size);
 	if (IS_ERR(dpt_obj) && !HAS_LMEM(i915)) {
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
index 3659350061a7..98ae3a3a986a 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -163,7 +163,8 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
 	obj = ERR_PTR(-ENODEV);
 	if (HAS_LMEM(dev_priv)) {
 		obj = i915_gem_object_create_lmem(dev_priv, size,
-						  I915_BO_ALLOC_CONTIGUOUS);
+						  I915_BO_ALLOC_CONTIGUOUS |
+						  I915_BO_ALLOC_USER);
 	} else {
 		/*
 		 * If the FB is too big, just don't use it since fbdev is not very
diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c b/drivers/gpu/drm/i915/display/intel_plane_initial.c
index bb6ea7de5c61..4a3680f6a3f5 100644
--- a/drivers/gpu/drm/i915/display/intel_plane_initial.c
+++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c
@@ -110,7 +110,8 @@ initial_plane_vma(struct drm_i915_private *i915,
 	    size * 2 > i915->dsm.usable_size)
 		return NULL;
 
-	obj = i915_gem_object_create_region_at(mem, phys_base, size, 0);
+	obj = i915_gem_object_create_region_at(mem, phys_base, size,
+					       I915_BO_ALLOC_USER);
 	if (IS_ERR(obj))
 		return NULL;
 
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/3] drm/i915: Add a helper func for gem obj mmap
  2023-03-06 10:28 [PATCH 1/3] drm/i915: Set I915_BO_ALLOC_USER for framebuffer Nirmoy Das
@ 2023-03-06 10:28 ` Nirmoy Das
  2023-03-06 14:26   ` Ville Syrjälä
  2023-03-06 10:28 ` [PATCH RFC 3/3] drm/i915/display: Implement fb_mmap callback function Nirmoy Das
  2023-03-06 14:21 ` [Intel-gfx] [PATCH 1/3] drm/i915: Set I915_BO_ALLOC_USER for framebuffer Ville Syrjälä
  2 siblings, 1 reply; 12+ messages in thread
From: Nirmoy Das @ 2023-03-06 10:28 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel, Nirmoy Das

Move gem obj mmap code to i915_gem_object_mmap() so that
this can be used by others.

Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 20 ++---------------
 drivers/gpu/drm/i915/gem/i915_gem_mman.c   | 25 ++++++++++++++++++++++
 drivers/gpu/drm/i915/gem/i915_gem_mman.h   |  1 +
 3 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
index fd556a076d05..831dd8ebf819 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
@@ -12,6 +12,7 @@
 #include <asm/smp.h>
 
 #include "gem/i915_gem_dmabuf.h"
+#include "gem/i915_gem_mman.h"
 #include "i915_drv.h"
 #include "i915_gem_object.h"
 #include "i915_scatterlist.h"
@@ -94,27 +95,10 @@ static void i915_gem_dmabuf_vunmap(struct dma_buf *dma_buf,
 static int i915_gem_dmabuf_mmap(struct dma_buf *dma_buf, struct vm_area_struct *vma)
 {
 	struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
-	struct drm_i915_private *i915 = to_i915(obj->base.dev);
-	int ret;
 
 	dma_resv_assert_held(dma_buf->resv);
 
-	if (obj->base.size < vma->vm_end - vma->vm_start)
-		return -EINVAL;
-
-	if (HAS_LMEM(i915))
-		return drm_gem_prime_mmap(&obj->base, vma);
-
-	if (!obj->base.filp)
-		return -ENODEV;
-
-	ret = call_mmap(obj->base.filp, vma);
-	if (ret)
-		return ret;
-
-	vma_set_file(vma, obj->base.filp);
-
-	return 0;
+	return i915_gem_object_mmap(obj, vma);
 }
 
 static int i915_gem_begin_cpu_access(struct dma_buf *dma_buf, enum dma_data_direction direction)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index 2aac6bf78740..d378720ca626 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -11,6 +11,8 @@
 
 #include <drm/drm_cache.h>
 
+#include "gem/i915_gem_lmem.h"
+
 #include "gt/intel_gt.h"
 #include "gt/intel_gt_requests.h"
 
@@ -1043,6 +1045,29 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma)
 	return 0;
 }
 
+int i915_gem_object_mmap(struct drm_i915_gem_object *obj, struct vm_area_struct *vma)
+{
+	struct drm_i915_private *i915 = to_i915(obj->base.dev);
+	int ret;
+
+	if (obj->base.size < vma->vm_end - vma->vm_start)
+		return -EINVAL;
+
+	if (HAS_LMEM(i915))
+		return drm_gem_prime_mmap(&obj->base, vma);
+
+	if (obj->base.filp) {
+		ret = call_mmap(obj->base.filp, vma);
+		if (ret)
+			return ret;
+
+		vma_set_file(vma, obj->base.filp);
+		return 0;
+	}
+
+	return -ENODEV;
+}
+
 #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
 #include "selftests/i915_gem_mman.c"
 #endif
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.h b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
index 1fa91b3033b3..303e81ddc5ba 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
@@ -30,4 +30,5 @@ void i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj);
 void i915_gem_object_runtime_pm_release_mmap_offset(struct drm_i915_gem_object *obj);
 void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj);
 
+int i915_gem_object_mmap(struct drm_i915_gem_object *obj, struct vm_area_struct *vma);
 #endif
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH RFC 3/3] drm/i915/display: Implement fb_mmap callback function
  2023-03-06 10:28 [PATCH 1/3] drm/i915: Set I915_BO_ALLOC_USER for framebuffer Nirmoy Das
  2023-03-06 10:28 ` [PATCH 2/3] drm/i915: Add a helper func for gem obj mmap Nirmoy Das
@ 2023-03-06 10:28 ` Nirmoy Das
  2023-03-06 14:32   ` Ville Syrjälä
  2023-03-06 14:21 ` [Intel-gfx] [PATCH 1/3] drm/i915: Set I915_BO_ALLOC_USER for framebuffer Ville Syrjälä
  2 siblings, 1 reply; 12+ messages in thread
From: Nirmoy Das @ 2023-03-06 10:28 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel, Nirmoy Das

If stolen memory allocation fails for fbdev, the driver will
fallback to system memory. Calculation of smem_start is wrong
for such framebuffer objs if the platform comes with no gmadr or
no aperture. Solve this by adding fb_mmap callback which also gives
driver more control.

Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
---
 drivers/gpu/drm/i915/display/intel_fbdev.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
index 98ae3a3a986a..ed0f9e2af3ed 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -40,8 +40,10 @@
 #include <drm/drm_crtc.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_fourcc.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 
 #include "gem/i915_gem_lmem.h"
+#include "gem/i915_gem_mman.h"
 
 #include "i915_drv.h"
 #include "intel_display_types.h"
@@ -120,6 +122,23 @@ static int intel_fbdev_pan_display(struct fb_var_screeninfo *var,
 	return ret;
 }
 
+#define to_intel_fbdev(x) container_of(x, struct intel_fbdev, helper)
+static int intel_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma)
+{
+	struct intel_fbdev *fbdev = to_intel_fbdev(info->par);
+	struct drm_gem_object *bo = drm_gem_fb_get_obj(&fbdev->fb->base, 0);
+	struct drm_i915_gem_object *obj = to_intel_bo(bo);
+	struct drm_device *dev = fbdev->helper.dev;
+
+	vma->vm_page_prot =
+			pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
+
+	if (obj->stolen)
+		return vm_iomap_memory(vma, info->fix.smem_start,
+				       info->fix.smem_len);
+
+	return i915_gem_object_mmap(obj, vma);
+}
 static const struct fb_ops intelfb_ops = {
 	.owner = THIS_MODULE,
 	DRM_FB_HELPER_DEFAULT_OPS,
@@ -131,6 +150,7 @@ static const struct fb_ops intelfb_ops = {
 	.fb_imageblit = drm_fb_helper_cfb_imageblit,
 	.fb_pan_display = intel_fbdev_pan_display,
 	.fb_blank = intel_fbdev_blank,
+	.fb_mmap = intel_fbdev_mmap,
 };
 
 static int intelfb_alloc(struct drm_fb_helper *helper,
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [Intel-gfx] [PATCH 1/3] drm/i915: Set I915_BO_ALLOC_USER for framebuffer
  2023-03-06 10:28 [PATCH 1/3] drm/i915: Set I915_BO_ALLOC_USER for framebuffer Nirmoy Das
  2023-03-06 10:28 ` [PATCH 2/3] drm/i915: Add a helper func for gem obj mmap Nirmoy Das
  2023-03-06 10:28 ` [PATCH RFC 3/3] drm/i915/display: Implement fb_mmap callback function Nirmoy Das
@ 2023-03-06 14:21 ` Ville Syrjälä
  2023-03-06 16:22   ` Das, Nirmoy
  2 siblings, 1 reply; 12+ messages in thread
From: Ville Syrjälä @ 2023-03-06 14:21 UTC (permalink / raw)
  To: Nirmoy Das; +Cc: intel-gfx, dri-devel

On Mon, Mar 06, 2023 at 11:28:48AM +0100, Nirmoy Das wrote:
> Framebuffer is exposed to userspace so set I915_BO_ALLOC_USER
> flag for it. This also make sure that ttm allocates offset
> for lmem objects.

I have no idea what that means.

> 
> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dpt.c           | 4 +++-
>  drivers/gpu/drm/i915/display/intel_fbdev.c         | 3 ++-
>  drivers/gpu/drm/i915/display/intel_plane_initial.c | 3 ++-
>  3 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c b/drivers/gpu/drm/i915/display/intel_dpt.c
> index ad1a37b515fb..2e6238881860 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpt.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpt.c
> @@ -254,7 +254,9 @@ intel_dpt_create(struct intel_framebuffer *fb)
>  
>  	size = round_up(size * sizeof(gen8_pte_t), I915_GTT_PAGE_SIZE);
>  
> -	dpt_obj = i915_gem_object_create_lmem(i915, size, I915_BO_ALLOC_CONTIGUOUS);
> +	dpt_obj = i915_gem_object_create_lmem(i915, size,
> +					      I915_BO_ALLOC_CONTIGUOUS |
> +					      I915_BO_ALLOC_USER);
>  	if (IS_ERR(dpt_obj) && i915_ggtt_has_aperture(to_gt(i915)->ggtt))
>  		dpt_obj = i915_gem_object_create_stolen(i915, size);
>  	if (IS_ERR(dpt_obj) && !HAS_LMEM(i915)) {
> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
> index 3659350061a7..98ae3a3a986a 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> @@ -163,7 +163,8 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
>  	obj = ERR_PTR(-ENODEV);
>  	if (HAS_LMEM(dev_priv)) {
>  		obj = i915_gem_object_create_lmem(dev_priv, size,
> -						  I915_BO_ALLOC_CONTIGUOUS);
> +						  I915_BO_ALLOC_CONTIGUOUS |
> +						  I915_BO_ALLOC_USER);
>  	} else {
>  		/*
>  		 * If the FB is too big, just don't use it since fbdev is not very
> diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c b/drivers/gpu/drm/i915/display/intel_plane_initial.c
> index bb6ea7de5c61..4a3680f6a3f5 100644
> --- a/drivers/gpu/drm/i915/display/intel_plane_initial.c
> +++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c
> @@ -110,7 +110,8 @@ initial_plane_vma(struct drm_i915_private *i915,
>  	    size * 2 > i915->dsm.usable_size)
>  		return NULL;
>  
> -	obj = i915_gem_object_create_region_at(mem, phys_base, size, 0);
> +	obj = i915_gem_object_create_region_at(mem, phys_base, size,
> +					       I915_BO_ALLOC_USER);
>  	if (IS_ERR(obj))
>  		return NULL;
>  
> -- 
> 2.39.0

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] drm/i915: Add a helper func for gem obj mmap
  2023-03-06 10:28 ` [PATCH 2/3] drm/i915: Add a helper func for gem obj mmap Nirmoy Das
@ 2023-03-06 14:26   ` Ville Syrjälä
  2023-03-06 16:18     ` Das, Nirmoy
  0 siblings, 1 reply; 12+ messages in thread
From: Ville Syrjälä @ 2023-03-06 14:26 UTC (permalink / raw)
  To: Nirmoy Das; +Cc: intel-gfx, dri-devel

On Mon, Mar 06, 2023 at 11:28:49AM +0100, Nirmoy Das wrote:
> Move gem obj mmap code to i915_gem_object_mmap() so that
> this can be used by others.
> 
> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 20 ++---------------
>  drivers/gpu/drm/i915/gem/i915_gem_mman.c   | 25 ++++++++++++++++++++++
>  drivers/gpu/drm/i915/gem/i915_gem_mman.h   |  1 +
>  3 files changed, 28 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
> index fd556a076d05..831dd8ebf819 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
> @@ -12,6 +12,7 @@
>  #include <asm/smp.h>
>  
>  #include "gem/i915_gem_dmabuf.h"
> +#include "gem/i915_gem_mman.h"
>  #include "i915_drv.h"
>  #include "i915_gem_object.h"
>  #include "i915_scatterlist.h"
> @@ -94,27 +95,10 @@ static void i915_gem_dmabuf_vunmap(struct dma_buf *dma_buf,
>  static int i915_gem_dmabuf_mmap(struct dma_buf *dma_buf, struct vm_area_struct *vma)
>  {
>  	struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
> -	struct drm_i915_private *i915 = to_i915(obj->base.dev);
> -	int ret;
>  
>  	dma_resv_assert_held(dma_buf->resv);
>  
> -	if (obj->base.size < vma->vm_end - vma->vm_start)
> -		return -EINVAL;
> -
> -	if (HAS_LMEM(i915))
> -		return drm_gem_prime_mmap(&obj->base, vma);
> -
> -	if (!obj->base.filp)
> -		return -ENODEV;
> -
> -	ret = call_mmap(obj->base.filp, vma);
> -	if (ret)
> -		return ret;
> -
> -	vma_set_file(vma, obj->base.filp);
> -
> -	return 0;
> +	return i915_gem_object_mmap(obj, vma);
>  }
>  
>  static int i915_gem_begin_cpu_access(struct dma_buf *dma_buf, enum dma_data_direction direction)
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> index 2aac6bf78740..d378720ca626 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> @@ -11,6 +11,8 @@
>  
>  #include <drm/drm_cache.h>
>  
> +#include "gem/i915_gem_lmem.h"
> +
>  #include "gt/intel_gt.h"
>  #include "gt/intel_gt_requests.h"
>  
> @@ -1043,6 +1045,29 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>  	return 0;
>  }
>  
> +int i915_gem_object_mmap(struct drm_i915_gem_object *obj, struct vm_area_struct *vma)
> +{
> +	struct drm_i915_private *i915 = to_i915(obj->base.dev);
> +	int ret;
> +
> +	if (obj->base.size < vma->vm_end - vma->vm_start)
> +		return -EINVAL;
> +
> +	if (HAS_LMEM(i915))
> +		return drm_gem_prime_mmap(&obj->base, vma);

Calling some prime stuff here doesn't smell right.

> +
> +	if (obj->base.filp) {
> +		ret = call_mmap(obj->base.filp, vma);
> +		if (ret)
> +			return ret;
> +
> +		vma_set_file(vma, obj->base.filp);
> +		return 0;
> +	}
> +
> +	return -ENODEV;
> +}
> +
>  #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
>  #include "selftests/i915_gem_mman.c"
>  #endif
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.h b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
> index 1fa91b3033b3..303e81ddc5ba 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
> @@ -30,4 +30,5 @@ void i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj);
>  void i915_gem_object_runtime_pm_release_mmap_offset(struct drm_i915_gem_object *obj);
>  void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj);
>  
> +int i915_gem_object_mmap(struct drm_i915_gem_object *obj, struct vm_area_struct *vma);
>  #endif
> -- 
> 2.39.0

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH RFC 3/3] drm/i915/display: Implement fb_mmap callback function
  2023-03-06 10:28 ` [PATCH RFC 3/3] drm/i915/display: Implement fb_mmap callback function Nirmoy Das
@ 2023-03-06 14:32   ` Ville Syrjälä
  2023-03-07 14:50     ` Das, Nirmoy
  0 siblings, 1 reply; 12+ messages in thread
From: Ville Syrjälä @ 2023-03-06 14:32 UTC (permalink / raw)
  To: Nirmoy Das; +Cc: intel-gfx, dri-devel

On Mon, Mar 06, 2023 at 11:28:50AM +0100, Nirmoy Das wrote:
> If stolen memory allocation fails for fbdev, the driver will
> fallback to system memory. Calculation of smem_start is wrong
> for such framebuffer objs if the platform comes with no gmadr or
> no aperture. Solve this by adding fb_mmap callback which also gives
> driver more control.
> 
> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_fbdev.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
> index 98ae3a3a986a..ed0f9e2af3ed 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> @@ -40,8 +40,10 @@
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_fb_helper.h>
>  #include <drm/drm_fourcc.h>
> +#include <drm/drm_gem_framebuffer_helper.h>
>  
>  #include "gem/i915_gem_lmem.h"
> +#include "gem/i915_gem_mman.h"
>  
>  #include "i915_drv.h"
>  #include "intel_display_types.h"
> @@ -120,6 +122,23 @@ static int intel_fbdev_pan_display(struct fb_var_screeninfo *var,
>  	return ret;
>  }
>  
> +#define to_intel_fbdev(x) container_of(x, struct intel_fbdev, helper)
> +static int intel_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma)
> +{
> +	struct intel_fbdev *fbdev = to_intel_fbdev(info->par);
> +	struct drm_gem_object *bo = drm_gem_fb_get_obj(&fbdev->fb->base, 0);
> +	struct drm_i915_gem_object *obj = to_intel_bo(bo);
> +	struct drm_device *dev = fbdev->helper.dev;

You seem to be missing the fb vs. mmio handling here entirely.

> +
> +	vma->vm_page_prot =
> +			pgprot_writecombine(vm_get_page_prot(vma->vm_flags));

Does that do something sane on eg. !PAT?

> +
> +	if (obj->stolen)
> +		return vm_iomap_memory(vma, info->fix.smem_start,
> +				       info->fix.smem_len);

Why doesn't i915_gem_object_mmap() know how to handle stolen?

> +
> +	return i915_gem_object_mmap(obj, vma);
> +}
>  static const struct fb_ops intelfb_ops = {
>  	.owner = THIS_MODULE,
>  	DRM_FB_HELPER_DEFAULT_OPS,
> @@ -131,6 +150,7 @@ static const struct fb_ops intelfb_ops = {
>  	.fb_imageblit = drm_fb_helper_cfb_imageblit,
>  	.fb_pan_display = intel_fbdev_pan_display,
>  	.fb_blank = intel_fbdev_blank,
> +	.fb_mmap = intel_fbdev_mmap,
>  };
>  
>  static int intelfb_alloc(struct drm_fb_helper *helper,
> -- 
> 2.39.0

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] drm/i915: Add a helper func for gem obj mmap
  2023-03-06 14:26   ` Ville Syrjälä
@ 2023-03-06 16:18     ` Das, Nirmoy
  0 siblings, 0 replies; 12+ messages in thread
From: Das, Nirmoy @ 2023-03-06 16:18 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, dri-devel


On 3/6/2023 3:26 PM, Ville Syrjälä wrote:
> On Mon, Mar 06, 2023 at 11:28:49AM +0100, Nirmoy Das wrote:
>> Move gem obj mmap code to i915_gem_object_mmap() so that
>> this can be used by others.
>>
>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 20 ++---------------
>>   drivers/gpu/drm/i915/gem/i915_gem_mman.c   | 25 ++++++++++++++++++++++
>>   drivers/gpu/drm/i915/gem/i915_gem_mman.h   |  1 +
>>   3 files changed, 28 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>> index fd556a076d05..831dd8ebf819 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>> @@ -12,6 +12,7 @@
>>   #include <asm/smp.h>
>>   
>>   #include "gem/i915_gem_dmabuf.h"
>> +#include "gem/i915_gem_mman.h"
>>   #include "i915_drv.h"
>>   #include "i915_gem_object.h"
>>   #include "i915_scatterlist.h"
>> @@ -94,27 +95,10 @@ static void i915_gem_dmabuf_vunmap(struct dma_buf *dma_buf,
>>   static int i915_gem_dmabuf_mmap(struct dma_buf *dma_buf, struct vm_area_struct *vma)
>>   {
>>   	struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
>> -	struct drm_i915_private *i915 = to_i915(obj->base.dev);
>> -	int ret;
>>   
>>   	dma_resv_assert_held(dma_buf->resv);
>>   
>> -	if (obj->base.size < vma->vm_end - vma->vm_start)
>> -		return -EINVAL;
>> -
>> -	if (HAS_LMEM(i915))
>> -		return drm_gem_prime_mmap(&obj->base, vma);
>> -
>> -	if (!obj->base.filp)
>> -		return -ENODEV;
>> -
>> -	ret = call_mmap(obj->base.filp, vma);
>> -	if (ret)
>> -		return ret;
>> -
>> -	vma_set_file(vma, obj->base.filp);
>> -
>> -	return 0;
>> +	return i915_gem_object_mmap(obj, vma);
>>   }
>>   
>>   static int i915_gem_begin_cpu_access(struct dma_buf *dma_buf, enum dma_data_direction direction)
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
>> index 2aac6bf78740..d378720ca626 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
>> @@ -11,6 +11,8 @@
>>   
>>   #include <drm/drm_cache.h>
>>   
>> +#include "gem/i915_gem_lmem.h"
>> +
>>   #include "gt/intel_gt.h"
>>   #include "gt/intel_gt_requests.h"
>>   
>> @@ -1043,6 +1045,29 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>>   	return 0;
>>   }
>>   
>> +int i915_gem_object_mmap(struct drm_i915_gem_object *obj, struct vm_area_struct *vma)
>> +{
>> +	struct drm_i915_private *i915 = to_i915(obj->base.dev);
>> +	int ret;
>> +
>> +	if (obj->base.size < vma->vm_end - vma->vm_start)
>> +		return -EINVAL;
>> +
>> +	if (HAS_LMEM(i915))
>> +		return drm_gem_prime_mmap(&obj->base, vma);
> Calling some prime stuff here doesn't smell right.

Yes, I should use drm_gem_mmap_obj() here.


>
>> +
>> +	if (obj->base.filp) {
>> +		ret = call_mmap(obj->base.filp, vma);
>> +		if (ret)
>> +			return ret;
>> +
>> +		vma_set_file(vma, obj->base.filp);
>> +		return 0;
>> +	}
>> +
>> +	return -ENODEV;
>> +}
>> +
>>   #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
>>   #include "selftests/i915_gem_mman.c"
>>   #endif
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.h b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
>> index 1fa91b3033b3..303e81ddc5ba 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.h
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
>> @@ -30,4 +30,5 @@ void i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj);
>>   void i915_gem_object_runtime_pm_release_mmap_offset(struct drm_i915_gem_object *obj);
>>   void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj);
>>   
>> +int i915_gem_object_mmap(struct drm_i915_gem_object *obj, struct vm_area_struct *vma);
>>   #endif
>> -- 
>> 2.39.0

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-gfx] [PATCH 1/3] drm/i915: Set I915_BO_ALLOC_USER for framebuffer
  2023-03-06 14:21 ` [Intel-gfx] [PATCH 1/3] drm/i915: Set I915_BO_ALLOC_USER for framebuffer Ville Syrjälä
@ 2023-03-06 16:22   ` Das, Nirmoy
  2023-03-06 17:30     ` Ville Syrjälä
  0 siblings, 1 reply; 12+ messages in thread
From: Das, Nirmoy @ 2023-03-06 16:22 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, dri-devel


On 3/6/2023 3:21 PM, Ville Syrjälä wrote:
> On Mon, Mar 06, 2023 at 11:28:48AM +0100, Nirmoy Das wrote:
>> Framebuffer is exposed to userspace so set I915_BO_ALLOC_USER
>> flag for it. This also make sure that ttm allocates offset
>> for lmem objects.
> I have no idea what that means.

Sorry for poor explanation.

Without I915_BO_ALLOC_USER, ttm will assume the obj as kernel buffer and 
will not allocate fake offset which I needed for fb_mmap callback to work.

Regards,
Nirmoy

>
>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_dpt.c           | 4 +++-
>>   drivers/gpu/drm/i915/display/intel_fbdev.c         | 3 ++-
>>   drivers/gpu/drm/i915/display/intel_plane_initial.c | 3 ++-
>>   3 files changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c b/drivers/gpu/drm/i915/display/intel_dpt.c
>> index ad1a37b515fb..2e6238881860 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dpt.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dpt.c
>> @@ -254,7 +254,9 @@ intel_dpt_create(struct intel_framebuffer *fb)
>>   
>>   	size = round_up(size * sizeof(gen8_pte_t), I915_GTT_PAGE_SIZE);
>>   
>> -	dpt_obj = i915_gem_object_create_lmem(i915, size, I915_BO_ALLOC_CONTIGUOUS);
>> +	dpt_obj = i915_gem_object_create_lmem(i915, size,
>> +					      I915_BO_ALLOC_CONTIGUOUS |
>> +					      I915_BO_ALLOC_USER);
>>   	if (IS_ERR(dpt_obj) && i915_ggtt_has_aperture(to_gt(i915)->ggtt))
>>   		dpt_obj = i915_gem_object_create_stolen(i915, size);
>>   	if (IS_ERR(dpt_obj) && !HAS_LMEM(i915)) {
>> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
>> index 3659350061a7..98ae3a3a986a 100644
>> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
>> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
>> @@ -163,7 +163,8 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
>>   	obj = ERR_PTR(-ENODEV);
>>   	if (HAS_LMEM(dev_priv)) {
>>   		obj = i915_gem_object_create_lmem(dev_priv, size,
>> -						  I915_BO_ALLOC_CONTIGUOUS);
>> +						  I915_BO_ALLOC_CONTIGUOUS |
>> +						  I915_BO_ALLOC_USER);
>>   	} else {
>>   		/*
>>   		 * If the FB is too big, just don't use it since fbdev is not very
>> diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c b/drivers/gpu/drm/i915/display/intel_plane_initial.c
>> index bb6ea7de5c61..4a3680f6a3f5 100644
>> --- a/drivers/gpu/drm/i915/display/intel_plane_initial.c
>> +++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c
>> @@ -110,7 +110,8 @@ initial_plane_vma(struct drm_i915_private *i915,
>>   	    size * 2 > i915->dsm.usable_size)
>>   		return NULL;
>>   
>> -	obj = i915_gem_object_create_region_at(mem, phys_base, size, 0);
>> +	obj = i915_gem_object_create_region_at(mem, phys_base, size,
>> +					       I915_BO_ALLOC_USER);
>>   	if (IS_ERR(obj))
>>   		return NULL;
>>   
>> -- 
>> 2.39.0

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-gfx] [PATCH 1/3] drm/i915: Set I915_BO_ALLOC_USER for framebuffer
  2023-03-06 16:22   ` Das, Nirmoy
@ 2023-03-06 17:30     ` Ville Syrjälä
  2023-03-07  7:20       ` Das, Nirmoy
  0 siblings, 1 reply; 12+ messages in thread
From: Ville Syrjälä @ 2023-03-06 17:30 UTC (permalink / raw)
  To: Das, Nirmoy; +Cc: intel-gfx, dri-devel

On Mon, Mar 06, 2023 at 05:22:19PM +0100, Das, Nirmoy wrote:
> 
> On 3/6/2023 3:21 PM, Ville Syrjälä wrote:
> > On Mon, Mar 06, 2023 at 11:28:48AM +0100, Nirmoy Das wrote:
> >> Framebuffer is exposed to userspace so set I915_BO_ALLOC_USER
> >> flag for it. This also make sure that ttm allocates offset
> >> for lmem objects.
> > I have no idea what that means.
> 
> Sorry for poor explanation.
> 
> Without I915_BO_ALLOC_USER, ttm will assume the obj as kernel buffer and 
> will not allocate fake offset which I needed for fb_mmap callback to work.

So that's the fake vm_pgoff thing? Doesn't that exist just so
mmap() through /dev/dri* can be passed a "gem handle"? 
With fbdev mmap we already know which BO we want to map so
why would any of that stuff even be needed?

> 
> Regards,
> Nirmoy
> 
> >
> >> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/display/intel_dpt.c           | 4 +++-
> >>   drivers/gpu/drm/i915/display/intel_fbdev.c         | 3 ++-
> >>   drivers/gpu/drm/i915/display/intel_plane_initial.c | 3 ++-
> >>   3 files changed, 7 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c b/drivers/gpu/drm/i915/display/intel_dpt.c
> >> index ad1a37b515fb..2e6238881860 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_dpt.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_dpt.c
> >> @@ -254,7 +254,9 @@ intel_dpt_create(struct intel_framebuffer *fb)
> >>   
> >>   	size = round_up(size * sizeof(gen8_pte_t), I915_GTT_PAGE_SIZE);
> >>   
> >> -	dpt_obj = i915_gem_object_create_lmem(i915, size, I915_BO_ALLOC_CONTIGUOUS);
> >> +	dpt_obj = i915_gem_object_create_lmem(i915, size,
> >> +					      I915_BO_ALLOC_CONTIGUOUS |
> >> +					      I915_BO_ALLOC_USER);
> >>   	if (IS_ERR(dpt_obj) && i915_ggtt_has_aperture(to_gt(i915)->ggtt))
> >>   		dpt_obj = i915_gem_object_create_stolen(i915, size);
> >>   	if (IS_ERR(dpt_obj) && !HAS_LMEM(i915)) {
> >> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
> >> index 3659350061a7..98ae3a3a986a 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> >> @@ -163,7 +163,8 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
> >>   	obj = ERR_PTR(-ENODEV);
> >>   	if (HAS_LMEM(dev_priv)) {
> >>   		obj = i915_gem_object_create_lmem(dev_priv, size,
> >> -						  I915_BO_ALLOC_CONTIGUOUS);
> >> +						  I915_BO_ALLOC_CONTIGUOUS |
> >> +						  I915_BO_ALLOC_USER);
> >>   	} else {
> >>   		/*
> >>   		 * If the FB is too big, just don't use it since fbdev is not very
> >> diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c b/drivers/gpu/drm/i915/display/intel_plane_initial.c
> >> index bb6ea7de5c61..4a3680f6a3f5 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_plane_initial.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c
> >> @@ -110,7 +110,8 @@ initial_plane_vma(struct drm_i915_private *i915,
> >>   	    size * 2 > i915->dsm.usable_size)
> >>   		return NULL;
> >>   
> >> -	obj = i915_gem_object_create_region_at(mem, phys_base, size, 0);
> >> +	obj = i915_gem_object_create_region_at(mem, phys_base, size,
> >> +					       I915_BO_ALLOC_USER);
> >>   	if (IS_ERR(obj))
> >>   		return NULL;
> >>   
> >> -- 
> >> 2.39.0

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-gfx] [PATCH 1/3] drm/i915: Set I915_BO_ALLOC_USER for framebuffer
  2023-03-06 17:30     ` Ville Syrjälä
@ 2023-03-07  7:20       ` Das, Nirmoy
  0 siblings, 0 replies; 12+ messages in thread
From: Das, Nirmoy @ 2023-03-07  7:20 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, dri-devel


On 3/6/2023 6:30 PM, Ville Syrjälä wrote:
> On Mon, Mar 06, 2023 at 05:22:19PM +0100, Das, Nirmoy wrote:
>> On 3/6/2023 3:21 PM, Ville Syrjälä wrote:
>>> On Mon, Mar 06, 2023 at 11:28:48AM +0100, Nirmoy Das wrote:
>>>> Framebuffer is exposed to userspace so set I915_BO_ALLOC_USER
>>>> flag for it. This also make sure that ttm allocates offset
>>>> for lmem objects.
>>> I have no idea what that means.
>> Sorry for poor explanation.
>>
>> Without I915_BO_ALLOC_USER, ttm will assume the obj as kernel buffer and
>> will not allocate fake offset which I needed for fb_mmap callback to work.
> So that's the fake vm_pgoff thing? Doesn't that exist just so
> mmap() through /dev/dri* can be passed a "gem handle"?
> With fbdev mmap we already know which BO we want to map so
> why would any of that stuff even be needed?


I was mainly concentrating on  using drm mmap API to achieve fb_mmap 
which eventually will call i915_gem_mmap()

and expects a  fake offset for the obj. I see your point: fb_mmap can be 
done without using drm mmap API which should be much simple . I will 
look into this and resend.


Thanks,

Nirmoy

>> Regards,
>> Nirmoy
>>
>>>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/display/intel_dpt.c           | 4 +++-
>>>>    drivers/gpu/drm/i915/display/intel_fbdev.c         | 3 ++-
>>>>    drivers/gpu/drm/i915/display/intel_plane_initial.c | 3 ++-
>>>>    3 files changed, 7 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c b/drivers/gpu/drm/i915/display/intel_dpt.c
>>>> index ad1a37b515fb..2e6238881860 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_dpt.c
>>>> +++ b/drivers/gpu/drm/i915/display/intel_dpt.c
>>>> @@ -254,7 +254,9 @@ intel_dpt_create(struct intel_framebuffer *fb)
>>>>    
>>>>    	size = round_up(size * sizeof(gen8_pte_t), I915_GTT_PAGE_SIZE);
>>>>    
>>>> -	dpt_obj = i915_gem_object_create_lmem(i915, size, I915_BO_ALLOC_CONTIGUOUS);
>>>> +	dpt_obj = i915_gem_object_create_lmem(i915, size,
>>>> +					      I915_BO_ALLOC_CONTIGUOUS |
>>>> +					      I915_BO_ALLOC_USER);
>>>>    	if (IS_ERR(dpt_obj) && i915_ggtt_has_aperture(to_gt(i915)->ggtt))
>>>>    		dpt_obj = i915_gem_object_create_stolen(i915, size);
>>>>    	if (IS_ERR(dpt_obj) && !HAS_LMEM(i915)) {
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
>>>> index 3659350061a7..98ae3a3a986a 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
>>>> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
>>>> @@ -163,7 +163,8 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
>>>>    	obj = ERR_PTR(-ENODEV);
>>>>    	if (HAS_LMEM(dev_priv)) {
>>>>    		obj = i915_gem_object_create_lmem(dev_priv, size,
>>>> -						  I915_BO_ALLOC_CONTIGUOUS);
>>>> +						  I915_BO_ALLOC_CONTIGUOUS |
>>>> +						  I915_BO_ALLOC_USER);
>>>>    	} else {
>>>>    		/*
>>>>    		 * If the FB is too big, just don't use it since fbdev is not very
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c b/drivers/gpu/drm/i915/display/intel_plane_initial.c
>>>> index bb6ea7de5c61..4a3680f6a3f5 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_plane_initial.c
>>>> +++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c
>>>> @@ -110,7 +110,8 @@ initial_plane_vma(struct drm_i915_private *i915,
>>>>    	    size * 2 > i915->dsm.usable_size)
>>>>    		return NULL;
>>>>    
>>>> -	obj = i915_gem_object_create_region_at(mem, phys_base, size, 0);
>>>> +	obj = i915_gem_object_create_region_at(mem, phys_base, size,
>>>> +					       I915_BO_ALLOC_USER);
>>>>    	if (IS_ERR(obj))
>>>>    		return NULL;
>>>>    
>>>> -- 
>>>> 2.39.0

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH RFC 3/3] drm/i915/display: Implement fb_mmap callback function
  2023-03-06 14:32   ` Ville Syrjälä
@ 2023-03-07 14:50     ` Das, Nirmoy
  0 siblings, 0 replies; 12+ messages in thread
From: Das, Nirmoy @ 2023-03-07 14:50 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, dri-devel

Hi Ville,

On 3/6/2023 3:32 PM, Ville Syrjälä wrote:
> On Mon, Mar 06, 2023 at 11:28:50AM +0100, Nirmoy Das wrote:
>> If stolen memory allocation fails for fbdev, the driver will
>> fallback to system memory. Calculation of smem_start is wrong
>> for such framebuffer objs if the platform comes with no gmadr or
>> no aperture. Solve this by adding fb_mmap callback which also gives
>> driver more control.
>>
>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_fbdev.c | 20 ++++++++++++++++++++
>>   1 file changed, 20 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
>> index 98ae3a3a986a..ed0f9e2af3ed 100644
>> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
>> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
>> @@ -40,8 +40,10 @@
>>   #include <drm/drm_crtc.h>
>>   #include <drm/drm_fb_helper.h>
>>   #include <drm/drm_fourcc.h>
>> +#include <drm/drm_gem_framebuffer_helper.h>
>>   
>>   #include "gem/i915_gem_lmem.h"
>> +#include "gem/i915_gem_mman.h"
>>   
>>   #include "i915_drv.h"
>>   #include "intel_display_types.h"
>> @@ -120,6 +122,23 @@ static int intel_fbdev_pan_display(struct fb_var_screeninfo *var,
>>   	return ret;
>>   }
>>   
>> +#define to_intel_fbdev(x) container_of(x, struct intel_fbdev, helper)
>> +static int intel_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma)
>> +{
>> +	struct intel_fbdev *fbdev = to_intel_fbdev(info->par);
>> +	struct drm_gem_object *bo = drm_gem_fb_get_obj(&fbdev->fb->base, 0);
>> +	struct drm_i915_gem_object *obj = to_intel_bo(bo);
>> +	struct drm_device *dev = fbdev->helper.dev;
> You seem to be missing the fb vs. mmio handling here entirely.


Could you please expand this more, I am not so familiar to fbdev code.


>
>> +
>> +	vma->vm_page_prot =
>> +			pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
> Does that do something sane on eg. !PAT?
>
>> +
>> +	if (obj->stolen)
>> +		return vm_iomap_memory(vma, info->fix.smem_start,
>> +				       info->fix.smem_len);
> Why doesn't i915_gem_object_mmap() know how to handle stolen?


Sent out another rfc series to address this.


Regards,

Nirmoy

>
>> +
>> +	return i915_gem_object_mmap(obj, vma);
>> +}
>>   static const struct fb_ops intelfb_ops = {
>>   	.owner = THIS_MODULE,
>>   	DRM_FB_HELPER_DEFAULT_OPS,
>> @@ -131,6 +150,7 @@ static const struct fb_ops intelfb_ops = {
>>   	.fb_imageblit = drm_fb_helper_cfb_imageblit,
>>   	.fb_pan_display = intel_fbdev_pan_display,
>>   	.fb_blank = intel_fbdev_blank,
>> +	.fb_mmap = intel_fbdev_mmap,
>>   };
>>   
>>   static int intelfb_alloc(struct drm_fb_helper *helper,
>> -- 
>> 2.39.0

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH RFC 3/3] drm/i915/display: Implement fb_mmap callback function
  2023-03-20 10:09 [PATCH v2: 1/3] drm/i915: Add a function to mmap framebuffer obj Nirmoy Das
@ 2023-03-20 10:09 ` Nirmoy Das
  0 siblings, 0 replies; 12+ messages in thread
From: Nirmoy Das @ 2023-03-20 10:09 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula, dri-devel, Matthew Auld, Andi Shyti, Nirmoy Das

If stolen memory allocation fails for fbdev, the driver will
fallback to system memory. Calculation of smem_start is wrong
for such framebuffer objs if the platform comes with no gmadr or
no aperture. Solve this by adding fb_mmap callback which will
use GTT if aperture is available otherwise will use cpu to access
the framebuffer.

v2: Use to_intel_fbdev() function(Jani)

Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Andi Shyti <andi.shyti@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_fbdev.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
index 8c3b3c3fd0e0..5e52bef868a0 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -40,8 +40,10 @@
 #include <drm/drm_crtc.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_fourcc.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 
 #include "gem/i915_gem_lmem.h"
+#include "gem/i915_gem_mman.h"
 
 #include "i915_drv.h"
 #include "intel_display_types.h"
@@ -119,6 +121,15 @@ static int intel_fbdev_pan_display(struct fb_var_screeninfo *var,
 	return ret;
 }
 
+static int intel_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma)
+{
+	struct intel_fbdev *fbdev = to_intel_fbdev(info->par);
+	struct drm_gem_object *bo = drm_gem_fb_get_obj(&fbdev->fb->base, 0);
+	struct drm_i915_gem_object *obj = to_intel_bo(bo);
+
+	return i915_gem_fb_mmap(obj, vma);
+}
+
 static const struct fb_ops intelfb_ops = {
 	.owner = THIS_MODULE,
 	DRM_FB_HELPER_DEFAULT_OPS,
@@ -130,6 +141,7 @@ static const struct fb_ops intelfb_ops = {
 	.fb_imageblit = drm_fb_helper_cfb_imageblit,
 	.fb_pan_display = intel_fbdev_pan_display,
 	.fb_blank = intel_fbdev_blank,
+	.fb_mmap = intel_fbdev_mmap,
 };
 
 static int intelfb_alloc(struct drm_fb_helper *helper,
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-03-20 10:09 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-06 10:28 [PATCH 1/3] drm/i915: Set I915_BO_ALLOC_USER for framebuffer Nirmoy Das
2023-03-06 10:28 ` [PATCH 2/3] drm/i915: Add a helper func for gem obj mmap Nirmoy Das
2023-03-06 14:26   ` Ville Syrjälä
2023-03-06 16:18     ` Das, Nirmoy
2023-03-06 10:28 ` [PATCH RFC 3/3] drm/i915/display: Implement fb_mmap callback function Nirmoy Das
2023-03-06 14:32   ` Ville Syrjälä
2023-03-07 14:50     ` Das, Nirmoy
2023-03-06 14:21 ` [Intel-gfx] [PATCH 1/3] drm/i915: Set I915_BO_ALLOC_USER for framebuffer Ville Syrjälä
2023-03-06 16:22   ` Das, Nirmoy
2023-03-06 17:30     ` Ville Syrjälä
2023-03-07  7:20       ` Das, Nirmoy
2023-03-20 10:09 [PATCH v2: 1/3] drm/i915: Add a function to mmap framebuffer obj Nirmoy Das
2023-03-20 10:09 ` [PATCH RFC 3/3] drm/i915/display: Implement fb_mmap callback function Nirmoy Das

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).