[PATCH v2 2/4] io-mapping: Specify mapping size for io_mapping_map_wc()

[PATCH v2 2/4] io-mapping: Specify mapping size for io_mapping_map_wc()

[Chris Wilson]

The ioremap() hidden behind the io_mapping_map_wc() convenience helper
can be used for remapping multiple pages. Extend the helper so that
future callers can use it for larger ranges.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Yishai Hadas <yishaih@mellanox.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: "Peter Zijlstra (Intel)" <peterz@infradead.org>
Cc: David Hildenbrand <dahi@linux.vnet.ibm.com>
Cc: intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Cc: netdev@vger.kernel.org
Cc: linux-rdma@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/gpu/drm/i915/intel_overlay.c    |  3 ++-
 drivers/net/ethernet/mellanox/mlx4/pd.c |  4 +++-
 include/linux/io-mapping.h              | 10 +++++++---
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index 9746b9841c13..0d5a376878d3 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -198,7 +198,8 @@ intel_overlay_map_regs(struct intel_overlay *overlay)
 		regs = (struct overlay_registers __iomem *)overlay->reg_bo->phys_handle->vaddr;
 	else
 		regs = io_mapping_map_wc(ggtt->mappable,
-					 overlay->flip_addr);
+					 overlay->flip_addr,
+					 PAGE_SIZE);
 
 	return regs;
 }
diff --git a/drivers/net/ethernet/mellanox/mlx4/pd.c b/drivers/net/ethernet/mellanox/mlx4/pd.c
index b3cc3ab63799..6fc156a3918d 100644
--- a/drivers/net/ethernet/mellanox/mlx4/pd.c
+++ b/drivers/net/ethernet/mellanox/mlx4/pd.c
@@ -205,7 +205,9 @@ int mlx4_bf_alloc(struct mlx4_dev *dev, struct mlx4_bf *bf, int node)
 			goto free_uar;
 		}
 
-		uar->bf_map = io_mapping_map_wc(priv->bf_mapping, uar->index << PAGE_SHIFT);
+		uar->bf_map = io_mapping_map_wc(priv->bf_mapping,
+						uar->index << PAGE_SHIFT,
+						PAGE_SIZE);
 		if (!uar->bf_map) {
 			err = -ENOMEM;
 			goto unamp_uar;
diff --git a/include/linux/io-mapping.h b/include/linux/io-mapping.h
index e399029b68c5..645ad06b5d52 100644
--- a/include/linux/io-mapping.h
+++ b/include/linux/io-mapping.h
@@ -100,14 +100,16 @@ io_mapping_unmap_atomic(void __iomem *vaddr)
 }
 
 static inline void __iomem *
-io_mapping_map_wc(struct io_mapping *mapping, unsigned long offset)
+io_mapping_map_wc(struct io_mapping *mapping,
+		  unsigned long offset,
+		  unsigned long size)
 {
 	resource_size_t phys_addr;
 
 	BUG_ON(offset >= mapping->size);
 	phys_addr = mapping->base + offset;
 
-	return ioremap_wc(phys_addr, PAGE_SIZE);
+	return ioremap_wc(phys_addr, size);
 }
 
 static inline void
@@ -155,7 +157,9 @@ io_mapping_unmap_atomic(void __iomem *vaddr)
 
 /* Non-atomic map/unmap */
 static inline void __iomem *
-io_mapping_map_wc(struct io_mapping *mapping, unsigned long offset)
+io_mapping_map_wc(struct io_mapping *mapping,
+		  unsigned long offset,
+		  unsigned long size)
 {
 	return ((char __force __iomem *) mapping) + offset;
 }
-- 
2.8.0.rc3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/4] io-mapping: Specify mapping size for io_mapping_map_wc()

[Chris Wilson]

On Mon, Apr 18, 2016 at 12:14:00PM +0100, Chris Wilson wrote:
> The ioremap() hidden behind the io_mapping_map_wc() convenience helper
> can be used for remapping multiple pages. Extend the helper so that
> future callers can use it for larger ranges.

