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

[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 <tzimmermann at suse.de>
> ---
>  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, &clip_copy);
>  		}
> -		helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, &clip_copy, 1);
> +		if (helper->fb->funcs->dirty)
> +			helper->fb->funcs->dirty(helper->fb, NULL, 0, 0,
> +						 &clip_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 = &helper->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(&helper->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 +2222,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 <noralf at tronnes.org>

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 <noralf at tronnes.org>

> +
>  	/**
>  	 * @quirk_addfb_prefer_xbgr_30bpp:
>  	 *
>