| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-id: <f9ed590f-2e2d-8c9b-8c5f-e27eb8e3f39e@samsung.com>
Date: Fri, 08 Sep 2017 11:21:10 +0900
From: Hoegeun Kwon <hoegeun.kwon@...sung.com>
To: Marek Szyprowski <m.szyprowski@...sung.com>,
Krzysztof Kozlowski <krzk@...nel.org>,
Inki Dae <inki.dae@...sung.com>, sw0312.kim@...sung.com,
airlied@...ux.ie, kgene@...nel.org, robh+dt@...nel.org,
mark.rutland@....com, catalin.marinas@....com, will.deacon@....com
Cc: dri-devel@...ts.freedesktop.org,
linux-arm-kernel@...ts.infradead.org,
linux-samsung-soc@...r.kernel.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, Hoegeun Kwon <hoegeun.kwon@...sung.com>
Subject: Re: [PATCH 3/3] drm/exynos/gsc: Add rotation hardware limits of
gscaler
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@...sung.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
Powered by blists - more mailing lists