Adding Luis Rodriguez as he has been active in the area of ioremap_*().

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Yishai Hadas <yishaih@mellanox.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: "Peter Zijlstra (Intel)" <peterz@infradead.org>
> Cc: David Hildenbrand <dahi@linux.vnet.ibm.com>
Cc: Luis R. Rodriguez <mcgrof@kernel.org>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: netdev@vger.kernel.org
> Cc: linux-rdma@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  drivers/gpu/drm/i915/intel_overlay.c    |  3 ++-
>  drivers/net/ethernet/mellanox/mlx4/pd.c |  4 +++-
>  include/linux/io-mapping.h              | 10 +++++++---
>  3 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
> index 9746b9841c13..0d5a376878d3 100644
> --- a/drivers/gpu/drm/i915/intel_overlay.c
> +++ b/drivers/gpu/drm/i915/intel_overlay.c
> @@ -198,7 +198,8 @@ intel_overlay_map_regs(struct intel_overlay *overlay)
>  		regs = (struct overlay_registers __iomem *)overlay->reg_bo->phys_handle->vaddr;
>  	else
>  		regs = io_mapping_map_wc(ggtt->mappable,
> -					 overlay->flip_addr);
> +					 overlay->flip_addr,
> +					 PAGE_SIZE);
>  
>  	return regs;
>  }
> diff --git a/drivers/net/ethernet/mellanox/mlx4/pd.c b/drivers/net/ethernet/mellanox/mlx4/pd.c
> index b3cc3ab63799..6fc156a3918d 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/pd.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/pd.c
> @@ -205,7 +205,9 @@ int mlx4_bf_alloc(struct mlx4_dev *dev, struct mlx4_bf *bf, int node)
>  			goto free_uar;
>  		}
>  
> -		uar->bf_map = io_mapping_map_wc(priv->bf_mapping, uar->index << PAGE_SHIFT);
> +		uar->bf_map = io_mapping_map_wc(priv->bf_mapping,
> +						uar->index << PAGE_SHIFT,
> +						PAGE_SIZE);
>  		if (!uar->bf_map) {
>  			err = -ENOMEM;
>  			goto unamp_uar;
> diff --git a/include/linux/io-mapping.h b/include/linux/io-mapping.h
> index e399029b68c5..645ad06b5d52 100644
> --- a/include/linux/io-mapping.h
> +++ b/include/linux/io-mapping.h
> @@ -100,14 +100,16 @@ io_mapping_unmap_atomic(void __iomem *vaddr)
>  }
>  
>  static inline void __iomem *
> -io_mapping_map_wc(struct io_mapping *mapping, unsigned long offset)
> +io_mapping_map_wc(struct io_mapping *mapping,
> +		  unsigned long offset,
> +		  unsigned long size)
>  {
>  	resource_size_t phys_addr;
>  
>  	BUG_ON(offset >= mapping->size);
>  	phys_addr = mapping->base + offset;
>  
> -	return ioremap_wc(phys_addr, PAGE_SIZE);
> +	return ioremap_wc(phys_addr, size);
>  }
>  
>  static inline void
> @@ -155,7 +157,9 @@ io_mapping_unmap_atomic(void __iomem *vaddr)
>  
>  /* Non-atomic map/unmap */
>  static inline void __iomem *
> -io_mapping_map_wc(struct io_mapping *mapping, unsigned long offset)
> +io_mapping_map_wc(struct io_mapping *mapping,
> +		  unsigned long offset,
> +		  unsigned long size)
>  {
>  	return ((char __force __iomem *) mapping) + offset;
>  }

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH v2 2/4] io-mapping: Specify mapping size for io_mapping_map_wc()

[Luis R. Rodriguez]

On Tue, Apr 19, 2016 at 01:02:25PM +0100, Chris Wilson wrote:
> On Mon, Apr 18, 2016 at 12:14:00PM +0100, Chris Wilson wrote:
> > The ioremap() hidden behind the io_mapping_map_wc() convenience helper
> > can be used for remapping multiple pages. Extend the helper so that
> > future callers can use it for larger ranges.
> 
> Adding Luis Rodriguez as he has been active in the area of ioremap_*().

Can someone bounce me a copy of the original e-mails? Can you also use
mcgrof@kernel.org ?

  Luis

Re: [PATCH v2 2/4] io-mapping: Specify mapping size for io_mapping_map_wc()

[Chris Wilson]

On Tue, Apr 19, 2016 at 02:30:28PM +0200, Luis R. Rodriguez wrote:
> On Tue, Apr 19, 2016 at 01:02:25PM +0100, Chris Wilson wrote:
> > On Mon, Apr 18, 2016 at 12:14:00PM +0100, Chris Wilson wrote:
> > > The ioremap() hidden behind the io_mapping_map_wc() convenience helper
> > > can be used for remapping multiple pages. Extend the helper so that
> > > future callers can use it for larger ranges.
> > 
> > Adding Luis Rodriguez as he has been active in the area of ioremap_*().
> 
> Can someone bounce me a copy of the original e-mails? Can you also use
> mcgrof@kernel.org ?

