linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] Add Aspeed AST2600 soc display support
@ 2021-11-01 11:01 tommy-huang
  2021-11-01 11:01 ` [PATCH 1/4] ARM: dts: aspeed: Add GFX node to AST2600 tommy-huang
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: tommy-huang @ 2021-11-01 11:01 UTC (permalink / raw)
  To: joel, airlied, daniel, robh+dt, andrew, linux-aspeed, dri-devel,
	devicetree, linux-arm-kernel, linux-kernel
  Cc: BMC-SW

The Aspeed AST2600 soc diaplay support is added in this patch.
Because some hw designed is changed in this version, add them 
in this patch.

v2:
  Remove some unnecessary patch.
  Refine for reviwer request.

v1:
  First add patch.

Joel Stanley (2):
  ARM: dts: aspeed: Add GFX node to AST2600
  ARM: dts: aspeed: ast2600-evb: Enable GFX device

tommy-huang (2):
  drm/aspeed: Update INTR_STS handling
  dt-bindings: gpu: Add ASPEED GFX bindings document

 .../devicetree/bindings/gpu/aspeed-gfx.txt    |  1 +
 arch/arm/boot/dts/aspeed-ast2600-evb.dts      | 18 +++++++++++++
 arch/arm/boot/dts/aspeed-g6.dtsi              | 11 ++++++++
 drivers/gpu/drm/aspeed/aspeed_gfx.h           |  2 ++
 drivers/gpu/drm/aspeed/aspeed_gfx_drv.c       | 26 ++++++++++++++++---
 5 files changed, 55 insertions(+), 3 deletions(-)

-- 
2.17.1


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

* [PATCH 1/4] ARM: dts: aspeed: Add GFX node to AST2600
  2021-11-01 11:01 [PATCH 0/4] Add Aspeed AST2600 soc display support tommy-huang
@ 2021-11-01 11:01 ` tommy-huang
  2021-11-01 11:01 ` [PATCH 2/4] ARM: dts: aspeed: ast2600-evb: Enable GFX device tommy-huang
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: tommy-huang @ 2021-11-01 11:01 UTC (permalink / raw)
  To: joel, airlied, daniel, robh+dt, andrew, linux-aspeed, dri-devel,
	devicetree, linux-arm-kernel, linux-kernel
  Cc: BMC-SW

From: Joel Stanley <joel@jms.id.au>

The GFX device is present in the AST2600 SoC.

Signed-off-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: tommy-huang <tommy_huang@aspeedtech.com>
---
 arch/arm/boot/dts/aspeed-g6.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed-g6.dtsi
index 1b47be1704f8..e38c3742761b 100644
--- a/arch/arm/boot/dts/aspeed-g6.dtsi
+++ b/arch/arm/boot/dts/aspeed-g6.dtsi
@@ -351,6 +351,17 @@
 				quality = <100>;
 			};
 
+			gfx: display@1e6e6000 {
+				compatible = "aspeed,ast2600-gfx", "aspeed,ast2500-gfx", "syscon";
+				reg = <0x1e6e6000 0x1000>;
+				reg-io-width = <4>;
+				clocks = <&syscon ASPEED_CLK_GATE_D1CLK>;
+				resets = <&syscon ASPEED_RESET_GRAPHICS>;
+				syscon = <&syscon>;
+				status = "disabled";
+				interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>;
+			};
+
 			xdma: xdma@1e6e7000 {
 				compatible = "aspeed,ast2600-xdma";
 				reg = <0x1e6e7000 0x100>;
-- 
2.17.1


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

* [PATCH 2/4] ARM: dts: aspeed: ast2600-evb: Enable GFX device
  2021-11-01 11:01 [PATCH 0/4] Add Aspeed AST2600 soc display support tommy-huang
  2021-11-01 11:01 ` [PATCH 1/4] ARM: dts: aspeed: Add GFX node to AST2600 tommy-huang
