From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ying.Liu@freescale.com (Liu Ying)
Date: Wed, 17 Dec 2014 17:44:33 +0800
Subject: [PATCH RFC 09/15] drm: imx: Add MIPI DSI host controller driver
In-Reply-To: <20141210131640.GD23558@ulmo.nvidia.com>
References: <1418200648-32656-1-git-send-email-Ying.Liu@freescale.com>
 <1418200648-32656-10-git-send-email-Ying.Liu@freescale.com>
 <20141210131640.GD23558@ulmo.nvidia.com>
Message-ID: <54915081.6020508@freescale.com>
To: linux-arm-kernel@lists.infradead.org
List-Id: linux-arm-kernel.lists.infradead.org

Hi Thierry,

Sorry for the late response.
I tried to address almost all your comments locally first.
More feedback below.

On 12/10/2014 09:16 PM, Thierry Reding wrote:
> On Wed, Dec 10, 2014 at 04:37:22PM +0800, Liu Ying wrote:
>> This patch adds i.MX MIPI DSI host controller driver support.
>> Currently, the driver supports the burst with sync pulses mode only.
>>
>> Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
>> ---
>>   .../devicetree/bindings/drm/imx/mipi_dsi.txt       |   81 ++
>>   drivers/gpu/drm/imx/Kconfig                        |    6 +
>>   drivers/gpu/drm/imx/Makefile                       |    1 +
>>   drivers/gpu/drm/imx/imx-mipi-dsi.c                 | 1017 ++++++++++++++++++++
>>   4 files changed, 1105 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/drm/imx/mipi_dsi.txt
>>   create mode 100644 drivers/gpu/drm/imx/imx-mipi-dsi.c
>>
>> diff --git a/Documentation/devicetree/bindings/drm/imx/mipi_dsi.txt b/Documentation/devicetree/bindings/drm/imx/mipi_dsi.txt
>> new file mode 100644
>> index 0000000..3d07fd7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/drm/imx/mipi_dsi.txt
>> @@ -0,0 +1,81 @@
>> +Device-Tree bindings for MIPI DSI host controller
>> +
>> +MIPI DSI host controller
>> +========================
>> +
>> +The MIPI DSI host controller is a Synopsys DesignWare IP.
>> +It is a digital core that implements all protocol functions defined
>> +in the MIPI DSI specification, providing an interface between the
>> +system and the MIPI DPHY, and allowing communication with a MIPI DSI
>> +compliant display.
>> +
>> +Required properties:
>> + - #address-cells : Should be <1>.
>> + - #size-cells : Should be <0>.
>> + - compatible : Should be "fsl,imx6q-mipi-dsi" for i.MX6q/sdl SoCs.
>> + - reg : Physical base address of the controller and length of memory
>> +         mapped region.
>> + - interrupts : The controller's interrupt number to the CPU(s).
>> + - gpr : Should be <&gpr>.
>> +         The phandle points to the iomuxc-gpr region containing the
>> +         multiplexer control register for the controller.
>
> Side-note: Shouldn't this really be a pinmux, then?

No. The muxing is inside the i.MX SoC.
There is a DT binding documentation for the system controller node(gpr)
at [1]. And, for i.MX DT sources, there are several existing use cases 
in which the gpr node is referred by other nodes.

[1] Documentation/devicetree/bindings/mfd/syscon.txt.

>
>> + - clocks, clock-names : Phandles to the controller pllref, pllref_gate
>> +           and core_cfg clocks, as described in [1] and [2].
>> + - panel at 0 : A panel node which contains a display-timings child node as
>> +             defined in [3].
>
> There's no need for these to be named panel@*. They could be bridges for
> example. And no, they shouldn't contain a display-timings child node
> either. Panels should have a proper driver and the driver being device
> specific it should have the timings embedded.

Ok, I'll move the timing to the panel driver.

>
>> + - port@[0-4] : Up to four port nodes with endpoint definitions as defined
>> +   in [4], corresponding to the four inputs to the controller multiplexer.
>> +   Note that each port node should contain the input-port property to
>> +   distinguish it from the panel node, as described in [5].
>
> [4] says that you can group all port nodes under a ports parent node. I
> think this is really what you want to do here to make it clear that the
> ports aren't part of the DSI host binding part of the device.
>

Accepted.

>> diff --git a/drivers/gpu/drm/imx/imx-mipi-dsi.c b/drivers/gpu/drm/imx/imx-mipi-dsi.c
> [...]
>> +/*
>> + * i.MX drm driver - MIPI DSI Host Controller
>> + *
>> + * Copyright (C) 2011-2014 Freescale Semiconductor, Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * as published by the Free Software Foundation; either version 2
>> + * of the License, or (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/component.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
>> +#include <linux/module.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/videodev2.h>
>> +#include <asm/div64.h>
>
> Don't you want the more generic linux/math64.h here?

I'll use linux/math64.h.

>
>> +#include <drm/drmP.h>
>> +#include <drm/drm_crtc_helper.h>
>> +#include <drm/drm_fb_helper.h>
>
> I don't see any of the functions defined in that header used here.

I'll remove this.

>
>> +#include <drm/drm_mipi_dsi.h>
>> +#include <drm/drm_panel.h>
>> +#include <video/mipi_display.h>
>> +#include <video/of_videomode.h>
>> +#include <video/videomode.h>
>> +
>> +#include "imx-drm.h"
>> +
>> +#define DRIVER_NAME 			"imx-mipi-dsi"
>> +
>> +#define	DSI_VERSION			0x00
>> +
>> +#define	DSI_PWR_UP			0x04
>> +#define	RESET				0
>> +#define	POWERUP				BIT(0)
>> +
>> +#define	DSI_CLKMGR_CFG			0x08
>> +#define TO_CLK_DIVIDSION(div)		(((div) & 0xff) << 8)
>> +#define TX_ESC_CLK_DIVIDSION(div)	(((div) & 0xff) << 0)
>
> Your indentation here is... weird.

I'll fix this.

>
>> +#define IMX_MIPI_DSI_MAX_DATA_LANES	2
>> +
>> +#define PHY_STATUS_TIMEOUT		10
>> +#define CMD_PKT_STATUS_TIMEOUT		20
>> +
>> +#define IMX_MIPI_DSI_PLD_DATA_BUF_SIZE	4
>> +
>> +#define MHZ				1000000
>
> Why not reuse the existing USEC_PER_SEC?

Accepted.

>
>> +#define host_to_dsi(host) container_of(host, struct imx_mipi_dsi, dsi_host)
>> +#define con_to_dsi(x) container_of(x, struct imx_mipi_dsi, connector)
>> +#define enc_to_dsi(x) container_of(x, struct imx_mipi_dsi, encoder)
>
> These should really be static inline functions for proper type safety.

Ok, I'll change to use functions.

>
>> +struct imx_mipi_dsi {
>> +	struct mipi_dsi_host dsi_host;
>> +	struct drm_connector connector;
>> +	struct drm_encoder encoder;
>> +	struct device_node *panel_node;
>> +	struct drm_panel *panel;
>> +	struct device *dev;
>> +
>> +	struct regmap *regmap;
>> +	void __iomem *base;
>> +
>> +	struct clk *pllref_clk;
>> +	struct clk *pllref_gate_clk;
>> +	struct clk *cfg_clk;
>> +
>> +	unsigned int lane_mbps; /* per lane */
>> +	u32 channel;
>> +	u32 lanes;
>> +	u32 format;
>> +	struct videomode vm;
>> +
>> +	bool enabled;
>
> Do you really need this flag?

I'll remove this flag.

>
>> +};
>> +
>> +enum {
>> +	STATUS_TO_CLEAR,
>> +	STATUS_TO_SET,
>> +};
>> +
>> +struct dphy_pll_testdin_map {
>> +	int max_mbps;
>
> unsigned?

Accepted.

>
>> +	u8 testdin;
>> +};
>> +
>> +/* The table is based on 27MHz DPHY pll reference clock. */
>> +static const struct dphy_pll_testdin_map dptdin_map[] = {
>> +	{160, 0x04}, {180, 0x24}, {200, 0x44}, {210, 0x06},
>> +	{240, 0x26}, {250, 0x46}, {270, 0x08}, {300, 0x28},
>> +	{330, 0x48}, {360, 0x2a}, {400, 0x4a}, {450, 0x0c},
>> +	{500, 0x2c}, {550, 0x0e}, {600, 0x2e}, {650, 0x10},
>> +	{700, 0x30}, {750, 0x12}, {800, 0x32}, {850, 0x14},
>> +	{900, 0x34}, {950, 0x54}, {1000, 0x74}
>> +};
>> +
>> +static int max_mbps_to_testdin(unsigned int max_mbps)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(dptdin_map); i++)
>> +		if (dptdin_map[i].max_mbps == max_mbps)
>> +			return dptdin_map[i].testdin;
>> +
>> +	return -EINVAL;
>> +}
>> +
>> +static int format_to_bpp(struct imx_mipi_dsi *dsi)
>> +{
>> +	switch (dsi->format) {
>> +	case MIPI_DSI_FMT_RGB888:
>> +	case MIPI_DSI_FMT_RGB666:
>> +		return 24;
>> +	case MIPI_DSI_FMT_RGB666_PACKED:
>> +		return 18;
>> +	case MIPI_DSI_FMT_RGB565:
>> +		return 16;
>> +	default:
>> +		dev_err(dsi->dev, "unsupported pixel format\n");
>> +		return -EINVAL;
>> +	}
>> +}
>
> Is there a reason why this can't be a standard MIPI DSI function?

How about moving this to include/drm/drm_mipi_dsi.h as an inline
function?

>
>> +
>> +static int check_status(struct imx_mipi_dsi *dsi, u32 reg, u32 status,
>> +			int timeout, bool to_set)
>> +{
>> +	u32 val;
>> +	bool out = false;
>> +
>> +	val = dsi_read(dsi, reg);
>> +	for (;;) {
>> +		out = to_set ? (val & status) : !(val & status);
>> +		if (out)
>> +			break;
>> +
>> +		if (!timeout--)
>> +			return -EFAULT;
>> +
>> +		msleep(1);
>> +		val = dsi_read(dsi, reg);
>> +	}
>> +	return 0;
>> +}
>
> You should probably use a properly timed loop here. msleep() isn't
> guaranteed to return after exactly one millisecond, so your timeout is
> never going to be accurate. Something like the following would be better
> in my opinion:
>
> 	timeout = jiffies + msecs_to_jiffies(timeout);
>
> 	while (time_before(jiffies, timeout)) {
> 		...
> 	}
>
> Also timeout should be unsigned long in that case.

Accepted.