Hopefully done. Please ping me if they don't arrive, or have a look at
https://cgit.freedesktop.org/~ickle/linux-2.6/log/?h=iomap
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/4] drm/i915: Move ioremap_wc tracking onto VMA

[Luis R. Rodriguez]

On Tue, Apr 19, 2016 at 01:33:58PM +0100, Chris Wilson wrote:
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 6ce2c31b9a81..9ef47329e8ae 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3346,6 +3346,15 @@ static void i915_gem_object_finish_gtt(struct drm_i915_gem_object *obj)
>  					    old_write_domain);
>  }
>  
> +static void __i915_vma_iounmap(struct i915_vma *vma)
> +{
> +	if (vma->iomap == NULL)
> +		return;
> +
> +	io_mapping_unmap(vma->iomap);

The NULL check could just be done by io_mapping_unmap() then you
can avoid this in other drivers too.

> +	vma->iomap = NULL;

You added accounting here, by simple int and inc / dec'ing it.
I cannot confirm if it is correctly avoiding races, can you
confirm?

Also you added accounting for the custom vma pinning thing and do
GEM_BUG_ON(vma->pin_count == 0); when you unpin one instance but *you do not*
do something like GEM_BUG_ON(vma->pin_count != 0); when you do the final full
iounmap. That seems rather sloppy.

iomapping stuff has its own custom data structure, why not just use that data
structure instead of the struct i915_vma and generalize this ? Drivers can
be buggy and best if we avoid custom driver accounting and just do it in a neat
generic fashion.

Then other drivers could use this too.

> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index 79ac202f3870..93f54a10042f 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -244,22 +245,23 @@ static int intelfb_create(struct drm_fb_helper *helper,
>  	info->flags = FBINFO_DEFAULT | FBINFO_CAN_FORCE_OUTPUT;
>  	info->fbops = &intelfb_ops;
>  
> +	vma = i915_gem_obj_to_ggtt(obj);
> +
>  	/* setup aperture base/size for vesafb takeover */
>  	info->apertures->ranges[0].base = dev->mode_config.fb_base;
>  	info->apertures->ranges[0].size = ggtt->mappable_end;
>  
> -	info->fix.smem_start = dev->mode_config.fb_base + i915_gem_obj_ggtt_offset(obj);
> -	info->fix.smem_len = size;
> +	info->fix.smem_start = dev->mode_config.fb_base + vma->node.start;
> +	info->fix.smem_len = vma->node.size;
>  
> -	info->screen_base =
> -		ioremap_wc(ggtt->mappable_base + i915_gem_obj_ggtt_offset(obj),
> -			   size);
> -	if (!info->screen_base) {
> +	vaddr = i915_vma_pin_iomap(vma);
> +	if (IS_ERR(vaddr)) {
>  		DRM_ERROR("Failed to remap framebuffer into virtual memory\n");
> -		ret = -ENOSPC;
> +		ret = PTR_ERR(vaddr);
>  		goto out_destroy_fbi;
>  	}
> -	info->screen_size = size;
> +	info->screen_base = vaddr;
> +	info->screen_size = vma->node.size;

some framebuffer drivers tend to use a generic start address of
iinfo->fix.smem_start and a length of info->fix.smem_len, this
driver sets the smem_start above, but its different than what
gets ioremap for a start address:

+               ptr = io_mapping_map_wc(i915_vm_to_ggtt(vma->vm)->mappable,
+                                       vma->node.start,
+                                       vma->node.size);

fix.smem_start is :


> +	info->fix.smem_start = dev->mode_config.fb_base + vma->node.start;

The smem_len matches though. Can you clarify if its correct for
the io_mapping_map_wc() should not be using info->fix.smem_start
(which is dev->mode_config.fb_base + vma->node.start)?

Reason I ask is since I noticed a while ago a lot of drivers
were using info->fix.smem_start and info->fix.smem_len consistently
for their ioremap'd areas it might make sense instead to let the
internal framebuffer (register_framebuffer()) optionally manage the
ioremap_wc() for drivers, given that this is pretty generic stuff.

  Luis
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/4] drm/i915: Move ioremap_wc tracking onto VMA

[Chris Wilson]

On Wed, Apr 20, 2016 at 11:10:54AM +0200, Luis R. Rodriguez wrote:
> On Tue, Apr 19, 2016 at 01:33:58PM +0100, Chris Wilson wrote:
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index 6ce2c31b9a81..9ef47329e8ae 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -3346,6 +3346,15 @@ static void i915_gem_object_finish_gtt(struct drm_i915_gem_object *obj)
> >  					    old_write_domain);
> >  }
> >  
> > +static void __i915_vma_iounmap(struct i915_vma *vma)
> > +{
> > +	if (vma->iomap == NULL)
> > +		return;
> > +
> > +	io_mapping_unmap(vma->iomap);
> 
> The NULL check could just be done by io_mapping_unmap() then you
> can avoid this in other drivers too.
> 
> > +	vma->iomap = NULL;
> 
> You added accounting here, by simple int and inc / dec'ing it.
> I cannot confirm if it is correctly avoiding races, can you
> confirm?

Yes, the vma->pin_count is guarded by the struct_mutex atm. (The
struct_mutex is our own BKL :(

> Also you added accounting for the custom vma pinning thing and do
> GEM_BUG_ON(vma->pin_count == 0); when you unpin one instance but *you do not*
> do something like GEM_BUG_ON(vma->pin_count != 0); when you do the final full
> iounmap. That seems rather sloppy.

It's placed next to the function where pin_count == 0 and only called
from it. Yes, I did think the same...
 
> iomapping stuff has its own custom data structure, why not just use that data
> structure instead of the struct i915_vma and generalize this ? Drivers can
> be buggy and best if we avoid custom driver accounting and just do it in a neat
> generic fashion.

Completely different tasks, as far as I am aware. The iomapping is about
providing CPU access to the IO region, dma-remapping about providing
device access to physical memory, and our own VMA is about how the
object sits in all the different views of both CPU and device address
spaces (of which there are many, and even the CPU accessible address
space is not the entirety of that particular address space). 
 
> Then other drivers could use this too.

drivers/gpu/drm/ttm (you didn't hear me say that...)

> > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> > index 79ac202f3870..93f54a10042f 100644
> > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > @@ -244,22 +245,23 @@ static int intelfb_create(struct drm_fb_helper *helper,
> >  	info->flags = FBINFO_DEFAULT | FBINFO_CAN_FORCE_OUTPUT;
> >  	info->fbops = &intelfb_ops;
> >  
> > +	vma = i915_gem_obj_to_ggtt(obj);
> > +
> >  	/* setup aperture base/size for vesafb takeover */
> >  	info->apertures->ranges[0].base = dev->mode_config.fb_base;
> >  	info->apertures->ranges[0].size = ggtt->mappable_end;
> >  
> > -	info->fix.smem_start = dev->mode_config.fb_base + i915_gem_obj_ggtt_offset(obj);
> > -	info->fix.smem_len = size;
> > +	info->fix.smem_start = dev->mode_config.fb_base + vma->node.start;
> > +	info->fix.smem_len = vma->node.size;
> >  
> > -	info->screen_base =
> > -		ioremap_wc(ggtt->mappable_base + i915_gem_obj_ggtt_offset(obj),
> > -			   size);
> > -	if (!info->screen_base) {
> > +	vaddr = i915_vma_pin_iomap(vma);
> > +	if (IS_ERR(vaddr)) {
> >  		DRM_ERROR("Failed to remap framebuffer into virtual memory\n");
> > -		ret = -ENOSPC;
> > +		ret = PTR_ERR(vaddr);
> >  		goto out_destroy_fbi;
> >  	}
> > -	info->screen_size = size;
> > +	info->screen_base = vaddr;
> > +	info->screen_size = vma->node.size;
> 
> some framebuffer drivers tend to use a generic start address of
> iinfo->fix.smem_start and a length of info->fix.smem_len, this
> driver sets the smem_start above, but its different than what
> gets ioremap for a start address:
> 
> +               ptr = io_mapping_map_wc(i915_vm_to_ggtt(vma->vm)->mappable,
> +                                       vma->node.start,
> +                                       vma->node.size);
> 
> fix.smem_start is :
> 
> 
> > +	info->fix.smem_start = dev->mode_config.fb_base + vma->node.start;
> 
> The smem_len matches though. Can you clarify if its correct for
> the io_mapping_map_wc() should not be using info->fix.smem_start
> (which is dev->mode_config.fb_base + vma->node.start)?

dev->mode_config.fb_base is the base address of the mappable region. It
is an inconsistently in naming that just hasn't annoyed me enough to
fix.
 
> Reason I ask is since I noticed a while ago a lot of drivers
> were using info->fix.smem_start and info->fix.smem_len consistently
> for their ioremap'd areas it might make sense instead to let the
> internal framebuffer (register_framebuffer()) optionally manage the
> ioremap_wc() for drivers, given that this is pretty generic stuff.

Apart from drivers like ours we would end up with multiple mappings to
the same region. It was just a little grevience that I think was worth
fixing. It does highlight how buggy our code is currently though as we
never relinquish that mapping when the driver is unloaded.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 4/4] drm/i915: Move ioremap_wc tracking onto VMA

[Daniel Vetter]

On Wed, Apr 20, 2016 at 11:10:54AM +0200, Luis R. Rodriguez wrote:
> Reason I ask is since I noticed a while ago a lot of drivers
> were using info->fix.smem_start and info->fix.smem_len consistently
> for their ioremap'd areas it might make sense instead to let the
> internal framebuffer (register_framebuffer()) optionally manage the
> ioremap_wc() for drivers, given that this is pretty generic stuff.

All that legacy fbdev stuff is just for legacy support, and I prefer to
have that as dumb as possible. There's been some discussion even around
lifting the "kick out firmware fb driver" out of fbdev, since we'd need it
to have a simple drm driver for e.g. uefi.

But I definitely don't want a legacy horror show like fbdev to
automagically take care of device mappings for drivers.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 4/4] drm/i915: Move ioremap_wc tracking onto VMA

[Luis R. Rodriguez]

On Wed, Apr 20, 2016 at 01:17:30PM +0200, Daniel Vetter wrote:
> On Wed, Apr 20, 2016 at 11:10:54AM +0200, Luis R. Rodriguez wrote:
> > Reason I ask is since I noticed a while ago a lot of drivers
> > were using info->fix.smem_start and info->fix.smem_len consistently
> > for their ioremap'd areas it might make sense instead to let the
> > internal framebuffer (register_framebuffer()) optionally manage the
> > ioremap_wc() for drivers, given that this is pretty generic stuff.
> 
> All that legacy fbdev stuff is just for legacy support, and I prefer to
> have that as dumb as possible. There's been some discussion even around
> lifting the "kick out firmware fb driver" out of fbdev, since we'd need it
> to have a simple drm driver for e.g. uefi.
> 
> But I definitely don't want a legacy horror show like fbdev to
> automagically take care of device mappings for drivers.

Makes sense, it also still begs the question if more modern APIs
could manage the ioremap for you. Evidence shows people get
sloppy and if things were done internally with helpers it may
be easier to later make adjustments.

  Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] drm/i915: Move ioremap_wc tracking onto VMA

[Daniel Vetter]

On Wed, Apr 20, 2016 at 11:27:27PM +0200, Luis R. Rodriguez wrote:
> On Wed, Apr 20, 2016 at 01:17:30PM +0200, Daniel Vetter wrote:
> > On Wed, Apr 20, 2016 at 11:10:54AM +0200, Luis R. Rodriguez wrote:
> > > Reason I ask is since I noticed a while ago a lot of drivers
> > > were using info->fix.smem_start and info->fix.smem_len consistently
> > > for their ioremap'd areas it might make sense instead to let the
> > > internal framebuffer (register_framebuffer()) optionally manage the
> > > ioremap_wc() for drivers, given that this is pretty generic stuff.
> > 
> > All that legacy fbdev stuff is just for legacy support, and I prefer to
> > have that as dumb as possible. There's been some discussion even around
> > lifting the "kick out firmware fb driver" out of fbdev, since we'd need it
> > to have a simple drm driver for e.g. uefi.
> > 
> > But I definitely don't want a legacy horror show like fbdev to
> > automagically take care of device mappings for drivers.
> 
> Makes sense, it also still begs the question if more modern APIs
> could manage the ioremap for you. Evidence shows people get
> sloppy and if things were done internally with helpers it may
> be easier to later make adjustments.

Real gpus generally have so much mmio space that you want to ioremap them
on demand. At least if you still care about 32bit support. And on-die gpus
on socs or similar tend to not have an mmio range to access the gfx
remapping range at all, but instead expect that to be done with gpu
pagetables.

So at least with gpus I don't see a real demand for this, and the existing
users are mostly old fbdev drivers that really no one should be touching
;-)

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx