This series is based on exynos-drm-next branch of Inki Dae's tree at: git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git
Changes since V2: Add DT property to know the type of FIMD output interface as per Inki's suggestion. Add samsung,output-type DT property in FIMD node for all boards supporting DP interface.
Ajay Kumar (7): [PATCH V3 1/7] drm/exynos: Support DP CLKCON register in FIMD driver [PATCH V3 2/7] ARM: dts: Add FIMD output property for snow [PATCH V3 3/7] ARM: dts: Add FIMD output property for peach_pit [PATCH V3 4/7] ARM: dts: Add FIMD output property for peach_pi [PATCH V3 5/7] ARM: dts: Add FIMD output property for smdk5250 [PATCH V3 6/7] ARM: dts: Add FIMD output property for smdk5420 [PATCH V3 7/7] ARM: dts: Add FIMD output property for arndale
.../devicetree/bindings/video/samsung-fimd.txt | 1 + arch/arm/boot/dts/exynos5250-arndale.dts | 1 + arch/arm/boot/dts/exynos5250-smdk5250.dts | 1 + arch/arm/boot/dts/exynos5250-snow.dts | 1 + arch/arm/boot/dts/exynos5420-peach-pit.dts | 1 + arch/arm/boot/dts/exynos5420-smdk5420.dts | 1 + arch/arm/boot/dts/exynos5800-peach-pi.dts | 1 + drivers/gpu/drm/exynos/exynos_drm_fimd.c | 23 ++++++++++++++++++++ include/video/samsung_fimd.h | 4 ++++ 9 files changed, 34 insertions(+)
Add the missing setting for DP CLKCON register.
This register is present on Exynos5 based FIMD controllers, and needs to be set if we are using DP.
Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com --- .../devicetree/bindings/video/samsung-fimd.txt | 1 + drivers/gpu/drm/exynos/exynos_drm_fimd.c | 23 ++++++++++++++++++++ include/video/samsung_fimd.h | 4 ++++ 3 files changed, 28 insertions(+)
diff --git a/Documentation/devicetree/bindings/video/samsung-fimd.txt b/Documentation/devicetree/bindings/video/samsung-fimd.txt index 2dad41b..12f3d7a 100644 --- a/Documentation/devicetree/bindings/video/samsung-fimd.txt +++ b/Documentation/devicetree/bindings/video/samsung-fimd.txt @@ -41,6 +41,7 @@ Optional Properties: - samsung,power-domain: a phandle to FIMD power domain node. - samsung,invert-vden: video enable signal is inverted - samsung,invert-vclk: video clock signal is inverted +- samsung,output-type: Type of display output interface(DPI=0, DSI=1, DP=2) - display-timings: timing settings for FIMD, as described in document [1]. Can be used in case timings cannot be provided otherwise or to override timings provided by the panel. diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 33161ad..aa74e90 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -72,6 +72,7 @@ struct fimd_driver_data { unsigned int has_shadowcon:1; unsigned int has_clksel:1; unsigned int has_limited_fmt:1; + unsigned int has_dp_clkcon:1; };
static struct fimd_driver_data s3c64xx_fimd_driver_data = { @@ -88,6 +89,13 @@ static struct fimd_driver_data exynos4_fimd_driver_data = { static struct fimd_driver_data exynos5_fimd_driver_data = { .timing_base = 0x20000, .has_shadowcon = 1, + .has_dp_clkcon = 1, +}; + +enum exynos_fimd_output_type { + EXYNOS_FIMD_OUTPUT_DPI, + EXYNOS_FIMD_OUTPUT_DSI, + EXYNOS_FIMD_OUTPUT_DP, };
struct fimd_win_data { @@ -125,6 +133,8 @@ struct fimd_context { struct exynos_drm_panel_info panel; struct fimd_driver_data *driver_data; struct exynos_drm_display *display; + + enum exynos_fimd_output_type exynos_fimd_output_type; };
static const struct of_device_id fimd_driver_dt_match[] = { @@ -331,6 +341,10 @@ static void fimd_commit(struct exynos_drm_manager *mgr) if (clkdiv > 1) val |= VIDCON0_CLKVAL_F(clkdiv - 1) | VIDCON0_CLKDIR;
+ if (ctx->driver_data->has_dp_clkcon && + ctx->exynos_fimd_output_type == EXYNOS_FIMD_OUTPUT_DP) + writel(DP_CLK_ENABLE, ctx->regs + DP_CLKCON); + writel(val, ctx->regs + VIDCON0); }
@@ -924,6 +938,7 @@ static int fimd_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct fimd_context *ctx; struct resource *res; + u32 fimd_output_type; int ret = -EINVAL;
ret = exynos_drm_component_add(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC, @@ -949,6 +964,14 @@ static int fimd_probe(struct platform_device *pdev) ctx->vidcon1 |= VIDCON1_INV_VDEN; if (of_property_read_bool(dev->of_node, "samsung,invert-vclk")) ctx->vidcon1 |= VIDCON1_INV_VCLK; + if (!of_property_read_u32(dev->of_node, "samsung,output-type", + &fimd_output_type)) { + if ((fimd_output_type < EXYNOS_FIMD_OUTPUT_DPI) || + (fimd_output_type > EXYNOS_FIMD_OUTPUT_DP)) + dev_err(dev, "invalid output type for FIMD\n"); + else + ctx->exynos_fimd_output_type = fimd_output_type; + }
ctx->bus_clk = devm_clk_get(dev, "fimd"); if (IS_ERR(ctx->bus_clk)) { diff --git a/include/video/samsung_fimd.h b/include/video/samsung_fimd.h index b039320..d8f4b0b 100644 --- a/include/video/samsung_fimd.h +++ b/include/video/samsung_fimd.h @@ -435,6 +435,10 @@ #define BLENDCON_NEW_8BIT_ALPHA_VALUE (1 << 0) #define BLENDCON_NEW_4BIT_ALPHA_VALUE (0 << 0)
+/* Video clock enable for DP */ +#define DP_CLKCON 0x27C +#define DP_CLK_ENABLE 0x2 + /* Notes on per-window bpp settings * * Value Win0 Win1 Win2 Win3 Win 4
On Friday, June 27, 2014 7:12 PM, Ajay Kumar wrote:
Add the missing setting for DP CLKCON register.
This register is present on Exynos5 based FIMD controllers, and needs to be set if we are using DP.
Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com
Reviewed-by: Jingoo Han jg1.han@samsung.com
Best regards, Jingoo Han
.../devicetree/bindings/video/samsung-fimd.txt | 1 + drivers/gpu/drm/exynos/exynos_drm_fimd.c | 23 ++++++++++++++++++++ include/video/samsung_fimd.h | 4 ++++ 3 files changed, 28 insertions(+)
diff --git a/Documentation/devicetree/bindings/video/samsung-fimd.txt b/Documentation/devicetree/bindings/video/samsung-fimd.txt index 2dad41b..12f3d7a 100644 --- a/Documentation/devicetree/bindings/video/samsung-fimd.txt +++ b/Documentation/devicetree/bindings/video/samsung-fimd.txt @@ -41,6 +41,7 @@ Optional Properties:
- samsung,power-domain: a phandle to FIMD power domain node.
- samsung,invert-vden: video enable signal is inverted
- samsung,invert-vclk: video clock signal is inverted
+- samsung,output-type: Type of display output interface(DPI=0, DSI=1, DP=2)
- display-timings: timing settings for FIMD, as described in document [1]. Can be used in case timings cannot be provided otherwise or to override timings provided by the panel.
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 33161ad..aa74e90 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -72,6 +72,7 @@ struct fimd_driver_data { unsigned int has_shadowcon:1; unsigned int has_clksel:1; unsigned int has_limited_fmt:1;
- unsigned int has_dp_clkcon:1;
};
static struct fimd_driver_data s3c64xx_fimd_driver_data = { @@ -88,6 +89,13 @@ static struct fimd_driver_data exynos4_fimd_driver_data = { static struct fimd_driver_data exynos5_fimd_driver_data = { .timing_base = 0x20000, .has_shadowcon = 1,
- .has_dp_clkcon = 1,
+};
+enum exynos_fimd_output_type {
- EXYNOS_FIMD_OUTPUT_DPI,
- EXYNOS_FIMD_OUTPUT_DSI,
- EXYNOS_FIMD_OUTPUT_DP,
};
struct fimd_win_data { @@ -125,6 +133,8 @@ struct fimd_context { struct exynos_drm_panel_info panel; struct fimd_driver_data *driver_data; struct exynos_drm_display *display;
- enum exynos_fimd_output_type exynos_fimd_output_type;
};
static const struct of_device_id fimd_driver_dt_match[] = { @@ -331,6 +341,10 @@ static void fimd_commit(struct exynos_drm_manager *mgr) if (clkdiv > 1) val |= VIDCON0_CLKVAL_F(clkdiv - 1) | VIDCON0_CLKDIR;
- if (ctx->driver_data->has_dp_clkcon &&
ctx->exynos_fimd_output_type == EXYNOS_FIMD_OUTPUT_DP)writel(DP_CLK_ENABLE, ctx->regs + DP_CLKCON);- writel(val, ctx->regs + VIDCON0);
}
@@ -924,6 +938,7 @@ static int fimd_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct fimd_context *ctx; struct resource *res;
u32 fimd_output_type; int ret = -EINVAL;
ret = exynos_drm_component_add(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC,
@@ -949,6 +964,14 @@ static int fimd_probe(struct platform_device *pdev) ctx->vidcon1 |= VIDCON1_INV_VDEN; if (of_property_read_bool(dev->of_node, "samsung,invert-vclk")) ctx->vidcon1 |= VIDCON1_INV_VCLK;
if (!of_property_read_u32(dev->of_node, "samsung,output-type",
&fimd_output_type)) {if ((fimd_output_type < EXYNOS_FIMD_OUTPUT_DPI) ||(fimd_output_type > EXYNOS_FIMD_OUTPUT_DP))dev_err(dev, "invalid output type for FIMD\n");elsectx->exynos_fimd_output_type = fimd_output_type;}
ctx->bus_clk = devm_clk_get(dev, "fimd"); if (IS_ERR(ctx->bus_clk)) {
diff --git a/include/video/samsung_fimd.h b/include/video/samsung_fimd.h index b039320..d8f4b0b 100644 --- a/include/video/samsung_fimd.h +++ b/include/video/samsung_fimd.h @@ -435,6 +435,10 @@ #define BLENDCON_NEW_8BIT_ALPHA_VALUE (1 << 0) #define BLENDCON_NEW_4BIT_ALPHA_VALUE (0 << 0)
+/* Video clock enable for DP */ +#define DP_CLKCON 0x27C +#define DP_CLK_ENABLE 0x2
/* Notes on per-window bpp settings
- Value Win0 Win1 Win2 Win3 Win 4
-- 1.7.9.5
Hi Ajay,
+CC DT
On 06/27/2014 12:12 PM, Ajay Kumar wrote:
Add the missing setting for DP CLKCON register.
This register is present on Exynos5 based FIMD controllers, and needs to be set if we are using DP.
Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com
.../devicetree/bindings/video/samsung-fimd.txt | 1 + drivers/gpu/drm/exynos/exynos_drm_fimd.c | 23 ++++++++++++++++++++ include/video/samsung_fimd.h | 4 ++++ 3 files changed, 28 insertions(+)
diff --git a/Documentation/devicetree/bindings/video/samsung-fimd.txt b/Documentation/devicetree/bindings/video/samsung-fimd.txt index 2dad41b..12f3d7a 100644 --- a/Documentation/devicetree/bindings/video/samsung-fimd.txt +++ b/Documentation/devicetree/bindings/video/samsung-fimd.txt @@ -41,6 +41,7 @@ Optional Properties:
- samsung,power-domain: a phandle to FIMD power domain node.
- samsung,invert-vden: video enable signal is inverted
- samsung,invert-vclk: video clock signal is inverted
+- samsung,output-type: Type of display output interface(DPI=0, DSI=1, DP=2)
There is no point in introducing this property. Exynos DRM have already logic which creates pipeline: fimd --> DPI|DSI|DP, this logic can be reused to determine display type. It can be done even without any additional callbacks, just by checking if there is connector of DRM_MODE_CONNECTOR_eDP type connected to fimd.
Regards Andrzej
- display-timings: timing settings for FIMD, as described in document [1]. Can be used in case timings cannot be provided otherwise or to override timings provided by the panel.
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 33161ad..aa74e90 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -72,6 +72,7 @@ struct fimd_driver_data { unsigned int has_shadowcon:1; unsigned int has_clksel:1; unsigned int has_limited_fmt:1;
- unsigned int has_dp_clkcon:1;
};
static struct fimd_driver_data s3c64xx_fimd_driver_data = { @@ -88,6 +89,13 @@ static struct fimd_driver_data exynos4_fimd_driver_data = { static struct fimd_driver_data exynos5_fimd_driver_data = { .timing_base = 0x20000, .has_shadowcon = 1,
- .has_dp_clkcon = 1,
+};
+enum exynos_fimd_output_type {
- EXYNOS_FIMD_OUTPUT_DPI,
- EXYNOS_FIMD_OUTPUT_DSI,
- EXYNOS_FIMD_OUTPUT_DP,
};
struct fimd_win_data { @@ -125,6 +133,8 @@ struct fimd_context { struct exynos_drm_panel_info panel; struct fimd_driver_data *driver_data; struct exynos_drm_display *display;
- enum exynos_fimd_output_type exynos_fimd_output_type;
};
static const struct of_device_id fimd_driver_dt_match[] = { @@ -331,6 +341,10 @@ static void fimd_commit(struct exynos_drm_manager *mgr) if (clkdiv > 1) val |= VIDCON0_CLKVAL_F(clkdiv - 1) | VIDCON0_CLKDIR;
- if (ctx->driver_data->has_dp_clkcon &&
ctx->exynos_fimd_output_type == EXYNOS_FIMD_OUTPUT_DP)writel(DP_CLK_ENABLE, ctx->regs + DP_CLKCON);- writel(val, ctx->regs + VIDCON0);
}
@@ -924,6 +938,7 @@ static int fimd_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct fimd_context *ctx; struct resource *res;
u32 fimd_output_type; int ret = -EINVAL;
ret = exynos_drm_component_add(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC,
@@ -949,6 +964,14 @@ static int fimd_probe(struct platform_device *pdev) ctx->vidcon1 |= VIDCON1_INV_VDEN; if (of_property_read_bool(dev->of_node, "samsung,invert-vclk")) ctx->vidcon1 |= VIDCON1_INV_VCLK;
if (!of_property_read_u32(dev->of_node, "samsung,output-type",
&fimd_output_type)) {if ((fimd_output_type < EXYNOS_FIMD_OUTPUT_DPI) ||(fimd_output_type > EXYNOS_FIMD_OUTPUT_DP))dev_err(dev, "invalid output type for FIMD\n");elsectx->exynos_fimd_output_type = fimd_output_type;}
ctx->bus_clk = devm_clk_get(dev, "fimd"); if (IS_ERR(ctx->bus_clk)) {
diff --git a/include/video/samsung_fimd.h b/include/video/samsung_fimd.h index b039320..d8f4b0b 100644 --- a/include/video/samsung_fimd.h +++ b/include/video/samsung_fimd.h @@ -435,6 +435,10 @@ #define BLENDCON_NEW_8BIT_ALPHA_VALUE (1 << 0) #define BLENDCON_NEW_4BIT_ALPHA_VALUE (0 << 0)
+/* Video clock enable for DP */ +#define DP_CLKCON 0x27C +#define DP_CLK_ENABLE 0x2
/* Notes on per-window bpp settings
- Value Win0 Win1 Win2 Win3 Win 4
Hi Andrej,
On Fri, Jun 27, 2014 at 4:52 PM, Andrzej Hajda a.hajda@samsung.com wrote:
Hi Ajay,
+CC DT
On 06/27/2014 12:12 PM, Ajay Kumar wrote:
Add the missing setting for DP CLKCON register.
This register is present on Exynos5 based FIMD controllers, and needs to be set if we are using DP.
Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com
.../devicetree/bindings/video/samsung-fimd.txt | 1 + drivers/gpu/drm/exynos/exynos_drm_fimd.c | 23 ++++++++++++++++++++ include/video/samsung_fimd.h | 4 ++++ 3 files changed, 28 insertions(+)
diff --git a/Documentation/devicetree/bindings/video/samsung-fimd.txt b/Documentation/devicetree/bindings/video/samsung-fimd.txt index 2dad41b..12f3d7a 100644 --- a/Documentation/devicetree/bindings/video/samsung-fimd.txt +++ b/Documentation/devicetree/bindings/video/samsung-fimd.txt @@ -41,6 +41,7 @@ Optional Properties:
- samsung,power-domain: a phandle to FIMD power domain node.
- samsung,invert-vden: video enable signal is inverted
- samsung,invert-vclk: video clock signal is inverted
+- samsung,output-type: Type of display output interface(DPI=0, DSI=1, DP=2)
There is no point in introducing this property. Exynos DRM have already logic which creates pipeline: fimd --> DPI|DSI|DP, this logic can be reused to determine display type. It can be done even without any additional callbacks, just by checking if there is connector of DRM_MODE_CONNECTOR_eDP type connected to fimd.
The mapping between crtc(struct exynos_drm_manager) and encoder(struct exynos_drm_display) in exynos drm happens by matching the exynos_drm_output_type variable in each structure. exynos_drm_output_type supports 3 types: LCD, HDMI and VIDI. FIMD statically chooses EXYNOS_DISPLAY_TYPE_LCD as the output type, and both DP and MIPI statically choose the same enum EXYNOS_DISPLAY_TYPE_LCD, as output type. So, we cannot use that logic to differentiate between DP/MIPI DSI.
Also, checking based on connector type doesn't hold good. The connector type will be DRM_MODE_CONNECTOR_LVDS in case of DP->LVDS or MIPI->LVDS panels!
Thanks and regards, Ajay Kumar
Regards Andrzej
- display-timings: timing settings for FIMD, as described in document [1]. Can be used in case timings cannot be provided otherwise or to override timings provided by the panel.
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 33161ad..aa74e90 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -72,6 +72,7 @@ struct fimd_driver_data { unsigned int has_shadowcon:1; unsigned int has_clksel:1; unsigned int has_limited_fmt:1;
unsigned int has_dp_clkcon:1;};
static struct fimd_driver_data s3c64xx_fimd_driver_data = { @@ -88,6 +89,13 @@ static struct fimd_driver_data exynos4_fimd_driver_data = { static struct fimd_driver_data exynos5_fimd_driver_data = { .timing_base = 0x20000, .has_shadowcon = 1,
.has_dp_clkcon = 1,+};
+enum exynos_fimd_output_type {
EXYNOS_FIMD_OUTPUT_DPI,EXYNOS_FIMD_OUTPUT_DSI,EXYNOS_FIMD_OUTPUT_DP,};
struct fimd_win_data { @@ -125,6 +133,8 @@ struct fimd_context { struct exynos_drm_panel_info panel; struct fimd_driver_data *driver_data; struct exynos_drm_display *display;
enum exynos_fimd_output_type exynos_fimd_output_type;};
static const struct of_device_id fimd_driver_dt_match[] = { @@ -331,6 +341,10 @@ static void fimd_commit(struct exynos_drm_manager *mgr) if (clkdiv > 1) val |= VIDCON0_CLKVAL_F(clkdiv - 1) | VIDCON0_CLKDIR;
if (ctx->driver_data->has_dp_clkcon &&ctx->exynos_fimd_output_type == EXYNOS_FIMD_OUTPUT_DP)writel(DP_CLK_ENABLE, ctx->regs + DP_CLKCON);writel(val, ctx->regs + VIDCON0);}
@@ -924,6 +938,7 @@ static int fimd_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct fimd_context *ctx; struct resource *res;
u32 fimd_output_type; int ret = -EINVAL; ret = exynos_drm_component_add(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC,@@ -949,6 +964,14 @@ static int fimd_probe(struct platform_device *pdev) ctx->vidcon1 |= VIDCON1_INV_VDEN; if (of_property_read_bool(dev->of_node, "samsung,invert-vclk")) ctx->vidcon1 |= VIDCON1_INV_VCLK;
if (!of_property_read_u32(dev->of_node, "samsung,output-type",&fimd_output_type)) {if ((fimd_output_type < EXYNOS_FIMD_OUTPUT_DPI) ||(fimd_output_type > EXYNOS_FIMD_OUTPUT_DP))dev_err(dev, "invalid output type for FIMD\n");elsectx->exynos_fimd_output_type = fimd_output_type;} ctx->bus_clk = devm_clk_get(dev, "fimd"); if (IS_ERR(ctx->bus_clk)) {diff --git a/include/video/samsung_fimd.h b/include/video/samsung_fimd.h index b039320..d8f4b0b 100644 --- a/include/video/samsung_fimd.h +++ b/include/video/samsung_fimd.h @@ -435,6 +435,10 @@ #define BLENDCON_NEW_8BIT_ALPHA_VALUE (1 << 0) #define BLENDCON_NEW_4BIT_ALPHA_VALUE (0 << 0)
+/* Video clock enable for DP */ +#define DP_CLKCON 0x27C +#define DP_CLK_ENABLE 0x2
/* Notes on per-window bpp settings
- Value Win0 Win1 Win2 Win3 Win 4
On 06/27/2014 01:48 PM, Ajay kumar wrote:
Hi Andrej,
On Fri, Jun 27, 2014 at 4:52 PM, Andrzej Hajda a.hajda@samsung.com wrote:
Hi Ajay,
+CC DT
On 06/27/2014 12:12 PM, Ajay Kumar wrote:
Add the missing setting for DP CLKCON register.
This register is present on Exynos5 based FIMD controllers, and needs to be set if we are using DP.
Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com
.../devicetree/bindings/video/samsung-fimd.txt | 1 + drivers/gpu/drm/exynos/exynos_drm_fimd.c | 23 ++++++++++++++++++++ include/video/samsung_fimd.h | 4 ++++ 3 files changed, 28 insertions(+)
diff --git a/Documentation/devicetree/bindings/video/samsung-fimd.txt b/Documentation/devicetree/bindings/video/samsung-fimd.txt index 2dad41b..12f3d7a 100644 --- a/Documentation/devicetree/bindings/video/samsung-fimd.txt +++ b/Documentation/devicetree/bindings/video/samsung-fimd.txt @@ -41,6 +41,7 @@ Optional Properties:
- samsung,power-domain: a phandle to FIMD power domain node.
- samsung,invert-vden: video enable signal is inverted
- samsung,invert-vclk: video clock signal is inverted
+- samsung,output-type: Type of display output interface(DPI=0, DSI=1, DP=2)
There is no point in introducing this property. Exynos DRM have already logic which creates pipeline: fimd --> DPI|DSI|DP, this logic can be reused to determine display type. It can be done even without any additional callbacks, just by checking if there is connector of DRM_MODE_CONNECTOR_eDP type connected to fimd.
The mapping between crtc(struct exynos_drm_manager) and encoder(struct exynos_drm_display) in exynos drm happens by matching the exynos_drm_output_type variable in each structure. exynos_drm_output_type supports 3 types: LCD, HDMI and VIDI. FIMD statically chooses EXYNOS_DISPLAY_TYPE_LCD as the output type, and both DP and MIPI statically choose the same enum EXYNOS_DISPLAY_TYPE_LCD, as output type. So, we cannot use that logic to differentiate between DP/MIPI DSI.
Also, checking based on connector type doesn't hold good. The connector type will be DRM_MODE_CONNECTOR_LVDS in case of DP->LVDS or MIPI->LVDS panels!
True, I forgot about bridges. So additional callback/field is necessary. See below.
Thanks and regards, Ajay Kumar
Regards Andrzej
- display-timings: timing settings for FIMD, as described in document [1]. Can be used in case timings cannot be provided otherwise or to override timings provided by the panel.
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 33161ad..aa74e90 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -72,6 +72,7 @@ struct fimd_driver_data { unsigned int has_shadowcon:1; unsigned int has_clksel:1; unsigned int has_limited_fmt:1;
unsigned int has_dp_clkcon:1;};
static struct fimd_driver_data s3c64xx_fimd_driver_data = { @@ -88,6 +89,13 @@ static struct fimd_driver_data exynos4_fimd_driver_data = { static struct fimd_driver_data exynos5_fimd_driver_data = { .timing_base = 0x20000, .has_shadowcon = 1,
.has_dp_clkcon = 1,+};
+enum exynos_fimd_output_type {
EXYNOS_FIMD_OUTPUT_DPI,EXYNOS_FIMD_OUTPUT_DSI,EXYNOS_FIMD_OUTPUT_DP,};
struct fimd_win_data { @@ -125,6 +133,8 @@ struct fimd_context { struct exynos_drm_panel_info panel; struct fimd_driver_data *driver_data; struct exynos_drm_display *display;
enum exynos_fimd_output_type exynos_fimd_output_type;};
static const struct of_device_id fimd_driver_dt_match[] = { @@ -331,6 +341,10 @@ static void fimd_commit(struct exynos_drm_manager *mgr) if (clkdiv > 1) val |= VIDCON0_CLKVAL_F(clkdiv - 1) | VIDCON0_CLKDIR;
if (ctx->driver_data->has_dp_clkcon &&ctx->exynos_fimd_output_type == EXYNOS_FIMD_OUTPUT_DP)writel(DP_CLK_ENABLE, ctx->regs + DP_CLKCON);writel(val, ctx->regs + VIDCON0);
New code should not split VIDCON0 related code. It should be moved few lines above or few lines below. Anyway this code should be rather placed in power related functions of dp encoder, as it enables dp. The only question is if DP_CLKCON update can be performed after VIDCON0 update. If yes the solution of the whole problem seems to be simple: - fimd should provide function fimd_set_dp_clk_gate or sth similar, - this function should be called in exynos_dp_poweron/exynos_dp_poweroff. I hope I have not missed anything this time.
Regards Andrzej
}
@@ -924,6 +938,7 @@ static int fimd_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct fimd_context *ctx; struct resource *res;
u32 fimd_output_type; int ret = -EINVAL; ret = exynos_drm_component_add(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC,@@ -949,6 +964,14 @@ static int fimd_probe(struct platform_device *pdev) ctx->vidcon1 |= VIDCON1_INV_VDEN; if (of_property_read_bool(dev->of_node, "samsung,invert-vclk")) ctx->vidcon1 |= VIDCON1_INV_VCLK;
if (!of_property_read_u32(dev->of_node, "samsung,output-type",&fimd_output_type)) {if ((fimd_output_type < EXYNOS_FIMD_OUTPUT_DPI) ||(fimd_output_type > EXYNOS_FIMD_OUTPUT_DP))dev_err(dev, "invalid output type for FIMD\n");elsectx->exynos_fimd_output_type = fimd_output_type;} ctx->bus_clk = devm_clk_get(dev, "fimd"); if (IS_ERR(ctx->bus_clk)) {diff --git a/include/video/samsung_fimd.h b/include/video/samsung_fimd.h index b039320..d8f4b0b 100644 --- a/include/video/samsung_fimd.h +++ b/include/video/samsung_fimd.h @@ -435,6 +435,10 @@ #define BLENDCON_NEW_8BIT_ALPHA_VALUE (1 << 0) #define BLENDCON_NEW_4BIT_ALPHA_VALUE (0 << 0)
+/* Video clock enable for DP */ +#define DP_CLKCON 0x27C +#define DP_CLK_ENABLE 0x2
/* Notes on per-window bpp settings
- Value Win0 Win1 Win2 Win3 Win 4
Hi Andrej,
On Fri, Jun 27, 2014 at 6:14 PM, Andrzej Hajda a.hajda@samsung.com wrote:
On 06/27/2014 01:48 PM, Ajay kumar wrote:
Hi Andrej,
On Fri, Jun 27, 2014 at 4:52 PM, Andrzej Hajda a.hajda@samsung.com wrote:
Hi Ajay,
+CC DT
On 06/27/2014 12:12 PM, Ajay Kumar wrote:
Add the missing setting for DP CLKCON register.
This register is present on Exynos5 based FIMD controllers, and needs to be set if we are using DP.
Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com
.../devicetree/bindings/video/samsung-fimd.txt | 1 + drivers/gpu/drm/exynos/exynos_drm_fimd.c | 23 ++++++++++++++++++++ include/video/samsung_fimd.h | 4 ++++ 3 files changed, 28 insertions(+)
diff --git a/Documentation/devicetree/bindings/video/samsung-fimd.txt b/Documentation/devicetree/bindings/video/samsung-fimd.txt index 2dad41b..12f3d7a 100644 --- a/Documentation/devicetree/bindings/video/samsung-fimd.txt +++ b/Documentation/devicetree/bindings/video/samsung-fimd.txt @@ -41,6 +41,7 @@ Optional Properties:
- samsung,power-domain: a phandle to FIMD power domain node.
- samsung,invert-vden: video enable signal is inverted
- samsung,invert-vclk: video clock signal is inverted
+- samsung,output-type: Type of display output interface(DPI=0, DSI=1, DP=2)
There is no point in introducing this property. Exynos DRM have already logic which creates pipeline: fimd --> DPI|DSI|DP, this logic can be reused to determine display type. It can be done even without any additional callbacks, just by checking if there is connector of DRM_MODE_CONNECTOR_eDP type connected to fimd.
The mapping between crtc(struct exynos_drm_manager) and encoder(struct exynos_drm_display) in exynos drm happens by matching the exynos_drm_output_type variable in each structure. exynos_drm_output_type supports 3 types: LCD, HDMI and VIDI. FIMD statically chooses EXYNOS_DISPLAY_TYPE_LCD as the output type, and both DP and MIPI statically choose the same enum EXYNOS_DISPLAY_TYPE_LCD, as output type. So, we cannot use that logic to differentiate between DP/MIPI DSI.
Also, checking based on connector type doesn't hold good. The connector type will be DRM_MODE_CONNECTOR_LVDS in case of DP->LVDS or MIPI->LVDS panels!
True, I forgot about bridges. So additional callback/field is necessary. See below.
Thanks and regards, Ajay Kumar
Regards Andrzej
- display-timings: timing settings for FIMD, as described in document [1]. Can be used in case timings cannot be provided otherwise or to override timings provided by the panel.
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 33161ad..aa74e90 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -72,6 +72,7 @@ struct fimd_driver_data { unsigned int has_shadowcon:1; unsigned int has_clksel:1; unsigned int has_limited_fmt:1;
unsigned int has_dp_clkcon:1;};
static struct fimd_driver_data s3c64xx_fimd_driver_data = { @@ -88,6 +89,13 @@ static struct fimd_driver_data exynos4_fimd_driver_data = { static struct fimd_driver_data exynos5_fimd_driver_data = { .timing_base = 0x20000, .has_shadowcon = 1,
.has_dp_clkcon = 1,+};
+enum exynos_fimd_output_type {
EXYNOS_FIMD_OUTPUT_DPI,EXYNOS_FIMD_OUTPUT_DSI,EXYNOS_FIMD_OUTPUT_DP,};
struct fimd_win_data { @@ -125,6 +133,8 @@ struct fimd_context { struct exynos_drm_panel_info panel; struct fimd_driver_data *driver_data; struct exynos_drm_display *display;
enum exynos_fimd_output_type exynos_fimd_output_type;};
static const struct of_device_id fimd_driver_dt_match[] = { @@ -331,6 +341,10 @@ static void fimd_commit(struct exynos_drm_manager *mgr) if (clkdiv > 1) val |= VIDCON0_CLKVAL_F(clkdiv - 1) | VIDCON0_CLKDIR;
if (ctx->driver_data->has_dp_clkcon &&ctx->exynos_fimd_output_type == EXYNOS_FIMD_OUTPUT_DP)writel(DP_CLK_ENABLE, ctx->regs + DP_CLKCON);writel(val, ctx->regs + VIDCON0);New code should not split VIDCON0 related code.It should be moved few lines above or few lines below.
Ok, for better readability.
Anyway this code should be rather placed in power related functions of dp encoder, as it enables dp. The only question is if DP_CLKCON update can be performed after VIDCON0 update. If yes the solution of the whole problem
I will check this.
seems to be simple:
- fimd should provide function fimd_set_dp_clk_gate or sth similar,
- this function should be called in exynos_dp_poweron/exynos_dp_poweroff.
I hope I have not missed anything this time.
But, it won't look good to export a FIMD function which sets a FIMD register, and call it in DP driver! What does Inki/Jingoo have to say about this?
Regards, Ajay
}
@@ -924,6 +938,7 @@ static int fimd_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct fimd_context *ctx; struct resource *res;
u32 fimd_output_type; int ret = -EINVAL; ret = exynos_drm_component_add(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC,@@ -949,6 +964,14 @@ static int fimd_probe(struct platform_device *pdev) ctx->vidcon1 |= VIDCON1_INV_VDEN; if (of_property_read_bool(dev->of_node, "samsung,invert-vclk")) ctx->vidcon1 |= VIDCON1_INV_VCLK;
if (!of_property_read_u32(dev->of_node, "samsung,output-type",&fimd_output_type)) {if ((fimd_output_type < EXYNOS_FIMD_OUTPUT_DPI) ||(fimd_output_type > EXYNOS_FIMD_OUTPUT_DP))dev_err(dev, "invalid output type for FIMD\n");elsectx->exynos_fimd_output_type = fimd_output_type;} ctx->bus_clk = devm_clk_get(dev, "fimd"); if (IS_ERR(ctx->bus_clk)) {diff --git a/include/video/samsung_fimd.h b/include/video/samsung_fimd.h index b039320..d8f4b0b 100644 --- a/include/video/samsung_fimd.h +++ b/include/video/samsung_fimd.h @@ -435,6 +435,10 @@ #define BLENDCON_NEW_8BIT_ALPHA_VALUE (1 << 0) #define BLENDCON_NEW_4BIT_ALPHA_VALUE (0 << 0)
+/* Video clock enable for DP */ +#define DP_CLKCON 0x27C +#define DP_CLK_ENABLE 0x2
/* Notes on per-window bpp settings
- Value Win0 Win1 Win2 Win3 Win 4
On 27.06.2014 15:02, Ajay kumar wrote:
Hi Andrej,
On Fri, Jun 27, 2014 at 6:14 PM, Andrzej Hajda a.hajda@samsung.com wrote:
On 06/27/2014 01:48 PM, Ajay kumar wrote:
Hi Andrej,
On Fri, Jun 27, 2014 at 4:52 PM, Andrzej Hajda a.hajda@samsung.com wrote:
Hi Ajay,
+CC DT
On 06/27/2014 12:12 PM, Ajay Kumar wrote:
Add the missing setting for DP CLKCON register.
This register is present on Exynos5 based FIMD controllers, and needs to be set if we are using DP.
Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com
.../devicetree/bindings/video/samsung-fimd.txt | 1 + drivers/gpu/drm/exynos/exynos_drm_fimd.c | 23 ++++++++++++++++++++ include/video/samsung_fimd.h | 4 ++++ 3 files changed, 28 insertions(+)
diff --git a/Documentation/devicetree/bindings/video/samsung-fimd.txt b/Documentation/devicetree/bindings/video/samsung-fimd.txt index 2dad41b..12f3d7a 100644 --- a/Documentation/devicetree/bindings/video/samsung-fimd.txt +++ b/Documentation/devicetree/bindings/video/samsung-fimd.txt @@ -41,6 +41,7 @@ Optional Properties:
- samsung,power-domain: a phandle to FIMD power domain node.
- samsung,invert-vden: video enable signal is inverted
- samsung,invert-vclk: video clock signal is inverted
+- samsung,output-type: Type of display output interface(DPI=0, DSI=1, DP=2)
There is no point in introducing this property. Exynos DRM have already logic which creates pipeline: fimd --> DPI|DSI|DP, this logic can be reused to determine display type. It can be done even without any additional callbacks, just by checking if there is connector of DRM_MODE_CONNECTOR_eDP type connected to fimd.
The mapping between crtc(struct exynos_drm_manager) and encoder(struct exynos_drm_display) in exynos drm happens by matching the exynos_drm_output_type variable in each structure. exynos_drm_output_type supports 3 types: LCD, HDMI and VIDI. FIMD statically chooses EXYNOS_DISPLAY_TYPE_LCD as the output type, and both DP and MIPI statically choose the same enum EXYNOS_DISPLAY_TYPE_LCD, as output type. So, we cannot use that logic to differentiate between DP/MIPI DSI.
Also, checking based on connector type doesn't hold good. The connector type will be DRM_MODE_CONNECTOR_LVDS in case of DP->LVDS or MIPI->LVDS panels!
True, I forgot about bridges. So additional callback/field is necessary. See below.
Thanks and regards, Ajay Kumar
Regards Andrzej
- display-timings: timing settings for FIMD, as described in document [1]. Can be used in case timings cannot be provided otherwise or to override timings provided by the panel.
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 33161ad..aa74e90 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -72,6 +72,7 @@ struct fimd_driver_data { unsigned int has_shadowcon:1; unsigned int has_clksel:1; unsigned int has_limited_fmt:1;
unsigned int has_dp_clkcon:1;};
static struct fimd_driver_data s3c64xx_fimd_driver_data = { @@ -88,6 +89,13 @@ static struct fimd_driver_data exynos4_fimd_driver_data = { static struct fimd_driver_data exynos5_fimd_driver_data = { .timing_base = 0x20000, .has_shadowcon = 1,
.has_dp_clkcon = 1,+};
+enum exynos_fimd_output_type {
EXYNOS_FIMD_OUTPUT_DPI,EXYNOS_FIMD_OUTPUT_DSI,EXYNOS_FIMD_OUTPUT_DP,};
struct fimd_win_data { @@ -125,6 +133,8 @@ struct fimd_context { struct exynos_drm_panel_info panel; struct fimd_driver_data *driver_data; struct exynos_drm_display *display;
enum exynos_fimd_output_type exynos_fimd_output_type;};
static const struct of_device_id fimd_driver_dt_match[] = { @@ -331,6 +341,10 @@ static void fimd_commit(struct exynos_drm_manager *mgr) if (clkdiv > 1) val |= VIDCON0_CLKVAL_F(clkdiv - 1) | VIDCON0_CLKDIR;
if (ctx->driver_data->has_dp_clkcon &&ctx->exynos_fimd_output_type == EXYNOS_FIMD_OUTPUT_DP)writel(DP_CLK_ENABLE, ctx->regs + DP_CLKCON);writel(val, ctx->regs + VIDCON0);New code should not split VIDCON0 related code.It should be moved few lines above or few lines below.
Ok, for better readability.
Anyway this code should be rather placed in power related functions of dp encoder, as it enables dp. The only question is if DP_CLKCON update can be performed after VIDCON0 update. If yes the solution of the whole problem
I will check this.
seems to be simple:
- fimd should provide function fimd_set_dp_clk_gate or sth similar,
- this function should be called in exynos_dp_poweron/exynos_dp_poweroff.
I hope I have not missed anything this time.
But, it won't look good to export a FIMD function which sets a FIMD register, and call it in DP driver! What does Inki/Jingoo have to say about this?
It looks much better than exporting knowledge already existing in Exynos DRM subsystem to firmware interface, though, and this approach can be easily modified in future, as opposed to _stable_ DT bindings.
Best regards, Tomasz
On Friday, June 27, 2014 10:03 PM, Ajay kumar wrote:
On Fri, Jun 27, 2014 at 6:14 PM, Andrzej Hajda a.hajda@samsung.com wrote:
On 06/27/2014 01:48 PM, Ajay kumar wrote:
On Fri, Jun 27, 2014 at 4:52 PM, Andrzej Hajda a.hajda@samsung.com wrote:
+CC DT
On 06/27/2014 12:12 PM, Ajay Kumar wrote:
Add the missing setting for DP CLKCON register.
This register is present on Exynos5 based FIMD controllers, and needs to be set if we are using DP.
Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com
.../devicetree/bindings/video/samsung-fimd.txt | 1 + drivers/gpu/drm/exynos/exynos_drm_fimd.c | 23 ++++++++++++++++++++ include/video/samsung_fimd.h | 4 ++++ 3 files changed, 28 insertions(+)
[.....]
static const struct of_device_id fimd_driver_dt_match[] = { @@ -331,6 +341,10 @@ static void fimd_commit(struct exynos_drm_manager *mgr) if (clkdiv > 1) val |= VIDCON0_CLKVAL_F(clkdiv - 1) | VIDCON0_CLKDIR;
if (ctx->driver_data->has_dp_clkcon &&ctx->exynos_fimd_output_type == EXYNOS_FIMD_OUTPUT_DP)writel(DP_CLK_ENABLE, ctx->regs + DP_CLKCON);writel(val, ctx->regs + VIDCON0);New code should not split VIDCON0 related code.It should be moved few lines above or few lines below.
Ok, for better readability.
Anyway this code should be rather placed in power related functions of dp encoder, as it enables dp. The only question is if DP_CLKCON update can be performed after VIDCON0 update. If yes the solution of the whole problem
I will check this.
seems to be simple:
- fimd should provide function fimd_set_dp_clk_gate or sth similar,
- this function should be called in exynos_dp_poweron/exynos_dp_poweroff.
I hope I have not missed anything this time.
But, it won't look good to export a FIMD function which sets a FIMD register, and call it in DP driver! What does Inki/Jingoo have to say about this?
I agree with Ajay Kumar's opinion. It doesn't look good to export the function to set FIMD register and call it by DP driver.
Best regards, Jingoo Han
Regards, Ajay
[....]
On 06/30/2014 03:14 AM, Jingoo Han wrote:
On Friday, June 27, 2014 10:03 PM, Ajay kumar wrote:
On Fri, Jun 27, 2014 at 6:14 PM, Andrzej Hajda a.hajda@samsung.com wrote:
On 06/27/2014 01:48 PM, Ajay kumar wrote:
On Fri, Jun 27, 2014 at 4:52 PM, Andrzej Hajda a.hajda@samsung.com wrote:
+CC DT
On 06/27/2014 12:12 PM, Ajay Kumar wrote:
Add the missing setting for DP CLKCON register.
This register is present on Exynos5 based FIMD controllers, and needs to be set if we are using DP.
Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com
.../devicetree/bindings/video/samsung-fimd.txt | 1 + drivers/gpu/drm/exynos/exynos_drm_fimd.c | 23 ++++++++++++++++++++ include/video/samsung_fimd.h | 4 ++++ 3 files changed, 28 insertions(+)
[.....]
static const struct of_device_id fimd_driver_dt_match[] = { @@ -331,6 +341,10 @@ static void fimd_commit(struct exynos_drm_manager *mgr) if (clkdiv > 1) val |= VIDCON0_CLKVAL_F(clkdiv - 1) | VIDCON0_CLKDIR;
if (ctx->driver_data->has_dp_clkcon &&ctx->exynos_fimd_output_type == EXYNOS_FIMD_OUTPUT_DP)writel(DP_CLK_ENABLE, ctx->regs + DP_CLKCON);writel(val, ctx->regs + VIDCON0);New code should not split VIDCON0 related code.It should be moved few lines above or few lines below.
Ok, for better readability.
Anyway this code should be rather placed in power related functions of dp encoder, as it enables dp. The only question is if DP_CLKCON update can be performed after VIDCON0 update. If yes the solution of the whole problem
I will check this.
seems to be simple:
- fimd should provide function fimd_set_dp_clk_gate or sth similar,
- this function should be called in exynos_dp_poweron/exynos_dp_poweroff.
I hope I have not missed anything this time.
But, it won't look good to export a FIMD function which sets a FIMD register, and call it in DP driver! What does Inki/Jingoo have to say about this?
I agree with Ajay Kumar's opinion. It doesn't look good to export the function to set FIMD register and call it by DP driver.
DP_CLKCON HW register shows clearly there is direct hardware dependency between DP and FIMD. Reflecting this dependency in drivers is just a consequence of HW design. Moreover the register gates also clock for MDNIE, this solution can be used there as well.
Anyway the most important is that we should avoid adding DT bindings for things we can evaluate in drivers.
Regards Andrzej
Best regards, Jingoo Han
Regards, Ajay
[....]
Hi Inki,
On Mon, Jun 30, 2014 at 11:01 AM, Andrzej Hajda a.hajda@samsung.com wrote:
On 06/30/2014 03:14 AM, Jingoo Han wrote:
On Friday, June 27, 2014 10:03 PM, Ajay kumar wrote:
On Fri, Jun 27, 2014 at 6:14 PM, Andrzej Hajda a.hajda@samsung.com wrote:
On 06/27/2014 01:48 PM, Ajay kumar wrote:
On Fri, Jun 27, 2014 at 4:52 PM, Andrzej Hajda a.hajda@samsung.com wrote:
+CC DT
On 06/27/2014 12:12 PM, Ajay Kumar wrote: > Add the missing setting for DP CLKCON register. > > This register is present on Exynos5 based FIMD controllers, > and needs to be set if we are using DP. > > Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com > --- > .../devicetree/bindings/video/samsung-fimd.txt | 1 + > drivers/gpu/drm/exynos/exynos_drm_fimd.c | 23 ++++++++++++++++++++ > include/video/samsung_fimd.h | 4 ++++ > 3 files changed, 28 insertions(+)
[.....]
> static const struct of_device_id fimd_driver_dt_match[] = { > @@ -331,6 +341,10 @@ static void fimd_commit(struct exynos_drm_manager *mgr) > if (clkdiv > 1) > val |= VIDCON0_CLKVAL_F(clkdiv - 1) | VIDCON0_CLKDIR; > > + if (ctx->driver_data->has_dp_clkcon && > + ctx->exynos_fimd_output_type == EXYNOS_FIMD_OUTPUT_DP) > + writel(DP_CLK_ENABLE, ctx->regs + DP_CLKCON); > + > writel(val, ctx->regs + VIDCON0);
New code should not split VIDCON0 related code.It should be moved few lines above or few lines below.
Ok, for better readability.
Anyway this code should be rather placed in power related functions of dp encoder, as it enables dp. The only question is if DP_CLKCON update can be performed after VIDCON0 update. If yes the solution of the whole problem
I will check this.
seems to be simple:
- fimd should provide function fimd_set_dp_clk_gate or sth similar,
- this function should be called in exynos_dp_poweron/exynos_dp_poweroff.
I hope I have not missed anything this time.
But, it won't look good to export a FIMD function which sets a FIMD register, and call it in DP driver! What does Inki/Jingoo have to say about this?
I agree with Ajay Kumar's opinion. It doesn't look good to export the function to set FIMD register and call it by DP driver.
DP_CLKCON HW register shows clearly there is direct hardware dependency between DP and FIMD. Reflecting this dependency in drivers is just a consequence of HW design. Moreover the register gates also clock for MDNIE, this solution can be used there as well.
Anyway the most important is that we should avoid adding DT bindings for things we can evaluate in drivers.
Which approach do you think is better? I shall make the patch for the same!
Thanks and regards, Ajay Kumar
2014-06-30 14:31 GMT+09:00 Andrzej Hajda a.hajda@samsung.com:
On 06/30/2014 03:14 AM, Jingoo Han wrote:
On Friday, June 27, 2014 10:03 PM, Ajay kumar wrote:
On Fri, Jun 27, 2014 at 6:14 PM, Andrzej Hajda a.hajda@samsung.com wrote:
On 06/27/2014 01:48 PM, Ajay kumar wrote:
On Fri, Jun 27, 2014 at 4:52 PM, Andrzej Hajda a.hajda@samsung.com wrote:
+CC DT
On 06/27/2014 12:12 PM, Ajay Kumar wrote: > Add the missing setting for DP CLKCON register. > > This register is present on Exynos5 based FIMD controllers, > and needs to be set if we are using DP. > > Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com > --- > .../devicetree/bindings/video/samsung-fimd.txt | 1 + > drivers/gpu/drm/exynos/exynos_drm_fimd.c | 23 ++++++++++++++++++++ > include/video/samsung_fimd.h | 4 ++++ > 3 files changed, 28 insertions(+)
[.....]
> static const struct of_device_id fimd_driver_dt_match[] = { > @@ -331,6 +341,10 @@ static void fimd_commit(struct exynos_drm_manager *mgr) > if (clkdiv > 1) > val |= VIDCON0_CLKVAL_F(clkdiv - 1) | VIDCON0_CLKDIR; > > + if (ctx->driver_data->has_dp_clkcon && > + ctx->exynos_fimd_output_type == EXYNOS_FIMD_OUTPUT_DP) > + writel(DP_CLK_ENABLE, ctx->regs + DP_CLKCON); > + > writel(val, ctx->regs + VIDCON0);
New code should not split VIDCON0 related code.It should be moved few lines above or few lines below.
Ok, for better readability.
Anyway this code should be rather placed in power related functions of dp encoder, as it enables dp. The only question is if DP_CLKCON update can be performed after VIDCON0 update. If yes the solution of the whole problem
I will check this.
seems to be simple:
- fimd should provide function fimd_set_dp_clk_gate or sth similar,
- this function should be called in exynos_dp_poweron/exynos_dp_poweroff.
I hope I have not missed anything this time.
But, it won't look good to export a FIMD function which sets a FIMD register, and call it in DP driver! What does Inki/Jingoo have to say about this?
I agree with Ajay Kumar's opinion. It doesn't look good to export the function to set FIMD register and call it by DP driver.
DP_CLKCON HW register shows clearly there is direct hardware dependency between DP and FIMD. Reflecting this dependency in drivers is just a consequence of HW design.
Right, and I cannot understand why mDNIe and DP clock enable bit exists in FIMD ip. :(
Moreover the register gates also clock for MDNIE, this solution can be used there as well.
Anyway the most important is that we should avoid adding DT bindings for things we can evaluate in drivers.
It wouldn't be best way only to avoid adding DT binding. DT binding could be good way to handle complicated hardware pipelines if needed. Of course, if driver can handle it simply, it would be better to avoid adding DT binding. However, Exynos SoC are complicated.
Exynos SoC have more IPs to should be considered; SMIES, mDNIe and MIE as image enhancement devices, and eDP, MIPI-DSI, and DPI (FIMD connected to panel directly) as Display bus devices and parallel panel device. And image enhancement device and Display bus device can be used together.
FIMD -------- Panel FIMD -------- Display bus device -------- Panel FIMD -------- image enhancement device -------- Panel FIMD -------- image enhancement device -------- FIMD-Lite -------- Panel FIMD -------- image enhancement device -------- Display bus device -------- Panel FIMD -------- image enhancement device -------- FIMD-Lite -------- Display bus device -------- Panel
And Display bus devices and parallel device couldn't be switched in runtime since kernel has been booted. However, image enhancement devices can be enabled or disabled in runtime so the output path of FIMD can be changed to another path dynamically - actually, I had handled such scenarios. So if Exynos drm driver should be considered for above all cases, it'd make Eyxnos drm driver too complicated.
If DT people and other SoC maintainers give us your opinions, it would be helpful for us. I will look into other SoC how they are handling similar cases.
Thanks, Inki Dae
Regards Andrzej
Best regards, Jingoo Han
Regards, Ajay
[....]
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
ping
On Mon, Jun 30, 2014 at 9:39 PM, Inki Dae inki.dae@samsung.com wrote:
2014-06-30 14:31 GMT+09:00 Andrzej Hajda a.hajda@samsung.com:
On 06/30/2014 03:14 AM, Jingoo Han wrote:
On Friday, June 27, 2014 10:03 PM, Ajay kumar wrote:
On Fri, Jun 27, 2014 at 6:14 PM, Andrzej Hajda a.hajda@samsung.com wrote:
On 06/27/2014 01:48 PM, Ajay kumar wrote:
On Fri, Jun 27, 2014 at 4:52 PM, Andrzej Hajda a.hajda@samsung.com wrote: > +CC DT > > On 06/27/2014 12:12 PM, Ajay Kumar wrote: >> Add the missing setting for DP CLKCON register. >> >> This register is present on Exynos5 based FIMD controllers, >> and needs to be set if we are using DP. >> >> Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com >> --- >> .../devicetree/bindings/video/samsung-fimd.txt | 1 + >> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 23 ++++++++++++++++++++ >> include/video/samsung_fimd.h | 4 ++++ >> 3 files changed, 28 insertions(+)
[.....]
>> static const struct of_device_id fimd_driver_dt_match[] = { >> @@ -331,6 +341,10 @@ static void fimd_commit(struct exynos_drm_manager *mgr) >> if (clkdiv > 1) >> val |= VIDCON0_CLKVAL_F(clkdiv - 1) | VIDCON0_CLKDIR; >> >> + if (ctx->driver_data->has_dp_clkcon && >> + ctx->exynos_fimd_output_type == EXYNOS_FIMD_OUTPUT_DP) >> + writel(DP_CLK_ENABLE, ctx->regs + DP_CLKCON); >> + >> writel(val, ctx->regs + VIDCON0);
New code should not split VIDCON0 related code.It should be moved few lines above or few lines below.
Ok, for better readability.
Anyway this code should be rather placed in power related functions of dp encoder, as it enables dp. The only question is if DP_CLKCON update can be performed after VIDCON0 update. If yes the solution of the whole problem
I will check this.
seems to be simple:
- fimd should provide function fimd_set_dp_clk_gate or sth similar,
- this function should be called in exynos_dp_poweron/exynos_dp_poweroff.
I hope I have not missed anything this time.
But, it won't look good to export a FIMD function which sets a FIMD register, and call it in DP driver! What does Inki/Jingoo have to say about this?
I agree with Ajay Kumar's opinion. It doesn't look good to export the function to set FIMD register and call it by DP driver.
DP_CLKCON HW register shows clearly there is direct hardware dependency between DP and FIMD. Reflecting this dependency in drivers is just a consequence of HW design.
Right, and I cannot understand why mDNIe and DP clock enable bit exists in FIMD ip. :(
Moreover the register gates also clock for MDNIE, this solution can be used there as well.
Anyway the most important is that we should avoid adding DT bindings for things we can evaluate in drivers.
It wouldn't be best way only to avoid adding DT binding. DT binding could be good way to handle complicated hardware pipelines if needed. Of course, if driver can handle it simply, it would be better to avoid adding DT binding. However, Exynos SoC are complicated.
Exynos SoC have more IPs to should be considered; SMIES, mDNIe and MIE as image enhancement devices, and eDP, MIPI-DSI, and DPI (FIMD connected to panel directly) as Display bus devices and parallel panel device. And image enhancement device and Display bus device can be used together.
FIMD -------- Panel FIMD -------- Display bus device -------- Panel FIMD -------- image enhancement device -------- Panel FIMD -------- image enhancement device -------- FIMD-Lite -------- Panel FIMD -------- image enhancement device -------- Display bus device -------- Panel FIMD -------- image enhancement device -------- FIMD-Lite -------- Display bus device -------- Panel
And Display bus devices and parallel device couldn't be switched in runtime since kernel has been booted. However, image enhancement devices can be enabled or disabled in runtime so the output path of FIMD can be changed to another path dynamically - actually, I had handled such scenarios. So if Exynos drm driver should be considered for above all cases, it'd make Eyxnos drm driver too complicated.
If DT people and other SoC maintainers give us your opinions, it would be helpful for us. I will look into other SoC how they are handling similar cases.
Thanks, Inki Dae
Regards Andrzej
Best regards, Jingoo Han
Regards, Ajay
[....]
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Set FIMD output type for snow as DP.
Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com --- arch/arm/boot/dts/exynos5250-snow.dts | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts index 079fdf9..dc3ae9c 100644 --- a/arch/arm/boot/dts/exynos5250-snow.dts +++ b/arch/arm/boot/dts/exynos5250-snow.dts @@ -400,6 +400,7 @@ fimd@14400000 { status = "okay"; samsung,invert-vclk; + samsung,output-type = <2>; };
dp-controller@145B0000 {
+DT mailing list
On Fri, Jun 27, 2014 at 3:42 PM, Ajay Kumar ajaykumar.rs@samsung.com wrote:
Set FIMD output type for snow as DP.
Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com
arch/arm/boot/dts/exynos5250-snow.dts | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts index 079fdf9..dc3ae9c 100644 --- a/arch/arm/boot/dts/exynos5250-snow.dts +++ b/arch/arm/boot/dts/exynos5250-snow.dts @@ -400,6 +400,7 @@ fimd@14400000 { status = "okay"; samsung,invert-vclk;
samsung,output-type = <2>; }; dp-controller@145B0000 {-- 1.7.9.5
Set FIMD output type for peach_pit as DP.
Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com --- arch/arm/boot/dts/exynos5420-peach-pit.dts | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts b/arch/arm/boot/dts/exynos5420-peach-pit.dts index 1c5b8f9..164f523 100644 --- a/arch/arm/boot/dts/exynos5420-peach-pit.dts +++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts @@ -255,6 +255,7 @@ &fimd { status = "okay"; samsung,invert-vclk; + samsung,output-type = <2>; };
&dp {
+DT mailing list
On Fri, Jun 27, 2014 at 3:42 PM, Ajay Kumar ajaykumar.rs@samsung.com wrote:
Set FIMD output type for peach_pit as DP.
Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com
arch/arm/boot/dts/exynos5420-peach-pit.dts | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts b/arch/arm/boot/dts/exynos5420-peach-pit.dts index 1c5b8f9..164f523 100644 --- a/arch/arm/boot/dts/exynos5420-peach-pit.dts +++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts @@ -255,6 +255,7 @@ &fimd { status = "okay"; samsung,invert-vclk;
samsung,output-type = <2>;};
&dp {
1.7.9.5
Set FIMD output type for peach_pi as DP.
Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com --- arch/arm/boot/dts/exynos5800-peach-pi.dts | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts b/arch/arm/boot/dts/exynos5800-peach-pi.dts index f3af207..21e2de2 100644 --- a/arch/arm/boot/dts/exynos5800-peach-pi.dts +++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts @@ -205,6 +205,7 @@ &fimd { status = "okay"; samsung,invert-vclk; + samsung,output-type = <2>; };
&hsi2c_9 {
+DT mailing list
On Fri, Jun 27, 2014 at 3:42 PM, Ajay Kumar ajaykumar.rs@samsung.com wrote:
Set FIMD output type for peach_pi as DP.
Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com
arch/arm/boot/dts/exynos5800-peach-pi.dts | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts b/arch/arm/boot/dts/exynos5800-peach-pi.dts index f3af207..21e2de2 100644 --- a/arch/arm/boot/dts/exynos5800-peach-pi.dts +++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts @@ -205,6 +205,7 @@ &fimd { status = "okay"; samsung,invert-vclk;
samsung,output-type = <2>;};
&hsi2c_9 {
1.7.9.5
Set FIMD output type for smdk5250 as DP.
Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com --- arch/arm/boot/dts/exynos5250-smdk5250.dts | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts b/arch/arm/boot/dts/exynos5250-smdk5250.dts index a794a70..ed28efb 100644 --- a/arch/arm/boot/dts/exynos5250-smdk5250.dts +++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts @@ -382,6 +382,7 @@
fimd@14400000 { status = "okay"; + samsung,output-type = <2>; display-timings { native-mode = <&timing0>; timing0: timing@0 {
+DT mailing list
On Fri, Jun 27, 2014 at 3:42 PM, Ajay Kumar ajaykumar.rs@samsung.com wrote:
Set FIMD output type for smdk5250 as DP.
Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com
arch/arm/boot/dts/exynos5250-smdk5250.dts | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts b/arch/arm/boot/dts/exynos5250-smdk5250.dts index a794a70..ed28efb 100644 --- a/arch/arm/boot/dts/exynos5250-smdk5250.dts +++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts @@ -382,6 +382,7 @@
fimd@14400000 { status = "okay";
samsung,output-type = <2>; display-timings { native-mode = <&timing0>; timing0: timing@0 {-- 1.7.9.5
Set FIMD output type for smdk5420 as DP.
Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com --- arch/arm/boot/dts/exynos5420-smdk5420.dts | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/exynos5420-smdk5420.dts b/arch/arm/boot/dts/exynos5420-smdk5420.dts index 6052aa9..5f05a19 100644 --- a/arch/arm/boot/dts/exynos5420-smdk5420.dts +++ b/arch/arm/boot/dts/exynos5420-smdk5420.dts @@ -120,6 +120,7 @@
fimd@14400000 { status = "okay"; + samsung,output-type = <2>; display-timings { native-mode = <&timing0>; timing0: timing@0 {
+DT mailing list
On Fri, Jun 27, 2014 at 3:42 PM, Ajay Kumar ajaykumar.rs@samsung.com wrote:
Set FIMD output type for smdk5420 as DP.
Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com
arch/arm/boot/dts/exynos5420-smdk5420.dts | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/exynos5420-smdk5420.dts b/arch/arm/boot/dts/exynos5420-smdk5420.dts index 6052aa9..5f05a19 100644 --- a/arch/arm/boot/dts/exynos5420-smdk5420.dts +++ b/arch/arm/boot/dts/exynos5420-smdk5420.dts @@ -120,6 +120,7 @@
fimd@14400000 { status = "okay";
samsung,output-type = <2>; display-timings { native-mode = <&timing0>; timing0: timing@0 {-- 1.7.9.5
Set FIMD output type for arndale as DP.
Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com --- arch/arm/boot/dts/exynos5250-arndale.dts | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/exynos5250-arndale.dts b/arch/arm/boot/dts/exynos5250-arndale.dts index d0de1f5..e803183 100644 --- a/arch/arm/boot/dts/exynos5250-arndale.dts +++ b/arch/arm/boot/dts/exynos5250-arndale.dts @@ -540,6 +540,7 @@
fimd: fimd@14400000 { status = "okay"; + samsung,output-type = <2>; display-timings { native-mode = <&timing0>; timing0: timing@0 {
+DT mailing list
On Fri, Jun 27, 2014 at 3:42 PM, Ajay Kumar ajaykumar.rs@samsung.com wrote:
Set FIMD output type for arndale as DP.
Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com
arch/arm/boot/dts/exynos5250-arndale.dts | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/exynos5250-arndale.dts b/arch/arm/boot/dts/exynos5250-arndale.dts index d0de1f5..e803183 100644 --- a/arch/arm/boot/dts/exynos5250-arndale.dts +++ b/arch/arm/boot/dts/exynos5250-arndale.dts @@ -540,6 +540,7 @@
fimd: fimd@14400000 { status = "okay";
samsung,output-type = <2>; display-timings { native-mode = <&timing0>; timing0: timing@0 {-- 1.7.9.5
+ DT mailing list
On 2014년 06월 27일 19:12, Ajay Kumar wrote:
This series is based on exynos-drm-next branch of Inki Dae's tree at: git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git
Changes since V2: Add DT property to know the type of FIMD output interface as per Inki's suggestion. Add samsung,output-type DT property in FIMD node for all boards supporting DP interface.
Ajay Kumar (7): [PATCH V3 1/7] drm/exynos: Support DP CLKCON register in FIMD driver [PATCH V3 2/7] ARM: dts: Add FIMD output property for snow [PATCH V3 3/7] ARM: dts: Add FIMD output property for peach_pit [PATCH V3 4/7] ARM: dts: Add FIMD output property for peach_pi [PATCH V3 5/7] ARM: dts: Add FIMD output property for smdk5250 [PATCH V3 6/7] ARM: dts: Add FIMD output property for smdk5420 [PATCH V3 7/7] ARM: dts: Add FIMD output property for arndale
.../devicetree/bindings/video/samsung-fimd.txt | 1 + arch/arm/boot/dts/exynos5250-arndale.dts | 1 + arch/arm/boot/dts/exynos5250-smdk5250.dts | 1 + arch/arm/boot/dts/exynos5250-snow.dts | 1 + arch/arm/boot/dts/exynos5420-peach-pit.dts | 1 + arch/arm/boot/dts/exynos5420-smdk5420.dts | 1 + arch/arm/boot/dts/exynos5800-peach-pi.dts | 1 + drivers/gpu/drm/exynos/exynos_drm_fimd.c | 23 ++++++++++++++++++++ include/video/samsung_fimd.h | 4 ++++ 9 files changed, 34 insertions(+)
On Fri, Jun 27, 2014 at 11:24:58AM +0100, Inki Dae wrote:
- DT mailing list
Thanks for the Cc.
Can we get the rest of the series? Judging a series based on its diffstat alone is a little hard...
Or is my mailbox filtering hiding the rest of these from me?
Any reason for not Ccing lakml?
Mark.
On 2014년 06월 27일 19:12, Ajay Kumar wrote:
This series is based on exynos-drm-next branch of Inki Dae's tree at: git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git
Changes since V2: Add DT property to know the type of FIMD output interface as per Inki's suggestion. Add samsung,output-type DT property in FIMD node for all boards supporting DP interface.
Ajay Kumar (7): [PATCH V3 1/7] drm/exynos: Support DP CLKCON register in FIMD driver [PATCH V3 2/7] ARM: dts: Add FIMD output property for snow [PATCH V3 3/7] ARM: dts: Add FIMD output property for peach_pit [PATCH V3 4/7] ARM: dts: Add FIMD output property for peach_pi [PATCH V3 5/7] ARM: dts: Add FIMD output property for smdk5250 [PATCH V3 6/7] ARM: dts: Add FIMD output property for smdk5420 [PATCH V3 7/7] ARM: dts: Add FIMD output property for arndale
.../devicetree/bindings/video/samsung-fimd.txt | 1 + arch/arm/boot/dts/exynos5250-arndale.dts | 1 + arch/arm/boot/dts/exynos5250-smdk5250.dts | 1 + arch/arm/boot/dts/exynos5250-snow.dts | 1 + arch/arm/boot/dts/exynos5420-peach-pit.dts | 1 + arch/arm/boot/dts/exynos5420-smdk5420.dts | 1 + arch/arm/boot/dts/exynos5800-peach-pi.dts | 1 + drivers/gpu/drm/exynos/exynos_drm_fimd.c | 23 ++++++++++++++++++++ include/video/samsung_fimd.h | 4 ++++ 9 files changed, 34 insertions(+)
-- 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
dri-devel@lists.freedesktop.org
-
Ajay Kumar -
Ajay kumar -
Andrzej Hajda -
Inki Dae -
Jingoo Han -
Mark Rutland -
Tomasz Figa