diff mbox series

[v3,RESEND,21/24] drm/exynos/decon5433: add local path support

Message ID 20190325071349.22600-22-a.hajda@samsung.com (mailing list archive)
State Not Applicable
Headers show
Series drm/exynos: add support for GSCALER planes on Exynos5433 | expand

Commit Message

Andrzej Hajda March 25, 2019, 7:13 a.m. UTC
GSCALERs in Exynos5433 have local path to DECON and DECON_TV.
They can be used as extra planes with support for non-RGB formats and scaling.
To enable it on DECON update_plane and disable_plane callback should
be modified. Moreover DSD mux should be set accordingly, and finally
atomic_check callback should be used to limit the number of active planes.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 80 +++++++++++++++----
 drivers/gpu/drm/exynos/regs-decon5433.h       |  6 ++
 2 files changed, 72 insertions(+), 14 deletions(-)

Comments

Krzysztof Kozlowski Feb. 6, 2022, 4:51 p.m. UTC | #1
On 25/03/2019 08:13, Andrzej Hajda wrote:
> GSCALERs in Exynos5433 have local path to DECON and DECON_TV.
> They can be used as extra planes with support for non-RGB formats and scaling.
> To enable it on DECON update_plane and disable_plane callback should
> be modified. Moreover DSD mux should be set accordingly, and finally
> atomic_check callback should be used to limit the number of active planes.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 80 +++++++++++++++----
>  drivers/gpu/drm/exynos/regs-decon5433.h       |  6 ++
>  2 files changed, 72 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ex
Hi guys!

I am working on DRM bindings conversion to DT schema format and I found
this set only partially applied. I merged the DTS patches ("dsd" clock),
but I think the driver and bindings were not picked up.

Nevertheless I am going to include the "dsd" clock in the new bindings,
so the DTS passes DT schema checks. Let me know if other approach
(revert of DTS change) should be taken.

Best regards,
Krzysztof
Inki Dae March 2, 2022, 1 a.m. UTC | #2
Hi Krzysztof,

22. 2. 7. 01:51에 Krzysztof Kozlowski 이(가) 쓴 글:
> On 25/03/2019 08:13, Andrzej Hajda wrote:
>> GSCALERs in Exynos5433 have local path to DECON and DECON_TV.
>> They can be used as extra planes with support for non-RGB formats and scaling.
>> To enable it on DECON update_plane and disable_plane callback should
>> be modified. Moreover DSD mux should be set accordingly, and finally
>> atomic_check callback should be used to limit the number of active planes.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 80 +++++++++++++++----
>>  drivers/gpu/drm/exynos/regs-decon5433.h       |  6 ++
>>  2 files changed, 72 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ex
> Hi guys!
> 
> I am working on DRM bindings conversion to DT schema format and I found
> this set only partially applied. I merged the DTS patches ("dsd" clock),
> but I think the driver and bindings were not picked up.
> 
> Nevertheless I am going to include the "dsd" clock in the new bindings,
> so the DTS passes DT schema checks. Let me know if other approach
> (revert of DTS change) should be taken.
> 

Sorry for late response.

As of now, "dsd" is a dead property not used anywhere.
This patch series makes real user not to work correctly due to ABI change.
How about reverting it until this patch series is merged after fixing the real user problem?

Thanks,
Inki Dae

> Best regards,
> Krzysztof
>
Krzysztof Kozlowski March 3, 2022, 4:03 p.m. UTC | #3
On 02/03/2022 02:00, Inki Dae wrote:
> Hi Krzysztof,
> 
> 22. 2. 7. 01:51에 Krzysztof Kozlowski 이(가) 쓴 글:
>> On 25/03/2019 08:13, Andrzej Hajda wrote:
>>> GSCALERs in Exynos5433 have local path to DECON and DECON_TV.
>>> They can be used as extra planes with support for non-RGB formats and scaling.
>>> To enable it on DECON update_plane and disable_plane callback should
>>> be modified. Moreover DSD mux should be set accordingly, and finally
>>> atomic_check callback should be used to limit the number of active planes.
>>>
>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>> ---
>>>  drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 80 +++++++++++++++----
>>>  drivers/gpu/drm/exynos/regs-decon5433.h       |  6 ++
>>>  2 files changed, 72 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/ex
>> Hi guys!
>>
>> I am working on DRM bindings conversion to DT schema format and I found
>> this set only partially applied. I merged the DTS patches ("dsd" clock),
>> but I think the driver and bindings were not picked up.
>>
>> Nevertheless I am going to include the "dsd" clock in the new bindings,
>> so the DTS passes DT schema checks. Let me know if other approach
>> (revert of DTS change) should be taken.
>>
> 
> Sorry for late response.
> 
> As of now, "dsd" is a dead property not used anywhere.
> This patch series makes real user not to work correctly due to ABI change.
> How about reverting it until this patch series is merged after fixing the real user problem?

