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);
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
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
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
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
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,
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__*/
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]);
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
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]);LCD_LAYERn_DMA_CB_LINE_VSTRIDE(plane_id),
width * fb->format->cpp[0]);
fb->pitches[1]);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:
- offsets is no logner needed
That's a mistake, will put back the offsets
- 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.
- 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
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
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);
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__ */
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); }
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),
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);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
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);
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
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
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);
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
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
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
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)
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;
ctrl = LCD_CTRL_VL2_ENABLE;
ctrl |= LCD_CTRL_VL2_ENABLE;
ctrl = LCD_CTRL_GL1_ENABLE;
ctrl |= LCD_CTRL_GL1_ENABLE;
ctrl = LCD_CTRL_GL2_ENABLE;
ctrl |= LCD_CTRL_GL2_ENABLE;@@ -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
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;
ctrl = LCD_CTRL_VL2_ENABLE;
ctrl |= LCD_CTRL_VL2_ENABLE;
ctrl = LCD_CTRL_GL1_ENABLE;
ctrl |= LCD_CTRL_GL1_ENABLE;
ctrl = LCD_CTRL_GL2_ENABLE;
ctrl |= LCD_CTRL_GL2_ENABLE;@@ -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
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
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#w...
Sam
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
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
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:
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
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
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:
- Clear AXI pipelined read enable bit
- Set LCD layers
- 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@@ -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
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:
- Clear AXI pipelined read enable bit
- Set LCD layers
- 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@@ -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
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-anith...
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-anith...
Looks OK.
Sam
dri-devel@lists.freedesktop.org
-
Anitha Chrisanthus -
Chrisanthus, Anitha
-
Dan Carpenter -
Sam Ravnborg -
Thomas Zimmermann