diff mbox series

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

Message ID 20190904115644.7620-3-tzimmermann@suse.de (mailing list archive)
State New, archived
Headers show
Series ast, mgag200: Map console BO while it's being displayed | expand

Commit Message

Thomas Zimmermann Sept. 4, 2019, 11:56 a.m. UTC
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@suse.de>
Cc: Noralf Trønnes <noralf@tronnes.org>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
Cc: CK Hu <ck.hu@mediatek.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: YueHaibing <yuehaibing@huawei.com>
Cc: Sam Bobroff <sbobroff@linux.ibm.com>
Cc: Huang Rui <ray.huang@amd.com>
Cc: "Y.C. Chen" <yc_chen@aspeedtech.com>
Cc: Rong Chen <rong.a.chen@intel.com>
Cc: Feng Tang <feng.tang@intel.com>
Cc: Huang Ying <ying.huang@intel.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>
---
 drivers/gpu/drm/ast/ast_mode.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

Gerd Hoffmann Sept. 5, 2019, 6:25 a.m. UTC | #1
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@suse.de>
> Cc: Noralf Trønnes <noralf@tronnes.org>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> Cc: CK Hu <ck.hu@mediatek.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: YueHaibing <yuehaibing@huawei.com>
> Cc: Sam Bobroff <sbobroff@linux.ibm.com>
> Cc: Huang Rui <ray.huang@amd.com>
> Cc: "Y.C. Chen" <yc_chen@aspeedtech.com>
> Cc: Rong Chen <rong.a.chen@intel.com>
> Cc: Feng Tang <feng.tang@intel.com>
> Cc: Huang Ying <ying.huang@intel.com>
> Cc: Davidlohr Bueso <dave@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
diff mbox series

Patch

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_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.
+		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.
+	if (crtc->primary->fb == fb_helper->buffer->fb) {
+		base = drm_gem_vram_kmap(gbo, true, NULL);
+		if (IS_ERR(base))
+			DRM_ERROR("failed to kmap fbcon\n");
+	}
+
 	return 0;
 
 err_drm_gem_vram_unpin:
@@ -605,10 +620,14 @@  static void ast_crtc_disable(struct drm_crtc *crtc)
 	DRM_DEBUG_KMS("\n");
 	ast_crtc_dpms(crtc, DRM_MODE_DPMS_OFF);
 	if (crtc->primary->fb) {
+		struct drm_fb_helper *fb_helper = crtc->dev->fb_helper;
 		struct drm_framebuffer *fb = crtc->primary->fb;
 		struct drm_gem_vram_object *gbo =
 			drm_gem_vram_of_gem(fb->obj[0]);
 
+		// Unmap if it's the fbdev FB.
+		if (fb == fb_helper->buffer->fb)
+			drm_gem_vram_kunmap(gbo);
 		drm_gem_vram_unpin(gbo);
 	}
 	crtc->primary->fb = NULL;