diff mbox series

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

Message ID 20210728003126.1425028-1-anitha.chrisanthus@intel.com (mailing list archive)
State New, archived
Headers show
Series [01/14] drm/kmb: Enable LCD DMA for low TVDDCV | expand

Commit Message

Chrisanthus, Anitha July 28, 2021, 12:31 a.m. UTC 
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(-)

Comments

Sam Ravnborg July 28, 2021, 7:04 a.m. UTC | #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:
> 
> 1) Clear AXI pipelined read enable bit
> 2) Set LCD layers
> 3) Set AXI pipelined read enable bit
> 
> With this update, LCD can start DMA when TVDDCV is reduced down to 700mV.
> 
> Fixes: 7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display")
> Signed-off-by: Edmund Dea <edmund.j.dea@intel.com>
Patch is missing your s-o-b.

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

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

With the two nits fixed:
Acked-by: Sam Ravnborg <sam@ravnborg.org>
Chrisanthus, Anitha July 29, 2021, 6:48 p.m. UTC | #2 
Hi Sam,
Please help! I tried to push the first two patches to drm-misc-fixes using dim push, but it pushed other things too besides these patches. I am sorry, don't know what went wrong.

Anitha

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

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

I see only these in drm-misc_fixes:

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

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

    Added macros for date and version

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

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

    drm/kmb: Enable LCD DMA for low TVDDCV

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

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

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

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


Looks OK.

	Sam
diff mbox series

Patch

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);