>
>> +static int imx_mipi_dsi_host_attach(struct mipi_dsi_host *host,
>> +				    struct mipi_dsi_device *device)
>> +{
>> +	struct imx_mipi_dsi *dsi = host_to_dsi(host);
>> +	int ret;
>> +
>> +	if (device->lanes > IMX_MIPI_DSI_MAX_DATA_LANES) {
>> +		dev_err(dsi->dev, "the number of data lanes(%d) is too many\n",
>> +				device->lanes);
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (!(device->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) ||
>> +	    !(device->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)) {
>> +		dev_err(dsi->dev, "device mode is unsupported\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	dsi->lanes = device->lanes;
>> +	dsi->channel = device->channel;
>> +	dsi->format = device->format;
>> +	dsi->panel_node = device->dev.of_node;
>> +	of_get_videomode(dsi->panel_node, &dsi->vm, 0);
>
> No, you shouldn't use this. Query the mode from the panel instead. Or
> rather, encode this requirement in ->mode_valid() since that'll give you
> direct access to the mode.
>
> That way, ->host_attach() becomes a filter for compatible devices, yet
> devices may support multiple modes, therefore ->mode_valid() can be used
> to filter out those that don't work with your DSI host controller. There
> is a subtle difference here between devices that are compatible with the
> controller and modes that the controller doesn't support.

Accepted.

In the next version ->host_attach(), I will check the peripheral
compatibility.  Also, I'll get the panel by calling of_drm_find_panel(), 
and then call the drm_panel_attach() function.
Do you think this is okay?

>
>> +
>> +	ret = imx_mipi_dsi_get_lane_bps(dsi, &dsi->lane_mbps);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	return 0;
>> +}
>> +
>> +static int imx_mipi_dsi_host_detach(struct mipi_dsi_host *host,
>> +				    struct mipi_dsi_device *device)
>> +{
>> +	struct imx_mipi_dsi *dsi = host_to_dsi(host);
>> +
>> +	dsi->panel_node = NULL;
>> +	dsi->panel = NULL;
>> +
>> +	return 0;
>> +}
>
> You'll want to detach from the panel here as well.

Ok. I'll call drm_panel_detach() here.

>
>> +
>> +static int imx_mipi_dsi_gen_pkt_hdr_write(struct imx_mipi_dsi *dsi, u32 val)
>> +{
>> +	int ret;
>> +
>> +	ret = check_status(dsi, DSI_CMD_PKT_STATUS, GEN_CMD_FULL,
>> +			   CMD_PKT_STATUS_TIMEOUT, STATUS_TO_CLEAR);
>> +	if (ret < 0) {
>> +		dev_err(dsi->dev, "failed to get avaliable command FIFO\n");
>> +		return ret;
>> +	}
>> +
>> +	dsi_write(dsi, DSI_GEN_HDR, val);
>> +
>> +	ret = check_status(dsi, DSI_CMD_PKT_STATUS,
>> +			   GEN_CMD_EMPTY | GEN_PLD_W_EMPTY,
>> +			   CMD_PKT_STATUS_TIMEOUT, STATUS_TO_SET);
>> +	if (ret < 0) {
>> +		dev_err(dsi->dev, "failed to write command FIFO\n");
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int imx_mipi_dsi_dcs_short_write(struct imx_mipi_dsi *dsi,
>> +					const struct mipi_dsi_msg *msg)
>> +{
>> +	const u16 *tx_buf = msg->tx_buf;
>> +	u32 val = GEN_HDATA(*tx_buf) | GEN_HTYPE(msg->type);
>> +
>> +	if (msg->tx_len > 2) {
>> +		dev_err(dsi->dev, "too long tx buf length %d for short write\n",
>> +			msg->tx_len);
>> +		return -EINVAL;
>> +	}
>> +
>> +	return imx_mipi_dsi_gen_pkt_hdr_write(dsi, val);
>> +}
>
> It seems like you would profit from converting to the newly introduced
> mipi_dsi_create_packet() helper which does most of the packing along
> with some sanity checking for you.

I looked at the helper. It seems it's not quite straightforward for me
to use it here.  The message type here defined by GEN_HTYPE() is 8bit
long, while the helper seems to take it as 6bit long?

>
>> +
>> +static int imx_mipi_dsi_dcs_long_write(struct imx_mipi_dsi *dsi,
>> +				       const struct mipi_dsi_msg *msg)
>> +{
>> +	const u32 *tx_buf = msg->tx_buf;
>> +	int len = msg->tx_len, ret;
>> +	u32 val = GEN_HDATA(msg->tx_len) | GEN_HTYPE(msg->type);
>> +
>> +	if (msg->tx_len < 3) {
>> +		dev_err(dsi->dev, "wrong tx buf length %d for long write\n",
>> +			msg->tx_len);
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* write the bulks */
>> +	while (len / IMX_MIPI_DSI_PLD_DATA_BUF_SIZE) {
>> +		dsi_write(dsi, DSI_GEN_PLD_DATA, *tx_buf);
>> +		tx_buf++;
>> +		len -= IMX_MIPI_DSI_PLD_DATA_BUF_SIZE;
>
> This is confusing. Maybe a better option would be to use sizeof(*tx_buf)
> instead of this macro. You're effectively consuming one u32 with each
> write, irrespective of what the value of the macro is.

Ok. I'll use sizeof(*tx_buf).

>
>> +		ret = check_status(dsi, DSI_CMD_PKT_STATUS, GEN_PLD_W_FULL,
>> +				   CMD_PKT_STATUS_TIMEOUT, STATUS_TO_CLEAR);
>> +		if (ret < 0) {
>> +			dev_err(dsi->dev, "failed to get avaliable "
>> +					"write payload FIFO\n");
>> +			return ret;
>> +		}
>> +	}
>> +
>> +	/* write the remainder */
>> +	if (len > 0) {
>> +		ret = check_status(dsi, DSI_CMD_PKT_STATUS, GEN_PLD_W_FULL,
>> +				   CMD_PKT_STATUS_TIMEOUT, STATUS_TO_CLEAR);
>> +		if (ret < 0) {
>> +			dev_err(dsi->dev, "failed to get avaliable "
>> +					"write payload FIFO\n");
>> +			return ret;
>> +		}
>> +		dsi_write(dsi, DSI_GEN_PLD_DATA, *tx_buf);
>> +	}
>
> This is really duplicating what the above loop does. Why can't you do it
> in a single loop? Also the transmission buffer isn't guaranteed to be a
> multiple of 4 bytes, so you could have the situation where len > 0 and
> len < 4 and yet you'll be reading 4 bytes by doing *tx_buf and accessing
> memory that you shouldn't.

Good catch.  I'll make it to be a single loop and avoid accessing memory
that I shouldn't in the next version.

>
>> +	return imx_mipi_dsi_gen_pkt_hdr_write(dsi, val);
>> +}
>> +
>> +static ssize_t imx_mipi_dsi_host_transfer(struct mipi_dsi_host *host,
>> +					  const struct mipi_dsi_msg *msg)
>> +{
>> +	struct imx_mipi_dsi *dsi = host_to_dsi(host);
>> +	int ret;
>> +
>> +	switch (msg->type) {
>> +	case MIPI_DSI_DCS_SHORT_WRITE:
>> +	case MIPI_DSI_DCS_SHORT_WRITE_PARAM:
>> +	case MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE:
>> +		ret = imx_mipi_dsi_dcs_short_write(dsi, msg);
>> +		break;
>> +	case MIPI_DSI_DCS_LONG_WRITE:
>> +		ret = imx_mipi_dsi_dcs_long_write(dsi, msg);
>> +		break;
>> +	default:
>> +		dev_err(dsi->dev, "unsupported message type\n");
>> +		ret = -EFAULT;
>
> EFAULT really isn't appropriate here.

I'll change it to EINVAL.

>
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static const struct mipi_dsi_host_ops imx_mipi_dsi_host_ops = {
>> +	.attach = imx_mipi_dsi_host_attach,
>> +	.detach = imx_mipi_dsi_host_detach,
>> +	.transfer = imx_mipi_dsi_host_transfer,
>> +};
>> +
>> +static enum drm_connector_status
>> +imx_mipi_dsi_detect(struct drm_connector *connector, bool force)
>> +{
>> +	struct imx_mipi_dsi *dsi = con_to_dsi(connector);
>> +
>> +	if (!dsi->panel) {
>> +		dsi->panel = of_drm_find_panel(dsi->panel_node);
>> +		if (dsi->panel)
>> +			drm_panel_attach(dsi->panel, &dsi->connector);
>> +	}
>> +
>> +	if (dsi->panel)
>> +		return connector_status_connected;
>> +
>> +	return connector_status_disconnected;
>> +
>> +}
>
> You really shouldn't be doing that here. of_drm_find_panel() returning
> NULL should be considered cause for deferring probe. I suspect that the
> driver doesn't really handle that very well at all, though, since if it
> did you would've run into the same issue that Benjamin ran into this
> morning.

I'll return connector_status_disconnected directly here. Is it okay?

>
>> +static void imx_mipi_dsi_encoder_prepare(struct drm_encoder *encoder)
>> +{
>> +	struct imx_mipi_dsi *dsi = enc_to_dsi(encoder);
>> +	u32 interface_pix_fmt;
>> +
>> +	switch (dsi->format) {
>> +	case MIPI_DSI_FMT_RGB888:
>> +		interface_pix_fmt = V4L2_PIX_FMT_RGB24;
>> +		break;
>> +	case MIPI_DSI_FMT_RGB565:
>> +		interface_pix_fmt = V4L2_PIX_FMT_RGB565;
>> +		break;
>> +	default:
>> +		dev_err(dsi->dev, "unsupported DSI pixel format\n");
>> +		return;
>
> Why even try doing this if you know upfront that it can't be done. You
> know much earlier than this that you can't drive the pixel format, why
> not abort then?
>
> People are much more likely to notice that the panel isn't supported if
> the DSI output doesn't even show up (or doesn't expose any modes) rather
> than if they have to find this error message in dmesg.

Accepted. I'll check the dsi format compatibility in ->host_attach().
And, how about changing the default patch to be this?

	default:
		BUG();
		return;
>
>> +static void imx_mipi_dsi_video_mode_config(struct imx_mipi_dsi *dsi)
>> +{
>> +	u32 val;
>> +
>> +	val = VID_MODE_TYPE_BURST_SYNC_PULSES | ENABLE_LOW_POWER;
>> +
>> +	dsi_write(dsi, DSI_VID_MODE_CFG, val);
>> +}
>
> You probably want to parameterize based on the DSI peripheral's flags. I
> see that you do reject devices with any other modes, so this may not be
> necessary now. Out of curiosity, the hardware supports other modes,
> right?

Yes, the hardware supports other modes according to it's register
definition.
I prefer not to parameterize at present as I do reject devices with 
other modes now.

>
> [...]
>> +MODULE_LICENSE("GPL v2");
>
> Sigh... according to your header comment this needs to be "GPL".

I'll change it to be "GPL".

Thanks,

Liu Ying

>
> Thierry
>

From mboxrd@z Thu Jan  1 00:00:00 1970
From: Liu Ying <Ying.Liu@freescale.com>
Subject: Re: [PATCH RFC 09/15] drm: imx: Add MIPI DSI host controller driver
Date: Wed, 17 Dec 2014 17:44:33 +0800
Message-ID: <54915081.6020508@freescale.com>
References: <1418200648-32656-1-git-send-email-Ying.Liu@freescale.com>
 <1418200648-32656-10-git-send-email-Ying.Liu@freescale.com>
 <20141210131640.GD23558@ulmo.nvidia.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="utf-8"; Format="flowed"
Content-Transfer-Encoding: base64
Return-path: <dri-devel-bounces@lists.freedesktop.org>
In-Reply-To: <20141210131640.GD23558@ulmo.nvidia.com>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/dri-devel>,
 <mailto:dri-devel-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/dri-devel>
List-Post: <mailto:dri-devel@lists.freedesktop.org>
List-Help: <mailto:dri-devel-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/dri-devel>,
 <mailto:dri-devel-request@lists.freedesktop.org?subject=subscribe>
Errors-To: dri-devel-bounces@lists.freedesktop.org
Sender: "dri-devel" <dri-devel-bounces@lists.freedesktop.org>
To: Thierry Reding <thierry.reding@gmail.com>
Cc: devicetree@vger.kernel.org, linux@arm.linux.org.uk, kernel@pengutronix.de, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, mturquette@linaro.org, linux-arm-kernel@lists.infradead.org
List-Id: devicetree@vger.kernel.org

SGkgVGhpZXJyeSwKClNvcnJ5IGZvciB0aGUgbGF0ZSByZXNwb25zZS4KSSB0cmllZCB0byBhZGRy
ZXNzIGFsbW9zdCBhbGwgeW91ciBjb21tZW50cyBsb2NhbGx5IGZpcnN0LgpNb3JlIGZlZWRiYWNr
IGJlbG93LgoKT24gMTIvMTAvMjAxNCAwOToxNiBQTSwgVGhpZXJyeSBSZWRpbmcgd3JvdGU6Cj4g
T24gV2VkLCBEZWMgMTAsIDIwMTQgYXQgMDQ6Mzc6MjJQTSArMDgwMCwgTGl1IFlpbmcgd3JvdGU6
Cj4+IFRoaXMgcGF0Y2ggYWRkcyBpLk1YIE1JUEkgRFNJIGhvc3QgY29udHJvbGxlciBkcml2ZXIg
c3VwcG9ydC4KPj4gQ3VycmVudGx5LCB0aGUgZHJpdmVyIHN1cHBvcnRzIHRoZSBidXJzdCB3aXRo
IHN5bmMgcHVsc2VzIG1vZGUgb25seS4KPj4KPj4gU2lnbmVkLW9mZi1ieTogTGl1IFlpbmcgPFlp
bmcuTGl1QGZyZWVzY2FsZS5jb20+Cj4+IC0tLQo+PiAgIC4uLi9kZXZpY2V0cmVlL2JpbmRpbmdz
L2RybS9pbXgvbWlwaV9kc2kudHh0ICAgICAgIHwgICA4MSArKwo+PiAgIGRyaXZlcnMvZ3B1L2Ry
bS9pbXgvS2NvbmZpZyAgICAgICAgICAgICAgICAgICAgICAgIHwgICAgNiArCj4+ICAgZHJpdmVy
cy9ncHUvZHJtL2lteC9NYWtlZmlsZSAgICAgICAgICAgICAgICAgICAgICAgfCAgICAxICsKPj4g
ICBkcml2ZXJzL2dwdS9kcm0vaW14L2lteC1taXBpLWRzaS5jICAgICAgICAgICAgICAgICB8IDEw
MTcgKysrKysrKysrKysrKysrKysrKysKPj4gICA0IGZpbGVzIGNoYW5nZWQsIDExMDUgaW5zZXJ0
aW9ucygrKQo+PiAgIGNyZWF0ZSBtb2RlIDEwMDY0NCBEb2N1bWVudGF0aW9uL2RldmljZXRyZWUv
YmluZGluZ3MvZHJtL2lteC9taXBpX2RzaS50eHQKPj4gICBjcmVhdGUgbW9kZSAxMDA2NDQgZHJp
dmVycy9ncHUvZHJtL2lteC9pbXgtbWlwaS1kc2kuYwo+Pgo+PiBkaWZmIC0tZ2l0IGEvRG9jdW1l
bnRhdGlvbi9kZXZpY2V0cmVlL2JpbmRpbmdzL2RybS9pbXgvbWlwaV9kc2kudHh0IGIvRG9jdW1l
bnRhdGlvbi9kZXZpY2V0cmVlL2JpbmRpbmdzL2RybS9pbXgvbWlwaV9kc2kudHh0Cj4+IG5ldyBm
aWxlIG1vZGUgMTAwNjQ0Cj4+IGluZGV4IDAwMDAwMDAuLjNkMDdmZDcKPj4gLS0tIC9kZXYvbnVs
bAo+PiArKysgYi9Eb2N1bWVudGF0aW9uL2RldmljZXRyZWUvYmluZGluZ3MvZHJtL2lteC9taXBp
X2RzaS50eHQKPj4gQEAgLTAsMCArMSw4MSBAQAo+PiArRGV2aWNlLVRyZWUgYmluZGluZ3MgZm9y
IE1JUEkgRFNJIGhvc3QgY29udHJvbGxlcgo+PiArCj4+ICtNSVBJIERTSSBob3N0IGNvbnRyb2xs
ZXIKPj4gKz09PT09PT09PT09PT09PT09PT09PT09PQo+PiArCj4+ICtUaGUgTUlQSSBEU0kgaG9z
dCBjb250cm9sbGVyIGlzIGEgU3lub3BzeXMgRGVzaWduV2FyZSBJUC4KPj4gK0l0IGlzIGEgZGln
aXRhbCBjb3JlIHRoYXQgaW1wbGVtZW50cyBhbGwgcHJvdG9jb2wgZnVuY3Rpb25zIGRlZmluZWQK
Pj4gK2luIHRoZSBNSVBJIERTSSBzcGVjaWZpY2F0aW9uLCBwcm92aWRpbmcgYW4gaW50ZXJmYWNl
IGJldHdlZW4gdGhlCj4+ICtzeXN0ZW0gYW5kIHRoZSBNSVBJIERQSFksIGFuZCBhbGxvd2luZyBj
b21tdW5pY2F0aW9uIHdpdGggYSBNSVBJIERTSQo+PiArY29tcGxpYW50IGRpc3BsYXkuCj4+ICsK
Pj4gK1JlcXVpcmVkIHByb3BlcnRpZXM6Cj4+ICsgLSAjYWRkcmVzcy1jZWxscyA6IFNob3VsZCBi
ZSA8MT4uCj4+ICsgLSAjc2l6ZS1jZWxscyA6IFNob3VsZCBiZSA8MD4uCj4+ICsgLSBjb21wYXRp
YmxlIDogU2hvdWxkIGJlICJmc2wsaW14NnEtbWlwaS1kc2kiIGZvciBpLk1YNnEvc2RsIFNvQ3Mu
Cj4+ICsgLSByZWcgOiBQaHlzaWNhbCBiYXNlIGFkZHJlc3Mgb2YgdGhlIGNvbnRyb2xsZXIgYW5k
IGxlbmd0aCBvZiBtZW1vcnkKPj4gKyAgICAgICAgIG1hcHBlZCByZWdpb24uCj4+ICsgLSBpbnRl
cnJ1cHRzIDogVGhlIGNvbnRyb2xsZXIncyBpbnRlcnJ1cHQgbnVtYmVyIHRvIHRoZSBDUFUocyku
Cj4+ICsgLSBncHIgOiBTaG91bGQgYmUgPCZncHI+Lgo+PiArICAgICAgICAgVGhlIHBoYW5kbGUg
cG9pbnRzIHRvIHRoZSBpb211eGMtZ3ByIHJlZ2lvbiBjb250YWluaW5nIHRoZQo+PiArICAgICAg
ICAgbXVsdGlwbGV4ZXIgY29udHJvbCByZWdpc3RlciBmb3IgdGhlIGNvbnRyb2xsZXIuCj4KPiBT
aWRlLW5vdGU6IFNob3VsZG4ndCB0aGlzIHJlYWxseSBiZSBhIHBpbm11eCwgdGhlbj8KCk5vLiBU
aGUgbXV4aW5nIGlzIGluc2lkZSB0aGUgaS5NWCBTb0MuClRoZXJlIGlzIGEgRFQgYmluZGluZyBk
b2N1bWVudGF0aW9uIGZvciB0aGUgc3lzdGVtIGNvbnRyb2xsZXIgbm9kZShncHIpCmF0IFsxXS4g
QW5kLCBmb3IgaS5NWCBEVCBzb3VyY2VzLCB0aGVyZSBhcmUgc2V2ZXJhbCBleGlzdGluZyB1c2Ug
Y2FzZXMgCmluIHdoaWNoIHRoZSBncHIgbm9kZSBpcyByZWZlcnJlZCBieSBvdGhlciBub2Rlcy4K
ClsxXSBEb2N1bWVudGF0aW9uL2RldmljZXRyZWUvYmluZGluZ3MvbWZkL3N5c2Nvbi50eHQuCgo+
Cj4+ICsgLSBjbG9ja3MsIGNsb2NrLW5hbWVzIDogUGhhbmRsZXMgdG8gdGhlIGNvbnRyb2xsZXIg
cGxscmVmLCBwbGxyZWZfZ2F0ZQo+PiArICAgICAgICAgICBhbmQgY29yZV9jZmcgY2xvY2tzLCBh
cyBkZXNjcmliZWQgaW4gWzFdIGFuZCBbMl0uCj4+ICsgLSBwYW5lbEAwIDogQSBwYW5lbCBub2Rl
IHdoaWNoIGNvbnRhaW5zIGEgZGlzcGxheS10aW1pbmdzIGNoaWxkIG5vZGUgYXMKPj4gKyAgICAg
ICAgICAgICBkZWZpbmVkIGluIFszXS4KPgo+IFRoZXJlJ3Mgbm8gbmVlZCBmb3IgdGhlc2UgdG8g
YmUgbmFtZWQgcGFuZWxAKi4gVGhleSBjb3VsZCBiZSBicmlkZ2VzIGZvcgo+IGV4YW1wbGUuIEFu
ZCBubywgdGhleSBzaG91bGRuJ3QgY29udGFpbiBhIGRpc3BsYXktdGltaW5ncyBjaGlsZCBub2Rl
Cj4gZWl0aGVyLiBQYW5lbHMgc2hvdWxkIGhhdmUgYSBwcm9wZXIgZHJpdmVyIGFuZCB0aGUgZHJp
dmVyIGJlaW5nIGRldmljZQo+IHNwZWNpZmljIGl0IHNob3VsZCBoYXZlIHRoZSB0aW1pbmdzIGVt
YmVkZGVkLgoKT2ssIEknbGwgbW92ZSB0aGUgdGltaW5nIHRvIHRoZSBwYW5lbCBkcml2ZXIuCgo+
Cj4+ICsgLSBwb3J0QFswLTRdIDogVXAgdG8gZm91ciBwb3J0IG5vZGVzIHdpdGggZW5kcG9pbnQg
ZGVmaW5pdGlvbnMgYXMgZGVmaW5lZAo+PiArICAgaW4gWzRdLCBjb3JyZXNwb25kaW5nIHRvIHRo
ZSBmb3VyIGlucHV0cyB0byB0aGUgY29udHJvbGxlciBtdWx0aXBsZXhlci4KPj4gKyAgIE5vdGUg
dGhhdCBlYWNoIHBvcnQgbm9kZSBzaG91bGQgY29udGFpbiB0aGUgaW5wdXQtcG9ydCBwcm9wZXJ0
eSB0bwo+PiArICAgZGlzdGluZ3Vpc2ggaXQgZnJvbSB0aGUgcGFuZWwgbm9kZSwgYXMgZGVzY3Jp
YmVkIGluIFs1XS4KPgo+IFs0XSBzYXlzIHRoYXQgeW91IGNhbiBncm91cCBhbGwgcG9ydCBub2Rl
cyB1bmRlciBhIHBvcnRzIHBhcmVudCBub2RlLiBJCj4gdGhpbmsgdGhpcyBpcyByZWFsbHkgd2hh
dCB5b3Ugd2FudCB0byBkbyBoZXJlIHRvIG1ha2UgaXQgY2xlYXIgdGhhdCB0aGUKPiBwb3J0cyBh
cmVuJ3QgcGFydCBvZiB0aGUgRFNJIGhvc3QgYmluZGluZyBwYXJ0IG9mIHRoZSBkZXZpY2UuCj4K
CkFjY2VwdGVkLgoKPj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9pbXgvaW14LW1pcGkt
ZHNpLmMgYi9kcml2ZXJzL2dwdS9kcm0vaW14L2lteC1taXBpLWRzaS5jCj4gWy4uLl0KPj4gKy8q
Cj4+ICsgKiBpLk1YIGRybSBkcml2ZXIgLSBNSVBJIERTSSBIb3N0IENvbnRyb2xsZXIKPj4gKyAq
Cj4+ICsgKiBDb3B5cmlnaHQgKEMpIDIwMTEtMjAxNCBGcmVlc2NhbGUgU2VtaWNvbmR1Y3Rvciwg
SW5jLgo+PiArICoKPj4gKyAqIFRoaXMgcHJvZ3JhbSBpcyBmcmVlIHNvZnR3YXJlOyB5b3UgY2Fu
IHJlZGlzdHJpYnV0ZSBpdCBhbmQvb3IKPj4gKyAqIG1vZGlmeSBpdCB1bmRlciB0aGUgdGVybXMg
b2YgdGhlIEdOVSBHZW5lcmFsIFB1YmxpYyBMaWNlbnNlCj4+ICsgKiBhcyBwdWJsaXNoZWQgYnkg
dGhlIEZyZWUgU29mdHdhcmUgRm91bmRhdGlvbjsgZWl0aGVyIHZlcnNpb24gMgo+PiArICogb2Yg
dGhlIExpY2Vuc2UsIG9yIChhdCB5b3VyIG9wdGlvbikgYW55IGxhdGVyIHZlcnNpb24uCj4+ICsg
Kgo+PiArICogVGhpcyBwcm9ncmFtIGlzIGRpc3RyaWJ1dGVkIGluIHRoZSBob3BlIHRoYXQgaXQg
d2lsbCBiZSB1c2VmdWwsCj4+ICsgKiBidXQgV0lUSE9VVCBBTlkgV0FSUkFOVFk7IHdpdGhvdXQg
ZXZlbiB0aGUgaW1wbGllZCB3YXJyYW50eSBvZgo+PiArICogTUVSQ0hBTlRBQklMSVRZIG9yIEZJ
VE5FU1MgRk9SIEEgUEFSVElDVUxBUiBQVVJQT1NFLiAgU2VlIHRoZQo+PiArICogR05VIEdlbmVy
YWwgUHVibGljIExpY2Vuc2UgZm9yIG1vcmUgZGV0YWlscy4KPj4gKyAqLwo+PiArCj4+ICsjaW5j
bHVkZSA8bGludXgvY2xrLmg+Cj4+ICsjaW5jbHVkZSA8bGludXgvY29tcG9uZW50Lmg+Cj4+ICsj
aW5jbHVkZSA8bGludXgvbWZkL3N5c2Nvbi5oPgo+PiArI2luY2x1ZGUgPGxpbnV4L21mZC9zeXNj
b24vaW14NnEtaW9tdXhjLWdwci5oPgo+PiArI2luY2x1ZGUgPGxpbnV4L21vZHVsZS5oPgo+PiAr
I2luY2x1ZGUgPGxpbnV4L29mX2FkZHJlc3MuaD4KPj4gKyNpbmNsdWRlIDxsaW51eC9vZl9kZXZp
Y2UuaD4KPj4gKyNpbmNsdWRlIDxsaW51eC9yZWdtYXAuaD4KPj4gKyNpbmNsdWRlIDxsaW51eC92
aWRlb2RldjIuaD4KPj4gKyNpbmNsdWRlIDxhc20vZGl2NjQuaD4KPgo+IERvbid0IHlvdSB3YW50
IHRoZSBtb3JlIGdlbmVyaWMgbGludXgvbWF0aDY0LmggaGVyZT8KCkknbGwgdXNlIGxpbnV4L21h
dGg2NC5oLgoKPgo+PiArI2luY2x1ZGUgPGRybS9kcm1QLmg+Cj4+ICsjaW5jbHVkZSA8ZHJtL2Ry
bV9jcnRjX2hlbHBlci5oPgo+PiArI2luY2x1ZGUgPGRybS9kcm1fZmJfaGVscGVyLmg+Cj4KPiBJ
IGRvbid0IHNlZSBhbnkgb2YgdGhlIGZ1bmN0aW9ucyBkZWZpbmVkIGluIHRoYXQgaGVhZGVyIHVz
ZWQgaGVyZS4KCkknbGwgcmVtb3ZlIHRoaXMuCgo+Cj4+ICsjaW5jbHVkZSA8ZHJtL2RybV9taXBp
X2RzaS5oPgo+PiArI2luY2x1ZGUgPGRybS9kcm1fcGFuZWwuaD4KPj4gKyNpbmNsdWRlIDx2aWRl
by9taXBpX2Rpc3BsYXkuaD4KPj4gKyNpbmNsdWRlIDx2aWRlby9vZl92aWRlb21vZGUuaD4KPj4g
KyNpbmNsdWRlIDx2aWRlby92aWRlb21vZGUuaD4KPj4gKwo+PiArI2luY2x1ZGUgImlteC1kcm0u
aCIKPj4gKwo+PiArI2RlZmluZSBEUklWRVJfTkFNRSAJCQkiaW14LW1pcGktZHNpIgo+PiArCj4+
ICsjZGVmaW5lCURTSV9WRVJTSU9OCQkJMHgwMAo+PiArCj4+ICsjZGVmaW5lCURTSV9QV1JfVVAJ
CQkweDA0Cj4+ICsjZGVmaW5lCVJFU0VUCQkJCTAKPj4gKyNkZWZpbmUJUE9XRVJVUAkJCQlCSVQo
MCkKPj4gKwo+PiArI2RlZmluZQlEU0lfQ0xLTUdSX0NGRwkJCTB4MDgKPj4gKyNkZWZpbmUgVE9f
Q0xLX0RJVklEU0lPTihkaXYpCQkoKChkaXYpICYgMHhmZikgPDwgOCkKPj4gKyNkZWZpbmUgVFhf
RVNDX0NMS19ESVZJRFNJT04oZGl2KQkoKChkaXYpICYgMHhmZikgPDwgMCkKPgo+IFlvdXIgaW5k
ZW50YXRpb24gaGVyZSBpcy4uLiB3ZWlyZC4KCkknbGwgZml4IHRoaXMuCgo+Cj4+ICsjZGVmaW5l
IElNWF9NSVBJX0RTSV9NQVhfREFUQV9MQU5FUwkyCj4+ICsKPj4gKyNkZWZpbmUgUEhZX1NUQVRV
U19USU1FT1VUCQkxMAo+PiArI2RlZmluZSBDTURfUEtUX1NUQVRVU19USU1FT1VUCQkyMAo+PiAr
Cj4+ICsjZGVmaW5lIElNWF9NSVBJX0RTSV9QTERfREFUQV9CVUZfU0laRQk0Cj4+ICsKPj4gKyNk
ZWZpbmUgTUhaCQkJCTEwMDAwMDAKPgo+IFdoeSBub3QgcmV1c2UgdGhlIGV4aXN0aW5nIFVTRUNf
UEVSX1NFQz8KCkFjY2VwdGVkLgoKPgo+PiArI2RlZmluZSBob3N0X3RvX2RzaShob3N0KSBjb250
YWluZXJfb2YoaG9zdCwgc3RydWN0IGlteF9taXBpX2RzaSwgZHNpX2hvc3QpCj4+ICsjZGVmaW5l
IGNvbl90b19kc2koeCkgY29udGFpbmVyX29mKHgsIHN0cnVjdCBpbXhfbWlwaV9kc2ksIGNvbm5l
Y3RvcikKPj4gKyNkZWZpbmUgZW5jX3RvX2RzaSh4KSBjb250YWluZXJfb2YoeCwgc3RydWN0IGlt
eF9taXBpX2RzaSwgZW5jb2RlcikKPgo+IFRoZXNlIHNob3VsZCByZWFsbHkgYmUgc3RhdGljIGlu
bGluZSBmdW5jdGlvbnMgZm9yIHByb3BlciB0eXBlIHNhZmV0eS4KCk9rLCBJJ2xsIGNoYW5nZSB0
byB1c2UgZnVuY3Rpb25zLgoKPgo+PiArc3RydWN0IGlteF9taXBpX2RzaSB7Cj4+ICsJc3RydWN0
IG1pcGlfZHNpX2hvc3QgZHNpX2hvc3Q7Cj4+ICsJc3RydWN0IGRybV9jb25uZWN0b3IgY29ubmVj
dG9yOwo+PiArCXN0cnVjdCBkcm1fZW5jb2RlciBlbmNvZGVyOwo+PiArCXN0cnVjdCBkZXZpY2Vf
bm9kZSAqcGFuZWxfbm9kZTsKPj4gKwlzdHJ1Y3QgZHJtX3BhbmVsICpwYW5lbDsKPj4gKwlzdHJ1
Y3QgZGV2aWNlICpkZXY7Cj4+ICsKPj4gKwlzdHJ1Y3QgcmVnbWFwICpyZWdtYXA7Cj4+ICsJdm9p
ZCBfX2lvbWVtICpiYXNlOwo+PiArCj4+ICsJc3RydWN0IGNsayAqcGxscmVmX2NsazsKPj4gKwlz
dHJ1Y3QgY2xrICpwbGxyZWZfZ2F0ZV9jbGs7Cj4+ICsJc3RydWN0IGNsayAqY2ZnX2NsazsKPj4g
Kwo+PiArCXVuc2lnbmVkIGludCBsYW5lX21icHM7IC8qIHBlciBsYW5lICovCj4+ICsJdTMyIGNo
YW5uZWw7Cj4+ICsJdTMyIGxhbmVzOwo+PiArCXUzMiBmb3JtYXQ7Cj4+ICsJc3RydWN0IHZpZGVv
bW9kZSB2bTsKPj4gKwo+PiArCWJvb2wgZW5hYmxlZDsKPgo+IERvIHlvdSByZWFsbHkgbmVlZCB0
aGlzIGZsYWc/CgpJJ2xsIHJlbW92ZSB0aGlzIGZsYWcuCgo+Cj4+ICt9Owo+PiArCj4+ICtlbnVt
IHsKPj4gKwlTVEFUVVNfVE9fQ0xFQVIsCj4+ICsJU1RBVFVTX1RPX1NFVCwKPj4gK307Cj4+ICsK
Pj4gK3N0cnVjdCBkcGh5X3BsbF90ZXN0ZGluX21hcCB7Cj4+ICsJaW50IG1heF9tYnBzOwo+Cj4g
dW5zaWduZWQ/CgpBY2NlcHRlZC4KCj4KPj4gKwl1OCB0ZXN0ZGluOwo+PiArfTsKPj4gKwo+PiAr
LyogVGhlIHRhYmxlIGlzIGJhc2VkIG9uIDI3TUh6IERQSFkgcGxsIHJlZmVyZW5jZSBjbG9jay4g
Ki8KPj4gK3N0YXRpYyBjb25zdCBzdHJ1Y3QgZHBoeV9wbGxfdGVzdGRpbl9tYXAgZHB0ZGluX21h
cFtdID0gewo+PiArCXsxNjAsIDB4MDR9LCB7MTgwLCAweDI0fSwgezIwMCwgMHg0NH0sIHsyMTAs
IDB4MDZ9LAo+PiArCXsyNDAsIDB4MjZ9LCB7MjUwLCAweDQ2fSwgezI3MCwgMHgwOH0sIHszMDAs
IDB4Mjh9LAo+PiArCXszMzAsIDB4NDh9LCB7MzYwLCAweDJhfSwgezQwMCwgMHg0YX0sIHs0NTAs
IDB4MGN9LAo+PiArCXs1MDAsIDB4MmN9LCB7NTUwLCAweDBlfSwgezYwMCwgMHgyZX0sIHs2NTAs
IDB4MTB9LAo+PiArCXs3MDAsIDB4MzB9LCB7NzUwLCAweDEyfSwgezgwMCwgMHgzMn0sIHs4NTAs
IDB4MTR9LAo+PiArCXs5MDAsIDB4MzR9LCB7OTUwLCAweDU0fSwgezEwMDAsIDB4NzR9Cj4+ICt9
Owo+PiArCj4+ICtzdGF0aWMgaW50IG1heF9tYnBzX3RvX3Rlc3RkaW4odW5zaWduZWQgaW50IG1h
eF9tYnBzKQo+PiArewo+PiArCWludCBpOwo+PiArCj4+ICsJZm9yIChpID0gMDsgaSA8IEFSUkFZ
X1NJWkUoZHB0ZGluX21hcCk7IGkrKykKPj4gKwkJaWYgKGRwdGRpbl9tYXBbaV0ubWF4X21icHMg
PT0gbWF4X21icHMpCj4+ICsJCQlyZXR1cm4gZHB0ZGluX21hcFtpXS50ZXN0ZGluOwo+PiArCj4+
ICsJcmV0dXJuIC1FSU5WQUw7Cj4+ICt9Cj4+ICsKPj4gK3N0YXRpYyBpbnQgZm9ybWF0X3RvX2Jw
cChzdHJ1Y3QgaW14X21pcGlfZHNpICpkc2kpCj4+ICt7Cj4+ICsJc3dpdGNoIChkc2ktPmZvcm1h
dCkgewo+PiArCWNhc2UgTUlQSV9EU0lfRk1UX1JHQjg4ODoKPj4gKwljYXNlIE1JUElfRFNJX0ZN
VF9SR0I2NjY6Cj4+ICsJCXJldHVybiAyNDsKPj4gKwljYXNlIE1JUElfRFNJX0ZNVF9SR0I2NjZf
UEFDS0VEOgo+PiArCQlyZXR1cm4gMTg7Cj4+ICsJY2FzZSBNSVBJX0RTSV9GTVRfUkdCNTY1Ogo+
PiArCQlyZXR1cm4gMTY7Cj4+ICsJZGVmYXVsdDoKPj4gKwkJZGV2X2Vycihkc2ktPmRldiwgInVu
c3VwcG9ydGVkIHBpeGVsIGZvcm1hdFxuIik7Cj4+ICsJCXJldHVybiAtRUlOVkFMOwo+PiArCX0K
Pj4gK30KPgo+IElzIHRoZXJlIGEgcmVhc29uIHdoeSB0aGlzIGNhbid0IGJlIGEgc3RhbmRhcmQg
TUlQSSBEU0kgZnVuY3Rpb24/CgpIb3cgYWJvdXQgbW92aW5nIHRoaXMgdG8gaW5jbHVkZS9kcm0v
ZHJtX21pcGlfZHNpLmggYXMgYW4gaW5saW5lCmZ1bmN0aW9uPwoKPgo+PiArCj4+ICtzdGF0aWMg
aW50IGNoZWNrX3N0YXR1cyhzdHJ1Y3QgaW14X21pcGlfZHNpICpkc2ksIHUzMiByZWcsIHUzMiBz
dGF0dXMsCj4+ICsJCQlpbnQgdGltZW91dCwgYm9vbCB0b19zZXQpCj4+ICt7Cj4+ICsJdTMyIHZh
bDsKPj4gKwlib29sIG91dCA9IGZhbHNlOwo+PiArCj4+ICsJdmFsID0gZHNpX3JlYWQoZHNpLCBy
ZWcpOwo+PiArCWZvciAoOzspIHsKPj4gKwkJb3V0ID0gdG9fc2V0ID8gKHZhbCAmIHN0YXR1cykg
OiAhKHZhbCAmIHN0YXR1cyk7Cj4+ICsJCWlmIChvdXQpCj4+ICsJCQlicmVhazsKPj4gKwo+PiAr
CQlpZiAoIXRpbWVvdXQtLSkKPj4gKwkJCXJldHVybiAtRUZBVUxUOwo+PiArCj4+ICsJCW1zbGVl
cCgxKTsKPj4gKwkJdmFsID0gZHNpX3JlYWQoZHNpLCByZWcpOwo+PiArCX0KPj4gKwlyZXR1cm4g
MDsKPj4gK30KPgo+IFlvdSBzaG91bGQgcHJvYmFibHkgdXNlIGEgcHJvcGVybHkgdGltZWQgbG9v
cCBoZXJlLiBtc2xlZXAoKSBpc24ndAo+IGd1YXJhbnRlZWQgdG8gcmV0dXJuIGFmdGVyIGV4YWN0
bHkgb25lIG1pbGxpc2Vjb25kLCBzbyB5b3VyIHRpbWVvdXQgaXMKPiBuZXZlciBnb2luZyB0byBi
ZSBhY2N1cmF0ZS4gU29tZXRoaW5nIGxpa2UgdGhlIGZvbGxvd2luZyB3b3VsZCBiZSBiZXR0ZXIK
PiBpbiBteSBvcGluaW9uOgo+Cj4gCXRpbWVvdXQgPSBqaWZmaWVzICsgbXNlY3NfdG9famlmZmll
cyh0aW1lb3V0KTsKPgo+IAl3aGlsZSAodGltZV9iZWZvcmUoamlmZmllcywgdGltZW91dCkpIHsK
PiAJCS4uLgo+IAl9Cj4KPiBBbHNvIHRpbWVvdXQgc2hvdWxkIGJlIHVuc2lnbmVkIGxvbmcgaW4g
dGhhdCBjYXNlLgoKQWNjZXB0ZWQuCgo+Cj4+ICtzdGF0aWMgaW50IGlteF9taXBpX2RzaV9ob3N0
X2F0dGFjaChzdHJ1Y3QgbWlwaV9kc2lfaG9zdCAqaG9zdCwKPj4gKwkJCQkgICAgc3RydWN0IG1p
cGlfZHNpX2RldmljZSAqZGV2aWNlKQo+PiArewo+PiArCXN0cnVjdCBpbXhfbWlwaV9kc2kgKmRz
aSA9IGhvc3RfdG9fZHNpKGhvc3QpOwo+PiArCWludCByZXQ7Cj4+ICsKPj4gKwlpZiAoZGV2aWNl
LT5sYW5lcyA+IElNWF9NSVBJX0RTSV9NQVhfREFUQV9MQU5FUykgewo+PiArCQlkZXZfZXJyKGRz
aS0+ZGV2LCAidGhlIG51bWJlciBvZiBkYXRhIGxhbmVzKCVkKSBpcyB0b28gbWFueVxuIiwKPj4g
KwkJCQlkZXZpY2UtPmxhbmVzKTsKPj4gKwkJcmV0dXJuIC1FSU5WQUw7Cj4+ICsJfQo+PiArCj4+
ICsJaWYgKCEoZGV2aWNlLT5tb2RlX2ZsYWdzICYgTUlQSV9EU0lfTU9ERV9WSURFT19CVVJTVCkg
fHwKPj4gKwkgICAgIShkZXZpY2UtPm1vZGVfZmxhZ3MgJiBNSVBJX0RTSV9NT0RFX1ZJREVPX1NZ
TkNfUFVMU0UpKSB7Cj4+ICsJCWRldl9lcnIoZHNpLT5kZXYsICJkZXZpY2UgbW9kZSBpcyB1bnN1
cHBvcnRlZFxuIik7Cj4+ICsJCXJldHVybiAtRUlOVkFMOwo+PiArCX0KPj4gKwo+PiArCWRzaS0+
bGFuZXMgPSBkZXZpY2UtPmxhbmVzOwo+PiArCWRzaS0+Y2hhbm5lbCA9IGRldmljZS0+Y2hhbm5l
bDsKPj4gKwlkc2ktPmZvcm1hdCA9IGRldmljZS0+Zm9ybWF0Owo+PiArCWRzaS0+cGFuZWxfbm9k
ZSA9IGRldmljZS0+ZGV2Lm9mX25vZGU7Cj4+ICsJb2ZfZ2V0X3ZpZGVvbW9kZShkc2ktPnBhbmVs
X25vZGUsICZkc2ktPnZtLCAwKTsKPgo+IE5vLCB5b3Ugc2hvdWxkbid0IHVzZSB0aGlzLiBRdWVy
eSB0aGUgbW9kZSBmcm9tIHRoZSBwYW5lbCBpbnN0ZWFkLiBPcgo+IHJhdGhlciwgZW5jb2RlIHRo
aXMgcmVxdWlyZW1lbnQgaW4gLT5tb2RlX3ZhbGlkKCkgc2luY2UgdGhhdCdsbCBnaXZlIHlvdQo+
IGRpcmVjdCBhY2Nlc3MgdG8gdGhlIG1vZGUuCj4KPiBUaGF0IHdheSwgLT5ob3N0X2F0dGFjaCgp
IGJlY29tZXMgYSBmaWx0ZXIgZm9yIGNvbXBhdGlibGUgZGV2aWNlcywgeWV0Cj4gZGV2aWNlcyBt
YXkgc3VwcG9ydCBtdWx0aXBsZSBtb2RlcywgdGhlcmVmb3JlIC0+bW9kZV92YWxpZCgpIGNhbiBi
ZSB1c2VkCj4gdG8gZmlsdGVyIG91dCB0aG9zZSB0aGF0IGRvbid0IHdvcmsgd2l0aCB5b3VyIERT
SSBob3N0IGNvbnRyb2xsZXIuIFRoZXJlCj4gaXMgYSBzdWJ0bGUgZGlmZmVyZW5jZSBoZXJlIGJl
dHdlZW4gZGV2aWNlcyB0aGF0IGFyZSBjb21wYXRpYmxlIHdpdGggdGhlCj4gY29udHJvbGxlciBh
bmQgbW9kZXMgdGhhdCB0aGUgY29udHJvbGxlciBkb2Vzbid0IHN1cHBvcnQuCgpBY2NlcHRlZC4K
CkluIHRoZSBuZXh0IHZlcnNpb24gLT5ob3N0X2F0dGFjaCgpLCBJIHdpbGwgY2hlY2sgdGhlIHBl
cmlwaGVyYWwKY29tcGF0aWJpbGl0eS4gIEFsc28sIEknbGwgZ2V0IHRoZSBwYW5lbCBieSBjYWxs
aW5nIG9mX2RybV9maW5kX3BhbmVsKCksIAphbmQgdGhlbiBjYWxsIHRoZSBkcm1fcGFuZWxfYXR0
YWNoKCkgZnVuY3Rpb24uCkRvIHlvdSB0aGluayB0aGlzIGlzIG9rYXk/Cgo+Cj4+ICsKPj4gKwly
ZXQgPSBpbXhfbWlwaV9kc2lfZ2V0X2xhbmVfYnBzKGRzaSwgJmRzaS0+bGFuZV9tYnBzKTsKPj4g
KwlpZiAocmV0IDwgMCkKPj4gKwkJcmV0dXJuIHJldDsKPj4gKwo+PiArCXJldHVybiAwOwo+PiAr
fQo+PiArCj4+ICtzdGF0aWMgaW50IGlteF9taXBpX2RzaV9ob3N0X2RldGFjaChzdHJ1Y3QgbWlw
aV9kc2lfaG9zdCAqaG9zdCwKPj4gKwkJCQkgICAgc3RydWN0IG1pcGlfZHNpX2RldmljZSAqZGV2
aWNlKQo+PiArewo+PiArCXN0cnVjdCBpbXhfbWlwaV9kc2kgKmRzaSA9IGhvc3RfdG9fZHNpKGhv
c3QpOwo+PiArCj4+ICsJZHNpLT5wYW5lbF9ub2RlID0gTlVMTDsKPj4gKwlkc2ktPnBhbmVsID0g
TlVMTDsKPj4gKwo+PiArCXJldHVybiAwOwo+PiArfQo+Cj4gWW91J2xsIHdhbnQgdG8gZGV0YWNo
IGZyb20gdGhlIHBhbmVsIGhlcmUgYXMgd2VsbC4KCk9rLiBJJ2xsIGNhbGwgZHJtX3BhbmVsX2Rl
dGFjaCgpIGhlcmUuCgo+Cj4+ICsKPj4gK3N0YXRpYyBpbnQgaW14X21pcGlfZHNpX2dlbl9wa3Rf
aGRyX3dyaXRlKHN0cnVjdCBpbXhfbWlwaV9kc2kgKmRzaSwgdTMyIHZhbCkKPj4gK3sKPj4gKwlp
bnQgcmV0Owo+PiArCj4+ICsJcmV0ID0gY2hlY2tfc3RhdHVzKGRzaSwgRFNJX0NNRF9QS1RfU1RB
VFVTLCBHRU5fQ01EX0ZVTEwsCj4+ICsJCQkgICBDTURfUEtUX1NUQVRVU19USU1FT1VULCBTVEFU
VVNfVE9fQ0xFQVIpOwo+PiArCWlmIChyZXQgPCAwKSB7Cj4+ICsJCWRldl9lcnIoZHNpLT5kZXYs
ICJmYWlsZWQgdG8gZ2V0IGF2YWxpYWJsZSBjb21tYW5kIEZJRk9cbiIpOwo+PiArCQlyZXR1cm4g
cmV0Owo+PiArCX0KPj4gKwo+PiArCWRzaV93cml0ZShkc2ksIERTSV9HRU5fSERSLCB2YWwpOwo+
PiArCj4+ICsJcmV0ID0gY2hlY2tfc3RhdHVzKGRzaSwgRFNJX0NNRF9QS1RfU1RBVFVTLAo+PiAr
CQkJICAgR0VOX0NNRF9FTVBUWSB8IEdFTl9QTERfV19FTVBUWSwKPj4gKwkJCSAgIENNRF9QS1Rf
U1RBVFVTX1RJTUVPVVQsIFNUQVRVU19UT19TRVQpOwo+PiArCWlmIChyZXQgPCAwKSB7Cj4+ICsJ
CWRldl9lcnIoZHNpLT5kZXYsICJmYWlsZWQgdG8gd3JpdGUgY29tbWFuZCBGSUZPXG4iKTsKPj4g
KwkJcmV0dXJuIHJldDsKPj4gKwl9Cj4+ICsKPj4gKwlyZXR1cm4gMDsKPj4gK30KPj4gKwo+PiAr
c3RhdGljIGludCBpbXhfbWlwaV9kc2lfZGNzX3Nob3J0X3dyaXRlKHN0cnVjdCBpbXhfbWlwaV9k
c2kgKmRzaSwKPj4gKwkJCQkJY29uc3Qgc3RydWN0IG1pcGlfZHNpX21zZyAqbXNnKQo+PiArewo+
PiArCWNvbnN0IHUxNiAqdHhfYnVmID0gbXNnLT50eF9idWY7Cj4+ICsJdTMyIHZhbCA9IEdFTl9I
REFUQSgqdHhfYnVmKSB8IEdFTl9IVFlQRShtc2ctPnR5cGUpOwo+PiArCj4+ICsJaWYgKG1zZy0+
dHhfbGVuID4gMikgewo+PiArCQlkZXZfZXJyKGRzaS0+ZGV2LCAidG9vIGxvbmcgdHggYnVmIGxl
bmd0aCAlZCBmb3Igc2hvcnQgd3JpdGVcbiIsCj4+ICsJCQltc2ctPnR4X2xlbik7Cj4+ICsJCXJl
dHVybiAtRUlOVkFMOwo+PiArCX0KPj4gKwo+PiArCXJldHVybiBpbXhfbWlwaV9kc2lfZ2VuX3Br
dF9oZHJfd3JpdGUoZHNpLCB2YWwpOwo+PiArfQo+Cj4gSXQgc2VlbXMgbGlrZSB5b3Ugd291bGQg
cHJvZml0IGZyb20gY29udmVydGluZyB0byB0aGUgbmV3bHkgaW50cm9kdWNlZAo+IG1pcGlfZHNp
X2NyZWF0ZV9wYWNrZXQoKSBoZWxwZXIgd2hpY2ggZG9lcyBtb3N0IG9mIHRoZSBwYWNraW5nIGFs
b25nCj4gd2l0aCBzb21lIHNhbml0eSBjaGVja2luZyBmb3IgeW91LgoKSSBsb29rZWQgYXQgdGhl
IGhlbHBlci4gSXQgc2VlbXMgaXQncyBub3QgcXVpdGUgc3RyYWlnaHRmb3J3YXJkIGZvciBtZQp0
byB1c2UgaXQgaGVyZS4gIFRoZSBtZXNzYWdlIHR5cGUgaGVyZSBkZWZpbmVkIGJ5IEdFTl9IVFlQ
RSgpIGlzIDhiaXQKbG9uZywgd2hpbGUgdGhlIGhlbHBlciBzZWVtcyB0byB0YWtlIGl0IGFzIDZi
aXQgbG9uZz8KCj4KPj4gKwo+PiArc3RhdGljIGludCBpbXhfbWlwaV9kc2lfZGNzX2xvbmdfd3Jp
dGUoc3RydWN0IGlteF9taXBpX2RzaSAqZHNpLAo+PiArCQkJCSAgICAgICBjb25zdCBzdHJ1Y3Qg
bWlwaV9kc2lfbXNnICptc2cpCj4+ICt7Cj4+ICsJY29uc3QgdTMyICp0eF9idWYgPSBtc2ctPnR4
X2J1ZjsKPj4gKwlpbnQgbGVuID0gbXNnLT50eF9sZW4sIHJldDsKPj4gKwl1MzIgdmFsID0gR0VO
X0hEQVRBKG1zZy0+dHhfbGVuKSB8IEdFTl9IVFlQRShtc2ctPnR5cGUpOwo+PiArCj4+ICsJaWYg
KG1zZy0+dHhfbGVuIDwgMykgewo+PiArCQlkZXZfZXJyKGRzaS0+ZGV2LCAid3JvbmcgdHggYnVm
IGxlbmd0aCAlZCBmb3IgbG9uZyB3cml0ZVxuIiwKPj4gKwkJCW1zZy0+dHhfbGVuKTsKPj4gKwkJ
cmV0dXJuIC1FSU5WQUw7Cj4+ICsJfQo+PiArCj4+ICsJLyogd3JpdGUgdGhlIGJ1bGtzICovCj4+
ICsJd2hpbGUgKGxlbiAvIElNWF9NSVBJX0RTSV9QTERfREFUQV9CVUZfU0laRSkgewo+PiArCQlk
c2lfd3JpdGUoZHNpLCBEU0lfR0VOX1BMRF9EQVRBLCAqdHhfYnVmKTsKPj4gKwkJdHhfYnVmKys7
Cj4+ICsJCWxlbiAtPSBJTVhfTUlQSV9EU0lfUExEX0RBVEFfQlVGX1NJWkU7Cj4KPiBUaGlzIGlz
IGNvbmZ1c2luZy4gTWF5YmUgYSBiZXR0ZXIgb3B0aW9uIHdvdWxkIGJlIHRvIHVzZSBzaXplb2Yo
KnR4X2J1ZikKPiBpbnN0ZWFkIG9mIHRoaXMgbWFjcm8uIFlvdSdyZSBlZmZlY3RpdmVseSBjb25z
dW1pbmcgb25lIHUzMiB3aXRoIGVhY2gKPiB3cml0ZSwgaXJyZXNwZWN0aXZlIG9mIHdoYXQgdGhl
IHZhbHVlIG9mIHRoZSBtYWNybyBpcy4KCk9rLiBJJ2xsIHVzZSBzaXplb2YoKnR4X2J1ZikuCgo+
Cj4+ICsJCXJldCA9IGNoZWNrX3N0YXR1cyhkc2ksIERTSV9DTURfUEtUX1NUQVRVUywgR0VOX1BM
RF9XX0ZVTEwsCj4+ICsJCQkJICAgQ01EX1BLVF9TVEFUVVNfVElNRU9VVCwgU1RBVFVTX1RPX0NM
RUFSKTsKPj4gKwkJaWYgKHJldCA8IDApIHsKPj4gKwkJCWRldl9lcnIoZHNpLT5kZXYsICJmYWls
ZWQgdG8gZ2V0IGF2YWxpYWJsZSAiCj4+ICsJCQkJCSJ3cml0ZSBwYXlsb2FkIEZJRk9cbiIpOwo+
PiArCQkJcmV0dXJuIHJldDsKPj4gKwkJfQo+PiArCX0KPj4gKwo+PiArCS8qIHdyaXRlIHRoZSBy
ZW1haW5kZXIgKi8KPj4gKwlpZiAobGVuID4gMCkgewo+PiArCQlyZXQgPSBjaGVja19zdGF0dXMo
ZHNpLCBEU0lfQ01EX1BLVF9TVEFUVVMsIEdFTl9QTERfV19GVUxMLAo+PiArCQkJCSAgIENNRF9Q
S1RfU1RBVFVTX1RJTUVPVVQsIFNUQVRVU19UT19DTEVBUik7Cj4+ICsJCWlmIChyZXQgPCAwKSB7
Cj4+ICsJCQlkZXZfZXJyKGRzaS0+ZGV2LCAiZmFpbGVkIHRvIGdldCBhdmFsaWFibGUgIgo+PiAr
CQkJCQkid3JpdGUgcGF5bG9hZCBGSUZPXG4iKTsKPj4gKwkJCXJldHVybiByZXQ7Cj4+ICsJCX0K
Pj4gKwkJZHNpX3dyaXRlKGRzaSwgRFNJX0dFTl9QTERfREFUQSwgKnR4X2J1Zik7Cj4+ICsJfQo+
Cj4gVGhpcyBpcyByZWFsbHkgZHVwbGljYXRpbmcgd2hhdCB0aGUgYWJvdmUgbG9vcCBkb2VzLiBX
aHkgY2FuJ3QgeW91IGRvIGl0Cj4gaW4gYSBzaW5nbGUgbG9vcD8gQWxzbyB0aGUgdHJhbnNtaXNz
aW9uIGJ1ZmZlciBpc24ndCBndWFyYW50ZWVkIHRvIGJlIGEKPiBtdWx0aXBsZSBvZiA0IGJ5dGVz
LCBzbyB5b3UgY291bGQgaGF2ZSB0aGUgc2l0dWF0aW9uIHdoZXJlIGxlbiA+IDAgYW5kCj4gbGVu
IDwgNCBhbmQgeWV0IHlvdSdsbCBiZSByZWFkaW5nIDQgYnl0ZXMgYnkgZG9pbmcgKnR4X2J1ZiBh
bmQgYWNjZXNzaW5nCj4gbWVtb3J5IHRoYXQgeW91IHNob3VsZG4ndC4KCkdvb2QgY2F0Y2guICBJ
J2xsIG1ha2UgaXQgdG8gYmUgYSBzaW5nbGUgbG9vcCBhbmQgYXZvaWQgYWNjZXNzaW5nIG1lbW9y
eQp0aGF0IEkgc2hvdWxkbid0IGluIHRoZSBuZXh0IHZlcnNpb24uCgo+Cj4+ICsJcmV0dXJuIGlt
eF9taXBpX2RzaV9nZW5fcGt0X2hkcl93cml0ZShkc2ksIHZhbCk7Cj4+ICt9Cj4+ICsKPj4gK3N0
YXRpYyBzc2l6ZV90IGlteF9taXBpX2RzaV9ob3N0X3RyYW5zZmVyKHN0cnVjdCBtaXBpX2RzaV9o
b3N0ICpob3N0LAo+PiArCQkJCQkgIGNvbnN0IHN0cnVjdCBtaXBpX2RzaV9tc2cgKm1zZykKPj4g
K3sKPj4gKwlzdHJ1Y3QgaW14X21pcGlfZHNpICpkc2kgPSBob3N0X3RvX2RzaShob3N0KTsKPj4g
KwlpbnQgcmV0Owo+PiArCj4+ICsJc3dpdGNoIChtc2ctPnR5cGUpIHsKPj4gKwljYXNlIE1JUElf
RFNJX0RDU19TSE9SVF9XUklURToKPj4gKwljYXNlIE1JUElfRFNJX0RDU19TSE9SVF9XUklURV9Q
QVJBTToKPj4gKwljYXNlIE1JUElfRFNJX1NFVF9NQVhJTVVNX1JFVFVSTl9QQUNLRVRfU0laRToK
Pj4gKwkJcmV0ID0gaW14X21pcGlfZHNpX2Rjc19zaG9ydF93cml0ZShkc2ksIG1zZyk7Cj4+ICsJ
CWJyZWFrOwo+PiArCWNhc2UgTUlQSV9EU0lfRENTX0xPTkdfV1JJVEU6Cj4+ICsJCXJldCA9IGlt
eF9taXBpX2RzaV9kY3NfbG9uZ193cml0ZShkc2ksIG1zZyk7Cj4+ICsJCWJyZWFrOwo+PiArCWRl
ZmF1bHQ6Cj4+ICsJCWRldl9lcnIoZHNpLT5kZXYsICJ1bnN1cHBvcnRlZCBtZXNzYWdlIHR5cGVc
biIpOwo+PiArCQlyZXQgPSAtRUZBVUxUOwo+Cj4gRUZBVUxUIHJlYWxseSBpc24ndCBhcHByb3By
aWF0ZSBoZXJlLgoKSSdsbCBjaGFuZ2UgaXQgdG8gRUlOVkFMLgoKPgo+PiArCX0KPj4gKwo+PiAr
CXJldHVybiByZXQ7Cj4+ICt9Cj4+ICsKPj4gK3N0YXRpYyBjb25zdCBzdHJ1Y3QgbWlwaV9kc2lf
aG9zdF9vcHMgaW14X21pcGlfZHNpX2hvc3Rfb3BzID0gewo+PiArCS5hdHRhY2ggPSBpbXhfbWlw
aV9kc2lfaG9zdF9hdHRhY2gsCj4+ICsJLmRldGFjaCA9IGlteF9taXBpX2RzaV9ob3N0X2RldGFj
aCwKPj4gKwkudHJhbnNmZXIgPSBpbXhfbWlwaV9kc2lfaG9zdF90cmFuc2ZlciwKPj4gK307Cj4+
ICsKPj4gK3N0YXRpYyBlbnVtIGRybV9jb25uZWN0b3Jfc3RhdHVzCj4+ICtpbXhfbWlwaV9kc2lf
ZGV0ZWN0KHN0cnVjdCBkcm1fY29ubmVjdG9yICpjb25uZWN0b3IsIGJvb2wgZm9yY2UpCj4+ICt7
Cj4+ICsJc3RydWN0IGlteF9taXBpX2RzaSAqZHNpID0gY29uX3RvX2RzaShjb25uZWN0b3IpOwo+
PiArCj4+ICsJaWYgKCFkc2ktPnBhbmVsKSB7Cj4+ICsJCWRzaS0+cGFuZWwgPSBvZl9kcm1fZmlu
ZF9wYW5lbChkc2ktPnBhbmVsX25vZGUpOwo+PiArCQlpZiAoZHNpLT5wYW5lbCkKPj4gKwkJCWRy
bV9wYW5lbF9hdHRhY2goZHNpLT5wYW5lbCwgJmRzaS0+Y29ubmVjdG9yKTsKPj4gKwl9Cj4+ICsK
Pj4gKwlpZiAoZHNpLT5wYW5lbCkKPj4gKwkJcmV0dXJuIGNvbm5lY3Rvcl9zdGF0dXNfY29ubmVj
dGVkOwo+PiArCj4+ICsJcmV0dXJuIGNvbm5lY3Rvcl9zdGF0dXNfZGlzY29ubmVjdGVkOwo+PiAr
Cj4+ICt9Cj4KPiBZb3UgcmVhbGx5IHNob3VsZG4ndCBiZSBkb2luZyB0aGF0IGhlcmUuIG9mX2Ry
bV9maW5kX3BhbmVsKCkgcmV0dXJuaW5nCj4gTlVMTCBzaG91bGQgYmUgY29uc2lkZXJlZCBjYXVz
ZSBmb3IgZGVmZXJyaW5nIHByb2JlLiBJIHN1c3BlY3QgdGhhdCB0aGUKPiBkcml2ZXIgZG9lc24n
dCByZWFsbHkgaGFuZGxlIHRoYXQgdmVyeSB3ZWxsIGF0IGFsbCwgdGhvdWdoLCBzaW5jZSBpZiBp
dAo+IGRpZCB5b3Ugd291bGQndmUgcnVuIGludG8gdGhlIHNhbWUgaXNzdWUgdGhhdCBCZW5qYW1p
biByYW4gaW50byB0aGlzCj4gbW9ybmluZy4KCkknbGwgcmV0dXJuIGNvbm5lY3Rvcl9zdGF0dXNf
ZGlzY29ubmVjdGVkIGRpcmVjdGx5IGhlcmUuIElzIGl0IG9rYXk/Cgo+Cj4+ICtzdGF0aWMgdm9p
ZCBpbXhfbWlwaV9kc2lfZW5jb2Rlcl9wcmVwYXJlKHN0cnVjdCBkcm1fZW5jb2RlciAqZW5jb2Rl
cikKPj4gK3sKPj4gKwlzdHJ1Y3QgaW14X21pcGlfZHNpICpkc2kgPSBlbmNfdG9fZHNpKGVuY29k
ZXIpOwo+PiArCXUzMiBpbnRlcmZhY2VfcGl4X2ZtdDsKPj4gKwo+PiArCXN3aXRjaCAoZHNpLT5m
b3JtYXQpIHsKPj4gKwljYXNlIE1JUElfRFNJX0ZNVF9SR0I4ODg6Cj4+ICsJCWludGVyZmFjZV9w
aXhfZm10ID0gVjRMMl9QSVhfRk1UX1JHQjI0Owo+PiArCQlicmVhazsKPj4gKwljYXNlIE1JUElf
RFNJX0ZNVF9SR0I1NjU6Cj4+ICsJCWludGVyZmFjZV9waXhfZm10ID0gVjRMMl9QSVhfRk1UX1JH
QjU2NTsKPj4gKwkJYnJlYWs7Cj4+ICsJZGVmYXVsdDoKPj4gKwkJZGV2X2Vycihkc2ktPmRldiwg
InVuc3VwcG9ydGVkIERTSSBwaXhlbCBmb3JtYXRcbiIpOwo+PiArCQlyZXR1cm47Cj4KPiBXaHkg
ZXZlbiB0cnkgZG9pbmcgdGhpcyBpZiB5b3Uga25vdyB1cGZyb250IHRoYXQgaXQgY2FuJ3QgYmUg
ZG9uZS4gWW91Cj4ga25vdyBtdWNoIGVhcmxpZXIgdGhhbiB0aGlzIHRoYXQgeW91IGNhbid0IGRy
aXZlIHRoZSBwaXhlbCBmb3JtYXQsIHdoeQo+IG5vdCBhYm9ydCB0aGVuPwo+Cj4gUGVvcGxlIGFy
ZSBtdWNoIG1vcmUgbGlrZWx5IHRvIG5vdGljZSB0aGF0IHRoZSBwYW5lbCBpc24ndCBzdXBwb3J0
ZWQgaWYKPiB0aGUgRFNJIG91dHB1dCBkb2Vzbid0IGV2ZW4gc2hvdyB1cCAob3IgZG9lc24ndCBl
eHBvc2UgYW55IG1vZGVzKSByYXRoZXIKPiB0aGFuIGlmIHRoZXkgaGF2ZSB0byBmaW5kIHRoaXMg
ZXJyb3IgbWVzc2FnZSBpbiBkbWVzZy4KCkFjY2VwdGVkLiBJJ2xsIGNoZWNrIHRoZSBkc2kgZm9y
bWF0IGNvbXBhdGliaWxpdHkgaW4gLT5ob3N0X2F0dGFjaCgpLgpBbmQsIGhvdyBhYm91dCBjaGFu
Z2luZyB0aGUgZGVmYXVsdCBwYXRjaCB0byBiZSB0aGlzPwoKCWRlZmF1bHQ6CgkJQlVHKCk7CgkJ
cmV0dXJuOwo+Cj4+ICtzdGF0aWMgdm9pZCBpbXhfbWlwaV9kc2lfdmlkZW9fbW9kZV9jb25maWco
c3RydWN0IGlteF9taXBpX2RzaSAqZHNpKQo+PiArewo+PiArCXUzMiB2YWw7Cj4+ICsKPj4gKwl2
YWwgPSBWSURfTU9ERV9UWVBFX0JVUlNUX1NZTkNfUFVMU0VTIHwgRU5BQkxFX0xPV19QT1dFUjsK
Pj4gKwo+PiArCWRzaV93cml0ZShkc2ksIERTSV9WSURfTU9ERV9DRkcsIHZhbCk7Cj4+ICt9Cj4K
PiBZb3UgcHJvYmFibHkgd2FudCB0byBwYXJhbWV0ZXJpemUgYmFzZWQgb24gdGhlIERTSSBwZXJp
cGhlcmFsJ3MgZmxhZ3MuIEkKPiBzZWUgdGhhdCB5b3UgZG8gcmVqZWN0IGRldmljZXMgd2l0aCBh
bnkgb3RoZXIgbW9kZXMsIHNvIHRoaXMgbWF5IG5vdCBiZQo+IG5lY2Vzc2FyeSBub3cuIE91dCBv
ZiBjdXJpb3NpdHksIHRoZSBoYXJkd2FyZSBzdXBwb3J0cyBvdGhlciBtb2RlcywKPiByaWdodD8K
ClllcywgdGhlIGhhcmR3YXJlIHN1cHBvcnRzIG90aGVyIG1vZGVzIGFjY29yZGluZyB0byBpdCdz
IHJlZ2lzdGVyCmRlZmluaXRpb24uCkkgcHJlZmVyIG5vdCB0byBwYXJhbWV0ZXJpemUgYXQgcHJl
c2VudCBhcyBJIGRvIHJlamVjdCBkZXZpY2VzIHdpdGggCm90aGVyIG1vZGVzIG5vdy4KCj4KPiBb
Li4uXQo+PiArTU9EVUxFX0xJQ0VOU0UoIkdQTCB2MiIpOwo+Cj4gU2lnaC4uLiBhY2NvcmRpbmcg
dG8geW91ciBoZWFkZXIgY29tbWVudCB0aGlzIG5lZWRzIHRvIGJlICJHUEwiLgoKSSdsbCBjaGFu
Z2UgaXQgdG8gYmUgIkdQTCIuCgpUaGFua3MsCgpMaXUgWWluZwoKPgo+IFRoaWVycnkKPgpfX19f
X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFp
bGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cDovL2xpc3RzLmZy
ZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo=

From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1751904AbaLQJk0 (ORCPT <rfc822;w@1wt.eu>);
	Wed, 17 Dec 2014 04:40:26 -0500
Received: from mail-bn1on0142.outbound.protection.outlook.com ([157.56.110.142]:9865
	"EHLO na01-bn1-obe.outbound.protection.outlook.com"
	rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP
	id S1750829AbaLQJkU (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
	Wed, 17 Dec 2014 04:40:20 -0500
Message-ID: <54915081.6020508@freescale.com>
Date: Wed, 17 Dec 2014 17:44:33 +0800
From: Liu Ying <Ying.Liu@freescale.com>
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0
MIME-Version: 1.0
To: Thierry Reding <thierry.reding@gmail.com>
CC: <dri-devel@lists.freedesktop.org>, <devicetree@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>,
        <p.zabel@pengutronix.de>, <shawn.guo@linaro.org>,
        <kernel@pengutronix.de>, <linux@arm.linux.org.uk>,
        <mturquette@linaro.org>, <airlied@linux.ie>
Subject: Re: [PATCH RFC 09/15] drm: imx: Add MIPI DSI host controller driver
References: <1418200648-32656-1-git-send-email-Ying.Liu@freescale.com>
 <1418200648-32656-10-git-send-email-Ying.Liu@freescale.com>
 <20141210131640.GD23558@ulmo.nvidia.com>
In-Reply-To: <20141210131640.GD23558@ulmo.nvidia.com>
Content-Type: text/plain; charset="windows-1252"; format=flowed
Content-Transfer-Encoding: 7bit
X-EOPAttributedMessage: 0
Authentication-Results: spf=fail (sender IP is 192.88.168.50)
 smtp.mailfrom=Ying.Liu@freescale.com; 
X-Forefront-Antispam-Report: CIP:192.88.168.50;CTRY:US;IPV:NLI;EFV:NLI;SFV:NSPM;SFS:(10019020)(6009001)(199003)(51704005)(377454003)(24454002)(164054003)(479174004)(189002)(20776003)(83506001)(64706001)(107046002)(6806004)(85426001)(65816999)(92566001)(99396003)(106466001)(23746002)(65956001)(2950100001)(59896002)(68736005)(110136001)(62966003)(117636001)(50466002)(87936001)(64126003)(104016003)(19580405001)(77096005)(4396001)(54356999)(47776003)(31966008)(120916001)(21056001)(19580395003)(86362001)(50986999)(87266999)(77156002)(99136001)(65806001)(76176999)(84676001)(97736003)(46102003)(105606002)(575784001)(36756003)(2004002)(217873001)(62816006);DIR:OUT;SFP:1102;SCL:1;SRVR:CY1PR0301MB0636;H:tx30smr01.am.freescale.net;FPR:;SPF:Fail;MLV:sfv;PTR:InfoDomainNonexistent;A:1;MX:1;LANG:en;
X-Microsoft-Antispam: UriScan:;
X-Microsoft-Antispam: BCL:0;PCL:0;RULEID:;SRVR:CY1PR0301MB0636;
X-Exchange-Antispam-Report-Test: UriScan:;
X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004);SRVR:CY1PR0301MB0636;
X-Forefront-PRVS: 042857DBB5
X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:;SRVR:CY1PR0301MB0636;
X-OriginatorOrg: freescale.com
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Thierry,

Sorry for the late response.
I tried to address almost all your comments locally first.
More feedback below.

On 12/10/2014 09:16 PM, Thierry Reding wrote:
> On Wed, Dec 10, 2014 at 04:37:22PM +0800, Liu Ying wrote:
>> This patch adds i.MX MIPI DSI host controller driver support.
>> Currently, the driver supports the burst with sync pulses mode only.
>>
>> Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
>> ---
>>   .../devicetree/bindings/drm/imx/mipi_dsi.txt       |   81 ++
>>   drivers/gpu/drm/imx/Kconfig                        |    6 +
>>   drivers/gpu/drm/imx/Makefile                       |    1 +
>>   drivers/gpu/drm/imx/imx-mipi-dsi.c                 | 1017 ++++++++++++++++++++
>>   4 files changed, 1105 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/drm/imx/mipi_dsi.txt
>>   create mode 100644 drivers/gpu/drm/imx/imx-mipi-dsi.c
>>
>> diff --git a/Documentation/devicetree/bindings/drm/imx/mipi_dsi.txt b/Documentation/devicetree/bindings/drm/imx/mipi_dsi.txt
>> new file mode 100644
>> index 0000000..3d07fd7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/drm/imx/mipi_dsi.txt
>> @@ -0,0 +1,81 @@
>> +Device-Tree bindings for MIPI DSI host controller
>> +
>> +MIPI DSI host controller
>> +========================
>> +
>> +The MIPI DSI host controller is a Synopsys DesignWare IP.
>> +It is a digital core that implements all protocol functions defined
>> +in the MIPI DSI specification, providing an interface between the
>> +system and the MIPI DPHY, and allowing communication with a MIPI DSI
>> +compliant display.
>> +
>> +Required properties:
>> + - #address-cells : Should be <1>.
>> + - #size-cells : Should be <0>.
>> + - compatible : Should be "fsl,imx6q-mipi-dsi" for i.MX6q/sdl SoCs.
>> + - reg : Physical base address of the controller and length of memory
>> +         mapped region.
>> + - interrupts : The controller's interrupt number to the CPU(s).
>> + - gpr : Should be <&gpr>.
>> +         The phandle points to the iomuxc-gpr region containing the
>> +         multiplexer control register for the controller.
>
> Side-note: Shouldn't this really be a pinmux, then?

No. The muxing is inside the i.MX SoC.
There is a DT binding documentation for the system controller node(gpr)
at [1]. And, for i.MX DT sources, there are several existing use cases 
in which the gpr node is referred by other nodes.

[1] Documentation/devicetree/bindings/mfd/syscon.txt.

>
>> + - clocks, clock-names : Phandles to the controller pllref, pllref_gate
>> +           and core_cfg clocks, as described in [1] and [2].
>> + - panel@0 : A panel node which contains a display-timings child node as
>> +             defined in [3].
>
> There's no need for these to be named panel@*. They could be bridges for
> example. And no, they shouldn't contain a display-timings child node
> either. Panels should have a proper driver and the driver being device
> specific it should have the timings embedded.

Ok, I'll move the timing to the panel driver.

>
>> + - port@[0-4] : Up to four port nodes with endpoint definitions as defined
>> +   in [4], corresponding to the four inputs to the controller multiplexer.
>> +   Note that each port node should contain the input-port property to
>> +   distinguish it from the panel node, as described in [5].
>
> [4] says that you can group all port nodes under a ports parent node. I
> think this is really what you want to do here to make it clear that the
> ports aren't part of the DSI host binding part of the device.
>

Accepted.

>> diff --git a/drivers/gpu/drm/imx/imx-mipi-dsi.c b/drivers/gpu/drm/imx/imx-mipi-dsi.c
> [...]
>> +/*
>> + * i.MX drm driver - MIPI DSI Host Controller
>> + *
>> + * Copyright (C) 2011-2014 Freescale Semiconductor, Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * as published by the Free Software Foundation; either version 2
>> + * of the License, or (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/component.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
>> +#include <linux/module.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/videodev2.h>
>> +#include <asm/div64.h>
>
> Don't you want the more generic linux/math64.h here?

I'll use linux/math64.h.

>
>> +#include <drm/drmP.h>
>> +#include <drm/drm_crtc_helper.h>
>> +#include <drm/drm_fb_helper.h>
>
> I don't see any of the functions defined in that header used here.

I'll remove this.

>
>> +#include <drm/drm_mipi_dsi.h>
>> +#include <drm/drm_panel.h>
>> +#include <video/mipi_display.h>
>> +#include <video/of_videomode.h>
>> +#include <video/videomode.h>
>> +
>> +#include "imx-drm.h"
>> +
>> +#define DRIVER_NAME 			"imx-mipi-dsi"
>> +
>> +#define	DSI_VERSION			0x00
>> +
>> +#define	DSI_PWR_UP			0x04
>> +#define	RESET				0
>> +#define	POWERUP				BIT(0)
>> +
>> +#define	DSI_CLKMGR_CFG			0x08
>> +#define TO_CLK_DIVIDSION(div)		(((div) & 0xff) << 8)
>> +#define TX_ESC_CLK_DIVIDSION(div)	(((div) & 0xff) << 0)
>
> Your indentation here is... weird.

I'll fix this.

>
>> +#define IMX_MIPI_DSI_MAX_DATA_LANES	2
>> +
>> +#define PHY_STATUS_TIMEOUT		10
>> +#define CMD_PKT_STATUS_TIMEOUT		20
>> +
>> +#define IMX_MIPI_DSI_PLD_DATA_BUF_SIZE	4
>> +
>> +#define MHZ				1000000
>
> Why not reuse the existing USEC_PER_SEC?

Accepted.

>
>> +#define host_to_dsi(host) container_of(host, struct imx_mipi_dsi, dsi_host)
>> +#define con_to_dsi(x) container_of(x, struct imx_mipi_dsi, connector)
>> +#define enc_to_dsi(x) container_of(x, struct imx_mipi_dsi, encoder)
>
> These should really be static inline functions for proper type safety.

Ok, I'll change to use functions.

>
>> +struct imx_mipi_dsi {
>> +	struct mipi_dsi_host dsi_host;
>> +	struct drm_connector connector;
>> +	struct drm_encoder encoder;
>> +	struct device_node *panel_node;
>> +	struct drm_panel *panel;
>> +	struct device *dev;
>> +
>> +	struct regmap *regmap;
>> +	void __iomem *base;
>> +
>> +	struct clk *pllref_clk;
>> +	struct clk *pllref_gate_clk;
>> +	struct clk *cfg_clk;
>> +
>> +	unsigned int lane_mbps; /* per lane */
>> +	u32 channel;
>> +	u32 lanes;
>> +	u32 format;
>> +	struct videomode vm;
>> +
>> +	bool enabled;
>
> Do you really need this flag?

I'll remove this flag.

>
>> +};
>> +
>> +enum {
>> +	STATUS_TO_CLEAR,
>> +	STATUS_TO_SET,
>> +};
>> +
>> +struct dphy_pll_testdin_map {
>> +	int max_mbps;
>
> unsigned?

Accepted.

>
>> +	u8 testdin;
>> +};
>> +
>> +/* The table is based on 27MHz DPHY pll reference clock. */
>> +static const struct dphy_pll_testdin_map dptdin_map[] = {
>> +	{160, 0x04}, {180, 0x24}, {200, 0x44}, {210, 0x06},
>> +	{240, 0x26}, {250, 0x46}, {270, 0x08}, {300, 0x28},
>> +	{330, 0x48}, {360, 0x2a}, {400, 0x4a}, {450, 0x0c},
>> +	{500, 0x2c}, {550, 0x0e}, {600, 0x2e}, {650, 0x10},
>> +	{700, 0x30}, {750, 0x12}, {800, 0x32}, {850, 0x14},
>> +	{900, 0x34}, {950, 0x54}, {1000, 0x74}
>> +};
>> +
>> +static int max_mbps_to_testdin(unsigned int max_mbps)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(dptdin_map); i++)
>> +		if (dptdin_map[i].max_mbps == max_mbps)
>> +			return dptdin_map[i].testdin;
>> +
>> +	return -EINVAL;
>> +}
>> +
>> +static int format_to_bpp(struct imx_mipi_dsi *dsi)
>> +{
>> +	switch (dsi->format) {
>> +	case MIPI_DSI_FMT_RGB888:
>> +	case MIPI_DSI_FMT_RGB666:
>> +		return 24;
>> +	case MIPI_DSI_FMT_RGB666_PACKED:
>> +		return 18;
>> +	case MIPI_DSI_FMT_RGB565:
>> +		return 16;
>> +	default:
>> +		dev_err(dsi->dev, "unsupported pixel format\n");
>> +		return -EINVAL;
>> +	}
>> +}
>
> Is there a reason why this can't be a standard MIPI DSI function?

How about moving this to include/drm/drm_mipi_dsi.h as an inline
function?

>
>> +
>> +static int check_status(struct imx_mipi_dsi *dsi, u32 reg, u32 status,
>> +			int timeout, bool to_set)
>> +{
>> +	u32 val;
>> +	bool out = false;
>> +
>> +	val = dsi_read(dsi, reg);
>> +	for (;;) {
>> +		out = to_set ? (val & status) : !(val & status);
>> +		if (out)
>> +			break;
>> +
>> +		if (!timeout--)
>> +			return -EFAULT;
>> +
>> +		msleep(1);
>> +		val = dsi_read(dsi, reg);
>> +	}
>> +	return 0;
>> +}
>
> You should probably use a properly timed loop here. msleep() isn't
> guaranteed to return after exactly one millisecond, so your timeout is
> never going to be accurate. Something like the following would be better
> in my opinion:
>
> 	timeout = jiffies + msecs_to_jiffies(timeout);
>
> 	while (time_before(jiffies, timeout)) {
> 		...
> 	}
>
> Also timeout should be unsigned long in that case.

Accepted.

>
>> +static int imx_mipi_dsi_host_attach(struct mipi_dsi_host *host,
>> +				    struct mipi_dsi_device *device)
>> +{
>> +	struct imx_mipi_dsi *dsi = host_to_dsi(host);
>> +	int ret;
>> +
>> +	if (device->lanes > IMX_MIPI_DSI_MAX_DATA_LANES) {
>> +		dev_err(dsi->dev, "the number of data lanes(%d) is too many\n",
>> +				device->lanes);
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (!(device->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) ||
>> +	    !(device->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)) {
>> +		dev_err(dsi->dev, "device mode is unsupported\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	dsi->lanes = device->lanes;
>> +	dsi->channel = device->channel;
>> +	dsi->format = device->format;
>> +	dsi->panel_node = device->dev.of_node;
>> +	of_get_videomode(dsi->panel_node, &dsi->vm, 0);
>
> No, you shouldn't use this. Query the mode from the panel instead. Or
> rather, encode this requirement in ->mode_valid() since that'll give you
> direct access to the mode.
>
> That way, ->host_attach() becomes a filter for compatible devices, yet
> devices may support multiple modes, therefore ->mode_valid() can be used
> to filter out those that don't work with your DSI host controller. There
> is a subtle difference here between devices that are compatible with the
> controller and modes that the controller doesn't support.

Accepted.

In the next version ->host_attach(), I will check the peripheral
compatibility.  Also, I'll get the panel by calling of_drm_find_panel(), 
and then call the drm_panel_attach() function.
Do you think this is okay?

>
>> +
>> +	ret = imx_mipi_dsi_get_lane_bps(dsi, &dsi->lane_mbps);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	return 0;
>> +}
>> +
>> +static int imx_mipi_dsi_host_detach(struct mipi_dsi_host *host,
>> +				    struct mipi_dsi_device *device)
>> +{
>> +	struct imx_mipi_dsi *dsi = host_to_dsi(host);
>> +
>> +	dsi->panel_node = NULL;
>> +	dsi->panel = NULL;
>> +
>> +	return 0;
>> +}
>
> You'll want to detach from the panel here as well.

Ok. I'll call drm_panel_detach() here.

>
>> +
>> +static int imx_mipi_dsi_gen_pkt_hdr_write(struct imx_mipi_dsi *dsi, u32 val)
>> +{
>> +	int ret;
>> +
>> +	ret = check_status(dsi, DSI_CMD_PKT_STATUS, GEN_CMD_FULL,
>> +			   CMD_PKT_STATUS_TIMEOUT, STATUS_TO_CLEAR);
>> +	if (ret < 0) {
>> +		dev_err(dsi->dev, "failed to get avaliable command FIFO\n");
>> +		return ret;
>> +	}
>> +
>> +	dsi_write(dsi, DSI_GEN_HDR, val);
>> +
>> +	ret = check_status(dsi, DSI_CMD_PKT_STATUS,
>> +			   GEN_CMD_EMPTY | GEN_PLD_W_EMPTY,
>> +			   CMD_PKT_STATUS_TIMEOUT, STATUS_TO_SET);
>> +	if (ret < 0) {
>> +		dev_err(dsi->dev, "failed to write command FIFO\n");
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int imx_mipi_dsi_dcs_short_write(struct imx_mipi_dsi *dsi,
>> +					const struct mipi_dsi_msg *msg)
>> +{
>> +	const u16 *tx_buf = msg->tx_buf;
>> +	u32 val = GEN_HDATA(*tx_buf) | GEN_HTYPE(msg->type);
>> +
>> +	if (msg->tx_len > 2) {
>> +		dev_err(dsi->dev, "too long tx buf length %d for short write\n",
>> +			msg->tx_len);
>> +		return -EINVAL;
>> +	}
>> +
>> +	return imx_mipi_dsi_gen_pkt_hdr_write(dsi, val);
>> +}
>
> It seems like you would profit from converting to the newly introduced
> mipi_dsi_create_packet() helper which does most of the packing along
> with some sanity checking for you.

I looked at the helper. It seems it's not quite straightforward for me
to use it here.  The message type here defined by GEN_HTYPE() is 8bit
long, while the helper seems to take it as 6bit long?

>
>> +
>> +static int imx_mipi_dsi_dcs_long_write(struct imx_mipi_dsi *dsi,
>> +				       const struct mipi_dsi_msg *msg)
>> +{
>> +	const u32 *tx_buf = msg->tx_buf;
>> +	int len = msg->tx_len, ret;
>> +	u32 val = GEN_HDATA(msg->tx_len) | GEN_HTYPE(msg->type);
>> +
>> +	if (msg->tx_len < 3) {
>> +		dev_err(dsi->dev, "wrong tx buf length %d for long write\n",
>> +			msg->tx_len);
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* write the bulks */
>> +	while (len / IMX_MIPI_DSI_PLD_DATA_BUF_SIZE) {
>> +		dsi_write(dsi, DSI_GEN_PLD_DATA, *tx_buf);
>> +		tx_buf++;
>> +		len -= IMX_MIPI_DSI_PLD_DATA_BUF_SIZE;
>
> This is confusing. Maybe a better option would be to use sizeof(*tx_buf)
> instead of this macro. You're effectively consuming one u32 with each
> write, irrespective of what the value of the macro is.

Ok. I'll use sizeof(*tx_buf).

>
>> +		ret = check_status(dsi, DSI_CMD_PKT_STATUS, GEN_PLD_W_FULL,
>> +				   CMD_PKT_STATUS_TIMEOUT, STATUS_TO_CLEAR);
>> +		if (ret < 0) {
>> +			dev_err(dsi->dev, "failed to get avaliable "
>> +					"write payload FIFO\n");
>> +			return ret;
>> +		}
>> +	}
>> +
>> +	/* write the remainder */
>> +	if (len > 0) {
>> +		ret = check_status(dsi, DSI_CMD_PKT_STATUS, GEN_PLD_W_FULL,
>> +				   CMD_PKT_STATUS_TIMEOUT, STATUS_TO_CLEAR);
>> +		if (ret < 0) {
>> +			dev_err(dsi->dev, "failed to get avaliable "
>> +					"write payload FIFO\n");
>> +			return ret;
>> +		}
>> +		dsi_write(dsi, DSI_GEN_PLD_DATA, *tx_buf);
>> +	}
>
> This is really duplicating what the above loop does. Why can't you do it
> in a single loop? Also the transmission buffer isn't guaranteed to be a
> multiple of 4 bytes, so you could have the situation where len > 0 and
> len < 4 and yet you'll be reading 4 bytes by doing *tx_buf and accessing
> memory that you shouldn't.

Good catch.  I'll make it to be a single loop and avoid accessing memory
that I shouldn't in the next version.

>
>> +	return imx_mipi_dsi_gen_pkt_hdr_write(dsi, val);
>> +}
>> +
>> +static ssize_t imx_mipi_dsi_host_transfer(struct mipi_dsi_host *host,
>> +					  const struct mipi_dsi_msg *msg)
>> +{
>> +	struct imx_mipi_dsi *dsi = host_to_dsi(host);
>> +	int ret;
>> +
>> +	switch (msg->type) {
>> +	case MIPI_DSI_DCS_SHORT_WRITE:
>> +	case MIPI_DSI_DCS_SHORT_WRITE_PARAM:
>> +	case MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE:
>> +		ret = imx_mipi_dsi_dcs_short_write(dsi, msg);
>> +		break;
>> +	case MIPI_DSI_DCS_LONG_WRITE:
>> +		ret = imx_mipi_dsi_dcs_long_write(dsi, msg);
>> +		break;
>> +	default:
>> +		dev_err(dsi->dev, "unsupported message type\n");
>> +		ret = -EFAULT;
>
> EFAULT really isn't appropriate here.

I'll change it to EINVAL.

>
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static const struct mipi_dsi_host_ops imx_mipi_dsi_host_ops = {
>> +	.attach = imx_mipi_dsi_host_attach,
>> +	.detach = imx_mipi_dsi_host_detach,
>> +	.transfer = imx_mipi_dsi_host_transfer,
>> +};
>> +
>> +static enum drm_connector_status
>> +imx_mipi_dsi_detect(struct drm_connector *connector, bool force)
>> +{
>> +	struct imx_mipi_dsi *dsi = con_to_dsi(connector);
>> +
>> +	if (!dsi->panel) {
>> +		dsi->panel = of_drm_find_panel(dsi->panel_node);
>> +		if (dsi->panel)
>> +			drm_panel_attach(dsi->panel, &dsi->connector);
>> +	}
>> +
>> +	if (dsi->panel)
>> +		return connector_status_connected;
>> +
>> +	return connector_status_disconnected;
>> +
>> +}
>
> You really shouldn't be doing that here. of_drm_find_panel() returning
> NULL should be considered cause for deferring probe. I suspect that the
> driver doesn't really handle that very well at all, though, since if it
> did you would've run into the same issue that Benjamin ran into this
> morning.

I'll return connector_status_disconnected directly here. Is it okay?

>
>> +static void imx_mipi_dsi_encoder_prepare(struct drm_encoder *encoder)
>> +{
>> +	struct imx_mipi_dsi *dsi = enc_to_dsi(encoder);
>> +	u32 interface_pix_fmt;
>> +
>> +	switch (dsi->format) {
>> +	case MIPI_DSI_FMT_RGB888:
>> +		interface_pix_fmt = V4L2_PIX_FMT_RGB24;
>> +		break;
>> +	case MIPI_DSI_FMT_RGB565:
>> +		interface_pix_fmt = V4L2_PIX_FMT_RGB565;
>> +		break;
>> +	default:
>> +		dev_err(dsi->dev, "unsupported DSI pixel format\n");
>> +		return;
>
> Why even try doing this if you know upfront that it can't be done. You
> know much earlier than this that you can't drive the pixel format, why
> not abort then?
>
> People are much more likely to notice that the panel isn't supported if
> the DSI output doesn't even show up (or doesn't expose any modes) rather
> than if they have to find this error message in dmesg.

Accepted. I'll check the dsi format compatibility in ->host_attach().
And, how about changing the default patch to be this?

	default:
		BUG();
		return;
>
>> +static void imx_mipi_dsi_video_mode_config(struct imx_mipi_dsi *dsi)
>> +{
>> +	u32 val;
>> +
>> +	val = VID_MODE_TYPE_BURST_SYNC_PULSES | ENABLE_LOW_POWER;
>> +
>> +	dsi_write(dsi, DSI_VID_MODE_CFG, val);
>> +}
>
> You probably want to parameterize based on the DSI peripheral's flags. I
> see that you do reject devices with any other modes, so this may not be
> necessary now. Out of curiosity, the hardware supports other modes,
> right?

Yes, the hardware supports other modes according to it's register
definition.
I prefer not to parameterize at present as I do reject devices with 
other modes now.

>
> [...]
>> +MODULE_LICENSE("GPL v2");
>
> Sigh... according to your header comment this needs to be "GPL".

I'll change it to be "GPL".

Thanks,

Liu Ying

>
> Thierry
>