[PATCH 01/14] drm/kmb: Enable LCD DMA for low TVDDCV

[PATCH 01/14] drm/kmb: Enable LCD DMA for low TVDDCV

[Anitha Chrisanthus]

From: Edmund Dea <edmund.j.dea@intel.com>

There's an undocumented dependency between LCD layer enable bits [2-5]
and the AXI pipelined read enable bit [28] in the LCD_CONTROL register.
The proper order of operation is:

1) Clear AXI pipelined read enable bit
2) Set LCD layers
3) Set AXI pipelined read enable bit

With this update, LCD can start DMA when TVDDCV is reduced down to 700mV.

Fixes: 7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display")
Signed-off-by: Edmund Dea <edmund.j.dea@intel.com>
---
 drivers/gpu/drm/kmb/kmb_drv.c   | 14 ++++++++++++++
 drivers/gpu/drm/kmb/kmb_plane.c | 15 +++++++++++++--
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/kmb/kmb_drv.c b/drivers/gpu/drm/kmb/kmb_drv.c
index 96ea1a2c11dd..c0b1c6f99249 100644
--- a/drivers/gpu/drm/kmb/kmb_drv.c
+++ b/drivers/gpu/drm/kmb/kmb_drv.c
@@ -203,6 +203,7 @@ static irqreturn_t handle_lcd_irq(struct drm_device *dev)
 	unsigned long status, val, val1;
 	int plane_id, dma0_state, dma1_state;
 	struct kmb_drm_private *kmb = to_kmb(dev);
+	u32 ctrl = 0;
 
 	status = kmb_read_lcd(kmb, LCD_INT_STATUS);
 
@@ -227,6 +228,19 @@ static irqreturn_t handle_lcd_irq(struct drm_device *dev)
 				kmb_clr_bitmask_lcd(kmb, LCD_CONTROL,
 						    kmb->plane_status[plane_id].ctrl);
 
+				ctrl = kmb_read_lcd(kmb, LCD_CONTROL);
+				if (!(ctrl & (LCD_CTRL_VL1_ENABLE |
+				    LCD_CTRL_VL2_ENABLE |
+				    LCD_CTRL_GL1_ENABLE |
+				    LCD_CTRL_GL2_ENABLE))) {
+					/* If no LCD layers are using DMA,
+					 * then disable DMA pipelined AXI read
+					 * transactions.
+					 */
+					kmb_clr_bitmask_lcd(kmb, LCD_CONTROL,
+							    LCD_CTRL_PIPELINE_DMA);
+				}
+
 				kmb->plane_status[plane_id].disable = false;
 			}
 		}
diff --git a/drivers/gpu/drm/kmb/kmb_plane.c b/drivers/gpu/drm/kmb/kmb_plane.c
index d5b6195856d1..2888dd5dcc2c 100644
--- a/drivers/gpu/drm/kmb/kmb_plane.c
+++ b/drivers/gpu/drm/kmb/kmb_plane.c
@@ -427,8 +427,14 @@ static void kmb_plane_atomic_update(struct drm_plane *plane,
 
 	kmb_set_bitmask_lcd(kmb, LCD_CONTROL, ctrl);
 
-	/* FIXME no doc on how to set output format,these values are
-	 * taken from the Myriadx tests
+	/* Enable pipeline AXI read transactions for the DMA
+	 * after setting graphics layers. This must be done
+	 * in a separate write cycle.
+	 */
+	kmb_set_bitmask_lcd(kmb, LCD_CONTROL, LCD_CTRL_PIPELINE_DMA);
+
+	/* FIXME no doc on how to set output format,these values are taken
+	 * from the Myriadx tests
 	 */
 	out_format |= LCD_OUTF_FORMAT_RGB888;
 
@@ -526,6 +532,11 @@ struct kmb_plane *kmb_plane_init(struct drm_device *drm)
 		plane->id = i;
 	}
 
+	/* Disable pipeline AXI read transactions for the DMA
+	 * prior to setting graphics layers
+	 */
+	kmb_clr_bitmask_lcd(kmb, LCD_CONTROL, LCD_CTRL_PIPELINE_DMA);
+
 	return primary;
 cleanup:
 	drmm_kfree(drm, plane);
-- 
2.25.1

[PATCH 02/14] drm/kmb: Define driver date and major/minor version

[Anitha Chrisanthus]

From: Edmund Dea <edmund.j.dea@intel.com>

Added macros for date and version

Fixes: 7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display")
Signed-off-by: Edmund Dea <edmund.j.dea@intel.com>
---
 drivers/gpu/drm/kmb/kmb_drv.c | 8 ++++----
 drivers/gpu/drm/kmb/kmb_drv.h | 5 +++++
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/kmb/kmb_drv.c b/drivers/gpu/drm/kmb/kmb_drv.c
index c0b1c6f99249..f54392ec4fab 100644
--- a/drivers/gpu/drm/kmb/kmb_drv.c
+++ b/drivers/gpu/drm/kmb/kmb_drv.c
@@ -425,10 +425,10 @@ static const struct drm_driver kmb_driver = {
 	.fops = &fops,
 	DRM_GEM_CMA_DRIVER_OPS_VMAP,
 	.name = "kmb-drm",
-	.desc = "KEEMBAY DISPLAY DRIVER ",
-	.date = "20201008",
-	.major = 1,
-	.minor = 0,
+	.desc = "KEEMBAY DISPLAY DRIVER",
+	.date = DRIVER_DATE,
+	.major = DRIVER_MAJOR,
+	.minor = DRIVER_MINOR,
 };
 
 static int kmb_remove(struct platform_device *pdev)
diff --git a/drivers/gpu/drm/kmb/kmb_drv.h b/drivers/gpu/drm/kmb/kmb_drv.h
index 02e806712a64..ebbaa5f422d5 100644
--- a/drivers/gpu/drm/kmb/kmb_drv.h
+++ b/drivers/gpu/drm/kmb/kmb_drv.h
@@ -15,6 +15,11 @@
 #define KMB_MAX_HEIGHT			1080 /*Max height in pixels */
 #define KMB_MIN_WIDTH                   1920 /*Max width in pixels */
 #define KMB_MIN_HEIGHT                  1080 /*Max height in pixels */
+
+#define DRIVER_DATE			"20210223"
+#define DRIVER_MAJOR			1
+#define DRIVER_MINOR			1
+
 #define KMB_LCD_DEFAULT_CLK		200000000
 #define KMB_SYS_CLK_MHZ			500
 
-- 
2.25.1

Re: [PATCH 02/14] drm/kmb: Define driver date and major/minor version

[Sam Ravnborg]

Hi Anitha,
On Tue, Jul 27, 2021 at 05:31:14PM -0700, Anitha Chrisanthus wrote:
> From: Edmund Dea <edmund.j.dea@intel.com>
> 
> Added macros for date and version
> 
> Fixes: 7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display")
> Signed-off-by: Edmund Dea <edmund.j.dea@intel.com>
Your s-o-b is missing.

I find it of no use with macros here, as the figures are not used
anywhere else, but whatever.

With s-o-b fixed:
Acked-by: Sam Ravnborg <sam@ravnborg.org>

> ---
>  drivers/gpu/drm/kmb/kmb_drv.c | 8 ++++----
>  drivers/gpu/drm/kmb/kmb_drv.h | 5 +++++
>  2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/kmb/kmb_drv.c b/drivers/gpu/drm/kmb/kmb_drv.c
> index c0b1c6f99249..f54392ec4fab 100644
> --- a/drivers/gpu/drm/kmb/kmb_drv.c
> +++ b/drivers/gpu/drm/kmb/kmb_drv.c
> @@ -425,10 +425,10 @@ static const struct drm_driver kmb_driver = {
>  	.fops = &fops,
>  	DRM_GEM_CMA_DRIVER_OPS_VMAP,
>  	.name = "kmb-drm",
> -	.desc = "KEEMBAY DISPLAY DRIVER ",
> -	.date = "20201008",
> -	.major = 1,
> -	.minor = 0,
> +	.desc = "KEEMBAY DISPLAY DRIVER",
> +	.date = DRIVER_DATE,
> +	.major = DRIVER_MAJOR,
> +	.minor = DRIVER_MINOR,
>  };
>  
>  static int kmb_remove(struct platform_device *pdev)
> diff --git a/drivers/gpu/drm/kmb/kmb_drv.h b/drivers/gpu/drm/kmb/kmb_drv.h
> index 02e806712a64..ebbaa5f422d5 100644
> --- a/drivers/gpu/drm/kmb/kmb_drv.h
> +++ b/drivers/gpu/drm/kmb/kmb_drv.h
> @@ -15,6 +15,11 @@
>  #define KMB_MAX_HEIGHT			1080 /*Max height in pixels */
>  #define KMB_MIN_WIDTH                   1920 /*Max width in pixels */
>  #define KMB_MIN_HEIGHT                  1080 /*Max height in pixels */
> +
> +#define DRIVER_DATE			"20210223"
> +#define DRIVER_MAJOR			1
> +#define DRIVER_MINOR			1
> +
>  #define KMB_LCD_DEFAULT_CLK		200000000
>  #define KMB_SYS_CLK_MHZ			500
>  
> -- 
> 2.25.1

Re: [PATCH 02/14] drm/kmb: Define driver date and major/minor version

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 2148 bytes --]

Hi,

just a friendly reminder that branches that end with -fixes are for 
fixes that are required in upstream ASAP. I found this patch in 
drm-misc-fixes. It's not important, so it should have gone into 
drm-misc-next instead.

Best regards
Thomas

Am 28.07.21 um 02:31 schrieb Anitha Chrisanthus:
> From: Edmund Dea <edmund.j.dea@intel.com>
> 
> Added macros for date and version
> 
> Fixes: 7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display")
> Signed-off-by: Edmund Dea <edmund.j.dea@intel.com>
> ---
>   drivers/gpu/drm/kmb/kmb_drv.c | 8 ++++----
>   drivers/gpu/drm/kmb/kmb_drv.h | 5 +++++
>   2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/kmb/kmb_drv.c b/drivers/gpu/drm/kmb/kmb_drv.c
> index c0b1c6f99249..f54392ec4fab 100644
> --- a/drivers/gpu/drm/kmb/kmb_drv.c
> +++ b/drivers/gpu/drm/kmb/kmb_drv.c
> @@ -425,10 +425,10 @@ static const struct drm_driver kmb_driver = {
>   	.fops = &fops,
>   	DRM_GEM_CMA_DRIVER_OPS_VMAP,
>   	.name = "kmb-drm",
> -	.desc = "KEEMBAY DISPLAY DRIVER ",
> -	.date = "20201008",
> -	.major = 1,
> -	.minor = 0,
> +	.desc = "KEEMBAY DISPLAY DRIVER",
> +	.date = DRIVER_DATE,
> +	.major = DRIVER_MAJOR,
> +	.minor = DRIVER_MINOR,
>   };
>   
>   static int kmb_remove(struct platform_device *pdev)
> diff --git a/drivers/gpu/drm/kmb/kmb_drv.h b/drivers/gpu/drm/kmb/kmb_drv.h
> index 02e806712a64..ebbaa5f422d5 100644
> --- a/drivers/gpu/drm/kmb/kmb_drv.h
> +++ b/drivers/gpu/drm/kmb/kmb_drv.h
> @@ -15,6 +15,11 @@
>   #define KMB_MAX_HEIGHT			1080 /*Max height in pixels */
>   #define KMB_MIN_WIDTH                   1920 /*Max width in pixels */
>   #define KMB_MIN_HEIGHT                  1080 /*Max height in pixels */
> +
> +#define DRIVER_DATE			"20210223"
> +#define DRIVER_MAJOR			1
> +#define DRIVER_MINOR			1
> +
>   #define KMB_LCD_DEFAULT_CLK		200000000
>   #define KMB_SYS_CLK_MHZ			500
>   
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

RE: [PATCH 02/14] drm/kmb: Define driver date and major/minor version

[Chrisanthus, Anitha]

Thanks Thomas, I'll keep this in mind for the next patch.

> -----Original Message-----
> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
> Thomas Zimmermann
> Sent: Wednesday, August 4, 2021 11:13 AM
> To: Chrisanthus, Anitha <anitha.chrisanthus@intel.com>; dri-
> devel@lists.freedesktop.org; Dea, Edmund J <edmund.j.dea@intel.com>
> Subject: Re: [PATCH 02/14] drm/kmb: Define driver date and major/minor
> version
> 
> Hi,
> 
> just a friendly reminder that branches that end with -fixes are for
> fixes that are required in upstream ASAP. I found this patch in
> drm-misc-fixes. It's not important, so it should have gone into
> drm-misc-next instead.
> 
> Best regards
> Thomas
> 
> Am 28.07.21 um 02:31 schrieb Anitha Chrisanthus:
> > From: Edmund Dea <edmund.j.dea@intel.com>
> >
> > Added macros for date and version
> >
> > Fixes: 7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display")
> > Signed-off-by: Edmund Dea <edmund.j.dea@intel.com>
> > ---
> >   drivers/gpu/drm/kmb/kmb_drv.c | 8 ++++----
> >   drivers/gpu/drm/kmb/kmb_drv.h | 5 +++++
> >   2 files changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/kmb/kmb_drv.c
> b/drivers/gpu/drm/kmb/kmb_drv.c
> > index c0b1c6f99249..f54392ec4fab 100644
> > --- a/drivers/gpu/drm/kmb/kmb_drv.c
> > +++ b/drivers/gpu/drm/kmb/kmb_drv.c
> > @@ -425,10 +425,10 @@ static const struct drm_driver kmb_driver = {
> >   	.fops = &fops,
> >   	DRM_GEM_CMA_DRIVER_OPS_VMAP,
> >   	.name = "kmb-drm",
> > -	.desc = "KEEMBAY DISPLAY DRIVER ",
> > -	.date = "20201008",
> > -	.major = 1,
> > -	.minor = 0,
> > +	.desc = "KEEMBAY DISPLAY DRIVER",
> > +	.date = DRIVER_DATE,
> > +	.major = DRIVER_MAJOR,
> > +	.minor = DRIVER_MINOR,
> >   };
> >
> >   static int kmb_remove(struct platform_device *pdev)
> > diff --git a/drivers/gpu/drm/kmb/kmb_drv.h
> b/drivers/gpu/drm/kmb/kmb_drv.h
> > index 02e806712a64..ebbaa5f422d5 100644
> > --- a/drivers/gpu/drm/kmb/kmb_drv.h
> > +++ b/drivers/gpu/drm/kmb/kmb_drv.h
> > @@ -15,6 +15,11 @@
> >   #define KMB_MAX_HEIGHT			1080 /*Max height in pixels */
> >   #define KMB_MIN_WIDTH                   1920 /*Max width in pixels */
> >   #define KMB_MIN_HEIGHT                  1080 /*Max height in pixels */
> > +
> > +#define DRIVER_DATE			"20210223"
> > +#define DRIVER_MAJOR			1
> > +#define DRIVER_MINOR			1
> > +
> >   #define KMB_LCD_DEFAULT_CLK		200000000
> >   #define KMB_SYS_CLK_MHZ			500
> >
> >
> 
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer

[PATCH 03/14] drm/kmb: Work around for higher system clock

[Anitha Chrisanthus]

Use a different value for system clock offset in the
ppl/llp ratio calculations for clocks higher than 500 Mhz.

Fixes: 98521f4d4b4c ("drm/kmb: Mipi DSI part of the display driver")
Signed-off-by: Anitha Chrisanthus <anitha.chrisanthus@intel.com>
---
 drivers/gpu/drm/kmb/kmb_dsi.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/kmb/kmb_dsi.c b/drivers/gpu/drm/kmb/kmb_dsi.c
index 231041b269f5..7e2371ffcb18 100644
--- a/drivers/gpu/drm/kmb/kmb_dsi.c
+++ b/drivers/gpu/drm/kmb/kmb_dsi.c
@@ -482,6 +482,10 @@ static u32 mipi_tx_fg_section_cfg(struct kmb_dsi *kmb_dsi,
 	return 0;
 }
 
