linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/15] Improvements and fixes for mxsfb DRM driver
@ 2019-08-14 10:48 Robert Chiras
  2019-08-14 10:48 ` [PATCH v2 01/15] drm/mxsfb: Update mxsfb to support a bridge Robert Chiras
                   ` (14 more replies)
  0 siblings, 15 replies; 27+ messages in thread
From: Robert Chiras @ 2019-08-14 10:48 UTC (permalink / raw)
  To: Guido Günther, Marek Vasut, Stefan Agner, David Airlie,
	Daniel Vetter, Rob Herring, Mark Rutland, Shawn Guo,
	Sascha Hauer, Fabio Estevam
  Cc: devicetree, linux-kernel, dri-devel, NXP Linux Team,
	Pengutronix Kernel Team, linux-arm-kernel

This patch-set improves the use of eLCDIF block on iMX 8 SoCs (like 8MQ, 8MM
and 8QXP). Following, are the new features added and fixes from this
patch-set:

1. Add support for drm_bridge
On 8MQ and 8MM, the LCDIF block is not directly connected to a parallel
display connector, where an LCD panel can be attached, but instead it is
connected to DSI controller. Since this DSI stands between the display
controller (eLCDIF) and the physical connector, the DSI can be implemented
as a DRM bridge. So, in order to be able to connect the mxsfb driver to
the DSI driver, the support for a drm_bridge was needed in mxsfb DRM
driver (the actual driver for the eLCDIF block).

2. Add support for additional pixel formats
Some of the pixel formats needed by Android were not implemented in this
driver, but they were actually supported. So, add support for them.

3. Add support for horizontal stride
Having support for horizontal stride allows the use of eLCDIF with a GPU
(for example) that can only output resolution sizes multiple of a power of
8. For example, 1080 is not a power of 16, so in order to support 1920x1080
output from GPUs that can produce linear buffers only in sizes multiple to 16,
this feature is needed.

3. Few minor features and bug-fixing
The addition of max-res DT property was actually needed in order to limit
the bandwidth usage of the eLCDIF block. This is need on systems where
multiple display controllers are presend and the memory bandwidth is not
enough to handle all of them at maximum capacity (like it is the case on
8MQ, where there are two display controllers: DCSS and eLCDIF).
The rest of the patches are bug-fixes.

v2:
- Collected Tested-by from Guido
- Split the first patch, which added more than one feature into relevant
  patches, explaining each feature added
- Also split the second patch into more patches, to differentiate between
  the feature itself (additional pixel formats support) and the cleanup
  of the register definitions for a better representation (guido)
- Included a patch submitted by Guido, while he was testing my patch-set

Guido Günther (1):
  drm/mxsfb: Read bus flags from bridge if present

Mirela Rabulea (1):
  drm/mxsfb: Signal mode changed when bpp changed

Robert Chiras (13):
  drm/mxsfb: Update mxsfb to support a bridge
  drm/mxsfb: Add defines for the rest of registers
  drm/mxsfb: Reset vital register for a proper initialization
  drm/mxsfb: Update register definitions using bit manipulation defines
  drm/mxsfb: Update mxsfb with additional pixel formats
  drm/mxsfb: Fix the vblank events
  dt-bindings: display: Add max-res property for mxsfb
  drm/mxsfb: Add max-res property for MXSFB
  drm/mxsfb: Update mxsfb to support LCD reset
  drm/mxsfb: Improve the axi clock usage
  drm/mxsfb: Clear OUTSTANDING_REQS bits
  drm/mxsfb: Add support for horizontal stride
  drm/mxsfb: Add support for live pixel format change

 .../devicetree/bindings/display/mxsfb.txt          |   6 +
 drivers/gpu/drm/mxsfb/mxsfb_crtc.c                 | 287 ++++++++++++++++++---
 drivers/gpu/drm/mxsfb/mxsfb_drv.c                  | 190 +++++++++++---
 drivers/gpu/drm/mxsfb/mxsfb_drv.h                  |  10 +-
 drivers/gpu/drm/mxsfb/mxsfb_out.c                  |  26 +-
 drivers/gpu/drm/mxsfb/mxsfb_regs.h                 | 193 +++++++++-----
 6 files changed, 573 insertions(+), 139 deletions(-)

-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v2 01/15] drm/mxsfb: Update mxsfb to support a bridge
  2019-08-14 10:48 [PATCH v2 00/15] Improvements and fixes for mxsfb DRM driver Robert Chiras
@ 2019-08-14 10:48 ` Robert Chiras
  2019-08-14 10:48 ` [PATCH v2 02/15] drm/mxsfb: Read bus flags from bridge if present Robert Chiras
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 27+ messages in thread
From: Robert Chiras @ 2019-08-14 10:48 UTC (permalink / raw)
  To: Guido Günther, Marek Vasut, Stefan Agner, David Airlie,
	Daniel Vetter, Rob Herring, Mark Rutland, Shawn Guo,
	Sascha Hauer, Fabio Estevam
  Cc: devicetree, linux-kernel, dri-devel, NXP Linux Team,
	Pengutronix Kernel Team, linux-arm-kernel

Currently, the MXSFB DRM driver only supports a panel. But, its output
display signal can also be redirected to another encoder, like a DSI
controller. In this case, that DSI controller may act like a drm_bridge.
In order support this use-case too, this patch adds support for drm_bridge
in mxsfb.

Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
Tested-by: Guido Günther <agx@sigxcpu.org>
---
 drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 17 +++++++++++---
 drivers/gpu/drm/mxsfb/mxsfb_drv.c  | 46 +++++++++++++++++++++++++++++++++-----
 drivers/gpu/drm/mxsfb/mxsfb_drv.h  |  4 +++-
 drivers/gpu/drm/mxsfb/mxsfb_out.c  | 26 +++++++++++----------
 4 files changed, 72 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
index 1242156..de09b93 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
@@ -95,8 +95,11 @@ static void mxsfb_set_bus_fmt(struct mxsfb_drm_private *mxsfb)
 
 	reg = readl(mxsfb->base + LCDC_CTRL);
 
