diff mbox series

[1/3] drm/i915: Set I915_BO_ALLOC_USER for framebuffer

Message ID 20230306102850.18299-1-nirmoy.das@intel.com (mailing list archive)
State New, archived
Headers show
Series [1/3] drm/i915: Set I915_BO_ALLOC_USER for framebuffer | expand

Commit Message

Das, Nirmoy March 6, 2023, 10:28 a.m. UTC
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(-)

Comments

Ville Syrjälä March 6, 2023, 2:21 p.m. UTC | #1
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
Das, Nirmoy March 6, 2023, 4:22 p.m. UTC | #2
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
Ville Syrjälä March 6, 2023, 5:30 p.m. UTC | #3
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
Das, Nirmoy March 7, 2023, 7:20 a.m. UTC | #4
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
diff mbox series

Patch

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;