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.dae@xxxxxxxxxxx> > --- > 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 > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html