-	if (mxsfb->connector.display_info.num_bus_formats)
-		bus_format = mxsfb->connector.display_info.bus_formats[0];
+	if (mxsfb->connector->display_info.num_bus_formats)
+		bus_format = mxsfb->connector->display_info.bus_formats[0];
+
+	DRM_DEV_DEBUG_DRIVER(drm->dev, "Using bus_format: 0x%08X\n",
+			     bus_format);
 
 	reg &= ~CTRL_BUS_WIDTH_MASK;
 	switch (bus_format) {
@@ -204,8 +207,9 @@ static dma_addr_t mxsfb_get_fb_paddr(struct mxsfb_drm_private *mxsfb)
 
 static void mxsfb_crtc_mode_set_nofb(struct mxsfb_drm_private *mxsfb)
 {
+	struct drm_device *drm = mxsfb->pipe.crtc.dev;
 	struct drm_display_mode *m = &mxsfb->pipe.crtc.state->adjusted_mode;
-	const u32 bus_flags = mxsfb->connector.display_info.bus_flags;
+	const u32 bus_flags = mxsfb->connector->display_info.bus_flags;
 	u32 vdctrl0, vsync_pulse_len, hsync_pulse_len;
 	int err;
 
@@ -229,6 +233,13 @@ static void mxsfb_crtc_mode_set_nofb(struct mxsfb_drm_private *mxsfb)
 
 	clk_set_rate(mxsfb->clk, m->crtc_clock * 1000);
 
+	DRM_DEV_DEBUG_DRIVER(drm->dev, "Pixel clock: %dkHz (actual: %dkHz)\n",
+			     m->crtc_clock,
+			     (int)(clk_get_rate(mxsfb->clk) / 1000));
+	DRM_DEV_DEBUG_DRIVER(drm->dev, "Connector bus_flags: 0x%08X\n",
+			     bus_flags);
+	DRM_DEV_DEBUG_DRIVER(drm->dev, "Mode flags: 0x%08X\n", m->flags);
+
 	writel(TRANSFER_COUNT_SET_VCOUNT(m->crtc_vdisplay) |
 	       TRANSFER_COUNT_SET_HCOUNT(m->crtc_hdisplay),
 	       mxsfb->base + mxsfb->devdata->transfer_count);
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index 878ef68..9dc69b7 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -101,9 +101,25 @@ static void mxsfb_pipe_enable(struct drm_simple_display_pipe *pipe,
 			      struct drm_crtc_state *crtc_state,
 			      struct drm_plane_state *plane_state)
 {
+	struct drm_connector *connector;
 	struct mxsfb_drm_private *mxsfb = drm_pipe_to_mxsfb_drm_private(pipe);
 	struct drm_device *drm = pipe->plane.dev;
 
+	if (!mxsfb->connector) {
+		list_for_each_entry(connector,
+				    &drm->mode_config.connector_list,
+				    head)
+			if (connector->encoder == &mxsfb->pipe.encoder) {
+				mxsfb->connector = connector;
+				break;
+			}
+	}
+
+	if (!mxsfb->connector) {
+		dev_warn(drm->dev, "No connector attached, using default\n");
+		mxsfb->connector = &mxsfb->panel_connector;
+	}
+
 	pm_runtime_get_sync(drm->dev);
 	drm_panel_prepare(mxsfb->panel);
 	mxsfb_crtc_enable(mxsfb);
@@ -129,6 +145,9 @@ static void mxsfb_pipe_disable(struct drm_simple_display_pipe *pipe)
 		drm_crtc_send_vblank_event(crtc, event);
 	}
 	spin_unlock_irq(&drm->event_lock);
+
+	if (mxsfb->connector != &mxsfb->panel_connector)
+		mxsfb->connector = NULL;
 }
 
 static void mxsfb_pipe_update(struct drm_simple_display_pipe *pipe,
@@ -226,16 +245,33 @@ static int mxsfb_load(struct drm_device *drm, unsigned long flags)
 
 	ret = drm_simple_display_pipe_init(drm, &mxsfb->pipe, &mxsfb_funcs,
 			mxsfb_formats, ARRAY_SIZE(mxsfb_formats), NULL,
-			&mxsfb->connector);
+			mxsfb->connector);
 	if (ret < 0) {
 		dev_err(drm->dev, "Cannot setup simple display pipe\n");
 		goto err_vblank;
 	}
 
-	ret = drm_panel_attach(mxsfb->panel, &mxsfb->connector);
-	if (ret) {
-		dev_err(drm->dev, "Cannot connect panel\n");
-		goto err_vblank;
+	/*
+	 * Attach panel only if there is one.
+	 * If there is no panel attach, it must be a bridge. In this case, we
+	 * need a reference to its connector for a proper initialization.
+	 * We will do this check in pipe->enable(), since the connector won't
+	 * be attached to an encoder until then.
+	 */
+
+	if (mxsfb->panel) {
+		ret = drm_panel_attach(mxsfb->panel, mxsfb->connector);
+		if (ret) {
+			dev_err(drm->dev, "Cannot connect panel: %d\n", ret);
+			goto err_vblank;
+		}
+	} else if (mxsfb->bridge) {
+		ret = drm_simple_display_pipe_attach_bridge(&mxsfb->pipe,
+							    mxsfb->bridge);
+		if (ret) {
+			dev_err(drm->dev, "Cannot connect bridge: %d\n", ret);
+			goto err_vblank;
+		}
 	}
 
 	drm->mode_config.min_width	= MXSFB_MIN_XRES;
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.h b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
index d975300..0b65b51 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.h
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
@@ -27,8 +27,10 @@ struct mxsfb_drm_private {
 	struct clk			*clk_disp_axi;
 
 	struct drm_simple_display_pipe	pipe;
-	struct drm_connector		connector;
+	struct drm_connector		panel_connector;
+	struct drm_connector		*connector;
 	struct drm_panel		*panel;
+	struct drm_bridge		*bridge;
 };
 
 int mxsfb_setup_crtc(struct drm_device *dev);
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_out.c b/drivers/gpu/drm/mxsfb/mxsfb_out.c
index 231d016..9506eec 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_out.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_out.c
@@ -21,7 +21,8 @@
 static struct mxsfb_drm_private *
 drm_connector_to_mxsfb_drm_private(struct drm_connector *connector)
 {
-	return container_of(connector, struct mxsfb_drm_private, connector);
+	return container_of(connector, struct mxsfb_drm_private,
+			    panel_connector);
 }
 
 static int mxsfb_panel_get_modes(struct drm_connector *connector)
@@ -76,22 +77,23 @@ static const struct drm_connector_funcs mxsfb_panel_connector_funcs = {
 int mxsfb_create_output(struct drm_device *drm)
 {
 	struct mxsfb_drm_private *mxsfb = drm->dev_private;
-	struct drm_panel *panel;
 	int ret;
 
-	ret = drm_of_find_panel_or_bridge(drm->dev->of_node, 0, 0, &panel, NULL);
+	ret = drm_of_find_panel_or_bridge(drm->dev->of_node, 0, 0,
+					  &mxsfb->panel, &mxsfb->bridge);
 	if (ret)
 		return ret;
 
-	mxsfb->connector.dpms = DRM_MODE_DPMS_OFF;
-	mxsfb->connector.polled = 0;
-	drm_connector_helper_add(&mxsfb->connector,
-			&mxsfb_panel_connector_helper_funcs);
-	ret = drm_connector_init(drm, &mxsfb->connector,
-				 &mxsfb_panel_connector_funcs,
-				 DRM_MODE_CONNECTOR_Unknown);
-	if (!ret)
-		mxsfb->panel = panel;
+	if (mxsfb->panel) {
+		mxsfb->connector = &mxsfb->panel_connector;
+		mxsfb->connector->dpms = DRM_MODE_DPMS_OFF;
+		mxsfb->connector->polled = 0;
+		drm_connector_helper_add(mxsfb->connector,
+					 &mxsfb_panel_connector_helper_funcs);
+		ret = drm_connector_init(drm, mxsfb->connector,
+					 &mxsfb_panel_connector_funcs,
+					 DRM_MODE_CONNECTOR_Unknown);
+	}
 
 	return ret;
 }
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 02/15] drm/mxsfb: Read bus flags from bridge if present
  2019-08-14 10:48 [PATCH v2 00/15] Improvements and fixes for mxsfb DRM driver Robert Chiras
  2019-08-14 10:48 ` [PATCH v2 01/15] drm/mxsfb: Update mxsfb to support a bridge Robert Chiras
@ 2019-08-14 10:48 ` Robert Chiras
  2019-08-14 10:48 ` [PATCH v2 03/15] drm/mxsfb: Add defines for the rest of registers Robert Chiras
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 27+ messages in thread
From: Robert Chiras @ 2019-08-14 10:48 UTC (permalink / raw)
  To: Guido Günther, Marek Vasut, Stefan Agner, David Airlie,
	Daniel Vetter, Rob Herring, Mark Rutland, Shawn Guo,
	Sascha Hauer, Fabio Estevam
  Cc: devicetree, linux-kernel, dri-devel, NXP Linux Team,
	Pengutronix Kernel Team, linux-arm-kernel

From: Guido Günther <agx@sigxcpu.org>

The bridge might have special requirmentes on the input bus. This
is e.g. used by the imx-nwl bridge.

Signed-off-by: Guido Günther <agx@sigxcpu.org>
Reviewed-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
index de09b93..b69ace8 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
@@ -209,7 +209,7 @@ static void mxsfb_crtc_mode_set_nofb(struct mxsfb_drm_private *mxsfb)
 {
 	struct drm_device *drm = mxsfb->pipe.crtc.dev;
 	struct drm_display_mode *m = &mxsfb->pipe.crtc.state->adjusted_mode;
-	const u32 bus_flags = mxsfb->connector->display_info.bus_flags;
+	u32 bus_flags = mxsfb->connector->display_info.bus_flags;
 	u32 vdctrl0, vsync_pulse_len, hsync_pulse_len;
 	int err;
 
@@ -233,6 +233,9 @@ static void mxsfb_crtc_mode_set_nofb(struct mxsfb_drm_private *mxsfb)
 
 	clk_set_rate(mxsfb->clk, m->crtc_clock * 1000);
 
+	if (mxsfb->bridge && mxsfb->bridge->timings)
+		bus_flags = mxsfb->bridge->timings->input_bus_flags;
+
 	DRM_DEV_DEBUG_DRIVER(drm->dev, "Pixel clock: %dkHz (actual: %dkHz)\n",
 			     m->crtc_clock,
 			     (int)(clk_get_rate(mxsfb->clk) / 1000));
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 03/15] drm/mxsfb: Add defines for the rest of registers
  2019-08-14 10:48 [PATCH v2 00/15] Improvements and fixes for mxsfb DRM driver Robert Chiras
  2019-08-14 10:48 ` [PATCH v2 01/15] drm/mxsfb: Update mxsfb to support a bridge Robert Chiras
  2019-08-14 10:48 ` [PATCH v2 02/15] drm/mxsfb: Read bus flags from bridge if present Robert Chiras
@ 2019-08-14 10:48 ` Robert Chiras
  2019-08-14 10:48 ` [PATCH v2 04/15] drm/mxsfb: Reset vital register for a proper initialization Robert Chiras
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 27+ messages in thread
From: Robert Chiras @ 2019-08-14 10:48 UTC (permalink / raw)
  To: Guido Günther, Marek Vasut, Stefan Agner, David Airlie,
	Daniel Vetter, Rob Herring, Mark Rutland, Shawn Guo,
	Sascha Hauer, Fabio Estevam
  Cc: devicetree, linux-kernel, dri-devel, NXP Linux Team,
	Pengutronix Kernel Team, linux-arm-kernel

Some of the existing registers in this controller are not defined, but
also not used. Add them to the register definitions, so that they can be
easily used in future improvements or fixes.

Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
---
 drivers/gpu/drm/mxsfb/mxsfb_regs.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/mxsfb/mxsfb_regs.h b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
index 932d7ea..71426aa 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_regs.h
+++ b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
@@ -14,19 +14,31 @@
 
 #define LCDC_CTRL			0x00
 #define LCDC_CTRL1			0x10
+#define LCDC_V4_CTRL2			0x20
 #define LCDC_V3_TRANSFER_COUNT		0x20
 #define LCDC_V4_TRANSFER_COUNT		0x30
 #define LCDC_V4_CUR_BUF			0x40
 #define LCDC_V4_NEXT_BUF		0x50
 #define LCDC_V3_CUR_BUF			0x30
 #define LCDC_V3_NEXT_BUF		0x40
+#define LCDC_TIMING			0x60
 #define LCDC_VDCTRL0			0x70
 #define LCDC_VDCTRL1			0x80
 #define LCDC_VDCTRL2			0x90
 #define LCDC_VDCTRL3			0xa0
 #define LCDC_VDCTRL4			0xb0
+#define LCDC_DVICTRL0			0xc0
+#define LCDC_DVICTRL1			0xd0
+#define LCDC_DVICTRL2			0xe0
+#define LCDC_DVICTRL3			0xf0
+#define LCDC_DVICTRL4			0x100
+#define LCDC_V4_DATA			0x180
+#define LCDC_V3_DATA			0x1b0
 #define LCDC_V4_DEBUG0			0x1d0
 #define LCDC_V3_DEBUG0			0x1f0
+#define LCDC_AS_CTRL			0x210
+#define LCDC_AS_BUF			0x220
+#define LCDC_AS_NEXT_BUF		0x230
 
 #define CTRL_SFTRST			(1 << 31)
 #define CTRL_CLKGATE			(1 << 30)
@@ -45,12 +57,15 @@
 #define CTRL_DF24			(1 << 1)
 #define CTRL_RUN			(1 << 0)
 
+#define CTRL1_RECOVERY_ON_UNDERFLOW	(1 << 24)
 #define CTRL1_FIFO_CLEAR		(1 << 21)
 #define CTRL1_SET_BYTE_PACKAGING(x)	(((x) & 0xf) << 16)
 #define CTRL1_GET_BYTE_PACKAGING(x)	(((x) >> 16) & 0xf)
 #define CTRL1_CUR_FRAME_DONE_IRQ_EN	(1 << 13)
 #define CTRL1_CUR_FRAME_DONE_IRQ	(1 << 9)
 
+#define CTRL2_OUTSTANDING_REQS__REQ_16		(4 << 21)
+
 #define TRANSFER_COUNT_SET_VCOUNT(x)	(((x) & 0xffff) << 16)
 #define TRANSFER_COUNT_GET_VCOUNT(x)	(((x) >> 16) & 0xffff)
 #define TRANSFER_COUNT_SET_HCOUNT(x)	((x) & 0xffff)
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 04/15] drm/mxsfb: Reset vital register for a proper initialization
  2019-08-14 10:48 [PATCH v2 00/15] Improvements and fixes for mxsfb DRM driver Robert Chiras
                   ` (2 preceding siblings ...)
  2019-08-14 10:48 ` [PATCH v2 03/15] drm/mxsfb: Add defines for the rest of registers Robert Chiras
@ 2019-08-14 10:48 ` Robert Chiras
  2019-08-14 11:11   ` Stefan Agner
  2019-08-14 10:48 ` [PATCH v2 05/15] drm/mxsfb: Update register definitions using bit manipulation defines Robert Chiras
                   ` (10 subsequent siblings)
  14 siblings, 1 reply; 27+ messages in thread
From: Robert Chiras @ 2019-08-14 10:48 UTC (permalink / raw)
  To: Guido Günther, Marek Vasut, Stefan Agner, David Airlie,
	Daniel Vetter, Rob Herring, Mark Rutland, Shawn Guo,
	Sascha Hauer, Fabio Estevam
  Cc: devicetree, linux-kernel, dri-devel, NXP Linux Team,
	Pengutronix Kernel Team, linux-arm-kernel

Some of the regiters need, like LCDC_CTRL and CTRL2_OUTSTANDING_REQS
needs to be properly cleared and initialized for a better start and stop
routine.

Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
---
 drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
index b69ace8..5e44f57 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
@@ -127,6 +127,10 @@ static void mxsfb_enable_controller(struct mxsfb_drm_private *mxsfb)
 		clk_prepare_enable(mxsfb->clk_disp_axi);
 	clk_prepare_enable(mxsfb->clk);
 
+	if (mxsfb->devdata->ipversion >= 4)
+		writel(CTRL2_OUTSTANDING_REQS(REQ_16),
+		       mxsfb->base + LCDC_V4_CTRL2 + REG_SET);
+
 	/* If it was disabled, re-enable the mode again */
 	writel(CTRL_DOTCLK_MODE, mxsfb->base + LCDC_CTRL + REG_SET);
 
@@ -136,12 +140,19 @@ static void mxsfb_enable_controller(struct mxsfb_drm_private *mxsfb)
 	writel(reg, mxsfb->base + LCDC_VDCTRL4);
 
 	writel(CTRL_RUN, mxsfb->base + LCDC_CTRL + REG_SET);
+	writel(CTRL1_RECOVERY_ON_UNDERFLOW, mxsfb->base + LCDC_CTRL1 + REG_SET);
 }
 
 static void mxsfb_disable_controller(struct mxsfb_drm_private *mxsfb)
 {
 	u32 reg;
 
+	if (mxsfb->devdata->ipversion >= 4)
+		writel(CTRL2_OUTSTANDING_REQS(0x7),
+		       mxsfb->base + LCDC_V4_CTRL2 + REG_CLR);
+
+	writel(CTRL_RUN, mxsfb->base + LCDC_CTRL + REG_CLR);
+
 	/*
 	 * Even if we disable the controller here, it will still continue
 	 * until its FIFOs are running out of data
@@ -295,6 +306,7 @@ void mxsfb_crtc_enable(struct mxsfb_drm_private *mxsfb)
 	dma_addr_t paddr;
 
 	mxsfb_enable_axi_clk(mxsfb);
+	writel(0, mxsfb->base + LCDC_CTRL);
 	mxsfb_crtc_mode_set_nofb(mxsfb);
 
 	/* Write cur_buf as well to avoid an initial corrupt frame */
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 05/15] drm/mxsfb: Update register definitions using bit manipulation defines
  2019-08-14 10:48 [PATCH v2 00/15] Improvements and fixes for mxsfb DRM driver Robert Chiras
                   ` (3 preceding siblings ...)
  2019-08-14 10:48 ` [PATCH v2 04/15] drm/mxsfb: Reset vital register for a proper initialization Robert Chiras
@ 2019-08-14 10:48 ` Robert Chiras
  2019-08-14 10:48 ` [PATCH v2 06/15] drm/mxsfb: Update mxsfb with additional pixel formats Robert Chiras
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 27+ messages in thread
From: Robert Chiras @ 2019-08-14 10:48 UTC (permalink / raw)
  To: Guido Günther, Marek Vasut, Stefan Agner, David Airlie,
	Daniel Vetter, Rob Herring, Mark Rutland, Shawn Guo,
	Sascha Hauer, Fabio Estevam
  Cc: devicetree, linux-kernel, dri-devel, NXP Linux Team,
	Pengutronix Kernel Team, linux-arm-kernel

Use BIT(x) and GEN_MASK(h, l) for better representation the inside of
various registers.

Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
---
 drivers/gpu/drm/mxsfb/mxsfb_regs.h | 151 ++++++++++++++++++++++---------------
 1 file changed, 89 insertions(+), 62 deletions(-)

diff --git a/drivers/gpu/drm/mxsfb/mxsfb_regs.h b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
index 71426aa..9fcb1db 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_regs.h
+++ b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
@@ -40,66 +40,93 @@
 #define LCDC_AS_BUF			0x220
 #define LCDC_AS_NEXT_BUF		0x230
 
-#define CTRL_SFTRST			(1 << 31)
-#define CTRL_CLKGATE			(1 << 30)
-#define CTRL_BYPASS_COUNT		(1 << 19)
-#define CTRL_VSYNC_MODE			(1 << 18)
-#define CTRL_DOTCLK_MODE		(1 << 17)
-#define CTRL_DATA_SELECT		(1 << 16)
-#define CTRL_SET_BUS_WIDTH(x)		(((x) & 0x3) << 10)
-#define CTRL_GET_BUS_WIDTH(x)		(((x) >> 10) & 0x3)
-#define CTRL_BUS_WIDTH_MASK		(0x3 << 10)
-#define CTRL_SET_WORD_LENGTH(x)		(((x) & 0x3) << 8)
-#define CTRL_GET_WORD_LENGTH(x)		(((x) >> 8) & 0x3)
-#define CTRL_MASTER			(1 << 5)
-#define CTRL_DF16			(1 << 3)
-#define CTRL_DF18			(1 << 2)
-#define CTRL_DF24			(1 << 1)
-#define CTRL_RUN			(1 << 0)
-
-#define CTRL1_RECOVERY_ON_UNDERFLOW	(1 << 24)
-#define CTRL1_FIFO_CLEAR		(1 << 21)
-#define CTRL1_SET_BYTE_PACKAGING(x)	(((x) & 0xf) << 16)
-#define CTRL1_GET_BYTE_PACKAGING(x)	(((x) >> 16) & 0xf)
-#define CTRL1_CUR_FRAME_DONE_IRQ_EN	(1 << 13)
-#define CTRL1_CUR_FRAME_DONE_IRQ	(1 << 9)
-
-#define CTRL2_OUTSTANDING_REQS__REQ_16		(4 << 21)
-
-#define TRANSFER_COUNT_SET_VCOUNT(x)	(((x) & 0xffff) << 16)
-#define TRANSFER_COUNT_GET_VCOUNT(x)	(((x) >> 16) & 0xffff)
-#define TRANSFER_COUNT_SET_HCOUNT(x)	((x) & 0xffff)
-#define TRANSFER_COUNT_GET_HCOUNT(x)	((x) & 0xffff)
-
-#define VDCTRL0_ENABLE_PRESENT		(1 << 28)
-#define VDCTRL0_VSYNC_ACT_HIGH		(1 << 27)
-#define VDCTRL0_HSYNC_ACT_HIGH		(1 << 26)
-#define VDCTRL0_DOTCLK_ACT_FALLING	(1 << 25)
-#define VDCTRL0_ENABLE_ACT_HIGH		(1 << 24)
-#define VDCTRL0_VSYNC_PERIOD_UNIT	(1 << 21)
-#define VDCTRL0_VSYNC_PULSE_WIDTH_UNIT	(1 << 20)
-#define VDCTRL0_HALF_LINE		(1 << 19)
-#define VDCTRL0_HALF_LINE_MODE		(1 << 18)
-#define VDCTRL0_SET_VSYNC_PULSE_WIDTH(x) ((x) & 0x3ffff)
-#define VDCTRL0_GET_VSYNC_PULSE_WIDTH(x) ((x) & 0x3ffff)
-
-#define VDCTRL2_SET_HSYNC_PERIOD(x)	((x) & 0x3ffff)
-#define VDCTRL2_GET_HSYNC_PERIOD(x)	((x) & 0x3ffff)
-
-#define VDCTRL3_MUX_SYNC_SIGNALS	(1 << 29)
-#define VDCTRL3_VSYNC_ONLY		(1 << 28)
-#define SET_HOR_WAIT_CNT(x)		(((x) & 0xfff) << 16)
-#define GET_HOR_WAIT_CNT(x)		(((x) >> 16) & 0xfff)
-#define SET_VERT_WAIT_CNT(x)		((x) & 0xffff)
-#define GET_VERT_WAIT_CNT(x)		((x) & 0xffff)
-
-#define VDCTRL4_SET_DOTCLK_DLY(x)	(((x) & 0x7) << 29) /* v4 only */
-#define VDCTRL4_GET_DOTCLK_DLY(x)	(((x) >> 29) & 0x7) /* v4 only */
-#define VDCTRL4_SYNC_SIGNALS_ON		(1 << 18)
-#define SET_DOTCLK_H_VALID_DATA_CNT(x)	((x) & 0x3ffff)
-
-#define DEBUG0_HSYNC			(1 < 26)
-#define DEBUG0_VSYNC			(1 < 25)
+/* reg bit manipulation */
+#define REG_PUT(x, h, l) (((x) << (l)) & GENMASK(h, l))
+#define REG_GET(x, h, l) (((x) & GENMASK(h, l)) >> (l))
+
+#define CTRL_SFTRST			BIT(31)
+#define CTRL_CLKGATE			BIT(30)
+#define CTRL_SHIFT_DIR(x)		REG_PUT((x), 26, 26)
+#define CTRL_SHIFT_NUM(x)		REG_PUT((x), 25, 21)
+#define CTRL_BYPASS_COUNT		BIT(19)
+#define CTRL_VSYNC_MODE			BIT(18)
+#define CTRL_DOTCLK_MODE		BIT(17)
+#define CTRL_DATA_SELECT		BIT(16)
+#define CTRL_INPUT_SWIZZLE(x)		REG_PUT((x), 15, 14)
+#define CTRL_CSC_SWIZZLE(x)		REG_PUT((x), 13, 12)
+#define CTRL_SET_BUS_WIDTH(x)		REG_PUT((x), 11, 10)
+#define CTRL_GET_BUS_WIDTH(x)		REG_GET((x), 11, 10)
+#define CTRL_BUS_WIDTH_MASK		REG_PUT((0x3), 11, 10)
+#define CTRL_SET_WORD_LENGTH(x)		REG_PUT((x), 9, 8)
+#define CTRL_GET_WORD_LENGTH(x)		REG_GET((x), 9, 8)
+#define CTRL_MASTER			BIT(5)
+#define CTRL_DF16			BIT(3)
+#define CTRL_DF18			BIT(2)
+#define CTRL_DF24			BIT(1)
+#define CTRL_RUN			BIT(0)
+
+#define CTRL1_RECOVERY_ON_UNDERFLOW	BIT(24)
+#define CTRL1_FIFO_CLEAR		BIT(21)
+
+/*
+ * BYTE_PACKAGING
+ *
+ * This bitfield is used to show which data bytes in a 32-bit word area valid.
+ * Default value 0xf indicates that all bytes are valid. For 8-bit transfers,
+ * any combination in this bitfield will mean valid data is present in the
+ * corresponding bytes. In the 16-bit mode, a 16-bit half-word is valid only if
+ * adjacent bits [1:0] or [3:2] or both are 1. A value of 0x0 will mean that
+ * none of the bytes are valid and should not be used. For example, set the bit
+ * field value to 0x7 if the display data is arranged in the 24-bit unpacked
+ * format (A-R-G-B where A value does not have be transmitted).
+ */
+#define CTRL1_SET_BYTE_PACKAGING(x)	REG_PUT((x), 19, 16)
+#define CTRL1_GET_BYTE_PACKAGING(x)	REG_GET((x), 19, 16)
+
+#define CTRL1_CUR_FRAME_DONE_IRQ_EN	BIT(13)
+#define CTRL1_CUR_FRAME_DONE_IRQ	BIT(9)
+
+#define CTRL2_OUTSTANDING_REQS(x)	REG_PUT((x), 23, 21)
+#define REQ_1	0
+#define REQ_2	1
+#define REQ_4	2
+#define REQ_8	3
+#define REQ_16	4
+
+#define TRANSFER_COUNT_SET_VCOUNT(x)	REG_PUT((x), 31, 16)
+#define TRANSFER_COUNT_GET_VCOUNT(x)	REG_GET((x), 31, 16)
+#define TRANSFER_COUNT_SET_HCOUNT(x)	REG_PUT((x), 15, 0)
+#define TRANSFER_COUNT_GET_HCOUNT(x)	REG_GET((x), 15, 0)
+
+#define VDCTRL0_ENABLE_PRESENT		BIT(28)
+#define VDCTRL0_VSYNC_ACT_HIGH		BIT(27)
+#define VDCTRL0_HSYNC_ACT_HIGH		BIT(26)
+#define VDCTRL0_DOTCLK_ACT_FALLING	BIT(25)
+#define VDCTRL0_ENABLE_ACT_HIGH		BIT(24)
+#define VDCTRL0_VSYNC_PERIOD_UNIT	BIT(21)
+#define VDCTRL0_VSYNC_PULSE_WIDTH_UNIT	BIT(20)
+#define VDCTRL0_HALF_LINE		BIT(19)
+#define VDCTRL0_HALF_LINE_MODE		BIT(18)
+#define VDCTRL0_SET_VSYNC_PULSE_WIDTH(x) REG_PUT((x), 17, 0)
+#define VDCTRL0_GET_VSYNC_PULSE_WIDTH(x) REG_GET((x), 17, 0)
+
+#define VDCTRL2_SET_HSYNC_PERIOD(x)	REG_PUT((x), 15, 0)
+#define VDCTRL2_GET_HSYNC_PERIOD(x)	REG_GET((x), 15, 0)
+
+#define VDCTRL3_MUX_SYNC_SIGNALS	BIT(29)
+#define VDCTRL3_VSYNC_ONLY		BIT(28)
+#define SET_HOR_WAIT_CNT(x)		REG_PUT((x), 27, 16)
+#define GET_HOR_WAIT_CNT(x)		REG_GET((x), 27, 16)
+#define SET_VERT_WAIT_CNT(x)		REG_PUT((x), 15, 0)
+#define GET_VERT_WAIT_CNT(x)		REG_GET((x), 15, 0)
+
+#define VDCTRL4_SET_DOTCLK_DLY(x)	REG_PUT((x), 31, 29) /* v4 only */
+#define VDCTRL4_GET_DOTCLK_DLY(x)	REG_GET((x), 31, 29) /* v4 only */
+#define VDCTRL4_SYNC_SIGNALS_ON		BIT(18)
+#define SET_DOTCLK_H_VALID_DATA_CNT(x)	REG_PUT((x), 17, 0)
+
+#define DEBUG0_HSYNC			BIT(26)
+#define DEBUG0_VSYNC			BIT(25)
 
 #define MXSFB_MIN_XRES			120
 #define MXSFB_MIN_YRES			120
@@ -116,7 +143,7 @@
 #define STMLCDIF_18BIT 2 /* pixel data bus to the display is of 18 bit width */
 #define STMLCDIF_24BIT 3 /* pixel data bus to the display is of 24 bit width */
 
-#define MXSFB_SYNC_DATA_ENABLE_HIGH_ACT	(1 << 6)
-#define MXSFB_SYNC_DOTCLK_FALLING_ACT	(1 << 7) /* negative edge sampling */
+#define MXSFB_SYNC_DATA_ENABLE_HIGH_ACT	BIT(6)
+#define MXSFB_SYNC_DOTCLK_FALLING_ACT	BIT(7) /* negative edge sampling */
 
 #endif /* __MXSFB_REGS_H__ */
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 06/15] drm/mxsfb: Update mxsfb with additional pixel formats
  2019-08-14 10:48 [PATCH v2 00/15] Improvements and fixes for mxsfb DRM driver Robert Chiras
                   ` (4 preceding siblings ...)
  2019-08-14 10:48 ` [PATCH v2 05/15] drm/mxsfb: Update register definitions using bit manipulation defines Robert Chiras
@ 2019-08-14 10:48 ` Robert Chiras
  2019-08-14 11:44   ` Daniel Stone
  2019-08-14 10:48 ` [PATCH v2 07/15] drm/mxsfb: Fix the vblank events Robert Chiras
                   ` (8 subsequent siblings)
  14 siblings, 1 reply; 27+ messages in thread
From: Robert Chiras @ 2019-08-14 10:48 UTC (permalink / raw)
  To: Guido Günther, Marek Vasut, Stefan Agner, David Airlie,
	Daniel Vetter, Rob Herring, Mark Rutland, Shawn Guo,
	Sascha Hauer, Fabio Estevam
  Cc: devicetree, linux-kernel, dri-devel, NXP Linux Team,
	Pengutronix Kernel Team, linux-arm-kernel

Since version 4 of eLCDIF, there are some registers that can do
transformations on the input data, like re-arranging the pixel
components. By doing that, we can support more pixel formats.
This patch adds support for X/ABGR and RGBX/A. Although, the local alpha
is not supported by eLCDIF, the alpha pixel formats were added to the
supported pixel formats but it will be ignored. This was necessary since
there are systems (like Android) that requires such pixel formats.

Also, add support for the following pixel formats:
            16 bpp: RG16 ,BG16, XR15, XB15, AR15, AB15

Set the bus format based on input from the user and panel
capabilities.
Save the bus format in crtc->mode.private_flags, so the bridge can
use it.

Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
---
 drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 147 ++++++++++++++++++++++++++++++-------
 drivers/gpu/drm/mxsfb/mxsfb_drv.c  |  30 ++++++--
 drivers/gpu/drm/mxsfb/mxsfb_drv.h  |   3 +-
 drivers/gpu/drm/mxsfb/mxsfb_regs.h |  15 ++++
 4 files changed, 163 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
index 5e44f57..1be29f5 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
@@ -43,14 +43,17 @@ static u32 set_hsync_pulse_width(struct mxsfb_drm_private *mxsfb, u32 val)
 }
 
 /* Setup the MXSFB registers for decoding the pixels out of the framebuffer */
-static int mxsfb_set_pixel_fmt(struct mxsfb_drm_private *mxsfb)
+static int mxsfb_set_pixel_fmt(struct mxsfb_drm_private *mxsfb, bool update)
 {
 	struct drm_crtc *crtc = &mxsfb->pipe.crtc;
 	struct drm_device *drm = crtc->dev;
 	const u32 format = crtc->primary->state->fb->format->format;
-	u32 ctrl, ctrl1;
+	u32 ctrl = 0, ctrl1 = 0;
+	bool bgr_format = true;
+	struct drm_format_name_buf format_name_buf;
 
-	ctrl = CTRL_BYPASS_COUNT | CTRL_MASTER;
+	if (!update)
+		ctrl = CTRL_BYPASS_COUNT | CTRL_MASTER;
 
 	/*
 	 * WARNING: The bus width, CTRL_SET_BUS_WIDTH(), is configured to
@@ -59,64 +62,158 @@ static int mxsfb_set_pixel_fmt(struct mxsfb_drm_private *mxsfb)
 	 * to arbitrary value. This limitation should not pose an issue.
 	 */
 
-	/* CTRL1 contains IRQ config and status bits, preserve those. */
-	ctrl1 = readl(mxsfb->base + LCDC_CTRL1);
-	ctrl1 &= CTRL1_CUR_FRAME_DONE_IRQ_EN | CTRL1_CUR_FRAME_DONE_IRQ;
+	if (!update) {
+		/* CTRL1 contains IRQ config and status bits, preserve those. */
+		ctrl1 = readl(mxsfb->base + LCDC_CTRL1);
+		ctrl1 &= CTRL1_CUR_FRAME_DONE_IRQ_EN | CTRL1_CUR_FRAME_DONE_IRQ;
+	}
+
+	DRM_DEV_DEBUG_DRIVER(drm->dev, "Setting up %s mode\n",
+			     drm_get_format_name(format, &format_name_buf));
+
+	/* Do some clean-up that we might have from a previous mode */
+	ctrl &= ~CTRL_SHIFT_DIR(1);
+	ctrl &= ~CTRL_SHIFT_NUM(0x3f);
+	if (mxsfb->devdata->ipversion >= 4)
+		writel(CTRL2_ODD_LINE_PATTERN(CTRL2_LINE_PATTERN_CLR) |
+		       CTRL2_EVEN_LINE_PATTERN(CTRL2_LINE_PATTERN_CLR),
+		       mxsfb->base + LCDC_V4_CTRL2 + REG_CLR);
 
 	switch (format) {
-	case DRM_FORMAT_RGB565:
-		dev_dbg(drm->dev, "Setting up RGB565 mode\n");
+	case DRM_FORMAT_BGR565: /* BG16 */
+		if (mxsfb->devdata->ipversion < 4)
+			goto err;
+		writel(CTRL2_ODD_LINE_PATTERN(CTRL2_LINE_PATTERN_BGR) |
+			CTRL2_EVEN_LINE_PATTERN(CTRL2_LINE_PATTERN_BGR),
+			mxsfb->base + LCDC_V4_CTRL2 + REG_SET);
+		/* Fall through */
+	case DRM_FORMAT_RGB565: /* RG16 */
+		ctrl |= CTRL_SET_WORD_LENGTH(0);
+		ctrl &= ~CTRL_DF16;
+		ctrl1 |= CTRL1_SET_BYTE_PACKAGING(0xf);
+		break;
+	case DRM_FORMAT_XBGR1555: /* XB15 */
+	case DRM_FORMAT_ABGR1555: /* AB15 */
+		if (mxsfb->devdata->ipversion < 4)
+			goto err;
+		writel(CTRL2_ODD_LINE_PATTERN(CTRL2_LINE_PATTERN_BGR) |
+			CTRL2_EVEN_LINE_PATTERN(CTRL2_LINE_PATTERN_BGR),
+			mxsfb->base + LCDC_V4_CTRL2 + REG_SET);
+		/* Fall through */
+	case DRM_FORMAT_XRGB1555: /* XR15 */
+	case DRM_FORMAT_ARGB1555: /* AR15 */
 		ctrl |= CTRL_SET_WORD_LENGTH(0);
+		ctrl |= CTRL_DF16;
 		ctrl1 |= CTRL1_SET_BYTE_PACKAGING(0xf);
 		break;
-	case DRM_FORMAT_XRGB8888:
-		dev_dbg(drm->dev, "Setting up XRGB8888 mode\n");
+	case DRM_FORMAT_RGBX8888: /* RX24 */
+	case DRM_FORMAT_RGBA8888: /* RA24 */
+		/* RGBX - > 0RGB */
+		ctrl |= CTRL_SHIFT_DIR(1);
+		ctrl |= CTRL_SHIFT_NUM(8);
+		bgr_format = false;
+		/* Fall through */
+	case DRM_FORMAT_XBGR8888: /* XB24 */
+	case DRM_FORMAT_ABGR8888: /* AB24 */
+		if (bgr_format) {
+			if (mxsfb->devdata->ipversion < 4)
+				goto err;
+			writel(CTRL2_ODD_LINE_PATTERN(CTRL2_LINE_PATTERN_BGR) |
+			       CTRL2_EVEN_LINE_PATTERN(CTRL2_LINE_PATTERN_BGR),
+			       mxsfb->base + LCDC_V4_CTRL2 + REG_SET);
+		}
+		/* Fall through */
+	case DRM_FORMAT_XRGB8888: /* XR24 */
+	case DRM_FORMAT_ARGB8888: /* AR24 */
 		ctrl |= CTRL_SET_WORD_LENGTH(3);
 		/* Do not use packed pixels = one pixel per word instead. */
 		ctrl1 |= CTRL1_SET_BYTE_PACKAGING(0x7);
 		break;
 	default:
-		dev_err(drm->dev, "Unhandled pixel format %08x\n", format);
-		return -EINVAL;
+		goto err;
 	}
 
-	writel(ctrl1, mxsfb->base + LCDC_CTRL1);
-	writel(ctrl, mxsfb->base + LCDC_CTRL);
+	if (update) {
+		writel(ctrl, mxsfb->base + LCDC_CTRL + REG_SET);
+		writel(ctrl1, mxsfb->base + LCDC_CTRL1 + REG_SET);
+	} else {
+		writel(ctrl, mxsfb->base + LCDC_CTRL);
+		writel(ctrl1, mxsfb->base + LCDC_CTRL1);
+	}
 
 	return 0;
+
+err:
+	DRM_DEV_ERROR(drm->dev, "Unhandled pixel format: %s\n",
+		      drm_get_format_name(format, &format_name_buf));
+
+	return -EINVAL;
+}
+
+static u32 get_bus_format_from_bpp(u32 bpp)
+{
+	switch (bpp) {
+	case 16:
+		return MEDIA_BUS_FMT_RGB565_1X16;
+	case 18:
+		return MEDIA_BUS_FMT_RGB666_1X18;
+	case 24:
+		return MEDIA_BUS_FMT_RGB888_1X24;
+	default:
+		return MEDIA_BUS_FMT_RGB888_1X24;
+	}
 }
 
 static void mxsfb_set_bus_fmt(struct mxsfb_drm_private *mxsfb)
 {
 	struct drm_crtc *crtc = &mxsfb->pipe.crtc;
+	unsigned int bits_per_pixel = crtc->primary->state->fb->format->depth;
 	struct drm_device *drm = crtc->dev;
 	u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
-	u32 reg;
-
-	reg = readl(mxsfb->base + LCDC_CTRL);
+	int num_bus_formats = mxsfb->connector->display_info.num_bus_formats;
+	const u32 *bus_formats = mxsfb->connector->display_info.bus_formats;
+	u32 reg = 0;
+	int i = 0;
+
+	/* match the user requested bus_format to one supported by the panel */
+	if (num_bus_formats) {
+		u32 user_bus_format = get_bus_format_from_bpp(bits_per_pixel);
+
+		bus_format = bus_formats[0];
+		for (i = 0; i < num_bus_formats; i++) {
+			if (user_bus_format == bus_formats[i]) {
+				bus_format = user_bus_format;
+				break;
+			}
+		}
+	}
 
-	if (mxsfb->connector->display_info.num_bus_formats)
-		bus_format = mxsfb->connector->display_info.bus_formats[0];
+	/*
+	 * CRTC will dictate the bus format via private_flags[16:1]
+	 * and private_flags[0] will signal a bus format change
+	 */
+	crtc->mode.private_flags &= ~0x1FFFF; /* clear bus format */
+	crtc->mode.private_flags |= (bus_format << 1); /* set bus format */
+	crtc->mode.private_flags |= 0x1; /* bus format change indication*/
 
 	DRM_DEV_DEBUG_DRIVER(drm->dev, "Using bus_format: 0x%08X\n",
 			     bus_format);
 
-	reg &= ~CTRL_BUS_WIDTH_MASK;
 	switch (bus_format) {
 	case MEDIA_BUS_FMT_RGB565_1X16:
-		reg |= CTRL_SET_BUS_WIDTH(STMLCDIF_16BIT);
+		reg = CTRL_SET_BUS_WIDTH(STMLCDIF_16BIT);
 		break;
 	case MEDIA_BUS_FMT_RGB666_1X18:
-		reg |= CTRL_SET_BUS_WIDTH(STMLCDIF_18BIT);
+		reg = CTRL_SET_BUS_WIDTH(STMLCDIF_18BIT);
 		break;
 	case MEDIA_BUS_FMT_RGB888_1X24:
-		reg |= CTRL_SET_BUS_WIDTH(STMLCDIF_24BIT);
+		reg = CTRL_SET_BUS_WIDTH(STMLCDIF_24BIT);
 		break;
 	default:
 		dev_err(drm->dev, "Unknown media bus format %d\n", bus_format);
 		break;
 	}
-	writel(reg, mxsfb->base + LCDC_CTRL);
+	writel(reg, mxsfb->base + LCDC_CTRL + REG_SET);
 }
 
 static void mxsfb_enable_controller(struct mxsfb_drm_private *mxsfb)
@@ -238,7 +335,7 @@ static void mxsfb_crtc_mode_set_nofb(struct mxsfb_drm_private *mxsfb)
 	/* Clear the FIFOs */
 	writel(CTRL1_FIFO_CLEAR, mxsfb->base + LCDC_CTRL1 + REG_SET);
 
-	err = mxsfb_set_pixel_fmt(mxsfb);
+	err = mxsfb_set_pixel_fmt(mxsfb, false);
 	if (err)
 		return;
 
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index 9dc69b7..2743975 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -43,6 +43,27 @@ enum mxsfb_devtype {
 	MXSFB_V4,
 };
 
+/*
+ * When adding new formats, make sure to update the num_formats from
+ * mxsfb_devdata below.
+ */
+static const u32 mxsfb_formats[] = {
+	/* MXSFB_V3 */
+	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_ARGB8888,
+	DRM_FORMAT_RGB565,
+	/* MXSFB_V4 */
+	DRM_FORMAT_XBGR8888,
+	DRM_FORMAT_ABGR8888,
+	DRM_FORMAT_RGBX8888,
+	DRM_FORMAT_RGBA8888,
+	DRM_FORMAT_ARGB1555,
+	DRM_FORMAT_XRGB1555,
+	DRM_FORMAT_ABGR1555,
+	DRM_FORMAT_XBGR1555,
+	DRM_FORMAT_BGR565
+};
+
 static const struct mxsfb_devdata mxsfb_devdata[] = {
 	[MXSFB_V3] = {
 		.transfer_count	= LCDC_V3_TRANSFER_COUNT,
@@ -52,6 +73,7 @@ static const struct mxsfb_devdata mxsfb_devdata[] = {
 		.hs_wdth_mask	= 0xff,
 		.hs_wdth_shift	= 24,
 		.ipversion	= 3,
+		.num_formats	= 3,
 	},
 	[MXSFB_V4] = {
 		.transfer_count	= LCDC_V4_TRANSFER_COUNT,
@@ -61,14 +83,10 @@ static const struct mxsfb_devdata mxsfb_devdata[] = {
 		.hs_wdth_mask	= 0x3fff,
 		.hs_wdth_shift	= 18,
 		.ipversion	= 4,
+		.num_formats	= ARRAY_SIZE(mxsfb_formats),
 	},
 };
 
-static const uint32_t mxsfb_formats[] = {
-	DRM_FORMAT_XRGB8888,
-	DRM_FORMAT_RGB565
-};
-
 static struct mxsfb_drm_private *
 drm_pipe_to_mxsfb_drm_private(struct drm_simple_display_pipe *pipe)
 {
@@ -244,7 +262,7 @@ static int mxsfb_load(struct drm_device *drm, unsigned long flags)
 	}
 
 	ret = drm_simple_display_pipe_init(drm, &mxsfb->pipe, &mxsfb_funcs,
-			mxsfb_formats, ARRAY_SIZE(mxsfb_formats), NULL,
+			mxsfb_formats, mxsfb->devdata->num_formats, NULL,
 			mxsfb->connector);
 	if (ret < 0) {
 		dev_err(drm->dev, "Cannot setup simple display pipe\n");
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.h b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
index 0b65b51..8fb65d3 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.h
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
@@ -16,6 +16,7 @@ struct mxsfb_devdata {
 	unsigned int	 hs_wdth_mask;
 	unsigned int	 hs_wdth_shift;
 	unsigned int	 ipversion;
+	unsigned int	 num_formats;
 };
 
 struct mxsfb_drm_private {
@@ -42,6 +43,6 @@ void mxsfb_disable_axi_clk(struct mxsfb_drm_private *mxsfb);
 void mxsfb_crtc_enable(struct mxsfb_drm_private *mxsfb);
 void mxsfb_crtc_disable(struct mxsfb_drm_private *mxsfb);
 void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb,
-			       struct drm_plane_state *state);
+			       struct drm_plane_state *old_state);
 
 #endif /* __MXSFB_DRV_H__ */
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_regs.h b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
index 9fcb1db..dc4daa0 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_regs.h
+++ b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
@@ -44,6 +44,11 @@
 #define REG_PUT(x, h, l) (((x) << (l)) & GENMASK(h, l))
 #define REG_GET(x, h, l) (((x) & GENMASK(h, l)) >> (l))
 
+#define SWIZZLE_LE		0 /* Little-Endian or No swap */
+#define SWIZZLE_BE		1 /* Big-Endian or swap all */
+#define SWIZZLE_HWD		2 /* Swap half-words */
+#define SWIZZLE_HWD_BYTE	3 /* Swap bytes within each half-word */
+
 #define CTRL_SFTRST			BIT(31)
 #define CTRL_CLKGATE			BIT(30)
 #define CTRL_SHIFT_DIR(x)		REG_PUT((x), 26, 26)
@@ -93,6 +98,16 @@
 #define REQ_8	3
 #define REQ_16	4
 
+#define CTRL2_ODD_LINE_PATTERN(x)	REG_PUT((x), 18, 16)
+#define CTRL2_EVEN_LINE_PATTERN(x)	REG_PUT((x), 14, 12)
+#define CTRL2_LINE_PATTERN_RGB	0
+#define CTRL2_LINE_PATTERN_RBG	1
+#define CTRL2_LINE_PATTERN_GBR	2
+#define CTRL2_LINE_PATTERN_GRB	3
+#define CTRL2_LINE_PATTERN_BRG	4
+#define CTRL2_LINE_PATTERN_BGR	5
+#define CTRL2_LINE_PATTERN_CLR	7
+
 #define TRANSFER_COUNT_SET_VCOUNT(x)	REG_PUT((x), 31, 16)
 #define TRANSFER_COUNT_GET_VCOUNT(x)	REG_GET((x), 31, 16)
 #define TRANSFER_COUNT_SET_HCOUNT(x)	REG_PUT((x), 15, 0)
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 07/15] drm/mxsfb: Fix the vblank events
  2019-08-14 10:48 [PATCH v2 00/15] Improvements and fixes for mxsfb DRM driver Robert Chiras
                   ` (5 preceding siblings ...)
  2019-08-14 10:48 ` [PATCH v2 06/15] drm/mxsfb: Update mxsfb with additional pixel formats Robert Chiras
@ 2019-08-14 10:48 ` Robert Chiras
  2019-08-14 17:31   ` Daniel Vetter
  2019-08-14 10:48 ` [PATCH v2 08/15] drm/mxsfb: Signal mode changed when bpp changed Robert Chiras
                   ` (7 subsequent siblings)
  14 siblings, 1 reply; 27+ messages in thread
From: Robert Chiras @ 2019-08-14 10:48 UTC (permalink / raw)
  To: Guido Günther, Marek Vasut, Stefan Agner, David Airlie,
	Daniel Vetter, Rob Herring, Mark Rutland, Shawn Guo,
	Sascha Hauer, Fabio Estevam
  Cc: devicetree, linux-kernel, dri-devel, NXP Linux Team,
	Pengutronix Kernel Team, linux-arm-kernel

Currently, the vblank support is not correctly implemented in MXSFB_DRM
driver. The call to drm_vblank_init is made with mode_config.num_crtc
which at that time is 0. Because of this, vblank is not activated, so
there won't be any vblank event submitted.
For example, when running modetest with the '-v' parameter will result
in an astronomical refresh rate (10000+ Hz), because of that.

Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
---
 drivers/gpu/drm/mxsfb/mxsfb_drv.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index 2743975..829abec 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -38,6 +38,9 @@
 #include "mxsfb_drv.h"
 #include "mxsfb_regs.h"
 
+/* The eLCDIF max possible CRTCs */
+#define MAX_CRTCS 1
+
 enum mxsfb_devtype {
 	MXSFB_V3,
 	MXSFB_V4,
@@ -138,6 +141,8 @@ static void mxsfb_pipe_enable(struct drm_simple_display_pipe *pipe,
 		mxsfb->connector = &mxsfb->panel_connector;
 	}
 
+	drm_crtc_vblank_on(&pipe->crtc);
+
 	pm_runtime_get_sync(drm->dev);
 	drm_panel_prepare(mxsfb->panel);
 	mxsfb_crtc_enable(mxsfb);
@@ -164,6 +169,8 @@ static void mxsfb_pipe_disable(struct drm_simple_display_pipe *pipe)
 	}
 	spin_unlock_irq(&drm->event_lock);
 
+	drm_crtc_vblank_off(&pipe->crtc);
+
 	if (mxsfb->connector != &mxsfb->panel_connector)
 		mxsfb->connector = NULL;
 }
@@ -246,7 +253,7 @@ static int mxsfb_load(struct drm_device *drm, unsigned long flags)
 
 	pm_runtime_enable(drm->dev);
 
-	ret = drm_vblank_init(drm, drm->mode_config.num_crtc);
+	ret = drm_vblank_init(drm, MAX_CRTCS);
 	if (ret < 0) {
 		dev_err(drm->dev, "Failed to initialise vblank\n");
 		goto err_vblank;
@@ -269,6 +276,8 @@ static int mxsfb_load(struct drm_device *drm, unsigned long flags)
 		goto err_vblank;
 	}
 
+	drm_crtc_vblank_off(&mxsfb->pipe.crtc);
+
 	/*
 	 * Attach panel only if there is one.
 	 * If there is no panel attach, it must be a bridge. In this case, we
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 08/15] drm/mxsfb: Signal mode changed when bpp changed
  2019-08-14 10:48 [PATCH v2 00/15] Improvements and fixes for mxsfb DRM driver Robert Chiras
                   ` (6 preceding siblings ...)
  2019-08-14 10:48 ` [PATCH v2 07/15] drm/mxsfb: Fix the vblank events Robert Chiras
@ 2019-08-14 10:48 ` Robert Chiras
  2019-08-14 10:48 ` [PATCH v2 09/15] dt-bindings: display: Add max-res property for mxsfb Robert Chiras
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 27+ messages in thread
From: Robert Chiras @ 2019-08-14 10:48 UTC (permalink / raw)
  To: Guido Günther, Marek Vasut, Stefan Agner, David Airlie,
	Daniel Vetter, Rob Herring, Mark Rutland, Shawn Guo,
	Sascha Hauer, Fabio Estevam
  Cc: devicetree, linux-kernel, dri-devel, NXP Linux Team,
	Pengutronix Kernel Team, linux-arm-kernel

From: Mirela Rabulea <mirela.rabulea@nxp.com>

Add mxsfb_atomic_helper_check to signal mode changed when bpp changed.
This will trigger the execution of disable/enable on
a modeset with different bpp than the current one.

Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
---
 drivers/gpu/drm/mxsfb/mxsfb_drv.c | 45 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index 829abec..59c8ba7 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -26,6 +26,7 @@
 #include <drm/drm_drv.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_fb_helper.h>
+#include <drm/drm_fourcc.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_irq.h>
@@ -108,9 +109,51 @@ void mxsfb_disable_axi_clk(struct mxsfb_drm_private *mxsfb)
 		clk_disable_unprepare(mxsfb->clk_axi);
 }
 
+/**
+ * mxsfb_atomic_helper_check - validate state object
+ * @dev: DRM device
+ * @state: the driver state object
+ *
+ * On top of the drm imlementation drm_atomic_helper_check,
+ * check if the bpp is changed, if so, signal mode_changed,
+ * this will trigger disable/enable
+ *
+ * RETURNS:
+ * Zero for success or -errno
+ */
+static int mxsfb_atomic_helper_check(struct drm_device *dev,
+				     struct drm_atomic_state *state)
+{
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *new_state;
+	int i, ret;
+
+	ret = drm_atomic_helper_check(dev, state);
+	if (ret)
+		return ret;
+
+	for_each_new_crtc_in_state(state, crtc, new_state, i) {
+		struct drm_plane_state *primary_state;
+		int old_bpp = 0;
+		int new_bpp = 0;
+
+		if (!crtc->primary || !crtc->primary->old_fb)
+			continue;
+		primary_state =
+			drm_atomic_get_plane_state(state, crtc->primary);
+		if (!primary_state || !primary_state->fb)
+			continue;
+		old_bpp = crtc->primary->old_fb->format->depth;
+		new_bpp = primary_state->fb->format->depth;
+		if (old_bpp != new_bpp)
+			new_state->mode_changed = true;
+	}
+	return ret;
+}
+
 static const struct drm_mode_config_funcs mxsfb_mode_config_funcs = {
 	.fb_create		= drm_gem_fb_create,
-	.atomic_check		= drm_atomic_helper_check,
+	.atomic_check		= mxsfb_atomic_helper_check,
 	.atomic_commit		= drm_atomic_helper_commit,
 };
 
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 09/15] dt-bindings: display: Add max-res property for mxsfb
  2019-08-14 10:48 [PATCH v2 00/15] Improvements and fixes for mxsfb DRM driver Robert Chiras
                   ` (7 preceding siblings ...)
  2019-08-14 10:48 ` [PATCH v2 08/15] drm/mxsfb: Signal mode changed when bpp changed Robert Chiras
@ 2019-08-14 10:48 ` Robert Chiras
  2019-08-14 11:03   ` Stefan Agner
  2019-08-14 10:48 ` [PATCH v2 10/15] drm/mxsfb: Add max-res property for MXSFB Robert Chiras
                   ` (5 subsequent siblings)
  14 siblings, 1 reply; 27+ messages in thread
From: Robert Chiras @ 2019-08-14 10:48 UTC (permalink / raw)
  To: Guido Günther, Marek Vasut, Stefan Agner, David Airlie,
	Daniel Vetter, Rob Herring, Mark Rutland, Shawn Guo,
	Sascha Hauer, Fabio Estevam
  Cc: devicetree, linux-kernel, dri-devel, NXP Linux Team,
	Pengutronix Kernel Team, linux-arm-kernel

Add new optional property 'max-res', to limit the maximum supported
resolution by the MXSFB_DRM driver.

Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
---
 Documentation/devicetree/bindings/display/mxsfb.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/mxsfb.txt b/Documentation/devicetree/bindings/display/mxsfb.txt
index 472e1ea..55e22ed 100644
--- a/Documentation/devicetree/bindings/display/mxsfb.txt
+++ b/Documentation/devicetree/bindings/display/mxsfb.txt
@@ -17,6 +17,12 @@ Required properties:
 Required sub-nodes:
   - port: The connection to an encoder chip.
 
+Optional properties:
+- max-res:	an array with a maximum of two integers, representing the
+		maximum supported resolution, in the form of
+		<maxX>, <maxY>; if one of the item is <0>, the default
+		driver-defined maximum resolution for that axis is used
+
 Example:
 
 	lcdif1: display-controller@2220000 {
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 10/15] drm/mxsfb: Add max-res property for MXSFB
  2019-08-14 10:48 [PATCH v2 00/15] Improvements and fixes for mxsfb DRM driver Robert Chiras
                   ` (8 preceding siblings ...)
  2019-08-14 10:48 ` [PATCH v2 09/15] dt-bindings: display: Add max-res property for mxsfb Robert Chiras
@ 2019-08-14 10:48 ` Robert Chiras
  2019-08-14 10:48 ` [PATCH v2 11/15] drm/mxsfb: Update mxsfb to support LCD reset Robert Chiras
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 27+ messages in thread
From: Robert Chiras @ 2019-08-14 10:48 UTC (permalink / raw)
  To: Guido Günther, Marek Vasut, Stefan Agner, David Airlie,
	Daniel Vetter, Rob Herring, Mark Rutland, Shawn Guo,
	Sascha Hauer, Fabio Estevam
  Cc: devicetree, linux-kernel, dri-devel, NXP Linux Team,
	Pengutronix Kernel Team, linux-arm-kernel

Because of stability issues, we may want to limit the maximum resolution
supported by the MXSFB (eLCDIF) driver.
This patch add support for a new property which we can use to impose such
limitation.

Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
---
 drivers/gpu/drm/mxsfb/mxsfb_drv.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index 59c8ba7..6dae2bd 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -264,6 +264,7 @@ static int mxsfb_load(struct drm_device *drm, unsigned long flags)
 	struct platform_device *pdev = to_platform_device(drm->dev);
 	struct mxsfb_drm_private *mxsfb;
 	struct resource *res;
+	u32 max_res[2] = {0, 0};
 	int ret;
 
 	mxsfb = devm_kzalloc(&pdev->dev, sizeof(*mxsfb), GFP_KERNEL);
@@ -344,10 +345,17 @@ static int mxsfb_load(struct drm_device *drm, unsigned long flags)
 		}
 	}
 
+	of_property_read_u32_array(drm->dev->of_node, "max-res",
+				   &max_res[0], 2);
+	if (!max_res[0])
+		max_res[0] = MXSFB_MAX_XRES;
+	if (!max_res[1])
+		max_res[1] = MXSFB_MAX_YRES;
+
 	drm->mode_config.min_width	= MXSFB_MIN_XRES;
 	drm->mode_config.min_height	= MXSFB_MIN_YRES;
-	drm->mode_config.max_width	= MXSFB_MAX_XRES;
-	drm->mode_config.max_height	= MXSFB_MAX_YRES;
+	drm->mode_config.max_width	= max_res[0];
+	drm->mode_config.max_height	= max_res[1];
 	drm->mode_config.funcs		= &mxsfb_mode_config_funcs;
 	drm->mode_config.helper_private	= &mxsfb_mode_config_helpers;
 
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 11/15] drm/mxsfb: Update mxsfb to support LCD reset
  2019-08-14 10:48 [PATCH v2 00/15] Improvements and fixes for mxsfb DRM driver Robert Chiras
                   ` (9 preceding siblings ...)
  2019-08-14 10:48 ` [PATCH v2 10/15] drm/mxsfb: Add max-res property for MXSFB Robert Chiras
@ 2019-08-14 10:48 ` Robert Chiras
  2019-08-14 10:48 ` [PATCH v2 12/15] drm/mxsfb: Improve the axi clock usage Robert Chiras
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 27+ messages in thread
From: Robert Chiras @ 2019-08-14 10:48 UTC (permalink / raw)
  To: Guido Günther, Marek Vasut, Stefan Agner, David Airlie,
	Daniel Vetter, Rob Herring, Mark Rutland, Shawn Guo,
	Sascha Hauer, Fabio Estevam
  Cc: devicetree, linux-kernel, dri-devel, NXP Linux Team,
	Pengutronix Kernel Team, linux-arm-kernel

The eLCDIF controller has control pin for the external LCD reset pin.
Add support for it and assert this pin in enable and de-assert it in
disable.

Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
---
 drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 14 ++++++++++----
 drivers/gpu/drm/mxsfb/mxsfb_regs.h |  2 ++
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
index 1be29f5..a4ba368 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
@@ -224,9 +224,12 @@ static void mxsfb_enable_controller(struct mxsfb_drm_private *mxsfb)
 		clk_prepare_enable(mxsfb->clk_disp_axi);
 	clk_prepare_enable(mxsfb->clk);
 
-	if (mxsfb->devdata->ipversion >= 4)
+	if (mxsfb->devdata->ipversion >= 4) {
 		writel(CTRL2_OUTSTANDING_REQS(REQ_16),
 		       mxsfb->base + LCDC_V4_CTRL2 + REG_SET);
+		/* Assert LCD Reset bit */
+		writel(CTRL2_LCD_RESET, mxsfb->base + LCDC_V4_CTRL2 + REG_SET);
+	}
 
 	/* If it was disabled, re-enable the mode again */
 	writel(CTRL_DOTCLK_MODE, mxsfb->base + LCDC_CTRL + REG_SET);
@@ -244,11 +247,14 @@ static void mxsfb_disable_controller(struct mxsfb_drm_private *mxsfb)
 {
 	u32 reg;
 
-	if (mxsfb->devdata->ipversion >= 4)
+	writel(CTRL_RUN, mxsfb->base + LCDC_CTRL + REG_CLR);
+
+	if (mxsfb->devdata->ipversion >= 4) {
 		writel(CTRL2_OUTSTANDING_REQS(0x7),
 		       mxsfb->base + LCDC_V4_CTRL2 + REG_CLR);
-
-	writel(CTRL_RUN, mxsfb->base + LCDC_CTRL + REG_CLR);
+		/* De-assert LCD Reset bit */
+		writel(CTRL2_LCD_RESET, mxsfb->base + LCDC_V4_CTRL2 + REG_CLR);
+	}
 
 	/*
 	 * Even if we disable the controller here, it will still continue
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_regs.h b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
index dc4daa0..0f63ba1 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_regs.h
+++ b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
@@ -108,6 +108,8 @@
 #define CTRL2_LINE_PATTERN_BGR	5
 #define CTRL2_LINE_PATTERN_CLR	7
 
+#define CTRL2_LCD_RESET			BIT(0)
+
 #define TRANSFER_COUNT_SET_VCOUNT(x)	REG_PUT((x), 31, 16)
 #define TRANSFER_COUNT_GET_VCOUNT(x)	REG_GET((x), 31, 16)
 #define TRANSFER_COUNT_SET_HCOUNT(x)	REG_PUT((x), 15, 0)
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 12/15] drm/mxsfb: Improve the axi clock usage
  2019-08-14 10:48 [PATCH v2 00/15] Improvements and fixes for mxsfb DRM driver Robert Chiras
                   ` (10 preceding siblings ...)
  2019-08-14 10:48 ` [PATCH v2 11/15] drm/mxsfb: Update mxsfb to support LCD reset Robert Chiras
@ 2019-08-14 10:48 ` Robert Chiras
  2019-08-14 11:06   ` Stefan Agner
  2019-08-14 10:48 ` [PATCH v2 13/15] drm/mxsfb: Clear OUTSTANDING_REQS bits Robert Chiras
                   ` (2 subsequent siblings)
  14 siblings, 1 reply; 27+ messages in thread
From: Robert Chiras @ 2019-08-14 10:48 UTC (permalink / raw)
  To: Guido Günther, Marek Vasut, Stefan Agner, David Airlie,
	Daniel Vetter, Rob Herring, Mark Rutland, Shawn Guo,
	Sascha Hauer, Fabio Estevam
  Cc: devicetree, linux-kernel, dri-devel, NXP Linux Team,
	Pengutronix Kernel Team, linux-arm-kernel

Currently, the enable of the axi clock return status is ignored, causing
issues when the enable fails then we try to disable it. Therefore, it is
better to check the return status and disable it only when enable
succeeded.
Also, remove the helper functions around clk_axi, since we can directly
use the clk API function for enable/disable the clock. Those functions
are already checking for NULL clk and returning 0 if that's the case.

Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
Acked-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 drivers/gpu/drm/mxsfb/mxsfb_crtc.c |  8 ++++----
 drivers/gpu/drm/mxsfb/mxsfb_drv.c  | 32 +++++++++++++-------------------
 drivers/gpu/drm/mxsfb/mxsfb_drv.h  |  3 ---
 3 files changed, 17 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
index a4ba368..e727f5e 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
@@ -408,7 +408,7 @@ void mxsfb_crtc_enable(struct mxsfb_drm_private *mxsfb)
 {
 	dma_addr_t paddr;
 
-	mxsfb_enable_axi_clk(mxsfb);
+	clk_prepare_enable(mxsfb->clk_axi);
 	writel(0, mxsfb->base + LCDC_CTRL);
 	mxsfb_crtc_mode_set_nofb(mxsfb);
 
@@ -425,7 +425,7 @@ void mxsfb_crtc_enable(struct mxsfb_drm_private *mxsfb)
 void mxsfb_crtc_disable(struct mxsfb_drm_private *mxsfb)
 {
 	mxsfb_disable_controller(mxsfb);
-	mxsfb_disable_axi_clk(mxsfb);
+	clk_disable_unprepare(mxsfb->clk_axi);
 }
 
 void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb,
@@ -451,8 +451,8 @@ void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb,
 
 	paddr = mxsfb_get_fb_paddr(mxsfb);
 	if (paddr) {
-		mxsfb_enable_axi_clk(mxsfb);
+		clk_prepare_enable(mxsfb->clk_axi);
 		writel(paddr, mxsfb->base + mxsfb->devdata->next_buf);
-		mxsfb_disable_axi_clk(mxsfb);
+		clk_disable_unprepare(mxsfb->clk_axi);
 	}
 }
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index 6dae2bd..694b287 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -97,18 +97,6 @@ drm_pipe_to_mxsfb_drm_private(struct drm_simple_display_pipe *pipe)
 	return container_of(pipe, struct mxsfb_drm_private, pipe);
 }
 
-void mxsfb_enable_axi_clk(struct mxsfb_drm_private *mxsfb)
-{
-	if (mxsfb->clk_axi)
-		clk_prepare_enable(mxsfb->clk_axi);
-}
-
-void mxsfb_disable_axi_clk(struct mxsfb_drm_private *mxsfb)
-{
-	if (mxsfb->clk_axi)
-		clk_disable_unprepare(mxsfb->clk_axi);
-}
-
 /**
  * mxsfb_atomic_helper_check - validate state object
  * @dev: DRM device
@@ -229,25 +217,31 @@ static void mxsfb_pipe_update(struct drm_simple_display_pipe *pipe,
 static int mxsfb_pipe_enable_vblank(struct drm_simple_display_pipe *pipe)
 {
 	struct mxsfb_drm_private *mxsfb = drm_pipe_to_mxsfb_drm_private(pipe);
+	int ret = 0;
+
+	ret = clk_prepare_enable(mxsfb->clk_axi);
+	if (ret)
+		return ret;
 
 	/* Clear and enable VBLANK IRQ */
-	mxsfb_enable_axi_clk(mxsfb);
 	writel(CTRL1_CUR_FRAME_DONE_IRQ, mxsfb->base + LCDC_CTRL1 + REG_CLR);
 	writel(CTRL1_CUR_FRAME_DONE_IRQ_EN, mxsfb->base + LCDC_CTRL1 + REG_SET);
-	mxsfb_disable_axi_clk(mxsfb);
+	clk_disable_unprepare(mxsfb->clk_axi);
 
-	return 0;
+	return ret;
 }
 
 static void mxsfb_pipe_disable_vblank(struct drm_simple_display_pipe *pipe)
 {
 	struct mxsfb_drm_private *mxsfb = drm_pipe_to_mxsfb_drm_private(pipe);
 
+	if (clk_prepare_enable(mxsfb->clk_axi))
+		return;
+
 	/* Disable and clear VBLANK IRQ */
-	mxsfb_enable_axi_clk(mxsfb);
 	writel(CTRL1_CUR_FRAME_DONE_IRQ_EN, mxsfb->base + LCDC_CTRL1 + REG_CLR);
 	writel(CTRL1_CUR_FRAME_DONE_IRQ, mxsfb->base + LCDC_CTRL1 + REG_CLR);
-	mxsfb_disable_axi_clk(mxsfb);
+	clk_disable_unprepare(mxsfb->clk_axi);
 }
 
 static struct drm_simple_display_pipe_funcs mxsfb_funcs = {
@@ -413,7 +407,7 @@ static irqreturn_t mxsfb_irq_handler(int irq, void *data)
 	struct mxsfb_drm_private *mxsfb = drm->dev_private;
 	u32 reg;
 
-	mxsfb_enable_axi_clk(mxsfb);
+	clk_prepare_enable(mxsfb->clk_axi);
 
 	reg = readl(mxsfb->base + LCDC_CTRL1);
 
@@ -422,7 +416,7 @@ static irqreturn_t mxsfb_irq_handler(int irq, void *data)
 
 	writel(CTRL1_CUR_FRAME_DONE_IRQ, mxsfb->base + LCDC_CTRL1 + REG_CLR);
 
-	mxsfb_disable_axi_clk(mxsfb);
+	clk_disable_unprepare(mxsfb->clk_axi);
 
 	return IRQ_HANDLED;
 }
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.h b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
index 8fb65d3..d6df8fe 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.h
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
@@ -37,9 +37,6 @@ struct mxsfb_drm_private {
 int mxsfb_setup_crtc(struct drm_device *dev);
 int mxsfb_create_output(struct drm_device *dev);
 
-void mxsfb_enable_axi_clk(struct mxsfb_drm_private *mxsfb);
-void mxsfb_disable_axi_clk(struct mxsfb_drm_private *mxsfb);
-
 void mxsfb_crtc_enable(struct mxsfb_drm_private *mxsfb);
 void mxsfb_crtc_disable(struct mxsfb_drm_private *mxsfb);
 void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb,
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 13/15] drm/mxsfb: Clear OUTSTANDING_REQS bits
  2019-08-14 10:48 [PATCH v2 00/15] Improvements and fixes for mxsfb DRM driver Robert Chiras
                   ` (11 preceding siblings ...)
  2019-08-14 10:48 ` [PATCH v2 12/15] drm/mxsfb: Improve the axi clock usage Robert Chiras
@ 2019-08-14 10:48 ` Robert Chiras
  2019-08-14 10:48 ` [PATCH v2 14/15] drm/mxsfb: Add support for horizontal stride Robert Chiras
  2019-08-14 10:48 ` [PATCH v2 15/15] drm/mxsfb: Add support for live pixel format change Robert Chiras
  14 siblings, 0 replies; 27+ messages in thread
From: Robert Chiras @ 2019-08-14 10:48 UTC (permalink / raw)
  To: Guido Günther, Marek Vasut, Stefan Agner, David Airlie,
	Daniel Vetter, Rob Herring, Mark Rutland, Shawn Guo,
	Sascha Hauer, Fabio Estevam
  Cc: devicetree, linux-kernel, dri-devel, NXP Linux Team,
	Pengutronix Kernel Team, linux-arm-kernel

Bit 21 can alter the CTRL2_OUTSTANDING_REQS value right after the eLCDIF
is enabled, since it comes up with default value of 1 (this behaviour
has been seen on some imx8 platforms).
In order to fix this, clear CTRL2_OUTSTANDING_REQS bits before setting
its value.

Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
---
 drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
index e727f5e..a12f53d 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
@@ -225,6 +225,13 @@ static void mxsfb_enable_controller(struct mxsfb_drm_private *mxsfb)
 	clk_prepare_enable(mxsfb->clk);
 
 	if (mxsfb->devdata->ipversion >= 4) {
+		/*
+		 * On some platforms, bit 21 is defaulted to 1, which may alter
+		 * the below setting. So, to make sure we have the right setting
+		 * clear all the bits for CTRL2_OUTSTANDING_REQS.
+		 */
+		writel(CTRL2_OUTSTANDING_REQS(0x7),
+		       mxsfb->base + LCDC_V4_CTRL2 + REG_CLR);
 		writel(CTRL2_OUTSTANDING_REQS(REQ_16),
 		       mxsfb->base + LCDC_V4_CTRL2 + REG_SET);
 		/* Assert LCD Reset bit */
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 14/15] drm/mxsfb: Add support for horizontal stride
  2019-08-14 10:48 [PATCH v2 00/15] Improvements and fixes for mxsfb DRM driver Robert Chiras
                   ` (12 preceding siblings ...)
  2019-08-14 10:48 ` [PATCH v2 13/15] drm/mxsfb: Clear OUTSTANDING_REQS bits Robert Chiras
@ 2019-08-14 10:48 ` Robert Chiras
  2019-08-14 10:48 ` [PATCH v2 15/15] drm/mxsfb: Add support for live pixel format change Robert Chiras
  14 siblings, 0 replies; 27+ messages in thread
From: Robert Chiras @ 2019-08-14 10:48 UTC (permalink / raw)
  To: Guido Günther, Marek Vasut, Stefan Agner, David Airlie,
	Daniel Vetter, Rob Herring, Mark Rutland, Shawn Guo,
	Sascha Hauer, Fabio Estevam
  Cc: devicetree, linux-kernel, dri-devel, NXP Linux Team,
	Pengutronix Kernel Team, linux-arm-kernel

Besides the eLCDIF block, there is another IP block, used in the past
for EPDC panels. Since the iMX.8mq doesn't have an EPDC connector, this
block is not documented, but we can use it to do additional operations
on the frame buffer.
In this case, we can use the pigeon registers from this IP block in
order to do horizontal crop on the frame buffer processed by the eLCDIF
block.

Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
---
 drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 79 ++++++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/mxsfb/mxsfb_drv.c  | 26 ++++++++++++-
 drivers/gpu/drm/mxsfb/mxsfb_regs.h | 16 ++++++++
 3 files changed, 115 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
index a12f53d..317575e 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
@@ -15,6 +15,7 @@
 
 #include <video/videomode.h>
 
+#include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_fb_cma_helper.h>
@@ -435,13 +436,66 @@ void mxsfb_crtc_disable(struct mxsfb_drm_private *mxsfb)
 	clk_disable_unprepare(mxsfb->clk_axi);
 }
 
+void mxsfb_set_fb_hcrop(struct mxsfb_drm_private *mxsfb, u32 src_w, u32 fb_w)
+{
+	u32 mask_cnt, htotal, hcount;
+	u32 vdctrl2, vdctrl3, vdctrl4, transfer_count;
+	u32 pigeon_12_0, pigeon_12_1, pigeon_12_2;
+
+	if (src_w == fb_w) {
+		writel(0x0, mxsfb->base + HW_EPDC_PIGEON_12_0);
+		writel(0x0, mxsfb->base + HW_EPDC_PIGEON_12_1);
+
+		return;
+	}
+
+	transfer_count = readl(mxsfb->base + LCDC_V4_TRANSFER_COUNT);
+	hcount = TRANSFER_COUNT_GET_HCOUNT(transfer_count);
+
+	transfer_count &= ~TRANSFER_COUNT_SET_HCOUNT(0xffff);
+	transfer_count |= TRANSFER_COUNT_SET_HCOUNT(fb_w);
+	writel(transfer_count, mxsfb->base + LCDC_V4_TRANSFER_COUNT);
+
+	vdctrl2 = readl(mxsfb->base + LCDC_VDCTRL2);
+	htotal  = VDCTRL2_GET_HSYNC_PERIOD(vdctrl2);
+	htotal  += fb_w - hcount;
+	vdctrl2 &= ~VDCTRL2_SET_HSYNC_PERIOD(0x3ffff);
+	vdctrl2 |= VDCTRL2_SET_HSYNC_PERIOD(htotal);
+	writel(vdctrl2, mxsfb->base + LCDC_VDCTRL2);
+
+	vdctrl4 = readl(mxsfb->base + LCDC_VDCTRL4);
+	vdctrl4 &= ~SET_DOTCLK_H_VALID_DATA_CNT(0x3ffff);
+	vdctrl4 |= SET_DOTCLK_H_VALID_DATA_CNT(fb_w);
+	writel(vdctrl4, mxsfb->base + LCDC_VDCTRL4);
+
+	/* configure related pigeon registers */
+	vdctrl3  = readl(mxsfb->base + LCDC_VDCTRL3);
+	mask_cnt = GET_HOR_WAIT_CNT(vdctrl3) - 5;
+
+	pigeon_12_0 = PIGEON_12_0_SET_STATE_MASK(0x24)		|
+		      PIGEON_12_0_SET_MASK_CNT(mask_cnt)	|
+		      PIGEON_12_0_SET_MASK_CNT_SEL(0x6)		|
+		      PIGEON_12_0_POL_ACTIVE_LOW		|
+		      PIGEON_12_0_EN;
+	writel(pigeon_12_0, mxsfb->base + HW_EPDC_PIGEON_12_0);
+
+	pigeon_12_1 = PIGEON_12_1_SET_CLR_CNT(src_w) |
+		      PIGEON_12_1_SET_SET_CNT(0x0);
+	writel(pigeon_12_1, mxsfb->base + HW_EPDC_PIGEON_12_1);
+
+	pigeon_12_2 = 0x0;
+	writel(pigeon_12_2, mxsfb->base + HW_EPDC_PIGEON_12_2);
+}
+
 void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb,
 			       struct drm_plane_state *state)
 {
 	struct drm_simple_display_pipe *pipe = &mxsfb->pipe;
 	struct drm_crtc *crtc = &pipe->crtc;
+	struct drm_plane_state *new_state = pipe->plane.state;
+	struct drm_framebuffer *fb = pipe->plane.state->fb;
 	struct drm_pending_vblank_event *event;
-	dma_addr_t paddr;
+	u32 fb_addr, src_off, src_w, stride, cpp = 0;
 
 	spin_lock_irq(&crtc->dev->event_lock);
 	event = crtc->state->event;
@@ -456,10 +510,27 @@ void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb,
 	}
 	spin_unlock_irq(&crtc->dev->event_lock);
 
-	paddr = mxsfb_get_fb_paddr(mxsfb);
-	if (paddr) {
+	if (!fb)
+		return;
+
+	fb_addr = mxsfb_get_fb_paddr(mxsfb);
+	if (mxsfb->devdata->ipversion >= 4) {
+		cpp = fb->format->cpp[0];
+		src_off = (new_state->src_y >> 16) * fb->pitches[0] +
+			  (new_state->src_x >> 16) * cpp;
+		fb_addr += fb->offsets[0] + src_off;
+	}
+
+	if (fb_addr) {
 		clk_prepare_enable(mxsfb->clk_axi);
-		writel(paddr, mxsfb->base + mxsfb->devdata->next_buf);
+		writel(fb_addr, mxsfb->base + mxsfb->devdata->next_buf);
 		clk_disable_unprepare(mxsfb->clk_axi);
 	}
+
+	if (mxsfb->devdata->ipversion >= 4 &&
+	    unlikely(drm_atomic_crtc_needs_modeset(new_state->crtc->state))) {
+		stride = DIV_ROUND_UP(fb->pitches[0], cpp);
+		src_w = new_state->src_w >> 16;
+		mxsfb_set_fb_hcrop(mxsfb, src_w, stride);
+	}
 }
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index 694b287..cc9c14f 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -136,6 +136,7 @@ static int mxsfb_atomic_helper_check(struct drm_device *dev,
 		if (old_bpp != new_bpp)
 			new_state->mode_changed = true;
 	}
