Re: [Intel-gfx] [PATCH 1/3] drm/i915: Set I915_BO_ALLOC_USER for framebuffer

2023-03-06 Thread Das, Nirmoy 



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 
---
   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

Re: [Intel-gfx] [PATCH 1/3] drm/i915: Set I915_BO_ALLOC_USER for framebuffer

2023-03-06 Thread Ville Syrjälä 
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 
> >> ---
> >>   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

Re: [Intel-gfx] [PATCH 1/3] drm/i915: Set I915_BO_ALLOC_USER for framebuffer

2023-03-06 Thread Das, Nirmoy 



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 
---
  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

Re: [Intel-gfx] [PATCH 1/3] drm/i915: Set I915_BO_ALLOC_USER for framebuffer

2023-03-06 Thread Ville Syrjälä 
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 
> ---
>  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

Re: [Intel-gfx] [PATCH 1/3] drm/i915: Set I915_BO_ALLOC_USER for framebuffer

2023-03-06 Thread Das, Nirmoy 



On 3/6/2023 2:49 PM, Matthew Auld wrote:

On 06/03/2023 13:31, Das, Nirmoy wrote:

Hi Matt,

On 3/6/2023 1:25 PM, Matthew Auld wrote:

On 06/03/2023 12:07, 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.

Signed-off-by: Nirmoy Das 
---
  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);


AFAICT this is just some driver internal stuff for display 
page-table, which gets mapped through GGTT or something, and is not 
the actual fb. Is it really exposed to the user?



I misunderstood this for something else. I will remove this.



  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);


ALLOC_USER has the side effect of also zeroing the memory 
underneath, IIRC. However this here is the pre-allocated fb (will 
have some boot logo stuff), so we shouldn't ever clear it.



This was my concern.  I wonder if there is any other better way than 
to use a temp buffer to copy the pre-allocated content and put it 
back after getting i915_gem_object_create_region_at().


If we need ALLOC_USER for this buffer then maybe just a new flag like 
BO_PREALLOCATED which skips all the clearing?



Sounds good, I will look into that.


Thanks,

Nirmoy






Regards,

Nirmoy





  if (IS_ERR(obj))
  return NULL;

Re: [Intel-gfx] [PATCH 1/3] drm/i915: Set I915_BO_ALLOC_USER for framebuffer

2023-03-06 Thread Matthew Auld 

On 06/03/2023 13:31, Das, Nirmoy wrote:

Hi Matt,

On 3/6/2023 1:25 PM, Matthew Auld wrote:

On 06/03/2023 12:07, 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.

Signed-off-by: Nirmoy Das 
---
  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);


AFAICT this is just some driver internal stuff for display page-table, 
which gets mapped through GGTT or something, and is not the actual fb. 
Is it really exposed to the user?



I misunderstood this for something else. I will remove this.




  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);


ALLOC_USER has the side effect of also zeroing the memory underneath, 
IIRC. However this here is the pre-allocated fb (will have some boot 
logo stuff), so we shouldn't ever clear it.



This was my concern.  I wonder if there is any other better way than to 
use a temp buffer to copy the pre-allocated content and put it back 
after getting i915_gem_object_create_region_at().


If we need ALLOC_USER for this buffer then maybe just a new flag like 
BO_PREALLOCATED which skips all the clearing?





Regards,

Nirmoy





  if (IS_ERR(obj))
  return NULL;

Re: [Intel-gfx] [PATCH 1/3] drm/i915: Set I915_BO_ALLOC_USER for framebuffer

2023-03-06 Thread Das, Nirmoy 

Hi Matt,

On 3/6/2023 1:25 PM, Matthew Auld wrote:

On 06/03/2023 12:07, 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.

Signed-off-by: Nirmoy Das 
---
  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);


AFAICT this is just some driver internal stuff for display page-table, 
which gets mapped through GGTT or something, and is not the actual fb. 
Is it really exposed to the user?



I misunderstood this for something else. I will remove this.




  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);


ALLOC_USER has the side effect of also zeroing the memory underneath, 
IIRC. However this here is the pre-allocated fb (will have some boot 
logo stuff), so we shouldn't ever clear it.



This was my concern.  I wonder if there is any other better way than to 
use a temp buffer to copy the pre-allocated content and put it back 
after getting i915_gem_object_create_region_at().



Regards,

Nirmoy





  if (IS_ERR(obj))
  return NULL;