Re: [PATCH v15 6/7] ext4: disable map_sync for async flush

2019-07-07 Thread Theodore Ts'o
On Fri, Jul 05, 2019 at 07:33:27PM +0530, Pankaj Gupta wrote:
> Dont support 'MAP_SYNC' with non-DAX files and DAX files
> with asynchronous dax_device. Virtio pmem provides
> asynchronous host page cache flush mechanism. We don't
> support 'MAP_SYNC' with virtio pmem and ext4.
> 
> Signed-off-by: Pankaj Gupta 
> Reviewed-by: Jan Kara 

Acked-by: Theodore Ts'o 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v2 3/6] drm/fb-helper: Instanciate shadow FB if configured in device's mode_config

2019-07-07 Thread Noralf Trønnes


Den 05.07.2019 11.26, skrev Thomas Zimmermann:
> Generic framebuffer emulation uses a shadow buffer for framebuffers with
> dirty() function. If drivers want to use the shadow FB without such a
> function, they can now set prefer_shadow or prefer_shadow_fbdev in their
> mode_config structures. The former flag is exported to userspace, the latter
> flag is fbdev-only.
> 
> Signed-off-by: Thomas Zimmermann 
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 19 ++-
>  include/drm/drm_mode_config.h   |  5 +
>  2 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 7ba6a0255821..56ef169e1814 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -421,7 +421,9 @@ static void drm_fb_helper_dirty_work(struct work_struct 
> *work)
>   return;
>   drm_fb_helper_dirty_blit_real(helper, _copy);
>   }
> - helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, _copy, 1);
> + if (helper->fb->funcs->dirty)
> + helper->fb->funcs->dirty(helper->fb, NULL, 0, 0,
> +  _copy, 1);
>  
>   if (helper->buffer)
>   drm_client_buffer_vunmap(helper->buffer);
> @@ -620,9 +622,6 @@ static void drm_fb_helper_dirty(struct fb_info *info, u32 
> x, u32 y,
>   struct drm_clip_rect *clip = >dirty_clip;
>   unsigned long flags;
>  
> - if (!helper->fb->funcs->dirty)
> - return;

drm_fb_helper_dirty() is called unconditionally by
drm_fb_helper_sys_imageblit() et al, so we need check with
drm_fbdev_use_shadow_fb() here.

> -
>   spin_lock_irqsave(>dirty_lock, flags);
>   clip->x1 = min_t(u32, clip->x1, x);
>   clip->y1 = min_t(u32, clip->y1, y);
> @@ -2166,6 +2165,16 @@ static struct fb_deferred_io drm_fbdev_defio = {
>   .deferred_io= drm_fb_helper_deferred_io,
>  };
>  
> +static bool drm_fbdev_use_shadow_fb(struct drm_fb_helper *fb_helper)
> +{
> + struct drm_device *dev = fb_helper->dev;
> + struct drm_framebuffer *fb = fb_helper->fb;
> +
> + return dev->mode_config.prefer_shadow_fbdev |
> +dev->mode_config.prefer_shadow |

Use logical OR here

> +!!fb->funcs->dirty;

and you can drop the the double NOT here.

> +}
> +
>  /**
>   * drm_fb_helper_generic_probe - Generic fbdev emulation probe helper
>   * @fb_helper: fbdev helper structure
> @@ -2213,7 +,7 @@ int drm_fb_helper_generic_probe(struct drm_fb_helper 
> *fb_helper,
>  
>   drm_fb_helper_fill_info(fbi, fb_helper, sizes);
>  
> - if (fb->funcs->dirty) {
> + if (drm_fbdev_use_shadow_fb(fb_helper)) {
>   struct fb_ops *fbops;
>   void *shadow;
>  
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 759d462d028b..e1c751aca353 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -347,6 +347,8 @@ struct drm_mode_config_funcs {
>   * @output_poll_work: delayed work for polling in process context
>   * @preferred_depth: preferred RBG pixel depth, used by fb helpers
>   * @prefer_shadow: hint to userspace to prefer shadow-fb rendering
> + * @prefer_shadow_fbdev: hint to framebuffer emulation to prefer shadow-fb \
> + rendering

It's preferred to have the doc together with the struct member. This way
it's less likely to be forgotten when things change. And we don't use
line cont. when the doc line is too long. Just continue on the next line
after an asterix.

>   * @cursor_width: hint to userspace for max cursor width
>   * @cursor_height: hint to userspace for max cursor height
>   * @helper_private: mid-layer private data
> @@ -852,6 +854,9 @@ struct drm_mode_config {
>   /* dumb ioctl parameters */
>   uint32_t preferred_depth, prefer_shadow;
>  
> + /* fbdev parameters */

No need for this comment.

Doc can look like this, I've done s/framebuffer/fbdev/:
/**
 * @prefer_shadow_fbdev:
 *
 * Hint to fbdev emulation to prefer shadow-fb rendering.
 */

> + uint32_t prefer_shadow_fbdev;

Use bool here.

With that:

Reviewed-by: Noralf Trønnes 

I have tested this on 2 drivers that use generic fbdev: vc4 (no shadow
buf) and mi0283qt which has a dirty callback.

Tested-by: Noralf Trønnes 

> +
>   /**
>* @quirk_addfb_prefer_xbgr_30bpp:
>*
> 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 3/6] drm/fb-helper: Instanciate shadow FB if configured in device's mode_config

2019-07-07 Thread Thomas Zimmermann
Hi

Am 07.07.19 um 16:37 schrieb Noralf Trønnes:
> 
> 
> Den 05.07.2019 11.26, skrev Thomas Zimmermann:
>> Generic framebuffer emulation uses a shadow buffer for framebuffers with
>> dirty() function. If drivers want to use the shadow FB without such a
>> function, they can now set prefer_shadow or prefer_shadow_fbdev in their
>> mode_config structures. The former flag is exported to userspace, the latter
>> flag is fbdev-only.
>>
>> Signed-off-by: Thomas Zimmermann 
>> ---
>>  drivers/gpu/drm/drm_fb_helper.c | 19 ++-
>>  include/drm/drm_mode_config.h   |  5 +
>>  2 files changed, 19 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c 
>> b/drivers/gpu/drm/drm_fb_helper.c
>> index 7ba6a0255821..56ef169e1814 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -421,7 +421,9 @@ static void drm_fb_helper_dirty_work(struct work_struct 
>> *work)
>>  return;
>>  drm_fb_helper_dirty_blit_real(helper, _copy);
>>  }
>> -helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, _copy, 1);
>> +if (helper->fb->funcs->dirty)
>> +helper->fb->funcs->dirty(helper->fb, NULL, 0, 0,
>> + _copy, 1);
>>  
>>  if (helper->buffer)
>>  drm_client_buffer_vunmap(helper->buffer);
>> @@ -620,9 +622,6 @@ static void drm_fb_helper_dirty(struct fb_info *info, 
>> u32 x, u32 y,
>>  struct drm_clip_rect *clip = >dirty_clip;
>>  unsigned long flags;
>>  
>> -if (!helper->fb->funcs->dirty)
>> -return;
> 
> drm_fb_helper_dirty() is called unconditionally by
> drm_fb_helper_sys_imageblit() et al, so we need check with
> drm_fbdev_use_shadow_fb() here.
> 
>> -
>>  spin_lock_irqsave(>dirty_lock, flags);
>>  clip->x1 = min_t(u32, clip->x1, x);
>>  clip->y1 = min_t(u32, clip->y1, y);
>> @@ -2166,6 +2165,16 @@ static struct fb_deferred_io drm_fbdev_defio = {
>>  .deferred_io= drm_fb_helper_deferred_io,
>>  };
>>  
>> +static bool drm_fbdev_use_shadow_fb(struct drm_fb_helper *fb_helper)
>> +{
>> +struct drm_device *dev = fb_helper->dev;
>> +struct drm_framebuffer *fb = fb_helper->fb;
>> +
>> +return dev->mode_config.prefer_shadow_fbdev |
>> +   dev->mode_config.prefer_shadow |
> 
> Use logical OR here
> 
>> +   !!fb->funcs->dirty;
> 
> and you can drop the the double NOT here.
> 
>> +}
>> +
>>  /**
>>   * drm_fb_helper_generic_probe - Generic fbdev emulation probe helper
>>   * @fb_helper: fbdev helper structure
>> @@ -2213,7 +,7 @@ int drm_fb_helper_generic_probe(struct drm_fb_helper 
>> *fb_helper,
>>  
>>  drm_fb_helper_fill_info(fbi, fb_helper, sizes);
>>  
>> -if (fb->funcs->dirty) {
>> +if (drm_fbdev_use_shadow_fb(fb_helper)) {
>>  struct fb_ops *fbops;
>>  void *shadow;
>>  
>> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
>> index 759d462d028b..e1c751aca353 100644
>> --- a/include/drm/drm_mode_config.h
>> +++ b/include/drm/drm_mode_config.h
>> @@ -347,6 +347,8 @@ struct drm_mode_config_funcs {
>>   * @output_poll_work: delayed work for polling in process context
>>   * @preferred_depth: preferred RBG pixel depth, used by fb helpers
>>   * @prefer_shadow: hint to userspace to prefer shadow-fb rendering
>> + * @prefer_shadow_fbdev: hint to framebuffer emulation to prefer shadow-fb \
>> +rendering
> 
> It's preferred to have the doc together with the struct member.

I just tried to follow the file's existing style, but OK, I don't mind.

> it's less likely to be forgotten when things change. And we don't use
> line cont. when the doc line is too long. Just continue on the next line
> after an asterix.
> 
>>   * @cursor_width: hint to userspace for max cursor width
>>   * @cursor_height: hint to userspace for max cursor height
>>   * @helper_private: mid-layer private data
>> @@ -852,6 +854,9 @@ struct drm_mode_config {
>>  /* dumb ioctl parameters */
>>  uint32_t preferred_depth, prefer_shadow;
>>  
>> +/* fbdev parameters */
> 
> No need for this comment.
> 
> Doc can look like this, I've done s/framebuffer/fbdev/:
>   /**
>* @prefer_shadow_fbdev:
>*
>* Hint to fbdev emulation to prefer shadow-fb rendering.
>*/
> 
>> +uint32_t prefer_shadow_fbdev;
> 
> Use bool here.
> 
> With that:
> 
> Reviewed-by: Noralf Trønnes 
> 
> I have tested this on 2 drivers that use generic fbdev: vc4 (no shadow
> buf) and mi0283qt which has a dirty callback.
> 
> Tested-by: Noralf Trønnes 

Thanks for reviewing and testing the patches.

Best regards
Thomas

> 
>> +
>>  /**
>>   * @quirk_addfb_prefer_xbgr_30bpp:
>>   *
>>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 

Re: [PATCH v2 1/6] drm/client: Support unmapping of DRM client buffers

2019-07-07 Thread Noralf Trønnes


Den 05.07.2019 11.26, skrev Thomas Zimmermann:
> DRM clients, such as the fbdev emulation, have their buffer objects
> mapped by default. Mapping a buffer implicitly prevents its relocation.
> Hence, the buffer may permanently consume video memory while it's
> allocated. This is a problem for drivers of low-memory devices, such as
> ast, mgag200 or older framebuffer hardware, which will then not have
> enough memory to display other content (e.g., X11).
> 
> This patch introduces drm_client_buffer_vmap() and _vunmap(). Internal
> DRM clients can use these functions to unmap and remap buffer objects
> as needed.
> 
> There's no reference counting for vmap operations. Callers are expected
> to either keep buffers mapped (as it is now), or call vmap and vunmap
> in pairs around code that accesses the mapped memory.
> 
> v2:
>   * remove several duplicated NULL-pointer checks
> 
> Signed-off-by: Thomas Zimmermann 
> ---
>  drivers/gpu/drm/drm_client.c | 67 ++--
>  include/drm/drm_client.h |  3 ++
>  2 files changed, 60 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index 410572f14257..66d8d645ac79 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -281,6 +281,43 @@ drm_client_buffer_create(struct drm_client_dev *client, 
> u32 width, u32 height, u
>  
>   buffer->gem = obj;
>  
> + vaddr = drm_client_buffer_vmap(buffer);
> + if (IS_ERR(vaddr)) {
> + ret = PTR_ERR(vaddr);
> + goto err_delete;
> + }
> +
> + return buffer;
> +
> +err_delete:
> + drm_client_buffer_delete(buffer);
> +
> + return ERR_PTR(ret);
> +}
> +
> +/**
> + * drm_client_buffer_vmap - Map DRM client buffer into address space
> + * @buffer: DRM client buffer
> + *
> + * This function maps a client buffer into kernel address space. If the
> + * buffer is already mapped, it returns the mapping's address.
> + *
> + * Client buffer mappings are not ref'counted. Each call to
> + * drm_client_buffer_vmap() should be followed by a call to
> + * drm_client_buffer_vunmap(); or the client buffer should be mapped
> + * throughout its lifetime. The latter is the default.
> + *
> + * Returns:
> + *   The mapped memory's address
> + */
> +void *
> +drm_client_buffer_vmap(struct drm_client_buffer *buffer)

I prefer to have this on one line.

> +{
> + void *vaddr;
> +
> + if (buffer->vaddr)
> + return buffer->vaddr;
> +
>   /*
>* FIXME: The dependency on GEM here isn't required, we could
>* convert the driver handle to a dma-buf instead and use the
> @@ -289,21 +326,31 @@ drm_client_buffer_create(struct drm_client_dev *client, 
> u32 width, u32 height, u
>* fd_install step out of the driver backend hooks, to make that
>* final step optional for internal users.
>*/
> - vaddr = drm_gem_vmap(obj);
> - if (IS_ERR(vaddr)) {
> - ret = PTR_ERR(vaddr);
> - goto err_delete;
> - }
> + vaddr = drm_gem_vmap(buffer->gem);
> + if (IS_ERR(vaddr))
> + return vaddr;
>  
>   buffer->vaddr = vaddr;
>  
> - return buffer;
> -
> -err_delete:
> - drm_client_buffer_delete(buffer);
> + return vaddr;
> +}
> +EXPORT_SYMBOL(drm_client_buffer_vmap);
>  
> - return ERR_PTR(ret);
> +/**
> + * drm_client_buffer_vunmap - Unmap DRM client buffer
> + * @buffer: DRM client buffer
> + *
> + * This function removes a client buffer's memory mmapping. This

s/mmapping/mapping/

> + * function is only required by clients that manage their buffers
> + * by themselves. By default, DRM client buffers are mapped throughout
> + * their entire lifetime.
> + */
> +void drm_client_buffer_vunmap(struct drm_client_buffer *buffer)
> +{
> + drm_gem_vunmap(buffer->gem, buffer->vaddr);
> + buffer->vaddr = NULL;
>  }
> +EXPORT_SYMBOL(drm_client_buffer_vunmap);
>  
>  static void drm_client_buffer_rmfb(struct drm_client_buffer *buffer)
>  {
> diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
> index 72d51d1e9dd9..e1db1d9da0bf 100644
> --- a/include/drm/drm_client.h
> +++ b/include/drm/drm_client.h
> @@ -149,6 +149,9 @@ struct drm_client_buffer {
>  struct drm_client_buffer *
>  drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 
> height, u32 format);
>  void drm_client_framebuffer_delete(struct drm_client_buffer *buffer);
> +void *
> +drm_client_buffer_vmap(struct drm_client_buffer *buffer);

Prefer to have this on one line.

Reviewed-by: Noralf Trønnes 

> +void drm_client_buffer_vunmap(struct drm_client_buffer *buffer);
>  
>  int drm_client_modeset_create(struct drm_client_dev *client);
>  void drm_client_modeset_free(struct drm_client_dev *client);
> 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 2/6] drm/fb-helper: Map DRM client buffer only when required

2019-07-07 Thread Noralf Trønnes


Den 05.07.2019 11.26, skrev Thomas Zimmermann:
> This patch changes DRM clients to not map the buffer by default. The
> buffer, like any buffer object, should be mapped and unmapped when
> needed.
> 
> An unmapped buffer object can be evicted to system memory and does
> not consume video ram until displayed. This allows to use generic fbdev
> emulation with drivers for low-memory devices, such as ast and mgag200.
> 
> This change affects the generic framebuffer console. HW-based consoles
> map their console buffer once and keep it mapped. Userspace can mmap this
> buffer into its address space. The shadow-buffered framebuffer console
> only needs the buffer object to be mapped during updates. While not being
> updated from the shadow buffer, the buffer object can remain unmapped.
> Userspace will always mmap the shadow buffer.
> 
> v2:
>   * change DRM client to not map buffer by default
>   * manually map client buffer for fbdev with HW framebuffer
> 
> Signed-off-by: Thomas Zimmermann 
> ---

Reviewed-by: Noralf Trønnes 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization