[PATCH v3 0/6] Exynos-gsc: Support the hardware rotation limits

[PATCH v3 0/6] Exynos-gsc: Support the hardware rotation limits

[Hoegeun Kwon]

Hello all,

The gscaler has hardware rotation limits. So this patch set support
the rotate hardware limits of gsc.

To avoid problems with bisectability, patches 1~4 must be merged and
then merged 5 and 6.

Changes for V3:
- Fixed of_match_node() to of_device_get_match_data() in drm gsc driver.
- Added hardware rotation limits for gsc driver of v4l2.
- Added the remove unnecessary compatible for DT document and Exynos dts.

Changes for V2:
- Added the interface info in binding document.
- Added clean name of compatible in Exynos dts.
- Added maximum supported picture size hardcoded into driver.

Best regards,
Hoegeun

Hoegeun Kwon (6):
  [media] exynos-gsc: Add compatible for Exynos 5250 and 5420 specific
    version
  ARM: dts: exynos: Add clean name of compatible.
  drm/exynos/gsc: Add hardware rotation limits
  [media] exynos-gsc: Add hardware rotation limits
  [media] exynos-gsc: Remove unnecessary compatible
  ARM: dts: exynos: Remove unnecessary compatible

 .../devicetree/bindings/media/exynos5-gsc.txt      |  7 +-
 arch/arm/boot/dts/exynos5250.dtsi                  |  8 +-
 arch/arm/boot/dts/exynos5420.dtsi                  |  4 +-
 drivers/gpu/drm/exynos/exynos_drm_gsc.c            | 93 ++++++++++++++-------
 drivers/media/platform/exynos-gsc/gsc-core.c       | 96 +++++++++++++++++++---
 include/uapi/drm/exynos_drm.h                      |  2 +
 6 files changed, 159 insertions(+), 51 deletions(-)

-- 
1.9.1

[PATCH v3 1/6] [media] exynos-gsc: Add compatible for Exynos 5250 and 5420 specific version

[Hoegeun Kwon]

Exynos 5250 and 5420 have different hardware rotation limits.
Since we have to distinguish between these two, we add different
compatible(samsung,exynos5250-gsc and samsung,exynos5420-gsc).

Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
---
 Documentation/devicetree/bindings/media/exynos5-gsc.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/media/exynos5-gsc.txt b/Documentation/devicetree/bindings/media/exynos5-gsc.txt
index 26ca25b..daa56d5 100644
--- a/Documentation/devicetree/bindings/media/exynos5-gsc.txt
+++ b/Documentation/devicetree/bindings/media/exynos5-gsc.txt
@@ -5,6 +5,7 @@ G-Scaler is used for scaling and color space conversion on EXYNOS5 SoCs.
 Required properties:
 - compatible: should be "samsung,exynos5-gsc" (for Exynos 5250, 5420 and
 	      5422 SoCs) or "samsung,exynos5433-gsc" (Exynos 5433)
+	      or "samsung,exynos5250-gsc" or "samsung,exynos5420-gsc"
 - reg: should contain G-Scaler physical address location and length.
 - interrupts: should contain G-Scaler interrupt number
 
-- 
1.9.1

[PATCH v3 2/6] ARM: dts: exynos: Add clean name of compatible.

[Hoegeun Kwon]

Exynos 5250 and 5420 have different hardware rotation limits. However,
currently it uses only one compatible - "exynos5-gsc". Since we have
to distinguish between these two, we add different compatible.

Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
---
 arch/arm/boot/dts/exynos5250.dtsi | 8 ++++----
 arch/arm/boot/dts/exynos5420.dtsi | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 8dbeb87..bf08101 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -637,7 +637,7 @@
 		};
 
 		gsc_0:  gsc@13e00000 {
-			compatible = "samsung,exynos5-gsc";
+			compatible = "samsung,exynos5-gsc", "samsung,exynos5250-gsc";
 			reg = <0x13e00000 0x1000>;
 			interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>;
 			power-domains = <&pd_gsc>;
@@ -647,7 +647,7 @@
 		};
 
 		gsc_1:  gsc@13e10000 {
-			compatible = "samsung,exynos5-gsc";
+			compatible = "samsung,exynos5-gsc", "samsung,exynos5250-gsc";
 			reg = <0x13e10000 0x1000>;
 			interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
 			power-domains = <&pd_gsc>;
@@ -657,7 +657,7 @@
 		};
 
 		gsc_2:  gsc@13e20000 {
-			compatible = "samsung,exynos5-gsc";
+			compatible = "samsung,exynos5-gsc", "samsung,exynos5250-gsc";
 			reg = <0x13e20000 0x1000>;
 			interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
 			power-domains = <&pd_gsc>;
@@ -667,7 +667,7 @@
 		};
 
 		gsc_3:  gsc@13e30000 {
-			compatible = "samsung,exynos5-gsc";
+			compatible = "samsung,exynos5-gsc", "samsung,exynos5250-gsc";
 			reg = <0x13e30000 0x1000>;
 			interrupts = <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>;
 			power-domains = <&pd_gsc>;
diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index 02d2f89..86afe77 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -658,7 +658,7 @@
 		};
 
 		gsc_0: video-scaler@13e00000 {
-			compatible = "samsung,exynos5-gsc";
+			compatible = "samsung,exynos5-gsc", "samsung,exynos5420-gsc";
 			reg = <0x13e00000 0x1000>;
 			interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&clock CLK_GSCL0>;
@@ -668,7 +668,7 @@
 		};
 
 		gsc_1: video-scaler@13e10000 {
-			compatible = "samsung,exynos5-gsc";
+			compatible = "samsung,exynos5-gsc", "samsung,exynos5420-gsc";
 			reg = <0x13e10000 0x1000>;
 			interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&clock CLK_GSCL1>;
-- 
1.9.1

Re: [PATCH v3 2/6] ARM: dts: exynos: Add clean name of compatible.

[Robin Murphy]

On 08/09/17 07:02, Hoegeun Kwon wrote:
> Exynos 5250 and 5420 have different hardware rotation limits. However,
> currently it uses only one compatible - "exynos5-gsc". Since we have
> to distinguish between these two, we add different compatible.
> 
> Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
> ---
>  arch/arm/boot/dts/exynos5250.dtsi | 8 ++++----
>  arch/arm/boot/dts/exynos5420.dtsi | 4 ++--
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
> index 8dbeb87..bf08101 100644
> --- a/arch/arm/boot/dts/exynos5250.dtsi
> +++ b/arch/arm/boot/dts/exynos5250.dtsi
> @@ -637,7 +637,7 @@
>  		};
>  
>  		gsc_0:  gsc@13e00000 {
> -			compatible = "samsung,exynos5-gsc";
> +			compatible = "samsung,exynos5-gsc", "samsung,exynos5250-gsc";

These should be the other way round - the most specific compatible
should come first, then the more general fallback afterwards.

(and similarly in all cases below)

Robin.

>  			reg = <0x13e00000 0x1000>;
>  			interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>;
>  			power-domains = <&pd_gsc>;
> @@ -647,7 +647,7 @@
>  		};
>  
>  		gsc_1:  gsc@13e10000 {
> -			compatible = "samsung,exynos5-gsc";
> +			compatible = "samsung,exynos5-gsc", "samsung,exynos5250-gsc";
>  			reg = <0x13e10000 0x1000>;
>  			interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
>  			power-domains = <&pd_gsc>;
> @@ -657,7 +657,7 @@
>  		};
>  
>  		gsc_2:  gsc@13e20000 {
> -			compatible = "samsung,exynos5-gsc";
> +			compatible = "samsung,exynos5-gsc", "samsung,exynos5250-gsc";
>  			reg = <0x13e20000 0x1000>;
>  			interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
>  			power-domains = <&pd_gsc>;
> @@ -667,7 +667,7 @@
>  		};
>  
>  		gsc_3:  gsc@13e30000 {
> -			compatible = "samsung,exynos5-gsc";
> +			compatible = "samsung,exynos5-gsc", "samsung,exynos5250-gsc";
>  			reg = <0x13e30000 0x1000>;
>  			interrupts = <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>;
>  			power-domains = <&pd_gsc>;
> diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
> index 02d2f89..86afe77 100644
> --- a/arch/arm/boot/dts/exynos5420.dtsi
> +++ b/arch/arm/boot/dts/exynos5420.dtsi
> @@ -658,7 +658,7 @@
>  		};
>  
>  		gsc_0: video-scaler@13e00000 {
> -			compatible = "samsung,exynos5-gsc";
> +			compatible = "samsung,exynos5-gsc", "samsung,exynos5420-gsc";
>  			reg = <0x13e00000 0x1000>;
>  			interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>;
>  			clocks = <&clock CLK_GSCL0>;
> @@ -668,7 +668,7 @@
>  		};
>  
>  		gsc_1: video-scaler@13e10000 {
> -			compatible = "samsung,exynos5-gsc";
> +			compatible = "samsung,exynos5-gsc", "samsung,exynos5420-gsc";
>  			reg = <0x13e10000 0x1000>;
>  			interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
>  			clocks = <&clock CLK_GSCL1>;
> 

[PATCH v3 3/6] drm/exynos/gsc: Add hardware rotation limits

[Hoegeun Kwon]

The gscaler has hardware rotation limits that need to be hardcoded
into driver. Distinguish them and add them to the property list.

The hardware rotation limits are related to the cropped source size.
When swap occurs, use rot_max size instead of crop_max size.

Also the scaling limits are related to pos size, use pos size to check
the limits.

Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_gsc.c | 93 +++++++++++++++++++++++----------
 include/uapi/drm/exynos_drm.h           |  2 +
 2 files changed, 66 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
index 0506b2b..a4fb347 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
@@ -17,6 +17,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/mfd/syscon.h>
 #include <linux/regmap.h>
+#include <linux/of_device.h>
 
 #include <drm/drmP.h>
 #include <drm/exynos_drm.h>
@@ -150,6 +151,15 @@ struct gsc_context {
 	bool	suspended;
 };
 
+/*
+ * struct gsc_driverdata - per device type driver data for init time.
+ *
+ * @rot_max: rotation max resolution.
+ */
+struct gsc_driverdata {
+	struct drm_exynos_sz rot_max;
+};
+
 /* 8-tap Filter Coefficient */
 static const int h_coef_8t[GSC_COEF_RATIO][GSC_COEF_ATTR][GSC_COEF_H_8T] = {
 	{	/* Ratio <= 65536 (~8:8) */
@@ -1401,6 +1411,23 @@ static int gsc_ippdrv_check_property(struct device *dev,
 	bool swap;
 	int i;
 
+	config = &property->config[EXYNOS_DRM_OPS_DST];
+
+	/* check for degree */
+	switch (config->degree) {
+	case EXYNOS_DRM_DEGREE_90:
+	case EXYNOS_DRM_DEGREE_270:
+		swap = true;
+		break;
+	case EXYNOS_DRM_DEGREE_0:
+	case EXYNOS_DRM_DEGREE_180:
+		swap = false;
+		break;
+	default:
+		DRM_ERROR("invalid degree.\n");
+		goto err_property;
+	}
+
 	for_each_ipp_ops(i) {
 		if ((i == EXYNOS_DRM_OPS_SRC) &&
 			(property->cmd == IPP_CMD_WB))
@@ -1416,21 +1443,6 @@ static int gsc_ippdrv_check_property(struct device *dev,
 			goto err_property;
 		}
 
-		/* check for degree */
-		switch (config->degree) {
-		case EXYNOS_DRM_DEGREE_90:
-		case EXYNOS_DRM_DEGREE_270:
-			swap = true;
-			break;
-		case EXYNOS_DRM_DEGREE_0:
-		case EXYNOS_DRM_DEGREE_180:
-			swap = false;
-			break;
-		default:
-			DRM_ERROR("invalid degree.\n");
-			goto err_property;
-		}
-
 		/* check for buffer bound */
 		if ((pos->x + pos->w > sz->hsize) ||
 			(pos->y + pos->h > sz->vsize)) {
@@ -1438,21 +1450,27 @@ static int gsc_ippdrv_check_property(struct device *dev,
 			goto err_property;
 		}
 
+		/*
+		 * The rotation hardware limits are related to the cropped
+		 * source size. So use rot_max size to check the limits when
+		 * swap happens. And also the scaling limits are related to pos
+		 * size, use pos size to check the limits.
+		 */
 		/* check for crop */
 		if ((i == EXYNOS_DRM_OPS_SRC) && (pp->crop)) {
 			if (swap) {
 				if ((pos->h < pp->crop_min.hsize) ||
-					(sz->vsize > pp->crop_max.hsize) ||
+					(pos->h > pp->rot_max.hsize) ||
 					(pos->w < pp->crop_min.vsize) ||
-					(sz->hsize > pp->crop_max.vsize)) {
+					(pos->w > pp->rot_max.vsize)) {
 					DRM_ERROR("out of crop size.\n");
 					goto err_property;
 				}
 			} else {
 				if ((pos->w < pp->crop_min.hsize) ||
-					(sz->hsize > pp->crop_max.hsize) ||
+					(pos->w > pp->crop_max.hsize) ||
 					(pos->h < pp->crop_min.vsize) ||
-					(sz->vsize > pp->crop_max.vsize)) {
+					(pos->h > pp->crop_max.vsize)) {
 					DRM_ERROR("out of crop size.\n");
 					goto err_property;
 				}
@@ -1463,17 +1481,17 @@ static int gsc_ippdrv_check_property(struct device *dev,
 		if ((i == EXYNOS_DRM_OPS_DST) && (pp->scale)) {
 			if (swap) {
 				if ((pos->h < pp->scale_min.hsize) ||
-					(sz->vsize > pp->scale_max.hsize) ||
+					(pos->h > pp->scale_max.hsize) ||
 					(pos->w < pp->scale_min.vsize) ||
-					(sz->hsize > pp->scale_max.vsize)) {
+					(pos->w > pp->scale_max.vsize)) {
 					DRM_ERROR("out of scale size.\n");
 					goto err_property;
 				}
 			} else {
 				if ((pos->w < pp->scale_min.hsize) ||
-					(sz->hsize > pp->scale_max.hsize) ||
+					(pos->w > pp->scale_max.hsize) ||
 					(pos->h < pp->scale_min.vsize) ||
-					(sz->vsize > pp->scale_max.vsize)) {
+					(pos->h > pp->scale_max.vsize)) {
 					DRM_ERROR("out of scale size.\n");
 					goto err_property;
 				}
@@ -1657,12 +1675,34 @@ static void gsc_ippdrv_stop(struct device *dev, enum drm_exynos_ipp_cmd cmd)
 	gsc_write(cfg, GSC_ENABLE);
 }
 
+static struct gsc_driverdata gsc_exynos5250_drvdata = {
+	.rot_max = { 2048, 2048 },
+};
+
+static struct gsc_driverdata gsc_exynos5420_drvdata = {
+	.rot_max = { 2016, 2016 },
+};
+
+static const struct of_device_id exynos_drm_gsc_of_match[] = {
+	{
+		.compatible = "samsung,exynos5250-gsc",
+		.data = &gsc_exynos5250_drvdata,
+	},
+	{
+		.compatible = "samsung,exynos5420-gsc",
+		.data = &gsc_exynos5420_drvdata,
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, exynos_drm_gsc_of_match);
+
 static int gsc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct gsc_context *ctx;
 	struct resource *res;
 	struct exynos_drm_ippdrv *ippdrv;
+	const struct gsc_driverdata *drv_data = of_device_get_match_data(dev);
 	int ret;
 
 	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
@@ -1722,6 +1762,7 @@ static int gsc_probe(struct platform_device *pdev)
 		dev_err(dev, "failed to init property list.\n");
 		return ret;
 	}
+	ctx->ippdrv.prop_list.rot_max = drv_data->rot_max;
 
 	DRM_DEBUG_KMS("id[%d]ippdrv[%pK]\n", ctx->id, ippdrv);
 
@@ -1784,12 +1825,6 @@ static int __maybe_unused gsc_runtime_resume(struct device *dev)
 	SET_RUNTIME_PM_OPS(gsc_runtime_suspend, gsc_runtime_resume, NULL)
 };
 
-static const struct of_device_id exynos_drm_gsc_of_match[] = {
-	{ .compatible = "samsung,exynos5-gsc" },
-	{ },
-};
-MODULE_DEVICE_TABLE(of, exynos_drm_gsc_of_match);
-
 struct platform_driver gsc_driver = {
 	.probe		= gsc_probe,
 	.remove		= gsc_remove,
diff --git a/include/uapi/drm/exynos_drm.h b/include/uapi/drm/exynos_drm.h
index cb3e9f9..d5d5518 100644
--- a/include/uapi/drm/exynos_drm.h
+++ b/include/uapi/drm/exynos_drm.h
@@ -192,6 +192,7 @@ enum drm_exynos_planer {
  * @crop_max: crop max resolution.
  * @scale_min: scale min resolution.
  * @scale_max: scale max resolution.
+ * @rot_max: rotation max resolution.
  */
 struct drm_exynos_ipp_prop_list {
 	__u32	version;
@@ -210,6 +211,7 @@ struct drm_exynos_ipp_prop_list {
 	struct drm_exynos_sz	crop_max;
 	struct drm_exynos_sz	scale_min;
 	struct drm_exynos_sz	scale_max;
+	struct drm_exynos_sz	rot_max;
 };
 
 /**
-- 
1.9.1

Re: [PATCH v3 3/6] drm/exynos/gsc: Add hardware rotation limits

[Robin Murphy]

On 08/09/17 07:02, Hoegeun Kwon wrote:
> The gscaler has hardware rotation limits that need to be hardcoded
> into driver. Distinguish them and add them to the property list.
> 
> The hardware rotation limits are related to the cropped source size.
> When swap occurs, use rot_max size instead of crop_max size.
> 
> Also the scaling limits are related to pos size, use pos size to check
> the limits.
> 
> Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_gsc.c | 93 +++++++++++++++++++++++----------
>  include/uapi/drm/exynos_drm.h           |  2 +
>  2 files changed, 66 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
> index 0506b2b..a4fb347 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
> @@ -17,6 +17,7 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/regmap.h>
> +#include <linux/of_device.h>
>  
>  #include <drm/drmP.h>
>  #include <drm/exynos_drm.h>
> @@ -150,6 +151,15 @@ struct gsc_context {
>  	bool	suspended;
>  };
>  
> +/*
> + * struct gsc_driverdata - per device type driver data for init time.
> + *
> + * @rot_max: rotation max resolution.
> + */
> +struct gsc_driverdata {
> +	struct drm_exynos_sz rot_max;
> +};
> +
>  /* 8-tap Filter Coefficient */
>  static const int h_coef_8t[GSC_COEF_RATIO][GSC_COEF_ATTR][GSC_COEF_H_8T] = {
>  	{	/* Ratio <= 65536 (~8:8) */
> @@ -1401,6 +1411,23 @@ static int gsc_ippdrv_check_property(struct device *dev,
>  	bool swap;
>  	int i;
>  
> +	config = &property->config[EXYNOS_DRM_OPS_DST];
> +
> +	/* check for degree */
> +	switch (config->degree) {
> +	case EXYNOS_DRM_DEGREE_90:
> +	case EXYNOS_DRM_DEGREE_270:
> +		swap = true;
> +		break;
> +	case EXYNOS_DRM_DEGREE_0:
> +	case EXYNOS_DRM_DEGREE_180:
> +		swap = false;
> +		break;
> +	default:
> +		DRM_ERROR("invalid degree.\n");
> +		goto err_property;
> +	}
> +
>  	for_each_ipp_ops(i) {
>  		if ((i == EXYNOS_DRM_OPS_SRC) &&
>  			(property->cmd == IPP_CMD_WB))
> @@ -1416,21 +1443,6 @@ static int gsc_ippdrv_check_property(struct device *dev,
>  			goto err_property;
>  		}
>  
> -		/* check for degree */
> -		switch (config->degree) {
> -		case EXYNOS_DRM_DEGREE_90:
> -		case EXYNOS_DRM_DEGREE_270:
> -			swap = true;
> -			break;
> -		case EXYNOS_DRM_DEGREE_0:
> -		case EXYNOS_DRM_DEGREE_180:
> -			swap = false;
> -			break;
> -		default:
> -			DRM_ERROR("invalid degree.\n");
> -			goto err_property;
> -		}
> -
>  		/* check for buffer bound */
>  		if ((pos->x + pos->w > sz->hsize) ||
>  			(pos->y + pos->h > sz->vsize)) {
> @@ -1438,21 +1450,27 @@ static int gsc_ippdrv_check_property(struct device *dev,
>  			goto err_property;
>  		}
>  
> +		/*
> +		 * The rotation hardware limits are related to the cropped
> +		 * source size. So use rot_max size to check the limits when
> +		 * swap happens. And also the scaling limits are related to pos
> +		 * size, use pos size to check the limits.
> +		 */
>  		/* check for crop */
>  		if ((i == EXYNOS_DRM_OPS_SRC) && (pp->crop)) {
>  			if (swap) {
>  				if ((pos->h < pp->crop_min.hsize) ||
> -					(sz->vsize > pp->crop_max.hsize) ||
> +					(pos->h > pp->rot_max.hsize) ||
>  					(pos->w < pp->crop_min.vsize) ||
> -					(sz->hsize > pp->crop_max.vsize)) {
> +					(pos->w > pp->rot_max.vsize)) {
>  					DRM_ERROR("out of crop size.\n");
>  					goto err_property;
>  				}
>  			} else {
>  				if ((pos->w < pp->crop_min.hsize) ||
> -					(sz->hsize > pp->crop_max.hsize) ||
> +					(pos->w > pp->crop_max.hsize) ||
>  					(pos->h < pp->crop_min.vsize) ||
> -					(sz->vsize > pp->crop_max.vsize)) {
> +					(pos->h > pp->crop_max.vsize)) {
>  					DRM_ERROR("out of crop size.\n");
>  					goto err_property;
>  				}
> @@ -1463,17 +1481,17 @@ static int gsc_ippdrv_check_property(struct device *dev,
>  		if ((i == EXYNOS_DRM_OPS_DST) && (pp->scale)) {
>  			if (swap) {
>  				if ((pos->h < pp->scale_min.hsize) ||
> -					(sz->vsize > pp->scale_max.hsize) ||
> +					(pos->h > pp->scale_max.hsize) ||
>  					(pos->w < pp->scale_min.vsize) ||
> -					(sz->hsize > pp->scale_max.vsize)) {
> +					(pos->w > pp->scale_max.vsize)) {
>  					DRM_ERROR("out of scale size.\n");
>  					goto err_property;
>  				}
>  			} else {
>  				if ((pos->w < pp->scale_min.hsize) ||
> -					(sz->hsize > pp->scale_max.hsize) ||
> +					(pos->w > pp->scale_max.hsize) ||
>  					(pos->h < pp->scale_min.vsize) ||
> -					(sz->vsize > pp->scale_max.vsize)) {
> +					(pos->h > pp->scale_max.vsize)) {
>  					DRM_ERROR("out of scale size.\n");
>  					goto err_property;
>  				}
> @@ -1657,12 +1675,34 @@ static void gsc_ippdrv_stop(struct device *dev, enum drm_exynos_ipp_cmd cmd)
>  	gsc_write(cfg, GSC_ENABLE);
>  }
>  
> +static struct gsc_driverdata gsc_exynos5250_drvdata = {
> +	.rot_max = { 2048, 2048 },
> +};
> +
> +static struct gsc_driverdata gsc_exynos5420_drvdata = {
> +	.rot_max = { 2016, 2016 },
> +};
> +
> +static const struct of_device_id exynos_drm_gsc_of_match[] = {
> +	{
> +		.compatible = "samsung,exynos5250-gsc",
> +		.data = &gsc_exynos5250_drvdata,
> +	},
> +	{
> +		.compatible = "samsung,exynos5420-gsc",
> +		.data = &gsc_exynos5420_drvdata,
> +	},
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, exynos_drm_gsc_of_match);
> +
>  static int gsc_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct gsc_context *ctx;
>  	struct resource *res;
>  	struct exynos_drm_ippdrv *ippdrv;
> +	const struct gsc_driverdata *drv_data = of_device_get_match_data(dev);
>  	int ret;
>  
>  	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> @@ -1722,6 +1762,7 @@ static int gsc_probe(struct platform_device *pdev)
>  		dev_err(dev, "failed to init property list.\n");
>  		return ret;
>  	}
> +	ctx->ippdrv.prop_list.rot_max = drv_data->rot_max;
>  
>  	DRM_DEBUG_KMS("id[%d]ippdrv[%pK]\n", ctx->id, ippdrv);
>  
> @@ -1784,12 +1825,6 @@ static int __maybe_unused gsc_runtime_resume(struct device *dev)
>  	SET_RUNTIME_PM_OPS(gsc_runtime_suspend, gsc_runtime_resume, NULL)
>  };
>  
> -static const struct of_device_id exynos_drm_gsc_of_match[] = {
> -	{ .compatible = "samsung,exynos5-gsc" },
> -	{ },
> -};
> -MODULE_DEVICE_TABLE(of, exynos_drm_gsc_of_match);

As Krzysztof noted, this breaks compatibility with existing DTBs, and
not everyone wants to update their firmware in lock-step with their
kernel. That said, as far as I can see there's little need to remove it
anyway - as long as there is some set of lowest-common-denominator
limits that will work on any hardware variant, there doesn't seem to be
much harm in keeping a generic fallback around indefinitely.

Robin.

> -
>  struct platform_driver gsc_driver = {
>  	.probe		= gsc_probe,
>  	.remove		= gsc_remove,
> diff --git a/include/uapi/drm/exynos_drm.h b/include/uapi/drm/exynos_drm.h
> index cb3e9f9..d5d5518 100644
> --- a/include/uapi/drm/exynos_drm.h
> +++ b/include/uapi/drm/exynos_drm.h
> @@ -192,6 +192,7 @@ enum drm_exynos_planer {
>   * @crop_max: crop max resolution.
>   * @scale_min: scale min resolution.
>   * @scale_max: scale max resolution.
> + * @rot_max: rotation max resolution.
>   */
>  struct drm_exynos_ipp_prop_list {
>  	__u32	version;
> @@ -210,6 +211,7 @@ struct drm_exynos_ipp_prop_list {
>  	struct drm_exynos_sz	crop_max;
>  	struct drm_exynos_sz	scale_min;
>  	struct drm_exynos_sz	scale_max;
> +	struct drm_exynos_sz	rot_max;
>  };
>  
>  /**
> 

[PATCH v3 4/6] [media] exynos-gsc: Add hardware rotation limits

[Hoegeun Kwon]

The hardware rotation limits of gsc depends on SOC (Exynos
5250/5420/5433). Distinguish them and add them to the driver data.

Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
---
 drivers/media/platform/exynos-gsc/gsc-core.c | 96 ++++++++++++++++++++++++----
 1 file changed, 83 insertions(+), 13 deletions(-)

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c
index 4380150..8f8636e 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/exynos-gsc/gsc-core.c
@@ -943,7 +943,37 @@ static irqreturn_t gsc_irq_handler(int irq, void *priv)
 	return IRQ_HANDLED;
 }
 
-static struct gsc_pix_max gsc_v_100_max = {
+static struct gsc_pix_max gsc_v_5250_max = {
+	.org_scaler_bypass_w	= 8192,
+	.org_scaler_bypass_h	= 8192,
+	.org_scaler_input_w	= 4800,
+	.org_scaler_input_h	= 3344,
+	.real_rot_dis_w		= 4800,
+	.real_rot_dis_h		= 3344,
+	.real_rot_en_w		= 2016,
+	.real_rot_en_h		= 2016,
+	.target_rot_dis_w	= 4800,
+	.target_rot_dis_h	= 3344,
+	.target_rot_en_w	= 2016,
+	.target_rot_en_h	= 2016,
+};
+
+static struct gsc_pix_max gsc_v_5420_max = {
+	.org_scaler_bypass_w	= 8192,
+	.org_scaler_bypass_h	= 8192,
+	.org_scaler_input_w	= 4800,
+	.org_scaler_input_h	= 3344,
+	.real_rot_dis_w		= 4800,
+	.real_rot_dis_h		= 3344,
+	.real_rot_en_w		= 2048,
+	.real_rot_en_h		= 2048,
+	.target_rot_dis_w	= 4800,
+	.target_rot_dis_h	= 3344,
+	.target_rot_en_w	= 2016,
+	.target_rot_en_h	= 2016,
+};
+
+static struct gsc_pix_max gsc_v_5433_max = {
 	.org_scaler_bypass_w	= 8192,
 	.org_scaler_bypass_h	= 8192,
 	.org_scaler_input_w	= 4800,
@@ -979,8 +1009,8 @@ static irqreturn_t gsc_irq_handler(int irq, void *priv)
 	.target_h		= 2,  /* yuv420 : 2, others : 1 */
 };
 
-static struct gsc_variant gsc_v_100_variant = {
-	.pix_max		= &gsc_v_100_max,
+static struct gsc_variant gsc_v_5250_variant = {
+	.pix_max		= &gsc_v_5250_max,
 	.pix_min		= &gsc_v_100_min,
 	.pix_align		= &gsc_v_100_align,
 	.in_buf_cnt		= 32,
@@ -992,12 +1022,48 @@ static irqreturn_t gsc_irq_handler(int irq, void *priv)
 	.local_sc_down		= 2,
 };
 
-static struct gsc_driverdata gsc_v_100_drvdata = {
+static struct gsc_variant gsc_v_5420_variant = {
+	.pix_max		= &gsc_v_5420_max,
+	.pix_min		= &gsc_v_100_min,
+	.pix_align		= &gsc_v_100_align,
+	.in_buf_cnt		= 32,
+	.out_buf_cnt		= 32,
+	.sc_up_max		= 8,
+	.sc_down_max		= 16,
+	.poly_sc_down_max	= 4,
+	.pre_sc_down_max	= 4,
+	.local_sc_down		= 2,
+};
+
+static struct gsc_variant gsc_v_5433_variant = {
+	.pix_max		= &gsc_v_5433_max,
+	.pix_min		= &gsc_v_100_min,
+	.pix_align		= &gsc_v_100_align,
+	.in_buf_cnt		= 32,
+	.out_buf_cnt		= 32,
+	.sc_up_max		= 8,
+	.sc_down_max		= 16,
+	.poly_sc_down_max	= 4,
+	.pre_sc_down_max	= 4,
+	.local_sc_down		= 2,
+};
+
+static struct gsc_driverdata gsc_v_5250_drvdata = {
 	.variant = {
-		[0] = &gsc_v_100_variant,
-		[1] = &gsc_v_100_variant,
-		[2] = &gsc_v_100_variant,
-		[3] = &gsc_v_100_variant,
+		[0] = &gsc_v_5250_variant,
+		[1] = &gsc_v_5250_variant,
+		[2] = &gsc_v_5250_variant,
+		[3] = &gsc_v_5250_variant,
+	},
+	.num_entities = 4,
+	.clk_names = { "gscl" },
+	.num_clocks = 1,
+};
+
+static struct gsc_driverdata gsc_v_5420_drvdata = {
+	.variant = {
+		[0] = &gsc_v_5420_variant,
+		[1] = &gsc_v_5420_variant,
 	},
 	.num_entities = 4,
 	.clk_names = { "gscl" },
@@ -1006,9 +1072,9 @@ static irqreturn_t gsc_irq_handler(int irq, void *priv)
 
 static struct gsc_driverdata gsc_5433_drvdata = {
 	.variant = {
-		[0] = &gsc_v_100_variant,
-		[1] = &gsc_v_100_variant,
-		[2] = &gsc_v_100_variant,
+		[0] = &gsc_v_5433_variant,
+		[1] = &gsc_v_5433_variant,
+		[2] = &gsc_v_5433_variant,
 	},
 	.num_entities = 3,
 	.clk_names = { "pclk", "aclk", "aclk_xiu", "aclk_gsclbend" },
@@ -1017,8 +1083,12 @@ static irqreturn_t gsc_irq_handler(int irq, void *priv)
 
 static const struct of_device_id exynos_gsc_match[] = {
 	{
-		.compatible = "samsung,exynos5-gsc",
-		.data = &gsc_v_100_drvdata,
+		.compatible = "samsung,exynos5250-gsc",
+		.data = &gsc_v_5250_drvdata,
+	},
+	{
+		.compatible = "samsung,exynos5420-gsc",
+		.data = &gsc_v_5420_drvdata,
 	},
 	{
 		.compatible = "samsung,exynos5433-gsc",
-- 
1.9.1

Re: [PATCH v3 4/6] [media] exynos-gsc: Add hardware rotation limits

[Sylwester Nawrocki]

On 09/08/2017 08:02 AM, Hoegeun Kwon wrote:
> The hardware rotation limits of gsc depends on SOC (Exynos
> 5250/5420/5433). Distinguish them and add them to the driver data.
> 
> Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
> ---
>   drivers/media/platform/exynos-gsc/gsc-core.c | 96 ++++++++++++++++++++++++----
>   1 file changed, 83 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c
> index 4380150..8f8636e 100644
> --- a/drivers/media/platform/exynos-gsc/gsc-core.c
> +++ b/drivers/media/platform/exynos-gsc/gsc-core.c
> @@ -943,7 +943,37 @@ static irqreturn_t gsc_irq_handler(int irq, void *priv)
>   	return IRQ_HANDLED;
>   }
>   
> -static struct gsc_pix_max gsc_v_100_max = {
> +static struct gsc_pix_max gsc_v_5250_max = {
> +	.org_scaler_bypass_w	= 8192,
> +	.org_scaler_bypass_h	= 8192,
> +	.org_scaler_input_w	= 4800,
> +	.org_scaler_input_h	= 3344,
> +	.real_rot_dis_w		= 4800,
> +	.real_rot_dis_h		= 3344,
> +	.real_rot_en_w		= 2016,
> +	.real_rot_en_h		= 2016,
> +	.target_rot_dis_w	= 4800,
> +	.target_rot_dis_h	= 3344,
> +	.target_rot_en_w	= 2016,
> +	.target_rot_en_h	= 2016,
> +};
> +
> +static struct gsc_pix_max gsc_v_5420_max = {
> +	.org_scaler_bypass_w	= 8192,
> +	.org_scaler_bypass_h	= 8192,
> +	.org_scaler_input_w	= 4800,
> +	.org_scaler_input_h	= 3344,
> +	.real_rot_dis_w		= 4800,
> +	.real_rot_dis_h		= 3344,
> +	.real_rot_en_w		= 2048,
> +	.real_rot_en_h		= 2048,
> +	.target_rot_dis_w	= 4800,
> +	.target_rot_dis_h	= 3344,
> +	.target_rot_en_w	= 2016,
> +	.target_rot_en_h	= 2016,
> +};
> +
> +static struct gsc_pix_max gsc_v_5433_max = {
>   	.org_scaler_bypass_w	= 8192,
>   	.org_scaler_bypass_h	= 8192,
>   	.org_scaler_input_w	= 4800,
> @@ -979,8 +1009,8 @@ static irqreturn_t gsc_irq_handler(int irq, void *priv)
>   	.target_h		= 2,  /* yuv420 : 2, others : 1 */
>   };
>   
> -static struct gsc_variant gsc_v_100_variant = {
> -	.pix_max		= &gsc_v_100_max,
> +static struct gsc_variant gsc_v_5250_variant = {
> +	.pix_max		= &gsc_v_5250_max,
>   	.pix_min		= &gsc_v_100_min,
>   	.pix_align		= &gsc_v_100_align,
>   	.in_buf_cnt		= 32,
> @@ -992,12 +1022,48 @@ static irqreturn_t gsc_irq_handler(int irq, void *priv)
>   	.local_sc_down		= 2,
>   };
>   
> -static struct gsc_driverdata gsc_v_100_drvdata = {
> +static struct gsc_variant gsc_v_5420_variant = {
> +	.pix_max		= &gsc_v_5420_max,
> +	.pix_min		= &gsc_v_100_min,
> +	.pix_align		= &gsc_v_100_align,
> +	.in_buf_cnt		= 32,
> +	.out_buf_cnt		= 32,
> +	.sc_up_max		= 8,
> +	.sc_down_max		= 16,
> +	.poly_sc_down_max	= 4,
> +	.pre_sc_down_max	= 4,
> +	.local_sc_down		= 2,
> +};
> +
> +static struct gsc_variant gsc_v_5433_variant = {
> +	.pix_max		= &gsc_v_5433_max,
> +	.pix_min		= &gsc_v_100_min,
> +	.pix_align		= &gsc_v_100_align,
> +	.in_buf_cnt		= 32,
> +	.out_buf_cnt		= 32,
> +	.sc_up_max		= 8,
> +	.sc_down_max		= 16,
> +	.poly_sc_down_max	= 4,
> +	.pre_sc_down_max	= 4,
> +	.local_sc_down		= 2,
> +};
> +
> +static struct gsc_driverdata gsc_v_5250_drvdata = {
>   	.variant = {
> -		[0] = &gsc_v_100_variant,
> -		[1] = &gsc_v_100_variant,
> -		[2] = &gsc_v_100_variant,
> -		[3] = &gsc_v_100_variant,
> +		[0] = &gsc_v_5250_variant,
> +		[1] = &gsc_v_5250_variant,
> +		[2] = &gsc_v_5250_variant,
> +		[3] = &gsc_v_5250_variant,
> +	},
> +	.num_entities = 4,
> +	.clk_names = { "gscl" },
> +	.num_clocks = 1,
> +};
> +
> +static struct gsc_driverdata gsc_v_5420_drvdata = {
> +	.variant = {
> +		[0] = &gsc_v_5420_variant,
> +		[1] = &gsc_v_5420_variant,
>   	},
>   	.num_entities = 4,
>   	.clk_names = { "gscl" },
> @@ -1006,9 +1072,9 @@ static irqreturn_t gsc_irq_handler(int irq, void *priv)
>   
>   static struct gsc_driverdata gsc_5433_drvdata = {
>   	.variant = {
> -		[0] = &gsc_v_100_variant,
> -		[1] = &gsc_v_100_variant,
> -		[2] = &gsc_v_100_variant,
> +		[0] = &gsc_v_5433_variant,
> +		[1] = &gsc_v_5433_variant,
> +		[2] = &gsc_v_5433_variant,
>   	},
>   	.num_entities = 3,
>   	.clk_names = { "pclk", "aclk", "aclk_xiu", "aclk_gsclbend" },
> @@ -1017,8 +1083,12 @@ static irqreturn_t gsc_irq_handler(int irq, void *priv)
>   
>   static const struct of_device_id exynos_gsc_match[] = {
>   	{
> -		.compatible = "samsung,exynos5-gsc",
> -		.data = &gsc_v_100_drvdata,

Can you keep the "samsung,exynos5-gsc" entry with the gsc_v_5250_variant
data, so that it can work with "samsung,exynos5-gsc" compatible in DT
on both exynos5250 and exynos5420 SoCs?

> +		.compatible = "samsung,exynos5250-gsc",
> +		.data = &gsc_v_5250_drvdata,
> +	},
> +	{
> +		.compatible = "samsung,exynos5420-gsc",
> +		.data = &gsc_v_5420_drvdata,
>   	},

-- 
Regards,
Sylwester

Re: [PATCH v3 4/6] [media] exynos-gsc: Add hardware rotation limits

[Hoegeun Kwon]

Hi Sylwester,

On 09/11/2017 06:35 PM, Sylwester Nawrocki wrote:
> On 09/08/2017 08:02 AM, Hoegeun Kwon wrote:
>> The hardware rotation limits of gsc depends on SOC (Exynos
>> 5250/5420/5433). Distinguish them and add them to the driver data.
>>
>> Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
>> ---
>>   drivers/media/platform/exynos-gsc/gsc-core.c | 96 
>> ++++++++++++++++++++++++----
>>   1 file changed, 83 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c 
>> b/drivers/media/platform/exynos-gsc/gsc-core.c
>> index 4380150..8f8636e 100644
>> --- a/drivers/media/platform/exynos-gsc/gsc-core.c
>> +++ b/drivers/media/platform/exynos-gsc/gsc-core.c
>> @@ -943,7 +943,37 @@ static irqreturn_t gsc_irq_handler(int irq, void 
>> *priv)
>>       return IRQ_HANDLED;
>>   }
>>   -static struct gsc_pix_max gsc_v_100_max = {
>> +static struct gsc_pix_max gsc_v_5250_max = {
>> +    .org_scaler_bypass_w    = 8192,
>> +    .org_scaler_bypass_h    = 8192,
>> +    .org_scaler_input_w    = 4800,
>> +    .org_scaler_input_h    = 3344,
>> +    .real_rot_dis_w        = 4800,
>> +    .real_rot_dis_h        = 3344,
>> +    .real_rot_en_w        = 2016,
>> +    .real_rot_en_h        = 2016,
>> +    .target_rot_dis_w    = 4800,
>> +    .target_rot_dis_h    = 3344,
>> +    .target_rot_en_w    = 2016,
>> +    .target_rot_en_h    = 2016,
>> +};
>> +
>> +static struct gsc_pix_max gsc_v_5420_max = {
>> +    .org_scaler_bypass_w    = 8192,
>> +    .org_scaler_bypass_h    = 8192,
>> +    .org_scaler_input_w    = 4800,
>> +    .org_scaler_input_h    = 3344,
>> +    .real_rot_dis_w        = 4800,
>> +    .real_rot_dis_h        = 3344,
>> +    .real_rot_en_w        = 2048,
>> +    .real_rot_en_h        = 2048,
>> +    .target_rot_dis_w    = 4800,
>> +    .target_rot_dis_h    = 3344,
>> +    .target_rot_en_w    = 2016,
>> +    .target_rot_en_h    = 2016,
>> +};
>> +
>> +static struct gsc_pix_max gsc_v_5433_max = {
>>       .org_scaler_bypass_w    = 8192,
>>       .org_scaler_bypass_h    = 8192,
>>       .org_scaler_input_w    = 4800,
>> @@ -979,8 +1009,8 @@ static irqreturn_t gsc_irq_handler(int irq, void 
>> *priv)
>>       .target_h        = 2,  /* yuv420 : 2, others : 1 */
>>   };
>>   -static struct gsc_variant gsc_v_100_variant = {
>> -    .pix_max        = &gsc_v_100_max,
>> +static struct gsc_variant gsc_v_5250_variant = {
>> +    .pix_max        = &gsc_v_5250_max,
>>       .pix_min        = &gsc_v_100_min,
>>       .pix_align        = &gsc_v_100_align,
>>       .in_buf_cnt        = 32,
>> @@ -992,12 +1022,48 @@ static irqreturn_t gsc_irq_handler(int irq, 
>> void *priv)
>>       .local_sc_down        = 2,
>>   };
>>   -static struct gsc_driverdata gsc_v_100_drvdata = {
>> +static struct gsc_variant gsc_v_5420_variant = {
>> +    .pix_max        = &gsc_v_5420_max,
>> +    .pix_min        = &gsc_v_100_min,
>> +    .pix_align        = &gsc_v_100_align,
>> +    .in_buf_cnt        = 32,
>> +    .out_buf_cnt        = 32,
>> +    .sc_up_max        = 8,
>> +    .sc_down_max        = 16,
>> +    .poly_sc_down_max    = 4,
>> +    .pre_sc_down_max    = 4,
>> +    .local_sc_down        = 2,
>> +};
>> +
>> +static struct gsc_variant gsc_v_5433_variant = {
>> +    .pix_max        = &gsc_v_5433_max,
>> +    .pix_min        = &gsc_v_100_min,
>> +    .pix_align        = &gsc_v_100_align,
>> +    .in_buf_cnt        = 32,
>> +    .out_buf_cnt        = 32,
>> +    .sc_up_max        = 8,
>> +    .sc_down_max        = 16,
>> +    .poly_sc_down_max    = 4,
>> +    .pre_sc_down_max    = 4,
>> +    .local_sc_down        = 2,
>> +};
>> +
>> +static struct gsc_driverdata gsc_v_5250_drvdata = {
>>       .variant = {
>> -        [0] = &gsc_v_100_variant,
>> -        [1] = &gsc_v_100_variant,
>> -        [2] = &gsc_v_100_variant,
>> -        [3] = &gsc_v_100_variant,
>> +        [0] = &gsc_v_5250_variant,
>> +        [1] = &gsc_v_5250_variant,
>> +        [2] = &gsc_v_5250_variant,
>> +        [3] = &gsc_v_5250_variant,
>> +    },
>> +    .num_entities = 4,
>> +    .clk_names = { "gscl" },
>> +    .num_clocks = 1,
>> +};
>> +
>> +static struct gsc_driverdata gsc_v_5420_drvdata = {
>> +    .variant = {
>> +        [0] = &gsc_v_5420_variant,
>> +        [1] = &gsc_v_5420_variant,
>>       },
>>       .num_entities = 4,
>>       .clk_names = { "gscl" },
>> @@ -1006,9 +1072,9 @@ static irqreturn_t gsc_irq_handler(int irq, 
>> void *priv)
>>     static struct gsc_driverdata gsc_5433_drvdata = {
>>       .variant = {
>> -        [0] = &gsc_v_100_variant,
>> -        [1] = &gsc_v_100_variant,
>> -        [2] = &gsc_v_100_variant,
>> +        [0] = &gsc_v_5433_variant,
>> +        [1] = &gsc_v_5433_variant,
>> +        [2] = &gsc_v_5433_variant,
>>       },
>>       .num_entities = 3,
>>       .clk_names = { "pclk", "aclk", "aclk_xiu", "aclk_gsclbend" },
>> @@ -1017,8 +1083,12 @@ static irqreturn_t gsc_irq_handler(int irq, 
>> void *priv)
>>     static const struct of_device_id exynos_gsc_match[] = {
>>       {
>> -        .compatible = "samsung,exynos5-gsc",
>> -        .data = &gsc_v_100_drvdata,
>
> Can you keep the "samsung,exynos5-gsc" entry with the gsc_v_5250_variant
> data, so that it can work with "samsung,exynos5-gsc" compatible in DT
> on both exynos5250 and exynos5420 SoCs?
>

Thank you for your question.

Exynos 5250 and 5420 have different hardware rotation limits.
Exynos 5250 is '.real_rot_en_w/h = 2016' and 5420 is '.real_rot_en_w/h = 
2048'.

So my opinion they must have different compatible.

Best regards,
Hoegeun

>> +        .compatible = "samsung,exynos5250-gsc",
>> +        .data = &gsc_v_5250_drvdata,
>> +    },
>> +    {
>> +        .compatible = "samsung,exynos5420-gsc",
>> +        .data = &gsc_v_5420_drvdata,
>>       },
>

Re: [PATCH v3 4/6] [media] exynos-gsc: Add hardware rotation limits

[Sylwester Nawrocki]

Hi Hoegeun,

On 09/13/2017 04:33 AM, Hoegeun Kwon wrote:
>>> @@ -1017,8 +1083,12 @@ static irqreturn_t gsc_irq_handler(int irq,
>>> void *priv)
>>>      static const struct of_device_id exynos_gsc_match[] = {
>>>        {
>>> -        .compatible = "samsung,exynos5-gsc",
>>> -        .data = &gsc_v_100_drvdata,
>> Can you keep the "samsung,exynos5-gsc" entry with the gsc_v_5250_variant
>> data, so that it can work with "samsung,exynos5-gsc" compatible in DT
>> on both exynos5250 and exynos5420 SoCs?
>>
> Thank you for your question.
> 
> Exynos 5250 and 5420 have different hardware rotation limits.
> Exynos 5250 is '.real_rot_en_w/h = 2016' and 5420 is '.real_rot_en_w/h =
> 2048'.
> 
> So my opinion they must have different compatible.

I think there is some misunderstanding, mu suggestion was to keep the 
"samsung,exynos5-gsc" compatible entry in addition to the new introduced 
ones: "samsung,exynos5250-gsc" and "samsung,exynos5420-gsc". That's in
order to make your changes possibly backward compatible, in theory 
the updated driver should still work without changes in dts.

-- 
Regards,
Sylwester

Re: [PATCH v3 4/6] [media] exynos-gsc: Add hardware rotation limits

[Hoegeun Kwon]

On 09/13/2017 06:11 PM, Sylwester Nawrocki wrote:
> Hi Hoegeun,
>
> On 09/13/2017 04:33 AM, Hoegeun Kwon wrote:
>>>> @@ -1017,8 +1083,12 @@ static irqreturn_t gsc_irq_handler(int irq,
>>>> void *priv)
>>>>       static const struct of_device_id exynos_gsc_match[] = {
>>>>         {
>>>> -        .compatible = "samsung,exynos5-gsc",
>>>> -        .data = &gsc_v_100_drvdata,
>>> Can you keep the "samsung,exynos5-gsc" entry with the gsc_v_5250_variant
>>> data, so that it can work with "samsung,exynos5-gsc" compatible in DT
>>> on both exynos5250 and exynos5420 SoCs?
>>>
>> Thank you for your question.
>>
>> Exynos 5250 and 5420 have different hardware rotation limits.
>> Exynos 5250 is '.real_rot_en_w/h = 2016' and 5420 is '.real_rot_en_w/h =
>> 2048'.
>>
>> So my opinion they must have different compatible.
> I think there is some misunderstanding, mu suggestion was to keep the
> "samsung,exynos5-gsc" compatible entry in addition to the new introduced
> ones: "samsung,exynos5250-gsc" and "samsung,exynos5420-gsc". That's in
> order to make your changes possibly backward compatible, in theory
> the updated driver should still work without changes in dts.


Thank you again for your explanation.

Yes, I understood.
I will keep "samsung,exynos5-gsc" compatible,
and add Exynos 5250/5420/5433 compatible.

Best regards,
Hoegeun

[PATCH v3 5/6] [media] exynos-gsc: Remove unnecessary compatible

[Hoegeun Kwon]

Currently, the compatible('samsung,exynos5-gsc') is not used.
Remove unnecessary compatible.

Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
---
 Documentation/devicetree/bindings/media/exynos5-gsc.txt | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/exynos5-gsc.txt b/Documentation/devicetree/bindings/media/exynos5-gsc.txt
index daa56d5..1ea05f1 100644
--- a/Documentation/devicetree/bindings/media/exynos5-gsc.txt
+++ b/Documentation/devicetree/bindings/media/exynos5-gsc.txt
@@ -3,9 +3,9 @@
 G-Scaler is used for scaling and color space conversion on EXYNOS5 SoCs.
 
 Required properties:
-- compatible: should be "samsung,exynos5-gsc" (for Exynos 5250, 5420 and
-	      5422 SoCs) or "samsung,exynos5433-gsc" (Exynos 5433)
-	      or "samsung,exynos5250-gsc" or "samsung,exynos5420-gsc"
+- compatible: should be "samsung,exynos5250-gsc", "samsung,exynos5420-gsc"
+	      or "samsung,exynos5433-gsc" (for Exynos 5250, 5420, 5422,
+	      and 5433 SoCs)
 - reg: should contain G-Scaler physical address location and length.
 - interrupts: should contain G-Scaler interrupt number
 
@@ -16,7 +16,7 @@ Optional properties:
 Example:
 
 gsc_0:  gsc@0x13e00000 {
-	compatible = "samsung,exynos5-gsc";
+	compatible = "samsung,exynos5250-gsc;
 	reg = <0x13e00000 0x1000>;
 	interrupts = <0 85 0>;
 };
-- 
1.9.1

Re: [PATCH v3 5/6] [media] exynos-gsc: Remove unnecessary compatible

[Krzysztof Kozlowski]

On Fri, Sep 8, 2017 at 8:02 AM, Hoegeun Kwon <hoegeun.kwon@samsung.com> wrote:
> Currently, the compatible('samsung,exynos5-gsc') is not used.
> Remove unnecessary compatible.
>
> Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
> ---
>  Documentation/devicetree/bindings/media/exynos5-gsc.txt | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/exynos5-gsc.txt b/Documentation/devicetree/bindings/media/exynos5-gsc.txt
> index daa56d5..1ea05f1 100644
> --- a/Documentation/devicetree/bindings/media/exynos5-gsc.txt
> +++ b/Documentation/devicetree/bindings/media/exynos5-gsc.txt
> @@ -3,9 +3,9 @@
>  G-Scaler is used for scaling and color space conversion on EXYNOS5 SoCs.
>
>  Required properties:
> -- compatible: should be "samsung,exynos5-gsc" (for Exynos 5250, 5420 and
> -             5422 SoCs) or "samsung,exynos5433-gsc" (Exynos 5433)
> -             or "samsung,exynos5250-gsc" or "samsung,exynos5420-gsc"
> +- compatible: should be "samsung,exynos5250-gsc", "samsung,exynos5420-gsc"
> +             or "samsung,exynos5433-gsc" (for Exynos 5250, 5420, 5422,
> +             and 5433 SoCs)

You should not remove the compatible (neither here nor in the driver)
but mark it as deprecated. Removal should happen slightly later.

Best regards,
Krzysztof

>  - reg: should contain G-Scaler physical address location and length.
>  - interrupts: should contain G-Scaler interrupt number
>
> @@ -16,7 +16,7 @@ Optional properties:
>  Example:
>
>  gsc_0:  gsc@0x13e00000 {
> -       compatible = "samsung,exynos5-gsc";
> +       compatible = "samsung,exynos5250-gsc;
>         reg = <0x13e00000 0x1000>;
>         interrupts = <0 85 0>;
>  };

[PATCH v3 6/6] ARM: dts: exynos: Remove unnecessary compatible

[Hoegeun Kwon]

Currently, the compatible('samsung,exynos5-gsc') is not used.
Remove unnecessary compatible.

Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
---
 arch/arm/boot/dts/exynos5250.dtsi | 8 ++++----
 arch/arm/boot/dts/exynos5420.dtsi | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index bf08101..e632faf 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -637,7 +637,7 @@
 		};
 
 		gsc_0:  gsc@13e00000 {
-			compatible = "samsung,exynos5-gsc", "samsung,exynos5250-gsc";
+			compatible = "samsung,exynos5250-gsc";
 			reg = <0x13e00000 0x1000>;
 			interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>;
 			power-domains = <&pd_gsc>;
@@ -647,7 +647,7 @@
 		};
 
 		gsc_1:  gsc@13e10000 {
-			compatible = "samsung,exynos5-gsc", "samsung,exynos5250-gsc";
+			compatible = "samsung,exynos5250-gsc";
 			reg = <0x13e10000 0x1000>;
 			interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
 			power-domains = <&pd_gsc>;
@@ -657,7 +657,7 @@
 		};
 
 		gsc_2:  gsc@13e20000 {
-			compatible = "samsung,exynos5-gsc", "samsung,exynos5250-gsc";
+			compatible = "samsung,exynos5250-gsc";
 			reg = <0x13e20000 0x1000>;
 			interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
 			power-domains = <&pd_gsc>;
@@ -667,7 +667,7 @@
 		};
 
 		gsc_3:  gsc@13e30000 {
-			compatible = "samsung,exynos5-gsc", "samsung,exynos5250-gsc";
+			compatible = "samsung,exynos5250-gsc";
 			reg = <0x13e30000 0x1000>;
 			interrupts = <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>;
 			power-domains = <&pd_gsc>;
diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index 86afe77..58392b3 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -658,7 +658,7 @@
 		};
 
 		gsc_0: video-scaler@13e00000 {
-			compatible = "samsung,exynos5-gsc", "samsung,exynos5420-gsc";
+			compatible = "samsung,exynos5420-gsc";
 			reg = <0x13e00000 0x1000>;
 			interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&clock CLK_GSCL0>;
@@ -668,7 +668,7 @@
 		};
 
 		gsc_1: video-scaler@13e10000 {
-			compatible = "samsung,exynos5-gsc", "samsung,exynos5420-gsc";
+			compatible = "samsung,exynos5420-gsc";
 			reg = <0x13e10000 0x1000>;
 			interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&clock CLK_GSCL1>;
-- 
1.9.1