[PATCH 00/10] drm/exynos/decon5433: add support to DECON-TV

[PATCH 00/10] drm/exynos/decon5433: add support to DECON-TV

[Andrzej Hajda]

Hi Inki,

This patchset adds support to DECON-TV in Exynos5433 SoC.
The main patch is prepended with few preparation patches:
- add three clocks required by HDMI pipeline,
- small bindings update,
- driver cleanup.

The patchset is based on the latest exynos-drm-next branch.

Regards
Andrzej


Andrzej Hajda (10):
  clk/samsung: exynos5433: add definitions of HDMI-PHY output clocks
  clk/samsung: exynos5433: add pclk_decon clock
  drm/exynos/decon5433: add PCLK clock
  dt-bindings: video: add PCLK clock entry to exynos5433-decon
  drm/exynos/decon5433: fix timing registers writes
  drm/exynos/decon5433: add function to set particular register bits
  drm/exynos/decon5433: merge different flag fields
  drm/exynos/decon5433: remove duplicated initialization
  dt-bindings: video: exynos5433-decon: add bindings for DECON-TV
  drm/exynos/decon5433: add support for DECON-TV

 .../devicetree/bindings/video/exynos5433-decon.txt |  23 +-
 drivers/clk/samsung/clk-exynos5433.c               |   8 +-
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c      | 320 ++++++++++-----------
 include/dt-bindings/clock/exynos5433.h             |   7 +-
 include/video/exynos5433_decon.h                   |  29 ++
 5 files changed, 214 insertions(+), 173 deletions(-)

-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 00/10] drm/exynos/decon5433: add support to DECON-TV

[Andrzej Hajda]

Hi Inki,

This patchset adds support to DECON-TV in Exynos5433 SoC.
The main patch is prepended with few preparation patches:
- add three clocks required by HDMI pipeline,
- small bindings update,
- driver cleanup.

The patchset is based on the latest exynos-drm-next branch.

Regards
Andrzej


Andrzej Hajda (10):
  clk/samsung: exynos5433: add definitions of HDMI-PHY output clocks
  clk/samsung: exynos5433: add pclk_decon clock
  drm/exynos/decon5433: add PCLK clock
  dt-bindings: video: add PCLK clock entry to exynos5433-decon
  drm/exynos/decon5433: fix timing registers writes
  drm/exynos/decon5433: add function to set particular register bits
  drm/exynos/decon5433: merge different flag fields
  drm/exynos/decon5433: remove duplicated initialization
  dt-bindings: video: exynos5433-decon: add bindings for DECON-TV
  drm/exynos/decon5433: add support for DECON-TV

 .../devicetree/bindings/video/exynos5433-decon.txt |  23 +-
 drivers/clk/samsung/clk-exynos5433.c               |   8 +-
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c      | 320 ++++++++++-----------
 include/dt-bindings/clock/exynos5433.h             |   7 +-
 include/video/exynos5433_decon.h                   |  29 ++
 5 files changed, 214 insertions(+), 173 deletions(-)

-- 
1.9.1

[PATCH 01/10] clk/samsung: exynos5433: add definitions of HDMI-PHY output clocks

[Andrzej Hajda]

HDMI driver must re-parent respective muxes during HDMI-PHY on/off
to HDMI-PHY output clocks. To reference those clocks their
definitions should be added.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/clk/samsung/clk-exynos5433.c   | 6 ++++--
 include/dt-bindings/clock/exynos5433.h | 5 ++++-
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos5433.c b/drivers/clk/samsung/clk-exynos5433.c
index 650ec13..e037406 100644
--- a/drivers/clk/samsung/clk-exynos5433.c
+++ b/drivers/clk/samsung/clk-exynos5433.c
@@ -2614,8 +2614,10 @@ static struct samsung_fixed_rate_clock disp_fixed_clks[] __initdata = {
 	FRATE(0, "phyclk_mipidphy0_rxclkesc0_phy", NULL, CLK_IS_ROOT,
 			100000000),
 	/* PHY clocks from HDMI_PHY */
-	FRATE(0, "phyclk_hdmiphy_tmds_clko_phy", NULL, CLK_IS_ROOT, 300000000),
-	FRATE(0, "phyclk_hdmiphy_pixel_clko_phy", NULL, CLK_IS_ROOT, 166000000),
+	FRATE(CLK_PHYCLK_HDMIPHY_TMDS_CLKO_PHY, "phyclk_hdmiphy_tmds_clko_phy",
+			NULL, CLK_IS_ROOT, 300000000),
+	FRATE(CLK_PHYCLK_HDMIPHY_PIXEL_CLKO_PHY, "phyclk_hdmiphy_pixel_clko_phy",
+			NULL, CLK_IS_ROOT, 166000000),
 };
 
 static struct samsung_mux_clock disp_mux_clks[] __initdata = {
diff --git a/include/dt-bindings/clock/exynos5433.h b/include/dt-bindings/clock/exynos5433.h
index 5bd80d5..4f0d566 100644
--- a/include/dt-bindings/clock/exynos5433.h
+++ b/include/dt-bindings/clock/exynos5433.h
@@ -765,7 +765,10 @@
 #define CLK_SCLK_RGB_VCLK				109
 #define CLK_SCLK_RGB_TV_VCLK				110
 
-#define DISP_NR_CLK					111
+#define CLK_PHYCLK_HDMIPHY_PIXEL_CLKO_PHY		111
+#define CLK_PHYCLK_HDMIPHY_TMDS_CLKO_PHY		112
+
+#define DISP_NR_CLK					113
 
 /* CMU_AUD */
 #define CLK_MOUT_AUD_PLL_USER				1
-- 
1.9.1

[PATCH 02/10] clk/samsung: exynos5433: add pclk_decon clock

[Andrzej Hajda]

This undocumented gate clock is used by DECON IP.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/clk/samsung/clk-exynos5433.c   | 2 ++
 include/dt-bindings/clock/exynos5433.h | 4 +++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/samsung/clk-exynos5433.c b/drivers/clk/samsung/clk-exynos5433.c
index e037406..e7b4533 100644
--- a/drivers/clk/samsung/clk-exynos5433.c
+++ b/drivers/clk/samsung/clk-exynos5433.c
@@ -2822,6 +2822,8 @@ static struct samsung_gate_clock disp_gate_clks[] __initdata = {
 			ENABLE_PCLK_DISP, 2, 0, 0),
 	GATE(CLK_PCLK_DECON_TV, "pclk_decon_tv", "div_pclk_disp",
 			ENABLE_PCLK_DISP, 1, 0, 0),
+	GATE(CLK_PCLK_DECON, "pclk_decon", "div_pclk_disp",
+			ENABLE_PCLK_DISP, 0, 0, 0),
 
 	/* ENABLE_SCLK_DISP */
 	GATE(CLK_PHYCLK_MIPIDPHY1_BITCLKDIV8, "phyclk_mipidphy1_bitclkdiv8",
diff --git a/include/dt-bindings/clock/exynos5433.h b/include/dt-bindings/clock/exynos5433.h
index 4f0d566..5c2636c 100644
--- a/include/dt-bindings/clock/exynos5433.h
+++ b/include/dt-bindings/clock/exynos5433.h
@@ -768,7 +768,9 @@
 #define CLK_PHYCLK_HDMIPHY_PIXEL_CLKO_PHY		111
 #define CLK_PHYCLK_HDMIPHY_TMDS_CLKO_PHY		112
 
-#define DISP_NR_CLK					113
+#define CLK_PCLK_DECON					113
+
+#define DISP_NR_CLK					114
 
 /* CMU_AUD */
 #define CLK_MOUT_AUD_PLL_USER				1
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 02/10] clk/samsung: exynos5433: add pclk_decon clock

[Andrzej Hajda]

This undocumented gate clock is used by DECON IP.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/clk/samsung/clk-exynos5433.c   | 2 ++
 include/dt-bindings/clock/exynos5433.h | 4 +++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/samsung/clk-exynos5433.c b/drivers/clk/samsung/clk-exynos5433.c
index e037406..e7b4533 100644
--- a/drivers/clk/samsung/clk-exynos5433.c
+++ b/drivers/clk/samsung/clk-exynos5433.c
@@ -2822,6 +2822,8 @@ static struct samsung_gate_clock disp_gate_clks[] __initdata = {
 			ENABLE_PCLK_DISP, 2, 0, 0),
 	GATE(CLK_PCLK_DECON_TV, "pclk_decon_tv", "div_pclk_disp",
 			ENABLE_PCLK_DISP, 1, 0, 0),
+	GATE(CLK_PCLK_DECON, "pclk_decon", "div_pclk_disp",
+			ENABLE_PCLK_DISP, 0, 0, 0),
 
 	/* ENABLE_SCLK_DISP */
 	GATE(CLK_PHYCLK_MIPIDPHY1_BITCLKDIV8, "phyclk_mipidphy1_bitclkdiv8",
diff --git a/include/dt-bindings/clock/exynos5433.h b/include/dt-bindings/clock/exynos5433.h
index 4f0d566..5c2636c 100644
--- a/include/dt-bindings/clock/exynos5433.h
+++ b/include/dt-bindings/clock/exynos5433.h
@@ -768,7 +768,9 @@
 #define CLK_PHYCLK_HDMIPHY_PIXEL_CLKO_PHY		111
 #define CLK_PHYCLK_HDMIPHY_TMDS_CLKO_PHY		112
 
-#define DISP_NR_CLK					113
+#define CLK_PCLK_DECON					113
+
+#define DISP_NR_CLK					114
 
 /* CMU_AUD */
 #define CLK_MOUT_AUD_PLL_USER				1
-- 
1.9.1

[PATCH 03/10] drm/exynos/decon5433: add PCLK clock

[Andrzej Hajda]

PCLK clock is used by DECON IP. The patch also replaces magic number with
number of clocks in array definition.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 1ea26dbb..b25d764 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -27,13 +27,23 @@
 #define CURSOR_WIN	2
 #define MIN_FB_WIDTH_FOR_16WORD_BURST	128
 
+static const char * const decon_clks_name[] = {
+	"pclk",
+	"aclk_decon",
+	"aclk_smmu_decon0x",
+	"aclk_xiu_decon0x",
+	"pclk_smmu_decon0x",
+	"sclk_decon_vclk",
+	"sclk_decon_eclk",
+};
+
 struct decon_context {
 	struct device			*dev;
 	struct drm_device		*drm_dev;
 	struct exynos_drm_crtc		*crtc;
 	struct exynos_drm_plane		planes[WINDOWS_NR];
 	void __iomem			*addr;
-	struct clk			*clks[6];
+	struct clk			*clks[ARRAY_SIZE(decon_clks_name)];
 	unsigned long			irq_flags;
 	int				pipe;
 	bool				suspended;
@@ -45,15 +55,6 @@ struct decon_context {
 	atomic_t			win_updated;
 };
 
-static const char * const decon_clks_name[] = {
-	"aclk_decon",
-	"aclk_smmu_decon0x",
-	"aclk_xiu_decon0x",
-	"pclk_smmu_decon0x",
-	"sclk_decon_vclk",
-	"sclk_decon_eclk",
-};
-
 static const uint32_t decon_formats[] = {
 	DRM_FORMAT_XRGB1555,
 	DRM_FORMAT_RGB565,
-- 
1.9.1

[PATCH 04/10] dt-bindings: video: add PCLK clock entry to exynos5433-decon

[Andrzej Hajda]

DECON IP requires this clock to access configuration registers.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 Documentation/devicetree/bindings/video/exynos5433-decon.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/video/exynos5433-decon.txt b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
index 377afbf..3dff78b 100644
--- a/Documentation/devicetree/bindings/video/exynos5433-decon.txt
+++ b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
@@ -16,7 +16,7 @@ Required properties:
 - clocks: must include clock specifiers corresponding to entries in the
 	  clock-names property.
 - clock-names: list of clock names sorted in the same order as the clocks
-	       property. Must contain "aclk_decon", "aclk_smmu_decon0x",
+	       property. Must contain "pclk", "aclk_decon", "aclk_smmu_decon0x",
 	       "aclk_xiu_decon0x", "pclk_smmu_decon0x", clk_decon_vclk",
 	       "sclk_decon_eclk"
 - ports: contains a port which is connected to mic node. address-cells and
-- 
1.9.1

[PATCH 05/10] drm/exynos/decon5433: fix timing registers writes

[Andrzej Hajda]

All timing registers should contain values decreased by one.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index b25d764..83e0939 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -104,7 +104,7 @@ static void decon_setup_trigger(struct decon_context *ctx)
 static void decon_commit(struct exynos_drm_crtc *crtc)
 {
 	struct decon_context *ctx = crtc->ctx;
-	struct drm_display_mode *mode = &crtc->base.mode;
+	struct drm_display_mode *m = &crtc->base.mode;
 	u32 val;
 
 	if (ctx->suspended)
@@ -122,29 +122,29 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
 		val |= VIDOUT_RGB_IF;
 	writel(val, ctx->addr + DECON_VIDOUTCON0);
 
-	val = VIDTCON2_LINEVAL(mode->vdisplay - 1) |
-		VIDTCON2_HOZVAL(mode->hdisplay - 1);
+	val = VIDTCON2_LINEVAL(m->vdisplay - 1) |
+		VIDTCON2_HOZVAL(m->hdisplay - 1);
 	writel(val, ctx->addr + DECON_VIDTCON2);
 
 	if (!ctx->i80_if) {
 		val = VIDTCON00_VBPD_F(
-				mode->crtc_vtotal - mode->crtc_vsync_end) |
+				m->crtc_vtotal - m->crtc_vsync_end - 1) |
 			VIDTCON00_VFPD_F(
-				mode->crtc_vsync_start - mode->crtc_vdisplay);
+				m->crtc_vsync_start - m->crtc_vdisplay - 1);
 		writel(val, ctx->addr + DECON_VIDTCON00);
 
 		val = VIDTCON01_VSPW_F(
-				mode->crtc_vsync_end - mode->crtc_vsync_start);
+				m->crtc_vsync_end - m->crtc_vsync_start - 1);
 		writel(val, ctx->addr + DECON_VIDTCON01);
 
 		val = VIDTCON10_HBPD_F(
-				mode->crtc_htotal - mode->crtc_hsync_end) |
+				m->crtc_htotal - m->crtc_hsync_end - 1) |
 			VIDTCON10_HFPD_F(
-				mode->crtc_hsync_start - mode->crtc_hdisplay);
+				m->crtc_hsync_start - m->crtc_hdisplay - 1);
 		writel(val, ctx->addr + DECON_VIDTCON10);
 
 		val = VIDTCON11_HSPW_F(
-				mode->crtc_hsync_end - mode->crtc_hsync_start);
+				m->crtc_hsync_end - m->crtc_hsync_start - 1);
 		writel(val, ctx->addr + DECON_VIDTCON11);
 	}
 
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 05/10] drm/exynos/decon5433: fix timing registers writes

[Andrzej Hajda]

All timing registers should contain values decreased by one.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index b25d764..83e0939 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -104,7 +104,7 @@ static void decon_setup_trigger(struct decon_context *ctx)
 static void decon_commit(struct exynos_drm_crtc *crtc)
 {
 	struct decon_context *ctx = crtc->ctx;
-	struct drm_display_mode *mode = &crtc->base.mode;
+	struct drm_display_mode *m = &crtc->base.mode;
 	u32 val;
 
 	if (ctx->suspended)
@@ -122,29 +122,29 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
 		val |= VIDOUT_RGB_IF;
 	writel(val, ctx->addr + DECON_VIDOUTCON0);
 
-	val = VIDTCON2_LINEVAL(mode->vdisplay - 1) |
-		VIDTCON2_HOZVAL(mode->hdisplay - 1);
+	val = VIDTCON2_LINEVAL(m->vdisplay - 1) |
+		VIDTCON2_HOZVAL(m->hdisplay - 1);
 	writel(val, ctx->addr + DECON_VIDTCON2);
 
 	if (!ctx->i80_if) {
 		val = VIDTCON00_VBPD_F(
-				mode->crtc_vtotal - mode->crtc_vsync_end) |
+				m->crtc_vtotal - m->crtc_vsync_end - 1) |
 			VIDTCON00_VFPD_F(
-				mode->crtc_vsync_start - mode->crtc_vdisplay);
+				m->crtc_vsync_start - m->crtc_vdisplay - 1);
 		writel(val, ctx->addr + DECON_VIDTCON00);
 
 		val = VIDTCON01_VSPW_F(
-				mode->crtc_vsync_end - mode->crtc_vsync_start);
+				m->crtc_vsync_end - m->crtc_vsync_start - 1);
 		writel(val, ctx->addr + DECON_VIDTCON01);
 
 		val = VIDTCON10_HBPD_F(
-				mode->crtc_htotal - mode->crtc_hsync_end) |
+				m->crtc_htotal - m->crtc_hsync_end - 1) |
 			VIDTCON10_HFPD_F(
-				mode->crtc_hsync_start - mode->crtc_hdisplay);
+				m->crtc_hsync_start - m->crtc_hdisplay - 1);
 		writel(val, ctx->addr + DECON_VIDTCON10);
 
 		val = VIDTCON11_HSPW_F(
-				mode->crtc_hsync_end - mode->crtc_hsync_start);
+				m->crtc_hsync_end - m->crtc_hsync_start - 1);
 		writel(val, ctx->addr + DECON_VIDTCON11);
 	}
 
-- 
1.9.1

[PATCH 06/10] drm/exynos/decon5433: add function to set particular register bits

[Andrzej Hajda]

The driver often sets only particular bits of configuration registers.
Using separate function to such action simplifies the code.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 69 ++++++++-------------------
 1 file changed, 19 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 83e0939..722c11a 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -62,6 +62,13 @@ static const uint32_t decon_formats[] = {
 	DRM_FORMAT_ARGB8888,
 };
 