The Exynos5433 DECON bindings were already merged by Rob, so someone
would need to send a revert. However this does not answer the actual
question - whether the "dsd" clock is necessary, whether it is there
(routed to DECON). If it is, it should stay in the bindings.

Best regards,
Krzysztof
Marek Szyprowski March 3, 2022, 4:11 p.m. UTC | #4
Hi Krzysztof,

On 03.03.2022 17:03, Krzysztof Kozlowski wrote:
> On 02/03/2022 02:00, Inki Dae wrote:
>> 22. 2. 7. 01:51에 Krzysztof Kozlowski 이(가) 쓴 글:
>>> On 25/03/2019 08:13, Andrzej Hajda wrote:
>>>> GSCALERs in Exynos5433 have local path to DECON and DECON_TV.
>>>> They can be used as extra planes with support for non-RGB formats and scaling.
>>>> To enable it on DECON update_plane and disable_plane callback should
>>>> be modified. Moreover DSD mux should be set accordingly, and finally
>>>> atomic_check callback should be used to limit the number of active planes.
>>>>
>>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>>> ---
>>>>   drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 80 +++++++++++++++----
>>>>   drivers/gpu/drm/exynos/regs-decon5433.h       |  6 ++
>>>>   2 files changed, 72 insertions(+), 14 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/ex
>>> Hi guys!
>>>
>>> I am working on DRM bindings conversion to DT schema format and I found
>>> this set only partially applied. I merged the DTS patches ("dsd" clock),
>>> but I think the driver and bindings were not picked up.
>>>
>>> Nevertheless I am going to include the "dsd" clock in the new bindings,
>>> so the DTS passes DT schema checks. Let me know if other approach
>>> (revert of DTS change) should be taken.
>>>
>> Sorry for late response.
>>
>> As of now, "dsd" is a dead property not used anywhere.
>> This patch series makes real user not to work correctly due to ABI change.
>> How about reverting it until this patch series is merged after fixing the real user problem?
> The Exynos5433 DECON bindings were already merged by Rob, so someone
> would need to send a revert. However this does not answer the actual
> question - whether the "dsd" clock is necessary, whether it is there
> (routed to DECON). If it is, it should stay in the bindings.

It is routed to DECON hardware and enabling it is needed to make the so 
called 'local path' (when DECON takes image data directly from the 
GSCALER hardware block instead of the memory buffer) working.

Best regards
Krzysztof Kozlowski March 3, 2022, 4:12 p.m. UTC | #5
On 03/03/2022 17:11, Marek Szyprowski wrote:
> Hi Krzysztof,
> 
> On 03.03.2022 17:03, Krzysztof Kozlowski wrote:
>> On 02/03/2022 02:00, Inki Dae wrote:
>>> 22. 2. 7. 01:51에 Krzysztof Kozlowski 이(가) 쓴 글:
>>>> On 25/03/2019 08:13, Andrzej Hajda wrote:
>>>>> GSCALERs in Exynos5433 have local path to DECON and DECON_TV.
>>>>> They can be used as extra planes with support for non-RGB formats and scaling.
>>>>> To enable it on DECON update_plane and disable_plane callback should
>>>>> be modified. Moreover DSD mux should be set accordingly, and finally
>>>>> atomic_check callback should be used to limit the number of active planes.
>>>>>
>>>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>>>> ---
>>>>>   drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 80 +++++++++++++++----
>>>>>   drivers/gpu/drm/exynos/regs-decon5433.h       |  6 ++
>>>>>   2 files changed, 72 insertions(+), 14 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/ex
>>>> Hi guys!
>>>>
>>>> I am working on DRM bindings conversion to DT schema format and I found
>>>> this set only partially applied. I merged the DTS patches ("dsd" clock),
>>>> but I think the driver and bindings were not picked up.
>>>>
>>>> Nevertheless I am going to include the "dsd" clock in the new bindings,
>>>> so the DTS passes DT schema checks. Let me know if other approach
>>>> (revert of DTS change) should be taken.
>>>>
>>> Sorry for late response.
>>>
>>> As of now, "dsd" is a dead property not used anywhere.
>>> This patch series makes real user not to work correctly due to ABI change.
>>> How about reverting it until this patch series is merged after fixing the real user problem?
>> The Exynos5433 DECON bindings were already merged by Rob, so someone
>> would need to send a revert. However this does not answer the actual
>> question - whether the "dsd" clock is necessary, whether it is there
>> (routed to DECON). If it is, it should stay in the bindings.
> 
> It is routed to DECON hardware and enabling it is needed to make the so 
> called 'local path' (when DECON takes image data directly from the 
> GSCALER hardware block instead of the memory buffer) working.

