[PATCH v2 2/3] drm/ast: Map fbdev framebuffer while it's being displayed

[Gerd Hoffmann]

On Wed, Sep 04, 2019 at 01:56:43PM +0200, Thomas Zimmermann wrote:
> The generic fbdev emulation will map and unmap the framebuffer's memory
> if required. As consoles are most often updated while being on screen,
> we map the fbdev buffer while it's being displayed. This avoids frequent
> map/unmap operations in the fbdev code. The original fbdev code in ast
> used the same trick to improve performance.
> 
> v2:
> 	* fix typo in comment
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
> Cc: Noralf Trønnes <noralf at tronnes.org>
> Cc: Dave Airlie <airlied at redhat.com>
> Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
> Cc: Thomas Gleixner <tglx at linutronix.de>
> Cc: Sam Ravnborg <sam at ravnborg.org>
> Cc: Gerd Hoffmann <kraxel at redhat.com>
> Cc: Oleksandr Andrushchenko <oleksandr_andrushchenko at epam.com>
> Cc: CK Hu <ck.hu at mediatek.com>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: Alex Deucher <alexander.deucher at amd.com>
> Cc: "Christian König" <christian.koenig at amd.com>
> Cc: YueHaibing <yuehaibing at huawei.com>
> Cc: Sam Bobroff <sbobroff at linux.ibm.com>
> Cc: Huang Rui <ray.huang at amd.com>
> Cc: "Y.C. Chen" <yc_chen at aspeedtech.com>
> Cc: Rong Chen <rong.a.chen at intel.com>
> Cc: Feng Tang <feng.tang at intel.com>
> Cc: Huang Ying <ying.huang at intel.com>
> Cc: Davidlohr Bueso <dave at stgolabs.net>
> ---
>  drivers/gpu/drm/ast/ast_mode.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index d349c721501c..c10fff652228 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -529,13 +529,20 @@ static int ast_crtc_do_set_base(struct drm_crtc *crtc,
>  				struct drm_framebuffer *fb,
>  				int x, int y, int atomic)
>  {
> +	struct drm_fb_helper *fb_helper = crtc->dev->fb_helper;

struct drm_framebuffer *fbcon = crtc->dev->fb_helper->buffer->fb ?

makes clear what is going on without excessive commenting ;)

>  	struct drm_gem_vram_object *gbo;
>  	int ret;
>  	s64 gpu_addr;
> +	void *base;
>  
>  	if (!atomic && fb) {
>  		gbo = drm_gem_vram_of_gem(fb->obj[0]);
>  		drm_gem_vram_unpin(gbo);
> +
> +		// Unmap fbdev FB if it's not being displayed
> +		// any longer.

I'd drop the comment.  It says *what* the comment is doing.  You should
be able to figure by just reading the code.  Comments should explain
*why* the code does something ...

> +		if (fb == fb_helper->buffer->fb)
> +			drm_gem_vram_kunmap(gbo);
>  	}
>  
>  	gbo = drm_gem_vram_of_gem(crtc->primary->fb->obj[0]);
> @@ -552,6 +559,14 @@ static int ast_crtc_do_set_base(struct drm_crtc *crtc,
>  	ast_set_offset_reg(crtc);
>  	ast_set_start_address_crt1(crtc, (u32)gpu_addr);
>  
> +	// Map fbdev FB while it's being displayed. This avoids frequent
> +	// mapping and unmapping within the fbdev code.

... like this one (avoid frequent map/unmap).

Comments should use /* */ style, especially multi line comments.
See also the comment section in
	Documentation/process/coding-style.rst

cheers,
  Gerd