+static inline void decon_set_bits(struct decon_context *ctx, u32 reg, u32 mask,
+				  u32 val)
+{
+	val = (val & mask) | (readl(ctx->addr + reg) & ~mask);
+	writel(val, ctx->addr + reg);
+}
+
 static int decon_enable_vblank(struct exynos_drm_crtc *crtc)
 {
 	struct decon_context *ctx = crtc->ctx;
@@ -215,16 +222,8 @@ static void decon_win_set_pixfmt(struct decon_context *ctx, unsigned int win,
 static void decon_shadow_protect_win(struct decon_context *ctx, int win,
 					bool protect)
 {
-	u32 val;
-
-	val = readl(ctx->addr + DECON_SHADOWCON);
-
-	if (protect)
-		val |= SHADOWCON_Wx_PROTECT(win);
-	else
-		val &= ~SHADOWCON_Wx_PROTECT(win);
-
-	writel(val, ctx->addr + DECON_SHADOWCON);
+	decon_set_bits(ctx, DECON_SHADOWCON, SHADOWCON_Wx_PROTECT(win),
+		       protect ? ~0 : 0);
 }
 
 static void decon_atomic_begin(struct exynos_drm_crtc *crtc,
@@ -278,14 +277,10 @@ static void decon_update_plane(struct exynos_drm_crtc *crtc,
 	decon_win_set_pixfmt(ctx, win, state->fb);
 
 	/* window enable */
-	val = readl(ctx->addr + DECON_WINCONx(win));
-	val |= WINCONx_ENWIN_F;
-	writel(val, ctx->addr + DECON_WINCONx(win));
+	decon_set_bits(ctx, DECON_WINCONx(win), WINCONx_ENWIN_F, ~0);
 
 	/* standalone update */
-	val = readl(ctx->addr + DECON_UPDATE);
-	val |= STANDALONE_UPDATE_F;
-	writel(val, ctx->addr + DECON_UPDATE);
+	decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0);
 }
 
 static void decon_disable_plane(struct exynos_drm_crtc *crtc,
@@ -293,7 +288,6 @@ static void decon_disable_plane(struct exynos_drm_crtc *crtc,
 {
 	struct decon_context *ctx = crtc->ctx;
 	unsigned int win = plane->zpos;
-	u32 val;
 
 	if (ctx->suspended)
 		return;
@@ -301,16 +295,12 @@ static void decon_disable_plane(struct exynos_drm_crtc *crtc,
 	decon_shadow_protect_win(ctx, win, true);
 
 	/* window disable */
-	val = readl(ctx->addr + DECON_WINCONx(win));
-	val &= ~WINCONx_ENWIN_F;
-	writel(val, ctx->addr + DECON_WINCONx(win));
+	decon_set_bits(ctx, DECON_WINCONx(win), WINCONx_ENWIN_F, 0);
 
 	decon_shadow_protect_win(ctx, win, false);
 
 	/* standalone update */
-	val = readl(ctx->addr + DECON_UPDATE);
-	val |= STANDALONE_UPDATE_F;
-	writel(val, ctx->addr + DECON_UPDATE);
+	decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0);
 }
 
 static void decon_atomic_flush(struct exynos_drm_crtc *crtc,
@@ -416,17 +406,12 @@ static void decon_disable(struct exynos_drm_crtc *crtc)
 void decon_te_irq_handler(struct exynos_drm_crtc *crtc)
 {
 	struct decon_context *ctx = crtc->ctx;
-	u32 val;
 
 	if (!test_bit(BIT_CLKS_ENABLED, &ctx->enabled))
 		return;
 
-	if (atomic_add_unless(&ctx->win_updated, -1, 0)) {
-		/* trigger */
-		val = readl(ctx->addr + DECON_TRIGCON);
-		val |= TRIGCON_SWTRIGCMD;
-		writel(val, ctx->addr + DECON_TRIGCON);
-	}
+	if (atomic_add_unless(&ctx->win_updated, -1, 0))
+		decon_set_bits(ctx, DECON_TRIGCON, TRIGCON_SWTRIGCMD, ~0);
 
 	drm_crtc_handle_vblank(&ctx->crtc->base);
 }
@@ -435,7 +420,6 @@ static void decon_clear_channels(struct exynos_drm_crtc *crtc)
 {
 	struct decon_context *ctx = crtc->ctx;
 	int win, i, ret;
-	u32 val;
 
 	DRM_DEBUG_KMS("%s\n", __FILE__);
 
@@ -446,25 +430,10 @@ static void decon_clear_channels(struct exynos_drm_crtc *crtc)
 	}
 
 	for (win = 0; win < WINDOWS_NR; win++) {
-		/* shadow update disable */
-		val = readl(ctx->addr + DECON_SHADOWCON);
-		val |= SHADOWCON_Wx_PROTECT(win);
-		writel(val, ctx->addr + DECON_SHADOWCON);
-
-		/* window disable */
-		val = readl(ctx->addr + DECON_WINCONx(win));
-		val &= ~WINCONx_ENWIN_F;
-		writel(val, ctx->addr + DECON_WINCONx(win));
-
-		/* shadow update enable */
-		val = readl(ctx->addr + DECON_SHADOWCON);
-		val &= ~SHADOWCON_Wx_PROTECT(win);
-		writel(val, ctx->addr + DECON_SHADOWCON);
-
-		/* standalone update */
-		val = readl(ctx->addr + DECON_UPDATE);
-		val |= STANDALONE_UPDATE_F;
-		writel(val, ctx->addr + DECON_UPDATE);
+		decon_shadow_protect_win(ctx, win, true);
+		decon_set_bits(ctx, DECON_WINCONx(win), WINCONx_ENWIN_F, 0);
+		decon_shadow_protect_win(ctx, win, false);
+		decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0);
 	}
 	/* TODO: wait for possible vsync */
 	msleep(50);
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 06/10] drm/exynos/decon5433: add function to set particular register bits

[Andrzej Hajda]

The driver often sets only particular bits of configuration registers.
Using separate function to such action simplifies the code.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 69 ++++++++-------------------
 1 file changed, 19 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 83e0939..722c11a 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -62,6 +62,13 @@ static const uint32_t decon_formats[] = {
 	DRM_FORMAT_ARGB8888,
 };
 
+static inline void decon_set_bits(struct decon_context *ctx, u32 reg, u32 mask,
+				  u32 val)
+{
+	val = (val & mask) | (readl(ctx->addr + reg) & ~mask);
+	writel(val, ctx->addr + reg);
+}
+
 static int decon_enable_vblank(struct exynos_drm_crtc *crtc)
 {
 	struct decon_context *ctx = crtc->ctx;
@@ -215,16 +222,8 @@ static void decon_win_set_pixfmt(struct decon_context *ctx, unsigned int win,
 static void decon_shadow_protect_win(struct decon_context *ctx, int win,
 					bool protect)
 {
-	u32 val;
-
-	val = readl(ctx->addr + DECON_SHADOWCON);
-
-	if (protect)
-		val |= SHADOWCON_Wx_PROTECT(win);
-	else
-		val &= ~SHADOWCON_Wx_PROTECT(win);
-
-	writel(val, ctx->addr + DECON_SHADOWCON);
+	decon_set_bits(ctx, DECON_SHADOWCON, SHADOWCON_Wx_PROTECT(win),
+		       protect ? ~0 : 0);
 }
 
 static void decon_atomic_begin(struct exynos_drm_crtc *crtc,
@@ -278,14 +277,10 @@ static void decon_update_plane(struct exynos_drm_crtc *crtc,
 	decon_win_set_pixfmt(ctx, win, state->fb);
 
 	/* window enable */
-	val = readl(ctx->addr + DECON_WINCONx(win));
-	val |= WINCONx_ENWIN_F;
-	writel(val, ctx->addr + DECON_WINCONx(win));
+	decon_set_bits(ctx, DECON_WINCONx(win), WINCONx_ENWIN_F, ~0);
 
 	/* standalone update */
-	val = readl(ctx->addr + DECON_UPDATE);
-	val |= STANDALONE_UPDATE_F;
-	writel(val, ctx->addr + DECON_UPDATE);
+	decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0);
 }
 
 static void decon_disable_plane(struct exynos_drm_crtc *crtc,
@@ -293,7 +288,6 @@ static void decon_disable_plane(struct exynos_drm_crtc *crtc,
 {
 	struct decon_context *ctx = crtc->ctx;
 	unsigned int win = plane->zpos;
-	u32 val;
 
 	if (ctx->suspended)
 		return;
@@ -301,16 +295,12 @@ static void decon_disable_plane(struct exynos_drm_crtc *crtc,
 	decon_shadow_protect_win(ctx, win, true);
 
 	/* window disable */
-	val = readl(ctx->addr + DECON_WINCONx(win));
-	val &= ~WINCONx_ENWIN_F;
-	writel(val, ctx->addr + DECON_WINCONx(win));
+	decon_set_bits(ctx, DECON_WINCONx(win), WINCONx_ENWIN_F, 0);
 
 	decon_shadow_protect_win(ctx, win, false);
 
 	/* standalone update */
-	val = readl(ctx->addr + DECON_UPDATE);
-	val |= STANDALONE_UPDATE_F;
-	writel(val, ctx->addr + DECON_UPDATE);
+	decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0);
 }
 
 static void decon_atomic_flush(struct exynos_drm_crtc *crtc,
@@ -416,17 +406,12 @@ static void decon_disable(struct exynos_drm_crtc *crtc)
 void decon_te_irq_handler(struct exynos_drm_crtc *crtc)
 {
 	struct decon_context *ctx = crtc->ctx;
-	u32 val;
 
 	if (!test_bit(BIT_CLKS_ENABLED, &ctx->enabled))
 		return;
 
-	if (atomic_add_unless(&ctx->win_updated, -1, 0)) {
-		/* trigger */
-		val = readl(ctx->addr + DECON_TRIGCON);
-		val |= TRIGCON_SWTRIGCMD;
-		writel(val, ctx->addr + DECON_TRIGCON);
-	}
+	if (atomic_add_unless(&ctx->win_updated, -1, 0))
+		decon_set_bits(ctx, DECON_TRIGCON, TRIGCON_SWTRIGCMD, ~0);
 
 	drm_crtc_handle_vblank(&ctx->crtc->base);
 }
@@ -435,7 +420,6 @@ static void decon_clear_channels(struct exynos_drm_crtc *crtc)
 {
 	struct decon_context *ctx = crtc->ctx;
 	int win, i, ret;
-	u32 val;
 
 	DRM_DEBUG_KMS("%s\n", __FILE__);
 
@@ -446,25 +430,10 @@ static void decon_clear_channels(struct exynos_drm_crtc *crtc)
 	}
 
 	for (win = 0; win < WINDOWS_NR; win++) {
-		/* shadow update disable */
-		val = readl(ctx->addr + DECON_SHADOWCON);
-		val |= SHADOWCON_Wx_PROTECT(win);
-		writel(val, ctx->addr + DECON_SHADOWCON);
-
-		/* window disable */
-		val = readl(ctx->addr + DECON_WINCONx(win));
-		val &= ~WINCONx_ENWIN_F;
-		writel(val, ctx->addr + DECON_WINCONx(win));
-
-		/* shadow update enable */
-		val = readl(ctx->addr + DECON_SHADOWCON);
-		val &= ~SHADOWCON_Wx_PROTECT(win);
-		writel(val, ctx->addr + DECON_SHADOWCON);
-
-		/* standalone update */
-		val = readl(ctx->addr + DECON_UPDATE);
-		val |= STANDALONE_UPDATE_F;
-		writel(val, ctx->addr + DECON_UPDATE);
+		decon_shadow_protect_win(ctx, win, true);
+		decon_set_bits(ctx, DECON_WINCONx(win), WINCONx_ENWIN_F, 0);
+		decon_shadow_protect_win(ctx, win, false);
+		decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0);
 	}
 	/* TODO: wait for possible vsync */
 	msleep(50);
-- 
1.9.1

[PATCH 07/10] drm/exynos/decon5433: merge different flag fields

[Andrzej Hajda]

Driver uses four different fields for internal flags. They can be merged
into one.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 61 +++++++++++++--------------
 1 file changed, 30 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 722c11a..265a77f 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -37,6 +37,13 @@ static const char * const decon_clks_name[] = {
 	"sclk_decon_eclk",
 };
 
+enum decon_flag_bits {
+	BIT_CLKS_ENABLED,
+	BIT_IRQS_ENABLED,
+	BIT_WIN_UPDATED,
+	BIT_SUSPENDED
+};
+
 struct decon_context {
 	struct device			*dev;
 	struct drm_device		*drm_dev;
@@ -44,15 +51,9 @@ struct decon_context {
 	struct exynos_drm_plane		planes[WINDOWS_NR];
 	void __iomem			*addr;
 	struct clk			*clks[ARRAY_SIZE(decon_clks_name)];
-	unsigned long			irq_flags;
 	int				pipe;
-	bool				suspended;
-
-#define BIT_CLKS_ENABLED		0
-#define BIT_IRQS_ENABLED		1
-	unsigned long			enabled;
+	unsigned long			flags;
 	bool				i80_if;
-	atomic_t			win_updated;
 };
 
 static const uint32_t decon_formats[] = {
@@ -74,10 +75,10 @@ static int decon_enable_vblank(struct exynos_drm_crtc *crtc)
 	struct decon_context *ctx = crtc->ctx;
 	u32 val;
 
-	if (ctx->suspended)
+	if (test_bit(BIT_SUSPENDED, &ctx->flags))
 		return -EPERM;
 
-	if (test_and_set_bit(0, &ctx->irq_flags)) {
+	if (test_and_set_bit(BIT_IRQS_ENABLED, &ctx->flags)) {
 		val = VIDINTCON0_INTEN;
 		if (ctx->i80_if)
 			val |= VIDINTCON0_FRAMEDONE;
@@ -94,10 +95,10 @@ static void decon_disable_vblank(struct exynos_drm_crtc *crtc)
 {
 	struct decon_context *ctx = crtc->ctx;
 
-	if (ctx->suspended)
+	if (test_bit(BIT_SUSPENDED, &ctx->flags))
 		return;
 
-	if (test_and_clear_bit(0, &ctx->irq_flags))
+	if (test_and_clear_bit(BIT_IRQS_ENABLED, &ctx->flags))
 		writel(0, ctx->addr + DECON_VIDINTCON0);
 }
 
@@ -114,7 +115,7 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
 	struct drm_display_mode *m = &crtc->base.mode;
 	u32 val;
 
-	if (ctx->suspended)
+	if (test_bit(BIT_SUSPENDED, &ctx->flags))
 		return;
 
 	/* enable clock gate */
@@ -231,7 +232,7 @@ static void decon_atomic_begin(struct exynos_drm_crtc *crtc,
 {
 	struct decon_context *ctx = crtc->ctx;
 
-	if (ctx->suspended)
+	if (test_bit(BIT_SUSPENDED, &ctx->flags))
 		return;
 
 	decon_shadow_protect_win(ctx, plane->zpos, true);
@@ -247,7 +248,7 @@ static void decon_update_plane(struct exynos_drm_crtc *crtc,
 	unsigned int pitch = state->fb->pitches[0];
 	u32 val;
 
-	if (ctx->suspended)
+	if (test_bit(BIT_SUSPENDED, &ctx->flags))
 		return;
 
 	val = COORDINATE_X(plane->crtc_x) | COORDINATE_Y(plane->crtc_y);
@@ -289,7 +290,7 @@ static void decon_disable_plane(struct exynos_drm_crtc *crtc,
 	struct decon_context *ctx = crtc->ctx;
 	unsigned int win = plane->zpos;
 
-	if (ctx->suspended)
+	if (test_bit(BIT_SUSPENDED, &ctx->flags))
 		return;
 
 	decon_shadow_protect_win(ctx, win, true);
@@ -308,13 +309,13 @@ static void decon_atomic_flush(struct exynos_drm_crtc *crtc,
 {
 	struct decon_context *ctx = crtc->ctx;
 
-	if (ctx->suspended)
+	if (test_bit(BIT_SUSPENDED, &ctx->flags))
 		return;
 
 	decon_shadow_protect_win(ctx, plane->zpos, false);
 
 	if (ctx->i80_if)
-		atomic_set(&ctx->win_updated, 1);
+		set_bit(BIT_WIN_UPDATED, &ctx->flags);
 }
 
 static void decon_swreset(struct decon_context *ctx)
@@ -346,11 +347,9 @@ static void decon_enable(struct exynos_drm_crtc *crtc)
 	int ret;
 	int i;
 
-	if (!ctx->suspended)
+	if (!test_and_clear_bit(BIT_SUSPENDED, &ctx->flags))
 		return;
 
-	ctx->suspended = false;
-
 	pm_runtime_get_sync(ctx->dev);
 
 	for (i = 0; i < ARRAY_SIZE(decon_clks_name); i++) {
@@ -359,10 +358,10 @@ static void decon_enable(struct exynos_drm_crtc *crtc)
 			goto err;
 	}
 
-	set_bit(BIT_CLKS_ENABLED, &ctx->enabled);
+	set_bit(BIT_CLKS_ENABLED, &ctx->flags);
 
 	/* if vblank was enabled status, enable it again. */
-	if (test_and_clear_bit(0, &ctx->irq_flags))
+	if (test_and_clear_bit(BIT_IRQS_ENABLED, &ctx->flags))
 		decon_enable_vblank(ctx->crtc);
 
 	decon_commit(ctx->crtc);
@@ -372,7 +371,7 @@ err:
 	while (--i >= 0)
 		clk_disable_unprepare(ctx->clks[i]);
 
-	ctx->suspended = true;
+	set_bit(BIT_SUSPENDED, &ctx->flags);
 }
 
 static void decon_disable(struct exynos_drm_crtc *crtc)
@@ -380,7 +379,7 @@ static void decon_disable(struct exynos_drm_crtc *crtc)
 	struct decon_context *ctx = crtc->ctx;
 	int i;
 
-	if (ctx->suspended)
+	if (test_bit(BIT_SUSPENDED, &ctx->flags))
 		return;
 
 	/*
@@ -396,21 +395,21 @@ static void decon_disable(struct exynos_drm_crtc *crtc)
 	for (i = 0; i < ARRAY_SIZE(decon_clks_name); i++)
 		clk_disable_unprepare(ctx->clks[i]);
 
-	clear_bit(BIT_CLKS_ENABLED, &ctx->enabled);
+	clear_bit(BIT_CLKS_ENABLED, &ctx->flags);
 
 	pm_runtime_put_sync(ctx->dev);
 
-	ctx->suspended = true;
+	set_bit(BIT_SUSPENDED, &ctx->flags);
 }
 
 void decon_te_irq_handler(struct exynos_drm_crtc *crtc)
 {
 	struct decon_context *ctx = crtc->ctx;
 
-	if (!test_bit(BIT_CLKS_ENABLED, &ctx->enabled))
+	if (!test_bit(BIT_CLKS_ENABLED, &ctx->flags))
 		return;
 
-	if (atomic_add_unless(&ctx->win_updated, -1, 0))
+	if (test_and_clear_bit(BIT_WIN_UPDATED, &ctx->flags))
 		decon_set_bits(ctx, DECON_TRIGCON, TRIGCON_SWTRIGCMD, ~0);
 
 	drm_crtc_handle_vblank(&ctx->crtc->base);
@@ -520,7 +519,7 @@ static irqreturn_t decon_vsync_irq_handler(int irq, void *dev_id)
 	struct decon_context *ctx = dev_id;
 	u32 val;
 
-	if (!test_bit(BIT_CLKS_ENABLED, &ctx->enabled))
+	if (!test_bit(BIT_CLKS_ENABLED, &ctx->flags))
 		goto out;
 
 	val = readl(ctx->addr + DECON_VIDINTCON1);
@@ -541,7 +540,7 @@ static irqreturn_t decon_lcd_sys_irq_handler(int irq, void *dev_id)
 	u32 val;
 	int win;
 
-	if (!test_bit(BIT_CLKS_ENABLED, &ctx->enabled))
+	if (!test_bit(BIT_CLKS_ENABLED, &ctx->flags))
 		goto out;
 
 	val = readl(ctx->addr + DECON_VIDINTCON1);
@@ -576,7 +575,7 @@ static int exynos5433_decon_probe(struct platform_device *pdev)
 	if (!ctx)
 		return -ENOMEM;
 
-	ctx->suspended = true;
+	__set_bit(BIT_SUSPENDED, &ctx->flags);
 	ctx->dev = dev;
 	if (of_get_child_by_name(dev->of_node, "i80-if-timings"))
 		ctx->i80_if = true;
-- 
1.9.1

[PATCH 08/10] drm/exynos/decon5433: remove duplicated initialization

[Andrzej Hajda]

Field .commit is already initialized few lines above.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 265a77f..3c9aa4e 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -448,7 +448,6 @@ static struct exynos_drm_crtc_ops decon_crtc_ops = {
 	.commit			= decon_commit,
 	.enable_vblank		= decon_enable_vblank,
 	.disable_vblank		= decon_disable_vblank,
-	.commit			= decon_commit,
 	.atomic_begin		= decon_atomic_begin,
 	.update_plane		= decon_update_plane,
 	.disable_plane		= decon_disable_plane,
-- 
1.9.1

[PATCH 09/10] dt-bindings: video: exynos5433-decon: add bindings for DECON-TV

[Andrzej Hajda]

DECON-TV(Display and Enhancement Controller for TV) is a variation
of DECON IP. Its main purpose is to produce video stream for HDMI IP.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 .../devicetree/bindings/video/exynos5433-decon.txt  | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/video/exynos5433-decon.txt b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
index 3dff78b..2a88c8d 100644
--- a/Documentation/devicetree/bindings/video/exynos5433-decon.txt
+++ b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
@@ -5,24 +5,27 @@ Exynos series of SoCs which transfers the image data from a video memory
 buffer to an external LCD interface.
 
 Required properties:
-- compatible: value should be "samsung,exynos5433-decon";
+- compatible: value should be one of:
+	"samsung,exynos5433-decon", "samsung,exynos5433-decon-tv";
 - reg: physical base address and length of the DECON registers set.
-- interrupts: should contain a list of all DECON IP block interrupts in the
-	      order: VSYNC, LCD_SYSTEM. The interrupt specifier format
-	      depends on the interrupt controller used.
-- interrupt-names: should contain the interrupt names: "vsync", "lcd_sys"
-		   in the same order as they were listed in the interrupts
-		   property.
+- interrupts: should contain interrupt specifier of VSYNC and optionally
+	      LCD_SYSTEM. The interrupt specifier format depends on
+	      the interrupt controller used.
+- interrupt-names: should contain the interrupt name "vsync" and optionally
+		   "lcd_sys" in the same order as they were listed in
+		   the interrupts property.
 - clocks: must include clock specifiers corresponding to entries in the
 	  clock-names property.
 - clock-names: list of clock names sorted in the same order as the clocks
 	       property. Must contain "pclk", "aclk_decon", "aclk_smmu_decon0x",
 	       "aclk_xiu_decon0x", "pclk_smmu_decon0x", clk_decon_vclk",
 	       "sclk_decon_eclk"
+
+Optional properties:
 - ports: contains a port which is connected to mic node. address-cells and
-	 size-cells must 1 and 0, respectively.
+	 size-cells must be 1 and 0, respectively.
 - port: contains an endpoint node which is connected to the endpoint in the mic
-	node. The reg value muset be 0.
+	node. The reg value must be 0.
 - i80-if-timings: specify whether the panel which is connected to decon uses
 		  i80 lcd interface or mipi video interface. This node contains
 		  no timing information as that of fimd does. Because there is
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 09/10] dt-bindings: video: exynos5433-decon: add bindings for DECON-TV

[Andrzej Hajda]

DECON-TV(Display and Enhancement Controller for TV) is a variation
of DECON IP. Its main purpose is to produce video stream for HDMI IP.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 .../devicetree/bindings/video/exynos5433-decon.txt  | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/video/exynos5433-decon.txt b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
index 3dff78b..2a88c8d 100644
--- a/Documentation/devicetree/bindings/video/exynos5433-decon.txt
+++ b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
@@ -5,24 +5,27 @@ Exynos series of SoCs which transfers the image data from a video memory
 buffer to an external LCD interface.
 
 Required properties:
-- compatible: value should be "samsung,exynos5433-decon";
+- compatible: value should be one of:
+	"samsung,exynos5433-decon", "samsung,exynos5433-decon-tv";
 - reg: physical base address and length of the DECON registers set.
-- interrupts: should contain a list of all DECON IP block interrupts in the
-	      order: VSYNC, LCD_SYSTEM. The interrupt specifier format
-	      depends on the interrupt controller used.
-- interrupt-names: should contain the interrupt names: "vsync", "lcd_sys"
-		   in the same order as they were listed in the interrupts
-		   property.
+- interrupts: should contain interrupt specifier of VSYNC and optionally
+	      LCD_SYSTEM. The interrupt specifier format depends on
+	      the interrupt controller used.
+- interrupt-names: should contain the interrupt name "vsync" and optionally
+		   "lcd_sys" in the same order as they were listed in
+		   the interrupts property.
 - clocks: must include clock specifiers corresponding to entries in the
 	  clock-names property.
 - clock-names: list of clock names sorted in the same order as the clocks
 	       property. Must contain "pclk", "aclk_decon", "aclk_smmu_decon0x",
 	       "aclk_xiu_decon0x", "pclk_smmu_decon0x", clk_decon_vclk",
 	       "sclk_decon_eclk"
+
+Optional properties:
 - ports: contains a port which is connected to mic node. address-cells and
-	 size-cells must 1 and 0, respectively.
+	 size-cells must be 1 and 0, respectively.
 - port: contains an endpoint node which is connected to the endpoint in the mic
-	node. The reg value muset be 0.
+	node. The reg value must be 0.
 - i80-if-timings: specify whether the panel which is connected to decon uses
 		  i80 lcd interface or mipi video interface. This node contains
 		  no timing information as that of fimd does. Because there is
-- 
1.9.1

[PATCH 10/10] drm/exynos/decon5433: add support for DECON-TV

[Andrzej Hajda]

DECON-TV IP is responsible for generating video stream which is transferred
to HDMI IP. It is almost fully compatible with DECON IP.

The patch is based on initial work of Hyungwon Hwang.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 154 ++++++++++++++++----------
 include/video/exynos5433_decon.h              |  29 +++++
 2 files changed, 122 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 3c9aa4e..fbe1b31 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -13,6 +13,7 @@
 #include <linux/platform_device.h>
 #include <linux/clk.h>
 #include <linux/component.h>
+#include <linux/of_device.h>
 #include <linux/of_gpio.h>
 #include <linux/pm_runtime.h>
 
@@ -37,6 +38,12 @@ static const char * const decon_clks_name[] = {
 	"sclk_decon_eclk",
 };
 
+enum decon_iftype {
+	IFTYPE_RGB,
+	IFTYPE_I80,
+	IFTYPE_HDMI
+};
+
 enum decon_flag_bits {
 	BIT_CLKS_ENABLED,
 	BIT_IRQS_ENABLED,
@@ -53,7 +60,8 @@ struct decon_context {
 	struct clk			*clks[ARRAY_SIZE(decon_clks_name)];
 	int				pipe;
 	unsigned long			flags;
-	bool				i80_if;
+	enum decon_iftype		out_type;
+	int				first_win;
 };
 
 static const uint32_t decon_formats[] = {
@@ -80,7 +88,7 @@ static int decon_enable_vblank(struct exynos_drm_crtc *crtc)
 
 	if (test_and_set_bit(BIT_IRQS_ENABLED, &ctx->flags)) {
 		val = VIDINTCON0_INTEN;
-		if (ctx->i80_if)
+		if (ctx->out_type == IFTYPE_I80)
 			val |= VIDINTCON0_FRAMEDONE;
 		else
 			val |= VIDINTCON0_INTFRMEN;
@@ -104,8 +112,11 @@ static void decon_disable_vblank(struct exynos_drm_crtc *crtc)
 
 static void decon_setup_trigger(struct decon_context *ctx)
 {
-	u32 val = TRIGCON_TRIGEN_PER_F | TRIGCON_TRIGEN_F |
-			TRIGCON_TE_AUTO_MASK | TRIGCON_SWTRIGEN;
+	u32 val = (ctx->out_type != IFTYPE_HDMI)
+		? TRIGCON_TRIGEN_PER_F | TRIGCON_TRIGEN_F |
+		  TRIGCON_TE_AUTO_MASK | TRIGCON_SWTRIGEN
+		: TRIGCON_TRIGEN_PER_F | TRIGCON_TRIGEN_F |
+		  TRIGCON_HWTRIGMASK_I80_RGB | TRIGCON_HWTRIGEN_I80_RGB;
 	writel(val, ctx->addr + DECON_TRIGCON);
 }
 
@@ -118,13 +129,22 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
 	if (test_bit(BIT_SUSPENDED, &ctx->flags))
 		return;
 
+	if (ctx->out_type == IFTYPE_HDMI) {
+		m->crtc_hsync_start = m->crtc_hdisplay + 10;
+		m->crtc_hsync_end = m->crtc_htotal - 92;
+		m->crtc_vsync_start = m->crtc_vdisplay + 1;
+		m->crtc_vsync_end = m->crtc_vsync_start + 1;
+	}
+
+	decon_set_bits(ctx, DECON_VIDCON0, VIDCON0_ENVID, 0);
+
 	/* enable clock gate */
 	val = CMU_CLKGAGE_MODE_SFR_F | CMU_CLKGAGE_MODE_MEM_F;
 	writel(val, ctx->addr + DECON_CMU);
 
 	/* lcd on and use command if */
 	val = VIDOUT_LCD_ON;
-	if (ctx->i80_if)
+	if (ctx->out_type == IFTYPE_I80)
 		val |= VIDOUT_COMMAND_IF;
 	else
 		val |= VIDOUT_RGB_IF;
@@ -134,7 +154,7 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
 		VIDTCON2_HOZVAL(m->hdisplay - 1);
 	writel(val, ctx->addr + DECON_VIDTCON2);
 
-	if (!ctx->i80_if) {
+	if (ctx->out_type != IFTYPE_I80) {
 		val = VIDTCON00_VBPD_F(
 				m->crtc_vtotal - m->crtc_vsync_end - 1) |
 			VIDTCON00_VFPD_F(
@@ -159,15 +179,9 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
 	decon_setup_trigger(ctx);
 
 	/* enable output and display signal */
-	val = VIDCON0_ENVID | VIDCON0_ENVID_F;
-	writel(val, ctx->addr + DECON_VIDCON0);
+	decon_set_bits(ctx, DECON_VIDCON0, VIDCON0_ENVID | VIDCON0_ENVID_F, ~0);
 }
 
-#define COORDINATE_X(x)		(((x) & 0xfff) << 12)
-#define COORDINATE_Y(x)		((x) & 0xfff)
-#define OFFSIZE(x)		(((x) & 0x3fff) << 14)
-#define PAGEWIDTH(x)		((x) & 0x3fff)
-
 static void decon_win_set_pixfmt(struct decon_context *ctx, unsigned int win,
 				 struct drm_framebuffer *fb)
 {
@@ -238,6 +252,10 @@ static void decon_atomic_begin(struct exynos_drm_crtc *crtc,
 	decon_shadow_protect_win(ctx, plane->zpos, true);
 }
 
+#define BIT_VAL(x, e, s) (((x) & ((1 << ((e) - (s) + 1)) - 1)) << (s))
+#define COORDINATE_X(x) BIT_VAL((x), 23, 12)
+#define COORDINATE_Y(x) BIT_VAL((x), 11, 0)
+
 static void decon_update_plane(struct exynos_drm_crtc *crtc,
 			       struct exynos_drm_plane *plane)
 {
@@ -271,8 +289,12 @@ static void decon_update_plane(struct exynos_drm_crtc *crtc,
 	val = plane->dma_addr[0] + pitch * plane->crtc_h;
 	writel(val, ctx->addr + DECON_VIDW0xADD1B0(win));
 
-	val = OFFSIZE(pitch - plane->crtc_w * bpp)
-		| PAGEWIDTH(plane->crtc_w * bpp);
+	if (ctx->out_type != IFTYPE_HDMI)
+		val = BIT_VAL(pitch - plane->crtc_w * bpp, 27, 14)
+			| BIT_VAL(plane->crtc_w * bpp, 13, 0);
+	else
+		val = BIT_VAL(pitch - plane->crtc_w * bpp, 29, 15)
+			| BIT_VAL(plane->crtc_w * bpp, 14, 0);
 	writel(val, ctx->addr + DECON_VIDW0xADD2(win));
 
 	decon_win_set_pixfmt(ctx, win, state->fb);
@@ -314,7 +336,7 @@ static void decon_atomic_flush(struct exynos_drm_crtc *crtc,
 
 	decon_shadow_protect_win(ctx, plane->zpos, false);
 
-	if (ctx->i80_if)
+	if (ctx->out_type == IFTYPE_I80)
 		set_bit(BIT_WIN_UPDATED, &ctx->flags);
 }
 
@@ -339,6 +361,17 @@ static void decon_swreset(struct decon_context *ctx)
 	}
 
 	WARN(tries == 0, "failed to software reset DECON\n");
+
+	if (ctx->out_type != IFTYPE_HDMI)
+		return;
+
+	writel(VIDCON0_CLKVALUP | VIDCON0_VLCKFREE, ctx->addr + DECON_VIDCON0);
+	decon_set_bits(ctx, DECON_CMU,
+		       CMU_CLKGAGE_MODE_SFR_F | CMU_CLKGAGE_MODE_MEM_F, ~0);
+	writel(VIDCON1_VCLK_RUN_VDEN_DISABLE, ctx->addr + DECON_VIDCON1);
+	writel(CRCCTRL_CRCEN | CRCCTRL_CRCSTART_F | CRCCTRL_CRCCLKEN,
+	       ctx->addr + DECON_CRCCTRL);
+	decon_setup_trigger(ctx);
 }
 
 static void decon_enable(struct exynos_drm_crtc *crtc)
@@ -387,7 +420,7 @@ static void decon_disable(struct exynos_drm_crtc *crtc)
 	 * suspend that connector. Otherwise we might try to scan from
 	 * a destroyed buffer later.
 	 */
-	for (i = 0; i < WINDOWS_NR; i++)
+	for (i = ctx->first_win; i < WINDOWS_NR; i++)
 		decon_disable_plane(crtc, &ctx->planes[i]);
 
 	decon_swreset(ctx);
@@ -461,25 +494,30 @@ static int decon_bind(struct device *dev, struct device *master, void *data)
 	struct drm_device *drm_dev = data;
 	struct exynos_drm_private *priv = drm_dev->dev_private;
 	struct exynos_drm_plane *exynos_plane;
+	enum exynos_drm_output_type out_type;
 	enum drm_plane_type type;
-	unsigned int zpos;
+	unsigned int win;
 	int ret;
 
 	ctx->drm_dev = drm_dev;
 	ctx->pipe = priv->pipe++;
 
-	for (zpos = 0; zpos < WINDOWS_NR; zpos++) {
-		type = exynos_plane_get_type(zpos, CURSOR_WIN);
-		ret = exynos_plane_init(drm_dev, &ctx->planes[zpos],
+	for (win = ctx->first_win; win < WINDOWS_NR; win++) {
+		int tmp = (win == ctx->first_win) ? 0 : win;
+
+		type = exynos_plane_get_type(tmp, CURSOR_WIN);
+		ret = exynos_plane_init(drm_dev, &ctx->planes[win],
 				1 << ctx->pipe, type, decon_formats,
-				ARRAY_SIZE(decon_formats), zpos);
+				ARRAY_SIZE(decon_formats), win);
 		if (ret)
 			return ret;
 	}
 
-	exynos_plane = &ctx->planes[DEFAULT_WIN];
+	exynos_plane = &ctx->planes[ctx->first_win];
+	out_type = (ctx->out_type == IFTYPE_HDMI) ? EXYNOS_DISPLAY_TYPE_HDMI
+						  : EXYNOS_DISPLAY_TYPE_LCD;
 	ctx->crtc = exynos_drm_crtc_create(drm_dev, &exynos_plane->base,
-					ctx->pipe, EXYNOS_DISPLAY_TYPE_LCD,
+					ctx->pipe, out_type,
 					&decon_crtc_ops, ctx);
 	if (IS_ERR(ctx->crtc)) {
 		ret = PTR_ERR(ctx->crtc);
@@ -513,27 +551,7 @@ static const struct component_ops decon_component_ops = {
 	.unbind = decon_unbind,
 };
 
-static irqreturn_t decon_vsync_irq_handler(int irq, void *dev_id)
-{
-	struct decon_context *ctx = dev_id;
-	u32 val;
-
-	if (!test_bit(BIT_CLKS_ENABLED, &ctx->flags))
-		goto out;
-
-	val = readl(ctx->addr + DECON_VIDINTCON1);
-	if (val & VIDINTCON1_INTFRMPEND) {
-		drm_crtc_handle_vblank(&ctx->crtc->base);
-
-		/* clear */
-		writel(VIDINTCON1_INTFRMPEND, ctx->addr + DECON_VIDINTCON1);
-	}
-
-out:
-	return IRQ_HANDLED;
-}
-
-static irqreturn_t decon_lcd_sys_irq_handler(int irq, void *dev_id)
+static irqreturn_t decon_irq_handler(int irq, void *dev_id)
 {
 	struct decon_context *ctx = dev_id;
 	u32 val;
@@ -543,8 +561,10 @@ static irqreturn_t decon_lcd_sys_irq_handler(int irq, void *dev_id)
 		goto out;
 
 	val = readl(ctx->addr + DECON_VIDINTCON1);
-	if (val & VIDINTCON1_INTFRMDONEPEND) {
-		for (win = 0 ; win < WINDOWS_NR ; win++) {
+	val &= VIDINTCON1_INTFRMDONEPEND | VIDINTCON1_INTFRMPEND;
+
+	if (val) {
+		for (win = ctx->first_win; win < WINDOWS_NR ; win++) {
 			struct exynos_drm_plane *plane = &ctx->planes[win];
 
 			if (!plane->pending_fb)
@@ -554,16 +574,29 @@ static irqreturn_t decon_lcd_sys_irq_handler(int irq, void *dev_id)
 		}
 
 		/* clear */
-		writel(VIDINTCON1_INTFRMDONEPEND,
-				ctx->addr + DECON_VIDINTCON1);
+		writel(val, ctx->addr + DECON_VIDINTCON1);
 	}
 
 out:
 	return IRQ_HANDLED;
 }
 
+static const struct of_device_id exynos5433_decon_driver_dt_match[] = {
+	{
+		.compatible = "samsung,exynos5433-decon",
+		.data = (void *)IFTYPE_RGB
+	},
+	{
+		.compatible = "samsung,exynos5433-decon-tv",
+		.data = (void *)IFTYPE_HDMI
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, exynos5433_decon_driver_dt_match);
+
 static int exynos5433_decon_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *of_id;
 	struct device *dev = &pdev->dev;
 	struct decon_context *ctx;
 	struct resource *res;
@@ -576,8 +609,14 @@ static int exynos5433_decon_probe(struct platform_device *pdev)
 
 	__set_bit(BIT_SUSPENDED, &ctx->flags);
 	ctx->dev = dev;
-	if (of_get_child_by_name(dev->of_node, "i80-if-timings"))
-		ctx->i80_if = true;
+
+	of_id = of_match_device(exynos5433_decon_driver_dt_match, &pdev->dev);
+	ctx->out_type = (enum decon_iftype)of_id->data;
+
+	if (ctx->out_type == IFTYPE_HDMI)
+		ctx->first_win = 1;
+	else if (of_get_child_by_name(dev->of_node, "i80-if-timings"))
+		ctx->out_type = IFTYPE_I80;
 
 	for (i = 0; i < ARRAY_SIZE(decon_clks_name); i++) {
 		struct clk *clk;
@@ -602,15 +641,14 @@ static int exynos5433_decon_probe(struct platform_device *pdev)
 	}
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_IRQ,
-			ctx->i80_if ? "lcd_sys" : "vsync");
+			(ctx->out_type == IFTYPE_I80) ? "lcd_sys" : "vsync");
 	if (!res) {
 		dev_err(dev, "cannot find IRQ resource\n");
 		return -ENXIO;
 	}
 
-	ret = devm_request_irq(dev, res->start, ctx->i80_if ?
-			decon_lcd_sys_irq_handler : decon_vsync_irq_handler, 0,
-			"drm_decon", ctx);
+	ret = devm_request_irq(dev, res->start, decon_irq_handler, 0,
+			       "drm_decon", ctx);
 	if (ret < 0) {
 		dev_err(dev, "lcd_sys irq request failed\n");
 		return ret;
@@ -641,12 +679,6 @@ static int exynos5433_decon_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct of_device_id exynos5433_decon_driver_dt_match[] = {
-	{ .compatible = "samsung,exynos5433-decon" },
-	{},
-};
-MODULE_DEVICE_TABLE(of, exynos5433_decon_driver_dt_match);
-
 struct platform_driver exynos5433_decon_driver = {
 	.probe		= exynos5433_decon_probe,
 	.remove		= exynos5433_decon_remove,
diff --git a/include/video/exynos5433_decon.h b/include/video/exynos5433_decon.h
index 3696575..c1c1ca1 100644
--- a/include/video/exynos5433_decon.h
+++ b/include/video/exynos5433_decon.h
@@ -82,6 +82,8 @@
 
 /* VIDCON0 */
 #define VIDCON0_SWRESET			(1 << 28)
+#define VIDCON0_CLKVALUP		(1 << 14)
+#define VIDCON0_VLCKFREE		(1 << 5)
 #define VIDCON0_STOP_STATUS		(1 << 2)
 #define VIDCON0_ENVID			(1 << 1)
 #define VIDCON0_ENVID_F			(1 << 0)
@@ -137,6 +139,13 @@
 /* DECON_UPDATE */
 #define STANDALONE_UPDATE_F		(1 << 0)
 
+/* DECON_VIDCON1 */
+#define VIDCON1_VCLK_MASK		(0x3 << 9)
+#define VIDCON1_VCLK_RUN_VDEN_DISABLE	(0x3 << 9)
+#define VIDCON1_VCLK_HOLD		(0x0 << 9)
+#define VIDCON1_VCLK_RUN		(0x1 << 9)
+
+
 /* DECON_VIDTCON00 */
 #define VIDTCON00_VBPD_F(x)		(((x) & 0xfff) << 16)
 #define VIDTCON00_VFPD_F(x)		((x) & 0xfff)
@@ -159,7 +168,27 @@
 #define TRIGCON_TRIGEN_PER_F		(1 << 31)
 #define TRIGCON_TRIGEN_F		(1 << 30)
 #define TRIGCON_TE_AUTO_MASK		(1 << 29)
+#define TRIGCON_WB_SWTRIGCMD		(1 << 28)
+#define TRIGCON_SWTRIGCMD_W4BUF		(1 << 26)
+#define TRIGCON_TRIGMODE_W4BUF		(1 << 25)
+#define TRIGCON_SWTRIGCMD_W3BUF		(1 << 21)
+#define TRIGCON_TRIGMODE_W3BUF		(1 << 20)
+#define TRIGCON_SWTRIGCMD_W2BUF		(1 << 16)
+#define TRIGCON_TRIGMODE_W2BUF		(1 << 15)
+#define TRIGCON_SWTRIGCMD_W1BUF		(1 << 11)
+#define TRIGCON_TRIGMODE_W1BUF		(1 << 10)
+#define TRIGCON_SWTRIGCMD_W0BUF		(1 << 6)
+#define TRIGCON_TRIGMODE_W0BUF		(1 << 5)
+#define TRIGCON_HWTRIGMASK_I80_RGB	(1 << 4)
+#define TRIGCON_HWTRIGEN_I80_RGB	(1 << 3)
+#define TRIGCON_HWTRIG_INV_I80_RGB	(1 << 2)
 #define TRIGCON_SWTRIGCMD		(1 << 1)
 #define TRIGCON_SWTRIGEN		(1 << 0)
 
+/* DECON_CRCCTRL */
+#define CRCCTRL_CRCCLKEN		(0x1 << 2)
+#define CRCCTRL_CRCSTART_F		(0x1 << 1)
+#define CRCCTRL_CRCEN			(0x1 << 0)
+#define CRCCTRL_MASK			(0x7)
+
 #endif /* EXYNOS_REGS_DECON_H */
-- 
1.9.1

Re: [PATCH 01/10] clk/samsung: exynos5433: add definitions of HDMI-PHY output clocks

[Michael Turquette]

Quoting Andrzej Hajda (2015-10-20 02:22:32)
> HDMI driver must re-parent respective muxes during HDMI-PHY on/off
> to HDMI-PHY output clocks. To reference those clocks their
> definitions should be added.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/clk/samsung/clk-exynos5433.c   | 6 ++++--
>  include/dt-bindings/clock/exynos5433.h | 5 ++++-
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/samsung/clk-exynos5433.c b/drivers/clk/samsung/clk-exynos5433.c
> index 650ec13..e037406 100644
> --- a/drivers/clk/samsung/clk-exynos5433.c
> +++ b/drivers/clk/samsung/clk-exynos5433.c
> @@ -2614,8 +2614,10 @@ static struct samsung_fixed_rate_clock disp_fixed_clks[] __initdata = {
>         FRATE(0, "phyclk_mipidphy0_rxclkesc0_phy", NULL, CLK_IS_ROOT,
>                         100000000),
>         /* PHY clocks from HDMI_PHY */
> -       FRATE(0, "phyclk_hdmiphy_tmds_clko_phy", NULL, CLK_IS_ROOT, 300000000),
> -       FRATE(0, "phyclk_hdmiphy_pixel_clko_phy", NULL, CLK_IS_ROOT, 166000000),
> +       FRATE(CLK_PHYCLK_HDMIPHY_TMDS_CLKO_PHY, "phyclk_hdmiphy_tmds_clko_phy",
> +                       NULL, CLK_IS_ROOT, 300000000),
> +       FRATE(CLK_PHYCLK_HDMIPHY_PIXEL_CLKO_PHY, "phyclk_hdmiphy_pixel_clko_phy",
> +                       NULL, CLK_IS_ROOT, 166000000),
>  };
>  
>  static struct samsung_mux_clock disp_mux_clks[] __initdata = {
> diff --git a/include/dt-bindings/clock/exynos5433.h b/include/dt-bindings/clock/exynos5433.h
> index 5bd80d5..4f0d566 100644
> --- a/include/dt-bindings/clock/exynos5433.h
> +++ b/include/dt-bindings/clock/exynos5433.h
> @@ -765,7 +765,10 @@
>  #define CLK_SCLK_RGB_VCLK                              109
>  #define CLK_SCLK_RGB_TV_VCLK                           110
>  
> -#define DISP_NR_CLK                                    111
> +#define CLK_PHYCLK_HDMIPHY_PIXEL_CLKO_PHY              111
> +#define CLK_PHYCLK_HDMIPHY_TMDS_CLKO_PHY               112
> +
> +#define DISP_NR_CLK                                    113

Why break compatibility with older DTBs?

Regards,
Mike

>  
>  /* CMU_AUD */
>  #define CLK_MOUT_AUD_PLL_USER                          1
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/10] clk/samsung: exynos5433: add definitions of HDMI-PHY output clocks

[Michael Turquette]

Quoting Andrzej Hajda (2015-10-20 02:22:32)
> HDMI driver must re-parent respective muxes during HDMI-PHY on/off
> to HDMI-PHY output clocks. To reference those clocks their
> definitions should be added.
> =

> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/clk/samsung/clk-exynos5433.c   | 6 ++++--
>  include/dt-bindings/clock/exynos5433.h | 5 ++++-
>  2 files changed, 8 insertions(+), 3 deletions(-)
> =

> diff --git a/drivers/clk/samsung/clk-exynos5433.c b/drivers/clk/samsung/c=
lk-exynos5433.c
> index 650ec13..e037406 100644
> --- a/drivers/clk/samsung/clk-exynos5433.c
> +++ b/drivers/clk/samsung/clk-exynos5433.c
> @@ -2614,8 +2614,10 @@ static struct samsung_fixed_rate_clock disp_fixed_=
clks[] __initdata =3D {
>         FRATE(0, "phyclk_mipidphy0_rxclkesc0_phy", NULL, CLK_IS_ROOT,
>                         100000000),
>         /* PHY clocks from HDMI_PHY */
> -       FRATE(0, "phyclk_hdmiphy_tmds_clko_phy", NULL, CLK_IS_ROOT, 30000=
0000),
> -       FRATE(0, "phyclk_hdmiphy_pixel_clko_phy", NULL, CLK_IS_ROOT, 1660=
00000),
> +       FRATE(CLK_PHYCLK_HDMIPHY_TMDS_CLKO_PHY, "phyclk_hdmiphy_tmds_clko=
_phy",
> +                       NULL, CLK_IS_ROOT, 300000000),
> +       FRATE(CLK_PHYCLK_HDMIPHY_PIXEL_CLKO_PHY, "phyclk_hdmiphy_pixel_cl=
ko_phy",
> +                       NULL, CLK_IS_ROOT, 166000000),
>  };
>  =

>  static struct samsung_mux_clock disp_mux_clks[] __initdata =3D {
> diff --git a/include/dt-bindings/clock/exynos5433.h b/include/dt-bindings=
/clock/exynos5433.h
> index 5bd80d5..4f0d566 100644
> --- a/include/dt-bindings/clock/exynos5433.h
> +++ b/include/dt-bindings/clock/exynos5433.h
> @@ -765,7 +765,10 @@
>  #define CLK_SCLK_RGB_VCLK                              109
>  #define CLK_SCLK_RGB_TV_VCLK                           110
>  =

> -#define DISP_NR_CLK                                    111
> +#define CLK_PHYCLK_HDMIPHY_PIXEL_CLKO_PHY              111
> +#define CLK_PHYCLK_HDMIPHY_TMDS_CLKO_PHY               112
> +
> +#define DISP_NR_CLK                                    113

Why break compatibility with older DTBs?

Regards,
Mike

>  =

>  /* CMU_AUD */
>  #define CLK_MOUT_AUD_PLL_USER                          1
> -- =

> 1.9.1
> =

> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/10] clk/samsung: exynos5433: add definitions of HDMI-PHY output clocks

[Marek Szyprowski]

Hello,

On 2015-10-20 12:34, Michael Turquette wrote:
> Quoting Andrzej Hajda (2015-10-20 02:22:32)
>> HDMI driver must re-parent respective muxes during HDMI-PHY on/off
>> to HDMI-PHY output clocks. To reference those clocks their
>> definitions should be added.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>>   drivers/clk/samsung/clk-exynos5433.c   | 6 ++++--
>>   include/dt-bindings/clock/exynos5433.h | 5 ++++-
>>   2 files changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/clk/samsung/clk-exynos5433.c b/drivers/clk/samsung/clk-exynos5433.c
>> index 650ec13..e037406 100644
>> --- a/drivers/clk/samsung/clk-exynos5433.c
>> +++ b/drivers/clk/samsung/clk-exynos5433.c
>> @@ -2614,8 +2614,10 @@ static struct samsung_fixed_rate_clock disp_fixed_clks[] __initdata = {
>>          FRATE(0, "phyclk_mipidphy0_rxclkesc0_phy", NULL, CLK_IS_ROOT,
>>                          100000000),
>>          /* PHY clocks from HDMI_PHY */
>> -       FRATE(0, "phyclk_hdmiphy_tmds_clko_phy", NULL, CLK_IS_ROOT, 300000000),
>> -       FRATE(0, "phyclk_hdmiphy_pixel_clko_phy", NULL, CLK_IS_ROOT, 166000000),
>> +       FRATE(CLK_PHYCLK_HDMIPHY_TMDS_CLKO_PHY, "phyclk_hdmiphy_tmds_clko_phy",
>> +                       NULL, CLK_IS_ROOT, 300000000),
>> +       FRATE(CLK_PHYCLK_HDMIPHY_PIXEL_CLKO_PHY, "phyclk_hdmiphy_pixel_clko_phy",
>> +                       NULL, CLK_IS_ROOT, 166000000),
>>   };
>>   
>>   static struct samsung_mux_clock disp_mux_clks[] __initdata = {
>> diff --git a/include/dt-bindings/clock/exynos5433.h b/include/dt-bindings/clock/exynos5433.h
>> index 5bd80d5..4f0d566 100644
>> --- a/include/dt-bindings/clock/exynos5433.h
>> +++ b/include/dt-bindings/clock/exynos5433.h
>> @@ -765,7 +765,10 @@
>>   #define CLK_SCLK_RGB_VCLK                              109
>>   #define CLK_SCLK_RGB_TV_VCLK                           110
>>   
>> -#define DISP_NR_CLK                                    111
>> +#define CLK_PHYCLK_HDMIPHY_PIXEL_CLKO_PHY              111
>> +#define CLK_PHYCLK_HDMIPHY_TMDS_CLKO_PHY               112
>> +
>> +#define DISP_NR_CLK                                    113
> Why break compatibility with older DTBs?

This patch just adds support for 2 more clocks to exynos 5433 clk driver,
which were previously undefined. How this break compatibility with older 
DTBs?

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH 01/10] clk/samsung: exynos5433: add definitions of HDMI-PHY output clocks

[Sylwester Nawrocki]

On 20/10/15 12:34, Michael Turquette wrote:
>> diff --git a/include/dt-bindings/clock/exynos5433.h b/include/dt-bindings/clock/exynos5433.h
>> > index 5bd80d5..4f0d566 100644
>> > --- a/include/dt-bindings/clock/exynos5433.h
>> > +++ b/include/dt-bindings/clock/exynos5433.h
>> > @@ -765,7 +765,10 @@
>> >  #define CLK_SCLK_RGB_VCLK                              109
>> >  #define CLK_SCLK_RGB_TV_VCLK                           110
>> >  
>> > -#define DISP_NR_CLK                                    111
>> > +#define CLK_PHYCLK_HDMIPHY_PIXEL_CLKO_PHY              111
>> > +#define CLK_PHYCLK_HDMIPHY_TMDS_CLKO_PHY               112
>> > +
>> > +#define DISP_NR_CLK                                    113
>
> Why break compatibility with older DTBs?

I used to be resistant to changing those _NR_CLK defines
in the past but then realized they are not part of the DT ABI.
These defines are used only in drivers and affect only size
of the provider's allocated clock array. The confusion may be
caused by the fact that the whole header is shared by the kernel
source and dts.

$ git grep -l _NR_CLK arch/arm/boot/dts drivers/clk/samsung/
drivers/clk/samsung/clk-exynos-clkout.c
drivers/clk/samsung/clk-exynos3250.c
drivers/clk/samsung/clk-exynos4.c
drivers/clk/samsung/clk-exynos4415.c
drivers/clk/samsung/clk-exynos5250.c
drivers/clk/samsung/clk-exynos5260.c
drivers/clk/samsung/clk-exynos5410.c
drivers/clk/samsung/clk-exynos5420.c
drivers/clk/samsung/clk-exynos5433.c
drivers/clk/samsung/clk-exynos5440.c
drivers/clk/samsung/clk-exynos7.c

There is no *_NR_CLK in any dts file.
New kernel will work will older DTB, the driver will just
register more clocks, which will not be dereferenced anywhere
in older dtb.

-- 
Regards,
Sylwester

Re: [PATCH 02/10] clk/samsung: exynos5433: add pclk_decon clock

[Krzysztof Kozlowski]

2015-10-20 18:22 GMT+09:00 Andrzej Hajda <a.hajda@samsung.com>:
> This undocumented gate clock is used by DECON IP.
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/clk/samsung/clk-exynos5433.c   | 2 ++
>  include/dt-bindings/clock/exynos5433.h | 4 +++-
>  2 files changed, 5 insertions(+), 1 deletion(-)
>

Indeed looks undocumented... but vendor tree has it so it makes sense to me:

Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

Re: [PATCH 00/10] drm/exynos/decon5433: add support to DECON-TV

[Krzysztof Kozlowski]

2015-10-20 18:22 GMT+09:00 Andrzej Hajda <a.hajda@samsung.com>:
> Hi Inki,
>
> This patchset adds support to DECON-TV in Exynos5433 SoC.
> The main patch is prepended with few preparation patches:
> - add three clocks required by HDMI pipeline,
> - small bindings update,
> - driver cleanup.
>
> The patchset is based on the latest exynos-drm-next branch.

So I am assuming this will go through Exynos drm tree. Please ping me
if you need something from my side (except dt-bindings I guess?).

Best regards,
Krzysztof

Re: [PATCH 04/10] dt-bindings: video: add PCLK clock entry to exynos5433-decon

[Krzysztof Kozlowski]

W dniu 20.10.2015 o 18:22, Andrzej Hajda pisze:
> DECON IP requires this clock to access configuration registers.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  Documentation/devicetree/bindings/video/exynos5433-decon.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/video/exynos5433-decon.txt b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
> index 377afbf..3dff78b 100644
> --- a/Documentation/devicetree/bindings/video/exynos5433-decon.txt
> +++ b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
> @@ -16,7 +16,7 @@ Required properties:
>  - clocks: must include clock specifiers corresponding to entries in the
>  	  clock-names property.
>  - clock-names: list of clock names sorted in the same order as the clocks
> -	       property. Must contain "aclk_decon", "aclk_smmu_decon0x",
> +	       property. Must contain "pclk", "aclk_decon", "aclk_smmu_decon0x",

I assume that old DTB wouldn't work at all, so there is no point in
maintaining ABI compatibility?

Best regards,
Krzysztof

Re: [PATCH 09/10] dt-bindings: video: exynos5433-decon: add bindings for DECON-TV

[Krzysztof Kozlowski]

W dniu 20.10.2015 o 18:22, Andrzej Hajda pisze:
> DECON-TV(Display and Enhancement Controller for TV) is a variation
> of DECON IP. Its main purpose is to produce video stream for HDMI IP.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  .../devicetree/bindings/video/exynos5433-decon.txt  | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/video/exynos5433-decon.txt b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
> index 3dff78b..2a88c8d 100644
> --- a/Documentation/devicetree/bindings/video/exynos5433-decon.txt
> +++ b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
> @@ -5,24 +5,27 @@ Exynos series of SoCs which transfers the image data from a video memory
>  buffer to an external LCD interface.
>  
>  Required properties:
> -- compatible: value should be "samsung,exynos5433-decon";
> +- compatible: value should be one of:
> +	"samsung,exynos5433-decon", "samsung,exynos5433-decon-tv";

Until this point it looked good.

>  - reg: physical base address and length of the DECON registers set.
> -- interrupts: should contain a list of all DECON IP block interrupts in the
> -	      order: VSYNC, LCD_SYSTEM. The interrupt specifier format
> -	      depends on the interrupt controller used.
> -- interrupt-names: should contain the interrupt names: "vsync", "lcd_sys"
> -		   in the same order as they were listed in the interrupts
> -		   property.
> +- interrupts: should contain interrupt specifier of VSYNC and optionally
> +	      LCD_SYSTEM. The interrupt specifier format depends on
> +	      the interrupt controller used.
> +- interrupt-names: should contain the interrupt name "vsync" and optionally
> +		   "lcd_sys" in the same order as they were listed in
> +		   the interrupts property.

The driver already did not require both interrupts, right? Only one of them?

>  - clocks: must include clock specifiers corresponding to entries in the
>  	  clock-names property.
>  - clock-names: list of clock names sorted in the same order as the clocks
>  	       property. Must contain "pclk", "aclk_decon", "aclk_smmu_decon0x",
>  	       "aclk_xiu_decon0x", "pclk_smmu_decon0x", clk_decon_vclk",
>  	       "sclk_decon_eclk"
> +
> +Optional properties:
>  - ports: contains a port which is connected to mic node. address-cells and
> -	 size-cells must 1 and 0, respectively.
> +	 size-cells must be 1 and 0, respectively.
>  - port: contains an endpoint node which is connected to the endpoint in the mic
> -	node. The reg value muset be 0.
> +	node. The reg value must be 0.
>  - i80-if-timings: specify whether the panel which is connected to decon uses
>  		  i80 lcd interface or mipi video interface. This node contains
>  		  no timing information as that of fimd does. Because there is
> 

This is cleanup, please split it.

Best regards,
Krzysztof
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 09/10] dt-bindings: video: exynos5433-decon: add bindings for DECON-TV

[Krzysztof Kozlowski]

W dniu 20.10.2015 o 18:22, Andrzej Hajda pisze:
> DECON-TV(Display and Enhancement Controller for TV) is a variation
> of DECON IP. Its main purpose is to produce video stream for HDMI IP.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  .../devicetree/bindings/video/exynos5433-decon.txt  | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/video/exynos5433-decon.txt b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
> index 3dff78b..2a88c8d 100644
> --- a/Documentation/devicetree/bindings/video/exynos5433-decon.txt
> +++ b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
> @@ -5,24 +5,27 @@ Exynos series of SoCs which transfers the image data from a video memory
>  buffer to an external LCD interface.
>  
>  Required properties:
> -- compatible: value should be "samsung,exynos5433-decon";
> +- compatible: value should be one of:
> +	"samsung,exynos5433-decon", "samsung,exynos5433-decon-tv";

Until this point it looked good.

>  - reg: physical base address and length of the DECON registers set.
> -- interrupts: should contain a list of all DECON IP block interrupts in the
> -	      order: VSYNC, LCD_SYSTEM. The interrupt specifier format
> -	      depends on the interrupt controller used.
> -- interrupt-names: should contain the interrupt names: "vsync", "lcd_sys"
> -		   in the same order as they were listed in the interrupts
> -		   property.
> +- interrupts: should contain interrupt specifier of VSYNC and optionally
> +	      LCD_SYSTEM. The interrupt specifier format depends on
> +	      the interrupt controller used.
> +- interrupt-names: should contain the interrupt name "vsync" and optionally
> +		   "lcd_sys" in the same order as they were listed in
> +		   the interrupts property.

The driver already did not require both interrupts, right? Only one of them?

>  - clocks: must include clock specifiers corresponding to entries in the
>  	  clock-names property.
>  - clock-names: list of clock names sorted in the same order as the clocks
>  	       property. Must contain "pclk", "aclk_decon", "aclk_smmu_decon0x",
>  	       "aclk_xiu_decon0x", "pclk_smmu_decon0x", clk_decon_vclk",
>  	       "sclk_decon_eclk"
> +
> +Optional properties:
>  - ports: contains a port which is connected to mic node. address-cells and
> -	 size-cells must 1 and 0, respectively.
> +	 size-cells must be 1 and 0, respectively.
>  - port: contains an endpoint node which is connected to the endpoint in the mic
> -	node. The reg value muset be 0.
> +	node. The reg value must be 0.
>  - i80-if-timings: specify whether the panel which is connected to decon uses
>  		  i80 lcd interface or mipi video interface. This node contains
>  		  no timing information as that of fimd does. Because there is
> 

This is cleanup, please split it.

Best regards,
Krzysztof

Re: [PATCH 04/10] dt-bindings: video: add PCLK clock entry to exynos5433-decon

[Andrzej Hajda]

On 10/20/2015 02:24 PM, Krzysztof Kozlowski wrote:
> W dniu 20.10.2015 o 18:22, Andrzej Hajda pisze:
>> DECON IP requires this clock to access configuration registers.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>>  Documentation/devicetree/bindings/video/exynos5433-decon.txt | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/video/exynos5433-decon.txt b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>> index 377afbf..3dff78b 100644
>> --- a/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>> +++ b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>> @@ -16,7 +16,7 @@ Required properties:
>>  - clocks: must include clock specifiers corresponding to entries in the
>>  	  clock-names property.
>>  - clock-names: list of clock names sorted in the same order as the clocks
>> -	       property. Must contain "aclk_decon", "aclk_smmu_decon0x",
>> +	       property. Must contain "pclk", "aclk_decon", "aclk_smmu_decon0x",
> I assume that old DTB wouldn't work at all, so there is no point in
> maintaining ABI compatibility?

Yes, Exynos 5433 has not yet landed in mainline.

Regards
Andrzej

>
> Best regards,
> Krzysztof
>
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 04/10] dt-bindings: video: add PCLK clock entry to exynos5433-decon

[Andrzej Hajda]

On 10/20/2015 02:24 PM, Krzysztof Kozlowski wrote:
> W dniu 20.10.2015 o 18:22, Andrzej Hajda pisze:
>> DECON IP requires this clock to access configuration registers.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>>  Documentation/devicetree/bindings/video/exynos5433-decon.txt | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/video/exynos5433-decon.txt b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>> index 377afbf..3dff78b 100644
>> --- a/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>> +++ b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>> @@ -16,7 +16,7 @@ Required properties:
>>  - clocks: must include clock specifiers corresponding to entries in the
>>  	  clock-names property.
>>  - clock-names: list of clock names sorted in the same order as the clocks
>> -	       property. Must contain "aclk_decon", "aclk_smmu_decon0x",
>> +	       property. Must contain "pclk", "aclk_decon", "aclk_smmu_decon0x",
> I assume that old DTB wouldn't work at all, so there is no point in
> maintaining ABI compatibility?

Yes, Exynos 5433 has not yet landed in mainline.

Regards
Andrzej

>
> Best regards,
> Krzysztof
>
>

Re: [PATCH 04/10] dt-bindings: video: add PCLK clock entry to exynos5433-decon

[Krzysztof Kozlowski]

2015-10-20 21:41 GMT+09:00 Andrzej Hajda <a.hajda@samsung.com>:
> On 10/20/2015 02:24 PM, Krzysztof Kozlowski wrote:
>> W dniu 20.10.2015 o 18:22, Andrzej Hajda pisze:
>>> DECON IP requires this clock to access configuration registers.
>>>
>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>> ---
>>>  Documentation/devicetree/bindings/video/exynos5433-decon.txt | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/video/exynos5433-decon.txt b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>>> index 377afbf..3dff78b 100644
>>> --- a/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>>> +++ b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>>> @@ -16,7 +16,7 @@ Required properties:
>>>  - clocks: must include clock specifiers corresponding to entries in the
>>>        clock-names property.
>>>  - clock-names: list of clock names sorted in the same order as the clocks
>>> -           property. Must contain "aclk_decon", "aclk_smmu_decon0x",
>>> +           property. Must contain "pclk", "aclk_decon", "aclk_smmu_decon0x",
>> I assume that old DTB wouldn't work at all, so there is no point in
>> maintaining ABI compatibility?
>
> Yes, Exynos 5433 has not yet landed in mainline.

No, no. I was asking if something using these compatibles would work
previously or not? The driver provided a compatible, an ABI. Now you
require additional field for the same compatible, so the ABI changes.

Best regards,
Krzysztof

Re: [PATCH 04/10] dt-bindings: video: add PCLK clock entry to exynos5433-decon

[Sylwester Nawrocki]

On 20/10/15 14:24, Krzysztof Kozlowski wrote:
> W dniu 20.10.2015 o 18:22, Andrzej Hajda pisze:
>> > DECON IP requires this clock to access configuration registers.
>> > 
>> > Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> > ---
>> >  Documentation/devicetree/bindings/video/exynos5433-decon.txt | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > 
>> > diff --git a/Documentation/devicetree/bindings/video/exynos5433-decon.txt b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>> > index 377afbf..3dff78b 100644
>> > --- a/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>> > +++ b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>> > @@ -16,7 +16,7 @@ Required properties:
>> >  - clocks: must include clock specifiers corresponding to entries in the
>> >  	  clock-names property.
>> >  - clock-names: list of clock names sorted in the same order as the clocks
>> > -	       property. Must contain "aclk_decon", "aclk_smmu_decon0x",
>> > +	       property. Must contain "pclk", "aclk_decon", "aclk_smmu_decon0x",
>
> I assume that old DTB wouldn't work at all, so there is no point in
> maintaining ABI compatibility?

As you know there is no single exynos5433 board dts that would use
the DECON IP block in mainline yet. I doubt anyone at this point
in mainline cares whether we require this additional clock or not.

-- 
Thanks,
Sylwester

Re: [PATCH 09/10] dt-bindings: video: exynos5433-decon: add bindings for DECON-TV

[Andrzej Hajda]

On 10/20/2015 02:30 PM, Krzysztof Kozlowski wrote:
> W dniu 20.10.2015 o 18:22, Andrzej Hajda pisze:
>> DECON-TV(Display and Enhancement Controller for TV) is a variation
>> of DECON IP. Its main purpose is to produce video stream for HDMI IP.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>>  .../devicetree/bindings/video/exynos5433-decon.txt  | 21 ++++++++++++---------
>>  1 file changed, 12 insertions(+), 9 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/video/exynos5433-decon.txt b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>> index 3dff78b..2a88c8d 100644
>> --- a/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>> +++ b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>> @@ -5,24 +5,27 @@ Exynos series of SoCs which transfers the image data from a video memory
>>  buffer to an external LCD interface.
>>  
>>  Required properties:
>> -- compatible: value should be "samsung,exynos5433-decon";
>> +- compatible: value should be one of:
>> +	"samsung,exynos5433-decon", "samsung,exynos5433-decon-tv";
> Until this point it looked good.
>
>>  - reg: physical base address and length of the DECON registers set.
>> -- interrupts: should contain a list of all DECON IP block interrupts in the
>> -	      order: VSYNC, LCD_SYSTEM. The interrupt specifier format
>> -	      depends on the interrupt controller used.
>> -- interrupt-names: should contain the interrupt names: "vsync", "lcd_sys"
>> -		   in the same order as they were listed in the interrupts
>> -		   property.
>> +- interrupts: should contain interrupt specifier of VSYNC and optionally
>> +	      LCD_SYSTEM. The interrupt specifier format depends on
>> +	      the interrupt controller used.
>> +- interrupt-names: should contain the interrupt name "vsync" and optionally
>> +		   "lcd_sys" in the same order as they were listed in
>> +		   the interrupts property.
> The driver already did not require both interrupts, right? Only one of them?

Right. More precisely it did not require since beginning.

>
>>  - clocks: must include clock specifiers corresponding to entries in the
>>  	  clock-names property.
>>  - clock-names: list of clock names sorted in the same order as the clocks
>>  	       property. Must contain "pclk", "aclk_decon", "aclk_smmu_decon0x",
>>  	       "aclk_xiu_decon0x", "pclk_smmu_decon0x", clk_decon_vclk",
>>  	       "sclk_decon_eclk"
>> +
>> +Optional properties:
>>  - ports: contains a port which is connected to mic node. address-cells and
>> -	 size-cells must 1 and 0, respectively.
>> +	 size-cells must be 1 and 0, respectively.
>>  - port: contains an endpoint node which is connected to the endpoint in the mic
>> -	node. The reg value muset be 0.
>> +	node. The reg value must be 0.
>>  - i80-if-timings: specify whether the panel which is connected to decon uses
>>  		  i80 lcd interface or mipi video interface. This node contains
>>  		  no timing information as that of fimd does. Because there is
>>
> This is cleanup, please split it.

Ok. I will split it into cleanup/fix patch, and 2nd one adding compatible.

Regards
Andrzej
>
> Best regards,
> Krzysztof
>

Re: [PATCH 09/10] dt-bindings: video: exynos5433-decon: add bindings for DECON-TV

[Krzysztof Kozlowski]

2015-10-20 21:53 GMT+09:00 Andrzej Hajda <a.hajda@samsung.com>:
> On 10/20/2015 02:30 PM, Krzysztof Kozlowski wrote:
>> W dniu 20.10.2015 o 18:22, Andrzej Hajda pisze:
>>> DECON-TV(Display and Enhancement Controller for TV) is a variation
>>> of DECON IP. Its main purpose is to produce video stream for HDMI IP.
>>>
>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>> ---
>>>  .../devicetree/bindings/video/exynos5433-decon.txt  | 21 ++++++++++++---------
>>>  1 file changed, 12 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/video/exynos5433-decon.txt b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>>> index 3dff78b..2a88c8d 100644
>>> --- a/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>>> +++ b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>>> @@ -5,24 +5,27 @@ Exynos series of SoCs which transfers the image data from a video memory
>>>  buffer to an external LCD interface.
>>>
>>>  Required properties:
>>> -- compatible: value should be "samsung,exynos5433-decon";
>>> +- compatible: value should be one of:
>>> +    "samsung,exynos5433-decon", "samsung,exynos5433-decon-tv";
>> Until this point it looked good.
>>
>>>  - reg: physical base address and length of the DECON registers set.
>>> -- interrupts: should contain a list of all DECON IP block interrupts in the
>>> -          order: VSYNC, LCD_SYSTEM. The interrupt specifier format
>>> -          depends on the interrupt controller used.
>>> -- interrupt-names: should contain the interrupt names: "vsync", "lcd_sys"
>>> -               in the same order as they were listed in the interrupts
>>> -               property.
>>> +- interrupts: should contain interrupt specifier of VSYNC and optionally
>>> +          LCD_SYSTEM. The interrupt specifier format depends on
>>> +          the interrupt controller used.
>>> +- interrupt-names: should contain the interrupt name "vsync" and optionally
>>> +               "lcd_sys" in the same order as they were listed in
>>> +               the interrupts property.
>> The driver already did not require both interrupts, right? Only one of them?
>
> Right. More precisely it did not require since beginning.

So can you move it to the cleanup patch with change below?

Best regards,
Krzysztof

Re: [PATCH 01/10] clk/samsung: exynos5433: add definitions of HDMI-PHY output clocks

[Michael Turquette]

Quoting Sylwester Nawrocki (2015-10-20 04:17:35)
> On 20/10/15 12:34, Michael Turquette wrote:
> >> diff --git a/include/dt-bindings/clock/exynos5433.h b/include/dt-bindings/clock/exynos5433.h
> >> > index 5bd80d5..4f0d566 100644
> >> > --- a/include/dt-bindings/clock/exynos5433.h
> >> > +++ b/include/dt-bindings/clock/exynos5433.h
> >> > @@ -765,7 +765,10 @@
> >> >  #define CLK_SCLK_RGB_VCLK                              109
> >> >  #define CLK_SCLK_RGB_TV_VCLK                           110
> >> >  
> >> > -#define DISP_NR_CLK                                    111
> >> > +#define CLK_PHYCLK_HDMIPHY_PIXEL_CLKO_PHY              111
> >> > +#define CLK_PHYCLK_HDMIPHY_TMDS_CLKO_PHY               112
> >> > +
> >> > +#define DISP_NR_CLK                                    113
> >
> > Why break compatibility with older DTBs?
> 
> I used to be resistant to changing those _NR_CLK defines
> in the past but then realized they are not part of the DT ABI.
> These defines are used only in drivers and affect only size
> of the provider's allocated clock array. The confusion may be
> caused by the fact that the whole header is shared by the kernel
> source and dts.
> 
> $ git grep -l _NR_CLK arch/arm/boot/dts drivers/clk/samsung/
> drivers/clk/samsung/clk-exynos-clkout.c
> drivers/clk/samsung/clk-exynos3250.c
> drivers/clk/samsung/clk-exynos4.c
> drivers/clk/samsung/clk-exynos4415.c
> drivers/clk/samsung/clk-exynos5250.c
> drivers/clk/samsung/clk-exynos5260.c
> drivers/clk/samsung/clk-exynos5410.c
> drivers/clk/samsung/clk-exynos5420.c
> drivers/clk/samsung/clk-exynos5433.c
> drivers/clk/samsung/clk-exynos5440.c
> drivers/clk/samsung/clk-exynos7.c
> 
> There is no *_NR_CLK in any dts file.
> New kernel will work will older DTB, the driver will just
> register more clocks, which will not be dereferenced anywhere
> in older dtb.

I skimmed through the code too fast. You're right, its a sentinel/max
value thing which is not part of the ABI.

Pardon the noise. Nothing to see here, move along...

Regards,
Mike

> 
> -- 
> Regards,
> Sylwester
> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 01/10] clk/samsung: exynos5433: add definitions of HDMI-PHY output clocks

[Michael Turquette]

Quoting Sylwester Nawrocki (2015-10-20 04:17:35)
> On 20/10/15 12:34, Michael Turquette wrote:
> >> diff --git a/include/dt-bindings/clock/exynos5433.h b/include/dt-bindi=
ngs/clock/exynos5433.h
> >> > index 5bd80d5..4f0d566 100644
> >> > --- a/include/dt-bindings/clock/exynos5433.h
> >> > +++ b/include/dt-bindings/clock/exynos5433.h
> >> > @@ -765,7 +765,10 @@
> >> >  #define CLK_SCLK_RGB_VCLK                              109
> >> >  #define CLK_SCLK_RGB_TV_VCLK                           110
> >> >  =

> >> > -#define DISP_NR_CLK                                    111
> >> > +#define CLK_PHYCLK_HDMIPHY_PIXEL_CLKO_PHY              111
> >> > +#define CLK_PHYCLK_HDMIPHY_TMDS_CLKO_PHY               112
> >> > +
> >> > +#define DISP_NR_CLK                                    113
> >
> > Why break compatibility with older DTBs?
> =

> I used to be resistant to changing those _NR_CLK defines
> in the past but then realized they are not part of the DT ABI.
> These defines are used only in drivers and affect only size
> of the provider's allocated clock array. The confusion may be
> caused by the fact that the whole header is shared by the kernel
> source and dts.
> =

> $ git grep -l _NR_CLK arch/arm/boot/dts drivers/clk/samsung/
> drivers/clk/samsung/clk-exynos-clkout.c
> drivers/clk/samsung/clk-exynos3250.c
> drivers/clk/samsung/clk-exynos4.c
> drivers/clk/samsung/clk-exynos4415.c
> drivers/clk/samsung/clk-exynos5250.c
> drivers/clk/samsung/clk-exynos5260.c
> drivers/clk/samsung/clk-exynos5410.c
> drivers/clk/samsung/clk-exynos5420.c
> drivers/clk/samsung/clk-exynos5433.c
> drivers/clk/samsung/clk-exynos5440.c
> drivers/clk/samsung/clk-exynos7.c
> =

> There is no *_NR_CLK in any dts file.
> New kernel will work will older DTB, the driver will just
> register more clocks, which will not be dereferenced anywhere
> in older dtb.

I skimmed through the code too fast. You're right, its a sentinel/max
value thing which is not part of the ABI.

Pardon the noise. Nothing to see here, move along...

Regards,
Mike

> =

> -- =

> Regards,
> Sylwester
> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/10] drm/exynos/decon5433: add support for DECON-TV

[Inki Dae]

Hi Andrzej,


2015년 10월 20일 18:22에 Andrzej Hajda 이(가) 쓴 글:
> DECON-TV IP is responsible for generating video stream which is transferred
> to HDMI IP. It is almost fully compatible with DECON IP.
>
> The patch is based on initial work of Hyungwon Hwang.
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>   drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 154 ++++++++++++++++----------
>   include/video/exynos5433_decon.h              |  29 +++++
>   2 files changed, 122 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> index 3c9aa4e..fbe1b31 100644
> --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> @@ -13,6 +13,7 @@
>   #include <linux/platform_device.h>
>   #include <linux/clk.h>
>   #include <linux/component.h>
> +#include <linux/of_device.h>
>   #include <linux/of_gpio.h>
>   #include <linux/pm_runtime.h>
>
> @@ -37,6 +38,12 @@ static const char * const decon_clks_name[] = {
>   	"sclk_decon_eclk",
>   };
>
> +enum decon_iftype {
> +	IFTYPE_RGB,
> +	IFTYPE_I80,
> +	IFTYPE_HDMI
> +};
> +
>   enum decon_flag_bits {
>   	BIT_CLKS_ENABLED,
>   	BIT_IRQS_ENABLED,
> @@ -53,7 +60,8 @@ struct decon_context {
>   	struct clk			*clks[ARRAY_SIZE(decon_clks_name)];
>   	int				pipe;
>   	unsigned long			flags;
> -	bool				i80_if;
> +	enum decon_iftype		out_type;
> +	int				first_win;
>   };
>
>   static const uint32_t decon_formats[] = {
> @@ -80,7 +88,7 @@ static int decon_enable_vblank(struct exynos_drm_crtc *crtc)
>
>   	if (test_and_set_bit(BIT_IRQS_ENABLED, &ctx->flags)) {
>   		val = VIDINTCON0_INTEN;
> -		if (ctx->i80_if)
> +		if (ctx->out_type == IFTYPE_I80)
>   			val |= VIDINTCON0_FRAMEDONE;
>   		else
>   			val |= VIDINTCON0_INTFRMEN;
> @@ -104,8 +112,11 @@ static void decon_disable_vblank(struct exynos_drm_crtc *crtc)
>
>   static void decon_setup_trigger(struct decon_context *ctx)
>   {
> -	u32 val = TRIGCON_TRIGEN_PER_F | TRIGCON_TRIGEN_F |
> -			TRIGCON_TE_AUTO_MASK | TRIGCON_SWTRIGEN;
> +	u32 val = (ctx->out_type != IFTYPE_HDMI)
> +		? TRIGCON_TRIGEN_PER_F | TRIGCON_TRIGEN_F |
> +		  TRIGCON_TE_AUTO_MASK | TRIGCON_SWTRIGEN
> +		: TRIGCON_TRIGEN_PER_F | TRIGCON_TRIGEN_F |
> +		  TRIGCON_HWTRIGMASK_I80_RGB | TRIGCON_HWTRIGEN_I80_RGB;
>   	writel(val, ctx->addr + DECON_TRIGCON);
>   }
>
> @@ -118,13 +129,22 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
>   	if (test_bit(BIT_SUSPENDED, &ctx->flags))
>   		return;
>
> +	if (ctx->out_type == IFTYPE_HDMI) {
> +		m->crtc_hsync_start = m->crtc_hdisplay + 10;
> +		m->crtc_hsync_end = m->crtc_htotal - 92;
> +		m->crtc_vsync_start = m->crtc_vdisplay + 1;
> +		m->crtc_vsync_end = m->crtc_vsync_start + 1;
> +	}
> +
> +	decon_set_bits(ctx, DECON_VIDCON0, VIDCON0_ENVID, 0);
> +
>   	/* enable clock gate */
>   	val = CMU_CLKGAGE_MODE_SFR_F | CMU_CLKGAGE_MODE_MEM_F;
>   	writel(val, ctx->addr + DECON_CMU);
>
>   	/* lcd on and use command if */
>   	val = VIDOUT_LCD_ON;
> -	if (ctx->i80_if)
> +	if (ctx->out_type == IFTYPE_I80)
>   		val |= VIDOUT_COMMAND_IF;
>   	else
>   		val |= VIDOUT_RGB_IF;
> @@ -134,7 +154,7 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
>   		VIDTCON2_HOZVAL(m->hdisplay - 1);
>   	writel(val, ctx->addr + DECON_VIDTCON2);
>
> -	if (!ctx->i80_if) {
> +	if (ctx->out_type != IFTYPE_I80) {
>   		val = VIDTCON00_VBPD_F(
>   				m->crtc_vtotal - m->crtc_vsync_end - 1) |
>   			VIDTCON00_VFPD_F(
> @@ -159,15 +179,9 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
>   	decon_setup_trigger(ctx);
>
>   	/* enable output and display signal */
> -	val = VIDCON0_ENVID | VIDCON0_ENVID_F;
> -	writel(val, ctx->addr + DECON_VIDCON0);
> +	decon_set_bits(ctx, DECON_VIDCON0, VIDCON0_ENVID | VIDCON0_ENVID_F, ~0);
>   }
>
> -#define COORDINATE_X(x)		(((x) & 0xfff) << 12)
> -#define COORDINATE_Y(x)		((x) & 0xfff)
> -#define OFFSIZE(x)		(((x) & 0x3fff) << 14)
> -#define PAGEWIDTH(x)		((x) & 0x3fff)
> -
>   static void decon_win_set_pixfmt(struct decon_context *ctx, unsigned int win,
>   				 struct drm_framebuffer *fb)
>   {
> @@ -238,6 +252,10 @@ static void decon_atomic_begin(struct exynos_drm_crtc *crtc,
>   	decon_shadow_protect_win(ctx, plane->zpos, true);
>   }
>
> +#define BIT_VAL(x, e, s) (((x) & ((1 << ((e) - (s) + 1)) - 1)) << (s))
> +#define COORDINATE_X(x) BIT_VAL((x), 23, 12)
> +#define COORDINATE_Y(x) BIT_VAL((x), 11, 0)
> +
>   static void decon_update_plane(struct exynos_drm_crtc *crtc,
>   			       struct exynos_drm_plane *plane)
>   {
> @@ -271,8 +289,12 @@ static void decon_update_plane(struct exynos_drm_crtc *crtc,
>   	val = plane->dma_addr[0] + pitch * plane->crtc_h;
>   	writel(val, ctx->addr + DECON_VIDW0xADD1B0(win));
>
> -	val = OFFSIZE(pitch - plane->crtc_w * bpp)
> -		| PAGEWIDTH(plane->crtc_w * bpp);
> +	if (ctx->out_type != IFTYPE_HDMI)
> +		val = BIT_VAL(pitch - plane->crtc_w * bpp, 27, 14)
> +			| BIT_VAL(plane->crtc_w * bpp, 13, 0);
> +	else
> +		val = BIT_VAL(pitch - plane->crtc_w * bpp, 29, 15)
> +			| BIT_VAL(plane->crtc_w * bpp, 14, 0);
>   	writel(val, ctx->addr + DECON_VIDW0xADD2(win));
>
>   	decon_win_set_pixfmt(ctx, win, state->fb);
> @@ -314,7 +336,7 @@ static void decon_atomic_flush(struct exynos_drm_crtc *crtc,
>
>   	decon_shadow_protect_win(ctx, plane->zpos, false);
>
> -	if (ctx->i80_if)
> +	if (ctx->out_type == IFTYPE_I80)
>   		set_bit(BIT_WIN_UPDATED, &ctx->flags);
>   }
>
> @@ -339,6 +361,17 @@ static void decon_swreset(struct decon_context *ctx)
>   	}
>
>   	WARN(tries == 0, "failed to software reset DECON\n");
> +
> +	if (ctx->out_type != IFTYPE_HDMI)
> +		return;
> +
> +	writel(VIDCON0_CLKVALUP | VIDCON0_VLCKFREE, ctx->addr + DECON_VIDCON0);
> +	decon_set_bits(ctx, DECON_CMU,
> +		       CMU_CLKGAGE_MODE_SFR_F | CMU_CLKGAGE_MODE_MEM_F, ~0);
> +	writel(VIDCON1_VCLK_RUN_VDEN_DISABLE, ctx->addr + DECON_VIDCON1);
> +	writel(CRCCTRL_CRCEN | CRCCTRL_CRCSTART_F | CRCCTRL_CRCCLKEN,
> +	       ctx->addr + DECON_CRCCTRL);
> +	decon_setup_trigger(ctx);
>   }
>
>   static void decon_enable(struct exynos_drm_crtc *crtc)
> @@ -387,7 +420,7 @@ static void decon_disable(struct exynos_drm_crtc *crtc)
>   	 * suspend that connector. Otherwise we might try to scan from
>   	 * a destroyed buffer later.
>   	 */
> -	for (i = 0; i < WINDOWS_NR; i++)
> +	for (i = ctx->first_win; i < WINDOWS_NR; i++)
>   		decon_disable_plane(crtc, &ctx->planes[i]);
>
>   	decon_swreset(ctx);
> @@ -461,25 +494,30 @@ static int decon_bind(struct device *dev, struct device *master, void *data)
>   	struct drm_device *drm_dev = data;
>   	struct exynos_drm_private *priv = drm_dev->dev_private;
>   	struct exynos_drm_plane *exynos_plane;
> +	enum exynos_drm_output_type out_type;
>   	enum drm_plane_type type;
> -	unsigned int zpos;
> +	unsigned int win;
>   	int ret;
>
>   	ctx->drm_dev = drm_dev;
>   	ctx->pipe = priv->pipe++;
>
> -	for (zpos = 0; zpos < WINDOWS_NR; zpos++) {
> -		type = exynos_plane_get_type(zpos, CURSOR_WIN);
> -		ret = exynos_plane_init(drm_dev, &ctx->planes[zpos],
> +	for (win = ctx->first_win; win < WINDOWS_NR; win++) {
> +		int tmp = (win == ctx->first_win) ? 0 : win;

AFAIK, DECON TV for Exynos5433 SoC has four hardware overlays so I guess 
you used ctx->first_win to initialize four planes in case of IFTYPE_HDMI.

However, with IFTYPE_HDMI ctx->first_win has 1, as a result, 
ctx->planes[] will have 0, 2nd, 3rd and 4th planes and this means we 
should use 0, 2nd ~ 4th hardware overlay excepting 1st overlay. Is this 
your intention?
Shouldn't ctx->planes[] have 1st, 2nd ,3rd and 4th planes so that we can 
use 1st ~ 4th hardware overlay for DECON TV?

DECON TV for Exynos5433 SoC can use only 1st ~ 4th hardware overlays.

And other thing, it may be trivial but I think it'd be better to use 
'ovl' instead of 'tmp.

Thanks,
Inki Dae
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 10/10] drm/exynos/decon5433: add support for DECON-TV

[Inki Dae]

Hi Andrzej,


2015년 10월 20일 18:22에 Andrzej Hajda 이(가) 쓴 글:
> DECON-TV IP is responsible for generating video stream which is transferred
> to HDMI IP. It is almost fully compatible with DECON IP.
>
> The patch is based on initial work of Hyungwon Hwang.
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>   drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 154 ++++++++++++++++----------
>   include/video/exynos5433_decon.h              |  29 +++++
>   2 files changed, 122 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> index 3c9aa4e..fbe1b31 100644
> --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> @@ -13,6 +13,7 @@
>   #include <linux/platform_device.h>
>   #include <linux/clk.h>
>   #include <linux/component.h>
> +#include <linux/of_device.h>
>   #include <linux/of_gpio.h>
>   #include <linux/pm_runtime.h>
>
> @@ -37,6 +38,12 @@ static const char * const decon_clks_name[] = {
>   	"sclk_decon_eclk",
>   };
>
> +enum decon_iftype {
> +	IFTYPE_RGB,
> +	IFTYPE_I80,
> +	IFTYPE_HDMI
> +};
> +
>   enum decon_flag_bits {
>   	BIT_CLKS_ENABLED,
>   	BIT_IRQS_ENABLED,
> @@ -53,7 +60,8 @@ struct decon_context {
>   	struct clk			*clks[ARRAY_SIZE(decon_clks_name)];
>   	int				pipe;
>   	unsigned long			flags;
> -	bool				i80_if;
> +	enum decon_iftype		out_type;
> +	int				first_win;
>   };
>
>   static const uint32_t decon_formats[] = {
> @@ -80,7 +88,7 @@ static int decon_enable_vblank(struct exynos_drm_crtc *crtc)
>
>   	if (test_and_set_bit(BIT_IRQS_ENABLED, &ctx->flags)) {
>   		val = VIDINTCON0_INTEN;
> -		if (ctx->i80_if)
> +		if (ctx->out_type == IFTYPE_I80)
>   			val |= VIDINTCON0_FRAMEDONE;
>   		else
>   			val |= VIDINTCON0_INTFRMEN;
> @@ -104,8 +112,11 @@ static void decon_disable_vblank(struct exynos_drm_crtc *crtc)
>
>   static void decon_setup_trigger(struct decon_context *ctx)
>   {
> -	u32 val = TRIGCON_TRIGEN_PER_F | TRIGCON_TRIGEN_F |
> -			TRIGCON_TE_AUTO_MASK | TRIGCON_SWTRIGEN;
> +	u32 val = (ctx->out_type != IFTYPE_HDMI)
> +		? TRIGCON_TRIGEN_PER_F | TRIGCON_TRIGEN_F |
> +		  TRIGCON_TE_AUTO_MASK | TRIGCON_SWTRIGEN
> +		: TRIGCON_TRIGEN_PER_F | TRIGCON_TRIGEN_F |
> +		  TRIGCON_HWTRIGMASK_I80_RGB | TRIGCON_HWTRIGEN_I80_RGB;
>   	writel(val, ctx->addr + DECON_TRIGCON);
>   }
>
> @@ -118,13 +129,22 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
>   	if (test_bit(BIT_SUSPENDED, &ctx->flags))
>   		return;
>
> +	if (ctx->out_type == IFTYPE_HDMI) {
> +		m->crtc_hsync_start = m->crtc_hdisplay + 10;
> +		m->crtc_hsync_end = m->crtc_htotal - 92;
> +		m->crtc_vsync_start = m->crtc_vdisplay + 1;
> +		m->crtc_vsync_end = m->crtc_vsync_start + 1;
> +	}
> +
> +	decon_set_bits(ctx, DECON_VIDCON0, VIDCON0_ENVID, 0);
> +
>   	/* enable clock gate */
>   	val = CMU_CLKGAGE_MODE_SFR_F | CMU_CLKGAGE_MODE_MEM_F;
>   	writel(val, ctx->addr + DECON_CMU);
>
>   	/* lcd on and use command if */
>   	val = VIDOUT_LCD_ON;
> -	if (ctx->i80_if)
> +	if (ctx->out_type == IFTYPE_I80)
>   		val |= VIDOUT_COMMAND_IF;
>   	else
>   		val |= VIDOUT_RGB_IF;
> @@ -134,7 +154,7 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
>   		VIDTCON2_HOZVAL(m->hdisplay - 1);
>   	writel(val, ctx->addr + DECON_VIDTCON2);
>
> -	if (!ctx->i80_if) {
> +	if (ctx->out_type != IFTYPE_I80) {
>   		val = VIDTCON00_VBPD_F(
>   				m->crtc_vtotal - m->crtc_vsync_end - 1) |
>   			VIDTCON00_VFPD_F(
> @@ -159,15 +179,9 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
>   	decon_setup_trigger(ctx);
>
>   	/* enable output and display signal */
> -	val = VIDCON0_ENVID | VIDCON0_ENVID_F;
> -	writel(val, ctx->addr + DECON_VIDCON0);
> +	decon_set_bits(ctx, DECON_VIDCON0, VIDCON0_ENVID | VIDCON0_ENVID_F, ~0);
>   }
>
> -#define COORDINATE_X(x)		(((x) & 0xfff) << 12)
> -#define COORDINATE_Y(x)		((x) & 0xfff)
> -#define OFFSIZE(x)		(((x) & 0x3fff) << 14)
> -#define PAGEWIDTH(x)		((x) & 0x3fff)
> -
>   static void decon_win_set_pixfmt(struct decon_context *ctx, unsigned int win,
>   				 struct drm_framebuffer *fb)
>   {
> @@ -238,6 +252,10 @@ static void decon_atomic_begin(struct exynos_drm_crtc *crtc,
>   	decon_shadow_protect_win(ctx, plane->zpos, true);
>   }
>
> +#define BIT_VAL(x, e, s) (((x) & ((1 << ((e) - (s) + 1)) - 1)) << (s))
> +#define COORDINATE_X(x) BIT_VAL((x), 23, 12)
> +#define COORDINATE_Y(x) BIT_VAL((x), 11, 0)
> +
>   static void decon_update_plane(struct exynos_drm_crtc *crtc,
>   			       struct exynos_drm_plane *plane)
>   {
> @@ -271,8 +289,12 @@ static void decon_update_plane(struct exynos_drm_crtc *crtc,
>   	val = plane->dma_addr[0] + pitch * plane->crtc_h;
>   	writel(val, ctx->addr + DECON_VIDW0xADD1B0(win));
>
> -	val = OFFSIZE(pitch - plane->crtc_w * bpp)
> -		| PAGEWIDTH(plane->crtc_w * bpp);
> +	if (ctx->out_type != IFTYPE_HDMI)
> +		val = BIT_VAL(pitch - plane->crtc_w * bpp, 27, 14)
> +			| BIT_VAL(plane->crtc_w * bpp, 13, 0);
> +	else
> +		val = BIT_VAL(pitch - plane->crtc_w * bpp, 29, 15)
> +			| BIT_VAL(plane->crtc_w * bpp, 14, 0);
>   	writel(val, ctx->addr + DECON_VIDW0xADD2(win));
>
>   	decon_win_set_pixfmt(ctx, win, state->fb);
> @@ -314,7 +336,7 @@ static void decon_atomic_flush(struct exynos_drm_crtc *crtc,
>
>   	decon_shadow_protect_win(ctx, plane->zpos, false);
>
> -	if (ctx->i80_if)
> +	if (ctx->out_type == IFTYPE_I80)
>   		set_bit(BIT_WIN_UPDATED, &ctx->flags);
>   }
>
> @@ -339,6 +361,17 @@ static void decon_swreset(struct decon_context *ctx)
>   	}
>
>   	WARN(tries == 0, "failed to software reset DECON\n");
> +
> +	if (ctx->out_type != IFTYPE_HDMI)
> +		return;
> +
> +	writel(VIDCON0_CLKVALUP | VIDCON0_VLCKFREE, ctx->addr + DECON_VIDCON0);
> +	decon_set_bits(ctx, DECON_CMU,
> +		       CMU_CLKGAGE_MODE_SFR_F | CMU_CLKGAGE_MODE_MEM_F, ~0);
> +	writel(VIDCON1_VCLK_RUN_VDEN_DISABLE, ctx->addr + DECON_VIDCON1);
> +	writel(CRCCTRL_CRCEN | CRCCTRL_CRCSTART_F | CRCCTRL_CRCCLKEN,
> +	       ctx->addr + DECON_CRCCTRL);
> +	decon_setup_trigger(ctx);
>   }
>
>   static void decon_enable(struct exynos_drm_crtc *crtc)
> @@ -387,7 +420,7 @@ static void decon_disable(struct exynos_drm_crtc *crtc)
>   	 * suspend that connector. Otherwise we might try to scan from
>   	 * a destroyed buffer later.
>   	 */
> -	for (i = 0; i < WINDOWS_NR; i++)
> +	for (i = ctx->first_win; i < WINDOWS_NR; i++)
>   		decon_disable_plane(crtc, &ctx->planes[i]);
>
>   	decon_swreset(ctx);
> @@ -461,25 +494,30 @@ static int decon_bind(struct device *dev, struct device *master, void *data)
>   	struct drm_device *drm_dev = data;
>   	struct exynos_drm_private *priv = drm_dev->dev_private;
>   	struct exynos_drm_plane *exynos_plane;
> +	enum exynos_drm_output_type out_type;
>   	enum drm_plane_type type;
> -	unsigned int zpos;
> +	unsigned int win;
>   	int ret;
>
>   	ctx->drm_dev = drm_dev;
>   	ctx->pipe = priv->pipe++;
>
> -	for (zpos = 0; zpos < WINDOWS_NR; zpos++) {
> -		type = exynos_plane_get_type(zpos, CURSOR_WIN);
> -		ret = exynos_plane_init(drm_dev, &ctx->planes[zpos],
> +	for (win = ctx->first_win; win < WINDOWS_NR; win++) {
> +		int tmp = (win == ctx->first_win) ? 0 : win;

AFAIK, DECON TV for Exynos5433 SoC has four hardware overlays so I guess 
you used ctx->first_win to initialize four planes in case of IFTYPE_HDMI.

However, with IFTYPE_HDMI ctx->first_win has 1, as a result, 
ctx->planes[] will have 0, 2nd, 3rd and 4th planes and this means we 
should use 0, 2nd ~ 4th hardware overlay excepting 1st overlay. Is this 
your intention?
Shouldn't ctx->planes[] have 1st, 2nd ,3rd and 4th planes so that we can 
use 1st ~ 4th hardware overlay for DECON TV?

DECON TV for Exynos5433 SoC can use only 1st ~ 4th hardware overlays.

And other thing, it may be trivial but I think it'd be better to use 
'ovl' instead of 'tmp.

Thanks,
Inki Dae

Re: [PATCH 10/10] drm/exynos/decon5433: add support for DECON-TV

[Andrzej Hajda]

On 10/23/2015 01:55 PM, Inki Dae wrote:
> Hi Andrzej,
>
>
> 2015년 10월 20일 18:22에 Andrzej Hajda 이(가) 쓴 글:
>> DECON-TV IP is responsible for generating video stream which is transferred
>> to HDMI IP. It is almost fully compatible with DECON IP.
>>
>> The patch is based on initial work of Hyungwon Hwang.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>>   drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 154 ++++++++++++++++----------
>>   include/video/exynos5433_decon.h              |  29 +++++
>>   2 files changed, 122 insertions(+), 61 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
>> index 3c9aa4e..fbe1b31 100644
>> --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
>> +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
>> @@ -13,6 +13,7 @@
>>   #include <linux/platform_device.h>
>>   #include <linux/clk.h>
>>   #include <linux/component.h>
>> +#include <linux/of_device.h>
>>   #include <linux/of_gpio.h>
>>   #include <linux/pm_runtime.h>
>>
>> @@ -37,6 +38,12 @@ static const char * const decon_clks_name[] = {
>>   	"sclk_decon_eclk",
>>   };
>>
>> +enum decon_iftype {
>> +	IFTYPE_RGB,
>> +	IFTYPE_I80,
>> +	IFTYPE_HDMI
>> +};
>> +
>>   enum decon_flag_bits {
>>   	BIT_CLKS_ENABLED,
>>   	BIT_IRQS_ENABLED,
>> @@ -53,7 +60,8 @@ struct decon_context {
>>   	struct clk			*clks[ARRAY_SIZE(decon_clks_name)];
>>   	int				pipe;
>>   	unsigned long			flags;
>> -	bool				i80_if;
>> +	enum decon_iftype		out_type;
>> +	int				first_win;
>>   };
>>
>>   static const uint32_t decon_formats[] = {
>> @@ -80,7 +88,7 @@ static int decon_enable_vblank(struct exynos_drm_crtc *crtc)
>>
>>   	if (test_and_set_bit(BIT_IRQS_ENABLED, &ctx->flags)) {
>>   		val = VIDINTCON0_INTEN;
>> -		if (ctx->i80_if)
>> +		if (ctx->out_type == IFTYPE_I80)
>>   			val |= VIDINTCON0_FRAMEDONE;
>>   		else
>>   			val |= VIDINTCON0_INTFRMEN;
>> @@ -104,8 +112,11 @@ static void decon_disable_vblank(struct exynos_drm_crtc *crtc)
>>
>>   static void decon_setup_trigger(struct decon_context *ctx)
>>   {
>> -	u32 val = TRIGCON_TRIGEN_PER_F | TRIGCON_TRIGEN_F |
>> -			TRIGCON_TE_AUTO_MASK | TRIGCON_SWTRIGEN;
>> +	u32 val = (ctx->out_type != IFTYPE_HDMI)
>> +		? TRIGCON_TRIGEN_PER_F | TRIGCON_TRIGEN_F |
>> +		  TRIGCON_TE_AUTO_MASK | TRIGCON_SWTRIGEN
>> +		: TRIGCON_TRIGEN_PER_F | TRIGCON_TRIGEN_F |
>> +		  TRIGCON_HWTRIGMASK_I80_RGB | TRIGCON_HWTRIGEN_I80_RGB;
>>   	writel(val, ctx->addr + DECON_TRIGCON);
>>   }
>>
>> @@ -118,13 +129,22 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
>>   	if (test_bit(BIT_SUSPENDED, &ctx->flags))
>>   		return;
>>
>> +	if (ctx->out_type == IFTYPE_HDMI) {
>> +		m->crtc_hsync_start = m->crtc_hdisplay + 10;
>> +		m->crtc_hsync_end = m->crtc_htotal - 92;
>> +		m->crtc_vsync_start = m->crtc_vdisplay + 1;
>> +		m->crtc_vsync_end = m->crtc_vsync_start + 1;
>> +	}
>> +
>> +	decon_set_bits(ctx, DECON_VIDCON0, VIDCON0_ENVID, 0);
>> +
>>   	/* enable clock gate */
>>   	val = CMU_CLKGAGE_MODE_SFR_F | CMU_CLKGAGE_MODE_MEM_F;
>>   	writel(val, ctx->addr + DECON_CMU);
>>
>>   	/* lcd on and use command if */
>>   	val = VIDOUT_LCD_ON;
>> -	if (ctx->i80_if)
>> +	if (ctx->out_type == IFTYPE_I80)
>>   		val |= VIDOUT_COMMAND_IF;
>>   	else
>>   		val |= VIDOUT_RGB_IF;
>> @@ -134,7 +154,7 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
>>   		VIDTCON2_HOZVAL(m->hdisplay - 1);
>>   	writel(val, ctx->addr + DECON_VIDTCON2);
>>
>> -	if (!ctx->i80_if) {
>> +	if (ctx->out_type != IFTYPE_I80) {
>>   		val = VIDTCON00_VBPD_F(
>>   				m->crtc_vtotal - m->crtc_vsync_end - 1) |
>>   			VIDTCON00_VFPD_F(
>> @@ -159,15 +179,9 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
>>   	decon_setup_trigger(ctx);
>>
>>   	/* enable output and display signal */
>> -	val = VIDCON0_ENVID | VIDCON0_ENVID_F;
>> -	writel(val, ctx->addr + DECON_VIDCON0);
>> +	decon_set_bits(ctx, DECON_VIDCON0, VIDCON0_ENVID | VIDCON0_ENVID_F, ~0);
>>   }
>>
>> -#define COORDINATE_X(x)		(((x) & 0xfff) << 12)
>> -#define COORDINATE_Y(x)		((x) & 0xfff)
>> -#define OFFSIZE(x)		(((x) & 0x3fff) << 14)
>> -#define PAGEWIDTH(x)		((x) & 0x3fff)
>> -
>>   static void decon_win_set_pixfmt(struct decon_context *ctx, unsigned int win,
>>   				 struct drm_framebuffer *fb)
>>   {
>> @@ -238,6 +252,10 @@ static void decon_atomic_begin(struct exynos_drm_crtc *crtc,
>>   	decon_shadow_protect_win(ctx, plane->zpos, true);
>>   }
>>
>> +#define BIT_VAL(x, e, s) (((x) & ((1 << ((e) - (s) + 1)) - 1)) << (s))
>> +#define COORDINATE_X(x) BIT_VAL((x), 23, 12)
>> +#define COORDINATE_Y(x) BIT_VAL((x), 11, 0)
>> +
>>   static void decon_update_plane(struct exynos_drm_crtc *crtc,
>>   			       struct exynos_drm_plane *plane)
>>   {
>> @@ -271,8 +289,12 @@ static void decon_update_plane(struct exynos_drm_crtc *crtc,
>>   	val = plane->dma_addr[0] + pitch * plane->crtc_h;
>>   	writel(val, ctx->addr + DECON_VIDW0xADD1B0(win));
>>
>> -	val = OFFSIZE(pitch - plane->crtc_w * bpp)
>> -		| PAGEWIDTH(plane->crtc_w * bpp);
>> +	if (ctx->out_type != IFTYPE_HDMI)
>> +		val = BIT_VAL(pitch - plane->crtc_w * bpp, 27, 14)
>> +			| BIT_VAL(plane->crtc_w * bpp, 13, 0);
>> +	else
>> +		val = BIT_VAL(pitch - plane->crtc_w * bpp, 29, 15)
>> +			| BIT_VAL(plane->crtc_w * bpp, 14, 0);
>>   	writel(val, ctx->addr + DECON_VIDW0xADD2(win));
>>
>>   	decon_win_set_pixfmt(ctx, win, state->fb);
>> @@ -314,7 +336,7 @@ static void decon_atomic_flush(struct exynos_drm_crtc *crtc,
>>
>>   	decon_shadow_protect_win(ctx, plane->zpos, false);
>>
>> -	if (ctx->i80_if)
>> +	if (ctx->out_type == IFTYPE_I80)
>>   		set_bit(BIT_WIN_UPDATED, &ctx->flags);
>>   }
>>
>> @@ -339,6 +361,17 @@ static void decon_swreset(struct decon_context *ctx)
>>   	}
>>
>>   	WARN(tries == 0, "failed to software reset DECON\n");
>> +
>> +	if (ctx->out_type != IFTYPE_HDMI)
>> +		return;
>> +
>> +	writel(VIDCON0_CLKVALUP | VIDCON0_VLCKFREE, ctx->addr + DECON_VIDCON0);
>> +	decon_set_bits(ctx, DECON_CMU,
>> +		       CMU_CLKGAGE_MODE_SFR_F | CMU_CLKGAGE_MODE_MEM_F, ~0);
>> +	writel(VIDCON1_VCLK_RUN_VDEN_DISABLE, ctx->addr + DECON_VIDCON1);
>> +	writel(CRCCTRL_CRCEN | CRCCTRL_CRCSTART_F | CRCCTRL_CRCCLKEN,
>> +	       ctx->addr + DECON_CRCCTRL);
>> +	decon_setup_trigger(ctx);
>>   }
>>
>>   static void decon_enable(struct exynos_drm_crtc *crtc)
>> @@ -387,7 +420,7 @@ static void decon_disable(struct exynos_drm_crtc *crtc)
>>   	 * suspend that connector. Otherwise we might try to scan from
>>   	 * a destroyed buffer later.
>>   	 */
>> -	for (i = 0; i < WINDOWS_NR; i++)
>> +	for (i = ctx->first_win; i < WINDOWS_NR; i++)
>>   		decon_disable_plane(crtc, &ctx->planes[i]);
>>
>>   	decon_swreset(ctx);
>> @@ -461,25 +494,30 @@ static int decon_bind(struct device *dev, struct device *master, void *data)
>>   	struct drm_device *drm_dev = data;
>>   	struct exynos_drm_private *priv = drm_dev->dev_private;
>>   	struct exynos_drm_plane *exynos_plane;
>> +	enum exynos_drm_output_type out_type;
>>   	enum drm_plane_type type;
>> -	unsigned int zpos;
>> +	unsigned int win;
>>   	int ret;
>>
>>   	ctx->drm_dev = drm_dev;
>>   	ctx->pipe = priv->pipe++;
>>
>> -	for (zpos = 0; zpos < WINDOWS_NR; zpos++) {
>> -		type = exynos_plane_get_type(zpos, CURSOR_WIN);
>> -		ret = exynos_plane_init(drm_dev, &ctx->planes[zpos],
>> +	for (win = ctx->first_win; win < WINDOWS_NR; win++) {
>> +		int tmp = (win == ctx->first_win) ? 0 : win;
> AFAIK, DECON TV for Exynos5433 SoC has four hardware overlays so I guess 
> you used ctx->first_win to initialize four planes in case of IFTYPE_HDMI.
>
> However, with IFTYPE_HDMI ctx->first_win has 1, as a result, 
> ctx->planes[] will have 0, 2nd, 3rd and 4th planes and this means we 
> should use 0, 2nd ~ 4th hardware overlay excepting 1st overlay. Is this 
> your intention?
> Shouldn't ctx->planes[] have 1st, 2nd ,3rd and 4th planes so that we can 
> use 1st ~ 4th hardware overlay for DECON TV?
>
> DECON TV for Exynos5433 SoC can use only 1st ~ 4th hardware overlays.
>
> And other thing, it may be trivial but I think it'd be better to use 
> 'ovl' instead of 'tmp.

DECON-TV has hardware window 0 with very limited functionality,
it is just background color, no scanout buffers. So it cannot be used
as drm plane. ctx->first_win contains value of the 1st valid window.
There is a question how to map windows to planes. I have chosen
the simplest solution - n'th hw window is mapped to n'th plane and have
zpos==n. The only problem is with function exynos_plane_get_type,
which returns DRM_PLANE_TYPE_PRIMARY only if their 1st argument is equal 0,
ie it assumes default window is always 0.
As an simple workaround I used 'tmp' variable which maps 1st window to 0.
This way exynos_plane_get_type returns expected value for all windows.
I plan replace this workaround with something nicer, but as it would involve
changes in other drivers I left this as TODO for later.
Regarding ctx->planes[], it is static array so it will have always one entry
invalid in case of TV, with this approach it will be th 0th entry, but with a gain
of consistency in windows/zpos/planes numbering.
If you prefer different approach please let me know.

Regards
Andrzej

>
> Thanks,
> Inki Dae
>

Re: [PATCH 10/10] drm/exynos/decon5433: add support for DECON-TV

[Inki Dae]

2015-10-23 22:03 GMT+09:00 Andrzej Hajda <a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>:
>
> On 10/23/2015 01:55 PM, Inki Dae wrote:
> > Hi Andrzej,
> >
> >
> > 2015년 10월 20일 18:22에 Andrzej Hajda 이(가) 쓴 글:
> >> DECON-TV IP is responsible for generating video stream which is transferred
> >> to HDMI IP. It is almost fully compatible with DECON IP.
> >>
> >> The patch is based on initial work of Hyungwon Hwang.
> >>
> >> Signed-off-by: Andrzej Hajda <a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> >> ---
> >>   drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 154 ++++++++++++++++----------
> >>   include/video/exynos5433_decon.h              |  29 +++++
> >>   2 files changed, 122 insertions(+), 61 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> >> index 3c9aa4e..fbe1b31 100644
> >> --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> >> +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> >> @@ -13,6 +13,7 @@
> >>   #include <linux/platform_device.h>
> >>   #include <linux/clk.h>
> >>   #include <linux/component.h>
> >> +#include <linux/of_device.h>
> >>   #include <linux/of_gpio.h>
> >>   #include <linux/pm_runtime.h>
> >>
> >> @@ -37,6 +38,12 @@ static const char * const decon_clks_name[] = {
> >>      "sclk_decon_eclk",
> >>   };
> >>
> >> +enum decon_iftype {
> >> +    IFTYPE_RGB,
> >> +    IFTYPE_I80,
> >> +    IFTYPE_HDMI
> >> +};
> >> +
> >>   enum decon_flag_bits {
> >>      BIT_CLKS_ENABLED,
> >>      BIT_IRQS_ENABLED,
> >> @@ -53,7 +60,8 @@ struct decon_context {
> >>      struct clk                      *clks[ARRAY_SIZE(decon_clks_name)];
> >>      int                             pipe;
> >>      unsigned long                   flags;
> >> -    bool                            i80_if;
> >> +    enum decon_iftype               out_type;
> >> +    int                             first_win;
> >>   };
> >>
> >>   static const uint32_t decon_formats[] = {
> >> @@ -80,7 +88,7 @@ static int decon_enable_vblank(struct exynos_drm_crtc *crtc)
> >>
> >>      if (test_and_set_bit(BIT_IRQS_ENABLED, &ctx->flags)) {
> >>              val = VIDINTCON0_INTEN;
> >> -            if (ctx->i80_if)
> >> +            if (ctx->out_type == IFTYPE_I80)
> >>                      val |= VIDINTCON0_FRAMEDONE;
> >>              else
> >>                      val |= VIDINTCON0_INTFRMEN;
> >> @@ -104,8 +112,11 @@ static void decon_disable_vblank(struct exynos_drm_crtc *crtc)
> >>
> >>   static void decon_setup_trigger(struct decon_context *ctx)
> >>   {
> >> -    u32 val = TRIGCON_TRIGEN_PER_F | TRIGCON_TRIGEN_F |
> >> -                    TRIGCON_TE_AUTO_MASK | TRIGCON_SWTRIGEN;
> >> +    u32 val = (ctx->out_type != IFTYPE_HDMI)
> >> +            ? TRIGCON_TRIGEN_PER_F | TRIGCON_TRIGEN_F |
> >> +              TRIGCON_TE_AUTO_MASK | TRIGCON_SWTRIGEN
> >> +            : TRIGCON_TRIGEN_PER_F | TRIGCON_TRIGEN_F |
> >> +              TRIGCON_HWTRIGMASK_I80_RGB | TRIGCON_HWTRIGEN_I80_RGB;
> >>      writel(val, ctx->addr + DECON_TRIGCON);
> >>   }
> >>
> >> @@ -118,13 +129,22 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
> >>      if (test_bit(BIT_SUSPENDED, &ctx->flags))
> >>              return;
> >>
> >> +    if (ctx->out_type == IFTYPE_HDMI) {
> >> +            m->crtc_hsync_start = m->crtc_hdisplay + 10;
> >> +            m->crtc_hsync_end = m->crtc_htotal - 92;
> >> +            m->crtc_vsync_start = m->crtc_vdisplay + 1;
> >> +            m->crtc_vsync_end = m->crtc_vsync_start + 1;
> >> +    }
> >> +
> >> +    decon_set_bits(ctx, DECON_VIDCON0, VIDCON0_ENVID, 0);
> >> +
> >>      /* enable clock gate */
> >>      val = CMU_CLKGAGE_MODE_SFR_F | CMU_CLKGAGE_MODE_MEM_F;
> >>      writel(val, ctx->addr + DECON_CMU);
> >>
> >>      /* lcd on and use command if */
> >>      val = VIDOUT_LCD_ON;
> >> -    if (ctx->i80_if)
> >> +    if (ctx->out_type == IFTYPE_I80)
> >>              val |= VIDOUT_COMMAND_IF;
> >>      else
> >>              val |= VIDOUT_RGB_IF;
> >> @@ -134,7 +154,7 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
> >>              VIDTCON2_HOZVAL(m->hdisplay - 1);
> >>      writel(val, ctx->addr + DECON_VIDTCON2);
> >>
> >> -    if (!ctx->i80_if) {
> >> +    if (ctx->out_type != IFTYPE_I80) {
> >>              val = VIDTCON00_VBPD_F(
> >>                              m->crtc_vtotal - m->crtc_vsync_end - 1) |
> >>                      VIDTCON00_VFPD_F(
> >> @@ -159,15 +179,9 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
> >>      decon_setup_trigger(ctx);
> >>
> >>      /* enable output and display signal */
> >> -    val = VIDCON0_ENVID | VIDCON0_ENVID_F;
> >> -    writel(val, ctx->addr + DECON_VIDCON0);
> >> +    decon_set_bits(ctx, DECON_VIDCON0, VIDCON0_ENVID | VIDCON0_ENVID_F, ~0);
> >>   }
> >>
> >> -#define COORDINATE_X(x)             (((x) & 0xfff) << 12)
> >> -#define COORDINATE_Y(x)             ((x) & 0xfff)
> >> -#define OFFSIZE(x)          (((x) & 0x3fff) << 14)
> >> -#define PAGEWIDTH(x)                ((x) & 0x3fff)
> >> -
> >>   static void decon_win_set_pixfmt(struct decon_context *ctx, unsigned int win,
> >>                               struct drm_framebuffer *fb)
> >>   {
> >> @@ -238,6 +252,10 @@ static void decon_atomic_begin(struct exynos_drm_crtc *crtc,
> >>      decon_shadow_protect_win(ctx, plane->zpos, true);
> >>   }
> >>
> >> +#define BIT_VAL(x, e, s) (((x) & ((1 << ((e) - (s) + 1)) - 1)) << (s))
> >> +#define COORDINATE_X(x) BIT_VAL((x), 23, 12)
> >> +#define COORDINATE_Y(x) BIT_VAL((x), 11, 0)
> >> +
> >>   static void decon_update_plane(struct exynos_drm_crtc *crtc,
> >>                             struct exynos_drm_plane *plane)
> >>   {
> >> @@ -271,8 +289,12 @@ static void decon_update_plane(struct exynos_drm_crtc *crtc,
> >>      val = plane->dma_addr[0] + pitch * plane->crtc_h;
> >>      writel(val, ctx->addr + DECON_VIDW0xADD1B0(win));
> >>
> >> -    val = OFFSIZE(pitch - plane->crtc_w * bpp)
> >> -            | PAGEWIDTH(plane->crtc_w * bpp);
> >> +    if (ctx->out_type != IFTYPE_HDMI)
> >> +            val = BIT_VAL(pitch - plane->crtc_w * bpp, 27, 14)
> >> +                    | BIT_VAL(plane->crtc_w * bpp, 13, 0);
> >> +    else
> >> +            val = BIT_VAL(pitch - plane->crtc_w * bpp, 29, 15)
> >> +                    | BIT_VAL(plane->crtc_w * bpp, 14, 0);
> >>      writel(val, ctx->addr + DECON_VIDW0xADD2(win));
> >>
> >>      decon_win_set_pixfmt(ctx, win, state->fb);
> >> @@ -314,7 +336,7 @@ static void decon_atomic_flush(struct exynos_drm_crtc *crtc,
> >>
> >>      decon_shadow_protect_win(ctx, plane->zpos, false);
> >>
> >> -    if (ctx->i80_if)
> >> +    if (ctx->out_type == IFTYPE_I80)
> >>              set_bit(BIT_WIN_UPDATED, &ctx->flags);
> >>   }
> >>
> >> @@ -339,6 +361,17 @@ static void decon_swreset(struct decon_context *ctx)
> >>      }
> >>
> >>      WARN(tries == 0, "failed to software reset DECON\n");
> >> +
> >> +    if (ctx->out_type != IFTYPE_HDMI)
> >> +            return;
> >> +
> >> +    writel(VIDCON0_CLKVALUP | VIDCON0_VLCKFREE, ctx->addr + DECON_VIDCON0);
> >> +    decon_set_bits(ctx, DECON_CMU,
> >> +                   CMU_CLKGAGE_MODE_SFR_F | CMU_CLKGAGE_MODE_MEM_F, ~0);
> >> +    writel(VIDCON1_VCLK_RUN_VDEN_DISABLE, ctx->addr + DECON_VIDCON1);
> >> +    writel(CRCCTRL_CRCEN | CRCCTRL_CRCSTART_F | CRCCTRL_CRCCLKEN,
> >> +           ctx->addr + DECON_CRCCTRL);
> >> +    decon_setup_trigger(ctx);
> >>   }
> >>
> >>   static void decon_enable(struct exynos_drm_crtc *crtc)
> >> @@ -387,7 +420,7 @@ static void decon_disable(struct exynos_drm_crtc *crtc)
> >>       * suspend that connector. Otherwise we might try to scan from
> >>       * a destroyed buffer later.
> >>       */
> >> -    for (i = 0; i < WINDOWS_NR; i++)
> >> +    for (i = ctx->first_win; i < WINDOWS_NR; i++)
> >>              decon_disable_plane(crtc, &ctx->planes[i]);
> >>
> >>      decon_swreset(ctx);
> >> @@ -461,25 +494,30 @@ static int decon_bind(struct device *dev, struct device *master, void *data)
> >>      struct drm_device *drm_dev = data;
> >>      struct exynos_drm_private *priv = drm_dev->dev_private;
> >>      struct exynos_drm_plane *exynos_plane;
> >> +    enum exynos_drm_output_type out_type;
> >>      enum drm_plane_type type;
> >> -    unsigned int zpos;
> >> +    unsigned int win;
> >>      int ret;
> >>
> >>      ctx->drm_dev = drm_dev;
> >>      ctx->pipe = priv->pipe++;
> >>
> >> -    for (zpos = 0; zpos < WINDOWS_NR; zpos++) {
> >> -            type = exynos_plane_get_type(zpos, CURSOR_WIN);
> >> -            ret = exynos_plane_init(drm_dev, &ctx->planes[zpos],
> >> +    for (win = ctx->first_win; win < WINDOWS_NR; win++) {
> >> +            int tmp = (win == ctx->first_win) ? 0 : win;
> > AFAIK, DECON TV for Exynos5433 SoC has four hardware overlays so I guess
> > you used ctx->first_win to initialize four planes in case of IFTYPE_HDMI.
> >
> > However, with IFTYPE_HDMI ctx->first_win has 1, as a result,
> > ctx->planes[] will have 0, 2nd, 3rd and 4th planes and this means we
> > should use 0, 2nd ~ 4th hardware overlay excepting 1st overlay. Is this
> > your intention?
> > Shouldn't ctx->planes[] have 1st, 2nd ,3rd and 4th planes so that we can
> > use 1st ~ 4th hardware overlay for DECON TV?
> >
> > DECON TV for Exynos5433 SoC can use only 1st ~ 4th hardware overlays.
> >
> > And other thing, it may be trivial but I think it'd be better to use
> > 'ovl' instead of 'tmp.
>
> DECON-TV has hardware window 0 with very limited functionality,
> it is just background color, no scanout buffers. So it cannot be used
> as drm plane. ctx->first_win contains value of the 1st valid window.
> There is a question how to map windows to planes. I have chosen
> the simplest solution - n'th hw window is mapped to n'th plane and have
> zpos==n. The only problem is with function exynos_plane_get_type,

What concerns me was each plane could be mapped to wrong hardware
overlay. However, this was my miss leading. I missed the variable
'tmp' is used only to get plane type.

>
> which returns DRM_PLANE_TYPE_PRIMARY only if their 1st argument is equal 0,
> ie it assumes default window is always 0.
> As an simple workaround I used 'tmp' variable which maps 1st window to 0.
> This way exynos_plane_get_type returns expected value for all windows.
> I plan replace this workaround with something nicer, but as it would involve
> changes in other drivers I left this as TODO for later.

Ok, let's have more cleanup later.

Thanks,
Inki Dae

> Regarding ctx->planes[], it is static array so it will have always one entry
> invalid in case of TV, with this approach it will be th 0th entry, but with a gain
> of consistency in windows/zpos/planes numbering.
> If you prefer different approach please let me know.
>
> Regards
> Andrzej
>
> >
> > Thanks,
> > Inki Dae
> >
>
> _______________________________________________
> dri-devel mailing list
> dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/10] drm/exynos/decon5433: add support for DECON-TV

[Inki Dae]

2015-10-23 22:03 GMT+09:00 Andrzej Hajda <a.hajda@samsung.com>:
>
> On 10/23/2015 01:55 PM, Inki Dae wrote:
> > Hi Andrzej,
> >
> >
> > 2015년 10월 20일 18:22에 Andrzej Hajda 이(가) 쓴 글:
> >> DECON-TV IP is responsible for generating video stream which is transferred
> >> to HDMI IP. It is almost fully compatible with DECON IP.
> >>
> >> The patch is based on initial work of Hyungwon Hwang.
> >>
> >> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> >> ---
> >>   drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 154 ++++++++++++++++----------
> >>   include/video/exynos5433_decon.h              |  29 +++++
> >>   2 files changed, 122 insertions(+), 61 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> >> index 3c9aa4e..fbe1b31 100644
> >> --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> >> +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> >> @@ -13,6 +13,7 @@
> >>   #include <linux/platform_device.h>
> >>   #include <linux/clk.h>
> >>   #include <linux/component.h>
> >> +#include <linux/of_device.h>
> >>   #include <linux/of_gpio.h>
> >>   #include <linux/pm_runtime.h>
> >>
> >> @@ -37,6 +38,12 @@ static const char * const decon_clks_name[] = {
> >>      "sclk_decon_eclk",
> >>   };
> >>
> >> +enum decon_iftype {
> >> +    IFTYPE_RGB,
> >> +    IFTYPE_I80,
> >> +    IFTYPE_HDMI
> >> +};
> >> +
> >>   enum decon_flag_bits {
> >>      BIT_CLKS_ENABLED,
> >>      BIT_IRQS_ENABLED,
> >> @@ -53,7 +60,8 @@ struct decon_context {
> >>      struct clk                      *clks[ARRAY_SIZE(decon_clks_name)];
> >>      int                             pipe;
> >>      unsigned long                   flags;
> >> -    bool                            i80_if;
> >> +    enum decon_iftype               out_type;
> >> +    int                             first_win;
> >>   };
> >>
> >>   static const uint32_t decon_formats[] = {
> >> @@ -80,7 +88,7 @@ static int decon_enable_vblank(struct exynos_drm_crtc *crtc)
> >>
> >>      if (test_and_set_bit(BIT_IRQS_ENABLED, &ctx->flags)) {
> >>              val = VIDINTCON0_INTEN;
> >> -            if (ctx->i80_if)
> >> +            if (ctx->out_type == IFTYPE_I80)
> >>                      val |= VIDINTCON0_FRAMEDONE;
> >>              else
> >>                      val |= VIDINTCON0_INTFRMEN;
> >> @@ -104,8 +112,11 @@ static void decon_disable_vblank(struct exynos_drm_crtc *crtc)
> >>
> >>   static void decon_setup_trigger(struct decon_context *ctx)
> >>   {
> >> -    u32 val = TRIGCON_TRIGEN_PER_F | TRIGCON_TRIGEN_F |
> >> -                    TRIGCON_TE_AUTO_MASK | TRIGCON_SWTRIGEN;
> >> +    u32 val = (ctx->out_type != IFTYPE_HDMI)
> >> +            ? TRIGCON_TRIGEN_PER_F | TRIGCON_TRIGEN_F |
> >> +              TRIGCON_TE_AUTO_MASK | TRIGCON_SWTRIGEN
> >> +            : TRIGCON_TRIGEN_PER_F | TRIGCON_TRIGEN_F |
> >> +              TRIGCON_HWTRIGMASK_I80_RGB | TRIGCON_HWTRIGEN_I80_RGB;
> >>      writel(val, ctx->addr + DECON_TRIGCON);
> >>   }
> >>
> >> @@ -118,13 +129,22 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
> >>      if (test_bit(BIT_SUSPENDED, &ctx->flags))
> >>              return;
> >>
> >> +    if (ctx->out_type == IFTYPE_HDMI) {
> >> +            m->crtc_hsync_start = m->crtc_hdisplay + 10;
> >> +            m->crtc_hsync_end = m->crtc_htotal - 92;
> >> +            m->crtc_vsync_start = m->crtc_vdisplay + 1;
> >> +            m->crtc_vsync_end = m->crtc_vsync_start + 1;
> >> +    }
> >> +
> >> +    decon_set_bits(ctx, DECON_VIDCON0, VIDCON0_ENVID, 0);
> >> +
> >>      /* enable clock gate */
> >>      val = CMU_CLKGAGE_MODE_SFR_F | CMU_CLKGAGE_MODE_MEM_F;
> >>      writel(val, ctx->addr + DECON_CMU);
> >>
> >>      /* lcd on and use command if */
> >>      val = VIDOUT_LCD_ON;
> >> -    if (ctx->i80_if)
> >> +    if (ctx->out_type == IFTYPE_I80)
> >>              val |= VIDOUT_COMMAND_IF;
> >>      else
> >>              val |= VIDOUT_RGB_IF;
> >> @@ -134,7 +154,7 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
> >>              VIDTCON2_HOZVAL(m->hdisplay - 1);
> >>      writel(val, ctx->addr + DECON_VIDTCON2);
> >>
> >> -    if (!ctx->i80_if) {
> >> +    if (ctx->out_type != IFTYPE_I80) {
> >>              val = VIDTCON00_VBPD_F(
> >>                              m->crtc_vtotal - m->crtc_vsync_end - 1) |
> >>                      VIDTCON00_VFPD_F(
> >> @@ -159,15 +179,9 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
> >>      decon_setup_trigger(ctx);
> >>
> >>      /* enable output and display signal */
> >> -    val = VIDCON0_ENVID | VIDCON0_ENVID_F;
> >> -    writel(val, ctx->addr + DECON_VIDCON0);
> >> +    decon_set_bits(ctx, DECON_VIDCON0, VIDCON0_ENVID | VIDCON0_ENVID_F, ~0);
> >>   }
> >>
> >> -#define COORDINATE_X(x)             (((x) & 0xfff) << 12)
> >> -#define COORDINATE_Y(x)             ((x) & 0xfff)
> >> -#define OFFSIZE(x)          (((x) & 0x3fff) << 14)
> >> -#define PAGEWIDTH(x)                ((x) & 0x3fff)
> >> -
> >>   static void decon_win_set_pixfmt(struct decon_context *ctx, unsigned int win,
> >>                               struct drm_framebuffer *fb)
> >>   {
> >> @@ -238,6 +252,10 @@ static void decon_atomic_begin(struct exynos_drm_crtc *crtc,
> >>      decon_shadow_protect_win(ctx, plane->zpos, true);
> >>   }
> >>
> >> +#define BIT_VAL(x, e, s) (((x) & ((1 << ((e) - (s) + 1)) - 1)) << (s))
> >> +#define COORDINATE_X(x) BIT_VAL((x), 23, 12)
> >> +#define COORDINATE_Y(x) BIT_VAL((x), 11, 0)
> >> +
> >>   static void decon_update_plane(struct exynos_drm_crtc *crtc,
> >>                             struct exynos_drm_plane *plane)
> >>   {
> >> @@ -271,8 +289,12 @@ static void decon_update_plane(struct exynos_drm_crtc *crtc,
> >>      val = plane->dma_addr[0] + pitch * plane->crtc_h;
> >>      writel(val, ctx->addr + DECON_VIDW0xADD1B0(win));
> >>
> >> -    val = OFFSIZE(pitch - plane->crtc_w * bpp)
> >> -            | PAGEWIDTH(plane->crtc_w * bpp);
> >> +    if (ctx->out_type != IFTYPE_HDMI)
> >> +            val = BIT_VAL(pitch - plane->crtc_w * bpp, 27, 14)
> >> +                    | BIT_VAL(plane->crtc_w * bpp, 13, 0);
> >> +    else
> >> +            val = BIT_VAL(pitch - plane->crtc_w * bpp, 29, 15)
> >> +                    | BIT_VAL(plane->crtc_w * bpp, 14, 0);
> >>      writel(val, ctx->addr + DECON_VIDW0xADD2(win));
> >>
> >>      decon_win_set_pixfmt(ctx, win, state->fb);
> >> @@ -314,7 +336,7 @@ static void decon_atomic_flush(struct exynos_drm_crtc *crtc,
> >>
> >>      decon_shadow_protect_win(ctx, plane->zpos, false);
> >>
> >> -    if (ctx->i80_if)
> >> +    if (ctx->out_type == IFTYPE_I80)
> >>              set_bit(BIT_WIN_UPDATED, &ctx->flags);
> >>   }
> >>
> >> @@ -339,6 +361,17 @@ static void decon_swreset(struct decon_context *ctx)
> >>      }
> >>
> >>      WARN(tries == 0, "failed to software reset DECON\n");
> >> +
> >> +    if (ctx->out_type != IFTYPE_HDMI)
> >> +            return;
> >> +
> >> +    writel(VIDCON0_CLKVALUP | VIDCON0_VLCKFREE, ctx->addr + DECON_VIDCON0);
> >> +    decon_set_bits(ctx, DECON_CMU,
> >> +                   CMU_CLKGAGE_MODE_SFR_F | CMU_CLKGAGE_MODE_MEM_F, ~0);
> >> +    writel(VIDCON1_VCLK_RUN_VDEN_DISABLE, ctx->addr + DECON_VIDCON1);
> >> +    writel(CRCCTRL_CRCEN | CRCCTRL_CRCSTART_F | CRCCTRL_CRCCLKEN,
> >> +           ctx->addr + DECON_CRCCTRL);
> >> +    decon_setup_trigger(ctx);
> >>   }
> >>
> >>   static void decon_enable(struct exynos_drm_crtc *crtc)
> >> @@ -387,7 +420,7 @@ static void decon_disable(struct exynos_drm_crtc *crtc)
> >>       * suspend that connector. Otherwise we might try to scan from
> >>       * a destroyed buffer later.
> >>       */
> >> -    for (i = 0; i < WINDOWS_NR; i++)
> >> +    for (i = ctx->first_win; i < WINDOWS_NR; i++)
> >>              decon_disable_plane(crtc, &ctx->planes[i]);
> >>
> >>      decon_swreset(ctx);
> >> @@ -461,25 +494,30 @@ static int decon_bind(struct device *dev, struct device *master, void *data)
> >>      struct drm_device *drm_dev = data;
> >>      struct exynos_drm_private *priv = drm_dev->dev_private;
> >>      struct exynos_drm_plane *exynos_plane;
> >> +    enum exynos_drm_output_type out_type;
> >>      enum drm_plane_type type;
> >> -    unsigned int zpos;
> >> +    unsigned int win;
> >>      int ret;
> >>
> >>      ctx->drm_dev = drm_dev;
> >>      ctx->pipe = priv->pipe++;
> >>
> >> -    for (zpos = 0; zpos < WINDOWS_NR; zpos++) {
> >> -            type = exynos_plane_get_type(zpos, CURSOR_WIN);
> >> -            ret = exynos_plane_init(drm_dev, &ctx->planes[zpos],
> >> +    for (win = ctx->first_win; win < WINDOWS_NR; win++) {
> >> +            int tmp = (win == ctx->first_win) ? 0 : win;
> > AFAIK, DECON TV for Exynos5433 SoC has four hardware overlays so I guess
> > you used ctx->first_win to initialize four planes in case of IFTYPE_HDMI.
> >
> > However, with IFTYPE_HDMI ctx->first_win has 1, as a result,
> > ctx->planes[] will have 0, 2nd, 3rd and 4th planes and this means we
> > should use 0, 2nd ~ 4th hardware overlay excepting 1st overlay. Is this
> > your intention?
> > Shouldn't ctx->planes[] have 1st, 2nd ,3rd and 4th planes so that we can
> > use 1st ~ 4th hardware overlay for DECON TV?
> >
> > DECON TV for Exynos5433 SoC can use only 1st ~ 4th hardware overlays.
> >
> > And other thing, it may be trivial but I think it'd be better to use
> > 'ovl' instead of 'tmp.
>
> DECON-TV has hardware window 0 with very limited functionality,
> it is just background color, no scanout buffers. So it cannot be used
> as drm plane. ctx->first_win contains value of the 1st valid window.
> There is a question how to map windows to planes. I have chosen
> the simplest solution - n'th hw window is mapped to n'th plane and have
> zpos==n. The only problem is with function exynos_plane_get_type,

What concerns me was each plane could be mapped to wrong hardware
overlay. However, this was my miss leading. I missed the variable
'tmp' is used only to get plane type.

>
> which returns DRM_PLANE_TYPE_PRIMARY only if their 1st argument is equal 0,
> ie it assumes default window is always 0.
> As an simple workaround I used 'tmp' variable which maps 1st window to 0.
> This way exynos_plane_get_type returns expected value for all windows.
> I plan replace this workaround with something nicer, but as it would involve
> changes in other drivers I left this as TODO for later.

Ok, let's have more cleanup later.

Thanks,
Inki Dae

> Regarding ctx->planes[], it is static array so it will have always one entry
> invalid in case of TV, with this approach it will be th 0th entry, but with a gain
> of consistency in windows/zpos/planes numbering.
> If you prefer different approach please let me know.
>
> Regards
> Andrzej
>
> >
> > Thanks,
> > Inki Dae
> >
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 09/10] dt-bindings: video: exynos5433-decon: add bindings for DECON-TV

[Andrzej Hajda]

DECON-TV(Display and Enhancement Controller for TV) is a variation
of DECON IP. Its main purpose is to produce video stream for HDMI IP.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
Hi Krzysztof,

I have decided to skip cleanup part as it would require more work,
not related to this patchset.

Regards
Andrzej

 Documentation/devicetree/bindings/video/exynos5433-decon.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/video/exynos5433-decon.txt b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
index 3dff78b..c9fd7b3 100644
--- a/Documentation/devicetree/bindings/video/exynos5433-decon.txt
+++ b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
@@ -5,7 +5,8 @@ Exynos series of SoCs which transfers the image data from a video memory
 buffer to an external LCD interface.
 
 Required properties:
-- compatible: value should be "samsung,exynos5433-decon";
+- compatible: value should be one of:
+	"samsung,exynos5433-decon", "samsung,exynos5433-decon-tv";
 - reg: physical base address and length of the DECON registers set.
 - interrupts: should contain a list of all DECON IP block interrupts in the
 	      order: VSYNC, LCD_SYSTEM. The interrupt specifier format
-- 
1.9.1

Re: [PATCH v2 09/10] dt-bindings: video: exynos5433-decon: add bindings for DECON-TV

[Krzysztof Kozlowski]

On 26.10.2015 20:59, Andrzej Hajda wrote:
> DECON-TV(Display and Enhancement Controller for TV) is a variation
> of DECON IP. Its main purpose is to produce video stream for HDMI IP.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> Hi Krzysztof,
> 
> I have decided to skip cleanup part as it would require more work,
> not related to this patchset.
> 
> Regards
> Andrzej
> 
>  Documentation/devicetree/bindings/video/exynos5433-decon.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Thanks for splitting this, looks good:

Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

Re: [PATCH v2 09/10] dt-bindings: video: exynos5433-decon: add bindings for DECON-TV

[Rob Herring]

On Mon, Oct 26, 2015 at 12:59:56PM +0100, Andrzej Hajda wrote:
> DECON-TV(Display and Enhancement Controller for TV) is a variation
> of DECON IP. Its main purpose is to produce video stream for HDMI IP.
> 
> Signed-off-by: Andrzej Hajda <a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

> ---
> Hi Krzysztof,
> 
> I have decided to skip cleanup part as it would require more work,
> not related to this patchset.
> 
> Regards
> Andrzej
> 
>  Documentation/devicetree/bindings/video/exynos5433-decon.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/video/exynos5433-decon.txt b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
> index 3dff78b..c9fd7b3 100644
> --- a/Documentation/devicetree/bindings/video/exynos5433-decon.txt
> +++ b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
> @@ -5,7 +5,8 @@ Exynos series of SoCs which transfers the image data from a video memory
>  buffer to an external LCD interface.
>  
>  Required properties:
> -- compatible: value should be "samsung,exynos5433-decon";
> +- compatible: value should be one of:
> +	"samsung,exynos5433-decon", "samsung,exynos5433-decon-tv";
>  - reg: physical base address and length of the DECON registers set.
>  - interrupts: should contain a list of all DECON IP block interrupts in the
>  	      order: VSYNC, LCD_SYSTEM. The interrupt specifier format
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 09/10] dt-bindings: video: exynos5433-decon: add bindings for DECON-TV

[Rob Herring]

On Mon, Oct 26, 2015 at 12:59:56PM +0100, Andrzej Hajda wrote:
> DECON-TV(Display and Enhancement Controller for TV) is a variation
> of DECON IP. Its main purpose is to produce video stream for HDMI IP.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>

Acked-by: Rob Herring <robh@kernel.org>

> ---
> Hi Krzysztof,
> 
> I have decided to skip cleanup part as it would require more work,
> not related to this patchset.
> 
> Regards
> Andrzej
> 
>  Documentation/devicetree/bindings/video/exynos5433-decon.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/video/exynos5433-decon.txt b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
> index 3dff78b..c9fd7b3 100644
> --- a/Documentation/devicetree/bindings/video/exynos5433-decon.txt
> +++ b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
> @@ -5,7 +5,8 @@ Exynos series of SoCs which transfers the image data from a video memory
>  buffer to an external LCD interface.
>  
>  Required properties:
> -- compatible: value should be "samsung,exynos5433-decon";
> +- compatible: value should be one of:
> +	"samsung,exynos5433-decon", "samsung,exynos5433-decon-tv";
>  - reg: physical base address and length of the DECON registers set.
>  - interrupts: should contain a list of all DECON IP block interrupts in the
>  	      order: VSYNC, LCD_SYSTEM. The interrupt specifier format
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 09/10] dt-bindings: video: exynos5433-decon: add bindings for DECON-TV

[Andrzej Hajda]

Hi Inki,

It seems this patch and 04/10 did not get picked up yet.
Could you merge it?

Regards
Andrzej

On 10/26/2015 12:59 PM, Andrzej Hajda wrote:
> DECON-TV(Display and Enhancement Controller for TV) is a variation
> of DECON IP. Its main purpose is to produce video stream for HDMI IP.
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> Hi Krzysztof,
>
> I have decided to skip cleanup part as it would require more work,
> not related to this patchset.
>
> Regards
> Andrzej
>
>  Documentation/devicetree/bindings/video/exynos5433-decon.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/video/exynos5433-decon.txt b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
> index 3dff78b..c9fd7b3 100644
> --- a/Documentation/devicetree/bindings/video/exynos5433-decon.txt
> +++ b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
> @@ -5,7 +5,8 @@ Exynos series of SoCs which transfers the image data from a video memory
>  buffer to an external LCD interface.
>  
>  Required properties:
> -- compatible: value should be "samsung,exynos5433-decon";
> +- compatible: value should be one of:
> +	"samsung,exynos5433-decon", "samsung,exynos5433-decon-tv";
>  - reg: physical base address and length of the DECON registers set.
>  - interrupts: should contain a list of all DECON IP block interrupts in the
>  	      order: VSYNC, LCD_SYSTEM. The interrupt specifier format

Re: [PATCH v2 09/10] dt-bindings: video: exynos5433-decon: add bindings for DECON-TV

[Inki Dae]

+ Rob Herring

2016년 01월 14일 19:36에 Andrzej Hajda 이(가) 쓴 글:
> Hi Inki,
> 
> It seems this patch and 04/10 did not get picked up yet.
> Could you merge it?

Yes, I can. However, 04/10 is required for acked-by from dt or SoC maintainer.
Krzysztof and Rob, could you give me acked-by?

Thanks,
Inki Dae

> 
> Regards
> Andrzej
> 
> On 10/26/2015 12:59 PM, Andrzej Hajda wrote:
>> DECON-TV(Display and Enhancement Controller for TV) is a variation
>> of DECON IP. Its main purpose is to produce video stream for HDMI IP.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>> Hi Krzysztof,
>>
>> I have decided to skip cleanup part as it would require more work,
>> not related to this patchset.
>>
>> Regards
>> Andrzej
>>
>>  Documentation/devicetree/bindings/video/exynos5433-decon.txt | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/video/exynos5433-decon.txt b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>> index 3dff78b..c9fd7b3 100644
>> --- a/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>> +++ b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>> @@ -5,7 +5,8 @@ Exynos series of SoCs which transfers the image data from a video memory
>>  buffer to an external LCD interface.
>>  
>>  Required properties:
>> -- compatible: value should be "samsung,exynos5433-decon";
>> +- compatible: value should be one of:
>> +	"samsung,exynos5433-decon", "samsung,exynos5433-decon-tv";
>>  - reg: physical base address and length of the DECON registers set.
>>  - interrupts: should contain a list of all DECON IP block interrupts in the
>>  	      order: VSYNC, LCD_SYSTEM. The interrupt specifier format
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 09/10] dt-bindings: video: exynos5433-decon: add bindings for DECON-TV

[Inki Dae]

+ Rob Herring

2016년 01월 14일 19:36에 Andrzej Hajda 이(가) 쓴 글:
> Hi Inki,
> 
> It seems this patch and 04/10 did not get picked up yet.
> Could you merge it?

Yes, I can. However, 04/10 is required for acked-by from dt or SoC maintainer.
Krzysztof and Rob, could you give me acked-by?

Thanks,
Inki Dae

> 
> Regards
> Andrzej
> 
> On 10/26/2015 12:59 PM, Andrzej Hajda wrote:
>> DECON-TV(Display and Enhancement Controller for TV) is a variation
>> of DECON IP. Its main purpose is to produce video stream for HDMI IP.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>> Hi Krzysztof,
>>
>> I have decided to skip cleanup part as it would require more work,
>> not related to this patchset.
>>
>> Regards
>> Andrzej
>>
>>  Documentation/devicetree/bindings/video/exynos5433-decon.txt | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/video/exynos5433-decon.txt b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>> index 3dff78b..c9fd7b3 100644
>> --- a/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>> +++ b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>> @@ -5,7 +5,8 @@ Exynos series of SoCs which transfers the image data from a video memory
>>  buffer to an external LCD interface.
>>  
>>  Required properties:
>> -- compatible: value should be "samsung,exynos5433-decon";
>> +- compatible: value should be one of:
>> +	"samsung,exynos5433-decon", "samsung,exynos5433-decon-tv";
>>  - reg: physical base address and length of the DECON registers set.
>>  - interrupts: should contain a list of all DECON IP block interrupts in the
>>  	      order: VSYNC, LCD_SYSTEM. The interrupt specifier format
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Re: [PATCH v2 09/10] dt-bindings: video: exynos5433-decon: add bindings for DECON-TV

[Krzysztof Kozlowski]

On 14.01.2016 19:49, Inki Dae wrote:
> + Rob Herring
> 
> 2016년 01월 14일 19:36에 Andrzej Hajda 이(가) 쓴 글:
>> Hi Inki,
>>
>> It seems this patch and 04/10 did not get picked up yet.
>> Could you merge it?
> 
> Yes, I can. However, 04/10 is required for acked-by from dt or SoC maintainer.
> Krzysztof and Rob, could you give me acked-by?
> 
> Thanks,
> Inki Dae

My ack is really not required, because this is a binding for a specific
driver from video subsystem. It does not fall into the arm/mach-exynos
nor the dts/*exynos*.

Anyway I reviewed the patch on 28th of October and Rob acked it on 13th
of November.

Best regards,
Krzysztof


>>
>> Regards
>> Andrzej
>>
>> On 10/26/2015 12:59 PM, Andrzej Hajda wrote:
>>> DECON-TV(Display and Enhancement Controller for TV) is a variation
>>> of DECON IP. Its main purpose is to produce video stream for HDMI IP.
>>>
>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>> ---
>>> Hi Krzysztof,
>>>
>>> I have decided to skip cleanup part as it would require more work,
>>> not related to this patchset.
>>>
>>> Regards
>>> Andrzej
>>>
>>>  Documentation/devicetree/bindings/video/exynos5433-decon.txt | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/video/exynos5433-decon.txt b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>>> index 3dff78b..c9fd7b3 100644
>>> --- a/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>>> +++ b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>>> @@ -5,7 +5,8 @@ Exynos series of SoCs which transfers the image data from a video memory
>>>  buffer to an external LCD interface.
>>>  
>>>  Required properties:
>>> -- compatible: value should be "samsung,exynos5433-decon";
>>> +- compatible: value should be one of:
>>> +	"samsung,exynos5433-decon", "samsung,exynos5433-decon-tv";
>>>  - reg: physical base address and length of the DECON registers set.
>>>  - interrupts: should contain a list of all DECON IP block interrupts in the
>>>  	      order: VSYNC, LCD_SYSTEM. The interrupt specifier format
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
> 

Re: [PATCH v2 09/10] dt-bindings: video: exynos5433-decon: add bindings for DECON-TV

[Inki Dae]

2016년 01월 18일 08:45에 Krzysztof Kozlowski 이(가) 쓴 글:
> On 14.01.2016 19:49, Inki Dae wrote:
>> + Rob Herring
>>
>> 2016년 01월 14일 19:36에 Andrzej Hajda 이(가) 쓴 글:
>>> Hi Inki,
>>>
>>> It seems this patch and 04/10 did not get picked up yet.
>>> Could you merge it?
>>
>> Yes, I can. However, 04/10 is required for acked-by from dt or SoC maintainer.
>> Krzysztof and Rob, could you give me acked-by?
>>
>> Thanks,
>> Inki Dae
> 
> My ack is really not required, because this is a binding for a specific
> driver from video subsystem. It does not fall into the arm/mach-exynos
> nor the dts/*exynos*.
> 
> Anyway I reviewed the patch on 28th of October and Rob acked it on 13th
> of November.

I meant 04/10 not 09/10 patch. :)

Thanks,
Inki Dae

> 
> Best regards,
> Krzysztof
> 
> 
>>>
>>> Regards
>>> Andrzej
>>>
>>> On 10/26/2015 12:59 PM, Andrzej Hajda wrote:
>>>> DECON-TV(Display and Enhancement Controller for TV) is a variation
>>>> of DECON IP. Its main purpose is to produce video stream for HDMI IP.
>>>>
>>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>>> ---
>>>> Hi Krzysztof,
>>>>
>>>> I have decided to skip cleanup part as it would require more work,
>>>> not related to this patchset.
>>>>
>>>> Regards
>>>> Andrzej
>>>>
>>>>  Documentation/devicetree/bindings/video/exynos5433-decon.txt | 3 ++-
>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/video/exynos5433-decon.txt b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>>>> index 3dff78b..c9fd7b3 100644
>>>> --- a/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>>>> +++ b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>>>> @@ -5,7 +5,8 @@ Exynos series of SoCs which transfers the image data from a video memory
>>>>  buffer to an external LCD interface.
>>>>  
>>>>  Required properties:
>>>> -- compatible: value should be "samsung,exynos5433-decon";
>>>> +- compatible: value should be one of:
>>>> +	"samsung,exynos5433-decon", "samsung,exynos5433-decon-tv";
>>>>  - reg: physical base address and length of the DECON registers set.
>>>>  - interrupts: should contain a list of all DECON IP block interrupts in the
>>>>  	      order: VSYNC, LCD_SYSTEM. The interrupt specifier format
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>>
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Re: [PATCH v2 09/10] dt-bindings: video: exynos5433-decon: add bindings for DECON-TV

[Andrzej Hajda]

Hi Inki,

On 01/18/2016 10:54 AM, Inki Dae wrote:
>
> 2016년 01월 18일 08:45에 Krzysztof Kozlowski 이(가) 쓴 글:
>> On 14.01.2016 19:49, Inki Dae wrote:
>>> + Rob Herring
>>>
>>> 2016년 01월 14일 19:36에 Andrzej Hajda 이(가) 쓴 글:
>>>> Hi Inki,
>>>>
>>>> It seems this patch and 04/10 did not get picked up yet.
>>>> Could you merge it?
>>> Yes, I can. However, 04/10 is required for acked-by from dt or SoC maintainer.
>>> Krzysztof and Rob, could you give me acked-by?
>>>
>>> Thanks,
>>> Inki Dae
>> My ack is really not required, because this is a binding for a specific
>> driver from video subsystem. It does not fall into the arm/mach-exynos
>> nor the dts/*exynos*.
>>
>> Anyway I reviewed the patch on 28th of October and Rob acked it on 13th
>> of November.
> I meant 04/10 not 09/10 patch. :)

There is document in kernel with clear explanation about the subject:
Documentation/devicetree/bindings/submitting-patches.txt.
See especially paragraph II.2 [1].

[1]: Documentation/devicetree/bindings/submitting-patches.txt#L50

Regards
Andrzej

>
> Thanks,
> Inki Dae
>
>> Best regards,
>> Krzysztof
>>
>>
>>>> Regards
>>>> Andrzej
>>>>
>>>> On 10/26/2015 12:59 PM, Andrzej Hajda wrote:
>>>>> DECON-TV(Display and Enhancement Controller for TV) is a variation
>>>>> of DECON IP. Its main purpose is to produce video stream for HDMI IP.
>>>>>
>>>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>>>> ---
>>>>> Hi Krzysztof,
>>>>>
>>>>> I have decided to skip cleanup part as it would require more work,
>>>>> not related to this patchset.
>>>>>
>>>>> Regards
>>>>> Andrzej
>>>>>
>>>>>  Documentation/devicetree/bindings/video/exynos5433-decon.txt | 3 ++-
>>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/video/exynos5433-decon.txt b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>>>>> index 3dff78b..c9fd7b3 100644
>>>>> --- a/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>>>>> +++ b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>>>>> @@ -5,7 +5,8 @@ Exynos series of SoCs which transfers the image data from a video memory
>>>>>  buffer to an external LCD interface.
>>>>>  
>>>>>  Required properties:
>>>>> -- compatible: value should be "samsung,exynos5433-decon";
>>>>> +- compatible: value should be one of:
>>>>> +	"samsung,exynos5433-decon", "samsung,exynos5433-decon-tv";
>>>>>  - reg: physical base address and length of the DECON registers set.
>>>>>  - interrupts: should contain a list of all DECON IP block interrupts in the
>>>>>  	      order: VSYNC, LCD_SYSTEM. The interrupt specifier format
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>
>>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 09/10] dt-bindings: video: exynos5433-decon: add bindings for DECON-TV

[Andrzej Hajda]

Hi Inki,

On 01/18/2016 10:54 AM, Inki Dae wrote:
>
> 2016년 01월 18일 08:45에 Krzysztof Kozlowski 이(가) 쓴 글:
>> On 14.01.2016 19:49, Inki Dae wrote:
>>> + Rob Herring
>>>
>>> 2016년 01월 14일 19:36에 Andrzej Hajda 이(가) 쓴 글:
>>>> Hi Inki,
>>>>
>>>> It seems this patch and 04/10 did not get picked up yet.
>>>> Could you merge it?
>>> Yes, I can. However, 04/10 is required for acked-by from dt or SoC maintainer.
>>> Krzysztof and Rob, could you give me acked-by?
>>>
>>> Thanks,
>>> Inki Dae
>> My ack is really not required, because this is a binding for a specific
>> driver from video subsystem. It does not fall into the arm/mach-exynos
>> nor the dts/*exynos*.
>>
>> Anyway I reviewed the patch on 28th of October and Rob acked it on 13th
>> of November.
> I meant 04/10 not 09/10 patch. :)

There is document in kernel with clear explanation about the subject:
Documentation/devicetree/bindings/submitting-patches.txt.
See especially paragraph II.2 [1].

[1]: Documentation/devicetree/bindings/submitting-patches.txt#L50

Regards
Andrzej

>
> Thanks,
> Inki Dae
>
>> Best regards,
>> Krzysztof
>>
>>
>>>> Regards
>>>> Andrzej
>>>>
>>>> On 10/26/2015 12:59 PM, Andrzej Hajda wrote:
>>>>> DECON-TV(Display and Enhancement Controller for TV) is a variation
>>>>> of DECON IP. Its main purpose is to produce video stream for HDMI IP.
>>>>>
>>>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>>>> ---
>>>>> Hi Krzysztof,
>>>>>
>>>>> I have decided to skip cleanup part as it would require more work,
>>>>> not related to this patchset.
>>>>>
>>>>> Regards
>>>>> Andrzej
>>>>>
>>>>>  Documentation/devicetree/bindings/video/exynos5433-decon.txt | 3 ++-
>>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/video/exynos5433-decon.txt b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>>>>> index 3dff78b..c9fd7b3 100644
>>>>> --- a/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>>>>> +++ b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>>>>> @@ -5,7 +5,8 @@ Exynos series of SoCs which transfers the image data from a video memory
>>>>>  buffer to an external LCD interface.
>>>>>  
>>>>>  Required properties:
>>>>> -- compatible: value should be "samsung,exynos5433-decon";
>>>>> +- compatible: value should be one of:
>>>>> +	"samsung,exynos5433-decon", "samsung,exynos5433-decon-tv";
>>>>>  - reg: physical base address and length of the DECON registers set.
>>>>>  - interrupts: should contain a list of all DECON IP block interrupts in the
>>>>>  	      order: VSYNC, LCD_SYSTEM. The interrupt specifier format
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>
>>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>

Re: [PATCH v2 09/10] dt-bindings: video: exynos5433-decon: add bindings for DECON-TV

[Inki Dae]

2016년 01월 18일 19:12에 Andrzej Hajda 이(가) 쓴 글:
> Hi Inki,
> 
> On 01/18/2016 10:54 AM, Inki Dae wrote:
>>
>> 2016년 01월 18일 08:45에 Krzysztof Kozlowski 이(가) 쓴 글:
>>> On 14.01.2016 19:49, Inki Dae wrote:
>>>> + Rob Herring
>>>>
>>>> 2016년 01월 14일 19:36에 Andrzej Hajda 이(가) 쓴 글:
>>>>> Hi Inki,
>>>>>
>>>>> It seems this patch and 04/10 did not get picked up yet.
>>>>> Could you merge it?
>>>> Yes, I can. However, 04/10 is required for acked-by from dt or SoC maintainer.
>>>> Krzysztof and Rob, could you give me acked-by?
>>>>
>>>> Thanks,
>>>> Inki Dae
>>> My ack is really not required, because this is a binding for a specific
>>> driver from video subsystem. It does not fall into the arm/mach-exynos
>>> nor the dts/*exynos*.
>>>
>>> Anyway I reviewed the patch on 28th of October and Rob acked it on 13th
>>> of November.
>> I meant 04/10 not 09/10 patch. :)
> 
> There is document in kernel with clear explanation about the subject:
> Documentation/devicetree/bindings/submitting-patches.txt.
> See especially paragraph II.2 [1].
> 
> [1]: Documentation/devicetree/bindings/submitting-patches.txt#L50

Got it. The document says,
"For driver (not subsystem) bindings: If you are comfortable with the
 binding, and it hasn't received an Acked-by from the devicetree
 maintainers after a few weeks, go ahead and take it."

Thanks,
Inki Dae

> 
> Regards
> Andrzej
> 
>>
>> Thanks,
>> Inki Dae
>>
>>> Best regards,
>>> Krzysztof
>>>
>>>
>>>>> Regards
>>>>> Andrzej
>>>>>
>>>>> On 10/26/2015 12:59 PM, Andrzej Hajda wrote:
>>>>>> DECON-TV(Display and Enhancement Controller for TV) is a variation
>>>>>> of DECON IP. Its main purpose is to produce video stream for HDMI IP.
>>>>>>
>>>>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>>>>> ---
>>>>>> Hi Krzysztof,
>>>>>>
>>>>>> I have decided to skip cleanup part as it would require more work,
>>>>>> not related to this patchset.
>>>>>>
>>>>>> Regards
>>>>>> Andrzej
>>>>>>
>>>>>>  Documentation/devicetree/bindings/video/exynos5433-decon.txt | 3 ++-
>>>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/video/exynos5433-decon.txt b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>>>>>> index 3dff78b..c9fd7b3 100644
>>>>>> --- a/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>>>>>> +++ b/Documentation/devicetree/bindings/video/exynos5433-decon.txt
>>>>>> @@ -5,7 +5,8 @@ Exynos series of SoCs which transfers the image data from a video memory
>>>>>>  buffer to an external LCD interface.
>>>>>>  
>>>>>>  Required properties:
>>>>>> -- compatible: value should be "samsung,exynos5433-decon";
>>>>>> +- compatible: value should be one of:
>>>>>> +	"samsung,exynos5433-decon", "samsung,exynos5433-decon-tv";
>>>>>>  - reg: physical base address and length of the DECON registers set.
>>>>>>  - interrupts: should contain a list of all DECON IP block interrupts in the
>>>>>>  	      order: VSYNC, LCD_SYSTEM. The interrupt specifier format
>>>>> --
>>>>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>>>>> the body of a message to majordomo@vger.kernel.org
>>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>>
>>>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
>