Awesome, thanks for confirmation! Bindings are good then.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 958972e3ee1e..b0332763594e 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -26,6 +26,10 @@ 
 #include "exynos_drm_fb.h"
 #include "regs-decon5433.h"
 
+#define DSD_CFG 0x1000
+#define DSD_CFG_GSCL_MODE(gsc, decon, wb) (((wb) << 1) | decon) << (3 + ((gsc) << 1))
+#define DSD_CFG_GSCL_MODE_MASK(gsc) DSD_CFG_GSCL_MODE(gsc, 1, 1)
+
 #define DSD_CFG_MUX 0x1004
 #define DSD_CFG_MUX_TE_UNMASK_GLOBAL BIT(13)
 
@@ -47,6 +51,7 @@  static const char * const decon_clks_name[] = {
 	"pclk_smmu_decon1x",
 	"sclk_decon_vclk",
 	"sclk_decon_eclk",
+	"dsd"
 };
 
 struct decon_context {
@@ -370,11 +375,40 @@  static void decon_shadow_protect(struct decon_context *ctx, bool protect)
 		       protect ? ~0 : 0);
 }
 
+static int decon_atomic_check(struct exynos_drm_crtc *crtc,
+			      struct drm_crtc_state *state)
+{
+	struct decon_context *ctx = to_decon(crtc);
+
+	if (hweight32(state->plane_mask) > WINDOWS_NR - ctx->first_win)
+		return -EINVAL;
+	return 0;
+}
+
+static void decon_set_gscl_mode(struct decon_context *ctx)
+{
+	u32 plane_mask = ctx->crtc.base.state->plane_mask;
+	struct drm_plane *bplane;
+	u32 mask = 0, val = 0;
+	bool decon_id = ctx->out_type & IFTYPE_HDMI;
+
+	drm_for_each_plane_mask(bplane, ctx->drm_dev, plane_mask) {
+		struct exynos_drm_plane *plane = to_exynos_plane(bplane);
+
+		if (!(plane->capabilities & EXYNOS_DRM_PLANE_CAP_GSCALER))
+			continue;
+		mask |= DSD_CFG_GSCL_MODE_MASK(plane->index);
+		val |= DSD_CFG_GSCL_MODE(plane->index, decon_id, 0);
+	}
+	regmap_update_bits(ctx->sysreg, DSD_CFG, mask, val);
+}
+
 static void decon_atomic_begin(struct exynos_drm_crtc *crtc)
 {
 	struct decon_context *ctx = to_decon(crtc);
 
 	decon_shadow_protect(ctx, true);
+	decon_set_gscl_mode(ctx);
 }
 
 #define BIT_VAL(x, e, s) (((x) & ((1 << ((e) - (s) + 1)) - 1)) << (s))
