Messages in this thread | | | Subject | Re: [PATCH 3/3] drm/exynos/gsc: Add rotation hardware limits of gscaler | From | Hoegeun Kwon <> | Date | Fri, 08 Sep 2017 11:21:10 +0900 |
| |
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
| |