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@xxxxxxx>
> Cc: Noralf Trønnes <noralf@xxxxxxxxxxx>
> Cc: Dave Airlie <airlied@xxxxxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: Sam Ravnborg <sam@xxxxxxxxxxxx>
> Cc: Gerd Hoffmann <kraxel@xxxxxxxxxx>
> Cc: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> Cc: CK Hu <ck.hu@xxxxxxxxxxxx>
> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> Cc: Alex Deucher <alexander.deucher@xxxxxxx>
> Cc: "Christian König" <christian.koenig@xxxxxxx>
> Cc: YueHaibing <yuehaibing@xxxxxxxxxx>
> Cc: Sam Bobroff <sbobroff@xxxxxxxxxxxxx>
> Cc: Huang Rui <ray.huang@xxxxxxx>
> Cc: "Y.C. Chen" <yc_chen@xxxxxxxxxxxxxx>
> Cc: Rong Chen <rong.a.chen@xxxxxxxxx>
> Cc: Feng Tang <feng.tang@xxxxxxxxx>
> Cc: Huang Ying <ying.huang@xxxxxxxxx>
> Cc: Davidlohr Bueso <dave@xxxxxxxxxxxx>
> ---
> 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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