+
 	return ret;
 }
 
@@ -149,6 +150,25 @@ static const struct drm_mode_config_helper_funcs mxsfb_mode_config_helpers = {
 	.atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
 };
 
+static int mxsfb_pipe_check(struct drm_simple_display_pipe *pipe,
+			    struct drm_plane_state *plane_state,
+			    struct drm_crtc_state *crtc_state)
+{
+	struct drm_framebuffer *fb = plane_state->fb;
+	struct drm_framebuffer *old_fb = pipe->plane.state->fb;
+
+	/* force 'mode_changed' when fb pitches changed, since
+	 * the pitch related registers configuration of LCDIF
+	 * can not be done when LCDIF is running.
+	 */
+	if (old_fb && likely(!crtc_state->mode_changed)) {
+		if (old_fb->pitches[0] != fb->pitches[0])
+			crtc_state->mode_changed = true;
+	}
+
+	return 0;
+}
+
 static void mxsfb_pipe_enable(struct drm_simple_display_pipe *pipe,
 			      struct drm_crtc_state *crtc_state,
 			      struct drm_plane_state *plane_state)
@@ -245,6 +265,7 @@ static void mxsfb_pipe_disable_vblank(struct drm_simple_display_pipe *pipe)
 }
 
 static struct drm_simple_display_pipe_funcs mxsfb_funcs = {
+	.check          = mxsfb_pipe_check,
 	.enable		= mxsfb_pipe_enable,
 	.disable	= mxsfb_pipe_disable,
 	.update		= mxsfb_pipe_update,
@@ -348,8 +369,9 @@ static int mxsfb_load(struct drm_device *drm, unsigned long flags)
 
 	drm->mode_config.min_width	= MXSFB_MIN_XRES;
 	drm->mode_config.min_height	= MXSFB_MIN_YRES;
-	drm->mode_config.max_width	= max_res[0];
-	drm->mode_config.max_height	= max_res[1];
+	/* Add additional 16 pixels for possible strides */
+	drm->mode_config.max_width	= max_res[0] + 16;
+	drm->mode_config.max_height	= max_res[1] + 16;
 	drm->mode_config.funcs		= &mxsfb_mode_config_funcs;
 	drm->mode_config.helper_private	= &mxsfb_mode_config_helpers;
 
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_regs.h b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
index 0f63ba1..df3279b 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_regs.h
+++ b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
@@ -145,6 +145,22 @@
 #define DEBUG0_HSYNC			BIT(26)
 #define DEBUG0_VSYNC			BIT(25)
 
+/* pigeon registers for crop */
+#define HW_EPDC_PIGEON_12_0		0xb00
+#define HW_EPDC_PIGEON_12_1		0xb10
+#define HW_EPDC_PIGEON_12_2		0xb20
+
+#define PIGEON_12_0_SET_STATE_MASK(x)	REG_PUT((x), 31, 24)
+#define PIGEON_12_0_SET_MASK_CNT(x)	REG_PUT((x), 23, 12)
+#define PIGEON_12_0_SET_MASK_CNT_SEL(x)	REG_PUT((x), 11,  8)
+#define PIGEON_12_0_SET_OFFSET(x)	REG_PUT((x),  7,  4)
+#define PIGEON_12_0_SET_INC_SEL(x)	REG_PUT((x),  3,  2)
+#define PIGEON_12_0_POL_ACTIVE_LOW	BIT(1)
+#define PIGEON_12_0_EN			BIT(0)
+
+#define PIGEON_12_1_SET_CLR_CNT(x)	REG_PUT((x), 31, 16)
+#define PIGEON_12_1_SET_SET_CNT(x)	REG_PUT((x), 15,  0)
+
 #define MXSFB_MIN_XRES			120
 #define MXSFB_MIN_YRES			120
 #define MXSFB_MAX_XRES			0xffff
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 15/15] drm/mxsfb: Add support for live pixel format change
  2019-08-14 10:48 [PATCH v2 00/15] Improvements and fixes for mxsfb DRM driver Robert Chiras
                   ` (13 preceding siblings ...)
  2019-08-14 10:48 ` [PATCH v2 14/15] drm/mxsfb: Add support for horizontal stride Robert Chiras
