[PATCH 0/3] drm/exynos/gsc: Support the rotate hardware limits of gsc

[PATCH 0/3] drm/exynos/gsc: Support the rotate hardware limits of gsc

[Hoegeun Kwon]

Hello all,

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

Best regards,
Hoegeun

Hoegeun Kwon (3):
  ARM: dts: exynos: Add the hardware rotation limits for gsc
  arm64: dts: exynos: Add the hardware rotation limits for gsc
  drm/exynos/gsc: Add rotation hardware limits of gscaler

 arch/arm/boot/dts/exynos5250.dtsi          |  8 ++++
 arch/arm/boot/dts/exynos5420.dtsi          |  4 ++
 arch/arm64/boot/dts/exynos/exynos5433.dtsi |  6 +++
 drivers/gpu/drm/exynos/exynos_drm_gsc.c    | 63 +++++++++++++++++++-----------
 include/uapi/drm/exynos_drm.h              |  2 +
 5 files changed, 60 insertions(+), 23 deletions(-)

-- 
1.9.1

[PATCH 1/3] ARM: dts: exynos: Add the hardware rotation limits for gsc

[Hoegeun Kwon]

The gscaler has maximum size of input or output rotation. Add the
hardware limits property for the gscaler rotation in the exynos5250
and exynos5420 dts.

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, 12 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 8dbeb87..d9434e0 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -643,6 +643,8 @@
 			power-domains = <&pd_gsc>;
 			clocks = <&clock CLK_GSCL0>;
 			clock-names = "gscl";
+			rot-max-hsize = <2048>;
+			rot-max-vsize = <2048>;
 			iommu = <&sysmmu_gsc0>;
 		};
 
@@ -653,6 +655,8 @@
 			power-domains = <&pd_gsc>;
 			clocks = <&clock CLK_GSCL1>;
 			clock-names = "gscl";
+			rot-max-hsize = <2048>;
+			rot-max-vsize = <2048>;
 			iommu = <&sysmmu_gsc1>;
 		};
 
@@ -663,6 +667,8 @@
 			power-domains = <&pd_gsc>;
 			clocks = <&clock CLK_GSCL2>;
 			clock-names = "gscl";
+			rot-max-hsize = <2048>;
+			rot-max-vsize = <2048>;
 			iommu = <&sysmmu_gsc2>;
 		};
 
@@ -673,6 +679,8 @@
 			power-domains = <&pd_gsc>;
 			clocks = <&clock CLK_GSCL3>;
 			clock-names = "gscl";
+			rot-max-hsize = <2048>;
+			rot-max-vsize = <2048>;
 			iommu = <&sysmmu_gsc3>;
 		};
 
diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index 02d2f89..0fd6be9 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -664,6 +664,8 @@
 			clocks = <&clock CLK_GSCL0>;
 			clock-names = "gscl";
 			power-domains = <&gsc_pd>;
+			rot-max-hsize = <2016>;
+			rot-max-vsize = <2016>;
 			iommus = <&sysmmu_gscl0>;
 		};
 
@@ -674,6 +676,8 @@
 			clocks = <&clock CLK_GSCL1>;
 			clock-names = "gscl";
 			power-domains = <&gsc_pd>;
+			rot-max-hsize = <2016>;
+			rot-max-vsize = <2016>;
 			iommus = <&sysmmu_gscl1>;
 		};
 
-- 
1.9.1

[PATCH 2/3] arm64: dts: exynos: Add the hardware rotation limits for gsc

[Hoegeun Kwon]

The gscaler has maximum size of input or output rotation. Add the
hardware limits property for the gscaler rotation in the exynos5433
dts.

Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
---
 arch/arm64/boot/dts/exynos/exynos5433.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
index 7fe994b..57c9997 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
@@ -891,6 +891,8 @@
 				 <&cmu_gscl CLK_ACLK_GSCL0>,
 				 <&cmu_gscl CLK_ACLK_XIU_GSCLX>,
 				 <&cmu_gscl CLK_ACLK_GSCLBEND_333>;
+			rot-max-hsize = <2047>;
+			rot-max-vsize = <2047>;
 			iommus = <&sysmmu_gscl0>;
 		};
 
@@ -904,6 +906,8 @@
 				 <&cmu_gscl CLK_ACLK_GSCL1>,
 				 <&cmu_gscl CLK_ACLK_XIU_GSCLX>,
 				 <&cmu_gscl CLK_ACLK_GSCLBEND_333>;
+			rot-max-hsize = <2047>;
+			rot-max-vsize = <2047>;
 			iommus = <&sysmmu_gscl1>;
 		};
 
