Messages in this thread | | | Subject | Re: [PATCH 3/3] drm/exynos/gsc: Add rotation hardware limits of gscaler | From | Marek Szyprowski <> | Date | Thu, 07 Sep 2017 13:25:22 +0200 |
| |
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
| |