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

From: Marek Szyprowski
Date: Thu Sep 07 2017 - 07:25:34 EST

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@xxxxxxxxxxx>
---
 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