@ 2021-11-01 11:01 ` tommy-huang
  2021-11-01 11:01 ` [PATCH 3/4] drm/aspeed: Update INTR_STS handling tommy-huang
  2021-11-01 11:01 ` [PATCH 4/4] dt-bindings: gpu: Add ASPEED GFX bindings document tommy-huang
  3 siblings, 0 replies; 9+ messages in thread
From: tommy-huang @ 2021-11-01 11:01 UTC (permalink / raw)
  To: joel, airlied, daniel, robh+dt, andrew, linux-aspeed, dri-devel,
	devicetree, linux-arm-kernel, linux-kernel
  Cc: BMC-SW

From: Joel Stanley <joel@jms.id.au>

Enable the GFX device with a framebuffer memory region.

Signed-off-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: tommy-huang <tommy_huang@aspeedtech.com>
---
 arch/arm/boot/dts/aspeed-ast2600-evb.dts | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-ast2600-evb.dts b/arch/arm/boot/dts/aspeed-ast2600-evb.dts
index b7eb552640cb..e223dad2abd0 100644
--- a/arch/arm/boot/dts/aspeed-ast2600-evb.dts
+++ b/arch/arm/boot/dts/aspeed-ast2600-evb.dts
@@ -23,6 +23,19 @@
 		reg = <0x80000000 0x80000000>;
 	};
 
+	reserved-memory {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		gfx_memory: framebuffer {
+			size = <0x01000000>;
+			alignment = <0x01000000>;
+			compatible = "shared-dma-pool";
+			reusable;
+		};
+	};
+
 	vcc_sdhci0: regulator-vcc-sdhci0 {
 		compatible = "regulator-fixed";
 		regulator-name = "SDHCI0 Vcc";
@@ -300,3 +313,8 @@
 	vqmmc-supply = <&vccq_sdhci1>;
 	clk-phase-sd-hs = <7>, <200>;
 };
+
+&gfx {
+	status = "okay";
+	memory-region = <&gfx_memory>;
+};
-- 
2.17.1


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

* [PATCH 3/4]     drm/aspeed: Update INTR_STS handling
  2021-11-01 11:01 [PATCH 0/4] Add Aspeed AST2600 soc display support tommy-huang
  2021-11-01 11:01 ` [PATCH 1/4] ARM: dts: aspeed: Add GFX node to AST2600 tommy-huang
  2021-11-01 11:01 ` [PATCH 2/4] ARM: dts: aspeed: ast2600-evb: Enable GFX device tommy-huang
@ 2021-11-01 11:01 ` tommy-huang
  2021-11-16  6:18   ` Joel Stanley
  2021-11-01 11:01 ` [PATCH 4/4] dt-bindings: gpu: Add ASPEED GFX bindings document tommy-huang
  3 siblings, 1 reply; 9+ messages in thread
From: tommy-huang @ 2021-11-01 11:01 UTC (permalink / raw)
  To: joel, airlied, daniel, robh+dt, andrew, linux-aspeed, dri-devel,
	devicetree, linux-arm-kernel, linux-kernel
  Cc: BMC-SW

    The V-sync INTR_STS is differnet on AST2600.
    Change into general rule to handle it.