@@ -917,6 +921,8 @@
 				 <&cmu_gscl CLK_ACLK_GSCL2>,
 				 <&cmu_gscl CLK_ACLK_XIU_GSCLX>,
 				 <&cmu_gscl CLK_ACLK_GSCLBEND_333>;
+			rot-max-hsize = <2047>;
+			rot-max-vsize = <2047>;
 			iommus = <&sysmmu_gscl2>;
 		};
 
-- 
1.9.1

[PATCH 3/3] drm/exynos/gsc: Add rotation hardware limits of gscaler

[Hoegeun Kwon]

The gscaler has hardware rotation limits that need to be imported from
dts. Parse them and add them to the property list.

The rotation hardware 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 post 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 | 63 +++++++++++++++++++++------------
 include/uapi/drm/exynos_drm.h           |  2 ++
 2 files changed, 42 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
index 0506b2b..dd9b057 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
@@ -1401,6 +1401,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 +1433,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 +1440,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 +1471,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;
 				}
@@ -1676,6 +1684,15 @@ static int gsc_probe(struct platform_device *pdev)
 			dev_warn(dev, "failed to get system register.\n");
 			ctx->sysreg = NULL;
 		}
+
+		ret = of_property_read_u32(dev->of_node, "rot-max-hsize",
+				&ctx->ippdrv.prop_list.rot_max.hsize);
+		ret |= of_property_read_u32(dev->of_node, "rot-max-vsize",
+				&ctx->ippdrv.prop_list.rot_max.vsize);
+		if (ret) {
+			dev_err(dev, "rot-max property should be provided by device tree.\n");
+			return -EINVAL;
+		}
 	}
 
 	/* clock control */
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 3/3] drm/exynos/gsc: Add rotation hardware limits of gscaler

[Krzysztof Kozlowski]

On Fri, Sep 1, 2017 at 3:47 AM, Hoegeun Kwon <hoegeun.kwon@samsung.com> wrote:
> The gscaler has hardware rotation limits that need to be imported from
> dts. Parse them and add them to the property list.
>
> The rotation hardware 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 post 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 | 63 +++++++++++++++++++++------------
>  include/uapi/drm/exynos_drm.h           |  2 ++
>  2 files changed, 42 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
> index 0506b2b..dd9b057 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
> @@ -1401,6 +1401,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 +1433,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 +1440,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 +1471,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;
>                                 }
> @@ -1676,6 +1684,15 @@ static int gsc_probe(struct platform_device *pdev)
>                         dev_warn(dev, "failed to get system register.\n");
>                         ctx->sysreg = NULL;
>                 }
> +
> +               ret = of_property_read_u32(dev->of_node, "rot-max-hsize",
> +                               &ctx->ippdrv.prop_list.rot_max.hsize);
> +               ret |= of_property_read_u32(dev->of_node, "rot-max-vsize",
> +                               &ctx->ippdrv.prop_list.rot_max.vsize);
> +               if (ret) {
> +                       dev_err(dev, "rot-max property should be provided by device tree.\n");
> +                       return -EINVAL;
> +               }

You're adding a required property thus breaking the ABI without
explicit reason. Also the bindings have to be updated.

Best regards,
Krzysztof

Re: [PATCH 3/3] drm/exynos/gsc: Add rotation hardware limits of gscaler

[Marek Szyprowski]

Hi Hoegeun,