@ 2019-08-14 10:48 ` Robert Chiras
  14 siblings, 0 replies; 27+ messages in thread
From: Robert Chiras @ 2019-08-14 10:48 UTC (permalink / raw)
  To: Guido Günther, Marek Vasut, Stefan Agner, David Airlie,
	Daniel Vetter, Rob Herring, Mark Rutland, Shawn Guo,
	Sascha Hauer, Fabio Estevam
  Cc: devicetree, linux-kernel, dri-devel, NXP Linux Team,
	Pengutronix Kernel Team, linux-arm-kernel

This IP requires full stop and re-start when changing display timings,
but we can change the pixel format while running.

Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
---
 drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
index 317575e..5607fc0 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
@@ -494,6 +494,7 @@ void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb,
 	struct drm_crtc *crtc = &pipe->crtc;
 	struct drm_plane_state *new_state = pipe->plane.state;
 	struct drm_framebuffer *fb = pipe->plane.state->fb;
+	struct drm_framebuffer *old_fb = state->fb;
 	struct drm_pending_vblank_event *event;
 	u32 fb_addr, src_off, src_w, stride, cpp = 0;
 
@@ -510,7 +511,7 @@ void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb,
 	}
 	spin_unlock_irq(&crtc->dev->event_lock);
 
-	if (!fb)
+	if (!fb || !old_fb)
 		return;
 
 	fb_addr = mxsfb_get_fb_paddr(mxsfb);
@@ -533,4 +534,17 @@ void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb,
 		src_w = new_state->src_w >> 16;
 		mxsfb_set_fb_hcrop(mxsfb, src_w, stride);
 	}
+
+	if (old_fb->format->format != fb->format->format) {
+		struct drm_format_name_buf old_fmt_buf;
+		struct drm_format_name_buf new_fmt_buf;
+
+		DRM_DEV_DEBUG_DRIVER(crtc->dev->dev,
+				"Switching pixel format: %s -> %s\n",
+				drm_get_format_name(old_fb->format->format,
+						    &old_fmt_buf),
+				drm_get_format_name(fb->format->format,
+						    &new_fmt_buf));
+		mxsfb_set_pixel_fmt(mxsfb, true);
+	}
 }
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 09/15] dt-bindings: display: Add max-res property for mxsfb
  2019-08-14 10:48 ` [PATCH v2 09/15] dt-bindings: display: Add max-res property for mxsfb Robert Chiras