+#define CLK_DIFF_LOW 50
+#define CLK_DIFF_HI 60
+#define SYSCLK_500  500
+
 static void mipi_tx_fg_cfg_regs(struct kmb_dsi *kmb_dsi, u8 frame_gen,
 				struct mipi_tx_frame_timing_cfg *fg_cfg)
 {
@@ -492,7 +496,12 @@ static void mipi_tx_fg_cfg_regs(struct kmb_dsi *kmb_dsi, u8 frame_gen,
 	/* 500 Mhz system clock minus 50 to account for the difference in
 	 * MIPI clock speed in RTL tests
 	 */
-	sysclk = kmb_dsi->sys_clk_mhz - 50;
+	if (kmb_dsi->sys_clk_mhz == SYSCLK_500) {
+		sysclk = kmb_dsi->sys_clk_mhz - CLK_DIFF_LOW;
+	} else {
+		/* 700 Mhz clk*/
+		sysclk = kmb_dsi->sys_clk_mhz - CLK_DIFF_HI;
+	}
 
 	/* PPL-Pixel Packing Layer, LLP-Low Level Protocol
 	 * Frame genartor timing parameters are clocked on the system clock,
-- 
2.25.1

[PATCH 04/14] drm/kmb : W/A for 256B cache alignment for video

[Anitha Chrisanthus]

For B0 silicon, the media driver pads the decoded video dmabufs for 256B
alignment. This is the backing buffer of the framebuffer and info in the
drm frame buffer is not correct for these buffers as this is done
internally in the media driver. This change extracts the meta data info
from dmabuf priv structure and uses that info for programming stride and
offsets in kmb_plane_atomic_update().

Fixes: 7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display")
Signed-off-by: Edmund Dea <edmund.j.dea@intel.com>
Signed-off-by: Anitha Chrisanthus <anitha.chrisanthus@intel.com>
---
 drivers/gpu/drm/kmb/kmb_drv.h    |  1 +
 drivers/gpu/drm/kmb/kmb_plane.c  | 38 ++++++++++++++++++++---
 drivers/gpu/drm/kmb/kmb_vidmem.h | 52 ++++++++++++++++++++++++++++++++
 3 files changed, 87 insertions(+), 4 deletions(-)
 create mode 100644 drivers/gpu/drm/kmb/kmb_vidmem.h

diff --git a/drivers/gpu/drm/kmb/kmb_drv.h b/drivers/gpu/drm/kmb/kmb_drv.h
index ebbaa5f422d5..0904e6eb2a09 100644
--- a/drivers/gpu/drm/kmb/kmb_drv.h
+++ b/drivers/gpu/drm/kmb/kmb_drv.h
@@ -49,6 +49,7 @@ struct kmb_drm_private {
 	int				kmb_under_flow;
 	int				kmb_flush_done;
 	int				layer_no;
+	struct viv_vidmem_metadata	*md_info;
 };
 
 static inline struct kmb_drm_private *to_kmb(const struct drm_device *dev)
diff --git a/drivers/gpu/drm/kmb/kmb_plane.c b/drivers/gpu/drm/kmb/kmb_plane.c
index 2888dd5dcc2c..e45419d6ed96 100644
--- a/drivers/gpu/drm/kmb/kmb_plane.c
+++ b/drivers/gpu/drm/kmb/kmb_plane.c
@@ -11,12 +11,16 @@
 #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_managed.h>
 #include <drm/drm_plane_helper.h>
 
+#include <linux/dma-buf.h>
+
 #include "kmb_drv.h"
 #include "kmb_plane.h"
 #include "kmb_regs.h"
+#include "kmb_vidmem.h"
 
 const u32 layer_irqs[] = {
 	LCD_INT_VL0,
@@ -294,8 +298,10 @@ static void kmb_plane_atomic_update(struct drm_plane *plane,
 	unsigned int ctrl = 0, val = 0, out_format = 0;
 	unsigned int src_w, src_h, crtc_x, crtc_y;
 	unsigned char plane_id;
-	int num_planes;
+	int num_planes, i;
 	static dma_addr_t addr[MAX_SUB_PLANES];
+	struct viv_vidmem_metadata *md = NULL;
+	struct drm_gem_object *gem_obj;
 
 	if (!plane || !new_plane_state || !old_plane_state)
 		return;
@@ -325,6 +331,16 @@ static void kmb_plane_atomic_update(struct drm_plane *plane,
 	drm_dbg(&kmb->drm,
 		"src_w=%d src_h=%d, fb->format->format=0x%x fb->flags=0x%x\n",
 		  src_w, src_h, fb->format->format, fb->flags);
+	gem_obj = drm_gem_fb_get_obj(fb, plane_id);
+	if (gem_obj && gem_obj->import_attach &&
+	    gem_obj->import_attach->dmabuf &&
+	    gem_obj->import_attach->dmabuf->priv) {
+		md = gem_obj->import_attach->dmabuf->priv;
+
+		/* Check if metadata is coming from hantro driver */
+		if (md->magic != HANTRO_IMAGE_VIV_META_DATA_MAGIC)
+			md = NULL;
+	}
 
 	width = fb->width;
 	height = fb->height;
@@ -332,6 +348,11 @@ static void kmb_plane_atomic_update(struct drm_plane *plane,
 	drm_dbg(&kmb->drm, "dma_len=%d ", dma_len);
 	kmb_write_lcd(kmb, LCD_LAYERn_DMA_LEN(plane_id), dma_len);
 	kmb_write_lcd(kmb, LCD_LAYERn_DMA_LEN_SHADOW(plane_id), dma_len);
+	if (md) {
+		for (i = 0; i < 3; i++)
+			fb->pitches[i] = md->plane[i].stride;
+	}
+
 	kmb_write_lcd(kmb, LCD_LAYERn_DMA_LINE_VSTRIDE(plane_id),
 		      fb->pitches[0]);
 	kmb_write_lcd(kmb, LCD_LAYERn_DMA_LINE_WIDTH(plane_id),
@@ -339,18 +360,22 @@ static void kmb_plane_atomic_update(struct drm_plane *plane,
 
 	addr[Y_PLANE] = drm_fb_cma_get_gem_addr(fb, new_plane_state, 0);
 	kmb_write_lcd(kmb, LCD_LAYERn_DMA_START_ADDR(plane_id),
-		      addr[Y_PLANE] + fb->offsets[0]);
+		      addr[Y_PLANE]);
 	val = get_pixel_format(fb->format->format);
 	val |= get_bits_per_pixel(fb->format);
 	/* Program Cb/Cr for planar formats */
 	if (num_planes > 1) {
 		kmb_write_lcd(kmb, LCD_LAYERn_DMA_CB_LINE_VSTRIDE(plane_id),
-			      width * fb->format->cpp[0]);
+				fb->pitches[1]);
 		kmb_write_lcd(kmb, LCD_LAYERn_DMA_CB_LINE_WIDTH(plane_id),
 			      (width * fb->format->cpp[0]));
 
 		addr[U_PLANE] = drm_fb_cma_get_gem_addr(fb, new_plane_state,
 							U_PLANE);
+		if (md) {
+			addr[U_PLANE] += md->plane[1].offset -
+					 (addr[U_PLANE] - addr[Y_PLANE]);
+		}
 		/* check if Cb/Cr is swapped*/
 		if (num_planes == 3 && (val & LCD_LAYER_CRCB_ORDER))
 			kmb_write_lcd(kmb,
@@ -364,7 +389,7 @@ static void kmb_plane_atomic_update(struct drm_plane *plane,
 		if (num_planes == 3) {
 			kmb_write_lcd(kmb,
 				      LCD_LAYERn_DMA_CR_LINE_VSTRIDE(plane_id),
-				      ((width) * fb->format->cpp[0]));
+				      fb->pitches[2]);
 
 			kmb_write_lcd(kmb,
 				      LCD_LAYERn_DMA_CR_LINE_WIDTH(plane_id),
@@ -373,6 +398,11 @@ static void kmb_plane_atomic_update(struct drm_plane *plane,
 			addr[V_PLANE] = drm_fb_cma_get_gem_addr(fb,
 								new_plane_state,
 								V_PLANE);
+			if (md) {
+				addr[V_PLANE] +=
+					md->plane[2].offset -
+					(addr[V_PLANE] - addr[Y_PLANE]);
+			}
 
 			/* check if Cb/Cr is swapped*/
 			if (val & LCD_LAYER_CRCB_ORDER)
diff --git a/drivers/gpu/drm/kmb/kmb_vidmem.h b/drivers/gpu/drm/kmb/kmb_vidmem.h
new file mode 100644
index 000000000000..06198d413f50
--- /dev/null
+++ b/drivers/gpu/drm/kmb/kmb_vidmem.h
@@ -0,0 +1,52 @@
+/* SPDX-License-Identifier: GPL-2.0-only
+ *
+ * Copyright © 2018-2020 Intel Corporation
+ */
+
+#ifndef __KMB_VIDMEM_H__
+#define __KMB_VIDMEM_H__
+
+#define HANTRO_MAGIC(ch0, ch1, ch2, ch3) \
+	    ((unsigned long)(unsigned char)(ch0) | \
+	    ((unsigned long)(unsigned char)(ch1) << 8) | \
+	    ((unsigned long)(unsigned char)(ch2) << 16) | \
+	    ((unsigned long)(unsigned char)(ch3) << 24))
+
+#define HANTRO_IMAGE_VIV_META_DATA_MAGIC HANTRO_MAGIC('V', 'I', 'V', 'M')
+
+struct viv_vidmem_metadata {
+	u32 magic;        // __FOURCC('v', 'i', 'v', 'm')
+	u32 dmabuf_size;  // DMABUF buffer size in byte (Maximum 4GB)
+	u32 time_stamp;   // time stamp for the DMABUF buffer
+
+	u32 image_format; // ImageFormat, determined plane number.
+	u32 compressed;   // if DMABUF buffer is compressed by DEC400
+	struct {
+	u32 offset; // plane buffer address offset from DMABUF address
+	u32 stride; // pitch in bytes
+	u32 width;  // width in pixels
+	u32 height; // height in pixels
+
+	u32 tile_format; // uncompressed tile format
+	u32 compress_format; // tile mode for DEC400
+
+	/** tile status buffer offset within this plane buffer. when it is 0，
+	 *  indicates using separate tile status buffer
+	 */
+	u32 ts_offset;
+	/** fd of separate tile status buffer of the plane buffer */
+	u32 ts_fd;
+	/** valid fd of the ts buffer in consumer side. */
+	u32 ts_fd2;
+	/** the vpu virtual address for this ts data buffer */
+	u32 ts_vaddr;
+
+	/** gpu fastclear enabled for the plane buffer */
+	u32 fc_enabled;
+	/** gpu fastclear color value (lower 32 bits) for the plane buffer */
+	u32 fc_value_lower;
+	/** gpu fastclear color value (upper 32 bits) for the plane buffer */
+	u32 fc_value_upper;
+	} plane[3];
+};
+#endif /*__KMB_VIDMEM_H__*/
-- 
2.25.1

Re: [PATCH 04/14] drm/kmb : W/A for 256B cache alignment for video

[Sam Ravnborg]

Hi Anitha,
On Tue, Jul 27, 2021 at 05:31:16PM -0700, Anitha Chrisanthus wrote:
> For B0 silicon, the media driver pads the decoded video dmabufs for 256B
> alignment. This is the backing buffer of the framebuffer and info in the
> drm frame buffer is not correct for these buffers as this is done
> internally in the media driver. This change extracts the meta data info
> from dmabuf priv structure and uses that info for programming stride and
> offsets in kmb_plane_atomic_update().
> 
> Fixes: 7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display")
> Signed-off-by: Edmund Dea <edmund.j.dea@intel.com>
> Signed-off-by: Anitha Chrisanthus <anitha.chrisanthus@intel.com>

Drop extra space in subject before ':'


> ---
>  drivers/gpu/drm/kmb/kmb_drv.h    |  1 +
>  drivers/gpu/drm/kmb/kmb_plane.c  | 38 ++++++++++++++++++++---
>  drivers/gpu/drm/kmb/kmb_vidmem.h | 52 ++++++++++++++++++++++++++++++++
>  3 files changed, 87 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/gpu/drm/kmb/kmb_vidmem.h
> 
> diff --git a/drivers/gpu/drm/kmb/kmb_drv.h b/drivers/gpu/drm/kmb/kmb_drv.h
> index ebbaa5f422d5..0904e6eb2a09 100644
> --- a/drivers/gpu/drm/kmb/kmb_drv.h
> +++ b/drivers/gpu/drm/kmb/kmb_drv.h
> @@ -49,6 +49,7 @@ struct kmb_drm_private {
>  	int				kmb_under_flow;
>  	int				kmb_flush_done;
>  	int				layer_no;
> +	struct viv_vidmem_metadata	*md_info;
I cannot see this member used in this patch - can it be dropped?

>  };
>  
>  static inline struct kmb_drm_private *to_kmb(const struct drm_device *dev)
> diff --git a/drivers/gpu/drm/kmb/kmb_plane.c b/drivers/gpu/drm/kmb/kmb_plane.c
> index 2888dd5dcc2c..e45419d6ed96 100644
> --- a/drivers/gpu/drm/kmb/kmb_plane.c
> +++ b/drivers/gpu/drm/kmb/kmb_plane.c
> @@ -11,12 +11,16 @@
>  #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_managed.h>
>  #include <drm/drm_plane_helper.h>
>  
> +#include <linux/dma-buf.h>
> +
>  #include "kmb_drv.h"
>  #include "kmb_plane.h"
>  #include "kmb_regs.h"
> +#include "kmb_vidmem.h"
>  
>  const u32 layer_irqs[] = {
>  	LCD_INT_VL0,
> @@ -294,8 +298,10 @@ static void kmb_plane_atomic_update(struct drm_plane *plane,
>  	unsigned int ctrl = 0, val = 0, out_format = 0;
>  	unsigned int src_w, src_h, crtc_x, crtc_y;
>  	unsigned char plane_id;
> -	int num_planes;
> +	int num_planes, i;
>  	static dma_addr_t addr[MAX_SUB_PLANES];
> +	struct viv_vidmem_metadata *md = NULL;
> +	struct drm_gem_object *gem_obj;
>  
>  	if (!plane || !new_plane_state || !old_plane_state)
>  		return;
> @@ -325,6 +331,16 @@ static void kmb_plane_atomic_update(struct drm_plane *plane,
>  	drm_dbg(&kmb->drm,
>  		"src_w=%d src_h=%d, fb->format->format=0x%x fb->flags=0x%x\n",
>  		  src_w, src_h, fb->format->format, fb->flags);
> +	gem_obj = drm_gem_fb_get_obj(fb, plane_id);
> +	if (gem_obj && gem_obj->import_attach &&
> +	    gem_obj->import_attach->dmabuf &&
> +	    gem_obj->import_attach->dmabuf->priv) {
> +		md = gem_obj->import_attach->dmabuf->priv;
> +
> +		/* Check if metadata is coming from hantro driver */
> +		if (md->magic != HANTRO_IMAGE_VIV_META_DATA_MAGIC)
> +			md = NULL;
> +	}
>  
>  	width = fb->width;
>  	height = fb->height;
> @@ -332,6 +348,11 @@ static void kmb_plane_atomic_update(struct drm_plane *plane,
>  	drm_dbg(&kmb->drm, "dma_len=%d ", dma_len);
>  	kmb_write_lcd(kmb, LCD_LAYERn_DMA_LEN(plane_id), dma_len);
>  	kmb_write_lcd(kmb, LCD_LAYERn_DMA_LEN_SHADOW(plane_id), dma_len);
> +	if (md) {
> +		for (i = 0; i < 3; i++)
> +			fb->pitches[i] = md->plane[i].stride;
> +	}
> +
>  	kmb_write_lcd(kmb, LCD_LAYERn_DMA_LINE_VSTRIDE(plane_id),
>  		      fb->pitches[0]);
>  	kmb_write_lcd(kmb, LCD_LAYERn_DMA_LINE_WIDTH(plane_id),
> @@ -339,18 +360,22 @@ static void kmb_plane_atomic_update(struct drm_plane *plane,
>  
>  	addr[Y_PLANE] = drm_fb_cma_get_gem_addr(fb, new_plane_state, 0);
>  	kmb_write_lcd(kmb, LCD_LAYERn_DMA_START_ADDR(plane_id),
> -		      addr[Y_PLANE] + fb->offsets[0]);
> +		      addr[Y_PLANE]);
>  	val = get_pixel_format(fb->format->format);
>  	val |= get_bits_per_pixel(fb->format);
>  	/* Program Cb/Cr for planar formats */
>  	if (num_planes > 1) {
>  		kmb_write_lcd(kmb, LCD_LAYERn_DMA_CB_LINE_VSTRIDE(plane_id),
> -			      width * fb->format->cpp[0]);
> +				fb->pitches[1]);
>  		kmb_write_lcd(kmb, LCD_LAYERn_DMA_CB_LINE_WIDTH(plane_id),
>  			      (width * fb->format->cpp[0]));
>  
>  		addr[U_PLANE] = drm_fb_cma_get_gem_addr(fb, new_plane_state,
>  							U_PLANE);
> +		if (md) {
> +			addr[U_PLANE] += md->plane[1].offset -
> +					 (addr[U_PLANE] - addr[Y_PLANE]);
> +		}

I failed to follow why:
1) offsets is no logner needed
2) If pitches is always set or only set with a hantro buffer
3) Why addr[U_PLANE] is assigned twice in the md != NULL case

>  		/* check if Cb/Cr is swapped*/
>  		if (num_planes == 3 && (val & LCD_LAYER_CRCB_ORDER))
>  			kmb_write_lcd(kmb,
> @@ -364,7 +389,7 @@ static void kmb_plane_atomic_update(struct drm_plane *plane,
>  		if (num_planes == 3) {
>  			kmb_write_lcd(kmb,
>  				      LCD_LAYERn_DMA_CR_LINE_VSTRIDE(plane_id),
> -				      ((width) * fb->format->cpp[0]));
> +				      fb->pitches[2]);
>  
>  			kmb_write_lcd(kmb,
>  				      LCD_LAYERn_DMA_CR_LINE_WIDTH(plane_id),
> @@ -373,6 +398,11 @@ static void kmb_plane_atomic_update(struct drm_plane *plane,
>  			addr[V_PLANE] = drm_fb_cma_get_gem_addr(fb,
>  								new_plane_state,
>  								V_PLANE);
> +			if (md) {
> +				addr[V_PLANE] +=
> +					md->plane[2].offset -
> +					(addr[V_PLANE] - addr[Y_PLANE]);
> +			}
Likewise - is pitches always valid and why assing addr[V_PLANE] twice?

>  
>  			/* check if Cb/Cr is swapped*/
>  			if (val & LCD_LAYER_CRCB_ORDER)
> diff --git a/drivers/gpu/drm/kmb/kmb_vidmem.h b/drivers/gpu/drm/kmb/kmb_vidmem.h
> new file mode 100644
> index 000000000000..06198d413f50
> --- /dev/null
> +++ b/drivers/gpu/drm/kmb/kmb_vidmem.h
> @@ -0,0 +1,52 @@
> +/* SPDX-License-Identifier: GPL-2.0-only
> + *
> + * Copyright © 2018-2020 Intel Corporation
> + */
> +
> +#ifndef __KMB_VIDMEM_H__
> +#define __KMB_VIDMEM_H__
> +
> +#define HANTRO_MAGIC(ch0, ch1, ch2, ch3) \
> +	    ((unsigned long)(unsigned char)(ch0) | \
> +	    ((unsigned long)(unsigned char)(ch1) << 8) | \
> +	    ((unsigned long)(unsigned char)(ch2) << 16) | \
> +	    ((unsigned long)(unsigned char)(ch3) << 24))
...

This header looks like it belongs outside the drm driver - I assume the
hantro driver needs this?
Or is this some uapi stuff?

	Sam

RE: [PATCH 04/14] drm/kmb : W/A for 256B cache alignment for video

[Chrisanthus, Anitha]

Hi Sam,
Thanks for you review.

> -----Original Message-----
> From: Sam Ravnborg <sam@ravnborg.org>
> Sent: Wednesday, July 28, 2021 12:16 AM
> To: Chrisanthus, Anitha <anitha.chrisanthus@intel.com>
> Cc: dri-devel@lists.freedesktop.org; Dea, Edmund J
> <edmund.j.dea@intel.com>
> Subject: Re: [PATCH 04/14] drm/kmb : W/A for 256B cache alignment for video
> 
> Hi Anitha,
> On Tue, Jul 27, 2021 at 05:31:16PM -0700, Anitha Chrisanthus wrote:
> > For B0 silicon, the media driver pads the decoded video dmabufs for 256B
> > alignment. This is the backing buffer of the framebuffer and info in the
> > drm frame buffer is not correct for these buffers as this is done
> > internally in the media driver. This change extracts the meta data info
> > from dmabuf priv structure and uses that info for programming stride and
> > offsets in kmb_plane_atomic_update().
> >
> > Fixes: 7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display")
> > Signed-off-by: Edmund Dea <edmund.j.dea@intel.com>
> > Signed-off-by: Anitha Chrisanthus <anitha.chrisanthus@intel.com>
> 
> Drop extra space in subject before ':'
ok
> 
> 
> > ---
> >  drivers/gpu/drm/kmb/kmb_drv.h    |  1 +
> >  drivers/gpu/drm/kmb/kmb_plane.c  | 38 ++++++++++++++++++++---
> >  drivers/gpu/drm/kmb/kmb_vidmem.h | 52
> ++++++++++++++++++++++++++++++++
> >  3 files changed, 87 insertions(+), 4 deletions(-)
> >  create mode 100644 drivers/gpu/drm/kmb/kmb_vidmem.h
> >
> > diff --git a/drivers/gpu/drm/kmb/kmb_drv.h
> b/drivers/gpu/drm/kmb/kmb_drv.h
> > index ebbaa5f422d5..0904e6eb2a09 100644
> > --- a/drivers/gpu/drm/kmb/kmb_drv.h
> > +++ b/drivers/gpu/drm/kmb/kmb_drv.h
> > @@ -49,6 +49,7 @@ struct kmb_drm_private {
> >  	int				kmb_under_flow;
> >  	int				kmb_flush_done;
> >  	int				layer_no;
> > +	struct viv_vidmem_metadata	*md_info;
> I cannot see this member used in this patch - can it be dropped?
Good catch, yes will remove it.
> 
> >  };
> >
> >  static inline struct kmb_drm_private *to_kmb(const struct drm_device *dev)
> > diff --git a/drivers/gpu/drm/kmb/kmb_plane.c
> b/drivers/gpu/drm/kmb/kmb_plane.c
> > index 2888dd5dcc2c..e45419d6ed96 100644
> > --- a/drivers/gpu/drm/kmb/kmb_plane.c
> > +++ b/drivers/gpu/drm/kmb/kmb_plane.c
> > @@ -11,12 +11,16 @@
> >  #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_managed.h>
> >  #include <drm/drm_plane_helper.h>
> >
> > +#include <linux/dma-buf.h>
> > +
> >  #include "kmb_drv.h"
> >  #include "kmb_plane.h"
> >  #include "kmb_regs.h"
> > +#include "kmb_vidmem.h"
> >
> >  const u32 layer_irqs[] = {
> >  	LCD_INT_VL0,
> > @@ -294,8 +298,10 @@ static void kmb_plane_atomic_update(struct
> drm_plane *plane,
> >  	unsigned int ctrl = 0, val = 0, out_format = 0;
> >  	unsigned int src_w, src_h, crtc_x, crtc_y;
> >  	unsigned char plane_id;
> > -	int num_planes;
> > +	int num_planes, i;
> >  	static dma_addr_t addr[MAX_SUB_PLANES];
> > +	struct viv_vidmem_metadata *md = NULL;
> > +	struct drm_gem_object *gem_obj;
> >
> >  	if (!plane || !new_plane_state || !old_plane_state)
> >  		return;
> > @@ -325,6 +331,16 @@ static void kmb_plane_atomic_update(struct
> drm_plane *plane,
> >  	drm_dbg(&kmb->drm,
> >  		"src_w=%d src_h=%d, fb->format->format=0x%x fb-
> >flags=0x%x\n",
> >  		  src_w, src_h, fb->format->format, fb->flags);
> > +	gem_obj = drm_gem_fb_get_obj(fb, plane_id);
> > +	if (gem_obj && gem_obj->import_attach &&
> > +	    gem_obj->import_attach->dmabuf &&
> > +	    gem_obj->import_attach->dmabuf->priv) {
> > +		md = gem_obj->import_attach->dmabuf->priv;
> > +
> > +		/* Check if metadata is coming from hantro driver */
> > +		if (md->magic != HANTRO_IMAGE_VIV_META_DATA_MAGIC)
> > +			md = NULL;
> > +	}
> >
> >  	width = fb->width;
> >  	height = fb->height;
> > @@ -332,6 +348,11 @@ static void kmb_plane_atomic_update(struct
> drm_plane *plane,
> >  	drm_dbg(&kmb->drm, "dma_len=%d ", dma_len);
> >  	kmb_write_lcd(kmb, LCD_LAYERn_DMA_LEN(plane_id), dma_len);
> >  	kmb_write_lcd(kmb, LCD_LAYERn_DMA_LEN_SHADOW(plane_id),
> dma_len);
> > +	if (md) {
> > +		for (i = 0; i < 3; i++)
> > +			fb->pitches[i] = md->plane[i].stride;
> > +	}
> > +
> >  	kmb_write_lcd(kmb, LCD_LAYERn_DMA_LINE_VSTRIDE(plane_id),
> >  		      fb->pitches[0]);
> >  	kmb_write_lcd(kmb, LCD_LAYERn_DMA_LINE_WIDTH(plane_id),
> > @@ -339,18 +360,22 @@ static void kmb_plane_atomic_update(struct
> drm_plane *plane,
> >
> >  	addr[Y_PLANE] = drm_fb_cma_get_gem_addr(fb, new_plane_state, 0);
> >  	kmb_write_lcd(kmb, LCD_LAYERn_DMA_START_ADDR(plane_id),
> > -		      addr[Y_PLANE] + fb->offsets[0]);
> > +		      addr[Y_PLANE]);
> >  	val = get_pixel_format(fb->format->format);
> >  	val |= get_bits_per_pixel(fb->format);
> >  	/* Program Cb/Cr for planar formats */
> >  	if (num_planes > 1) {
> >  		kmb_write_lcd(kmb,
> LCD_LAYERn_DMA_CB_LINE_VSTRIDE(plane_id),
> > -			      width * fb->format->cpp[0]);
> > +				fb->pitches[1]);
> >  		kmb_write_lcd(kmb,
> LCD_LAYERn_DMA_CB_LINE_WIDTH(plane_id),
> >  			      (width * fb->format->cpp[0]));
> >
> >  		addr[U_PLANE] = drm_fb_cma_get_gem_addr(fb,
> new_plane_state,
> >  							U_PLANE);
> > +		if (md) {
> > +			addr[U_PLANE] += md->plane[1].offset -
> > +					 (addr[U_PLANE] - addr[Y_PLANE]);
> > +		}
> 
> I failed to follow why:
> 1) offsets is no logner needed
That's a mistake, will put back the offsets
> 2) If pitches is always set or only set with a hantro buffer
Pitches is set by hantro driver to a different value, I'm going to send v2 which combines this with patch 09 which reverts some of the above changes.
> 3) Why addr[U_PLANE] is assigned twice in the md != NULL case
I agree this is confusing, will make send v2 with simplified and separate calculations for addr[U and V plane} for md==NULL and !=NULL case
> 
> >  		/* check if Cb/Cr is swapped*/
> >  		if (num_planes == 3 && (val & LCD_LAYER_CRCB_ORDER))
> >  			kmb_write_lcd(kmb,
> > @@ -364,7 +389,7 @@ static void kmb_plane_atomic_update(struct
> drm_plane *plane,
> >  		if (num_planes == 3) {
> >  			kmb_write_lcd(kmb,
> >
> LCD_LAYERn_DMA_CR_LINE_VSTRIDE(plane_id),
> > -				      ((width) * fb->format->cpp[0]));
> > +				      fb->pitches[2]);
> >
> >  			kmb_write_lcd(kmb,
> >
> LCD_LAYERn_DMA_CR_LINE_WIDTH(plane_id),
> > @@ -373,6 +398,11 @@ static void kmb_plane_atomic_update(struct
> drm_plane *plane,
> >  			addr[V_PLANE] = drm_fb_cma_get_gem_addr(fb,
> >
> 	new_plane_state,
> >  								V_PLANE);
> > +			if (md) {
> > +				addr[V_PLANE] +=
> > +					md->plane[2].offset -
> > +					(addr[V_PLANE] - addr[Y_PLANE]);
> > +			}
> Likewise - is pitches always valid and why assing addr[V_PLANE] twice?
Answered above, will send V2
> 
> >
> >  			/* check if Cb/Cr is swapped*/
> >  			if (val & LCD_LAYER_CRCB_ORDER)
> > diff --git a/drivers/gpu/drm/kmb/kmb_vidmem.h
> b/drivers/gpu/drm/kmb/kmb_vidmem.h
> > new file mode 100644
> > index 000000000000..06198d413f50
> > --- /dev/null
> > +++ b/drivers/gpu/drm/kmb/kmb_vidmem.h
> > @@ -0,0 +1,52 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only
> > + *
> > + * Copyright (c) 2018-2020 Intel Corporation
> > + */
> > +
> > +#ifndef __KMB_VIDMEM_H__
> > +#define __KMB_VIDMEM_H__
> > +
> > +#define HANTRO_MAGIC(ch0, ch1, ch2, ch3) \
> > +	    ((unsigned long)(unsigned char)(ch0) | \
> > +	    ((unsigned long)(unsigned char)(ch1) << 8) | \
> > +	    ((unsigned long)(unsigned char)(ch2) << 16) | \
> > +	    ((unsigned long)(unsigned char)(ch3) << 24))
> ...
> 
> This header looks like it belongs outside the drm driver - I assume the
> hantro driver needs this?
This is from hantro driver and I agree it does not belong here, but we need this struct to extract the meta data info. Hantro driver is not upstreamed yet, how should I tackle this?
> Or is this some uapi stuff?
> 
> 	Sam

[PATCH 05/14] drm/kmb: Limit supported mode to 1080p

[Anitha Chrisanthus]

KMB only supports single resolution(1080p), this commit checks for
1920x1080x60 or 1920x1080x59 in crtc_mode_valid.

Fixes: 7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display")
Signed-off-by: Anitha Chrisanthus <anitha.chrisanthus@intel.com>
---
 drivers/gpu/drm/kmb/kmb_crtc.c | 28 ++++++++++++++++++++++++++++
 drivers/gpu/drm/kmb/kmb_drv.h  | 15 +++++++++++++--
 2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/kmb/kmb_crtc.c b/drivers/gpu/drm/kmb/kmb_crtc.c
index 44327bc629ca..44626044c85e 100644
--- a/drivers/gpu/drm/kmb/kmb_crtc.c
+++ b/drivers/gpu/drm/kmb/kmb_crtc.c
@@ -185,11 +185,39 @@ static void kmb_crtc_atomic_flush(struct drm_crtc *crtc,
 	spin_unlock_irq(&crtc->dev->event_lock);
 }
 
+static enum drm_mode_status
+		kmb_crtc_mode_valid(struct drm_crtc *crtc,
+				    const struct drm_display_mode *mode)
+{
+	int refresh;
+	struct drm_device *dev = crtc->dev;
+
+	if (mode->vdisplay < KMB_CRTC_MAX_HEIGHT) {
+		drm_dbg(dev, "height = %d less than %d",
+			mode->vdisplay, KMB_CRTC_MAX_HEIGHT);
+		return MODE_BAD_VVALUE;
+	}
+	if (mode->hdisplay < KMB_CRTC_MAX_WIDTH) {
+		drm_dbg(dev, "width = %d less than %d",
+			mode->hdisplay, KMB_CRTC_MAX_WIDTH);
+		return MODE_BAD_HVALUE;
+	}
+	refresh = drm_mode_vrefresh(mode);
+	if (refresh < KMB_MIN_VREFRESH || refresh > KMB_MAX_VREFRESH) {
+		drm_dbg(dev, "refresh = %d less than %d or greater than %d",
+			refresh, KMB_MIN_VREFRESH, KMB_MAX_VREFRESH);
+		return MODE_BAD;
+	}
+
+	return MODE_OK;
+}
+
 static const struct drm_crtc_helper_funcs kmb_crtc_helper_funcs = {
 	.atomic_begin = kmb_crtc_atomic_begin,
 	.atomic_enable = kmb_crtc_atomic_enable,
 	.atomic_disable = kmb_crtc_atomic_disable,
 	.atomic_flush = kmb_crtc_atomic_flush,
+	.mode_valid = kmb_crtc_mode_valid,
 };
 
 int kmb_setup_crtc(struct drm_device *drm)
diff --git a/drivers/gpu/drm/kmb/kmb_drv.h b/drivers/gpu/drm/kmb/kmb_drv.h
index 0904e6eb2a09..fefb1052058c 100644
--- a/drivers/gpu/drm/kmb/kmb_drv.h
+++ b/drivers/gpu/drm/kmb/kmb_drv.h
@@ -18,8 +18,19 @@
 
 #define DRIVER_DATE			"20210223"
 #define DRIVER_MAJOR			1
-#define DRIVER_MINOR			1
-
+#define DRIVER_MINOR			2
+
+/* Platform definitions */
+#define KMB_CRTC_MAX_WIDTH		1920 /* max width in pixels */
+#define KMB_CRTC_MAX_HEIGHT		1080 /* max height in pixels */
+#define KMB_CRTC_MIN_WIDTH		1920
+#define KMB_CRTC_MIN_HEIGHT		1080
+#define KMB_FB_MAX_WIDTH		1920
+#define KMB_FB_MAX_HEIGHT		1080
+#define KMB_FB_MIN_WIDTH		1
+#define KMB_FB_MIN_HEIGHT		1
+#define KMB_MIN_VREFRESH		59    /*vertical refresh in Hz */
+#define KMB_MAX_VREFRESH		60    /*vertical refresh in Hz */
 #define KMB_LCD_DEFAULT_CLK		200000000
 #define KMB_SYS_CLK_MHZ			500
 
-- 
2.25.1

[PATCH 06/14] drm/kmb: Remove clearing DPHY regs

[Anitha Chrisanthus]

From: Edmund Dea <edmund.j.dea@intel.com>

Don't clear the shared DPHY registers common to MIPI Rx and MIPI Tx during
DSI initialization since this was causing MIPI Rx reset. Rest of the
writes are bitwise, so do not affect Mipi Rx side.

Fixes: 98521f4d4b4c ("drm/kmb: Mipi DSI part of the display driver")
Signed-off-by: Anitha Chrisanthus <anitha.chrisanthus@intel.com>
Signed-off-by: Edmund Dea <edmund.j.dea@intel.com>
---
 drivers/gpu/drm/kmb/kmb_dsi.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/kmb/kmb_dsi.c b/drivers/gpu/drm/kmb/kmb_dsi.c
index 7e2371ffcb18..5bc6c84073a3 100644
--- a/drivers/gpu/drm/kmb/kmb_dsi.c
+++ b/drivers/gpu/drm/kmb/kmb_dsi.c
@@ -1393,11 +1393,6 @@ int kmb_dsi_mode_set(struct kmb_dsi *kmb_dsi, struct drm_display_mode *mode,
 		mipi_tx_init_cfg.lane_rate_mbps = data_rate;
 	}
 
-	kmb_write_mipi(kmb_dsi, DPHY_ENABLE, 0);
-	kmb_write_mipi(kmb_dsi, DPHY_INIT_CTRL0, 0);
-	kmb_write_mipi(kmb_dsi, DPHY_INIT_CTRL1, 0);
-	kmb_write_mipi(kmb_dsi, DPHY_INIT_CTRL2, 0);
-
 	/* Initialize mipi controller */
 	mipi_tx_init_cntrl(kmb_dsi, &mipi_tx_init_cfg);
 
-- 
2.25.1

[PATCH 07/14] drm/kmb: Disable change of plane parameters

[Anitha Chrisanthus]

From: Edmund Dea <edmund.j.dea@intel.com>

Due to HW limitations, KMB cannot change height, width, or
pixel format after initial plane configuration.

Fixes: 7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display")
Signed-off-by: Edmund Dea <edmund.j.dea@intel.com>
---
 drivers/gpu/drm/kmb/kmb_crtc.c  |  2 ++
 drivers/gpu/drm/kmb/kmb_drv.h   |  1 +
 drivers/gpu/drm/kmb/kmb_plane.c | 44 ++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/kmb/kmb_plane.h |  6 +++++
 4 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/kmb/kmb_crtc.c b/drivers/gpu/drm/kmb/kmb_crtc.c
index 44626044c85e..42c8010ef2f6 100644
--- a/drivers/gpu/drm/kmb/kmb_crtc.c
+++ b/drivers/gpu/drm/kmb/kmb_crtc.c
@@ -226,6 +226,8 @@ int kmb_setup_crtc(struct drm_device *drm)
 	struct kmb_plane *primary;
 	int ret;
 
+	memset(kmb->init_disp_cfg, 0, sizeof(kmb->init_disp_cfg));
+
 	primary = kmb_plane_init(drm);
 	if (IS_ERR(primary))
 		return PTR_ERR(primary);
diff --git a/drivers/gpu/drm/kmb/kmb_drv.h b/drivers/gpu/drm/kmb/kmb_drv.h
index fefb1052058c..dc0679a70cc5 100644
--- a/drivers/gpu/drm/kmb/kmb_drv.h
+++ b/drivers/gpu/drm/kmb/kmb_drv.h
@@ -56,6 +56,7 @@ struct kmb_drm_private {
 	spinlock_t			irq_lock;
 	int				irq_lcd;
 	int				sys_clk_mhz;
+	struct disp_cfg			init_disp_cfg[KMB_MAX_PLANES];
 	struct layer_status		plane_status[KMB_MAX_PLANES];
 	int				kmb_under_flow;
 	int				kmb_flush_done;
diff --git a/drivers/gpu/drm/kmb/kmb_plane.c b/drivers/gpu/drm/kmb/kmb_plane.c
index e45419d6ed96..dacec5c4266f 100644
--- a/drivers/gpu/drm/kmb/kmb_plane.c
+++ b/drivers/gpu/drm/kmb/kmb_plane.c
@@ -71,12 +71,26 @@ static const u32 kmb_formats_v[] = {
 
 static unsigned int check_pixel_format(struct drm_plane *plane, u32 format)
 {
+	struct kmb_drm_private *kmb;
+	struct kmb_plane *kmb_plane = to_kmb_plane(plane);
 	int i;
+	int plane_id = kmb_plane->id;
+	struct disp_cfg init_disp_cfg;
 
+	kmb = to_kmb(plane->dev);
+	init_disp_cfg = kmb->init_disp_cfg[plane_id];
+	/* Due to HW limitations, changing pixel format after initial
+	 * plane configuration is not supported.
+	 */
+	if (init_disp_cfg.format && init_disp_cfg.format != format) {
+		drm_dbg(&kmb->drm, "Cannot change format after initial plane configuration");
+		return -EINVAL;
+	}
 	for (i = 0; i < plane->format_count; i++) {
 		if (plane->format_types[i] == format)
 			return 0;
 	}
+
 	return -EINVAL;
 }
 
@@ -85,11 +99,17 @@ static int kmb_plane_atomic_check(struct drm_plane *plane,
 {
 	struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state,
 										 plane);
+	struct kmb_drm_private *kmb;
+	struct kmb_plane *kmb_plane = to_kmb_plane(plane);
+	int plane_id = kmb_plane->id;
+	struct disp_cfg init_disp_cfg;
 	struct drm_framebuffer *fb;
 	int ret;
 	struct drm_crtc_state *crtc_state;
 	bool can_position;
 
+	kmb = to_kmb(plane->dev);
+	init_disp_cfg = kmb->init_disp_cfg[plane_id];
 	fb = new_plane_state->fb;
 	if (!fb || !new_plane_state->crtc)
 		return 0;
@@ -102,6 +122,16 @@ static int kmb_plane_atomic_check(struct drm_plane *plane,
 		return -EINVAL;
 	if (new_plane_state->crtc_w < KMB_MIN_WIDTH || new_plane_state->crtc_h < KMB_MIN_HEIGHT)
 		return -EINVAL;
+
+	/* Due to HW limitations, changing plane height or width after
+	 * initial plane configuration is not supported.
+	 */
+	if ((init_disp_cfg.width && init_disp_cfg.height) &&
+	    (init_disp_cfg.width != fb->width ||
+	    init_disp_cfg.height != fb->height)) {
+		drm_dbg(&kmb->drm, "Cannot change plane height or width after initial configuration");
+		return -EINVAL;
+	}
 	can_position = (plane->type == DRM_PLANE_TYPE_OVERLAY);
 	crtc_state =
 		drm_atomic_get_existing_crtc_state(state,
@@ -300,6 +330,7 @@ static void kmb_plane_atomic_update(struct drm_plane *plane,
 	unsigned char plane_id;
 	int num_planes, i;
 	static dma_addr_t addr[MAX_SUB_PLANES];
+	struct disp_cfg *init_disp_cfg;
 	struct viv_vidmem_metadata *md = NULL;
 	struct drm_gem_object *gem_obj;
 
@@ -323,7 +354,8 @@ static void kmb_plane_atomic_update(struct drm_plane *plane,
 	}
 	spin_unlock_irq(&kmb->irq_lock);
 
-	src_w = (new_plane_state->src_w >> 16);
+	init_disp_cfg = &kmb->init_disp_cfg[plane_id];
+	src_w = new_plane_state->src_w >> 16;
 	src_h = new_plane_state->src_h >> 16;
 	crtc_x = new_plane_state->crtc_x;
 	crtc_y = new_plane_state->crtc_y;
@@ -478,6 +510,16 @@ static void kmb_plane_atomic_update(struct drm_plane *plane,
 
 	/* Enable DMA */
 	kmb_write_lcd(kmb, LCD_LAYERn_DMA_CFG(plane_id), dma_cfg);
+
+	/* Save initial display config */
+	if (!init_disp_cfg->width ||
+	    !init_disp_cfg->height ||
+	    !init_disp_cfg->format) {
+		init_disp_cfg->width = width;
+		init_disp_cfg->height = height;
+		init_disp_cfg->format = fb->format->format;
+	}
+
 	drm_dbg(&kmb->drm, "dma_cfg=0x%x LCD_DMA_CFG=0x%x\n", dma_cfg,
 		kmb_read_lcd(kmb, LCD_LAYERn_DMA_CFG(plane_id)));
 
diff --git a/drivers/gpu/drm/kmb/kmb_plane.h b/drivers/gpu/drm/kmb/kmb_plane.h
index 486490f7a3ec..99207b35365c 100644
--- a/drivers/gpu/drm/kmb/kmb_plane.h
+++ b/drivers/gpu/drm/kmb/kmb_plane.h
@@ -62,6 +62,12 @@ struct layer_status {
 	u32 ctrl;
 };
 
+struct disp_cfg {
+	unsigned int width;
+	unsigned int height;
+	unsigned int format;
+};
+
 struct kmb_plane *kmb_plane_init(struct drm_device *drm);
 void kmb_plane_destroy(struct drm_plane *plane);
 #endif /* __KMB_PLANE_H__ */
-- 
2.25.1

[PATCH 08/14] drm/kmb: Corrected typo in handle_lcd_irq

[Anitha Chrisanthus]

Check for Overflow bits for layer3 in the irq handler.

Fixes: 7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display")
Signed-off-by: Anitha Chrisanthus <anitha.chrisanthus@intel.com>
---
 drivers/gpu/drm/kmb/kmb_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/kmb/kmb_drv.c b/drivers/gpu/drm/kmb/kmb_drv.c
index f54392ec4fab..bb7eca9e13ae 100644
--- a/drivers/gpu/drm/kmb/kmb_drv.c
+++ b/drivers/gpu/drm/kmb/kmb_drv.c
@@ -381,7 +381,7 @@ static irqreturn_t handle_lcd_irq(struct drm_device *dev)
 		if (val & LAYER3_DMA_FIFO_UNDERFLOW)
 			drm_dbg(&kmb->drm,
 				"LAYER3:GL1 DMA UNDERFLOW val = 0x%lx", val);
-		if (val & LAYER3_DMA_FIFO_UNDERFLOW)
+		if (val & LAYER3_DMA_FIFO_OVERFLOW)
 			drm_dbg(&kmb->drm,
 				"LAYER3:GL1 DMA OVERFLOW val = 0x%lx", val);
 	}
-- 
2.25.1

[PATCH 09/14] drm/kmb : W/A for planar formats

[Anitha Chrisanthus]

This is a work around for fully planar formats, where color corruption
was observed for formats like YU12, YU16 etc. Set the DMA Vstride and
Line width for U and V planes to the same as the Y plane and not the
actual pitch. For decoded video frames, continue to use the info from
metadata.

Fixes: 7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display")
Signed-off-by: Anitha Chrisanthus <anitha.chrisanthus@intel.com>
---
 drivers/gpu/drm/kmb/kmb_plane.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/kmb/kmb_plane.c b/drivers/gpu/drm/kmb/kmb_plane.c
index dacec5c4266f..4523af949ea1 100644
--- a/drivers/gpu/drm/kmb/kmb_plane.c
+++ b/drivers/gpu/drm/kmb/kmb_plane.c
@@ -333,6 +333,7 @@ static void kmb_plane_atomic_update(struct drm_plane *plane,
 	struct disp_cfg *init_disp_cfg;
 	struct viv_vidmem_metadata *md = NULL;
 	struct drm_gem_object *gem_obj;
+	unsigned int cb_stride, cr_stride;
 
 	if (!plane || !new_plane_state || !old_plane_state)
 		return;
@@ -397,8 +398,10 @@ static void kmb_plane_atomic_update(struct drm_plane *plane,
 	val |= get_bits_per_pixel(fb->format);
 	/* Program Cb/Cr for planar formats */
 	if (num_planes > 1) {
-		kmb_write_lcd(kmb, LCD_LAYERn_DMA_CB_LINE_VSTRIDE(plane_id),
-				fb->pitches[1]);
+		cb_stride = md ? fb->pitches[1] : width * fb->format->cpp[0];
+		kmb_write_lcd(kmb,
+			      LCD_LAYERn_DMA_CB_LINE_VSTRIDE(plane_id),
+			      cb_stride);
 		kmb_write_lcd(kmb, LCD_LAYERn_DMA_CB_LINE_WIDTH(plane_id),
 			      (width * fb->format->cpp[0]));
 
@@ -419,9 +422,11 @@ static void kmb_plane_atomic_update(struct drm_plane *plane,
 					addr[U_PLANE]);
 
 		if (num_planes == 3) {
+			cr_stride = md ? fb->pitches[2] :
+				    width * fb->format->cpp[0];
 			kmb_write_lcd(kmb,
 				      LCD_LAYERn_DMA_CR_LINE_VSTRIDE(plane_id),
-				      fb->pitches[2]);
+				      cr_stride);
 
 			kmb_write_lcd(kmb,
 				      LCD_LAYERn_DMA_CR_LINE_WIDTH(plane_id),
-- 
2.25.1

RE: [PATCH 09/14] drm/kmb : W/A for planar formats

[Chrisanthus, Anitha]

Please ignore this patch. Will combine this with 256B w/a patch.

> -----Original Message-----
> From: Chrisanthus, Anitha <anitha.chrisanthus@intel.com>
> Sent: Tuesday, July 27, 2021 5:31 PM
> To: dri-devel@lists.freedesktop.org; Chrisanthus, Anitha
> <anitha.chrisanthus@intel.com>; Dea, Edmund J <edmund.j.dea@intel.com>
> Subject: [PATCH 09/14] drm/kmb : W/A for planar formats
> 
> This is a work around for fully planar formats, where color corruption
> was observed for formats like YU12, YU16 etc. Set the DMA Vstride and
> Line width for U and V planes to the same as the Y plane and not the
> actual pitch. For decoded video frames, continue to use the info from
> metadata.
> 
> Fixes: 7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display")
> Signed-off-by: Anitha Chrisanthus <anitha.chrisanthus@intel.com>
> ---
>  drivers/gpu/drm/kmb/kmb_plane.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/kmb/kmb_plane.c
> b/drivers/gpu/drm/kmb/kmb_plane.c
> index dacec5c4266f..4523af949ea1 100644
> --- a/drivers/gpu/drm/kmb/kmb_plane.c
> +++ b/drivers/gpu/drm/kmb/kmb_plane.c
> @@ -333,6 +333,7 @@ static void kmb_plane_atomic_update(struct
> drm_plane *plane,
>  	struct disp_cfg *init_disp_cfg;
>  	struct viv_vidmem_metadata *md = NULL;
>  	struct drm_gem_object *gem_obj;
> +	unsigned int cb_stride, cr_stride;
> 
>  	if (!plane || !new_plane_state || !old_plane_state)
>  		return;
> @@ -397,8 +398,10 @@ static void kmb_plane_atomic_update(struct
> drm_plane *plane,
>  	val |= get_bits_per_pixel(fb->format);
>  	/* Program Cb/Cr for planar formats */
>  	if (num_planes > 1) {
> -		kmb_write_lcd(kmb,
> LCD_LAYERn_DMA_CB_LINE_VSTRIDE(plane_id),
> -				fb->pitches[1]);
> +		cb_stride = md ? fb->pitches[1] : width * fb->format->cpp[0];
> +		kmb_write_lcd(kmb,
> +			      LCD_LAYERn_DMA_CB_LINE_VSTRIDE(plane_id),
> +			      cb_stride);
>  		kmb_write_lcd(kmb,
> LCD_LAYERn_DMA_CB_LINE_WIDTH(plane_id),
>  			      (width * fb->format->cpp[0]));
> 
> @@ -419,9 +422,11 @@ static void kmb_plane_atomic_update(struct
> drm_plane *plane,
>  					addr[U_PLANE]);
> 
>  		if (num_planes == 3) {
> +			cr_stride = md ? fb->pitches[2] :
> +				    width * fb->format->cpp[0];
>  			kmb_write_lcd(kmb,
> 
> LCD_LAYERn_DMA_CR_LINE_VSTRIDE(plane_id),
> -				      fb->pitches[2]);
> +				      cr_stride);
> 
>  			kmb_write_lcd(kmb,
> 
> LCD_LAYERn_DMA_CR_LINE_WIDTH(plane_id),
> --
> 2.25.1

[PATCH 10/14] drm/kmb: Enable ADV bridge after modeset

[Anitha Chrisanthus]

On KMB, ADV bridge must be programmed and powered on prior to
MIPI DSI HW initialization.

Fixes: 98521f4d4b4c ("drm/kmb: Mipi DSI part of the display driver")
Signed-off-by: Anitha Chrisanthus <anitha.chrisanthus@intel.com>
---
 drivers/gpu/drm/kmb/kmb_dsi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/kmb/kmb_dsi.c b/drivers/gpu/drm/kmb/kmb_dsi.c
index 5bc6c84073a3..1cca0fe6f35f 100644
--- a/drivers/gpu/drm/kmb/kmb_dsi.c
+++ b/drivers/gpu/drm/kmb/kmb_dsi.c
@@ -1341,6 +1341,7 @@ static void connect_lcd_to_mipi(struct kmb_dsi *kmb_dsi)
 		return;
 	}
 
+	drm_bridge_chain_enable(adv_bridge);
 	/* DISABLE MIPI->CIF CONNECTION */
 	regmap_write(msscam, MSS_MIPI_CIF_CFG, 0);
 
-- 
2.25.1

Re: [PATCH 10/14] drm/kmb: Enable ADV bridge after modeset

[Sam Ravnborg]

On Tue, Jul 27, 2021 at 05:31:22PM -0700, Anitha Chrisanthus wrote:
> On KMB, ADV bridge must be programmed and powered on prior to
> MIPI DSI HW initialization.
> 
> Fixes: 98521f4d4b4c ("drm/kmb: Mipi DSI part of the display driver")
> Signed-off-by: Anitha Chrisanthus <anitha.chrisanthus@intel.com>
> ---
>  drivers/gpu/drm/kmb/kmb_dsi.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/kmb/kmb_dsi.c b/drivers/gpu/drm/kmb/kmb_dsi.c
> index 5bc6c84073a3..1cca0fe6f35f 100644
> --- a/drivers/gpu/drm/kmb/kmb_dsi.c
> +++ b/drivers/gpu/drm/kmb/kmb_dsi.c
> @@ -1341,6 +1341,7 @@ static void connect_lcd_to_mipi(struct kmb_dsi *kmb_dsi)
>  		return;
>  	}
>  
> +	drm_bridge_chain_enable(adv_bridge);
This function is about to be deleted.

Please use the atomic variant drm_atomic_bridge_chain_enable()

Also, I cannot see why this display driver has to call this.
Something else seems missing here...

	Sam

[PATCH 11/14] drm/kmb: Prune display modes with CRTC vfp < 4

[Anitha Chrisanthus]

From: Edmund Dea <edmund.j.dea@intel.com>

Monitors with vfp < 4 are not supported in KMB display. This change
prunes display modes with vfp < 4.

Fixes: 7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display")
Signed-off-by: Edmund Dea <edmund.j.dea@intel.com>
---
 drivers/gpu/drm/kmb/kmb_crtc.c | 6 ++++++
 drivers/gpu/drm/kmb/kmb_drv.h  | 1 +
 2 files changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/kmb/kmb_crtc.c b/drivers/gpu/drm/kmb/kmb_crtc.c
index 42c8010ef2f6..0e42c40f0dce 100644
--- a/drivers/gpu/drm/kmb/kmb_crtc.c
+++ b/drivers/gpu/drm/kmb/kmb_crtc.c
@@ -191,6 +191,7 @@ static enum drm_mode_status
 {
 	int refresh;
 	struct drm_device *dev = crtc->dev;
+	int vfp = mode->vsync_start - mode->vdisplay;
 
 	if (mode->vdisplay < KMB_CRTC_MAX_HEIGHT) {
 		drm_dbg(dev, "height = %d less than %d",
@@ -209,6 +210,11 @@ static enum drm_mode_status
 		return MODE_BAD;
 	}
 
+	if (vfp < KMB_CRTC_MIN_VFP) {
+		drm_dbg(dev, "vfp = %d less than %d", vfp, KMB_CRTC_MIN_VFP);
+		return MODE_BAD;
+	}
+
 	return MODE_OK;
 }
 
diff --git a/drivers/gpu/drm/kmb/kmb_drv.h b/drivers/gpu/drm/kmb/kmb_drv.h
index dc0679a70cc5..c4af1d927e45 100644
--- a/drivers/gpu/drm/kmb/kmb_drv.h
+++ b/drivers/gpu/drm/kmb/kmb_drv.h
@@ -21,6 +21,7 @@
 #define DRIVER_MINOR			2
 
 /* Platform definitions */
+#define KMB_CRTC_MIN_VFP		4
 #define KMB_CRTC_MAX_WIDTH		1920 /* max width in pixels */
 #define KMB_CRTC_MAX_HEIGHT		1080 /* max height in pixels */
 #define KMB_CRTC_MIN_WIDTH		1920
-- 
2.25.1

[PATCH 12/14] drm/kmb: Fix possible oops in error handling

[Anitha Chrisanthus]

If kmb_dsi_init() fails the "kmb->kmb_dsi" variable is an error pointer.
This can potentially result in kernel panic when kmb_dsi_host_unregister is
called.

Fixes: 7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display")
Fixes: 98521f4d4b4c ("drm/kmb: Mipi DSI part of the display driver")
Cc: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Anitha Chrisanthus <anitha.chrisanthus@intel.com>
---
 drivers/gpu/drm/kmb/kmb_drv.c | 9 ++++++---
 drivers/gpu/drm/kmb/kmb_dsi.c | 9 +++++----
 drivers/gpu/drm/kmb/kmb_dsi.h | 3 ++-
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/kmb/kmb_drv.c b/drivers/gpu/drm/kmb/kmb_drv.c
index bb7eca9e13ae..12f35c43d838 100644
--- a/drivers/gpu/drm/kmb/kmb_drv.c
+++ b/drivers/gpu/drm/kmb/kmb_drv.c
@@ -454,8 +454,9 @@ static int kmb_remove(struct platform_device *pdev)
 	dev_set_drvdata(dev, NULL);
 
 	/* Unregister DSI host */
-	kmb_dsi_host_unregister(kmb->kmb_dsi);
+	kmb_dsi_host_unregister();
 	drm_atomic_helper_shutdown(drm);
+	drm_dev_put(drm);
 	return 0;
 }
 
@@ -519,7 +520,7 @@ static int kmb_probe(struct platform_device *pdev)
 	if (IS_ERR(kmb->kmb_dsi)) {
 		drm_err(&kmb->drm, "failed to initialize DSI\n");
 		ret = PTR_ERR(kmb->kmb_dsi);
-		goto err_free1;
+		goto err_free2;
 	}
 
 	kmb->kmb_dsi->dev = &dsi_pdev->dev;
@@ -555,8 +556,10 @@ static int kmb_probe(struct platform_device *pdev)
 	drm_crtc_cleanup(&kmb->crtc);
 	drm_mode_config_cleanup(&kmb->drm);
  err_free1:
+	kmb_dsi_clk_disable(kmb->kmb_dsi);
+ err_free2:
 	dev_set_drvdata(dev, NULL);
-	kmb_dsi_host_unregister(kmb->kmb_dsi);
+	kmb_dsi_host_unregister();
 
 	return ret;
 }
diff --git a/drivers/gpu/drm/kmb/kmb_dsi.c b/drivers/gpu/drm/kmb/kmb_dsi.c
index 1cca0fe6f35f..a500172ada87 100644
--- a/drivers/gpu/drm/kmb/kmb_dsi.c
+++ b/drivers/gpu/drm/kmb/kmb_dsi.c
@@ -172,17 +172,17 @@ mipi_hs_freq_range[MIPI_DPHY_DEFAULT_BIT_RATES] = {
 	{.default_bit_rate_mbps = 2500, .hsfreqrange_code = 0x49}
 };
 
-static void kmb_dsi_clk_disable(struct kmb_dsi *kmb_dsi)
+void kmb_dsi_clk_disable(struct kmb_dsi *kmb_dsi)
 {
 	clk_disable_unprepare(kmb_dsi->clk_mipi);
 	clk_disable_unprepare(kmb_dsi->clk_mipi_ecfg);
 	clk_disable_unprepare(kmb_dsi->clk_mipi_cfg);
 }
 
-void kmb_dsi_host_unregister(struct kmb_dsi *kmb_dsi)
+void kmb_dsi_host_unregister(void)
 {
-	kmb_dsi_clk_disable(kmb_dsi);
-	mipi_dsi_host_unregister(kmb_dsi->host);
+	if (dsi_host)
+		mipi_dsi_host_unregister(dsi_host);
 }
 
 /*
@@ -229,6 +229,7 @@ int kmb_dsi_host_bridge_init(struct device *dev)
 			dsi_device = kzalloc(sizeof(*dsi_device), GFP_KERNEL);
 			if (!dsi_device) {
 				kfree(dsi_host);
+				dsi_host = NULL;
 				return -ENOMEM;
 			}
 		}
diff --git a/drivers/gpu/drm/kmb/kmb_dsi.h b/drivers/gpu/drm/kmb/kmb_dsi.h
index 66b7c500d9bc..89e85c993609 100644
--- a/drivers/gpu/drm/kmb/kmb_dsi.h
+++ b/drivers/gpu/drm/kmb/kmb_dsi.h
@@ -378,7 +378,8 @@ static inline void kmb_clr_bit_mipi(struct kmb_dsi *kmb_dsi,
 
 int kmb_dsi_host_bridge_init(struct device *dev);
 struct kmb_dsi *kmb_dsi_init(struct platform_device *pdev);
-void kmb_dsi_host_unregister(struct kmb_dsi *kmb_dsi);
+void kmb_dsi_host_unregister(void);
+void kmb_dsi_clk_disable(struct kmb_dsi *kmb_dsi);
 int kmb_dsi_mode_set(struct kmb_dsi *kmb_dsi, struct drm_display_mode *mode,
 		     int sys_clk_mhz);
 int kmb_dsi_map_mmio(struct kmb_dsi *kmb_dsi);
-- 
2.25.1

Re: [PATCH 12/14] drm/kmb: Fix possible oops in error handling

[Sam Ravnborg]

Hi Anitha,

On Tue, Jul 27, 2021 at 05:31:24PM -0700, Anitha Chrisanthus wrote:
> If kmb_dsi_init() fails the "kmb->kmb_dsi" variable is an error pointer.
> This can potentially result in kernel panic when kmb_dsi_host_unregister is
> called.
> 
> Fixes: 7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display")
> Fixes: 98521f4d4b4c ("drm/kmb: Mipi DSI part of the display driver")
> Cc: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Anitha Chrisanthus <anitha.chrisanthus@intel.com>
> ---
>  drivers/gpu/drm/kmb/kmb_drv.c | 9 ++++++---
>  drivers/gpu/drm/kmb/kmb_dsi.c | 9 +++++----
>  drivers/gpu/drm/kmb/kmb_dsi.h | 3 ++-
>  3 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/kmb/kmb_drv.c b/drivers/gpu/drm/kmb/kmb_drv.c
> index bb7eca9e13ae..12f35c43d838 100644
> --- a/drivers/gpu/drm/kmb/kmb_drv.c
> +++ b/drivers/gpu/drm/kmb/kmb_drv.c
> @@ -454,8 +454,9 @@ static int kmb_remove(struct platform_device *pdev)
>  	dev_set_drvdata(dev, NULL);
>  
>  	/* Unregister DSI host */
> -	kmb_dsi_host_unregister(kmb->kmb_dsi);
> +	kmb_dsi_host_unregister();
>  	drm_atomic_helper_shutdown(drm);
> +	drm_dev_put(drm);
>  	return 0;
>  }
>  
> @@ -519,7 +520,7 @@ static int kmb_probe(struct platform_device *pdev)
>  	if (IS_ERR(kmb->kmb_dsi)) {
>  		drm_err(&kmb->drm, "failed to initialize DSI\n");
>  		ret = PTR_ERR(kmb->kmb_dsi);
> -		goto err_free1;
> +		goto err_free2;
>  	}
>  
>  	kmb->kmb_dsi->dev = &dsi_pdev->dev;
> @@ -555,8 +556,10 @@ static int kmb_probe(struct platform_device *pdev)
>  	drm_crtc_cleanup(&kmb->crtc);
>  	drm_mode_config_cleanup(&kmb->drm);
>   err_free1:
> +	kmb_dsi_clk_disable(kmb->kmb_dsi);
> + err_free2:
>  	dev_set_drvdata(dev, NULL);
> -	kmb_dsi_host_unregister(kmb->kmb_dsi);
> +	kmb_dsi_host_unregister();
>  

This really looks like a step backward. There should not be a eed to
call unregister if kmb_dsi is not a valid pointer in the first place.
Also drn_dev_put() should not be needed with the use of drmm
infrastructure.



>  	return ret;
>  }
> diff --git a/drivers/gpu/drm/kmb/kmb_dsi.c b/drivers/gpu/drm/kmb/kmb_dsi.c
> index 1cca0fe6f35f..a500172ada87 100644
> --- a/drivers/gpu/drm/kmb/kmb_dsi.c
> +++ b/drivers/gpu/drm/kmb/kmb_dsi.c
> @@ -172,17 +172,17 @@ mipi_hs_freq_range[MIPI_DPHY_DEFAULT_BIT_RATES] = {
>  	{.default_bit_rate_mbps = 2500, .hsfreqrange_code = 0x49}
>  };
>  
> -static void kmb_dsi_clk_disable(struct kmb_dsi *kmb_dsi)
> +void kmb_dsi_clk_disable(struct kmb_dsi *kmb_dsi)
>  {
>  	clk_disable_unprepare(kmb_dsi->clk_mipi);
>  	clk_disable_unprepare(kmb_dsi->clk_mipi_ecfg);
>  	clk_disable_unprepare(kmb_dsi->clk_mipi_cfg);
>  }
>  
> -void kmb_dsi_host_unregister(struct kmb_dsi *kmb_dsi)
> +void kmb_dsi_host_unregister(void)
>  {
> -	kmb_dsi_clk_disable(kmb_dsi);
> -	mipi_dsi_host_unregister(kmb_dsi->host);
> +	if (dsi_host)
> +		mipi_dsi_host_unregister(dsi_host);
>  }
I thought we had killed the global dsi_host variable??
Seems some cleanup is till needed here.

	Sam

RE: [PATCH 12/14] drm/kmb: Fix possible oops in error handling

[Chrisanthus, Anitha]

Hi Sam,

> -----Original Message-----
> From: Sam Ravnborg <sam@ravnborg.org>
> Sent: Wednesday, July 28, 2021 12:27 AM
> To: Chrisanthus, Anitha <anitha.chrisanthus@intel.com>
> Cc: dri-devel@lists.freedesktop.org; Dea, Edmund J
> <edmund.j.dea@intel.com>; Dan Carpenter <dan.carpenter@oracle.com>
> Subject: Re: [PATCH 12/14] drm/kmb: Fix possible oops in error handling
> 
> Hi Anitha,
> 
> On Tue, Jul 27, 2021 at 05:31:24PM -0700, Anitha Chrisanthus wrote:
> > If kmb_dsi_init() fails the "kmb->kmb_dsi" variable is an error pointer.
> > This can potentially result in kernel panic when kmb_dsi_host_unregister is
> > called.
> >
> > Fixes: 7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display")
> > Fixes: 98521f4d4b4c ("drm/kmb: Mipi DSI part of the display driver")
> > Cc: Dan Carpenter <dan.carpenter@oracle.com>
> > Signed-off-by: Anitha Chrisanthus <anitha.chrisanthus@intel.com>
> > ---
> >  drivers/gpu/drm/kmb/kmb_drv.c | 9 ++++++---
> >  drivers/gpu/drm/kmb/kmb_dsi.c | 9 +++++----
> >  drivers/gpu/drm/kmb/kmb_dsi.h | 3 ++-
> >  3 files changed, 13 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/kmb/kmb_drv.c
> b/drivers/gpu/drm/kmb/kmb_drv.c
> > index bb7eca9e13ae..12f35c43d838 100644
> > --- a/drivers/gpu/drm/kmb/kmb_drv.c
> > +++ b/drivers/gpu/drm/kmb/kmb_drv.c
> > @@ -454,8 +454,9 @@ static int kmb_remove(struct platform_device
> *pdev)
> >  	dev_set_drvdata(dev, NULL);
> >
> >  	/* Unregister DSI host */
> > -	kmb_dsi_host_unregister(kmb->kmb_dsi);
> > +	kmb_dsi_host_unregister();
> >  	drm_atomic_helper_shutdown(drm);
> > +	drm_dev_put(drm);
> >  	return 0;
> >  }
> >
> > @@ -519,7 +520,7 @@ static int kmb_probe(struct platform_device *pdev)
> >  	if (IS_ERR(kmb->kmb_dsi)) {
> >  		drm_err(&kmb->drm, "failed to initialize DSI\n");
> >  		ret = PTR_ERR(kmb->kmb_dsi);
> > -		goto err_free1;
> > +		goto err_free2;
> >  	}
> >
> >  	kmb->kmb_dsi->dev = &dsi_pdev->dev;
> > @@ -555,8 +556,10 @@ static int kmb_probe(struct platform_device *pdev)
> >  	drm_crtc_cleanup(&kmb->crtc);
> >  	drm_mode_config_cleanup(&kmb->drm);
> >   err_free1:
> > +	kmb_dsi_clk_disable(kmb->kmb_dsi);
> > + err_free2:
> >  	dev_set_drvdata(dev, NULL);
> > -	kmb_dsi_host_unregister(kmb->kmb_dsi);
> > +	kmb_dsi_host_unregister();
> >
> 
> This really looks like a step backward. There should not be a eed to
> call unregister if kmb_dsi is not a valid pointer in the first place.
> Also drn_dev_put() should not be needed with the use of drmm
> infrastructure.
Agree, I was trying to address issues with Dan's original patch.
I will keep the original code, with only this change
        if (IS_ERR(kmb->kmb_dsi)) {
                drm_err(&kmb->drm, "failed to initialize DSI\n");
-               ret = PTR_ERR(kmb->kmb_dsi);
-               goto err_free2;
+               dev_set_drvdata(dev, NULL);
+               return PTR_ERR(kmb->kmb_dsi);
Will send v2
> 
> 
> 
> >  	return ret;
> >  }
> > diff --git a/drivers/gpu/drm/kmb/kmb_dsi.c
> b/drivers/gpu/drm/kmb/kmb_dsi.c
> > index 1cca0fe6f35f..a500172ada87 100644
> > --- a/drivers/gpu/drm/kmb/kmb_dsi.c
> > +++ b/drivers/gpu/drm/kmb/kmb_dsi.c
> > @@ -172,17 +172,17 @@
> mipi_hs_freq_range[MIPI_DPHY_DEFAULT_BIT_RATES] = {
> >  	{.default_bit_rate_mbps = 2500, .hsfreqrange_code = 0x49}
> >  };
> >
> > -static void kmb_dsi_clk_disable(struct kmb_dsi *kmb_dsi)
> > +void kmb_dsi_clk_disable(struct kmb_dsi *kmb_dsi)
> >  {
> >  	clk_disable_unprepare(kmb_dsi->clk_mipi);
> >  	clk_disable_unprepare(kmb_dsi->clk_mipi_ecfg);
> >  	clk_disable_unprepare(kmb_dsi->clk_mipi_cfg);
> >  }
> >
> > -void kmb_dsi_host_unregister(struct kmb_dsi *kmb_dsi)
> > +void kmb_dsi_host_unregister(void)
> >  {
> > -	kmb_dsi_clk_disable(kmb_dsi);
> > -	mipi_dsi_host_unregister(kmb_dsi->host);
> > +	if (dsi_host)
> > +		mipi_dsi_host_unregister(dsi_host);
> >  }
> I thought we had killed the global dsi_host variable??
> Seems some cleanup is till needed here.
> 
> 	Sam

Re: [PATCH 12/14] drm/kmb: Fix possible oops in error handling

[Dan Carpenter]

On Tue, Jul 27, 2021 at 05:31:24PM -0700, Anitha Chrisanthus wrote:
> If kmb_dsi_init() fails the "kmb->kmb_dsi" variable is an error pointer.
> This can potentially result in kernel panic when kmb_dsi_host_unregister is
> called.
> 
> Fixes: 7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display")
> Fixes: 98521f4d4b4c ("drm/kmb: Mipi DSI part of the display driver")
> Cc: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Anitha Chrisanthus <anitha.chrisanthus@intel.com>
> ---
>  drivers/gpu/drm/kmb/kmb_drv.c | 9 ++++++---
>  drivers/gpu/drm/kmb/kmb_dsi.c | 9 +++++----
>  drivers/gpu/drm/kmb/kmb_dsi.h | 3 ++-
>  3 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/kmb/kmb_drv.c b/drivers/gpu/drm/kmb/kmb_drv.c
> index bb7eca9e13ae..12f35c43d838 100644
> --- a/drivers/gpu/drm/kmb/kmb_drv.c
> +++ b/drivers/gpu/drm/kmb/kmb_drv.c
> @@ -454,8 +454,9 @@ static int kmb_remove(struct platform_device *pdev)
>  	dev_set_drvdata(dev, NULL);
>  
>  	/* Unregister DSI host */
> -	kmb_dsi_host_unregister(kmb->kmb_dsi);
> +	kmb_dsi_host_unregister();
>  	drm_atomic_helper_shutdown(drm);
> +	drm_dev_put(drm);
>  	return 0;
>  }
>  
> @@ -519,7 +520,7 @@ static int kmb_probe(struct platform_device *pdev)
>  	if (IS_ERR(kmb->kmb_dsi)) {
>  		drm_err(&kmb->drm, "failed to initialize DSI\n");
>  		ret = PTR_ERR(kmb->kmb_dsi);
> -		goto err_free1;
> +		goto err_free2;

Don't use numberred labels.  The label names should say what the goto
does just like a function name says what calling the function does.
The existing label names in this function mostly use "Come From" label
style which is not useful either.

When you're writing probe function keep track in your head of the most
recent successful allocation and then when an error occurs that's what
you have to free.

	a = alloc();
	if (!a)
		return;  <-- nothing to free

	b = alloc();
	if (!b)
		goto free_a;  <-- good name.  a is the most recent.

	c = alloc();
	if (!c)
		goto free_b;

	return 0;

free_b:
	free(b);
free_a:
	free(a);

Then copy and past the error handling and add a free(c) to create the
release function:

void release(struct foo *p)
{
	free(c);
	free(b);
	free(a);

}

regards,
dan carpenter

[PATCH 13/14] drm/kmb: Enable alpha blended second plane

[Anitha Chrisanthus]

From: Edmund Dea <edmund.j.dea@intel.com>

Enable one additional plane that is alpha blended on top
of the primary plane.

Signed-off-by: Edmund Dea <edmund.j.dea@intel.com>
---
 drivers/gpu/drm/kmb/kmb_drv.c   |  8 ++--
 drivers/gpu/drm/kmb/kmb_plane.c | 81 +++++++++++++++++++++++++++++----
 drivers/gpu/drm/kmb/kmb_plane.h |  5 +-
 drivers/gpu/drm/kmb/kmb_regs.h  |  3 ++
 4 files changed, 82 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/kmb/kmb_drv.c b/drivers/gpu/drm/kmb/kmb_drv.c
index 12f35c43d838..d0de1db03493 100644
--- a/drivers/gpu/drm/kmb/kmb_drv.c
+++ b/drivers/gpu/drm/kmb/kmb_drv.c
@@ -173,10 +173,10 @@ static int kmb_setup_mode_config(struct drm_device *drm)
 	ret = drmm_mode_config_init(drm);
 	if (ret)
 		return ret;
-	drm->mode_config.min_width = KMB_MIN_WIDTH;
-	drm->mode_config.min_height = KMB_MIN_HEIGHT;
-	drm->mode_config.max_width = KMB_MAX_WIDTH;
-	drm->mode_config.max_height = KMB_MAX_HEIGHT;
+	drm->mode_config.min_width = KMB_FB_MIN_WIDTH;
+	drm->mode_config.min_height = KMB_FB_MIN_HEIGHT;
+	drm->mode_config.max_width = KMB_FB_MAX_WIDTH;
+	drm->mode_config.max_height = KMB_FB_MAX_HEIGHT;
 	drm->mode_config.funcs = &kmb_mode_config_funcs;
 
 	ret = kmb_setup_crtc(drm);
diff --git a/drivers/gpu/drm/kmb/kmb_plane.c b/drivers/gpu/drm/kmb/kmb_plane.c
index 4523af949ea1..cbe4e981d73e 100644
--- a/drivers/gpu/drm/kmb/kmb_plane.c
+++ b/drivers/gpu/drm/kmb/kmb_plane.c
@@ -118,9 +118,10 @@ static int kmb_plane_atomic_check(struct drm_plane *plane,
 	if (ret)
 		return ret;
 
-	if (new_plane_state->crtc_w > KMB_MAX_WIDTH || new_plane_state->crtc_h > KMB_MAX_HEIGHT)
-		return -EINVAL;
-	if (new_plane_state->crtc_w < KMB_MIN_WIDTH || new_plane_state->crtc_h < KMB_MIN_HEIGHT)
+	if (new_plane_state->crtc_w > KMB_FB_MAX_WIDTH ||
+	    new_plane_state->crtc_h > KMB_FB_MAX_HEIGHT ||
+	    new_plane_state->crtc_w < KMB_FB_MIN_WIDTH ||
+	    new_plane_state->crtc_h < KMB_FB_MIN_HEIGHT)
 		return -EINVAL;
 
 	/* Due to HW limitations, changing plane height or width after
@@ -311,6 +312,44 @@ static void config_csc(struct kmb_drm_private *kmb, int plane_id)
 	kmb_write_lcd(kmb, LCD_LAYERn_CSC_OFF3(plane_id), csc_coef_lcd[11]);
 }
 
+static void kmb_plane_set_alpha(struct kmb_drm_private *kmb,
+				const struct drm_plane_state *state,
+				unsigned char plane_id,
+				unsigned int *val)
+{
+	u16 plane_alpha = state->alpha;
+	u16 pixel_blend_mode = state->pixel_blend_mode;
+	int has_alpha = state->fb->format->has_alpha;
+
+	if (plane_alpha != DRM_BLEND_ALPHA_OPAQUE)
+		*val |= LCD_LAYER_ALPHA_STATIC;
+
+	if (has_alpha) {
+		switch (pixel_blend_mode) {
+		case DRM_MODE_BLEND_PIXEL_NONE:
+			break;
+		case DRM_MODE_BLEND_PREMULTI:
+			*val |= LCD_LAYER_ALPHA_EMBED | LCD_LAYER_ALPHA_PREMULT;
+			break;
+		case DRM_MODE_BLEND_COVERAGE:
+			*val |= LCD_LAYER_ALPHA_EMBED;
+			break;
+		default:
+			DRM_DEBUG("Missing pixel blend mode case (%s == %ld)\n",
+				  __stringify(pixel_blend_mode),
+				  (long)pixel_blend_mode);
+			break;
+		}
+	}
+
+	if (plane_alpha == DRM_BLEND_ALPHA_OPAQUE && !has_alpha) {
+		*val &= LCD_LAYER_ALPHA_DISABLED;
+		return;
+	}
+
+	kmb_write_lcd(kmb, LCD_LAYERn_ALPHA(plane_id), plane_alpha);
+}
+
 static void kmb_plane_atomic_update(struct drm_plane *plane,
 				    struct drm_atomic_state *state)
 {
@@ -341,11 +380,12 @@ static void kmb_plane_atomic_update(struct drm_plane *plane,
 	fb = new_plane_state->fb;
 	if (!fb)
 		return;
+
 	num_planes = fb->format->num_planes;
 	kmb_plane = to_kmb_plane(plane);
-	plane_id = kmb_plane->id;
 
 	kmb = to_kmb(plane->dev);
+	plane_id = kmb_plane->id;
 
 	spin_lock_irq(&kmb->irq_lock);
 	if (kmb->kmb_under_flow || kmb->kmb_flush_done) {
@@ -467,20 +507,32 @@ static void kmb_plane_atomic_update(struct drm_plane *plane,
 		config_csc(kmb, plane_id);
 	}
 
+	kmb_plane_set_alpha(kmb, plane->state, plane_id, &val);
+
 	kmb_write_lcd(kmb, LCD_LAYERn_CFG(plane_id), val);
 
+	/* Configure LCD_CONTROL */
+	ctrl = kmb_read_lcd(kmb, LCD_CONTROL);
+
+	/* Set layer blending config */
+	ctrl &= ~LCD_CTRL_ALPHA_ALL;
+	ctrl |= LCD_CTRL_ALPHA_BOTTOM_VL1 |
+		LCD_CTRL_ALPHA_BLEND_VL2;
+
+	ctrl &= ~LCD_CTRL_ALPHA_BLEND_BKGND_DISABLE;
+
 	switch (plane_id) {
 	case LAYER_0:
-		ctrl = LCD_CTRL_VL1_ENABLE;
+		ctrl |= LCD_CTRL_VL1_ENABLE;
 		break;
 	case LAYER_1:
-		ctrl = LCD_CTRL_VL2_ENABLE;
+		ctrl |= LCD_CTRL_VL2_ENABLE;
 		break;
 	case LAYER_2:
-		ctrl = LCD_CTRL_GL1_ENABLE;
+		ctrl |= LCD_CTRL_GL1_ENABLE;
 		break;
 	case LAYER_3:
-		ctrl = LCD_CTRL_GL2_ENABLE;
+		ctrl |= LCD_CTRL_GL2_ENABLE;
 		break;
 	}
 
@@ -492,7 +544,7 @@ static void kmb_plane_atomic_update(struct drm_plane *plane,
 	 */
 	ctrl |= LCD_CTRL_VHSYNC_IDLE_LVL;
 
-	kmb_set_bitmask_lcd(kmb, LCD_CONTROL, ctrl);
+	kmb_write_lcd(kmb, LCD_CONTROL, ctrl);
 
 	/* Enable pipeline AXI read transactions for the DMA
 	 * after setting graphics layers. This must be done
@@ -567,6 +619,9 @@ struct kmb_plane *kmb_plane_init(struct drm_device *drm)
 	enum drm_plane_type plane_type;
 	const u32 *plane_formats;
 	int num_plane_formats;
+	unsigned int blend_caps = BIT(DRM_MODE_BLEND_PIXEL_NONE) |
+				  BIT(DRM_MODE_BLEND_PREMULTI)   |
+				  BIT(DRM_MODE_BLEND_COVERAGE);
 
 	for (i = 0; i < KMB_MAX_PLANES; i++) {
 		plane = drmm_kzalloc(drm, sizeof(*plane), GFP_KERNEL);
@@ -598,8 +653,16 @@ struct kmb_plane *kmb_plane_init(struct drm_device *drm)
 		drm_dbg(drm, "%s : %d i=%d type=%d",
 			__func__, __LINE__,
 			  i, plane_type);
+		drm_plane_create_alpha_property(&plane->base_plane);
+
+		drm_plane_create_blend_mode_property(&plane->base_plane,
+						     blend_caps);
+
+		drm_plane_create_zpos_immutable_property(&plane->base_plane, i);
+
 		drm_plane_helper_add(&plane->base_plane,
 				     &kmb_plane_helper_funcs);
+
 		if (plane_type == DRM_PLANE_TYPE_PRIMARY) {
 			primary = plane;
 			kmb->plane = plane;
diff --git a/drivers/gpu/drm/kmb/kmb_plane.h b/drivers/gpu/drm/kmb/kmb_plane.h
index 99207b35365c..b51144044fe8 100644
--- a/drivers/gpu/drm/kmb/kmb_plane.h
+++ b/drivers/gpu/drm/kmb/kmb_plane.h
@@ -35,6 +35,9 @@
 #define POSSIBLE_CRTCS 1
 #define to_kmb_plane(x) container_of(x, struct kmb_plane, base_plane)
 
+#define POSSIBLE_CRTCS		1
+#define KMB_MAX_PLANES		2
+
 enum layer_id {
 	LAYER_0,
 	LAYER_1,
@@ -43,8 +46,6 @@ enum layer_id {
 	/* KMB_MAX_PLANES */
 };
 
-#define KMB_MAX_PLANES 1
-
 enum sub_plane_id {
 	Y_PLANE,
 	U_PLANE,
diff --git a/drivers/gpu/drm/kmb/kmb_regs.h b/drivers/gpu/drm/kmb/kmb_regs.h
index 48150569f702..9756101b0d32 100644
--- a/drivers/gpu/drm/kmb/kmb_regs.h
+++ b/drivers/gpu/drm/kmb/kmb_regs.h
@@ -43,8 +43,10 @@
 #define LCD_CTRL_OUTPUT_ENABLED			  BIT(19)
 #define LCD_CTRL_BPORCH_ENABLE			  BIT(21)
 #define LCD_CTRL_FPORCH_ENABLE			  BIT(22)
+#define LCD_CTRL_ALPHA_BLEND_BKGND_DISABLE	  BIT(23)
 #define LCD_CTRL_PIPELINE_DMA			  BIT(28)
 #define LCD_CTRL_VHSYNC_IDLE_LVL		  BIT(31)
+#define LCD_CTRL_ALPHA_ALL			  (0xff << 6)
 
 /* interrupts */
 #define LCD_INT_STATUS				(0x4 * 0x001)
@@ -115,6 +117,7 @@
 #define LCD_LAYER_ALPHA_EMBED			BIT(5)
 #define LCD_LAYER_ALPHA_COMBI			(LCD_LAYER_ALPHA_STATIC | \
 						      LCD_LAYER_ALPHA_EMBED)
+#define LCD_LAYER_ALPHA_DISABLED		~(LCD_LAYER_ALPHA_COMBI)
 /* RGB multiplied with alpha */
 #define LCD_LAYER_ALPHA_PREMULT			BIT(6)
 #define LCD_LAYER_INVERT_COL			BIT(7)
-- 
2.25.1

Re: [PATCH 13/14] drm/kmb: Enable alpha blended second plane

[Sam Ravnborg]

Hi Anitha,
On Tue, Jul 27, 2021 at 05:31:25PM -0700, Anitha Chrisanthus wrote:
> From: Edmund Dea <edmund.j.dea@intel.com>
> 
> Enable one additional plane that is alpha blended on top
> of the primary plane.
> 
> Signed-off-by: Edmund Dea <edmund.j.dea@intel.com>
Your s-o-b is missing.

With this fixed:
Acked-by: Sam Ravnborg <sam@ravnborg.org>
> ---
>  drivers/gpu/drm/kmb/kmb_drv.c   |  8 ++--
>  drivers/gpu/drm/kmb/kmb_plane.c | 81 +++++++++++++++++++++++++++++----
>  drivers/gpu/drm/kmb/kmb_plane.h |  5 +-
>  drivers/gpu/drm/kmb/kmb_regs.h  |  3 ++
>  4 files changed, 82 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/kmb/kmb_drv.c b/drivers/gpu/drm/kmb/kmb_drv.c
> index 12f35c43d838..d0de1db03493 100644
> --- a/drivers/gpu/drm/kmb/kmb_drv.c
> +++ b/drivers/gpu/drm/kmb/kmb_drv.c
> @@ -173,10 +173,10 @@ static int kmb_setup_mode_config(struct drm_device *drm)
>  	ret = drmm_mode_config_init(drm);
>  	if (ret)
>  		return ret;
> -	drm->mode_config.min_width = KMB_MIN_WIDTH;
> -	drm->mode_config.min_height = KMB_MIN_HEIGHT;
> -	drm->mode_config.max_width = KMB_MAX_WIDTH;
> -	drm->mode_config.max_height = KMB_MAX_HEIGHT;
> +	drm->mode_config.min_width = KMB_FB_MIN_WIDTH;
> +	drm->mode_config.min_height = KMB_FB_MIN_HEIGHT;
> +	drm->mode_config.max_width = KMB_FB_MAX_WIDTH;
> +	drm->mode_config.max_height = KMB_FB_MAX_HEIGHT;
>  	drm->mode_config.funcs = &kmb_mode_config_funcs;
>  
>  	ret = kmb_setup_crtc(drm);
> diff --git a/drivers/gpu/drm/kmb/kmb_plane.c b/drivers/gpu/drm/kmb/kmb_plane.c
> index 4523af949ea1..cbe4e981d73e 100644
> --- a/drivers/gpu/drm/kmb/kmb_plane.c
> +++ b/drivers/gpu/drm/kmb/kmb_plane.c
> @@ -118,9 +118,10 @@ static int kmb_plane_atomic_check(struct drm_plane *plane,
>  	if (ret)
>  		return ret;
>  
> -	if (new_plane_state->crtc_w > KMB_MAX_WIDTH || new_plane_state->crtc_h > KMB_MAX_HEIGHT)
> -		return -EINVAL;
> -	if (new_plane_state->crtc_w < KMB_MIN_WIDTH || new_plane_state->crtc_h < KMB_MIN_HEIGHT)
> +	if (new_plane_state->crtc_w > KMB_FB_MAX_WIDTH ||
> +	    new_plane_state->crtc_h > KMB_FB_MAX_HEIGHT ||
> +	    new_plane_state->crtc_w < KMB_FB_MIN_WIDTH ||
> +	    new_plane_state->crtc_h < KMB_FB_MIN_HEIGHT)
>  		return -EINVAL;
>  
>  	/* Due to HW limitations, changing plane height or width after
> @@ -311,6 +312,44 @@ static void config_csc(struct kmb_drm_private *kmb, int plane_id)
>  	kmb_write_lcd(kmb, LCD_LAYERn_CSC_OFF3(plane_id), csc_coef_lcd[11]);
>  }
>  
> +static void kmb_plane_set_alpha(struct kmb_drm_private *kmb,
> +				const struct drm_plane_state *state,
> +				unsigned char plane_id,
> +				unsigned int *val)
> +{
> +	u16 plane_alpha = state->alpha;
> +	u16 pixel_blend_mode = state->pixel_blend_mode;
> +	int has_alpha = state->fb->format->has_alpha;
> +
> +	if (plane_alpha != DRM_BLEND_ALPHA_OPAQUE)
> +		*val |= LCD_LAYER_ALPHA_STATIC;
> +
> +	if (has_alpha) {
> +		switch (pixel_blend_mode) {
> +		case DRM_MODE_BLEND_PIXEL_NONE:
> +			break;
> +		case DRM_MODE_BLEND_PREMULTI:
> +			*val |= LCD_LAYER_ALPHA_EMBED | LCD_LAYER_ALPHA_PREMULT;
> +			break;
> +		case DRM_MODE_BLEND_COVERAGE:
> +			*val |= LCD_LAYER_ALPHA_EMBED;
> +			break;
> +		default:
> +			DRM_DEBUG("Missing pixel blend mode case (%s == %ld)\n",
> +				  __stringify(pixel_blend_mode),
> +				  (long)pixel_blend_mode);
> +			break;
> +		}
> +	}
> +
> +	if (plane_alpha == DRM_BLEND_ALPHA_OPAQUE && !has_alpha) {
> +		*val &= LCD_LAYER_ALPHA_DISABLED;
> +		return;
> +	}
> +
> +	kmb_write_lcd(kmb, LCD_LAYERn_ALPHA(plane_id), plane_alpha);
> +}
> +
>  static void kmb_plane_atomic_update(struct drm_plane *plane,
>  				    struct drm_atomic_state *state)
>  {
> @@ -341,11 +380,12 @@ static void kmb_plane_atomic_update(struct drm_plane *plane,
>  	fb = new_plane_state->fb;
>  	if (!fb)
>  		return;
> +
>  	num_planes = fb->format->num_planes;
>  	kmb_plane = to_kmb_plane(plane);
> -	plane_id = kmb_plane->id;
>  
>  	kmb = to_kmb(plane->dev);
> +	plane_id = kmb_plane->id;
>  
>  	spin_lock_irq(&kmb->irq_lock);
>  	if (kmb->kmb_under_flow || kmb->kmb_flush_done) {
> @@ -467,20 +507,32 @@ static void kmb_plane_atomic_update(struct drm_plane *plane,
>  		config_csc(kmb, plane_id);
>  	}
>  
> +	kmb_plane_set_alpha(kmb, plane->state, plane_id, &val);
> +
>  	kmb_write_lcd(kmb, LCD_LAYERn_CFG(plane_id), val);
>  
> +	/* Configure LCD_CONTROL */
> +	ctrl = kmb_read_lcd(kmb, LCD_CONTROL);
> +
> +	/* Set layer blending config */
> +	ctrl &= ~LCD_CTRL_ALPHA_ALL;
> +	ctrl |= LCD_CTRL_ALPHA_BOTTOM_VL1 |
> +		LCD_CTRL_ALPHA_BLEND_VL2;
> +
> +	ctrl &= ~LCD_CTRL_ALPHA_BLEND_BKGND_DISABLE;
> +
>  	switch (plane_id) {
>  	case LAYER_0:
> -		ctrl = LCD_CTRL_VL1_ENABLE;
> +		ctrl |= LCD_CTRL_VL1_ENABLE;
>  		break;
>  	case LAYER_1:
> -		ctrl = LCD_CTRL_VL2_ENABLE;
> +		ctrl |= LCD_CTRL_VL2_ENABLE;
>  		break;
>  	case LAYER_2:
> -		ctrl = LCD_CTRL_GL1_ENABLE;
> +		ctrl |= LCD_CTRL_GL1_ENABLE;
>  		break;
>  	case LAYER_3:
> -		ctrl = LCD_CTRL_GL2_ENABLE;
> +		ctrl |= LCD_CTRL_GL2_ENABLE;
>  		break;
>  	}
>  
> @@ -492,7 +544,7 @@ static void kmb_plane_atomic_update(struct drm_plane *plane,
>  	 */
>  	ctrl |= LCD_CTRL_VHSYNC_IDLE_LVL;
>  
> -	kmb_set_bitmask_lcd(kmb, LCD_CONTROL, ctrl);
> +	kmb_write_lcd(kmb, LCD_CONTROL, ctrl);
>  
>  	/* Enable pipeline AXI read transactions for the DMA
>  	 * after setting graphics layers. This must be done
> @@ -567,6 +619,9 @@ struct kmb_plane *kmb_plane_init(struct drm_device *drm)
>  	enum drm_plane_type plane_type;
>  	const u32 *plane_formats;
>  	int num_plane_formats;
> +	unsigned int blend_caps = BIT(DRM_MODE_BLEND_PIXEL_NONE) |
> +				  BIT(DRM_MODE_BLEND_PREMULTI)   |
> +				  BIT(DRM_MODE_BLEND_COVERAGE);
>  
>  	for (i = 0; i < KMB_MAX_PLANES; i++) {
>  		plane = drmm_kzalloc(drm, sizeof(*plane), GFP_KERNEL);
> @@ -598,8 +653,16 @@ struct kmb_plane *kmb_plane_init(struct drm_device *drm)
>  		drm_dbg(drm, "%s : %d i=%d type=%d",
>  			__func__, __LINE__,
>  			  i, plane_type);
> +		drm_plane_create_alpha_property(&plane->base_plane);
> +
> +		drm_plane_create_blend_mode_property(&plane->base_plane,
> +						     blend_caps);
> +
> +		drm_plane_create_zpos_immutable_property(&plane->base_plane, i);
> +
>  		drm_plane_helper_add(&plane->base_plane,
>  				     &kmb_plane_helper_funcs);
> +
>  		if (plane_type == DRM_PLANE_TYPE_PRIMARY) {
>  			primary = plane;
>  			kmb->plane = plane;
> diff --git a/drivers/gpu/drm/kmb/kmb_plane.h b/drivers/gpu/drm/kmb/kmb_plane.h
> index 99207b35365c..b51144044fe8 100644
> --- a/drivers/gpu/drm/kmb/kmb_plane.h
> +++ b/drivers/gpu/drm/kmb/kmb_plane.h
> @@ -35,6 +35,9 @@
>  #define POSSIBLE_CRTCS 1
>  #define to_kmb_plane(x) container_of(x, struct kmb_plane, base_plane)
>  
> +#define POSSIBLE_CRTCS		1
> +#define KMB_MAX_PLANES		2
> +
>  enum layer_id {
>  	LAYER_0,
>  	LAYER_1,
> @@ -43,8 +46,6 @@ enum layer_id {
>  	/* KMB_MAX_PLANES */
>  };
>  
> -#define KMB_MAX_PLANES 1
> -
>  enum sub_plane_id {
>  	Y_PLANE,
>  	U_PLANE,
> diff --git a/drivers/gpu/drm/kmb/kmb_regs.h b/drivers/gpu/drm/kmb/kmb_regs.h
> index 48150569f702..9756101b0d32 100644
> --- a/drivers/gpu/drm/kmb/kmb_regs.h
> +++ b/drivers/gpu/drm/kmb/kmb_regs.h
> @@ -43,8 +43,10 @@
>  #define LCD_CTRL_OUTPUT_ENABLED			  BIT(19)
>  #define LCD_CTRL_BPORCH_ENABLE			  BIT(21)
>  #define LCD_CTRL_FPORCH_ENABLE			  BIT(22)
> +#define LCD_CTRL_ALPHA_BLEND_BKGND_DISABLE	  BIT(23)
>  #define LCD_CTRL_PIPELINE_DMA			  BIT(28)
>  #define LCD_CTRL_VHSYNC_IDLE_LVL		  BIT(31)
> +#define LCD_CTRL_ALPHA_ALL			  (0xff << 6)
>  
>  /* interrupts */
>  #define LCD_INT_STATUS				(0x4 * 0x001)
> @@ -115,6 +117,7 @@
>  #define LCD_LAYER_ALPHA_EMBED			BIT(5)
>  #define LCD_LAYER_ALPHA_COMBI			(LCD_LAYER_ALPHA_STATIC | \
>  						      LCD_LAYER_ALPHA_EMBED)
> +#define LCD_LAYER_ALPHA_DISABLED		~(LCD_LAYER_ALPHA_COMBI)
>  /* RGB multiplied with alpha */
>  #define LCD_LAYER_ALPHA_PREMULT			BIT(6)
>  #define LCD_LAYER_INVERT_COL			BIT(7)
> -- 
> 2.25.1

RE: [PATCH 13/14] drm/kmb: Enable alpha blended second plane

[Chrisanthus, Anitha]

Hi Sam,
Thanks. Where should this go, drm-misc-fixes or drm-misc-next?

Anitha

> -----Original Message-----
> From: Sam Ravnborg <sam@ravnborg.org>
> Sent: Wednesday, July 28, 2021 12:29 AM
> To: Chrisanthus, Anitha <anitha.chrisanthus@intel.com>
> Cc: dri-devel@lists.freedesktop.org; Dea, Edmund J
> <edmund.j.dea@intel.com>
> Subject: Re: [PATCH 13/14] drm/kmb: Enable alpha blended second plane
> 
> Hi Anitha,
> On Tue, Jul 27, 2021 at 05:31:25PM -0700, Anitha Chrisanthus wrote:
> > From: Edmund Dea <edmund.j.dea@intel.com>
> >
> > Enable one additional plane that is alpha blended on top
> > of the primary plane.
> >
> > Signed-off-by: Edmund Dea <edmund.j.dea@intel.com>
> Your s-o-b is missing.
> 
> With this fixed:
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> > ---
> >  drivers/gpu/drm/kmb/kmb_drv.c   |  8 ++--
> >  drivers/gpu/drm/kmb/kmb_plane.c | 81 +++++++++++++++++++++++++++++-
> ---
> >  drivers/gpu/drm/kmb/kmb_plane.h |  5 +-
> >  drivers/gpu/drm/kmb/kmb_regs.h  |  3 ++
> >  4 files changed, 82 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/kmb/kmb_drv.c
> b/drivers/gpu/drm/kmb/kmb_drv.c
> > index 12f35c43d838..d0de1db03493 100644
> > --- a/drivers/gpu/drm/kmb/kmb_drv.c
> > +++ b/drivers/gpu/drm/kmb/kmb_drv.c
> > @@ -173,10 +173,10 @@ static int kmb_setup_mode_config(struct
> drm_device *drm)
> >  	ret = drmm_mode_config_init(drm);
> >  	if (ret)
> >  		return ret;
> > -	drm->mode_config.min_width = KMB_MIN_WIDTH;
> > -	drm->mode_config.min_height = KMB_MIN_HEIGHT;
> > -	drm->mode_config.max_width = KMB_MAX_WIDTH;
> > -	drm->mode_config.max_height = KMB_MAX_HEIGHT;
> > +	drm->mode_config.min_width = KMB_FB_MIN_WIDTH;
> > +	drm->mode_config.min_height = KMB_FB_MIN_HEIGHT;
> > +	drm->mode_config.max_width = KMB_FB_MAX_WIDTH;
> > +	drm->mode_config.max_height = KMB_FB_MAX_HEIGHT;
> >  	drm->mode_config.funcs = &kmb_mode_config_funcs;
> >
> >  	ret = kmb_setup_crtc(drm);
> > diff --git a/drivers/gpu/drm/kmb/kmb_plane.c
> b/drivers/gpu/drm/kmb/kmb_plane.c
> > index 4523af949ea1..cbe4e981d73e 100644
> > --- a/drivers/gpu/drm/kmb/kmb_plane.c
> > +++ b/drivers/gpu/drm/kmb/kmb_plane.c
> > @@ -118,9 +118,10 @@ static int kmb_plane_atomic_check(struct
> drm_plane *plane,
> >  	if (ret)
> >  		return ret;
> >
> > -	if (new_plane_state->crtc_w > KMB_MAX_WIDTH || new_plane_state-
> >crtc_h > KMB_MAX_HEIGHT)
> > -		return -EINVAL;
> > -	if (new_plane_state->crtc_w < KMB_MIN_WIDTH || new_plane_state-
> >crtc_h < KMB_MIN_HEIGHT)
> > +	if (new_plane_state->crtc_w > KMB_FB_MAX_WIDTH ||
> > +	    new_plane_state->crtc_h > KMB_FB_MAX_HEIGHT ||
> > +	    new_plane_state->crtc_w < KMB_FB_MIN_WIDTH ||
> > +	    new_plane_state->crtc_h < KMB_FB_MIN_HEIGHT)
> >  		return -EINVAL;
> >
> >  	/* Due to HW limitations, changing plane height or width after
> > @@ -311,6 +312,44 @@ static void config_csc(struct kmb_drm_private
> *kmb, int plane_id)
> >  	kmb_write_lcd(kmb, LCD_LAYERn_CSC_OFF3(plane_id),
> csc_coef_lcd[11]);
> >  }
> >
> > +static void kmb_plane_set_alpha(struct kmb_drm_private *kmb,
> > +				const struct drm_plane_state *state,
> > +				unsigned char plane_id,
> > +				unsigned int *val)
> > +{
> > +	u16 plane_alpha = state->alpha;
> > +	u16 pixel_blend_mode = state->pixel_blend_mode;
> > +	int has_alpha = state->fb->format->has_alpha;
> > +
> > +	if (plane_alpha != DRM_BLEND_ALPHA_OPAQUE)
> > +		*val |= LCD_LAYER_ALPHA_STATIC;
> > +
> > +	if (has_alpha) {
> > +		switch (pixel_blend_mode) {
> > +		case DRM_MODE_BLEND_PIXEL_NONE:
> > +			break;
> > +		case DRM_MODE_BLEND_PREMULTI:
> > +			*val |= LCD_LAYER_ALPHA_EMBED |
> LCD_LAYER_ALPHA_PREMULT;
> > +			break;
> > +		case DRM_MODE_BLEND_COVERAGE:
> > +			*val |= LCD_LAYER_ALPHA_EMBED;
> > +			break;
> > +		default:
> > +			DRM_DEBUG("Missing pixel blend mode case (%s ==
> %ld)\n",
> > +				  __stringify(pixel_blend_mode),
> > +				  (long)pixel_blend_mode);
> > +			break;
> > +		}
> > +	}
> > +
> > +	if (plane_alpha == DRM_BLEND_ALPHA_OPAQUE && !has_alpha) {
> > +		*val &= LCD_LAYER_ALPHA_DISABLED;
> > +		return;
> > +	}
> > +
> > +	kmb_write_lcd(kmb, LCD_LAYERn_ALPHA(plane_id), plane_alpha);
> > +}
> > +
> >  static void kmb_plane_atomic_update(struct drm_plane *plane,
> >  				    struct drm_atomic_state *state)
> >  {
> > @@ -341,11 +380,12 @@ static void kmb_plane_atomic_update(struct
> drm_plane *plane,
> >  	fb = new_plane_state->fb;
> >  	if (!fb)
> >  		return;
> > +
> >  	num_planes = fb->format->num_planes;
> >  	kmb_plane = to_kmb_plane(plane);
> > -	plane_id = kmb_plane->id;
> >
> >  	kmb = to_kmb(plane->dev);
> > +	plane_id = kmb_plane->id;
> >
> >  	spin_lock_irq(&kmb->irq_lock);
> >  	if (kmb->kmb_under_flow || kmb->kmb_flush_done) {
> > @@ -467,20 +507,32 @@ static void kmb_plane_atomic_update(struct
> drm_plane *plane,
> >  		config_csc(kmb, plane_id);
> >  	}
> >
> > +	kmb_plane_set_alpha(kmb, plane->state, plane_id, &val);
> > +
> >  	kmb_write_lcd(kmb, LCD_LAYERn_CFG(plane_id), val);
> >
> > +	/* Configure LCD_CONTROL */
> > +	ctrl = kmb_read_lcd(kmb, LCD_CONTROL);
> > +
> > +	/* Set layer blending config */
> > +	ctrl &= ~LCD_CTRL_ALPHA_ALL;
> > +	ctrl |= LCD_CTRL_ALPHA_BOTTOM_VL1 |
> > +		LCD_CTRL_ALPHA_BLEND_VL2;
> > +
> > +	ctrl &= ~LCD_CTRL_ALPHA_BLEND_BKGND_DISABLE;
> > +
> >  	switch (plane_id) {
> >  	case LAYER_0:
> > -		ctrl = LCD_CTRL_VL1_ENABLE;
> > +		ctrl |= LCD_CTRL_VL1_ENABLE;
> >  		break;
> >  	case LAYER_1:
> > -		ctrl = LCD_CTRL_VL2_ENABLE;
> > +		ctrl |= LCD_CTRL_VL2_ENABLE;
> >  		break;
> >  	case LAYER_2:
> > -		ctrl = LCD_CTRL_GL1_ENABLE;
> > +		ctrl |= LCD_CTRL_GL1_ENABLE;
> >  		break;
> >  	case LAYER_3:
> > -		ctrl = LCD_CTRL_GL2_ENABLE;
> > +		ctrl |= LCD_CTRL_GL2_ENABLE;
> >  		break;
> >  	}
> >
> > @@ -492,7 +544,7 @@ static void kmb_plane_atomic_update(struct
> drm_plane *plane,
> >  	 */
> >  	ctrl |= LCD_CTRL_VHSYNC_IDLE_LVL;
> >
> > -	kmb_set_bitmask_lcd(kmb, LCD_CONTROL, ctrl);
> > +	kmb_write_lcd(kmb, LCD_CONTROL, ctrl);
> >
> >  	/* Enable pipeline AXI read transactions for the DMA
> >  	 * after setting graphics layers. This must be done
> > @@ -567,6 +619,9 @@ struct kmb_plane *kmb_plane_init(struct
> drm_device *drm)
> >  	enum drm_plane_type plane_type;
> >  	const u32 *plane_formats;
> >  	int num_plane_formats;
> > +	unsigned int blend_caps = BIT(DRM_MODE_BLEND_PIXEL_NONE) |
> > +				  BIT(DRM_MODE_BLEND_PREMULTI)   |
> > +				  BIT(DRM_MODE_BLEND_COVERAGE);
> >
> >  	for (i = 0; i < KMB_MAX_PLANES; i++) {
> >  		plane = drmm_kzalloc(drm, sizeof(*plane), GFP_KERNEL);
> > @@ -598,8 +653,16 @@ struct kmb_plane *kmb_plane_init(struct
> drm_device *drm)
> >  		drm_dbg(drm, "%s : %d i=%d type=%d",
> >  			__func__, __LINE__,
> >  			  i, plane_type);
> > +		drm_plane_create_alpha_property(&plane->base_plane);
> > +
> > +		drm_plane_create_blend_mode_property(&plane-
> >base_plane,
> > +						     blend_caps);
> > +
> > +		drm_plane_create_zpos_immutable_property(&plane-
> >base_plane, i);
> > +
> >  		drm_plane_helper_add(&plane->base_plane,
> >  				     &kmb_plane_helper_funcs);
> > +
> >  		if (plane_type == DRM_PLANE_TYPE_PRIMARY) {
> >  			primary = plane;
> >  			kmb->plane = plane;
> > diff --git a/drivers/gpu/drm/kmb/kmb_plane.h
> b/drivers/gpu/drm/kmb/kmb_plane.h
> > index 99207b35365c..b51144044fe8 100644
> > --- a/drivers/gpu/drm/kmb/kmb_plane.h
> > +++ b/drivers/gpu/drm/kmb/kmb_plane.h
> > @@ -35,6 +35,9 @@
> >  #define POSSIBLE_CRTCS 1
> >  #define to_kmb_plane(x) container_of(x, struct kmb_plane, base_plane)
> >
> > +#define POSSIBLE_CRTCS		1
> > +#define KMB_MAX_PLANES		2
> > +
> >  enum layer_id {
> >  	LAYER_0,
> >  	LAYER_1,
> > @@ -43,8 +46,6 @@ enum layer_id {
> >  	/* KMB_MAX_PLANES */
> >  };
> >
> > -#define KMB_MAX_PLANES 1
> > -
> >  enum sub_plane_id {
> >  	Y_PLANE,
> >  	U_PLANE,
> > diff --git a/drivers/gpu/drm/kmb/kmb_regs.h
> b/drivers/gpu/drm/kmb/kmb_regs.h
> > index 48150569f702..9756101b0d32 100644
> > --- a/drivers/gpu/drm/kmb/kmb_regs.h
> > +++ b/drivers/gpu/drm/kmb/kmb_regs.h
> > @@ -43,8 +43,10 @@
> >  #define LCD_CTRL_OUTPUT_ENABLED			  BIT(19)
> >  #define LCD_CTRL_BPORCH_ENABLE			  BIT(21)
> >  #define LCD_CTRL_FPORCH_ENABLE			  BIT(22)
> > +#define LCD_CTRL_ALPHA_BLEND_BKGND_DISABLE	  BIT(23)
> >  #define LCD_CTRL_PIPELINE_DMA			  BIT(28)
> >  #define LCD_CTRL_VHSYNC_IDLE_LVL		  BIT(31)
> > +#define LCD_CTRL_ALPHA_ALL			  (0xff << 6)
> >
> >  /* interrupts */
> >  #define LCD_INT_STATUS				(0x4 * 0x001)
> > @@ -115,6 +117,7 @@
> >  #define LCD_LAYER_ALPHA_EMBED			BIT(5)
> >  #define LCD_LAYER_ALPHA_COMBI
> 	(LCD_LAYER_ALPHA_STATIC | \
> >
> LCD_LAYER_ALPHA_EMBED)
> > +#define LCD_LAYER_ALPHA_DISABLED
> 	~(LCD_LAYER_ALPHA_COMBI)
> >  /* RGB multiplied with alpha */
> >  #define LCD_LAYER_ALPHA_PREMULT			BIT(6)
> >  #define LCD_LAYER_INVERT_COL			BIT(7)
> > --
> > 2.25.1

Re: [PATCH 13/14] drm/kmb: Enable alpha blended second plane

[Sam Ravnborg]

Hi Anitha,

On Mon, Aug 02, 2021 at 08:44:26PM +0000, Chrisanthus, Anitha wrote:
> Hi Sam,
> Thanks. Where should this go, drm-misc-fixes or drm-misc-next?

Looks like a drm-misc-next candidate to me.
I may improve something for existing users, but it does not look like it
fixes an existing bug.

	Sam

Re: [PATCH 13/14] drm/kmb: Enable alpha blended second plane

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 946 bytes --]

Hi

Am 03.08.21 um 07:10 schrieb Sam Ravnborg:
> Hi Anitha,
> 
> On Mon, Aug 02, 2021 at 08:44:26PM +0000, Chrisanthus, Anitha wrote:
>> Hi Sam,
>> Thanks. Where should this go, drm-misc-fixes or drm-misc-next?
> 
> Looks like a drm-misc-next candidate to me.
> I may improve something for existing users, but it does not look like it
> fixes an existing bug.

I found this patch in drm-misc-fixes, although it doesn't look like a 
bugfix. It should have gone into drm-misc-next. See [1]. If it indeed 
belongs into drm-misc-fixes, it certainly should have contained a Fixes tag.

Best regards
Thomas

[1] 
https://drm.pages.freedesktop.org/maintainer-tools/committer-drm-misc.html#where-do-i-apply-my-patch

> 
> 	Sam
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH 13/14] drm/kmb: Enable alpha blended second plane

[Sam Ravnborg]

Hi Thomas,

On Wed, Sep 08, 2021 at 07:50:42PM +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 03.08.21 um 07:10 schrieb Sam Ravnborg:
> > Hi Anitha,
> > 
> > On Mon, Aug 02, 2021 at 08:44:26PM +0000, Chrisanthus, Anitha wrote:
> > > Hi Sam,
> > > Thanks. Where should this go, drm-misc-fixes or drm-misc-next?
> > 
> > Looks like a drm-misc-next candidate to me.
> > I may improve something for existing users, but it does not look like it
> > fixes an existing bug.
> 
> I found this patch in drm-misc-fixes, although it doesn't look like a
> bugfix. It should have gone into drm-misc-next. See [1]. If it indeed
> belongs into drm-misc-fixes, it certainly should have contained a Fixes tag.

The patch fixes some warnings that has become errors the last week.
Anitha pinged me about it, but I failed to followup. So in the end it
was applied to shut up the warning => errors.

	Sam

Re: [PATCH 13/14] drm/kmb: Enable alpha blended second plane

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 1327 bytes --]

Hi

Am 08.09.21 um 21:31 schrieb Sam Ravnborg:
> Hi Thomas,
> 
> On Wed, Sep 08, 2021 at 07:50:42PM +0200, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 03.08.21 um 07:10 schrieb Sam Ravnborg:
>>> Hi Anitha,
>>>
>>> On Mon, Aug 02, 2021 at 08:44:26PM +0000, Chrisanthus, Anitha wrote:
>>>> Hi Sam,
>>>> Thanks. Where should this go, drm-misc-fixes or drm-misc-next?
>>>
>>> Looks like a drm-misc-next candidate to me.
>>> I may improve something for existing users, but it does not look like it
>>> fixes an existing bug.
>>
>> I found this patch in drm-misc-fixes, although it doesn't look like a
>> bugfix. It should have gone into drm-misc-next. See [1]. If it indeed
>> belongs into drm-misc-fixes, it certainly should have contained a Fixes tag.
> 
> The patch fixes some warnings that has become errors the last week.
> Anitha pinged me about it, but I failed to followup. So in the end it
> was applied to shut up the warning => errors.

Thanks for reply. I cc'd Dave, as he intended to not merge the rsp PR 
this week. Maybe the patch is important enough.

Best regards
Thomas

> 
> 	Sam
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[PATCH 14/14] drm/kmb: Enable support for fbcon (framebuffer console)

[Anitha Chrisanthus]

From: Edmund Dea <edmund.j.dea@intel.com>

Enable support for fbcon (framebuffer console).
The user can initialize fbcon by loading kmb-drm with the parameter
console=1.

Signed-off-by: Edmund Dea <edmund.j.dea@intel.com>
Signed-off-by: Anitha Chrisanthus <anitha.chrisanthus@intel.com>
---
 drivers/gpu/drm/kmb/kmb_drv.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/kmb/kmb_drv.c b/drivers/gpu/drm/kmb/kmb_drv.c
index d0de1db03493..d39d004f513a 100644
--- a/drivers/gpu/drm/kmb/kmb_drv.c
+++ b/drivers/gpu/drm/kmb/kmb_drv.c
@@ -5,6 +5,7 @@
 
 #include <linux/clk.h>
 #include <linux/module.h>
+#include <linux/moduleparam.h>
 #include <linux/of_graph.h>
 #include <linux/of_platform.h>
 #include <linux/of_reserved_mem.h>
@@ -15,6 +16,7 @@
 
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_drv.h>
+#include <drm/drm_fb_helper.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_irq.h>
@@ -25,7 +27,13 @@
 #include "kmb_dsi.h"
 #include "kmb_regs.h"
 
-static int kmb_display_clk_enable(struct kmb_drm_private *kmb)
+/* Module Parameters */
+static bool console;
+module_param(console, bool, 0400);
+MODULE_PARM_DESC(console,
+		 "Enable framebuffer console support (0=disable [default], 1=on)");
+
+int kmb_display_clk_enable(struct kmb_drm_private *kmb)
 {
 	int ret = 0;
 
@@ -546,6 +554,9 @@ static int kmb_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_register;
 
+	if (console)
+		drm_fbdev_generic_setup(&kmb->drm, 32);
+
 	return 0;
 
  err_register:
-- 
2.25.1

Re: [PATCH 14/14] drm/kmb: Enable support for fbcon (framebuffer console)

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3016 bytes --]

Hi Anitha,

I love your patch! Perhaps something to improve:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-tip/drm-tip drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next linus/master v5.14-rc3 next-20210727]
[cannot apply to drm/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Anitha-Chrisanthus/drm-kmb-Enable-LCD-DMA-for-low-TVDDCV/20210728-083335
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: arm64-randconfig-r012-20210728 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project c49df15c278857adecd12db6bb1cdc96885f7079)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm64 cross compiling tool for clang build
        # apt-get install binutils-aarch64-linux-gnu
        # https://github.com/0day-ci/linux/commit/58e51349abb9f7085642cbdb5cf5853162be96fe
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Anitha-Chrisanthus/drm-kmb-Enable-LCD-DMA-for-low-TVDDCV/20210728-083335
        git checkout 58e51349abb9f7085642cbdb5cf5853162be96fe
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/kmb/kmb_drv.c:36:5: warning: no previous prototype for function 'kmb_display_clk_enable' [-Wmissing-prototypes]
   int kmb_display_clk_enable(struct kmb_drm_private *kmb)
       ^
   drivers/gpu/drm/kmb/kmb_drv.c:36:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int kmb_display_clk_enable(struct kmb_drm_private *kmb)
   ^
   static 
   1 warning generated.


vim +/kmb_display_clk_enable +36 drivers/gpu/drm/kmb/kmb_drv.c

    29	
    30	/* Module Parameters */
    31	static bool console;
    32	module_param(console, bool, 0400);
    33	MODULE_PARM_DESC(console,
    34			 "Enable framebuffer console support (0=disable [default], 1=on)");
    35	
  > 36	int kmb_display_clk_enable(struct kmb_drm_private *kmb)
    37	{
    38		int ret = 0;
    39	
    40		ret = clk_prepare_enable(kmb->kmb_clk.clk_lcd);
    41		if (ret) {
    42			drm_err(&kmb->drm, "Failed to enable LCD clock: %d\n", ret);
    43			return ret;
    44		}
    45		DRM_INFO("SUCCESS : enabled LCD clocks\n");
    46		return 0;
    47	}
    48	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 38888 bytes --]

Re: [PATCH 14/14] drm/kmb: Enable support for fbcon (framebuffer console)

[Sam Ravnborg]

Hi Anitha,

On Tue, Jul 27, 2021 at 05:31:26PM -0700, Anitha Chrisanthus wrote:
> From: Edmund Dea <edmund.j.dea@intel.com>
> 
> Enable support for fbcon (framebuffer console).
> The user can initialize fbcon by loading kmb-drm with the parameter
> console=1.

I do not see the poit of the boot parameter??
Why is it needed here but not in other drivers?

> 
> Signed-off-by: Edmund Dea <edmund.j.dea@intel.com>
> Signed-off-by: Anitha Chrisanthus <anitha.chrisanthus@intel.com>
> ---
>  drivers/gpu/drm/kmb/kmb_drv.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/kmb/kmb_drv.c b/drivers/gpu/drm/kmb/kmb_drv.c
> index d0de1db03493..d39d004f513a 100644
> --- a/drivers/gpu/drm/kmb/kmb_drv.c
> +++ b/drivers/gpu/drm/kmb/kmb_drv.c
> @@ -5,6 +5,7 @@
>  
>  #include <linux/clk.h>
>  #include <linux/module.h>
> +#include <linux/moduleparam.h>
>  #include <linux/of_graph.h>
>  #include <linux/of_platform.h>
>  #include <linux/of_reserved_mem.h>
> @@ -15,6 +16,7 @@
>  
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_drv.h>
> +#include <drm/drm_fb_helper.h>
>  #include <drm/drm_gem_cma_helper.h>
>  #include <drm/drm_gem_framebuffer_helper.h>
>  #include <drm/drm_irq.h>
> @@ -25,7 +27,13 @@
>  #include "kmb_dsi.h"
>  #include "kmb_regs.h"
>  
> -static int kmb_display_clk_enable(struct kmb_drm_private *kmb)
> +/* Module Parameters */
> +static bool console;
> +module_param(console, bool, 0400);
> +MODULE_PARM_DESC(console,
> +		 "Enable framebuffer console support (0=disable [default], 1=on)");
> +
> +int kmb_display_clk_enable(struct kmb_drm_private *kmb)
kmb_display_clk_enable lost a "static" - it will result in a warning if
you build with W=1

>  {
>  	int ret = 0;
>  
> @@ -546,6 +554,9 @@ static int kmb_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err_register;
>  
> +	if (console)
> +		drm_fbdev_generic_setup(&kmb->drm, 32);
> +
>  	return 0;
>  
>   err_register:
> -- 
> 2.25.1

RE: [PATCH 14/14] drm/kmb: Enable support for fbcon (framebuffer console)

[Chrisanthus, Anitha]

Hi Sam,
Thanks for the review.

> -----Original Message-----
> From: Sam Ravnborg <sam@ravnborg.org>
> Sent: Wednesday, July 28, 2021 12:31 AM
> To: Chrisanthus, Anitha <anitha.chrisanthus@intel.com>
> Cc: dri-devel@lists.freedesktop.org; Dea, Edmund J
> <edmund.j.dea@intel.com>
> Subject: Re: [PATCH 14/14] drm/kmb: Enable support for fbcon (framebuffer
> console)
> 
> Hi Anitha,
> 
> On Tue, Jul 27, 2021 at 05:31:26PM -0700, Anitha Chrisanthus wrote:
> > From: Edmund Dea <edmund.j.dea@intel.com>
> >
> > Enable support for fbcon (framebuffer console).
> > The user can initialize fbcon by loading kmb-drm with the parameter
> > console=1.
> 
> I do not see the poit of the boot parameter??
> Why is it needed here but not in other drivers?
By default, console is not enabled in this driver, customer only needs it when they are doing initial setups
and then want it disabled after.
> 
> >
> > Signed-off-by: Edmund Dea <edmund.j.dea@intel.com>
> > Signed-off-by: Anitha Chrisanthus <anitha.chrisanthus@intel.com>
> > ---
> >  drivers/gpu/drm/kmb/kmb_drv.c | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/kmb/kmb_drv.c
> b/drivers/gpu/drm/kmb/kmb_drv.c
> > index d0de1db03493..d39d004f513a 100644
> > --- a/drivers/gpu/drm/kmb/kmb_drv.c
> > +++ b/drivers/gpu/drm/kmb/kmb_drv.c
> > @@ -5,6 +5,7 @@
> >
> >  #include <linux/clk.h>
> >  #include <linux/module.h>
> > +#include <linux/moduleparam.h>
> >  #include <linux/of_graph.h>
> >  #include <linux/of_platform.h>
> >  #include <linux/of_reserved_mem.h>
> > @@ -15,6 +16,7 @@
> >
> >  #include <drm/drm_atomic_helper.h>
> >  #include <drm/drm_drv.h>
> > +#include <drm/drm_fb_helper.h>
> >  #include <drm/drm_gem_cma_helper.h>
> >  #include <drm/drm_gem_framebuffer_helper.h>
> >  #include <drm/drm_irq.h>
> > @@ -25,7 +27,13 @@
> >  #include "kmb_dsi.h"
> >  #include "kmb_regs.h"
> >
> > -static int kmb_display_clk_enable(struct kmb_drm_private *kmb)
> > +/* Module Parameters */
> > +static bool console;
> > +module_param(console, bool, 0400);
> > +MODULE_PARM_DESC(console,
> > +		 "Enable framebuffer console support (0=disable [default],
> 1=on)");
> > +
> > +int kmb_display_clk_enable(struct kmb_drm_private *kmb)
> kmb_display_clk_enable lost a "static" - it will result in a warning if
> you build with W=1
Will fix it in v2
> 
> >  {
> >  	int ret = 0;
> >
> > @@ -546,6 +554,9 @@ static int kmb_probe(struct platform_device *pdev)
> >  	if (ret)
> >  		goto err_register;
> >
> > +	if (console)
> > +		drm_fbdev_generic_setup(&kmb->drm, 32);
> > +
> >  	return 0;
> >
> >   err_register:
> > --
> > 2.25.1

Re: [PATCH 14/14] drm/kmb: Enable support for fbcon (framebuffer console)

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2607 bytes --]

Hi Anitha,

I love your patch! Perhaps something to improve:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next linus/master v5.14-rc3]
[cannot apply to drm-tip/drm-tip next-20210730]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Anitha-Chrisanthus/drm-kmb-Enable-LCD-DMA-for-low-TVDDCV/20210728-083335
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: parisc-randconfig-r011-20210728 (attached as .config)
compiler: hppa-linux-gcc (GCC) 10.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/58e51349abb9f7085642cbdb5cf5853162be96fe
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Anitha-Chrisanthus/drm-kmb-Enable-LCD-DMA-for-low-TVDDCV/20210728-083335
        git checkout 58e51349abb9f7085642cbdb5cf5853162be96fe
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross ARCH=parisc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/kmb/kmb_drv.c:36:5: warning: no previous prototype for 'kmb_display_clk_enable' [-Wmissing-prototypes]
      36 | int kmb_display_clk_enable(struct kmb_drm_private *kmb)
         |     ^~~~~~~~~~~~~~~~~~~~~~


vim +/kmb_display_clk_enable +36 drivers/gpu/drm/kmb/kmb_drv.c

    29	
    30	/* Module Parameters */
    31	static bool console;
    32	module_param(console, bool, 0400);
    33	MODULE_PARM_DESC(console,
    34			 "Enable framebuffer console support (0=disable [default], 1=on)");
    35	
  > 36	int kmb_display_clk_enable(struct kmb_drm_private *kmb)
    37	{
    38		int ret = 0;
    39	
    40		ret = clk_prepare_enable(kmb->kmb_clk.clk_lcd);
    41		if (ret) {
    42			drm_err(&kmb->drm, "Failed to enable LCD clock: %d\n", ret);
    43			return ret;
    44		}
    45		DRM_INFO("SUCCESS : enabled LCD clocks\n");
    46		return 0;
    47	}
    48	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 28205 bytes --]

Re: [PATCH 01/14] drm/kmb: Enable LCD DMA for low TVDDCV

[Sam Ravnborg]

Hi Anitha,
On Tue, Jul 27, 2021 at 05:31:13PM -0700, Anitha Chrisanthus wrote:
> From: Edmund Dea <edmund.j.dea@intel.com>
> 
> There's an undocumented dependency between LCD layer enable bits [2-5]
> and the AXI pipelined read enable bit [28] in the LCD_CONTROL register.
> The proper order of operation is:
> 
> 1) Clear AXI pipelined read enable bit
> 2) Set LCD layers
> 3) Set AXI pipelined read enable bit
> 
> With this update, LCD can start DMA when TVDDCV is reduced down to 700mV.
> 
> Fixes: 7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display")
> Signed-off-by: Edmund Dea <edmund.j.dea@intel.com>
Patch is missing your s-o-b.

> ---
>  drivers/gpu/drm/kmb/kmb_drv.c   | 14 ++++++++++++++
>  drivers/gpu/drm/kmb/kmb_plane.c | 15 +++++++++++++--
>  2 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/kmb/kmb_drv.c b/drivers/gpu/drm/kmb/kmb_drv.c
> index 96ea1a2c11dd..c0b1c6f99249 100644
> --- a/drivers/gpu/drm/kmb/kmb_drv.c
> +++ b/drivers/gpu/drm/kmb/kmb_drv.c
> @@ -203,6 +203,7 @@ static irqreturn_t handle_lcd_irq(struct drm_device *dev)
>  	unsigned long status, val, val1;
>  	int plane_id, dma0_state, dma1_state;
>  	struct kmb_drm_private *kmb = to_kmb(dev);
> +	u32 ctrl = 0;
>  
>  	status = kmb_read_lcd(kmb, LCD_INT_STATUS);
>  
> @@ -227,6 +228,19 @@ static irqreturn_t handle_lcd_irq(struct drm_device *dev)
>  				kmb_clr_bitmask_lcd(kmb, LCD_CONTROL,
>  						    kmb->plane_status[plane_id].ctrl);
>  
> +				ctrl = kmb_read_lcd(kmb, LCD_CONTROL);
> +				if (!(ctrl & (LCD_CTRL_VL1_ENABLE |
> +				    LCD_CTRL_VL2_ENABLE |
> +				    LCD_CTRL_GL1_ENABLE |
> +				    LCD_CTRL_GL2_ENABLE))) {
> +					/* If no LCD layers are using DMA,
> +					 * then disable DMA pipelined AXI read
> +					 * transactions.
> +					 */
> +					kmb_clr_bitmask_lcd(kmb, LCD_CONTROL,
> +							    LCD_CTRL_PIPELINE_DMA);
> +				}
> +
This function could benefit from a few helper functions to avoid all the
indent. But this is un-related to this patch.

>  				kmb->plane_status[plane_id].disable = false;
>  			}
>  		}
> diff --git a/drivers/gpu/drm/kmb/kmb_plane.c b/drivers/gpu/drm/kmb/kmb_plane.c
> index d5b6195856d1..2888dd5dcc2c 100644
> --- a/drivers/gpu/drm/kmb/kmb_plane.c
> +++ b/drivers/gpu/drm/kmb/kmb_plane.c
> @@ -427,8 +427,14 @@ static void kmb_plane_atomic_update(struct drm_plane *plane,
>  
>  	kmb_set_bitmask_lcd(kmb, LCD_CONTROL, ctrl);
>  
> -	/* FIXME no doc on how to set output format,these values are
> -	 * taken from the Myriadx tests
> +	/* Enable pipeline AXI read transactions for the DMA
> +	 * after setting graphics layers. This must be done
> +	 * in a separate write cycle.
> +	 */
> +	kmb_set_bitmask_lcd(kmb, LCD_CONTROL, LCD_CTRL_PIPELINE_DMA);
> +
> +	/* FIXME no doc on how to set output format,these values are taken
                                                    ^ add space
> +	 * from the Myriadx tests
>  	 */
>  	out_format |= LCD_OUTF_FORMAT_RGB888;
>  
> @@ -526,6 +532,11 @@ struct kmb_plane *kmb_plane_init(struct drm_device *drm)
>  		plane->id = i;
>  	}
>  
> +	/* Disable pipeline AXI read transactions for the DMA
> +	 * prior to setting graphics layers
> +	 */
> +	kmb_clr_bitmask_lcd(kmb, LCD_CONTROL, LCD_CTRL_PIPELINE_DMA);
> +
>  	return primary;
>  cleanup:
>  	drmm_kfree(drm, plane);

With the two nits fixed:
Acked-by: Sam Ravnborg <sam@ravnborg.org>

RE: [PATCH 01/14] drm/kmb: Enable LCD DMA for low TVDDCV

[Chrisanthus, Anitha]

Hi Sam,
Please help! I tried to push the first two patches to drm-misc-fixes using dim push, but it pushed other things too besides these patches. I am sorry, don't know what went wrong.

Anitha

> -----Original Message-----
> From: Sam Ravnborg <sam@ravnborg.org>
> Sent: Wednesday, July 28, 2021 12:05 AM
> To: Chrisanthus, Anitha <anitha.chrisanthus@intel.com>
> Cc: dri-devel@lists.freedesktop.org; Dea, Edmund J
> <edmund.j.dea@intel.com>
> Subject: Re: [PATCH 01/14] drm/kmb: Enable LCD DMA for low TVDDCV
> 
> Hi Anitha,
> On Tue, Jul 27, 2021 at 05:31:13PM -0700, Anitha Chrisanthus wrote:
> > From: Edmund Dea <edmund.j.dea@intel.com>
> >
> > There's an undocumented dependency between LCD layer enable bits [2-5]
> > and the AXI pipelined read enable bit [28] in the LCD_CONTROL register.
> > The proper order of operation is:
> >
> > 1) Clear AXI pipelined read enable bit
> > 2) Set LCD layers
> > 3) Set AXI pipelined read enable bit
> >
> > With this update, LCD can start DMA when TVDDCV is reduced down to
> 700mV.
> >
> > Fixes: 7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display")
> > Signed-off-by: Edmund Dea <edmund.j.dea@intel.com>
> Patch is missing your s-o-b.
> 
> > ---
> >  drivers/gpu/drm/kmb/kmb_drv.c   | 14 ++++++++++++++
> >  drivers/gpu/drm/kmb/kmb_plane.c | 15 +++++++++++++--
> >  2 files changed, 27 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/kmb/kmb_drv.c
> b/drivers/gpu/drm/kmb/kmb_drv.c
> > index 96ea1a2c11dd..c0b1c6f99249 100644
> > --- a/drivers/gpu/drm/kmb/kmb_drv.c
> > +++ b/drivers/gpu/drm/kmb/kmb_drv.c
> > @@ -203,6 +203,7 @@ static irqreturn_t handle_lcd_irq(struct drm_device
> *dev)
> >  	unsigned long status, val, val1;
> >  	int plane_id, dma0_state, dma1_state;
> >  	struct kmb_drm_private *kmb = to_kmb(dev);
> > +	u32 ctrl = 0;
> >
> >  	status = kmb_read_lcd(kmb, LCD_INT_STATUS);
> >
> > @@ -227,6 +228,19 @@ static irqreturn_t handle_lcd_irq(struct drm_device
> *dev)
> >  				kmb_clr_bitmask_lcd(kmb, LCD_CONTROL,
> >  						    kmb-
> >plane_status[plane_id].ctrl);
> >
> > +				ctrl = kmb_read_lcd(kmb, LCD_CONTROL);
> > +				if (!(ctrl & (LCD_CTRL_VL1_ENABLE |
> > +				    LCD_CTRL_VL2_ENABLE |
> > +				    LCD_CTRL_GL1_ENABLE |
> > +				    LCD_CTRL_GL2_ENABLE))) {
> > +					/* If no LCD layers are using DMA,
> > +					 * then disable DMA pipelined AXI read
> > +					 * transactions.
> > +					 */
> > +					kmb_clr_bitmask_lcd(kmb,
> LCD_CONTROL,
> > +
> LCD_CTRL_PIPELINE_DMA);
> > +				}
> > +
> This function could benefit from a few helper functions to avoid all the
> indent. But this is un-related to this patch.
> 
> >  				kmb->plane_status[plane_id].disable = false;
> >  			}
> >  		}
> > diff --git a/drivers/gpu/drm/kmb/kmb_plane.c
> b/drivers/gpu/drm/kmb/kmb_plane.c
> > index d5b6195856d1..2888dd5dcc2c 100644
> > --- a/drivers/gpu/drm/kmb/kmb_plane.c
> > +++ b/drivers/gpu/drm/kmb/kmb_plane.c
> > @@ -427,8 +427,14 @@ static void kmb_plane_atomic_update(struct
> drm_plane *plane,
> >
> >  	kmb_set_bitmask_lcd(kmb, LCD_CONTROL, ctrl);
> >
> > -	/* FIXME no doc on how to set output format,these values are
> > -	 * taken from the Myriadx tests
> > +	/* Enable pipeline AXI read transactions for the DMA
> > +	 * after setting graphics layers. This must be done
> > +	 * in a separate write cycle.
> > +	 */
> > +	kmb_set_bitmask_lcd(kmb, LCD_CONTROL,
> LCD_CTRL_PIPELINE_DMA);
> > +
> > +	/* FIXME no doc on how to set output format,these values are taken
>                                                     ^ add space
> > +	 * from the Myriadx tests
> >  	 */
> >  	out_format |= LCD_OUTF_FORMAT_RGB888;
> >
> > @@ -526,6 +532,11 @@ struct kmb_plane *kmb_plane_init(struct
> drm_device *drm)
> >  		plane->id = i;
> >  	}
> >
> > +	/* Disable pipeline AXI read transactions for the DMA
> > +	 * prior to setting graphics layers
> > +	 */
> > +	kmb_clr_bitmask_lcd(kmb, LCD_CONTROL,
> LCD_CTRL_PIPELINE_DMA);
> > +
> >  	return primary;
> >  cleanup:
> >  	drmm_kfree(drm, plane);
> 
> With the two nits fixed:
> Acked-by: Sam Ravnborg <sam@ravnborg.org>

Re: [PATCH 01/14] drm/kmb: Enable LCD DMA for low TVDDCV

[Sam Ravnborg]

Hi Anitha,

On Thu, Jul 29, 2021 at 06:48:45PM +0000, Chrisanthus, Anitha wrote:
> Hi Sam,
> Please help! I tried to push the first two patches to drm-misc-fixes using dim push, but it pushed other things too besides these patches. I am sorry, don't know what went wrong.
>

I see only these in drm-misc_fixes:

ommit eb92830cdbc232a0e8166c48061ca276132646a7 (HEAD -> drm-misc-fixes, drm-misc/for-linux-next-fixes, drm-misc/drm-misc-fixes)
Author: Edmund Dea <edmund.j.dea@intel.com>
Date:   Wed Aug 26 13:17:29 2020 -0700

    drm/kmb: Define driver date and major/minor version

    Added macros for date and version

    Fixes: 7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display")
    Signed-off-by: Edmund Dea <edmund.j.dea@intel.com>
    Signed-off-by: Anitha Chrisanthus <anitha.chrisanthus@intel.com>
    Acked-by: Sam Ravnborg <sam@ravnborg.org>
    Link: https://patchwork.freedesktop.org/patch/msgid/20210728003126.1425028-2-anitha.chrisanthus@intel.com

commit 0aab5dce395636eddf4e5f33eba88390328a95b4
Author: Edmund Dea <edmund.j.dea@intel.com>
Date:   Tue Aug 25 14:51:17 2020 -0700

    drm/kmb: Enable LCD DMA for low TVDDCV

    There's an undocumented dependency between LCD layer enable bits [2-5]
    and the AXI pipelined read enable bit [28] in the LCD_CONTROL register.
    The proper order of operation is:

    1) Clear AXI pipelined read enable bit
    2) Set LCD layers
    3) Set AXI pipelined read enable bit

    With this update, LCD can start DMA when TVDDCV is reduced down to 700mV.

    Fixes: 7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display")
    Signed-off-by: Edmund Dea <edmund.j.dea@intel.com>
    Signed-off-by: Anitha Chrisanthus <anitha.chrisanthus@intel.com>
    Acked-by: Sam Ravnborg <sam@ravnborg.org>
    Link: https://patchwork.freedesktop.org/patch/msgid/20210728003126.1425028-1-anitha.chrisanthus@intel.com


Looks OK.

	Sam