Messages in this thread | | | Date | Thu, 16 Mar 2023 16:46:35 +0800 | Subject | Re: [PATCH v7 2/2] drm: add kms driver for loongson display controller | From | Sui jingfeng <> |
| |
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.
>> +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. > Ok, we can help to provide more HW specific document and material.
Currently, our hardware is just a trivial compare with ati/amdgpu graphics.
Thomas and you and are experts at graphics driver domain,
Maybe you could do me a favor, give me a reviewed-by for the TTM and PRIME part :)
> Regards, > Christian.
| |