On 2017-09-01 03:47, Hoegeun Kwon wrote:
> The gscaler has hardware rotation limits that need to be imported from
> dts. Parse them and add them to the property list.
>
> The rotation hardware 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 post 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 | 63 +++++++++++++++++++++------------
>   include/uapi/drm/exynos_drm.h           |  2 ++
>   2 files changed, 42 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
> index 0506b2b..dd9b057 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
> @@ -1401,6 +1401,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 +1433,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 +1440,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 +1471,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;
>   				}
> @@ -1676,6 +1684,15 @@ static int gsc_probe(struct platform_device *pdev)
>   			dev_warn(dev, "failed to get system register.\n");
>   			ctx->sysreg = NULL;
>   		}
> +
> +		ret = of_property_read_u32(dev->of_node, "rot-max-hsize",
> +				&ctx->ippdrv.prop_list.rot_max.hsize);
> +		ret |= of_property_read_u32(dev->of_node, "rot-max-vsize",
> +				&ctx->ippdrv.prop_list.rot_max.vsize);
> +		if (ret) {
> +			dev_err(dev, "rot-max property should be provided by device tree.\n");
> +			return -EINVAL;
> +		}
>   	}
>   
>   	/* clock control */
> 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;
>   };
>   
>   /**

IMO maximum supported picture size should be hardcoded into driver, there
is no need to add device tree properties for that. Please also check v4l2
driver for Exynos GSC.

Currently it uses only one compatible - "exynos5-gsc", but imho You should
simply replace it with "exynos5250-gsc" and "exynos5420-gsc", and add those
variants with proper maximum supported size (2047 and 2016 respectively).

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

Re: [PATCH 3/3] drm/exynos/gsc: Add rotation hardware limits of gscaler

[Hoegeun Kwon]

On 09/01/2017 04:31 PM, Marek Szyprowski wrote:
> Hi Hoegeun,
>
> On 2017-09-01 03:47, Hoegeun Kwon wrote:
>> The gscaler has hardware rotation limits that need to be imported from
>> dts. Parse them and add them to the property list.
>>
>> The rotation hardware 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 post 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 | 63 
>> +++++++++++++++++++++------------
>>   include/uapi/drm/exynos_drm.h           |  2 ++
>>   2 files changed, 42 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c 
>> b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
>> index 0506b2b..dd9b057 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
>> @@ -1401,6 +1401,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 +1433,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 +1440,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 +1471,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;
>>                   }
>> @@ -1676,6 +1684,15 @@ static int gsc_probe(struct platform_device 
>> *pdev)
>>               dev_warn(dev, "failed to get system register.\n");
>>               ctx->sysreg = NULL;
>>           }
>> +
>> +        ret = of_property_read_u32(dev->of_node, "rot-max-hsize",
>> +                &ctx->ippdrv.prop_list.rot_max.hsize);
>> +        ret |= of_property_read_u32(dev->of_node, "rot-max-vsize",
>> +                &ctx->ippdrv.prop_list.rot_max.vsize);
>> +        if (ret) {
>> +            dev_err(dev, "rot-max property should be provided by 
>> device tree.\n");
>> +            return -EINVAL;
>> +        }
>>       }
>>         /* clock control */
>> 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;
>>   };
>>     /**
>
> IMO maximum supported picture size should be hardcoded into driver, there
> is no need to add device tree properties for that. Please also check v4l2
> driver for Exynos GSC.
>
> Currently it uses only one compatible - "exynos5-gsc", but imho You 
> should
> simply replace it with "exynos5250-gsc" and "exynos5420-gsc", and add 
> those
> variants with proper maximum supported size (2047 and 2016 respectively).
>
> Best regards

Hi Krzysztof and Marek,

Thanks Krzysztof and Marek reviews.

As Marek says, rot_max size will be hardcoded into driver,
then it will not break the ABI. And also,
I will check the v4l2 driver for Exynos GSC.

Best regards,
Hoegeun

Re: [PATCH 3/3] drm/exynos/gsc: Add rotation hardware limits of gscaler

[Hoegeun Kwon]

On 09/04/2017 03:19 PM, Hoegeun Kwon wrote:
> On 09/01/2017 04:31 PM, Marek Szyprowski wrote:
>> Hi Hoegeun,
>>
>> On 2017-09-01 03:47, Hoegeun Kwon wrote:
>>> The gscaler has hardware rotation limits that need to be imported from
>>> dts. Parse them and add them to the property list.
>>>
>>> The rotation hardware 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 post 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 | 63 
>>> +++++++++++++++++++++------------
>>>   include/uapi/drm/exynos_drm.h           |  2 ++
>>>   2 files changed, 42 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c 
>>> b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
>>> index 0506b2b..dd9b057 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
>>> @@ -1401,6 +1401,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 +1433,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 +1440,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 +1471,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;
>>>                   }
>>> @@ -1676,6 +1684,15 @@ static int gsc_probe(struct platform_device 
>>> *pdev)
>>>               dev_warn(dev, "failed to get system register.\n");
>>>               ctx->sysreg = NULL;
>>>           }
>>> +
>>> +        ret = of_property_read_u32(dev->of_node, "rot-max-hsize",
>>> +                &ctx->ippdrv.prop_list.rot_max.hsize);
>>> +        ret |= of_property_read_u32(dev->of_node, "rot-max-vsize",
>>> +                &ctx->ippdrv.prop_list.rot_max.vsize);
>>> +        if (ret) {
>>> +            dev_err(dev, "rot-max property should be provided by 
>>> device tree.\n");
>>> +            return -EINVAL;
>>> +        }
>>>       }
>>>         /* clock control */
>>> 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;
>>>   };
>>>     /**
>>
>> IMO maximum supported picture size should be hardcoded into driver, 
>> there
>> is no need to add device tree properties for that. Please also check 
>> v4l2
>> driver for Exynos GSC.
>>
>> Currently it uses only one compatible - "exynos5-gsc", but imho You 
>> should
>> simply replace it with "exynos5250-gsc" and "exynos5420-gsc", and add 
>> those
>> variants with proper maximum supported size (2047 and 2016 
>> respectively).
>>
>> Best regards
>
> Hi Krzysztof and Marek,
>
> Thanks Krzysztof and Marek reviews.
>
> As Marek says, rot_max size will be hardcoded into driver,
> then it will not break the ABI. And also,
> I will check the v4l2 driver for Exynos GSC.
>
> Best regards,
> Hoegeun
>

Hi Marek,

I have checked v4l2 driver for Exynos GSC. The v4l2 driver supports
Exynos 5250 and 5433 GSC. Currently, the hardware limits rotation is
set to 2047 in the v4l2 driver.

In my opinion don't need to fix it, because the Exynos 5250 has a
hardware rotation limits of 2048 and the Exynos 5250 has a hardware
rotation limits of 2047.

Please tell me if you have any other opinion.

Best regards,
Hoegeun

Re: [PATCH 3/3] drm/exynos/gsc: Add rotation hardware limits of gscaler

[Marek Szyprowski]

Hi Hoegeun,

On 2017-09-07 07:16, Hoegeun Kwon wrote:
> On 09/04/2017 03:19 PM, Hoegeun Kwon wrote:
>> On 09/01/2017 04:31 PM, Marek Szyprowski wrote:
>>> Hi Hoegeun,
>>>
>>> On 2017-09-01 03:47, Hoegeun Kwon wrote:
>>>> The gscaler has hardware rotation limits that need to be imported from
>>>> dts. Parse them and add them to the property list.
>>>>
>>>> The rotation hardware 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 post 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 | 63 
>>>> +++++++++++++++++++++------------
>>>>   include/uapi/drm/exynos_drm.h           |  2 ++
>>>>   2 files changed, 42 insertions(+), 23 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c 
>>>> b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
>>>> index 0506b2b..dd9b057 100644
>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c
>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
>>>> @@ -1401,6 +1401,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 +1433,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 +1440,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 +1471,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;
>>>>                   }
>>>> @@ -1676,6 +1684,15 @@ static int gsc_probe(struct platform_device 
>>>> *pdev)
>>>>               dev_warn(dev, "failed to get system register.\n");
>>>>               ctx->sysreg = NULL;
>>>>           }
>>>> +
>>>> +        ret = of_property_read_u32(dev->of_node, "rot-max-hsize",
>>>> + &ctx->ippdrv.prop_list.rot_max.hsize);
>>>> +        ret |= of_property_read_u32(dev->of_node, "rot-max-vsize",
>>>> + &ctx->ippdrv.prop_list.rot_max.vsize);
>>>> +        if (ret) {
>>>> +            dev_err(dev, "rot-max property should be provided by 
>>>> device tree.\n");
>>>> +            return -EINVAL;
>>>> +        }
>>>>       }
>>>>         /* clock control */
>>>> 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;
>>>>   };
>>>>     /**
>>>
>>> IMO maximum supported picture size should be hardcoded into driver, 
>>> there
>>> is no need to add device tree properties for that. Please also check 
>>> v4l2
>>> driver for Exynos GSC.
>>>
>>> Currently it uses only one compatible - "exynos5-gsc", but imho You 
>>> should
>>> simply replace it with "exynos5250-gsc" and "exynos5420-gsc", and 
>>> add those
>>> variants with proper maximum supported size (2047 and 2016 
>>> respectively).
>>>
>>> Best regards
>>
>> Hi Krzysztof and Marek,
>>
>> Thanks Krzysztof and Marek reviews.
>>
>> As Marek says, rot_max size will be hardcoded into driver,
>> then it will not break the ABI. And also,
>> I will check the v4l2 driver for Exynos GSC.
>>
>> Best regards,
>> Hoegeun
>>
>
> Hi Marek,
>
> I have checked v4l2 driver for Exynos GSC. The v4l2 driver supports
> Exynos 5250 and 5433 GSC. Currently, the hardware limits rotation is
> set to 2047 in the v4l2 driver.
>

V4l2 GSC driver also supports Exynos5420/5422 SoCs, see
# git grep "samsung,exynos5-gsc" arch/arm/boot/dts/

> In my opinion don't need to fix it, because the Exynos 5250 has a
> hardware rotation limits of 2048 and the Exynos 5250 has a hardware
> rotation limits of 2047.

Like you pointed earlier, Exynos 5420/5422/5800 supports rotation only
up to 2016x2016, so additional patch for v4l2 is also needed.

>
> Please tell me if you have any other opinion.
>

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

Re: [PATCH 3/3] drm/exynos/gsc: Add rotation hardware limits of gscaler

[Hoegeun Kwon]

On 09/07/2017 08:25 PM, Marek Szyprowski wrote:
> Hi Hoegeun,
>
> On 2017-09-07 07:16, Hoegeun Kwon wrote:
>> On 09/04/2017 03:19 PM, Hoegeun Kwon wrote:
>>> On 09/01/2017 04:31 PM, Marek Szyprowski wrote:
>>>> Hi Hoegeun,
>>>>
>>>> On 2017-09-01 03:47, Hoegeun Kwon wrote:
>>>>> The gscaler has hardware rotation limits that need to be imported 
>>>>> from
>>>>> dts. Parse them and add them to the property list.
>>>>>
>>>>> The rotation hardware 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 post 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 | 63 
>>>>> +++++++++++++++++++++------------
>>>>>   include/uapi/drm/exynos_drm.h           |  2 ++
>>>>>   2 files changed, 42 insertions(+), 23 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c 
>>>>> b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
>>>>> index 0506b2b..dd9b057 100644
>>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c
>>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
>>>>> @@ -1401,6 +1401,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 +1433,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 +1440,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 +1471,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;
>>>>>                   }
>>>>> @@ -1676,6 +1684,15 @@ static int gsc_probe(struct platform_device 
>>>>> *pdev)
>>>>>               dev_warn(dev, "failed to get system register.\n");
>>>>>               ctx->sysreg = NULL;
>>>>>           }
>>>>> +
>>>>> +        ret = of_property_read_u32(dev->of_node, "rot-max-hsize",
>>>>> + &ctx->ippdrv.prop_list.rot_max.hsize);
>>>>> +        ret |= of_property_read_u32(dev->of_node, "rot-max-vsize",
>>>>> + &ctx->ippdrv.prop_list.rot_max.vsize);
>>>>> +        if (ret) {
>>>>> +            dev_err(dev, "rot-max property should be provided by 
>>>>> device tree.\n");
>>>>> +            return -EINVAL;
>>>>> +        }
>>>>>       }
>>>>>         /* clock control */
>>>>> 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;
>>>>>   };
>>>>>     /**
>>>>
>>>> IMO maximum supported picture size should be hardcoded into driver, 
>>>> there
>>>> is no need to add device tree properties for that. Please also 
>>>> check v4l2
>>>> driver for Exynos GSC.
>>>>
>>>> Currently it uses only one compatible - "exynos5-gsc", but imho You 
>>>> should
>>>> simply replace it with "exynos5250-gsc" and "exynos5420-gsc", and 
>>>> add those
>>>> variants with proper maximum supported size (2047 and 2016 
>>>> respectively).
>>>>
>>>> Best regards
>>>
>>> Hi Krzysztof and Marek,
>>>
>>> Thanks Krzysztof and Marek reviews.
>>>
>>> As Marek says, rot_max size will be hardcoded into driver,
>>> then it will not break the ABI. And also,
>>> I will check the v4l2 driver for Exynos GSC.
>>>
>>> Best regards,
>>> Hoegeun
>>>
>>
>> Hi Marek,
>>
>> I have checked v4l2 driver for Exynos GSC. The v4l2 driver supports
>> Exynos 5250 and 5433 GSC. Currently, the hardware limits rotation is
>> set to 2047 in the v4l2 driver.
>>
>
> V4l2 GSC driver also supports Exynos5420/5422 SoCs, see
> # git grep "samsung,exynos5-gsc" arch/arm/boot/dts/
>
>> In my opinion don't need to fix it, because the Exynos 5250 has a
>> hardware rotation limits of 2048 and the Exynos 5250 has a hardware
>> rotation limits of 2047.
>
> Like you pointed earlier, Exynos 5420/5422/5800 supports rotation only
> up to 2016x2016, so additional patch for v4l2 is also needed.

Ok, I mistook it as not supporting the Exynos5420.
So, I will also upload patches related to v4l2 from patch ver3.

Best regards,
Hoegeun

>
>>
>> Please tell me if you have any other opinion.
>>
>
> Best regards