@ 2019-08-14 11:03   ` Stefan Agner
  2019-08-14 11:14     ` [EXT] " Robert Chiras
  0 siblings, 1 reply; 27+ messages in thread
From: Stefan Agner @ 2019-08-14 11:03 UTC (permalink / raw)
  To: Robert Chiras, robh+dt, Mark Rutland
  Cc: Marek Vasut, devicetree, Pengutronix Kernel Team, David Airlie,
	Fabio Estevam, Guido Günther, linux-kernel, dri-devel,
	NXP Linux Team, Daniel Vetter, Shawn Guo, Sascha Hauer,
	linux-arm-kernel

On 2019-08-14 12:48, Robert Chiras wrote:
> Add new optional property 'max-res', to limit the maximum supported
> resolution by the MXSFB_DRM driver.

I would also mention the reason why we need this.

I guess this needs a vendor prefix as well (fsl,max-res). I also would
like to have the ack of the device tree folks here.

--
Stefan

> 
> Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
> ---
>  Documentation/devicetree/bindings/display/mxsfb.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/mxsfb.txt
> b/Documentation/devicetree/bindings/display/mxsfb.txt
> index 472e1ea..55e22ed 100644
> --- a/Documentation/devicetree/bindings/display/mxsfb.txt
> +++ b/Documentation/devicetree/bindings/display/mxsfb.txt
> @@ -17,6 +17,12 @@ Required properties:
>  Required sub-nodes:
>    - port: The connection to an encoder chip.
>  
> +Optional properties:
> +- max-res:	an array with a maximum of two integers, representing the
> +		maximum supported resolution, in the form of
> +		<maxX>, <maxY>; if one of the item is <0>, the default
> +		driver-defined maximum resolution for that axis is used
> +
>  Example:
>  
>  	lcdif1: display-controller@2220000 {

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 12/15] drm/mxsfb: Improve the axi clock usage
  2019-08-14 10:48 ` [PATCH v2 12/15] drm/mxsfb: Improve the axi clock usage Robert Chiras
@ 2019-08-14 11:06   ` Stefan Agner
  2019-08-14 11:35     ` [EXT] " Robert Chiras
  0 siblings, 1 reply; 27+ messages in thread
From: Stefan Agner @ 2019-08-14 11:06 UTC (permalink / raw)
  To: Robert Chiras
  Cc: Marek Vasut, Mark Rutland, Pengutronix Kernel Team, devicetree,
	David Airlie, Fabio Estevam, Guido Günther, linux-kernel,
	dri-devel, Rob Herring, NXP Linux Team, Daniel Vetter, Shawn Guo,
	Sascha Hauer, linux-arm-kernel

On 2019-08-14 12:48, Robert Chiras wrote:
> Currently, the enable of the axi clock return status is ignored, causing
> issues when the enable fails then we try to disable it. Therefore, it is
> better to check the return status and disable it only when enable
> succeeded.

Is this actually the case in real world sometimes? Why is it failing?

I guess if we do this in one place, we should do it in all places (e.g.
also in mxsfb_crtc_enable, mxsfb_plane_atomic_update..)

--
Stefan

> Also, remove the helper functions around clk_axi, since we can directly
> use the clk API function for enable/disable the clock. Those functions
> are already checking for NULL clk and returning 0 if that's the case.


> 
> Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
> Acked-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---
>  drivers/gpu/drm/mxsfb/mxsfb_crtc.c |  8 ++++----
>  drivers/gpu/drm/mxsfb/mxsfb_drv.c  | 32 +++++++++++++-------------------
>  drivers/gpu/drm/mxsfb/mxsfb_drv.h  |  3 ---
>  3 files changed, 17 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> index a4ba368..e727f5e 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> @@ -408,7 +408,7 @@ void mxsfb_crtc_enable(struct mxsfb_drm_private *mxsfb)
>  {
>  	dma_addr_t paddr;
>  
> -	mxsfb_enable_axi_clk(mxsfb);
> +	clk_prepare_enable(mxsfb->clk_axi);
>  	writel(0, mxsfb->base + LCDC_CTRL);
>  	mxsfb_crtc_mode_set_nofb(mxsfb);
>  
> @@ -425,7 +425,7 @@ void mxsfb_crtc_enable(struct mxsfb_drm_private *mxsfb)
>  void mxsfb_crtc_disable(struct mxsfb_drm_private *mxsfb)
>  {
>  	mxsfb_disable_controller(mxsfb);
> -	mxsfb_disable_axi_clk(mxsfb);
> +	clk_disable_unprepare(mxsfb->clk_axi);
>  }
>  
>  void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb,
> @@ -451,8 +451,8 @@ void mxsfb_plane_atomic_update(struct
> mxsfb_drm_private *mxsfb,
>  
>  	paddr = mxsfb_get_fb_paddr(mxsfb);
>  	if (paddr) {
> -		mxsfb_enable_axi_clk(mxsfb);
> +		clk_prepare_enable(mxsfb->clk_axi);
>  		writel(paddr, mxsfb->base + mxsfb->devdata->next_buf);
> -		mxsfb_disable_axi_clk(mxsfb);
> +		clk_disable_unprepare(mxsfb->clk_axi);
>  	}
>  }
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> index 6dae2bd..694b287 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> @@ -97,18 +97,6 @@ drm_pipe_to_mxsfb_drm_private(struct
> drm_simple_display_pipe *pipe)
>  	return container_of(pipe, struct mxsfb_drm_private, pipe);
>  }
>  
> -void mxsfb_enable_axi_clk(struct mxsfb_drm_private *mxsfb)
> -{
> -	if (mxsfb->clk_axi)
> -		clk_prepare_enable(mxsfb->clk_axi);
> -}
> -
> -void mxsfb_disable_axi_clk(struct mxsfb_drm_private *mxsfb)
> -{
> -	if (mxsfb->clk_axi)
> -		clk_disable_unprepare(mxsfb->clk_axi);
> -}
> -
>  /**
>   * mxsfb_atomic_helper_check - validate state object
>   * @dev: DRM device
> @@ -229,25 +217,31 @@ static void mxsfb_pipe_update(struct
> drm_simple_display_pipe *pipe,
>  static int mxsfb_pipe_enable_vblank(struct drm_simple_display_pipe *pipe)
>  {
>  	struct mxsfb_drm_private *mxsfb = drm_pipe_to_mxsfb_drm_private(pipe);
> +	int ret = 0;
> +
> +	ret = clk_prepare_enable(mxsfb->clk_axi);
> +	if (ret)
> +		return ret;
>  
>  	/* Clear and enable VBLANK IRQ */
> -	mxsfb_enable_axi_clk(mxsfb);
>  	writel(CTRL1_CUR_FRAME_DONE_IRQ, mxsfb->base + LCDC_CTRL1 + REG_CLR);
>  	writel(CTRL1_CUR_FRAME_DONE_IRQ_EN, mxsfb->base + LCDC_CTRL1 + REG_SET);
> -	mxsfb_disable_axi_clk(mxsfb);
> +	clk_disable_unprepare(mxsfb->clk_axi);
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static void mxsfb_pipe_disable_vblank(struct drm_simple_display_pipe *pipe)
>  {
>  	struct mxsfb_drm_private *mxsfb = drm_pipe_to_mxsfb_drm_private(pipe);
>  
> +	if (clk_prepare_enable(mxsfb->clk_axi))
> +		return;
> +
>  	/* Disable and clear VBLANK IRQ */
> -	mxsfb_enable_axi_clk(mxsfb);
>  	writel(CTRL1_CUR_FRAME_DONE_IRQ_EN, mxsfb->base + LCDC_CTRL1 + REG_CLR);
>  	writel(CTRL1_CUR_FRAME_DONE_IRQ, mxsfb->base + LCDC_CTRL1 + REG_CLR);
> -	mxsfb_disable_axi_clk(mxsfb);
> +	clk_disable_unprepare(mxsfb->clk_axi);
>  }
>  
>  static struct drm_simple_display_pipe_funcs mxsfb_funcs = {
> @@ -413,7 +407,7 @@ static irqreturn_t mxsfb_irq_handler(int irq, void *data)
>  	struct mxsfb_drm_private *mxsfb = drm->dev_private;
>  	u32 reg;
>  
> -	mxsfb_enable_axi_clk(mxsfb);
> +	clk_prepare_enable(mxsfb->clk_axi);
>  
>  	reg = readl(mxsfb->base + LCDC_CTRL1);
>  
> @@ -422,7 +416,7 @@ static irqreturn_t mxsfb_irq_handler(int irq, void *data)
>  
>  	writel(CTRL1_CUR_FRAME_DONE_IRQ, mxsfb->base + LCDC_CTRL1 + REG_CLR);
>  
> -	mxsfb_disable_axi_clk(mxsfb);
> +	clk_disable_unprepare(mxsfb->clk_axi);
>  
>  	return IRQ_HANDLED;
>  }
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.h
> b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
> index 8fb65d3..d6df8fe 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.h
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
> @@ -37,9 +37,6 @@ struct mxsfb_drm_private {
>  int mxsfb_setup_crtc(struct drm_device *dev);
>  int mxsfb_create_output(struct drm_device *dev);
>  
> -void mxsfb_enable_axi_clk(struct mxsfb_drm_private *mxsfb);
> -void mxsfb_disable_axi_clk(struct mxsfb_drm_private *mxsfb);
> -
>  void mxsfb_crtc_enable(struct mxsfb_drm_private *mxsfb);
>  void mxsfb_crtc_disable(struct mxsfb_drm_private *mxsfb);
>  void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb,

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 04/15] drm/mxsfb: Reset vital register for a proper initialization
  2019-08-14 10:48 ` [PATCH v2 04/15] drm/mxsfb: Reset vital register for a proper initialization Robert Chiras
@ 2019-08-14 11:11   ` Stefan Agner
  2019-08-14 11:38     ` [EXT] " Robert Chiras
  0 siblings, 1 reply; 27+ messages in thread
From: Stefan Agner @ 2019-08-14 11:11 UTC (permalink / raw)
  To: Robert Chiras
  Cc: Marek Vasut, Mark Rutland, Pengutronix Kernel Team, devicetree,
	David Airlie, Fabio Estevam, Guido Günther, linux-kernel,
	dri-devel, Rob Herring, NXP Linux Team, Daniel Vetter, Shawn Guo,
	Sascha Hauer, linux-arm-kernel

On 2019-08-14 12:48, Robert Chiras wrote:
> Some of the regiters need, like LCDC_CTRL and CTRL2_OUTSTANDING_REQS

Typo in registers, and there is a need to many.

> needs to be properly cleared and initialized for a better start and stop
> routine.



> 
> Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
> ---
>  drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> index b69ace8..5e44f57 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> @@ -127,6 +127,10 @@ static void mxsfb_enable_controller(struct
> mxsfb_drm_private *mxsfb)
>  		clk_prepare_enable(mxsfb->clk_disp_axi);
>  	clk_prepare_enable(mxsfb->clk);
>  
> +	if (mxsfb->devdata->ipversion >= 4)
> +		writel(CTRL2_OUTSTANDING_REQS(REQ_16),
> +		       mxsfb->base + LCDC_V4_CTRL2 + REG_SET);
> +
>  	/* If it was disabled, re-enable the mode again */
>  	writel(CTRL_DOTCLK_MODE, mxsfb->base + LCDC_CTRL + REG_SET);
>  
> @@ -136,12 +140,19 @@ static void mxsfb_enable_controller(struct
> mxsfb_drm_private *mxsfb)
>  	writel(reg, mxsfb->base + LCDC_VDCTRL4);
>  
>  	writel(CTRL_RUN, mxsfb->base + LCDC_CTRL + REG_SET);
> +	writel(CTRL1_RECOVERY_ON_UNDERFLOW, mxsfb->base + LCDC_CTRL1 + REG_SET);

