Reply: Re: [PATCH v3 1/3] drm/loongson: Add DRM Driver for Loongson 7A1000 bridge chip

[李晨阳]

Hi Sam,

Thank you very much for your attention to our driver.
I will consider your opinion, The next version is due in the near future.

> -----Original Messages-----
> From: "Sam Ravnborg" <sam at ravnborg.org>
> Sent Time: 2021-07-28 01:41:30 (Wednesday)
> To: "Daniel Vetter" <daniel at ffwll.ch>
> Subject: Re: [PATCH v3 1/3] drm/loongson: Add DRM Driver for Loongson 7A1000 bridge chip
> 
> Hi Chenyang,
> 
> I browsed the code on lore and noticed a few things and thought it
> better to bring it to your attention now.
> 
> The general structure of the drivers seems good and coding style is
> fine. The feedback is mostly stuff we have decided to do different over
> time, so likely because you based the driver on some older driver.
> And it can be hard to follow all the refactoring going on - something
> that you get for free (almost) when is is mainlined.
> 
> 1) Use drm_ for logging whereever possible (need drm_device)
> 2) Do not use irq mid-layer support in drm_driver, it is about to be
>    legacy only. In other words avoid using drm_irq* stuff.
> 3) Look at drm_drv to see code snippet how to use the drmm*
>    infrastructure. It will save you some cleanup and in general make the
>    driver more stable
> 4) Sort includes alphabetically, and split them on in blocks.
>    <linux *=""> is one block
>    <newline>
>    <drm drm_*=""> is next block
> 5) Put entry in makefile in alphabetical order
> 6) You most like can use the simple_encoder stuff we have today
> 7) The *_load and *_unlod names where used in the past. Maybe be
>    inspired by some newer driver here. _load functiosn is something used
>    by legacy drivers so it confuses me a little.
> 
> I look forward to see next revision of the patch-set.
> And sorry for not providing these high-level feedback issues before - I
> have not had time to look at your driver.
> 
> 	Sam


------------------------------
LiChenyang</drm></newline></linux></daniel at ffwll.ch></sam at ravnborg.org>