@@ -394,6 +428,9 @@  static void decon_update_plane(struct exynos_drm_crtc *crtc,
 	dma_addr_t dma_addr = exynos_drm_fb_dma_addr(fb, 0);
 	u32 val;
 
+	if (plane->ops && plane->ops->update_plane)
+		plane->ops->update_plane(plane);
+
 	if (crtc->base.mode.flags & DRM_MODE_FLAG_INTERLACE) {
 		val = COORDINATE_X(state->crtc.x) |
 			COORDINATE_Y(state->crtc.y / 2);
@@ -419,25 +456,38 @@  static void decon_update_plane(struct exynos_drm_crtc *crtc,
 		VIDOSD_Wx_ALPHA_B_F(0x0);
 	writel(val, ctx->addr + DECON_VIDOSDxD(win));
 
-	writel(dma_addr, ctx->addr + DECON_VIDW0xADD0B0(win));
-
-	val = dma_addr + pitch * state->src.h;
-	writel(val, ctx->addr + DECON_VIDW0xADD1B0(win));
-
-	if (!(ctx->out_type & IFTYPE_HDMI))
-		val = BIT_VAL(pitch - state->crtc.w * cpp, 27, 14)
-			| BIT_VAL(state->crtc.w * cpp, 13, 0);
-	else
-		val = BIT_VAL(pitch - state->crtc.w * cpp, 29, 15)
-			| BIT_VAL(state->crtc.w * cpp, 14, 0);
-	writel(val, ctx->addr + DECON_VIDW0xADD2(win));
-
 	decon_win_set_pixfmt(ctx, plane);
 
+	if (plane->capabilities & EXYNOS_DRM_PLANE_CAP_GSCALER) {
+		writel(UPDATE_SCHEME_OTF_PER_FRAME,
+			ctx->addr + DECON_UPDATE_SCHEME);
+		decon_set_bits(ctx, DECON_WINCONx(win),
+			WINCONx_ENLOCAL_F | WINCONx_LOCALSEL_MASK,
+			WINCONx_ENLOCAL_F | WINCONx_LOCALSEL_F(plane->index));
+	} else {
+		writel(dma_addr, ctx->addr + DECON_VIDW0xADD0B0(win));
+		val = dma_addr + pitch * state->src.h;
+		writel(val, ctx->addr + DECON_VIDW0xADD1B0(win));
+		if (!(ctx->out_type & IFTYPE_HDMI))
+			val = BIT_VAL(pitch - state->crtc.w * cpp, 27, 14)
+				| BIT_VAL(state->crtc.w * cpp, 13, 0);
+		else
+			val = BIT_VAL(pitch - state->crtc.w * cpp, 29, 15)
+				| BIT_VAL(state->crtc.w * cpp, 14, 0);
+		writel(val, ctx->addr + DECON_VIDW0xADD2(win));
+	}
+
 	/* window enable */
 	decon_set_bits(ctx, DECON_WINCONx(win), WINCONx_ENWIN_F, ~0);
 }
 
+static void decon_disable_plane(struct exynos_drm_crtc *crtc,
+			       struct exynos_drm_plane *plane)
+{
+	if (plane->ops && plane->ops->disable_plane)
+		plane->ops->disable_plane(plane);
+}
+
 static void decon_atomic_flush(struct exynos_drm_crtc *crtc)
 {
 	struct decon_context *ctx = to_decon(crtc);
@@ -448,7 +498,7 @@  static void decon_atomic_flush(struct exynos_drm_crtc *crtc)
 	for (; win < WINDOWS_NR; ++win) {
 		if (!readl(ctx->addr + DECON_WINCONx(win)) & WINCONx_ENWIN_F)
 			break;
-		decon_set_bits(ctx, DECON_WINCONx(win), WINCONx_ENWIN_F, 0);
+		writel(0, ctx->addr + DECON_WINCONx(win));
 	}
 
 	spin_lock_irqsave(&ctx->vblank_lock, flags);
@@ -590,8 +640,10 @@  static const struct exynos_drm_crtc_ops decon_crtc_ops = {
 	.disable		= decon_disable,
 	.enable_vblank		= decon_enable_vblank,
 	.disable_vblank		= decon_disable_vblank,
+	.atomic_check		= decon_atomic_check,
 	.atomic_begin		= decon_atomic_begin,
 	.update_plane		= decon_update_plane,
+	.disable_plane		= decon_disable_plane,
 	.mode_valid		= decon_mode_valid,
 	.atomic_flush		= decon_atomic_flush,
 };
diff --git a/drivers/gpu/drm/exynos/regs-decon5433.h b/drivers/gpu/drm/exynos/regs-decon5433.h
index 63db6974bf14..21a1c0bdd10a 100644
--- a/drivers/gpu/drm/exynos/regs-decon5433.h
+++ b/drivers/gpu/drm/exynos/regs-decon5433.h
@@ -98,6 +98,9 @@ 
 #define VIDOUT_COMMAND_IF		(0x2 << 20)
 
 /* WINCONx */
+#define WINCONx_LOCALSEL_F(n)		((n) << 21)
+#define WINCONx_LOCALSEL_MASK		(0x3 << 21)
+#define WINCONx_ENLOCAL_F		(0x1 << 20)
 #define WINCONx_HAWSWP_F		(1 << 16)
 #define WINCONx_WSWP_F			(1 << 15)
 #define WINCONx_BURSTLEN_MASK		(0x3 << 10)
@@ -184,6 +187,9 @@ 
 #define VIDTCON2_LINEVAL(x)		(((x) & 0xfff) << 16)
 #define VIDTCON2_HOZVAL(x)		((x) & 0xfff)
 
+/* DECON_UPDATE_SCHEME */
+#define UPDATE_SCHEME_OTF_PER_FRAME	(1 << 31)
+
 /* TRIGCON */
 #define TRIGCON_TRIGEN_PER_F		(1 << 31)
 #define TRIGCON_TRIGEN_F		(1 << 30)