Re: [PATCH 2/2] drm/panel: add visionox vtdr6130 DSI panel driver

From: Neil Armstrong <neil.armstrong@linaro.org>
To: Rayyan Ansari <rayyan@ansari.sh>,
	Thierry Reding <thierry.reding@gmail.com>,
	Sam Ravnborg <sam@ravnborg.org>, David Airlie <airlied@gmail.com>,
	Daniel Vetter <daniel@ffwll.ch>, Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org,
	linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] drm/panel: add visionox vtdr6130 DSI panel driver
Date: Thu, 5 Jan 2023 10:17:25 +0100	[thread overview]
Message-ID: <2d90fb92-45ac-7311-ef27-ce82c6b891a4@linaro.org> (raw)
In-Reply-To: <8c8d0276-61a8-71d6-17b0-f6b384ab9d94@ansari.sh>

Hi,

On 04/01/2023 19:25, Rayyan Ansari wrote:
> Hello,
> I am not a maintainer, but I thought I might give a few comments.

Any comment is always welcome !

> 
> On 03/01/2023 14:22, Neil Armstrong wrote:
>> Add support for the 1080x2400 Visionox VTDR6130 AMOLED DSI panel
>> found on the Qualcomm SM8550 MTP board.
>>
>> By default the the panel is configured to work with DSI compressed
>> streams, but can work in uncompressed video mode since 1080x2400 in
>> RGB888 fits in the 4 DSI lanes bandwidth.
>>
>> While display compression is preferred for performance and power
>> reasons, let's start with the uncompressed video mode support and
>> add the DSC support later on.
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
>>   drivers/gpu/drm/panel/Kconfig                   |   8 +
>>   drivers/gpu/drm/panel/Makefile                  |   1 +
>>   drivers/gpu/drm/panel/panel-visionox-vtdr6130.c | 366 ++++++++++++++++++++++++
>>   3 files changed, 375 insertions(+)
>>

<snip>

>> +
>> +MODULE_AUTHOR("Neil Armstron <neil.armstrong@linaro.org>");
> 
> Missed a "g" here :-)
> 
>> +MODULE_DESCRIPTION("Panel driver for the visionox VTDR6130 AMOLED DSI panel");
> 
> I am not sure how the company capitalises the name, but to keep it consistent with the other mentions, I think it should be "Visionox".

Thanks, I'll fix these.

Neil

> 
>> +MODULE_LICENSE("GPL");
>>
> 