Messages in this thread | | | Date | Thu, 16 Mar 2023 18:12:46 +0800 | From | Sui jingfeng <> | Subject | Re: [PATCH v7 2/2] drm: add kms driver for loongson display controller |
| |
On 2023/3/16 16:46, Sui jingfeng wrote: > > On 2023/3/16 15:18, Christian König wrote: >> >> >> Am 15.03.23 um 22:15 schrieb Sui Jingfeng: >>> From: suijingfeng <suijingfeng@loongson.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@60Hz, >>> 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@30Hz. 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@30Hz 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@loongson.cn> >>> Signed-off-by: suijingfeng <suijingfeng@loongson.cn> >>> Signed-off-by: Sui Jingfeng <15330273260@189.cn> >> >> [SNIP] >> > Hi, > > I send my patch series with my personal email (15330273260@189.cn), > because it is more reliable. > > I don't mind remove it when this driver is applied. >
We know that we should do this job cleanly.
But i am worry about the troubles it may cause when send patch with my company email(suijingfeng <suijingfeng@loongson.cn>)
So I just follow the my conventional way of sending patch about this series,
I will use my company email(suijingfeng <suijingfeng@loongson.cn>) in the future.
>>> +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. >> > OK, > > To make sense, the caller can cast the return to s64. Use 0 is also ok. > > In our daily usage, tbo->pin_count ==0 never happens. A warning > message is enough. > > I will revise this at next version. > > >> 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. > Maybe you could do me a favor, give me a reviewed-by for the TTM and > PRIME part :) > Here missing a '?' at the end.
> >> Regards, >> Christian.
| |