linux-kernel - Re: [PATCH v7 2/2] drm: add kms driver for loongson display controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <efcc3a66-78ca-4e0a-c0fb-527da376fc06@amd.com>
Date:   Thu, 16 Mar 2023 08:18:13 +0100
From:   Christian König <christian.koenig@....com>
To:     Sui Jingfeng <15330273260@....cn>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <mripard@...nel.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        David Airlie <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>,
        suijingfeng <suijingfeng@...ngson.cn>,
        Sumit Semwal <sumit.semwal@...aro.org>
Cc:     linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        linux-media@...r.kernel.org, linaro-mm-sig@...ts.linaro.org,
        Li Yi <liyi@...ngson.cn>
Subject: Re: [PATCH v7 2/2] drm: add kms driver for loongson display
 controller



Am 15.03.23 um 22:15 schrieb Sui Jingfeng:
> From: suijingfeng <suijingfeng@...ngson.cn>
>
> Loongson display controller IP has been integrated in both Loongson
> North Bridge chipset(ls7a1000 and ls7a2000) and Loongson SoCs(ls2k1000
> and ls2k2000 etc), it even has been included in Loongson BMC products.
>
> This display controller is a PCI device, it has two display pipe. For
> the DC in LS7A1000 and LS2K1000 each way has a DVO output interface
> which provide RGB888 signals, vertical & horizontal synchronisations,
> and the pixel clock. Each CRTC is able to support 1920x1080@...z,
> the maximum resolution is 2048x2048 according to the hardware spec.
>
> For the DC in LS7A2000, each display pipe is equipped with a built-in
> HDMI encoder which is compliant with HDMI 1.4 specification, thus it
> support 3840x2160@...z. The first display pipe is also equipped with
> a transparent vga encoder which is parallel with the HDMI encoder.
> The DC in LS7A2000 is more complete, besides above feature, it has
> two hardware cursors, two hardware vblank counter and two scanout
> position recorders.
>
>   v1 -> v2:
>    1) Use hpd status reg when polling for ls7a2000
>    2) Fix all warnings emerged when compile with W=1
>
>   v2 -> v3:
>    1) Add COMPILE_TEST in Kconfig and make the driver off by default
>    2) Alphabetical sorting headers (Thomas)
>    3) Untangle register access functions as much as possible (Thomas)
>    4) Switch to TTM based memory manager and prefer cached mapping
>       for Loongson SoC (Thomas)
>    5) Add chip id detection method, now all models are distinguishable.
>    6) Revise builtin HDMI phy driver, nearly all main stream mode
>       below 4K@...z is tested, this driver supported these mode very
>       well including clone display mode and extend display mode.
>
>   v3 -> v4:
>    1) Quickly fix a small mistake.
>
>   v4 -> v5:
>    1) Drop potential support for Loongson 2K series SoC temporary,
>       this part should be resend with the DT binding patch in the future.
>    2) Add per display pipe debugfs support to the builtin HDMI encoder.
>    3) Rewrite atomic_update() for hardware cursors plane(Thomas)
>    4) Rewrite encoder and connector initialization part, untangle it
>       according to the chip(Thomas).
>
>   v5 -> v6:
>    1) Remove stray code which didn't get used, say lsdc_of_get_reserved_ram
>    2) Fix all typos I could found, make sentences and code more readable
>    3) Untange lsdc_hdmi*_connector_detect() function according to the pipe
>    4) After a serious consideration, we rename this driver as loongson.
>       Because we also have drivers toward the LoongGPU IP in LS7A2000 and
>       LS2K2000. Besides, there are also drivers about the external encoder,
>       HDMI audio driver and vbios support etc. This patch only provide DC
>       driver part, my teammate Li Yi believe that loongson will be more
>       suitable for loongson graphics than lsdc in the long run.
>
>       loongson.ko = LSDC + LoongGPU + encoders driver + vbios/DT ...
>
>    v6 -> v7:
>    1) Add prime support, self-sharing is works. sharing buffer with etnaviv
>       is also tested, and its works with limitation.
>    2) Implement buffer objects tracking with list_head.
>    3) S3(sleep to RAM) is tested on ls3a5000+ls7a2000 evb and it works.
>    4) Rewrite lsdc_bo_move, since ttm core stop allocating resources
>       during BO creation. Patch V1 ~ V6 of this series no longer works
>       on latest kernel. Thus, we send V7.
>
> Signed-off-by: Li Yi <liyi@...ngson.cn>
> Signed-off-by: suijingfeng <suijingfeng@...ngson.cn>
> Signed-off-by: Sui Jingfeng <15330273260@....cn>

[SNIP]

> +u64 lsdc_bo_gpu_offset(struct ttm_buffer_object *tbo)
> +{
> +	struct drm_device *ddev = tbo->base.dev;
> +	struct ttm_resource *resource = tbo->resource;
> +
> +	if (drm_WARN_ON(ddev, !tbo->pin_count))
> +		return -ENODEV;

Returning -ENODEV when the function return value is unsigned doesn't 
make much sense. I would also use 0 here.

Apart from that I briefly skimmed over the prime and TTM handling and 
couldn't find anything obviously wrong.

I obviously can't review the hw specific stuff, but over all looks 
pretty good to me.

Regards,
Christian.