This patchset includes dt support for exynos rotator. The exynos4 platform is only dt-based since 3.10, we should convert driver data and ids to dt-based parsing methods. The rotator driver has a limit table to get size limit. The minimum size of RGB888 format is 8 x 8 and maximum size is 8K x 8K. The other format, YCbCr420 2-Plane has 32 x 32 min size and 32K x 32K max size. Each format should be multiple of 'align' value.
Chanho Park (3): drm/exynos: add device tree support for rotator drm/exynos: add dt-binding documentation for rotator dts: ARM: add a rotator node for exynos4
.../bindings/drm/exynos/samsung-rotator.txt | 35 +++++++ arch/arm/boot/dts/exynos4.dtsi | 23 ++++ drivers/gpu/drm/exynos/exynos_drm_rotator.c | 110 ++++++++++++++------ 3 files changed, 138 insertions(+), 30 deletions(-) create mode 100644 Documentation/devicetree/bindings/drm/exynos/samsung-rotator.txt
The exynos4 platform is only dt-based since 3.10, we should convert driver data and ids to dt-based parsing methods. The rotator driver has a limit table to get size limit. The minimum size of RGB888 format is 8 x 8 and maximum size is 8K x 8K. The other format, YCbCr420 2-Plane has 32 x 32 min size and 32K x 32K max size. Each format should be multiple of 'align' value.
Signed-off-by: Chanho Park chanho61.park@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com --- drivers/gpu/drm/exynos/exynos_drm_rotator.c | 110 +++++++++++++++++++-------- 1 file changed, 80 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_rotator.c b/drivers/gpu/drm/exynos/exynos_drm_rotator.c index 427640a..b353a10 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_rotator.c +++ b/drivers/gpu/drm/exynos/exynos_drm_rotator.c @@ -96,7 +96,7 @@ struct rot_context { struct resource *regs_res; void __iomem *regs; struct clk *clock; - struct rot_limit_table *limit_tbl; + struct rot_limit_table limit_tbl; int irq; int cur_buf_id[EXYNOS_DRM_OPS_MAX]; bool suspended; @@ -167,7 +167,7 @@ static irqreturn_t rotator_irq_handler(int irq, void *arg) static void rotator_align_size(struct rot_context *rot, u32 fmt, u32 *hsize, u32 *vsize) { - struct rot_limit_table *limit_tbl = rot->limit_tbl; + struct rot_limit_table *limit_tbl = &rot->limit_tbl; struct rot_limit *limit; u32 mask, val;
@@ -632,6 +632,72 @@ static int rotator_ippdrv_start(struct device *dev, enum drm_exynos_ipp_cmd cmd) return 0; }
+static const struct of_device_id exynos_rotator_match[] = { + { + .compatible = "samsung,exynos4210-rotator", + }, + {}, +}; +MODULE_DEVICE_TABLE(of, exynos_rotator_match); + +static int rotator_parse_dt_tbl(struct device_node *np, struct rot_limit *rlim) +{ + int ret; + + ret = of_property_read_u32(np, "min_w", &rlim->min_w); + if (ret) + return ret; + + ret = of_property_read_u32(np, "min_h", &rlim->min_h); + if (ret) + return ret; + + ret = of_property_read_u32(np, "max_w", &rlim->max_w); + if (ret) + return ret; + + ret = of_property_read_u32(np, "max_h", &rlim->max_h); + if (ret) + return ret; + + ret = of_property_read_u32(np, "align", &rlim->align); + if (ret) + return ret; + + return 0; +} + +static int rotator_parse_dt(struct device *dev, struct rot_context *rot) +{ + struct device_node *ycbcr_node, *rgb888_node; + int ret; + + ycbcr_node = of_get_child_by_name(dev->of_node, "ycbcr420_2p"); + if (!ycbcr_node) { + dev_err(dev, "can't find ycbcr420_2p node\n"); + return -ENODEV; + } + + rgb888_node = of_get_child_by_name(dev->of_node, "rgb888"); + if (!rgb888_node) { + dev_err(dev, "can't find rgb888 node\n"); + return -ENODEV; + } + + ret = rotator_parse_dt_tbl(ycbcr_node, &rot->limit_tbl.ycbcr420_2p); + if (ret) { + dev_err(dev, "failed to parse ycbcr420 data\n"); + return ret; + } + ret = rotator_parse_dt_tbl(rgb888_node, &rot->limit_tbl.rgb888); + if (ret) { + dev_err(dev, "failed to parse rgb888 data\n"); + return ret; + } + + return 0; +} + static int rotator_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; @@ -639,14 +705,23 @@ static int rotator_probe(struct platform_device *pdev) struct exynos_drm_ippdrv *ippdrv; int ret;
+ if (!dev->of_node) { + dev_err(dev, "Cannot find device tree node\n"); + return -ENODEV; + } + rot = devm_kzalloc(dev, sizeof(*rot), GFP_KERNEL); if (!rot) { dev_err(dev, "failed to allocate rot\n"); return -ENOMEM; }
- rot->limit_tbl = (struct rot_limit_table *) - platform_get_device_id(pdev)->driver_data; + ret = rotator_parse_dt(dev, rot); + if (ret) { + dev_err(dev, "failed parse dt info\n"); + devm_kfree(dev, rot); + return ret; + }
rot->regs_res = platform_get_resource(pdev, IORESOURCE_MEM, 0); rot->regs = devm_ioremap_resource(dev, rot->regs_res); @@ -718,31 +793,6 @@ static int rotator_remove(struct platform_device *pdev) return 0; }
-static struct rot_limit_table rot_limit_tbl = { - .ycbcr420_2p = { - .min_w = 32, - .min_h = 32, - .max_w = SZ_32K, - .max_h = SZ_32K, - .align = 3, - }, - .rgb888 = { - .min_w = 8, - .min_h = 8, - .max_w = SZ_8K, - .max_h = SZ_8K, - .align = 2, - }, -}; - -static struct platform_device_id rotator_driver_ids[] = { - { - .name = "exynos-rot", - .driver_data = (unsigned long)&rot_limit_tbl, - }, - {}, -}; - static int rotator_clk_crtl(struct rot_context *rot, bool enable) { if (enable) { @@ -804,10 +854,10 @@ static const struct dev_pm_ops rotator_pm_ops = { struct platform_driver rotator_driver = { .probe = rotator_probe, .remove = rotator_remove, - .id_table = rotator_driver_ids, .driver = { .name = "exynos-rot", .owner = THIS_MODULE, .pm = &rotator_pm_ops, + .of_match_table = of_match_ptr(exynos_rotator_match), }, };
-----Original Message----- From: linux-samsung-soc-owner@vger.kernel.org [mailto:linux-samsung-soc- owner@vger.kernel.org] On Behalf Of Chanho Park Sent: Monday, July 22, 2013 3:49 PM To: inki.dae@samsung.com; kgene.kim@samsung.com Cc: jy0922.shim@samsung.com; sw0312.kim@samsung.com; kyungmin.park@samsung.com; dri-devel@lists.freedesktop.org; linux-samsung- soc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; devicetree- discuss@lists.ozlabs.org; Chanho Park Subject: [PATCH 1/3] drm/exynos: add device tree support for rotator
The exynos4 platform is only dt-based since 3.10, we should convert driver data and ids to dt-based parsing methods. The rotator driver has a limit table to get size limit. The minimum size of RGB888 format is 8 x 8 and maximum size is 8K x 8K. The other format, YCbCr420 2-Plane has 32 x 32 min size and 32K x 32K max size. Each format should be multiple of 'align' value.
Signed-off-by: Chanho Park chanho61.park@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com
drivers/gpu/drm/exynos/exynos_drm_rotator.c | 110 +++++++++++++++++++---
1 file changed, 80 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_rotator.c b/drivers/gpu/drm/exynos/exynos_drm_rotator.c index 427640a..b353a10 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_rotator.c +++ b/drivers/gpu/drm/exynos/exynos_drm_rotator.c @@ -96,7 +96,7 @@ struct rot_context { struct resource *regs_res; void __iomem *regs; struct clk *clock;
- struct rot_limit_table *limit_tbl;
- struct rot_limit_table limit_tbl; int irq; int cur_buf_id[EXYNOS_DRM_OPS_MAX]; bool suspended;
@@ -167,7 +167,7 @@ static irqreturn_t rotator_irq_handler(int irq, void *arg) static void rotator_align_size(struct rot_context *rot, u32 fmt, u32 *hsize, u32 *vsize) {
- struct rot_limit_table *limit_tbl = rot->limit_tbl;
- struct rot_limit_table *limit_tbl = &rot->limit_tbl; struct rot_limit *limit; u32 mask, val;
@@ -632,6 +632,72 @@ static int rotator_ippdrv_start(struct device *dev, enum drm_exynos_ipp_cmd cmd) return 0; }
+static const struct of_device_id exynos_rotator_match[] = {
- {
.compatible = "samsung,exynos4210-rotator",- },
- {},
+}; +MODULE_DEVICE_TABLE(of, exynos_rotator_match);
Rotator device cannot be plugged in. Please remove the above MODULE_DEVICE_TABLE.
+static int rotator_parse_dt_tbl(struct device_node *np, struct rot_limit *rlim) +{
- int ret;
- ret = of_property_read_u32(np, "min_w", &rlim->min_w);
- if (ret)
return ret;- ret = of_property_read_u32(np, "min_h", &rlim->min_h);
- if (ret)
return ret;- ret = of_property_read_u32(np, "max_w", &rlim->max_w);
- if (ret)
return ret;- ret = of_property_read_u32(np, "max_h", &rlim->max_h);
- if (ret)
return ret;- ret = of_property_read_u32(np, "align", &rlim->align);
- if (ret)
return ret;- return 0;
+}
+static int rotator_parse_dt(struct device *dev, struct rot_context *rot) +{
- struct device_node *ycbcr_node, *rgb888_node;
- int ret;
- ycbcr_node = of_get_child_by_name(dev->of_node, "ycbcr420_2p");
- if (!ycbcr_node) {
dev_err(dev, "can't find ycbcr420_2p node\n");return -ENODEV;- }
- rgb888_node = of_get_child_by_name(dev->of_node, "rgb888");
- if (!rgb888_node) {
dev_err(dev, "can't find rgb888 node\n");return -ENODEV;- }
- ret = rotator_parse_dt_tbl(ycbcr_node, &rot->limit_tbl.ycbcr420_2p);
- if (ret) {
dev_err(dev, "failed to parse ycbcr420 data\n");return ret;- }
- ret = rotator_parse_dt_tbl(rgb888_node, &rot->limit_tbl.rgb888);
- if (ret) {
dev_err(dev, "failed to parse rgb888 data\n");return ret;- }
- return 0;
+}
static int rotator_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; @@ -639,14 +705,23 @@ static int rotator_probe(struct platform_device *pdev) struct exynos_drm_ippdrv *ippdrv; int ret;
- if (!dev->of_node) {
dev_err(dev, "Cannot find device tree node\n");return -ENODEV;- }
- rot = devm_kzalloc(dev, sizeof(*rot), GFP_KERNEL); if (!rot) { dev_err(dev, "failed to allocate rot\n"); return -ENOMEM; }
- rot->limit_tbl = (struct rot_limit_table *)
platform_get_device_id(pdev)->driver_data;
ret = rotator_parse_dt(dev, rot);
if (ret) {
dev_err(dev, "failed parse dt info\n");devm_kfree(dev, rot);return ret;}
rot->regs_res = platform_get_resource(pdev, IORESOURCE_MEM, 0); rot->regs = devm_ioremap_resource(dev, rot->regs_res);
@@ -718,31 +793,6 @@ static int rotator_remove(struct platform_device *pdev) return 0; }
-static struct rot_limit_table rot_limit_tbl = {
- .ycbcr420_2p = {
.min_w = 32,.min_h = 32,.max_w = SZ_32K,.max_h = SZ_32K,.align = 3,- },
- .rgb888 = {
.min_w = 8,.min_h = 8,.max_w = SZ_8K,.max_h = SZ_8K,.align = 2,- },
-};
-static struct platform_device_id rotator_driver_ids[] = {
- {
.name = "exynos-rot",.driver_data = (unsigned long)&rot_limit_tbl,- },
- {},
-};
static int rotator_clk_crtl(struct rot_context *rot, bool enable) { if (enable) { @@ -804,10 +854,10 @@ static const struct dev_pm_ops rotator_pm_ops = { struct platform_driver rotator_driver = { .probe = rotator_probe, .remove = rotator_remove,
- .id_table = rotator_driver_ids, .driver = { .name = "exynos-rot", .owner = THIS_MODULE, .pm = &rotator_pm_ops,
.of_match_table = of_match_ptr(exynos_rotator_match),};
1.7.9.5
-- 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
On Monday 22 of July 2013 15:49:25 Chanho Park wrote:
The exynos4 platform is only dt-based since 3.10, we should convert driver data and ids to dt-based parsing methods. The rotator driver has a limit table to get size limit. The minimum size of RGB888 format is 8 x 8 and maximum size is 8K x 8K. The other format, YCbCr420 2-Plane has 32 x 32 min size and 32K x 32K max size. Each format should be multiple of 'align' value.
Signed-off-by: Chanho Park chanho61.park@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com
drivers/gpu/drm/exynos/exynos_drm_rotator.c | 110 +++++++++++++++++++-------- 1 file changed, 80 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_rotator.c b/drivers/gpu/drm/exynos/exynos_drm_rotator.c index 427640a..b353a10 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_rotator.c +++ b/drivers/gpu/drm/exynos/exynos_drm_rotator.c @@ -96,7 +96,7 @@ struct rot_context { struct resource *regs_res; void __iomem *regs; struct clk *clock;
- struct rot_limit_table *limit_tbl;
- struct rot_limit_table limit_tbl; int irq; int cur_buf_id[EXYNOS_DRM_OPS_MAX]; bool suspended;
@@ -167,7 +167,7 @@ static irqreturn_t rotator_irq_handler(int irq, void *arg) static void rotator_align_size(struct rot_context *rot, u32 fmt, u32 *hsize, u32 *vsize) {
- struct rot_limit_table *limit_tbl = rot->limit_tbl;
- struct rot_limit_table *limit_tbl = &rot->limit_tbl; struct rot_limit *limit; u32 mask, val;
@@ -632,6 +632,72 @@ static int rotator_ippdrv_start(struct device *dev, enum drm_exynos_ipp_cmd cmd) return 0; }
+static const struct of_device_id exynos_rotator_match[] = {
- {
.compatible = "samsung,exynos4210-rotator",- },
- {},
+}; +MODULE_DEVICE_TABLE(of, exynos_rotator_match);
+static int rotator_parse_dt_tbl(struct device_node *np, struct rot_limit *rlim) +{
- int ret;
- ret = of_property_read_u32(np, "min_w", &rlim->min_w);
- if (ret)
return ret;- ret = of_property_read_u32(np, "min_h", &rlim->min_h);
- if (ret)
return ret;- ret = of_property_read_u32(np, "max_w", &rlim->max_w);
- if (ret)
return ret;- ret = of_property_read_u32(np, "max_h", &rlim->max_h);
- if (ret)
return ret;- ret = of_property_read_u32(np, "align", &rlim->align);
- if (ret)
return ret;- return 0;
+}
+static int rotator_parse_dt(struct device *dev, struct rot_context *rot) +{
- struct device_node *ycbcr_node, *rgb888_node;
- int ret;
- ycbcr_node = of_get_child_by_name(dev->of_node, "ycbcr420_2p");
- if (!ycbcr_node) {
dev_err(dev, "can't find ycbcr420_2p node\n");return -ENODEV;- }
- rgb888_node = of_get_child_by_name(dev->of_node, "rgb888");
- if (!rgb888_node) {
dev_err(dev, "can't find rgb888 node\n");return -ENODEV;- }
- ret = rotator_parse_dt_tbl(ycbcr_node, &rot-
limit_tbl.ycbcr420_2p);
- if (ret) {
dev_err(dev, "failed to parse ycbcr420 data\n");return ret;- }
- ret = rotator_parse_dt_tbl(rgb888_node, &rot->limit_tbl.rgb888);
- if (ret) {
dev_err(dev, "failed to parse rgb888 data\n");return ret;- }
- return 0;
+}
static int rotator_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; @@ -639,14 +705,23 @@ static int rotator_probe(struct platform_device *pdev) struct exynos_drm_ippdrv *ippdrv; int ret;
- if (!dev->of_node) {
dev_err(dev, "Cannot find device tree node\n");return -ENODEV;- }
- rot = devm_kzalloc(dev, sizeof(*rot), GFP_KERNEL); if (!rot) { dev_err(dev, "failed to allocate rot\n"); return -ENOMEM; }
- rot->limit_tbl = (struct rot_limit_table *)
platform_get_device_id(pdev)->driver_data;
ret = rotator_parse_dt(dev, rot);
if (ret) {
dev_err(dev, "failed parse dt info\n");devm_kfree(dev, rot);return ret;}
rot->regs_res = platform_get_resource(pdev, IORESOURCE_MEM, 0); rot->regs = devm_ioremap_resource(dev, rot->regs_res);
@@ -718,31 +793,6 @@ static int rotator_remove(struct platform_device *pdev) return 0; }
-static struct rot_limit_table rot_limit_tbl = {
- .ycbcr420_2p = {
.min_w = 32,.min_h = 32,.max_w = SZ_32K,.max_h = SZ_32K,.align = 3,- },
- .rgb888 = {
.min_w = 8,.min_h = 8,.max_w = SZ_8K,.max_h = SZ_8K,.align = 2,- },
-};
-static struct platform_device_id rotator_driver_ids[] = {
- {
.name = "exynos-rot",.driver_data = (unsigned long)&rot_limit_tbl,- },
- {},
-};
I would keep these driver data here and just point to them from match_data of OF match table, as suggested in other replies.
Best regards, Tomasz
This patch adds a dt-binding document for exynos rotator. It describes which nodes should be defined to use the rotator.
Signed-off-by: Chanho Park chanho61.park@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com --- .../bindings/drm/exynos/samsung-rotator.txt | 35 ++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 Documentation/devicetree/bindings/drm/exynos/samsung-rotator.txt
diff --git a/Documentation/devicetree/bindings/drm/exynos/samsung-rotator.txt b/Documentation/devicetree/bindings/drm/exynos/samsung-rotator.txt new file mode 100644 index 0000000..6b1d704 --- /dev/null +++ b/Documentation/devicetree/bindings/drm/exynos/samsung-rotator.txt @@ -0,0 +1,35 @@ +* Samsung Image Rotator + +Required properties: + - compatible : value should be the "samsung,exynos4210". + - reg : Physical base address of the IP registers and length of memory + mapped region. + - interrupts : interrupt number to the CPU. + - clocks : clock number of exynos4 rotator clock. + - clocks : clock name of rotator + - status : "okay" or "disabled" + - limit table for image formats : min_w/min_h/max_w/max_h for min/max of image + +Example: + rotator: rotator@12810000 { + compatible = "samsung,exynos4210-rotator"; + reg = <0x12810000 0x1000>; + interrupts = <0 83 0>; + clocks = <&clock 278>; + clock-names = "rotator"; + status = "disabled"; + ycbcr420_2p { + min_w = <32>; + min_h = <32>; + max_w = <32768>; + max_h = <32768>; + align = <3>; + }; + rgb888 { + min_w = <8>; + min_h = <8>; + max_w = <8192>; + max_h = <8192>; + align = <2>; + }; + };
On Mon, Jul 22, 2013 at 07:49:26AM +0100, Chanho Park wrote:
This patch adds a dt-binding document for exynos rotator. It describes which nodes should be defined to use the rotator.
Signed-off-by: Chanho Park chanho61.park@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com
.../bindings/drm/exynos/samsung-rotator.txt | 35 ++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 Documentation/devicetree/bindings/drm/exynos/samsung-rotator.txt
diff --git a/Documentation/devicetree/bindings/drm/exynos/samsung-rotator.txt b/Documentation/devicetree/bindings/drm/exynos/samsung-rotator.txt new file mode 100644 index 0000000..6b1d704 --- /dev/null +++ b/Documentation/devicetree/bindings/drm/exynos/samsung-rotator.txt @@ -0,0 +1,35 @@ +* Samsung Image Rotator
+Required properties:
- compatible : value should be the "samsung,exynos4210".
- reg : Physical base address of the IP registers and length of memory
mapped region.
- interrupts : interrupt number to the CPU.
- clocks : clock number of exynos4 rotator clock.
- clocks : clock name of rotator
clock-names?
- status : "okay" or "disabled"
- limit table for image formats : min_w/min_h/max_w/max_h for min/max of image
Limit table? This doesn't seem to be a well-defined binding, and it seems like a relatively generic thing to describe.
+Example:
- rotator: rotator@12810000 {
compatible = "samsung,exynos4210-rotator";reg = <0x12810000 0x1000>;interrupts = <0 83 0>;clocks = <&clock 278>;clock-names = "rotator";status = "disabled";ycbcr420_2p {
Which names are allowed for these subnodes?
min_w = <32>;min_h = <32>;max_w = <32768>;max_h = <32768>;align = <3>;
min-width, min-height, max-width, max-height? What units are they in?
What does alignment specify exactly?
Are these a configurable part of the rotator hardware, or are these values always the same? If thery're always the same, there's no need to describe in in the devicetree.
Thanks, Mark.
};rgb888 {min_w = <8>;min_h = <8>;max_w = <8192>;max_h = <8192>;align = <2>;};- };
-- 1.7.9.5
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
-----Original Message----- From: Mark Rutland [mailto:mark.rutland@arm.com] Sent: Monday, July 22, 2013 5:48 PM To: Chanho Park Cc: inki.dae@samsung.com; kgene.kim@samsung.com; linux-samsung- soc@vger.kernel.org; jy0922.shim@samsung.com; devicetree- discuss@lists.ozlabs.org; sw0312.kim@samsung.com; dri- devel@lists.freedesktop.org; kyungmin.park@samsung.com; linux-arm- kernel@lists.infradead.org Subject: Re: [PATCH 2/3] drm/exynos: add dt-binding documentation for rotator
On Mon, Jul 22, 2013 at 07:49:26AM +0100, Chanho Park wrote:
This patch adds a dt-binding document for exynos rotator. It describes
which
nodes should be defined to use the rotator.
Signed-off-by: Chanho Park chanho61.park@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com
.../bindings/drm/exynos/samsung-rotator.txt | 35
++++++++++++++++++++
1 file changed, 35 insertions(+) create mode 100644
Documentation/devicetree/bindings/drm/exynos/samsung-rotator.txt
diff --git a/Documentation/devicetree/bindings/drm/exynos/samsung-
rotator.txt b/Documentation/devicetree/bindings/drm/exynos/samsung- rotator.txt
new file mode 100644 index 0000000..6b1d704 --- /dev/null +++ b/Documentation/devicetree/bindings/drm/exynos/samsung-rotator.txt @@ -0,0 +1,35 @@ +* Samsung Image Rotator
+Required properties:
- compatible : value should be the "samsung,exynos4210".
Please, add more compatible strings for other SoC.
- reg : Physical base address of the IP registers and length of
memory
mapped region.
- interrupts : interrupt number to the CPU.
- clocks : clock number of exynos4 rotator clock.
- clocks : clock name of rotator
clock-names?
- status : "okay" or "disabled"
- limit table for image formats : min_w/min_h/max_w/max_h for min/max
of image
Limit table? This doesn't seem to be a well-defined binding, and it seems like a relatively generic thing to describe.
+Example:
- rotator: rotator@12810000 {
compatible = "samsung,exynos4210-rotator";reg = <0x12810000 0x1000>;interrupts = <0 83 0>;clocks = <&clock 278>;clock-names = "rotator";status = "disabled";ycbcr420_2p {Which names are allowed for these subnodes?
min_w = <32>;min_h = <32>;max_w = <32768>;max_h = <32768>;align = <3>;min-width, min-height, max-width, max-height? What units are they in?
What does alignment specify exactly?
Are these a configurable part of the rotator hardware, or are these values always the same?
Right, that seems like configurable part. At least, min_w/h and max_w/h can be different values according to SoC and pixel formats so they should be described enough in this dt-binding document file.
Thanks, Inki Dae
If thery're always the same, there's no need to describe in in the devicetree.
Thanks, Mark.
};rgb888 {min_w = <8>;min_h = <8>;max_w = <8192>;max_h = <8192>;align = <2>;};- };
-- 1.7.9.5
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Am Montag, den 22.07.2013, 21:37 +0900 schrieb Inki Dae:
-----Original Message----- From: Mark Rutland [mailto:mark.rutland@arm.com] Sent: Monday, July 22, 2013 5:48 PM To: Chanho Park Cc: inki.dae@samsung.com; kgene.kim@samsung.com; linux-samsung- soc@vger.kernel.org; jy0922.shim@samsung.com; devicetree- discuss@lists.ozlabs.org; sw0312.kim@samsung.com; dri- devel@lists.freedesktop.org; kyungmin.park@samsung.com; linux-arm- kernel@lists.infradead.org Subject: Re: [PATCH 2/3] drm/exynos: add dt-binding documentation for rotator
On Mon, Jul 22, 2013 at 07:49:26AM +0100, Chanho Park wrote:
This patch adds a dt-binding document for exynos rotator. It describes
which
nodes should be defined to use the rotator.
Signed-off-by: Chanho Park chanho61.park@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com
.../bindings/drm/exynos/samsung-rotator.txt | 35
++++++++++++++++++++
1 file changed, 35 insertions(+) create mode 100644
Documentation/devicetree/bindings/drm/exynos/samsung-rotator.txt
diff --git a/Documentation/devicetree/bindings/drm/exynos/samsung-
rotator.txt b/Documentation/devicetree/bindings/drm/exynos/samsung- rotator.txt
new file mode 100644 index 0000000..6b1d704 --- /dev/null +++ b/Documentation/devicetree/bindings/drm/exynos/samsung-rotator.txt @@ -0,0 +1,35 @@ +* Samsung Image Rotator
+Required properties:
- compatible : value should be the "samsung,exynos4210".
Please, add more compatible strings for other SoC.
- reg : Physical base address of the IP registers and length of
memory
mapped region.
- interrupts : interrupt number to the CPU.
- clocks : clock number of exynos4 rotator clock.
- clocks : clock name of rotator
clock-names?
- status : "okay" or "disabled"
- limit table for image formats : min_w/min_h/max_w/max_h for min/max
of image
Limit table? This doesn't seem to be a well-defined binding, and it seems like a relatively generic thing to describe.
+Example:
- rotator: rotator@12810000 {
compatible = "samsung,exynos4210-rotator";reg = <0x12810000 0x1000>;interrupts = <0 83 0>;clocks = <&clock 278>;clock-names = "rotator";status = "disabled";ycbcr420_2p {Which names are allowed for these subnodes?
min_w = <32>;min_h = <32>;max_w = <32768>;max_h = <32768>;align = <3>;min-width, min-height, max-width, max-height? What units are they in?
What does alignment specify exactly?
Are these a configurable part of the rotator hardware, or are these values always the same?
Right, that seems like configurable part. At least, min_w/h and max_w/h can be different values according to SoC and pixel formats so they should be described enough in this dt-binding document file.
Is there really this much configurability? Could each of those values be a different on different SoCs, or could you just extract those values from the SoC/rotator hardware version and thus the DT compatible string?
If thery're always the same, there's no need to describe in in the devicetree.
Thanks, Mark.
};rgb888 {min_w = <8>;min_h = <8>;max_w = <8192>;max_h = <8192>;align = <2>;};- };
-- 1.7.9.5
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
-----Original Message----- From: linux-samsung-soc-owner@vger.kernel.org [mailto:linux-samsung-soc- owner@vger.kernel.org] On Behalf Of Lucas Stach Sent: Monday, July 22, 2013 9:47 PM To: Inki Dae Cc: 'Mark Rutland'; 'Chanho Park'; linux-samsung-soc@vger.kernel.org; devicetree-discuss@lists.ozlabs.org; sw0312.kim@samsung.com; dri- devel@lists.freedesktop.org; kyungmin.park@samsung.com; kgene.kim@samsung.com; linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 2/3] drm/exynos: add dt-binding documentation for rotator
Am Montag, den 22.07.2013, 21:37 +0900 schrieb Inki Dae:
-----Original Message----- From: Mark Rutland [mailto:mark.rutland@arm.com] Sent: Monday, July 22, 2013 5:48 PM To: Chanho Park Cc: inki.dae@samsung.com; kgene.kim@samsung.com; linux-samsung- soc@vger.kernel.org; jy0922.shim@samsung.com; devicetree- discuss@lists.ozlabs.org; sw0312.kim@samsung.com; dri- devel@lists.freedesktop.org; kyungmin.park@samsung.com; linux-arm- kernel@lists.infradead.org Subject: Re: [PATCH 2/3] drm/exynos: add dt-binding documentation for rotator
On Mon, Jul 22, 2013 at 07:49:26AM +0100, Chanho Park wrote:
This patch adds a dt-binding document for exynos rotator. It
describes
which
nodes should be defined to use the rotator.
Signed-off-by: Chanho Park chanho61.park@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com
.../bindings/drm/exynos/samsung-rotator.txt | 35
++++++++++++++++++++
1 file changed, 35 insertions(+) create mode 100644
Documentation/devicetree/bindings/drm/exynos/samsung-rotator.txt
diff --git a/Documentation/devicetree/bindings/drm/exynos/samsung-
rotator.txt b/Documentation/devicetree/bindings/drm/exynos/samsung- rotator.txt
new file mode 100644 index 0000000..6b1d704 --- /dev/null +++ b/Documentation/devicetree/bindings/drm/exynos/samsung-
rotator.txt
@@ -0,0 +1,35 @@ +* Samsung Image Rotator
+Required properties:
- compatible : value should be the "samsung,exynos4210".
Please, add more compatible strings for other SoC.
- reg : Physical base address of the IP registers and length of
memory
mapped region.
- interrupts : interrupt number to the CPU.
- clocks : clock number of exynos4 rotator clock.
- clocks : clock name of rotator
clock-names?
- status : "okay" or "disabled"
- limit table for image formats : min_w/min_h/max_w/max_h for
min/max
of image
Limit table? This doesn't seem to be a well-defined binding, and it seems like a relatively generic thing to describe.
+Example:
- rotator: rotator@12810000 {
compatible = "samsung,exynos4210-rotator";reg = <0x12810000 0x1000>;interrupts = <0 83 0>;clocks = <&clock 278>;clock-names = "rotator";status = "disabled";ycbcr420_2p {Which names are allowed for these subnodes?
min_w = <32>;min_h = <32>;max_w = <32768>;max_h = <32768>;align = <3>;min-width, min-height, max-width, max-height? What units are they in?
What does alignment specify exactly?
Are these a configurable part of the rotator hardware, or are these values always the same?
Right, that seems like configurable part. At least, min_w/h and max_w/h
can
be different values according to SoC and pixel formats so they should be described enough in this dt-binding document file.
Is there really this much configurability? Could each of those values be a different on different SoCs
Not much but Yes. Rotator devices of Exynos SoC support various pixel formats; RGB888, RGB565, YUV422, YUV420-2Plane, and YUV420-3Plane, and each of these pixel formats could be different limitation to minimum and maximum sizes. For example, the below shows the limitation to Rotator device of Exynos4412 SoC according to pixel formats, Image format Minimum size Maximum size RGB888 8x8 8kx8k RGB565 16x16 16kx16k YUV422 16x16 16kx16k YUV420 2-Plane 32x32 32kx32k(in case of Y components) YUV420 3-Plane 64x32 32kx32k(in case of Y components)
And, I guess those limitations are slightly different according to Exynos SoC versions as long as I know.
Thanks, Inki Dae
, or could you just extract those values from the SoC/rotator hardware version and thus the DT compatible string?
If thery're always the same, there's no need to describe in in the devicetree.
Thanks, Mark.
};rgb888 {min_w = <8>;min_h = <8>;max_w = <8192>;max_h = <8192>;align = <2>;};- };
-- 1.7.9.5
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
-- Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
-- 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
On 07/22/2013 03:31 PM, Inki Dae wrote:
---Original Message-----
From: linux-samsung-soc-owner@vger.kernel.org [mailto:linux-samsung-soc- owner@vger.kernel.org] On Behalf Of Lucas Stach Sent: Monday, July 22, 2013 9:47 PM To: Inki Dae Cc: 'Mark Rutland'; 'Chanho Park'; linux-samsung-soc@vger.kernel.org; devicetree-discuss@lists.ozlabs.org; sw0312.kim@samsung.com; dri- devel@lists.freedesktop.org; kyungmin.park@samsung.com; kgene.kim@samsung.com; linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 2/3] drm/exynos: add dt-binding documentation for rotator
Am Montag, den 22.07.2013, 21:37 +0900 schrieb Inki Dae:
> -----Original Message----- > From: Mark Rutland [mailto:mark.rutland@arm.com] > Sent: Monday, July 22, 2013 5:48 PM > To: Chanho Park > Cc: inki.dae@samsung.com; kgene.kim@samsung.com; linux-samsung- > soc@vger.kernel.org; jy0922.shim@samsung.com; devicetree- > discuss@lists.ozlabs.org; sw0312.kim@samsung.com; dri- > devel@lists.freedesktop.org; kyungmin.park@samsung.com; linux-arm- > kernel@lists.infradead.org > Subject: Re: [PATCH 2/3] drm/exynos: add dt-binding documentation for > rotator > > On Mon, Jul 22, 2013 at 07:49:26AM +0100, Chanho Park wrote: > > > This patch adds a dt-binding document for exynos rotator. It
describes
> which > > > nodes should be defined to use the rotator. > > > > > > Signed-off-by: Chanho Park chanho61.park@samsung.com > > > Signed-off-by: Kyungmin Park kyungmin.park@samsung.com > > > --- > > > .../bindings/drm/exynos/samsung-rotator.txt | 35 > ++++++++++++++++++++ > > > 1 file changed, 35 insertions(+) > > > create mode 100644 > Documentation/devicetree/bindings/drm/exynos/samsung-rotator.txt > > > > > > diff --git a/Documentation/devicetree/bindings/drm/exynos/samsung- > rotator.txt b/Documentation/devicetree/bindings/drm/exynos/samsung- > rotator.txt > > > new file mode 100644 > > > index 0000000..6b1d704 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/drm/exynos/samsung-
rotator.txt
> > > @@ -0,0 +1,35 @@ > > > +* Samsung Image Rotator > > > + > > > +Required properties: > > > + - compatible : value should be the "samsung,exynos4210".
Please, add more compatible strings for other SoC.
> > > + - reg : Physical base address of the IP registers and length of > memory > > > + mapped region. > > > + - interrupts : interrupt number to the CPU. > > > + - clocks : clock number of exynos4 rotator clock. > > > + - clocks : clock name of rotator > > clock-names? > > > > + - status : "okay" or "disabled" > > > + - limit table for image formats : min_w/min_h/max_w/max_h for
min/max
> of image > > Limit table? This doesn't seem to be a well-defined binding, and it > seems like a relatively generic thing to describe. > > > > + > > > +Example: > > > + rotator: rotator@12810000 { > > > + compatible = "samsung,exynos4210-rotator"; > > > + reg = <0x12810000 0x1000>; > > > + interrupts = <0 83 0>; > > > + clocks = <&clock 278>; > > > + clock-names = "rotator"; > > > + status = "disabled"; > > > + ycbcr420_2p { > > Which names are allowed for these subnodes? > > > > + min_w = <32>; > > > + min_h = <32>; > > > + max_w = <32768>; > > > + max_h = <32768>; > > > + align = <3>; > > min-width, min-height, max-width, max-height? What units are they in? > > What does alignment specify exactly? > > Are these a configurable part of the rotator hardware, or are these > values always the same?
Right, that seems like configurable part. At least, min_w/h and max_w/h
can
be different values according to SoC and pixel formats so they should be described enough in this dt-binding document file.
Is there really this much configurability? Could each of those values be a different on different SoCs
Not much but Yes. Rotator devices of Exynos SoC support various pixel formats; RGB888, RGB565, YUV422, YUV420-2Plane, and YUV420-3Plane, and each of these pixel formats could be different limitation to minimum and maximum sizes. For example, the below shows the limitation to Rotator device of Exynos4412 SoC according to pixel formats, Image format Minimum size Maximum size RGB888 8x8 8kx8k RGB565 16x16 16kx16k YUV422 16x16 16kx16k YUV420 2-Plane 32x32 32kx32k(in case of Y components) YUV420 3-Plane 64x32 32kx32k(in case of Y components)
And, I guess those limitations are slightly different according to Exynos SoC versions as long as I know.
Thanks, Inki Dae
, or could you just extract those values from the SoC/rotator hardware version and thus the DT compatible string?
My gut feeling is that those constraints could be well defined in the driver as static data and selected by the compatible property. Defining this in Device Tree may easily get out of control, when the limits become dependant on more parameters.
Whether devices should be described in much detail in DT rather than using multiple compatible strings had been discussed multiple times, for instance please see thread [1].
Regards, Sylwester
[1] http://www.mail-archive.com/devicetree-discuss@lists.ozlabs.org/msg36691.htm...
On Monday 22 of July 2013 16:00:22 Sylwester Nawrocki wrote:
On 07/22/2013 03:31 PM, Inki Dae wrote:
---Original Message-----
From: linux-samsung-soc-owner@vger.kernel.org [mailto:linux-samsung-soc- owner@vger.kernel.org] On Behalf Of Lucas Stach Sent: Monday, July 22, 2013 9:47 PM To: Inki Dae Cc: 'Mark Rutland'; 'Chanho Park'; linux-samsung-soc@vger.kernel.org; devicetree-discuss@lists.ozlabs.org; sw0312.kim@samsung.com; dri- devel@lists.freedesktop.org; kyungmin.park@samsung.com; kgene.kim@samsung.com; linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 2/3] drm/exynos: add dt-binding documentation for rotator
Am Montag, den 22.07.2013, 21:37 +0900 schrieb Inki Dae:
> > -----Original Message----- > > From: Mark Rutland [mailto:mark.rutland@arm.com] > > Sent: Monday, July 22, 2013 5:48 PM > > To: Chanho Park > > Cc: inki.dae@samsung.com; kgene.kim@samsung.com; > > linux-samsung- > > soc@vger.kernel.org; jy0922.shim@samsung.com; devicetree- > > discuss@lists.ozlabs.org; sw0312.kim@samsung.com; dri- > > devel@lists.freedesktop.org; kyungmin.park@samsung.com; > > linux-arm- > > kernel@lists.infradead.org > > Subject: Re: [PATCH 2/3] drm/exynos: add dt-binding > > documentation for > > rotator > > > > On Mon, Jul 22, 2013 at 07:49:26AM +0100, Chanho Park wrote: > > > > This patch adds a dt-binding document for exynos rotator. > > > > It
describes
> > which > > > > > > nodes should be defined to use the rotator. > > > > > > > > Signed-off-by: Chanho Park chanho61.park@samsung.com > > > > Signed-off-by: Kyungmin Park kyungmin.park@samsung.com > > > > --- > > > > > > > > .../bindings/drm/exynos/samsung-rotator.txt | 35 > > > > ++++++++++++++++++++ > > > > > > 1 file changed, 35 insertions(+) > > > > create mode 100644 > > > > Documentation/devicetree/bindings/drm/exynos/samsung-rotator. > > txt > > > > > > diff --git > > > > a/Documentation/devicetree/bindings/drm/exynos/samsung- > > > > rotator.txt > > b/Documentation/devicetree/bindings/drm/exynos/samsung- > > rotator.txt > > > > > > new file mode 100644 > > > > index 0000000..6b1d704 > > > > --- /dev/null > > > > +++ > > > > b/Documentation/devicetree/bindings/drm/exynos/samsung-
rotator.txt
> > > > @@ -0,0 +1,35 @@ > > > > +* Samsung Image Rotator > > > > + > > > > +Required properties: > > > > + - compatible : value should be the > > > > "samsung,exynos4210".
Please, add more compatible strings for other SoC.
> > > > + - reg : Physical base address of the IP registers and > > > > length of > > > > memory > > > > > > + mapped region. > > > > + - interrupts : interrupt number to the CPU. > > > > + - clocks : clock number of exynos4 rotator clock. > > > > + - clocks : clock name of rotator > > > > clock-names? > > > > > > + - status : "okay" or "disabled" > > > > + - limit table for image formats : > > > > min_w/min_h/max_w/max_h for
min/max
> > of image > > > > Limit table? This doesn't seem to be a well-defined binding, > > and it > > seems like a relatively generic thing to describe. > > > > > > + > > > > +Example: > > > > + rotator: rotator@12810000 { > > > > + compatible = "samsung,exynos4210-rotator"; > > > > + reg = <0x12810000 0x1000>; > > > > + interrupts = <0 83 0>; > > > > + clocks = <&clock 278>; > > > > + clock-names = "rotator"; > > > > + status = "disabled"; > > > > + ycbcr420_2p { > > > > Which names are allowed for these subnodes? > > > > > > + min_w = <32>; > > > > + min_h = <32>; > > > > + max_w = <32768>; > > > > + max_h = <32768>; > > > > + align = <3>; > > > > min-width, min-height, max-width, max-height? What units are > > they in? > > > > What does alignment specify exactly? > > > > Are these a configurable part of the rotator hardware, or are > > these > > values always the same?
Right, that seems like configurable part. At least, min_w/h and max_w/h
can
be different values according to SoC and pixel formats so they should be described enough in this dt-binding document file.
Is there really this much configurability? Could each of those values be a different on different SoCs
Not much but Yes. Rotator devices of Exynos SoC support various pixel formats; RGB888, RGB565, YUV422, YUV420-2Plane, and YUV420-3Plane, and each of these pixel formats could be different limitation to minimum and maximum sizes. For example, the below shows the limitation to Rotator device of Exynos4412 SoC according to pixel formats,> Image format Minimum size Maximum size
RGB888 8x8 8kx8k RGB565 16x16 16kx16k YUV422 16x16 16kx16k YUV420 2-Plane 32x32 32kx32k(in case of Y components) YUV420 3-Plane 64x32 32kx32k(in case of Y components)>And, I guess those limitations are slightly different according to Exynos SoC versions as long as I know.
Thanks, Inki Dae
, or could you just extract those values from the SoC/rotator hardware version and thus the DT compatible string?
My gut feeling is that those constraints could be well defined in the driver as static data and selected by the compatible property. Defining this in Device Tree may easily get out of control, when the limits become dependant on more parameters.
Whether devices should be described in much detail in DT rather than using multiple compatible strings had been discussed multiple times, for instance please see thread [1].
+1
The binding should not describe hardware functionality and how it works, but rather what hardware it is, unless it is really necessary. In this case this information can be placed in per-compatible static driver data, so there is no such need.
Best regards, Tomasz
-----Original Message----- From: linux-samsung-soc-owner@vger.kernel.org [mailto:linux-samsung-soc- owner@vger.kernel.org] On Behalf Of Tomasz Figa Sent: Tuesday, July 23, 2013 4:29 AM To: Sylwester Nawrocki Cc: Inki Dae; 'Chanho Park'; 'Lucas Stach'; 'Mark Rutland'; linux-samsung- soc@vger.kernel.org; devicetree-discuss@lists.ozlabs.org; sw0312.kim@samsung.com; dri-devel@lists.freedesktop.org; kyungmin.park@samsung.com; kgene.kim@samsung.com; linux-arm- kernel@lists.infradead.org Subject: Re: [PATCH 2/3] drm/exynos: add dt-binding documentation for rotator
On Monday 22 of July 2013 16:00:22 Sylwester Nawrocki wrote:
On 07/22/2013 03:31 PM, Inki Dae wrote:
---Original Message-----
From: linux-samsung-soc-owner@vger.kernel.org [mailto:linux-samsung-soc- owner@vger.kernel.org] On Behalf Of Lucas Stach Sent: Monday, July 22, 2013 9:47 PM To: Inki Dae Cc: 'Mark Rutland'; 'Chanho Park'; linux-samsung-soc@vger.kernel.org; devicetree-discuss@lists.ozlabs.org; sw0312.kim@samsung.com; dri- devel@lists.freedesktop.org; kyungmin.park@samsung.com; kgene.kim@samsung.com; linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 2/3] drm/exynos: add dt-binding documentation for rotator
Am Montag, den 22.07.2013, 21:37 +0900 schrieb Inki Dae:
> > > -----Original Message----- > > > From: Mark Rutland [mailto:mark.rutland@arm.com] > > > Sent: Monday, July 22, 2013 5:48 PM > > > To: Chanho Park > > > Cc: inki.dae@samsung.com; kgene.kim@samsung.com; > > > linux-samsung- > > > soc@vger.kernel.org; jy0922.shim@samsung.com; devicetree- > > > discuss@lists.ozlabs.org; sw0312.kim@samsung.com; dri- > > > devel@lists.freedesktop.org; kyungmin.park@samsung.com; > > > linux-arm- > > > kernel@lists.infradead.org > > > Subject: Re: [PATCH 2/3] drm/exynos: add dt-binding > > > documentation for > > > rotator > > > > > > On Mon, Jul 22, 2013 at 07:49:26AM +0100, Chanho Park wrote: > > > > > This patch adds a dt-binding document for exynos rotator. > > > > > It
describes
> > > which > > > > > > > > nodes should be defined to use the rotator. > > > > > > > > > > Signed-off-by: Chanho Park chanho61.park@samsung.com > > > > > Signed-off-by: Kyungmin Park kyungmin.park@samsung.com > > > > > --- > > > > > > > > > > .../bindings/drm/exynos/samsung-rotator.txt | 35 > > > > > > ++++++++++++++++++++ > > > > > > > > 1 file changed, 35 insertions(+) > > > > > create mode 100644 > > > > > > Documentation/devicetree/bindings/drm/exynos/samsung-rotator. > > > txt > > > > > > > > diff --git > > > > > a/Documentation/devicetree/bindings/drm/exynos/samsung- > > > > > > rotator.txt > > > b/Documentation/devicetree/bindings/drm/exynos/samsung- > > > rotator.txt > > > > > > > > new file mode 100644 > > > > > index 0000000..6b1d704 > > > > > --- /dev/null > > > > > +++ > > > > > b/Documentation/devicetree/bindings/drm/exynos/samsung-
rotator.txt
> > > > > @@ -0,0 +1,35 @@ > > > > > +* Samsung Image Rotator > > > > > + > > > > > +Required properties: > > > > > + - compatible : value should be the > > > > > "samsung,exynos4210". > > Please, add more compatible strings for other SoC. > > > > > > + - reg : Physical base address of the IP registers and > > > > > length of > > > > > > memory > > > > > > > > + mapped region. > > > > > + - interrupts : interrupt number to the CPU. > > > > > + - clocks : clock number of exynos4 rotator clock. > > > > > + - clocks : clock name of rotator > > > > > > clock-names? > > > > > > > > + - status : "okay" or "disabled" > > > > > + - limit table for image formats : > > > > > min_w/min_h/max_w/max_h for
min/max
> > > of image > > > > > > Limit table? This doesn't seem to be a well-defined binding, > > > and it > > > seems like a relatively generic thing to describe. > > > > > > > > + > > > > > +Example: > > > > > + rotator: rotator@12810000 { > > > > > + compatible = "samsung,exynos4210-rotator"; > > > > > + reg = <0x12810000 0x1000>; > > > > > + interrupts = <0 83 0>; > > > > > + clocks = <&clock 278>; > > > > > + clock-names = "rotator"; > > > > > + status = "disabled"; > > > > > + ycbcr420_2p { > > > > > > Which names are allowed for these subnodes? > > > > > > > > + min_w = <32>; > > > > > + min_h = <32>; > > > > > + max_w = <32768>; > > > > > + max_h = <32768>; > > > > > + align = <3>; > > > > > > min-width, min-height, max-width, max-height? What units are > > > they in? > > > > > > What does alignment specify exactly? > > > > > > Are these a configurable part of the rotator hardware, or are > > > these > > > values always the same? > > Right, that seems like configurable part. At least, min_w/h and > max_w/h
can
> be different values according to SoC and pixel formats so they > should be described enough in this dt-binding document file.
Is there really this much configurability? Could each of those values be a different on different SoCs
Not much but Yes. Rotator devices of Exynos SoC support various pixel formats; RGB888, RGB565, YUV422, YUV420-2Plane, and YUV420-3Plane, and each of these pixel formats could be different limitation to minimum and maximum sizes. For example, the below shows the limitation to Rotator device of Exynos4412 SoC according to pixel formats,> Image format Minimum size Maximum size
RGB888 8x8 8kx8k RGB565 16x16 16kx16k YUV422 16x16 16kx16k YUV420 2-Plane 32x32 32kx32k(in case of Y components) YUV420 3-Plane 64x32 32kx32k(in case of Y components)>And, I guess those limitations are slightly different according to Exynos SoC versions as long as I know.
Thanks, Inki Dae
, or could you just extract those values from the SoC/rotator hardware version and thus the DT compatible string?
My gut feeling is that those constraints could be well defined in the driver as static data and selected by the compatible property. Defining this in Device Tree may easily get out of control, when the limits become dependant on more parameters.
Whether devices should be described in much detail in DT rather than using multiple compatible strings had been discussed multiple times, for instance please see thread [1].
+1
The binding should not describe hardware functionality and how it works, but rather what hardware it is, unless it is really necessary.
Good comments. Agree.
Thanks, Inki Dae
In this case this information can be placed in per-compatible static driver data, so there is no such need.
Best regards, Tomasz
-- 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
Hi Mark, Thank you for your review.
-----Original Message----- From: Mark Rutland [mailto:mark.rutland@arm.com] Sent: Monday, July 22, 2013 5:48 PM To: Chanho Park Cc: inki.dae@samsung.com; kgene.kim@samsung.com; linux-samsung- soc@vger.kernel.org; jy0922.shim@samsung.com; devicetree- discuss@lists.ozlabs.org; sw0312.kim@samsung.com; dri- devel@lists.freedesktop.org; kyungmin.park@samsung.com; linux-arm- kernel@lists.infradead.org Subject: Re: [PATCH 2/3] drm/exynos: add dt-binding documentation for rotator
On Mon, Jul 22, 2013 at 07:49:26AM +0100, Chanho Park wrote:
This patch adds a dt-binding document for exynos rotator. It describes which nodes should be defined to use the rotator.
Signed-off-by: Chanho Park chanho61.park@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com
.../bindings/drm/exynos/samsung-rotator.txt | 35
++++++++++++++++++++
1 file changed, 35 insertions(+) create mode 100644 Documentation/devicetree/bindings/drm/exynos/samsung-rotator.txt
diff --git a/Documentation/devicetree/bindings/drm/exynos/samsung-rotator.txt b/Documentation/devicetree/bindings/drm/exynos/samsung-rotator.txt new file mode 100644 index 0000000..6b1d704 --- /dev/null +++ b/Documentation/devicetree/bindings/drm/exynos/samsung-rotator.txt @@ -0,0 +1,35 @@ +* Samsung Image Rotator
+Required properties:
- compatible : value should be the "samsung,exynos4210".
- reg : Physical base address of the IP registers and length of
memory
mapped region.
- interrupts : interrupt number to the CPU.
- clocks : clock number of exynos4 rotator clock.
- clocks : clock name of rotator
clock-names?
Yes. It seems to have a mistake during copy and paste. I'll modify it next patch.
- status : "okay" or "disabled"
- limit table for image formats : min_w/min_h/max_w/max_h for
- min/max of image
Limit table? This doesn't seem to be a well-defined binding, and it seems like a relatively generic thing to describe.
+Example:
- rotator: rotator@12810000 {
compatible = "samsung,exynos4210-rotator";reg = <0x12810000 0x1000>;interrupts = <0 83 0>;clocks = <&clock 278>;clock-names = "rotator";status = "disabled";ycbcr420_2p {Which names are allowed for these subnodes?
min_w = <32>;min_h = <32>;max_w = <32768>;max_h = <32768>;align = <3>;min-width, min-height, max-width, max-height? What units are they in?
What does alignment specify exactly?
Are these a configurable part of the rotator hardware, or are these values always the same? If thery're always the same, there's no need to describe in in the devicetree.
I've checked the rotator limitation for all of exynos4 chipsets and exynos5250. As a result, these have same value. I think it seems to be better leave on static value. I'll prepare next patches including your suggestion.
Thanks
Best Regards, Chanho Park
Thanks, Mark.
};rgb888 {min_w = <8>;min_h = <8>;max_w = <8192>;max_h = <8192>;align = <2>;};- };
-- 1.7.9.5
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
-----Original Message----- From: Chanho Park [mailto:chanho61.park@samsusng.com] Sent: Tuesday, July 23, 2013 10:22 AM To: 'Mark Rutland'; 'Chanho Park' Cc: inki.dae@samsung.com; kgene.kim@samsung.com; linux-samsung- soc@vger.kernel.org; jy0922.shim@samsung.com; sw0312.kim@samsung.com; dri- devel@lists.freedesktop.org; kyungmin.park@samsung.com; linux-arm- kernel@lists.infradead.org Subject: RE: [PATCH 2/3] drm/exynos: add dt-binding documentation for rotator
Hi Mark, Thank you for your review.
-----Original Message----- From: Mark Rutland [mailto:mark.rutland@arm.com] Sent: Monday, July 22, 2013 5:48 PM To: Chanho Park Cc: inki.dae@samsung.com; kgene.kim@samsung.com; linux-samsung- soc@vger.kernel.org; jy0922.shim@samsung.com; devicetree- discuss@lists.ozlabs.org; sw0312.kim@samsung.com; dri- devel@lists.freedesktop.org; kyungmin.park@samsung.com; linux-arm- kernel@lists.infradead.org Subject: Re: [PATCH 2/3] drm/exynos: add dt-binding documentation for rotator
On Mon, Jul 22, 2013 at 07:49:26AM +0100, Chanho Park wrote:
This patch adds a dt-binding document for exynos rotator. It describes which nodes should be defined to use the rotator.
Signed-off-by: Chanho Park chanho61.park@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com
.../bindings/drm/exynos/samsung-rotator.txt | 35
++++++++++++++++++++
1 file changed, 35 insertions(+) create mode 100644 Documentation/devicetree/bindings/drm/exynos/samsung-rotator.txt
diff --git a/Documentation/devicetree/bindings/drm/exynos/samsung-rotator.txt b/Documentation/devicetree/bindings/drm/exynos/samsung-rotator.txt new file mode 100644 index 0000000..6b1d704 --- /dev/null +++ b/Documentation/devicetree/bindings/drm/exynos/samsung-rotator.txt @@ -0,0 +1,35 @@ +* Samsung Image Rotator
+Required properties:
- compatible : value should be the "samsung,exynos4210".
- reg : Physical base address of the IP registers and length of
memory
mapped region.
- interrupts : interrupt number to the CPU.
- clocks : clock number of exynos4 rotator clock.
- clocks : clock name of rotator
clock-names?
Yes. It seems to have a mistake during copy and paste. I'll modify it next patch.
- status : "okay" or "disabled"
- limit table for image formats : min_w/min_h/max_w/max_h for
- min/max of image
Limit table? This doesn't seem to be a well-defined binding, and it seems like a relatively generic thing to describe.
+Example:
- rotator: rotator@12810000 {
compatible = "samsung,exynos4210-rotator";reg = <0x12810000 0x1000>;interrupts = <0 83 0>;clocks = <&clock 278>;clock-names = "rotator";status = "disabled";ycbcr420_2p {Which names are allowed for these subnodes?
min_w = <32>;min_h = <32>;max_w = <32768>;max_h = <32768>;align = <3>;min-width, min-height, max-width, max-height? What units are they in?
What does alignment specify exactly?
Are these a configurable part of the rotator hardware, or are these values always the same? If thery're always the same, there's no need to describe in in the devicetree.
I've checked the rotator limitation for all of exynos4 chipsets and exynos5250. As a result, these have same value.
Not true. See the Exynos4210 user manual again. At least, the manual says maximum size is 64k x 64k in case of Y components.
I think it seems to be better leave on static value. I'll prepare next patches including your suggestion.
So, you should consider such a little bit difference.
Thanks
Best Regards, Chanho Park
Thanks, Mark.
};rgb888 {min_w = <8>;min_h = <8>;max_w = <8192>;max_h = <8192>;align = <2>;};- };
-- 1.7.9.5
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
-----Original Message----- From: Inki Dae [mailto:inki.dae@samsung.com] Sent: Tuesday, July 23, 2013 10:36 AM To: 'Chanho Park'; 'Mark Rutland'; 'Chanho Park' Cc: kgene.kim@samsung.com; linux-samsung-soc@vger.kernel.org; jy0922.shim@samsung.com; sw0312.kim@samsung.com; dri- devel@lists.freedesktop.org; kyungmin.park@samsung.com; linux-arm- kernel@lists.infradead.org Subject: RE: [PATCH 2/3] drm/exynos: add dt-binding documentation for rotator
-----Original Message----- From: Chanho Park [mailto:chanho61.park@samsusng.com] Sent: Tuesday, July 23, 2013 10:22 AM To: 'Mark Rutland'; 'Chanho Park' Cc: inki.dae@samsung.com; kgene.kim@samsung.com; linux-samsung- soc@vger.kernel.org; jy0922.shim@samsung.com; sw0312.kim@samsung.com; dri- devel@lists.freedesktop.org; kyungmin.park@samsung.com; linux-arm- kernel@lists.infradead.org Subject: RE: [PATCH 2/3] drm/exynos: add dt-binding documentation for rotator
Hi Mark, Thank you for your review.
-----Original Message----- From: Mark Rutland [mailto:mark.rutland@arm.com] Sent: Monday, July 22, 2013 5:48 PM To: Chanho Park Cc: inki.dae@samsung.com; kgene.kim@samsung.com; linux-samsung- soc@vger.kernel.org; jy0922.shim@samsung.com; devicetree- discuss@lists.ozlabs.org; sw0312.kim@samsung.com; dri- devel@lists.freedesktop.org; kyungmin.park@samsung.com; linux-arm- kernel@lists.infradead.org Subject: Re: [PATCH 2/3] drm/exynos: add dt-binding documentation for rotator
On Mon, Jul 22, 2013 at 07:49:26AM +0100, Chanho Park wrote:
This patch adds a dt-binding document for exynos rotator. It describes which nodes should be defined to use the rotator.
Signed-off-by: Chanho Park chanho61.park@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com
.../bindings/drm/exynos/samsung-rotator.txt | 35
++++++++++++++++++++
1 file changed, 35 insertions(+) create mode 100644 Documentation/devicetree/bindings/drm/exynos/samsung-rotator.txt
diff --git a/Documentation/devicetree/bindings/drm/exynos/samsung-rotator.txt b/Documentation/devicetree/bindings/drm/exynos/samsung-rotator.txt new file mode 100644 index 0000000..6b1d704 --- /dev/null +++ b/Documentation/devicetree/bindings/drm/exynos/samsung-rotator +++ .txt @@ -0,0 +1,35 @@ +* Samsung Image Rotator
+Required properties:
- compatible : value should be the "samsung,exynos4210".
- reg : Physical base address of the IP registers and length of
memory
mapped region.
- interrupts : interrupt number to the CPU.
- clocks : clock number of exynos4 rotator clock.
- clocks : clock name of rotator
clock-names?
Yes. It seems to have a mistake during copy and paste. I'll modify it next patch.
- status : "okay" or "disabled"
- limit table for image formats : min_w/min_h/max_w/max_h for
- min/max of image
Limit table? This doesn't seem to be a well-defined binding, and it seems like a relatively generic thing to describe.
+Example:
- rotator: rotator@12810000 {
compatible = "samsung,exynos4210-rotator";reg = <0x12810000 0x1000>;interrupts = <0 83 0>;clocks = <&clock 278>;clock-names = "rotator";status = "disabled";ycbcr420_2p {Which names are allowed for these subnodes?
min_w = <32>;min_h = <32>;max_w = <32768>;max_h = <32768>;align = <3>;min-width, min-height, max-width, max-height? What units are they in?
What does alignment specify exactly?
Are these a configurable part of the rotator hardware, or are these values always the same? If thery're always the same, there's no need to describe in in the devicetree.
I've checked the rotator limitation for all of exynos4 chipsets and exynos5250. As a result, these have same value.
Not true. See the Exynos4210 user manual again. At least, the manual says maximum size is 64k x 64k in case of Y components.
Yes. The exynos4210 has different maximum limits of RGB888/YCbCr420 2p compared with any other SoCs(exynos4x12/exynos5250). I'll make a next patch with considering it.
Thanks
Best Regards, Chanho Park.
I think it seems to be better leave on static value. I'll prepare next patches including your suggestion.
So, you should consider such a little bit difference.
Thanks
Best Regards, Chanho Park
Thanks, Mark.
};rgb888 {min_w = <8>;min_h = <8>;max_w = <8192>;max_h = <8192>;align = <2>;};- };
-- 1.7.9.5
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
This patch adds a device node of rotator for exynos4 platform. It has proper register and clock information. It also has limit table to get restrictions of the image size.
Signed-off-by: Chanho Park chanho61.park@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com --- arch/arm/boot/dts/exynos4.dtsi | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)
diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi index 3f94fe8..7d7dcef 100644 --- a/arch/arm/boot/dts/exynos4.dtsi +++ b/arch/arm/boot/dts/exynos4.dtsi @@ -398,4 +398,27 @@ samsung,power-domain = <&pd_lcd0>; status = "disabled"; }; + + rotator: rotator@12810000 { + compatible = "samsung,exynos4210-rotator"; + reg = <0x12810000 0x1000>; + interrupts = <0 83 0>; + clocks = <&clock 278>; + clock-names = "rotator"; + status = "disabled"; + ycbcr420_2p { + min_w = <32>; + min_h = <32>; + max_w = <32768>; + max_h = <32768>; + align = <3>; + }; + rgb888 { + min_w = <8>; + min_h = <8>; + max_w = <8192>; + max_h = <8192>; + align = <2>; + }; + }; };
dri-devel@lists.freedesktop.org
-
Chanho Park -
Chanho Park -
Inki Dae -
Lucas Stach -
Mark Rutland -
Sylwester Nawrocki -
Tomasz Figa