This seems not to be accounted for in the commit message. Can you do
this in a separate commit?

Also I suggest to introduce CTRL1_RECOVERY_ON_UNDERFLOW in that same
commit.

--
Stefan

>  }
>  
>  static void mxsfb_disable_controller(struct mxsfb_drm_private *mxsfb)
>  {
>  	u32 reg;
>  
> +	if (mxsfb->devdata->ipversion >= 4)
> +		writel(CTRL2_OUTSTANDING_REQS(0x7),
> +		       mxsfb->base + LCDC_V4_CTRL2 + REG_CLR);
> +
> +	writel(CTRL_RUN, mxsfb->base + LCDC_CTRL + REG_CLR);
> +
>  	/*
>  	 * Even if we disable the controller here, it will still continue
>  	 * until its FIFOs are running out of data
> @@ -295,6 +306,7 @@ void mxsfb_crtc_enable(struct mxsfb_drm_private *mxsfb)
>  	dma_addr_t paddr;
>  
>  	mxsfb_enable_axi_clk(mxsfb);
> +	writel(0, mxsfb->base + LCDC_CTRL);
>  	mxsfb_crtc_mode_set_nofb(mxsfb);
>  
>  	/* Write cur_buf as well to avoid an initial corrupt frame */

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [EXT] Re: [PATCH v2 09/15] dt-bindings: display: Add max-res property for mxsfb
  2019-08-14 11:03   ` Stefan Agner
@ 2019-08-14 11:14     ` Robert Chiras
  2019-08-14 11:25       ` Stefan Agner
  0 siblings, 1 reply; 27+ messages in thread
From: Robert Chiras @ 2019-08-14 11:14 UTC (permalink / raw)
  To: stefan, mark.rutland, robh+dt
  Cc: marex, devicetree, kernel, airlied, shawnguo, agx, linux-kernel,
	dri-devel, dl-linux-imx, daniel, festevam, s.hauer,
	linux-arm-kernel

Hi Stefan,
On Mi, 2019-08-14 at 13:03 +0200, Stefan Agner wrote:
> On 2019-08-14 12:48, Robert Chiras wrote:
> > 
> > Add new optional property 'max-res', to limit the maximum supported
> > resolution by the MXSFB_DRM driver.
> I would also mention the reason why we need this.
> 
> I guess this needs a vendor prefix as well (fsl,max-res). I also
> would
> like to have the ack of the device tree folks here.
Rob Herring also aked be about this, and I'll copy here the reply, with
explanations:

Indeed, this limitation is actually due to bandwidth limitation, but
the problem is that this limitation comes on i.MX8M (known as mScale
850D), where the memory bandwidth cannot support: GPU/VPU workload in
the same time with both DCSS driving 4k@60 and eLCDIF driving 1080p@60.
Since eLCDIF is a secondary display we though to add the posibility to
limit it's bandwidth by limiting the resolution.
If you say that more details are needed, I can add them in the
description.
> 
> --
> Stefan
> 
> > 
> > 
> > Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
> > ---
> >  Documentation/devicetree/bindings/display/mxsfb.txt | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/display/mxsfb.txt
> > b/Documentation/devicetree/bindings/display/mxsfb.txt
> > index 472e1ea..55e22ed 100644
> > --- a/Documentation/devicetree/bindings/display/mxsfb.txt
> > +++ b/Documentation/devicetree/bindings/display/mxsfb.txt
> > @@ -17,6 +17,12 @@ Required properties:
> >  Required sub-nodes:
> >    - port: The connection to an encoder chip.
> > 
> > +Optional properties:
> > +- max-res:   an array with a maximum of two integers, representing
> > the
> > +             maximum supported resolution, in the form of
> > +             <maxX>, <maxY>; if one of the item is <0>, the
> > default
> > +             driver-defined maximum resolution for that axis is
> > used
> > +
> >  Example:
> > 
> >       lcdif1: display-controller@2220000 {

Thanks,
Robert
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [EXT] Re: [PATCH v2 09/15] dt-bindings: display: Add max-res property for mxsfb
  2019-08-14 11:14     ` [EXT] " Robert Chiras
@ 2019-08-14 11:25       ` Stefan Agner
  2019-08-14 12:03         ` Robert Chiras
  0 siblings, 1 reply; 27+ messages in thread
From: Stefan Agner @ 2019-08-14 11:25 UTC (permalink / raw)
  To: Robert Chiras
  Cc: mark.rutland, marex, kernel, devicetree, airlied, shawnguo, agx,
	linux-kernel, dri-devel, robh+dt, dl-linux-imx, daniel, festevam,
	s.hauer, linux-arm-kernel

On 2019-08-14 13:14, Robert Chiras wrote:
> Hi Stefan,
> On Mi, 2019-08-14 at 13:03 +0200, Stefan Agner wrote:
>> On 2019-08-14 12:48, Robert Chiras wrote:
>> >
>> > Add new optional property 'max-res', to limit the maximum supported
>> > resolution by the MXSFB_DRM driver.
>> I would also mention the reason why we need this.
>>
>> I guess this needs a vendor prefix as well (fsl,max-res). I also
>> would
>> like to have the ack of the device tree folks here.
> Rob Herring also aked be about this, and I'll copy here the reply, with
> explanations:
> 
> Indeed, this limitation is actually due to bandwidth limitation, but
> the problem is that this limitation comes on i.MX8M (known as mScale
> 850D), where the memory bandwidth cannot support: GPU/VPU workload in
> the same time with both DCSS driving 4k@60 and eLCDIF driving 1080p@60.
> Since eLCDIF is a secondary display we though to add the posibility to
> limit it's bandwidth by limiting the resolution.
> If you say that more details are needed, I can add them in the
> description.

Oh sorry I missed that.

Rob Herring also wrote:
> I suppose what you are after is bandwidth limits? IIRC, there's already 
> some bindings expressing such limits. Also, wouldn't you need to account 
> for bpp and using the 2nd plane (IIRC that there is one).

I guess the binding he refers to is max-memory-bandwidth, which is used
in multiple driver already. It makes sense to reuse this property
instead of inventing a new set of property which is also not taking bpp
into account...

The pl111 driver implements this property, it should be fairly easy to
adopt that code.

--
Stefan



>>
>> --
>> Stefan
>>
>> >
>> >
>> > Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
>> > ---
>> >  Documentation/devicetree/bindings/display/mxsfb.txt | 6 ++++++
>> >  1 file changed, 6 insertions(+)
>> >
>> > diff --git a/Documentation/devicetree/bindings/display/mxsfb.txt
>> > b/Documentation/devicetree/bindings/display/mxsfb.txt
>> > index 472e1ea..55e22ed 100644
>> > --- a/Documentation/devicetree/bindings/display/mxsfb.txt
>> > +++ b/Documentation/devicetree/bindings/display/mxsfb.txt
>> > @@ -17,6 +17,12 @@ Required properties:
>> >  Required sub-nodes:
>> >    - port: The connection to an encoder chip.
>> >
>> > +Optional properties:
>> > +- max-res:   an array with a maximum of two integers, representing
>> > the
>> > +             maximum supported resolution, in the form of
>> > +             <maxX>, <maxY>; if one of the item is <0>, the
>> > default
>> > +             driver-defined maximum resolution for that axis is
>> > used
>> > +
>> >  Example:
>> >
>> >       lcdif1: display-controller@2220000 {
> 
> Thanks,
> Robert

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [EXT] Re: [PATCH v2 12/15] drm/mxsfb: Improve the axi clock usage
  2019-08-14 11:06   ` Stefan Agner
@ 2019-08-14 11:35     ` Robert Chiras
  0 siblings, 0 replies; 27+ messages in thread
From: Robert Chiras @ 2019-08-14 11:35 UTC (permalink / raw)
  To: stefan
  Cc: marex, devicetree, kernel, airlied, shawnguo, agx, linux-kernel,
	dri-devel, robh+dt, dl-linux-imx, daniel, mark.rutland, festevam,
	s.hauer, linux-arm-kernel

Hi Stefan,

On Mi, 2019-08-14 at 13:06 +0200, Stefan Agner wrote:
> On 2019-08-14 12:48, Robert Chiras wrote:
> > 
> > Currently, the enable of the axi clock return status is ignored,
> > causing
> > issues when the enable fails then we try to disable it. Therefore,
> > it is
> > better to check the return status and disable it only when enable
> > succeeded.
> Is this actually the case in real world sometimes? Why is it failing?
When I noticed that fail, we had some restrictions in SCU firmware, so
that the clock cannot be enabled if the power domain was down. Still,
at that time I noticed it is redundant to have functions that checks if
a clock is NULL or not, since the clk_* functions are already doing
this.
> 
> I guess if we do this in one place, we should do it in all places
> (e.g.
> also in mxsfb_crtc_enable, mxsfb_plane_atomic_update..)
I add specific checks only in the vblank functions, because those
functions can be called before calling the pipe enable/disable
functions which enables the spefic power domain and, at this point, the
clock enable/disable calls should not fail.
> 
> --
> Stefan
> 
> > 
> > Also, remove the helper functions around clk_axi, since we can
> > directly
> > use the clk API function for enable/disable the clock. Those
> > functions
> > are already checking for NULL clk and returning 0 if that's the
> > case.
> 
> > 
> > 
> > Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
> > Acked-by: Leonard Crestez <leonard.crestez@nxp.com>
> > ---
> >  drivers/gpu/drm/mxsfb/mxsfb_crtc.c |  8 ++++----
> >  drivers/gpu/drm/mxsfb/mxsfb_drv.c  | 32 +++++++++++++-------------
> > ------
> >  drivers/gpu/drm/mxsfb/mxsfb_drv.h  |  3 ---
> >  3 files changed, 17 insertions(+), 26 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> > b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> > index a4ba368..e727f5e 100644
> > --- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> > +++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> > @@ -408,7 +408,7 @@ void mxsfb_crtc_enable(struct mxsfb_drm_private
> > *mxsfb)
> >  {
> >       dma_addr_t paddr;
> > 
> > -     mxsfb_enable_axi_clk(mxsfb);
> > +     clk_prepare_enable(mxsfb->clk_axi);
> >       writel(0, mxsfb->base + LCDC_CTRL);
> >       mxsfb_crtc_mode_set_nofb(mxsfb);
> > 
> > @@ -425,7 +425,7 @@ void mxsfb_crtc_enable(struct mxsfb_drm_private
> > *mxsfb)
> >  void mxsfb_crtc_disable(struct mxsfb_drm_private *mxsfb)
> >  {
> >       mxsfb_disable_controller(mxsfb);
> > -     mxsfb_disable_axi_clk(mxsfb);
> > +     clk_disable_unprepare(mxsfb->clk_axi);
> >  }
> > 
> >  void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb,
> > @@ -451,8 +451,8 @@ void mxsfb_plane_atomic_update(struct
> > mxsfb_drm_private *mxsfb,
> > 
> >       paddr = mxsfb_get_fb_paddr(mxsfb);
> >       if (paddr) {
> > -             mxsfb_enable_axi_clk(mxsfb);
> > +             clk_prepare_enable(mxsfb->clk_axi);
> >               writel(paddr, mxsfb->base + mxsfb->devdata-
> > >next_buf);
> > -             mxsfb_disable_axi_clk(mxsfb);
> > +             clk_disable_unprepare(mxsfb->clk_axi);
> >       }
> >  }
> > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> > b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> > index 6dae2bd..694b287 100644
> > --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> > +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> > @@ -97,18 +97,6 @@ drm_pipe_to_mxsfb_drm_private(struct
> > drm_simple_display_pipe *pipe)
> >       return container_of(pipe, struct mxsfb_drm_private, pipe);
> >  }
> > 
> > -void mxsfb_enable_axi_clk(struct mxsfb_drm_private *mxsfb)
> > -{
> > -     if (mxsfb->clk_axi)
> > -             clk_prepare_enable(mxsfb->clk_axi);
> > -}
> > -
> > -void mxsfb_disable_axi_clk(struct mxsfb_drm_private *mxsfb)
> > -{
> > -     if (mxsfb->clk_axi)
> > -             clk_disable_unprepare(mxsfb->clk_axi);
> > -}
> > -
> >  /**
> >   * mxsfb_atomic_helper_check - validate state object
> >   * @dev: DRM device
> > @@ -229,25 +217,31 @@ static void mxsfb_pipe_update(struct
> > drm_simple_display_pipe *pipe,
> >  static int mxsfb_pipe_enable_vblank(struct drm_simple_display_pipe
> > *pipe)
> >  {
> >       struct mxsfb_drm_private *mxsfb =
> > drm_pipe_to_mxsfb_drm_private(pipe);
> > +     int ret = 0;
> > +
> > +     ret = clk_prepare_enable(mxsfb->clk_axi);
> > +     if (ret)
> > +             return ret;
> > 
> >       /* Clear and enable VBLANK IRQ */
> > -     mxsfb_enable_axi_clk(mxsfb);
> >       writel(CTRL1_CUR_FRAME_DONE_IRQ, mxsfb->base + LCDC_CTRL1 +
> > REG_CLR);
> >       writel(CTRL1_CUR_FRAME_DONE_IRQ_EN, mxsfb->base + LCDC_CTRL1
> > + REG_SET);
> > -     mxsfb_disable_axi_clk(mxsfb);
> > +     clk_disable_unprepare(mxsfb->clk_axi);
> > 
> > -     return 0;
> > +     return ret;
> >  }
> > 
> >  static void mxsfb_pipe_disable_vblank(struct
> > drm_simple_display_pipe *pipe)
> >  {
> >       struct mxsfb_drm_private *mxsfb =
> > drm_pipe_to_mxsfb_drm_private(pipe);
> > 
> > +     if (clk_prepare_enable(mxsfb->clk_axi))
> > +             return;
> > +
> >       /* Disable and clear VBLANK IRQ */
> > -     mxsfb_enable_axi_clk(mxsfb);
> >       writel(CTRL1_CUR_FRAME_DONE_IRQ_EN, mxsfb->base + LCDC_CTRL1
> > + REG_CLR);
> >       writel(CTRL1_CUR_FRAME_DONE_IRQ, mxsfb->base + LCDC_CTRL1 +
> > REG_CLR);
> > -     mxsfb_disable_axi_clk(mxsfb);
> > +     clk_disable_unprepare(mxsfb->clk_axi);
> >  }
> > 
> >  static struct drm_simple_display_pipe_funcs mxsfb_funcs = {
> > @@ -413,7 +407,7 @@ static irqreturn_t mxsfb_irq_handler(int irq,
> > void *data)
> >       struct mxsfb_drm_private *mxsfb = drm->dev_private;
> >       u32 reg;
> > 
> > -     mxsfb_enable_axi_clk(mxsfb);
> > +     clk_prepare_enable(mxsfb->clk_axi);
> > 
> >       reg = readl(mxsfb->base + LCDC_CTRL1);
> > 
> > @@ -422,7 +416,7 @@ static irqreturn_t mxsfb_irq_handler(int irq,
> > void *data)
> > 
> >       writel(CTRL1_CUR_FRAME_DONE_IRQ, mxsfb->base + LCDC_CTRL1 +
> > REG_CLR);
> > 
> > -     mxsfb_disable_axi_clk(mxsfb);
> > +     clk_disable_unprepare(mxsfb->clk_axi);
> > 
> >       return IRQ_HANDLED;
> >  }
> > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.h
> > b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
> > index 8fb65d3..d6df8fe 100644
> > --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.h
> > +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
> > @@ -37,9 +37,6 @@ struct mxsfb_drm_private {
> >  int mxsfb_setup_crtc(struct drm_device *dev);
> >  int mxsfb_create_output(struct drm_device *dev);
> > 
> > -void mxsfb_enable_axi_clk(struct mxsfb_drm_private *mxsfb);
> > -void mxsfb_disable_axi_clk(struct mxsfb_drm_private *mxsfb);
> > -
> >  void mxsfb_crtc_enable(struct mxsfb_drm_private *mxsfb);
> >  void mxsfb_crtc_disable(struct mxsfb_drm_private *mxsfb);
> >  void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb,

Thanks,
Robert
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [EXT] Re: [PATCH v2 04/15] drm/mxsfb: Reset vital register for a proper initialization
  2019-08-14 11:11   ` Stefan Agner
@ 2019-08-14 11:38     ` Robert Chiras
  0 siblings, 0 replies; 27+ messages in thread
From: Robert Chiras @ 2019-08-14 11:38 UTC (permalink / raw)
  To: stefan
  Cc: marex, devicetree, kernel, airlied, shawnguo, agx, linux-kernel,
	dri-devel, robh+dt, dl-linux-imx, daniel, mark.rutland, festevam,
	s.hauer, linux-arm-kernel

Hi Stefan,

On Mi, 2019-08-14 at 13:11 +0200, Stefan Agner wrote:
> On 2019-08-14 12:48, Robert Chiras wrote:
> > 
> > Some of the regiters need, like LCDC_CTRL and
> > CTRL2_OUTSTANDING_REQS
> Typo in registers, and there is a need to many.
Thanks, will fix this.
> 
> > 
> > needs to be properly cleared and initialized for a better start and
> > stop
> > routine.
> 
> 
> > 
> > 
> > Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
> > ---
> >  drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> > b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> > index b69ace8..5e44f57 100644
> > --- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> > +++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> > @@ -127,6 +127,10 @@ static void mxsfb_enable_controller(struct
> > mxsfb_drm_private *mxsfb)
> >               clk_prepare_enable(mxsfb->clk_disp_axi);
> >       clk_prepare_enable(mxsfb->clk);
> > 
> > +     if (mxsfb->devdata->ipversion >= 4)
> > +             writel(CTRL2_OUTSTANDING_REQS(REQ_16),
> > +                    mxsfb->base + LCDC_V4_CTRL2 + REG_SET);
> > +
> >       /* If it was disabled, re-enable the mode again */
> >       writel(CTRL_DOTCLK_MODE, mxsfb->base + LCDC_CTRL + REG_SET);
> > 
> > @@ -136,12 +140,19 @@ static void mxsfb_enable_controller(struct
> > mxsfb_drm_private *mxsfb)
> >       writel(reg, mxsfb->base + LCDC_VDCTRL4);
> > 
> >       writel(CTRL_RUN, mxsfb->base + LCDC_CTRL + REG_SET);
> > +     writel(CTRL1_RECOVERY_ON_UNDERFLOW, mxsfb->base + LCDC_CTRL1
> > + REG_SET);
> This seems not to be accounted for in the commit message. Can you do
> this in a separate commit?
> 
> Also I suggest to introduce CTRL1_RECOVERY_ON_UNDERFLOW in that same
> commit.
You are right, I missed this one in the description. I will add this
one too.
> 
> --
> Stefan
> 
> > 
> >  }
> > 
> >  static void mxsfb_disable_controller(struct mxsfb_drm_private
> > *mxsfb)
> >  {
> >       u32 reg;
> > 
> > +     if (mxsfb->devdata->ipversion >= 4)
> > +             writel(CTRL2_OUTSTANDING_REQS(0x7),
> > +                    mxsfb->base + LCDC_V4_CTRL2 + REG_CLR);
> > +
> > +     writel(CTRL_RUN, mxsfb->base + LCDC_CTRL + REG_CLR);
> > +
> >       /*
> >        * Even if we disable the controller here, it will still
> > continue
> >        * until its FIFOs are running out of data
> > @@ -295,6 +306,7 @@ void mxsfb_crtc_enable(struct mxsfb_drm_private
> > *mxsfb)
> >       dma_addr_t paddr;
> > 
> >       mxsfb_enable_axi_clk(mxsfb);
> > +     writel(0, mxsfb->base + LCDC_CTRL);
> >       mxsfb_crtc_mode_set_nofb(mxsfb);
> > 
> >       /* Write cur_buf as well to avoid an initial corrupt frame */

Thanks,
Robert
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 06/15] drm/mxsfb: Update mxsfb with additional pixel formats
  2019-08-14 10:48 ` [PATCH v2 06/15] drm/mxsfb: Update mxsfb with additional pixel formats Robert Chiras
@ 2019-08-14 11:44   ` Daniel Stone
  2019-08-14 14:58     ` [EXT] " Robert Chiras
  0 siblings, 1 reply; 27+ messages in thread
From: Daniel Stone @ 2019-08-14 11:44 UTC (permalink / raw)
  To: Robert Chiras
  Cc: Marek Vasut, Mark Rutland, Pengutronix Kernel Team, dri-devel,
	devicetree, David Airlie, Fabio Estevam, Guido Günther,
	Linux Kernel Mailing List, Stefan Agner, Rob Herring,
	NXP Linux Team, Daniel Vetter, Shawn Guo, Sascha Hauer,
	linux-arm-kernel

Hi Robert,

On Wed, 14 Aug 2019 at 11:49, Robert Chiras <robert.chiras@nxp.com> wrote:
> +       case DRM_FORMAT_BGR565: /* BG16 */
> +               if (mxsfb->devdata->ipversion < 4)
> +                       goto err;
> +               writel(CTRL2_ODD_LINE_PATTERN(CTRL2_LINE_PATTERN_BGR) |
> +                       CTRL2_EVEN_LINE_PATTERN(CTRL2_LINE_PATTERN_BGR),
> +                       mxsfb->base + LCDC_V4_CTRL2 + REG_SET);
> +               /* Fall through */
> +       case DRM_FORMAT_RGB565: /* RG16 */
> +               ctrl |= CTRL_SET_WORD_LENGTH(0);
> +               ctrl &= ~CTRL_DF16;
> +               ctrl1 |= CTRL1_SET_BYTE_PACKAGING(0xf);
> +               break;

For non-BGR formats, do you need to write RGB line-pattern back to the
CTRL2 register? Otherwise, if you start with BGR565 then switch back
to RGB565, presumably CTRL2 would still be programmed for BGR so you
would display inverted channels.

Same goes for all the other BGR/RGB format pairs below.

Cheers,
Daniel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [EXT] Re: [PATCH v2 09/15] dt-bindings: display: Add max-res property for mxsfb
  2019-08-14 11:25       ` Stefan Agner
@ 2019-08-14 12:03         ` Robert Chiras
  0 siblings, 0 replies; 27+ messages in thread
From: Robert Chiras @ 2019-08-14 12:03 UTC (permalink / raw)
  To: stefan
  Cc: marex, devicetree, kernel, airlied, shawnguo, agx, linux-kernel,
	dri-devel, robh+dt, dl-linux-imx, daniel, mark.rutland, festevam,
	s.hauer, linux-arm-kernel

Hi Stefann,
On Mi, 2019-08-14 at 13:25 +0200, Stefan Agner wrote:
> On 2019-08-14 13:14, Robert Chiras wrote:
> > 
> > Hi Stefan,
> > On Mi, 2019-08-14 at 13:03 +0200, Stefan Agner wrote:
> > > 
> > > On 2019-08-14 12:48, Robert Chiras wrote:
> > > > 
> > > > 
> > > > Add new optional property 'max-res', to limit the maximum
> > > > supported
> > > > resolution by the MXSFB_DRM driver.
> > > I would also mention the reason why we need this.
> > > 
> > > I guess this needs a vendor prefix as well (fsl,max-res). I also
> > > would
> > > like to have the ack of the device tree folks here.
> > Rob Herring also aked be about this, and I'll copy here the reply,
> > with
> > explanations:
> > 
> > Indeed, this limitation is actually due to bandwidth limitation,
> > but
> > the problem is that this limitation comes on i.MX8M (known as
> > mScale
> > 850D), where the memory bandwidth cannot support: GPU/VPU workload
> > in
> > the same time with both DCSS driving 4k@60 and eLCDIF driving 1080p
> > @60.
> > Since eLCDIF is a secondary display we though to add the posibility
> > to
> > limit it's bandwidth by limiting the resolution.
> > If you say that more details are needed, I can add them in the
> > description.
> Oh sorry I missed that.
> 
> Rob Herring also wrote:
> > 
> > I suppose what you are after is bandwidth limits? IIRC, there's
> > already
> > some bindings expressing such limits. Also, wouldn't you need to
> > account
> > for bpp and using the 2nd plane (IIRC that there is one).
> I guess the binding he refers to is max-memory-bandwidth, which is
> used
> in multiple driver already. It makes sense to reuse this property
> instead of inventing a new set of property which is also not taking
> bpp
> into account...
Actually I saw this binding, but I thought it is strictly related to
that driver, where it is documented. Now, that you say this one is used
in many drivers (and recalling that you suggested for my 'max-res' to
add a prefix), I get it now.
> 
> The pl111 driver implements this property, it should be fairly easy
> to
> adopt that code.
Sure, I will do something similar here, too.
> 
> --
> Stefan
> 
> 
> 
> > 
> > > 
> > > 
> > > --
> > > Stefan
> > > 
> > > > 
> > > > 
> > > > 
> > > > Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
> > > > ---
> > > >  Documentation/devicetree/bindings/display/mxsfb.txt | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > > 
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/display/mxsfb.txt
> > > > b/Documentation/devicetree/bindings/display/mxsfb.txt
> > > > index 472e1ea..55e22ed 100644
> > > > --- a/Documentation/devicetree/bindings/display/mxsfb.txt
> > > > +++ b/Documentation/devicetree/bindings/display/mxsfb.txt
> > > > @@ -17,6 +17,12 @@ Required properties:
> > > >  Required sub-nodes:
> > > >    - port: The connection to an encoder chip.
> > > > 
> > > > +Optional properties:
> > > > +- max-res:   an array with a maximum of two integers,
> > > > representing
> > > > the
> > > > +             maximum supported resolution, in the form of
> > > > +             <maxX>, <maxY>; if one of the item is <0>, the
> > > > default
> > > > +             driver-defined maximum resolution for that axis
> > > > is
> > > > used
> > > > +
> > > >  Example:
> > > > 
> > > >       lcdif1: display-controller@2220000 {
> > Thanks,
> > Robert
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [EXT] Re: [PATCH v2 06/15] drm/mxsfb: Update mxsfb with additional pixel formats
  2019-08-14 11:44   ` Daniel Stone
@ 2019-08-14 14:58     ` Robert Chiras
  0 siblings, 0 replies; 27+ messages in thread
From: Robert Chiras @ 2019-08-14 14:58 UTC (permalink / raw)
  To: daniel
  Cc: marex, devicetree, s.hauer, dri-devel, airlied, shawnguo, agx,
	linux-kernel, stefan, robh+dt, dl-linux-imx, daniel,
	mark.rutland, festevam, kernel, linux-arm-kernel

Hi Daniel,

On Mi, 2019-08-14 at 12:44 +0100, Daniel Stone wrote:
> Hi Robert,
> 
> On Wed, 14 Aug 2019 at 11:49, Robert Chiras <robert.chiras@nxp.com>
> wrote:
> > 
> > +       case DRM_FORMAT_BGR565: /* BG16 */
> > +               if (mxsfb->devdata->ipversion < 4)
> > +                       goto err;
> > +               writel(CTRL2_ODD_LINE_PATTERN(CTRL2_LINE_PATTERN_BG
> > R) |
> > +                       CTRL2_EVEN_LINE_PATTERN(CTRL2_LINE_PATTERN_
> > BGR),
> > +                       mxsfb->base + LCDC_V4_CTRL2 + REG_SET);
> > +               /* Fall through */
> > +       case DRM_FORMAT_RGB565: /* RG16 */
> > +               ctrl |= CTRL_SET_WORD_LENGTH(0);
> > +               ctrl &= ~CTRL_DF16;
> > +               ctrl1 |= CTRL1_SET_BYTE_PACKAGING(0xf);
> > +               break;
> For non-BGR formats, do you need to write RGB line-pattern back to
> the
> CTRL2 register? Otherwise, if you start with BGR565 then switch back
> to RGB565, presumably CTRL2 would still be programmed for BGR so you
> would display inverted channels.
The LINE_PATTERN from LCDC_V4_CTRL2 register is cleared above, with
this code:
+       if (mxsfb->devdata->ipversion >= 4)
+               writel(CTRL2_ODD_LINE_PATTERN(CTRL2_LINE_PATTERN_CLR) |
+                      CTRL2_EVEN_LINE_PATTERN(CTRL2_LINE_PATTERN_CLR),
+                      mxsfb->base + LCDC_V4_CTRL2 + REG_CLR);
> 
> Same goes for all the other BGR/RGB format pairs below.
> 
> Cheers,
> Daniel

Thanks,
Robert
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 07/15] drm/mxsfb: Fix the vblank events
  2019-08-14 10:48 ` [PATCH v2 07/15] drm/mxsfb: Fix the vblank events Robert Chiras
@ 2019-08-14 17:31   ` Daniel Vetter
  0 siblings, 0 replies; 27+ messages in thread
From: Daniel Vetter @ 2019-08-14 17:31 UTC (permalink / raw)
  To: Robert Chiras
  Cc: Marek Vasut, Mark Rutland, Pengutronix Kernel Team, dri-devel,
	devicetree, David Airlie, Fabio Estevam, Guido Günther,
	linux-kernel, Stefan Agner, Rob Herring, NXP Linux Team,
	Daniel Vetter, Shawn Guo, Sascha Hauer, linux-arm-kernel

On Wed, Aug 14, 2019 at 01:48:43PM +0300, Robert Chiras wrote:
> Currently, the vblank support is not correctly implemented in MXSFB_DRM
> driver. The call to drm_vblank_init is made with mode_config.num_crtc
> which at that time is 0. Because of this, vblank is not activated, so
> there won't be any vblank event submitted.
> For example, when running modetest with the '-v' parameter will result
> in an astronomical refresh rate (10000+ Hz), because of that.

Uh, that sounds a bit like a bug somewhere. If vblank doesn't work, we
should give userspace an error back.

Maybe we need more checks in drm_vblank_init()? Can you pls look into
that?

> 
> Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
> ---
>  drivers/gpu/drm/mxsfb/mxsfb_drv.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> index 2743975..829abec 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> @@ -38,6 +38,9 @@
>  #include "mxsfb_drv.h"
>  #include "mxsfb_regs.h"
>  
> +/* The eLCDIF max possible CRTCs */
> +#define MAX_CRTCS 1
> +
>  enum mxsfb_devtype {
>  	MXSFB_V3,
>  	MXSFB_V4,
> @@ -138,6 +141,8 @@ static void mxsfb_pipe_enable(struct drm_simple_display_pipe *pipe,
>  		mxsfb->connector = &mxsfb->panel_connector;
>  	}
>  
> +	drm_crtc_vblank_on(&pipe->crtc);
> +
>  	pm_runtime_get_sync(drm->dev);
>  	drm_panel_prepare(mxsfb->panel);
>  	mxsfb_crtc_enable(mxsfb);
> @@ -164,6 +169,8 @@ static void mxsfb_pipe_disable(struct drm_simple_display_pipe *pipe)
>  	}
>  	spin_unlock_irq(&drm->event_lock);
>  
> +	drm_crtc_vblank_off(&pipe->crtc);
> +
>  	if (mxsfb->connector != &mxsfb->panel_connector)
>  		mxsfb->connector = NULL;
>  }
> @@ -246,7 +253,7 @@ static int mxsfb_load(struct drm_device *drm, unsigned long flags)
>  
>  	pm_runtime_enable(drm->dev);
>  
> -	ret = drm_vblank_init(drm, drm->mode_config.num_crtc);
> +	ret = drm_vblank_init(drm, MAX_CRTCS);
>  	if (ret < 0) {
>  		dev_err(drm->dev, "Failed to initialise vblank\n");
>  		goto err_vblank;
> @@ -269,6 +276,8 @@ static int mxsfb_load(struct drm_device *drm, unsigned long flags)
>  		goto err_vblank;
>  	}
>  
> +	drm_crtc_vblank_off(&mxsfb->pipe.crtc);

Are you sure you need this one here? vblanks should be off after
drm_vblank_init() is called.

Thanks, Daniel

> +
>  	/*
>  	 * Attach panel only if there is one.
>  	 * If there is no panel attach, it must be a bridge. In this case, we
> -- 
> 2.7.4
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2019-08-14 17:32 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-14 10:48 [PATCH v2 00/15] Improvements and fixes for mxsfb DRM driver Robert Chiras
2019-08-14 10:48 ` [PATCH v2 01/15] drm/mxsfb: Update mxsfb to support a bridge Robert Chiras
2019-08-14 10:48 ` [PATCH v2 02/15] drm/mxsfb: Read bus flags from bridge if present Robert Chiras
2019-08-14 10:48 ` [PATCH v2 03/15] drm/mxsfb: Add defines for the rest of registers Robert Chiras
2019-08-14 10:48 ` [PATCH v2 04/15] drm/mxsfb: Reset vital register for a proper initialization Robert Chiras
2019-08-14 11:11   ` Stefan Agner
2019-08-14 11:38     ` [EXT] " Robert Chiras
2019-08-14 10:48 ` [PATCH v2 05/15] drm/mxsfb: Update register definitions using bit manipulation defines Robert Chiras
2019-08-14 10:48 ` [PATCH v2 06/15] drm/mxsfb: Update mxsfb with additional pixel formats Robert Chiras
2019-08-14 11:44   ` Daniel Stone
2019-08-14 14:58     ` [EXT] " Robert Chiras
2019-08-14 10:48 ` [PATCH v2 07/15] drm/mxsfb: Fix the vblank events Robert Chiras
2019-08-14 17:31   ` Daniel Vetter
2019-08-14 10:48 ` [PATCH v2 08/15] drm/mxsfb: Signal mode changed when bpp changed Robert Chiras
2019-08-14 10:48 ` [PATCH v2 09/15] dt-bindings: display: Add max-res property for mxsfb Robert Chiras
2019-08-14 11:03   ` Stefan Agner
2019-08-14 11:14     ` [EXT] " Robert Chiras
2019-08-14 11:25       ` Stefan Agner
2019-08-14 12:03         ` Robert Chiras
2019-08-14 10:48 ` [PATCH v2 10/15] drm/mxsfb: Add max-res property for MXSFB Robert Chiras
2019-08-14 10:48 ` [PATCH v2 11/15] drm/mxsfb: Update mxsfb to support LCD reset Robert Chiras
2019-08-14 10:48 ` [PATCH v2 12/15] drm/mxsfb: Improve the axi clock usage Robert Chiras
2019-08-14 11:06   ` Stefan Agner
2019-08-14 11:35     ` [EXT] " Robert Chiras
2019-08-14 10:48 ` [PATCH v2 13/15] drm/mxsfb: Clear OUTSTANDING_REQS bits Robert Chiras
2019-08-14 10:48 ` [PATCH v2 14/15] drm/mxsfb: Add support for horizontal stride Robert Chiras
2019-08-14 10:48 ` [PATCH v2 15/15] drm/mxsfb: Add support for live pixel format change Robert Chiras

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).