Signed-off-by: tommy-huang <tommy_huang@aspeedtech.com>
---
 drivers/gpu/drm/aspeed/aspeed_gfx.h     |  2 ++
 drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 26 ++++++++++++++++++++++---
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx.h b/drivers/gpu/drm/aspeed/aspeed_gfx.h
index 96501152bafa..5eed9275bce7 100644
--- a/drivers/gpu/drm/aspeed/aspeed_gfx.h
+++ b/drivers/gpu/drm/aspeed/aspeed_gfx.h
@@ -12,6 +12,8 @@ struct aspeed_gfx {
 	struct regmap			*scu;
 
 	u32				dac_reg;
+	u32				int_reg;
+	u32				int_clr_reg;
 	u32				vga_scratch_reg;
 	u32				throd_val;
 	u32				scan_line_max;
diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
index b53fee6f1c17..1092060cb59c 100644
--- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
+++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
@@ -60,6 +60,8 @@
 
 struct aspeed_gfx_config {
 	u32 dac_reg;		/* DAC register in SCU */
+	u32 int_status_reg;	/* Interrupt status register */
+	u32 int_clear_reg;	/* Interrupt clear register */
 	u32 vga_scratch_reg;	/* VGA scratch register in SCU */
 	u32 throd_val;		/* Default Threshold Seting */
 	u32 scan_line_max;	/* Max memory size of one scan line */
@@ -67,6 +69,8 @@ struct aspeed_gfx_config {
 
 static const struct aspeed_gfx_config ast2400_config = {
 	.dac_reg = 0x2c,
+	.int_status_reg = 0x60,
+	.int_clear_reg = 0x60,
 	.vga_scratch_reg = 0x50,
 	.throd_val = CRT_THROD_LOW(0x1e) | CRT_THROD_HIGH(0x12),
 	.scan_line_max = 64,
@@ -74,14 +78,26 @@ static const struct aspeed_gfx_config ast2400_config = {
 
 static const struct aspeed_gfx_config ast2500_config = {
 	.dac_reg = 0x2c,
+	.int_status_reg = 0x60,
+	.int_clear_reg = 0x60,
 	.vga_scratch_reg = 0x50,
 	.throd_val = CRT_THROD_LOW(0x24) | CRT_THROD_HIGH(0x3c),
 	.scan_line_max = 128,
 };
 
+static const struct aspeed_gfx_config ast2600_config = {
+	.dac_reg = 0xc0,
+	.int_status_reg = 0x60,
+	.int_clear_reg = 0x68,
+	.vga_scratch_reg = 0x50,
+	.throd_val = CRT_THROD_LOW(0x50) | CRT_THROD_HIGH(0x70),
+	.scan_line_max = 128,
+};
+
 static const struct of_device_id aspeed_gfx_match[] = {
 	{ .compatible = "aspeed,ast2400-gfx", .data = &ast2400_config },
 	{ .compatible = "aspeed,ast2500-gfx", .data = &ast2500_config },
+	{ .compatible = "aspeed,ast2600-gfx", .data = &ast2600_config },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, aspeed_gfx_match);
@@ -113,13 +129,15 @@ static irqreturn_t aspeed_gfx_irq_handler(int irq, void *data)
 {
 	struct drm_device *drm = data;
 	struct aspeed_gfx *priv = to_aspeed_gfx(drm);
-	u32 reg;
+	u32 reg, clr_reg;
 
-	reg = readl(priv->base + CRT_CTRL1);
+	reg = readl(priv->base + priv->int_reg);
 
 	if (reg & CRT_CTRL_VERTICAL_INTR_STS) {
 		drm_crtc_handle_vblank(&priv->pipe.crtc);
-		writel(reg, priv->base + CRT_CTRL1);
+		clr_reg = (readl(priv->base + priv->int_clr_reg) |
+			CRT_CTRL_VERTICAL_INTR_STS);
+		writel(clr_reg, priv->base + priv->int_clr_reg);
 		return IRQ_HANDLED;
 	}
 
@@ -147,6 +165,8 @@ static int aspeed_gfx_load(struct drm_device *drm)
 	config = match->data;
 
 	priv->dac_reg = config->dac_reg;
+	priv->int_reg = config->int_status_reg;
+	priv->int_clr_reg = config->int_clear_reg;
 	priv->vga_scratch_reg = config->vga_scratch_reg;
 	priv->throd_val = config->throd_val;
 	priv->scan_line_max = config->scan_line_max;
-- 
2.17.1


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

* [PATCH 4/4] dt-bindings: gpu: Add ASPEED GFX bindings document
  2021-11-01 11:01 [PATCH 0/4] Add Aspeed AST2600 soc display support tommy-huang
                   ` (2 preceding siblings ...)
  2021-11-01 11:01 ` [PATCH 3/4] drm/aspeed: Update INTR_STS handling tommy-huang
@ 2021-11-01 11:01 ` tommy-huang
  2021-11-01 20:01   ` Rob Herring
  3 siblings, 1 reply; 9+ messages in thread
From: tommy-huang @ 2021-11-01 11:01 UTC (permalink / raw)
  To: joel, airlied, daniel, robh+dt, andrew, linux-aspeed, dri-devel,
	devicetree, linux-arm-kernel, linux-kernel
  Cc: BMC-SW

Add ast2600-gfx description for gfx driver.

Signed-off-by: tommy-huang <tommy_huang@aspeedtech.com>
---
 Documentation/devicetree/bindings/gpu/aspeed-gfx.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/gpu/aspeed-gfx.txt b/Documentation/devicetree/bindings/gpu/aspeed-gfx.txt
index 958bdf962339..29ecf119cef2 100644
--- a/Documentation/devicetree/bindings/gpu/aspeed-gfx.txt
+++ b/Documentation/devicetree/bindings/gpu/aspeed-gfx.txt
@@ -3,6 +3,7 @@ Device tree configuration for the GFX display device on the ASPEED SoCs
 Required properties:
   - compatible
     * Must be one of the following:
+      + aspeed,ast2600-gfx
       + aspeed,ast2500-gfx
       + aspeed,ast2400-gfx
     * In addition, the ASPEED pinctrl bindings require the 'syscon' property to
-- 
2.17.1


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

* Re: [PATCH 4/4] dt-bindings: gpu: Add ASPEED GFX bindings document
  2021-11-01 11:01 ` [PATCH 4/4] dt-bindings: gpu: Add ASPEED GFX bindings document tommy-huang
@ 2021-11-01 20:01   ` Rob Herring
  2021-11-02  0:20     ` Tommy Huang
  0 siblings, 1 reply; 9+ messages in thread
From: Rob Herring @ 2021-11-01 20:01 UTC (permalink / raw)
  To: tommy-huang
  Cc: dri-devel, linux-arm-kernel, linux-aspeed, daniel, BMC-SW,
	devicetree, linux-kernel, robh+dt, joel, andrew, airlied

On Mon, 01 Nov 2021 19:01:07 +0800, tommy-huang wrote:
> Add ast2600-gfx description for gfx driver.
> 
> Signed-off-by: tommy-huang <tommy_huang@aspeedtech.com>
> ---
>  Documentation/devicetree/bindings/gpu/aspeed-gfx.txt | 1 +
>  1 file changed, 1 insertion(+)
> 


Please add Acked-by/Reviewed-by tags when posting new versions. However,
there's no need to repost patches *only* to add the tags. The upstream
maintainer will do that for acks received on the version they apply.

If a tag was not added on purpose, please state why and what changed.


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

* RE: [PATCH 4/4] dt-bindings: gpu: Add ASPEED GFX bindings document
  2021-11-01 20:01   ` Rob Herring
@ 2021-11-02  0:20     ` Tommy Huang
  0 siblings, 0 replies; 9+ messages in thread
From: Tommy Huang @ 2021-11-02  0:20 UTC (permalink / raw)
  To: Rob Herring
  Cc: dri-devel, linux-arm-kernel, linux-aspeed, daniel, BMC-SW,
	devicetree, linux-kernel, robh+dt, joel, andrew, airlied

Hi Rob,

	Thanks for your comments.
	I got it.

	By Tommy

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Tuesday, November 2, 2021 4:02 AM
> To: Tommy Huang <tommy_huang@aspeedtech.com>
> Cc: dri-devel@lists.freedesktop.org; linux-arm-kernel@lists.infradead.org;
> linux-aspeed@lists.ozlabs.org; daniel@ffwll.ch; BMC-SW
> <BMC-SW@aspeedtech.com>; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org; robh+dt@kernel.org; joel@jms.id.au;
> andrew@aj.id.au; airlied@linux.ie
> Subject: Re: [PATCH 4/4] dt-bindings: gpu: Add ASPEED GFX bindings document
> 
> On Mon, 01 Nov 2021 19:01:07 +0800, tommy-huang wrote:
> > Add ast2600-gfx description for gfx driver.
> >
> > Signed-off-by: tommy-huang <tommy_huang@aspeedtech.com>
> > ---
> >  Documentation/devicetree/bindings/gpu/aspeed-gfx.txt | 1 +
> >  1 file changed, 1 insertion(+)
> >
> 
> 
> Please add Acked-by/Reviewed-by tags when posting new versions. However,
> there's no need to repost patches *only* to add the tags. The upstream
> maintainer will do that for acks received on the version they apply.
> 
> If a tag was not added on purpose, please state why and what changed.


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

* Re: [PATCH 3/4] drm/aspeed: Update INTR_STS handling
  2021-11-01 11:01 ` [PATCH 3/4] drm/aspeed: Update INTR_STS handling tommy-huang
@ 2021-11-16  6:18   ` Joel Stanley
  2021-11-16  7:18     ` Tommy Huang
  0 siblings, 1 reply; 9+ messages in thread
From: Joel Stanley @ 2021-11-16  6:18 UTC (permalink / raw)
  To: tommy-huang
  Cc: David Airlie, Daniel Vetter, Rob Herring, Andrew Jeffery,
	linux-aspeed, open list:DRM DRIVERS, devicetree, Linux ARM,
	Linux Kernel Mailing List, BMC-SW

On Mon, 1 Nov 2021 at 11:01, tommy-huang <tommy_huang@aspeedtech.com> wrote:
>
>     The V-sync INTR_STS is differnet on AST2600.
>     Change into general rule to handle it.
>
> Signed-off-by: tommy-huang <tommy_huang@aspeedtech.com>
> ---
>  drivers/gpu/drm/aspeed/aspeed_gfx.h     |  2 ++
>  drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 26 ++++++++++++++++++++++---
>  2 files changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx.h b/drivers/gpu/drm/aspeed/aspeed_gfx.h
> index 96501152bafa..5eed9275bce7 100644
> --- a/drivers/gpu/drm/aspeed/aspeed_gfx.h
> +++ b/drivers/gpu/drm/aspeed/aspeed_gfx.h
> @@ -12,6 +12,8 @@ struct aspeed_gfx {
>         struct regmap                   *scu;
>
>         u32                             dac_reg;
> +       u32                             int_reg;
> +       u32                             int_clr_reg;
>         u32                             vga_scratch_reg;
>         u32                             throd_val;
>         u32                             scan_line_max;
> diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
> index b53fee6f1c17..1092060cb59c 100644
> --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
> +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
> @@ -60,6 +60,8 @@
>
>  struct aspeed_gfx_config {
>         u32 dac_reg;            /* DAC register in SCU */
> +       u32 int_status_reg;     /* Interrupt status register */

This is the same for all supported chips; do you need to introduce the
variable for it?

> +       u32 int_clear_reg;      /* Interrupt clear register */
>         u32 vga_scratch_reg;    /* VGA scratch register in SCU */
>         u32 throd_val;          /* Default Threshold Seting */
>         u32 scan_line_max;      /* Max memory size of one scan line */
> @@ -67,6 +69,8 @@ struct aspeed_gfx_config {
>
>  static const struct aspeed_gfx_config ast2400_config = {
>         .dac_reg = 0x2c,
> +       .int_status_reg = 0x60,
> +       .int_clear_reg = 0x60,
>         .vga_scratch_reg = 0x50,
>         .throd_val = CRT_THROD_LOW(0x1e) | CRT_THROD_HIGH(0x12),
>         .scan_line_max = 64,
> @@ -74,14 +78,26 @@ static const struct aspeed_gfx_config ast2400_config = {
>
>  static const struct aspeed_gfx_config ast2500_config = {
>         .dac_reg = 0x2c,
> +       .int_status_reg = 0x60,
> +       .int_clear_reg = 0x60,
>         .vga_scratch_reg = 0x50,
>         .throd_val = CRT_THROD_LOW(0x24) | CRT_THROD_HIGH(0x3c),
>         .scan_line_max = 128,
>  };
>
> +static const struct aspeed_gfx_config ast2600_config = {
> +       .dac_reg = 0xc0,
> +       .int_status_reg = 0x60,
> +       .int_clear_reg = 0x68,
> +       .vga_scratch_reg = 0x50,
> +       .throd_val = CRT_THROD_LOW(0x50) | CRT_THROD_HIGH(0x70),
> +       .scan_line_max = 128,
> +};

This patch combines adding the clear_reg functionality with support
for the 2600. Can you split them out; add int_clear_reg first, and
then add support for the 2600 in the following patch?


> +
>  static const struct of_device_id aspeed_gfx_match[] = {
>         { .compatible = "aspeed,ast2400-gfx", .data = &ast2400_config },
>         { .compatible = "aspeed,ast2500-gfx", .data = &ast2500_config },
> +       { .compatible = "aspeed,ast2600-gfx", .data = &ast2600_config },
>         { },
>  };
>  MODULE_DEVICE_TABLE(of, aspeed_gfx_match);
> @@ -113,13 +129,15 @@ static irqreturn_t aspeed_gfx_irq_handler(int irq, void *data)
>  {
>         struct drm_device *drm = data;
>         struct aspeed_gfx *priv = to_aspeed_gfx(drm);
> -       u32 reg;
> +       u32 reg, clr_reg;
>
> -       reg = readl(priv->base + CRT_CTRL1);
> +       reg = readl(priv->base + priv->int_reg);
>
>         if (reg & CRT_CTRL_VERTICAL_INTR_STS) {
>                 drm_crtc_handle_vblank(&priv->pipe.crtc);
> -               writel(reg, priv->base + CRT_CTRL1);
> +               clr_reg = (readl(priv->base + priv->int_clr_reg) |
> +                       CRT_CTRL_VERTICAL_INTR_STS);
> +               writel(clr_reg, priv->base + priv->int_clr_reg);
You do not need to do read-modify-write.

On the 2500 you're writing back the value you read. On the 2600 all of
the bits are read only except the INTR_STS bit, which is W1C, so you
can use the value you read from CRT_CTRL1, so this should be enough:

 writel(reg, priv->base + priv->int_clr_reg);

>                 return IRQ_HANDLED;
>         }
>
> @@ -147,6 +165,8 @@ static int aspeed_gfx_load(struct drm_device *drm)
>         config = match->data;
>
>         priv->dac_reg = config->dac_reg;
> +       priv->int_reg = config->int_status_reg;
> +       priv->int_clr_reg = config->int_clear_reg;
>         priv->vga_scratch_reg = config->vga_scratch_reg;
>         priv->throd_val = config->throd_val;
>         priv->scan_line_max = config->scan_line_max;
> --
> 2.17.1
>

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

* RE: [PATCH 3/4] drm/aspeed: Update INTR_STS handling
  2021-11-16  6:18   ` Joel Stanley
@ 2021-11-16  7:18     ` Tommy Huang
  0 siblings, 0 replies; 9+ messages in thread
From: Tommy Huang @ 2021-11-16  7:18 UTC (permalink / raw)
  To: Joel Stanley
  Cc: David Airlie, Daniel Vetter, Rob Herring, Andrew Jeffery,
	linux-aspeed, open list:DRM DRIVERS, devicetree, Linux ARM,
	Linux Kernel Mailing List, BMC-SW

Hi Joel,

	Thanks for your comment.
	I will modify the patch and send it again.

> -----Original Message-----
> From: Joel Stanley <joel@jms.id.au>
> Sent: Tuesday, November 16, 2021 2:19 PM
> To: Tommy Huang <tommy_huang@aspeedtech.com>
> Cc: David Airlie <airlied@linux.ie>; Daniel Vetter <daniel@ffwll.ch>; Rob
> Herring <robh+dt@kernel.org>; Andrew Jeffery <andrew@aj.id.au>;
> linux-aspeed <linux-aspeed@lists.ozlabs.org>; open list:DRM DRIVERS
> <dri-devel@lists.freedesktop.org>; devicetree <devicetree@vger.kernel.org>;
> Linux ARM <linux-arm-kernel@lists.infradead.org>; Linux Kernel Mailing List
> <linux-kernel@vger.kernel.org>; BMC-SW <BMC-SW@aspeedtech.com>
> Subject: Re: [PATCH 3/4] drm/aspeed: Update INTR_STS handling
> 
> On Mon, 1 Nov 2021 at 11:01, tommy-huang
> <tommy_huang@aspeedtech.com> wrote:
> >
> >     The V-sync INTR_STS is differnet on AST2600.
> >     Change into general rule to handle it.
> >
> > Signed-off-by: tommy-huang <tommy_huang@aspeedtech.com>
> > ---
> >  drivers/gpu/drm/aspeed/aspeed_gfx.h     |  2 ++
> >  drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 26
> > ++++++++++++++++++++++---
> >  2 files changed, 25 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx.h
> > b/drivers/gpu/drm/aspeed/aspeed_gfx.h
> > index 96501152bafa..5eed9275bce7 100644
> > --- a/drivers/gpu/drm/aspeed/aspeed_gfx.h
> > +++ b/drivers/gpu/drm/aspeed/aspeed_gfx.h
> > @@ -12,6 +12,8 @@ struct aspeed_gfx {
> >         struct regmap                   *scu;
> >
> >         u32                             dac_reg;
> > +       u32                             int_reg;
> > +       u32                             int_clr_reg;
> >         u32                             vga_scratch_reg;
> >         u32                             throd_val;
> >         u32                             scan_line_max;
> > diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
> > b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
> > index b53fee6f1c17..1092060cb59c 100644
> > --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
> > +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
> > @@ -60,6 +60,8 @@
> >
> >  struct aspeed_gfx_config {
> >         u32 dac_reg;            /* DAC register in SCU */
> > +       u32 int_status_reg;     /* Interrupt status register */
> 
> This is the same for all supported chips; do you need to introduce the variable
> for it?
> 
> > +       u32 int_clear_reg;      /* Interrupt clear register */
> >         u32 vga_scratch_reg;    /* VGA scratch register in SCU */
> >         u32 throd_val;          /* Default Threshold Seting */
> >         u32 scan_line_max;      /* Max memory size of one scan line */
> > @@ -67,6 +69,8 @@ struct aspeed_gfx_config {
> >
> >  static const struct aspeed_gfx_config ast2400_config = {
> >         .dac_reg = 0x2c,
> > +       .int_status_reg = 0x60,
> > +       .int_clear_reg = 0x60,
> >         .vga_scratch_reg = 0x50,
> >         .throd_val = CRT_THROD_LOW(0x1e) | CRT_THROD_HIGH(0x12),
> >         .scan_line_max = 64,
> > @@ -74,14 +78,26 @@ static const struct aspeed_gfx_config
> > ast2400_config = {
> >
> >  static const struct aspeed_gfx_config ast2500_config = {
> >         .dac_reg = 0x2c,
> > +       .int_status_reg = 0x60,
> > +       .int_clear_reg = 0x60,
> >         .vga_scratch_reg = 0x50,
> >         .throd_val = CRT_THROD_LOW(0x24) | CRT_THROD_HIGH(0x3c),
> >         .scan_line_max = 128,
> >  };
> >
> > +static const struct aspeed_gfx_config ast2600_config = {
> > +       .dac_reg = 0xc0,
> > +       .int_status_reg = 0x60,
> > +       .int_clear_reg = 0x68,
> > +       .vga_scratch_reg = 0x50,
> > +       .throd_val = CRT_THROD_LOW(0x50) | CRT_THROD_HIGH(0x70),
> > +       .scan_line_max = 128,
> > +};
> 
> This patch combines adding the clear_reg functionality with support for the
> 2600. Can you split them out; add int_clear_reg first, and then add support for
> the 2600 in the following patch?
> 
> 
> > +
> >  static const struct of_device_id aspeed_gfx_match[] = {
> >         { .compatible = "aspeed,ast2400-gfx", .data = &ast2400_config },
> >         { .compatible = "aspeed,ast2500-gfx", .data = &ast2500_config
> > },
> > +       { .compatible = "aspeed,ast2600-gfx", .data = &ast2600_config
> > + },
> >         { },
> >  };
> >  MODULE_DEVICE_TABLE(of, aspeed_gfx_match); @@ -113,13 +129,15 @@
> > static irqreturn_t aspeed_gfx_irq_handler(int irq, void *data)  {
> >         struct drm_device *drm = data;
> >         struct aspeed_gfx *priv = to_aspeed_gfx(drm);
> > -       u32 reg;
> > +       u32 reg, clr_reg;
> >
> > -       reg = readl(priv->base + CRT_CTRL1);
> > +       reg = readl(priv->base + priv->int_reg);
> >
> >         if (reg & CRT_CTRL_VERTICAL_INTR_STS) {
> >                 drm_crtc_handle_vblank(&priv->pipe.crtc);
> > -               writel(reg, priv->base + CRT_CTRL1);
> > +               clr_reg = (readl(priv->base + priv->int_clr_reg) |
> > +                       CRT_CTRL_VERTICAL_INTR_STS);
> > +               writel(clr_reg, priv->base + priv->int_clr_reg);
> You do not need to do read-modify-write.
> 
> On the 2500 you're writing back the value you read. On the 2600 all of the bits
> are read only except the INTR_STS bit, which is W1C, so you can use the value
> you read from CRT_CTRL1, so this should be enough:
> 
>  writel(reg, priv->base + priv->int_clr_reg);
> 
> >                 return IRQ_HANDLED;
> >         }
> >
> > @@ -147,6 +165,8 @@ static int aspeed_gfx_load(struct drm_device *drm)
> >         config = match->data;
> >
> >         priv->dac_reg = config->dac_reg;
> > +       priv->int_reg = config->int_status_reg;
> > +       priv->int_clr_reg = config->int_clear_reg;
> >         priv->vga_scratch_reg = config->vga_scratch_reg;
> >         priv->throd_val = config->throd_val;
> >         priv->scan_line_max = config->scan_line_max;
> > --
> > 2.17.1
> >

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

end of thread, other threads:[~2021-11-16  7:19 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-01 11:01 [PATCH 0/4] Add Aspeed AST2600 soc display support tommy-huang
2021-11-01 11:01 ` [PATCH 1/4] ARM: dts: aspeed: Add GFX node to AST2600 tommy-huang
2021-11-01 11:01 ` [PATCH 2/4] ARM: dts: aspeed: ast2600-evb: Enable GFX device tommy-huang
2021-11-01 11:01 ` [PATCH 3/4] drm/aspeed: Update INTR_STS handling tommy-huang
2021-11-16  6:18   ` Joel Stanley
2021-11-16  7:18     ` Tommy Huang
2021-11-01 11:01 ` [PATCH 4/4] dt-bindings: gpu: Add ASPEED GFX bindings document tommy-huang
2021-11-01 20:01   ` Rob Herring
2021-11-02  0:20     ` Tommy Huang

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