This patch changes userspace parameter but Kernel driver shouldn't touch such 
parameter.
Please ignore this patch. Marek will post a generic solution soon, which fixes 
the image broken issue without touching userspace parameter.

Thanks,
Inki Dae


2018년 05월 24일 10:04에 Inki Dae 이(가) 쓴 글:
> Fixed image broken issue while video play back with 854x480.
> 
> GScaler device of Exynos5433 has IN_ID_MAX field in GSC_IN_CON
> register, which determines how much GScaler DMA has to drain
> data from system memory at once.
> 
> Therefore, image size should be fixed up before IPP driver verifies
> whether gem buffer is enough for it or not.
> 
> Changelog v2:
> - Fix align limit of image width size to 16bytes because with other values
>   , 4 and 8bytes, it doesn't work.
> - Change the function name from gst_get_align_size to gst_set_align_limit.
>   gst_set_align_limit function name is more reasonable.
> - Call fixup buffer function before calling size limit check.
> - Remove checking align of image offset, x and y. The offest values have
>   no any limit but only size.
> 
> Signed-off-by: Inki Dae <inki....@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_gsc.c | 94 
> +++++++++++++++++++++++++++------
>  drivers/gpu/drm/exynos/exynos_drm_ipp.c | 18 +++++--
>  drivers/gpu/drm/exynos/exynos_drm_ipp.h | 12 +++++
>  3 files changed, 106 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c 
> b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
> index e99dd1e..8bc31b5 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
> @@ -86,6 +86,28 @@ struct gsc_scaler {
>       unsigned long main_vratio;
>  };
>  
> +/**
> + * struct gsc_driverdata - per device type driver data for init time.
> + *
> + * @limits: picture size limits array
> + * @clk_names: names of clocks needed by this variant
> + * @num_clocks: the number of clocks needed by this variant
> + * @has_in_ic_max: indicate whether GSCALER_IN_CON register has
> + *                   IN_IC_MAX field which means maxinum AXI
> + *                   read capability.
> + * @in_ic_max_shift: indicate which position IN_IC_MAX field is located.
> + * @in_ic_max_mask: A mask value to IN_IC_MAX field.
> + */
> +struct gsc_driverdata {
> +     const struct drm_exynos_ipp_limit *limits;
> +     int             num_limits;
> +     const char      *clk_names[GSC_MAX_CLOCKS];
> +     int             num_clocks;
> +     unsigned int    has_in_ic_max:1;
> +     unsigned int    in_ic_max_shift;
> +     unsigned int    in_ic_max_mask;
> +};
> +
>  /*
>   * A structure of gsc context.
>   *
> @@ -96,6 +118,9 @@ struct gsc_scaler {
>   * @id: gsc id.
>   * @irq: irq number.
>   * @rotation: supports rotation of src.
> + * @align_size: A size that GSC_SRCIMG_WIDTH value of GSC_SRCIMG_SIZE
> + *           register should be aligned with(in byte).
> + * @driver_data: a pointer to gsc_driverdata.
>   */
>  struct gsc_context {
>       struct exynos_drm_ipp ipp;
> @@ -114,20 +139,8 @@ struct gsc_context {
>       int     id;
>       int     irq;
>       bool    rotation;
> -};
> -
> -/**
> - * struct gsc_driverdata - per device type driver data for init time.
> - *
> - * @limits: picture size limits array
> - * @clk_names: names of clocks needed by this variant
> - * @num_clocks: the number of clocks needed by this variant
> - */
> -struct gsc_driverdata {
> -     const struct drm_exynos_ipp_limit *limits;
> -     int             num_limits;
> -     const char      *clk_names[GSC_MAX_CLOCKS];
> -     int             num_clocks;
> +     unsigned int align_size;
> +     struct gsc_driverdata *driver_data;
>  };
>  
>  /* 8-tap Filter Coefficient */
> @@ -1095,6 +1108,15 @@ static void gsc_start(struct gsc_context *ctx)
>       gsc_write(cfg, GSC_ENABLE);
>  }
>  
> +static void gsc_fixup(struct exynos_drm_ipp *ipp,
> +                       struct exynos_drm_ipp_task *task)
> +{
> +     struct gsc_context *ctx = container_of(ipp, struct gsc_context, ipp);
> +     struct exynos_drm_ipp_buffer *src = &task->src;
> +
> +     src->buf.width = ALIGN(src->buf.width, ctx->align_size);
> +}
> +
>  static int gsc_commit(struct exynos_drm_ipp *ipp,
>                         struct exynos_drm_ipp_task *task)
>  {
> @@ -1124,6 +1146,41 @@ static int gsc_commit(struct exynos_drm_ipp *ipp,
>       return 0;
>  }
>  
> +static void gsc_set_align_limit(struct gsc_context *ctx)
> +{
> +     const struct drm_exynos_ipp_limit *limits = ctx->driver_data->limits;
> +
> +     if (ctx->driver_data->has_in_ic_max) {
> +             u32 cfg, mask, shift;
> +
> +             mask = ctx->driver_data->in_ic_max_mask;
> +             shift = ctx->driver_data->in_ic_max_shift;
> +
> +             pm_runtime_get_sync(ctx->dev);
> +
> +             cfg = gsc_read(GSC_IN_CON);
> +
> +             /*
> +              * fix align limit to 16bytes. With other limit values, 4
> +              * and 8, it doesn't work.
> +              */
> +             cfg = (cfg & ~(mask << shift));
> +             cfg |= 2 << shift;
> +
> +             gsc_write(cfg, GSC_IN_CON);
> +
> +             pm_runtime_mark_last_busy(ctx->dev);
> +             pm_runtime_put_autosuspend(ctx->dev);
> +
> +             ctx->align_size = 16;
> +     } else {
> +             /* No HW register to get the align limit so use fixed value. */
> +             ctx->align_size = limits->h.align;
> +     }
> +
> +     DRM_DEBUG_KMS("align_size = %d\n", ctx->align_size);
> +}
> +
>  static void gsc_abort(struct exynos_drm_ipp *ipp,
>                         struct exynos_drm_ipp_task *task)
>  {
> @@ -1142,6 +1199,7 @@ static void gsc_abort(struct exynos_drm_ipp *ipp,
>  }
>  
>  static struct exynos_drm_ipp_funcs ipp_funcs = {
> +     .fixup = gsc_fixup,
>       .commit = gsc_commit,
>       .abort = gsc_abort,
>  };
> @@ -1155,6 +1213,8 @@ static int gsc_bind(struct device *dev, struct device 
> *master, void *data)
>       ctx->drm_dev = drm_dev;
>       drm_iommu_attach_device(drm_dev, dev);
>  
> +     gsc_set_align_limit(ctx);
> +
>       exynos_drm_ipp_register(drm_dev, ipp, &ipp_funcs,
>                       DRM_EXYNOS_IPP_CAP_CROP | DRM_EXYNOS_IPP_CAP_ROTATE |
>                       DRM_EXYNOS_IPP_CAP_SCALE | DRM_EXYNOS_IPP_CAP_CONVERT,
> @@ -1221,6 +1281,7 @@ static int gsc_probe(struct platform_device *pdev)
>       }
>       ctx->formats = formats;
>       ctx->num_formats = ARRAY_SIZE(gsc_formats);
> +     ctx->driver_data = driver_data;
>  
>       /* clock control */
>       for (i = 0; i < ctx->num_clocks; i++) {
> @@ -1340,7 +1401,7 @@ static int __maybe_unused gsc_runtime_resume(struct 
> device *dev)
>  };
>  
>  static const struct drm_exynos_ipp_limit gsc_5433_limits[] = {
> -     { IPP_SIZE_LIMIT(BUFFER, .h = { 32, 8191, 2 }, .v = { 16, 8191, 2 }) },
> +     { IPP_SIZE_LIMIT(BUFFER, .h = { 32, 8191, 16 }, .v = { 16, 8191, 2 }) },
>       { IPP_SIZE_LIMIT(AREA, .h = { 16, 4800, 1 }, .v = { 8, 3344, 1 }) },
>       { IPP_SIZE_LIMIT(ROTATED, .h = { 32, 2047 }, .v = { 8, 8191 }) },
>       { IPP_SCALE_LIMIT(.h = { (1 << 16) / 16, (1 << 16) * 8 },
> @@ -1366,6 +1427,9 @@ static int __maybe_unused gsc_runtime_resume(struct 
> device *dev)
>       .num_clocks = 4,
>       .limits = gsc_5433_limits,
>       .num_limits = ARRAY_SIZE(gsc_5433_limits),
> +     .has_in_ic_max = 1,
> +     .in_ic_max_shift = 24,
> +     .in_ic_max_mask = 0x3,
>  };
>  
>  static const struct of_device_id exynos_drm_gsc_of_match[] = {
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c 
> b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> index 26374e5..2319c12 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> @@ -510,9 +510,7 @@ static int exynos_drm_ipp_check_size_limits(struct 
> exynos_drm_ipp_buffer *buf,
>       }
>       __get_size_limit(limits, num_limits, id, &l);
>       if (!__size_limit_check(buf->rect.w, lh) ||
> -         !__align_check(buf->rect.x, lh->align) ||
> -         !__size_limit_check(buf->rect.h, lv) ||
> -         !__align_check(buf->rect.y, lv->align))
> +         !__size_limit_check(buf->rect.h, lv))
>               return -EINVAL;
>  
>       return 0;
> @@ -560,6 +558,14 @@ static int exynos_drm_ipp_check_scale_limits(
>       return 0;
>  }
>  
> +static void exynos_drm_ipp_fixup_buffer(struct exynos_drm_ipp_task *task)
> +{
> +     struct exynos_drm_ipp *ipp = task->ipp;
> +
> +     if (ipp->funcs->fixup)
> +             ipp->funcs->fixup(ipp, task);
> +}
> +
>  static int exynos_drm_ipp_task_check(struct exynos_drm_ipp_task *task)
>  {
>       struct exynos_drm_ipp *ipp = task->ipp;
> @@ -607,6 +613,12 @@ static int exynos_drm_ipp_task_check(struct 
> exynos_drm_ipp_task *task)
>               return -EINVAL;
>       }
>  
> +     /*
> +      * image size should be fixed up before setup buffer call
> +      * which verifies whether image size exceeds gem buffer size or not.
> +      */
> +     exynos_drm_ipp_fixup_buffer(task);
> +
>       src_fmt = __ipp_format_get(ipp, src->buf.fourcc, src->buf.modifier,
>                                  DRM_EXYNOS_IPP_FORMAT_SOURCE);
>       if (!src_fmt) {
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.h 
> b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
> index 0b27d4a..5c2059f 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
> @@ -20,6 +20,18 @@
>   */
>  struct exynos_drm_ipp_funcs {
>       /**
> +      * @fixup:
> +      *
> +      * Correct buffer size according to hardware limit of each DMA device.
> +      * Some DMA device has maximum memory read capability through AXI bus,
> +      * which reads data from memory as a given bytes.
> +      * Therefore, IPP driver needs to check if the buffer size meets
> +      * the HW limlit of each DMA device such as GScaler, Scaler,
> +      * Rotator and FIMC.
> +      */
> +     void (*fixup)(struct exynos_drm_ipp *ipp,
> +                  struct exynos_drm_ipp_task *task);
> +     /**
>        * @commit:
>        *
>        * This is the main entry point to start framebuffer processing
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to