linux-kernel - Re: [PATCH v5 3/4] drm/bridge: Introduce LT9611 DSI to HDMI bridge

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: <20200719171806.GA55541@ravnborg.org>
Date:   Sun, 19 Jul 2020 19:18:06 +0200
From:   Sam Ravnborg <sam@...nborg.org>
To:     Vinod Koul <vkoul@...nel.org>
Cc:     Daniel Vetter <daniel@...ll.ch>, David Airlie <airlied@...ux.ie>,
        Rob Clark <robdclark@...il.com>, linux-arm-msm@...r.kernel.org,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Andrzej Hajda <a.hajda@...sung.com>,
        Neil Armstrong <narmstrong@...libre.com>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        Jonas Karlman <jonas@...boo.se>,
        Jernej Skrabec <jernej.skrabec@...l.net>,
        Rob Herring <robh+dt@...nel.org>, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        Emil Velikov <emil.l.velikov@...il.com>,
        Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        John Stultz <john.stultz@...aro.org>
Subject: Re: [PATCH v5 3/4] drm/bridge: Introduce LT9611 DSI to HDMI bridge

Hi Vinod.

Three trivial points below.
The rest looks good.

With these fixed you can add:
Reviewed-by: Sam Ravnborg <sam@...nborg.org>

	Sam

On Wed, Jul 08, 2020 at 04:05:58PM +0530, Vinod Koul wrote:
> Lontium Lt9611 is a DSI to HDMI bridge which supports two DSI ports and
> I2S port as an input and HDMI port as output
> 
> Co-developed-by: Bjorn Andersson <bjorn.andersson@...aro.org>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@...aro.org>
> Co-developed-by: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
> Tested-by: John Stultz <john.stultz@...aro.org>
> Signed-off-by: Vinod Koul <vkoul@...nel.org>
> ---
>  drivers/gpu/drm/bridge/Kconfig          |   13 +
>  drivers/gpu/drm/bridge/Makefile         |    1 +
>  drivers/gpu/drm/bridge/lontium-lt9611.c | 1142 +++++++++++++++++++++++
>  3 files changed, 1156 insertions(+)
>  create mode 100644 drivers/gpu/drm/bridge/lontium-lt9611.c
> 
> +
> +#include <drm/drm_probe_helper.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_bridge.h>
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_print.h>

In alphabetical order. drm_probe_helper needs to be moved.

> +
> +#define EDID_SEG_SIZE	256
> +#define EDID_LEN	32
> +#define EDID_LOOP	8
> +#define KEY_DDC_ACCS_DONE 0x02
> +#define DDC_NO_ACK	0x50
> +


> +static void lt9611_pcr_setup(struct lt9611 *lt9611, const struct drm_display_mode *mode)
> +{
> +	const struct reg_sequence reg_cfg[] = {
> +		{ 0x830b, 0x01 },
> +		{ 0x830c, 0x10 },
> +		{ 0x8348, 0x00 },
> +		{ 0x8349, 0x81 },
> +
> +		/* stage 1 */
> +		{ 0x8321, 0x4a },
> +		{ 0x8324, 0x71 },
> +		{ 0x8325, 0x30 },
> +		{ 0x832a, 0x01 },
> +
> +		/* stage 2 */
> +		{ 0x834a, 0x40 },
> +		{ 0x831d, 0x10 },
> +
> +		/* MK limit */
> +		{ 0x832d, 0x38 },
> +		{ 0x8331, 0x08 },
> +	};
> +	const struct reg_sequence reg_cfg2[] = {
> +			{ 0x830b, 0x03 },
> +			{ 0x830c, 0xd0 },
> +			{ 0x8348, 0x03 },
> +			{ 0x8349, 0xe0 },
> +			{ 0x8324, 0x72 },
> +			{ 0x8325, 0x00 },
> +			{ 0x832a, 0x01 },
> +			{ 0x834a, 0x10 },
> +			{ 0x831d, 0x10 },
> +			{ 0x8326, 0x37 },
Block above is indented one tab too much.

> +static int lt9611_bridge_attach(struct drm_bridge *bridge,
> +				enum drm_bridge_attach_flags flags)
> +{
> +	struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
> +	int ret;
> +
> +	if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
> +		dev_err(lt9611->dev, "Fix bridge driver to make connector optional!");
> +		return -EINVAL;
> +	}
This should say that the display driver should be fixed.
If a display driver expects this bridge to create the connector
it would not work.