From d615ed77118f66be4efb1f1ab7f7e7922b58e301 Mon Sep 17 00:00:00 2001 From: DarkBahamut Date: Wed, 1 Mar 2017 18:23:57 +0000 Subject: [PATCH 01/24] arm: dts: odroidxu3: Enable per cpu thermal trips 1. Each A15 cores thermal sensor now correctly used and will trigger the fan or passive throttling as required. 2. Separate file used for trip points to allow unique labels per trip per cpu to be generated without having to duplicate the trips each time. Keeps code clear and allows for easy changes. 3. Trip points tweaked to optimise performance. A7's kept at full speed for longer since they contribute little to the thermal load. Efficiency is improved by not throttling them until required. A15's throttled earlier to manage performance better under heavy loads to extract maximum performance from the available cooling. --- .../boot/dts/exynos5422-odroidxu3-common.dtsi | 108 ++++-------------- .../dts/exynos5422-odroidxu3-trip-points.dtsi | 100 ++++++++++++++++ 2 files changed, 123 insertions(+), 85 deletions(-) create mode 100644 arch/arm/boot/dts/exynos5422-odroidxu3-trip-points.dtsi diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi index 9067d83e343f..cbd6df2a25a6 100755 --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi @@ -64,7 +64,7 @@ cooling-min-state = <0>; cooling-max-state = <3>; #cooling-cells = <2>; - cooling-levels = <0 150 190 252>; + cooling-levels = <0 110 170 230>; }; mali: mali@0x11800000 { @@ -107,90 +107,28 @@ thermal-zones { cpu0_thermal: cpu0-thermal { - thermal-sensors = <&tmu_cpu0 0 &tmu_cpu1 0 &tmu_cpu2 0 &tmu_cpu3 0>; - polling-delay-passive = <250>; - polling-delay = <1000>; - trips { - cpu_alert0: cpu-alert-0 { - temperature = <70000>; /* millicelsius */ - hysteresis = <10000>; /* millicelsius */ - type = "passive"; - }; - cpu_alert1: cpu-alert-1 { - temperature = <75000>; /* millicelsius */ - hysteresis = <10000>; /* millicelsius */ - type = "passive"; - }; - cpu_alert2: cpu-alert-2 { - temperature = <80000>; /* millicelsius */ - hysteresis = <10000>; /* millicelsius */ - type = "passive"; - }; - cpu_alert3: cpu-alert-3 { - temperature = <90000>; /* millicelsius */ - hysteresis = <10000>; /* millicelsius */ - type = "passive"; - }; - cpu_alert4: cpu-alert-4 { - temperature = <95000>; /* millicelsius */ - hysteresis = <10000>; /* millicelsius */ - type = "passive"; - }; - cpu_alert5: cpu-alert-5 { - temperature = <103000>; /* millicelsius */ - hysteresis = <10000>; /* millicelsius */ - type = "passive"; - }; - cpu_alert6: cpu-alert-6 { - temperature = <110000>; /* millicelsius */ - hysteresis = <10000>; /* millicelsius */ - type = "passive"; - }; - cpu_criti0: cpu-crit-0 { - temperature = <115000>; /* millicelsius */ - hysteresis = <10000>; /* millicelsius */ - type = "critical"; - }; - }; - cooling-maps { - map0 { - trip = <&cpu_alert0>; - cooling-device = <&fan0 0 1>; - }; - map1 { - trip = <&cpu_alert1>; - cooling-device = <&fan0 1 2>; - }; - map2 { - trip = <&cpu_alert2>; - cooling-device = <&fan0 2 3>; - }; - /* - * First trip for A7: 1000Mhz - * First trip for A15: 1400Mhz - */ - map3 { - trip = <&cpu_alert3>; - cooling-device = <&cpu0 0 6>; - }; - map4 { - trip = <&cpu_alert3>; - cooling-device = <&cpu4 0 3>; - }; - - /* - * Second trip for A15: 600Mhz - * Second trip for A7: 600Mhz - */ - map5 { - trip = <&cpu_alert4>; - cooling-device = <&cpu0 3 7>; - }; - map6 { - trip = <&cpu_alert4>; - cooling-device = <&cpu4 3 14>; - }; - }; + thermal-sensors = <&tmu_cpu0 0>; + #define CPU_THERMAL_ZONE_NUM 0 + #include "exynos5422-odroidxu3-trip-points.dtsi" + #undef CPU_THERMAL_ZONE_NUM + }; + cpu1_thermal: cpu1-thermal { + thermal-sensors = <&tmu_cpu1 0>; + #define CPU_THERMAL_ZONE_NUM 1 + #include "exynos5422-odroidxu3-trip-points.dtsi" + #undef CPU_THERMAL_ZONE_NUM + }; + cpu2_thermal: cpu2-thermal { + thermal-sensors = <&tmu_cpu2 0>; + #define CPU_THERMAL_ZONE_NUM 2 + #include "exynos5422-odroidxu3-trip-points.dtsi" + #undef CPU_THERMAL_ZONE_NUM + }; + cpu3_thermal: cpu3-thermal { + thermal-sensors = <&tmu_cpu3 0>; + #define CPU_THERMAL_ZONE_NUM 3 + #include "exynos5422-odroidxu3-trip-points.dtsi" + #undef CPU_THERMAL_ZONE_NUM }; }; }; diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-trip-points.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-trip-points.dtsi new file mode 100644 index 000000000000..8037b2f69f05 --- /dev/null +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-trip-points.dtsi @@ -0,0 +1,100 @@ +/* + * Device tree sources for default OdroidXU3/Exynos5422 thermal zone definition + * + * Copyright (c) 2015 Lukasz Majewski + * Anand Moon + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + */ + +#define _TOKENPASTE(x, y) x ## y +#define TOKENPASTE(x, y) _TOKENPASTE(x, y) +#define UNIQIFY(label) TOKENPASTE(label, CPU_THERMAL_ZONE_NUM) + + polling-delay-passive = <250>; + polling-delay = <1000>; + trips { + UNIQIFY(cpu_alert0): cpu-alert-0 { + temperature = <70000>; /* millicelsius */ + hysteresis = <10000>; /* millicelsius */ + type = "active"; + }; + UNIQIFY(cpu_alert1): cpu-alert-1 { + temperature = <75000>; /* millicelsius */ + hysteresis = <10000>; /* millicelsius */ + type = "active"; + }; + UNIQIFY(cpu_alert2): cpu-alert-2 { + temperature = <80000>; /* millicelsius */ + hysteresis = <10000>; /* millicelsius */ + type = "active"; + }; + UNIQIFY(cpu_alert3): cpu-alert-3 { + temperature = <85000>; /* millicelsius */ + hysteresis = <3000>; /* millicelsius */ + type = "passive"; + }; + UNIQIFY(cpu_alert4): cpu-alert-4 { + temperature = <90000>; /* millicelsius */ + hysteresis = <3000>; /* millicelsius */ + type = "passive"; + }; + UNIQIFY(cpu_alert5): cpu-alert-5 { + temperature = <95000>; /* millicelsius */ + hysteresis = <3000>; /* millicelsius */ + type = "passive"; + }; + UNIQIFY(cpu_criti0): cpu-crit-0 { + temperature = <115000>; /* millicelsius */ + hysteresis = <3000>; /* millicelsius */ + type = "critical"; + }; + }; + cooling-maps { + map0 { + trip = <&UNIQIFY(cpu_alert0)>; + cooling-device = <&fan0 0 1>; + }; + map1 { + trip = <&UNIQIFY(cpu_alert1)>; + cooling-device = <&fan0 1 2>; + }; + map2 { + trip = <&UNIQIFY(cpu_alert2)>; + cooling-device = <&fan0 2 3>; + }; + /* + * When reaching cpu_alert3, reduce A15 cores by 1 step. + * The 2GHz step causes high thermals on multithreaded workloads + * so better performance is gained by managing it out early. + */ + map3 { + trip = <&UNIQIFY(cpu_alert3)>; + cooling-device = <&cpu4 0 1>; + }; + /* + * When reaching cpu_alert4, reduce A15 cores by 3 steps + * to further manage the performance level while keeping + * thermals under control. + */ + map4 { + trip = <&UNIQIFY(cpu_alert4)>; + cooling-device = <&cpu4 2 4>; + }; + /* + * When reaching cpu_alert5, reduce all CPUs to ensure thermal + * safety. A7 cores don't produce much thermal load so they are + * reduced less to optimise performance. + */ + map5 { + trip = <&UNIQIFY(cpu_alert5)>; + cooling-device = <&cpu0 0 2>; + }; + map6 { + trip = <&UNIQIFY(cpu_alert5)>; + cooling-device = <&cpu4 5 14>; + }; + }; From 5df83c5bcca234beea3cb97ea3e3153a4627ffec Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Fri, 30 Sep 2016 18:16:41 -0300 Subject: [PATCH 02/24] exynos-gsc: change spamming try_fmt log message to debug The driver try_fmt handler prints a message each time that the image size has been changed due the maximum and minimum width and height. Since user-space can try different format and sizes, this logs a lot of unnecessary messages. Change the message log level to debug and while being there, also add a new line to the message. Signed-off-by: Javier Martinez Canillas Signed-off-by: Sylwester Nawrocki Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/exynos-gsc/gsc-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c index 787bd16c19e5..fac0c0246ad4 100644 --- a/drivers/media/platform/exynos-gsc/gsc-core.c +++ b/drivers/media/platform/exynos-gsc/gsc-core.c @@ -441,7 +441,7 @@ int gsc_try_fmt_mplane(struct gsc_ctx *ctx, struct v4l2_format *f) v4l_bound_align_image(&pix_mp->width, min_w, max_w, mod_x, &pix_mp->height, min_h, max_h, mod_y, 0); if (tmp_w != pix_mp->width || tmp_h != pix_mp->height) - pr_info("Image size has been modified from %dx%d to %dx%d", + pr_debug("Image size has been modified from %dx%d to %dx%d\n", tmp_w, tmp_h, pix_mp->width, pix_mp->height); pix_mp->num_planes = fmt->num_planes; From c834d86b04eb40e5f6dff249f64babc00e23961a Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Fri, 30 Sep 2016 18:16:42 -0300 Subject: [PATCH 03/24] exynos-gsc: don't clear format when freeing buffers with REQBUFS(0) User-space applications can use the VIDIOC_REQBUFS ioctl to determine if a memory mapped, user pointer or DMABUF based I/O is supported by a driver. For example, GStreamer attempts to determine the I/O methods supported by the driver by doing many VIDIOC_REQBUFS ioctl calls with different memory types and count 0. And then the real VIDIOC_REQBUFS call with count == n is be made to allocate the buffers. But for count 0, the driver not only frees the buffers but also clears the format set before with VIDIOC_S_FMT. This is a problem since STREAMON fails if a format isn't set but GStreamer first sets a format and then tries to determine the supported I/O methods, so the format will be cleared on REQBUFS(0), before the call to STREAMON. To avoid this issue, only free the buffers on VIDIOC_REQBUFS(0) but don't clear the format. Since is completely valid to set the format and then do different calls to REQBUFS before a call to STREAMON. Signed-off-by: Javier Martinez Canillas Signed-off-by: Sylwester Nawrocki Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/exynos-gsc/gsc-m2m.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c b/drivers/media/platform/exynos-gsc/gsc-m2m.c index 9f03b791b711..e2a16b52f87d 100644 --- a/drivers/media/platform/exynos-gsc/gsc-m2m.c +++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c @@ -365,14 +365,8 @@ static int gsc_m2m_reqbufs(struct file *file, void *fh, max_cnt = (reqbufs->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) ? gsc->variant->in_buf_cnt : gsc->variant->out_buf_cnt; - if (reqbufs->count > max_cnt) { + if (reqbufs->count > max_cnt) return -EINVAL; - } else if (reqbufs->count == 0) { - if (reqbufs->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) - gsc_ctx_state_lock_clear(GSC_SRC_FMT, ctx); - else - gsc_ctx_state_lock_clear(GSC_DST_FMT, ctx); - } return v4l2_m2m_reqbufs(file, ctx->m2m_ctx, reqbufs); } From dd5bf75ce7d48d2c6c67f271fb9820d2d1b24898 Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Fri, 30 Sep 2016 18:16:43 -0300 Subject: [PATCH 04/24] exynos-gsc: fix supported RGB pixel format The driver exposes 32-bit A/XRGB 8-8-8-8 as supported format but testing shows that using this format produces frames with wrong colors. The test was done with the following GStreamer pipeline: $ gst-launch-1.0 videotestsrc num-buffers=20 ! video/x-raw,format=UYVY \ ! v4l2video3convert ! video/x-raw,format=xRGB ! videoconvert ! kmssink The manual seems to state that the Pixel Format are in Little Endianness so instead use the 32-bit BGRA/X 8-8-8-8 pixel format. This format works correctly when using the following pipeline: $ gst-launch-1.0 videotestsrc num-buffers=20 ! video/x-raw,format=UYVY \ ! v4l2video3convert ! video/x-raw,format=BGRx ! kmssink This change is similar to commit 7f2816e51ea1 ("[media] s5p-fimc: Changed RGB32 to BGR32") that fixed the same issue on a different Samsung driver. Suggested-by: Nicolas Dufresne Signed-off-by: Javier Martinez Canillas Signed-off-by: Sylwester Nawrocki Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/exynos-gsc/gsc-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c index fac0c0246ad4..8bb1d2be7234 100644 --- a/drivers/media/platform/exynos-gsc/gsc-core.c +++ b/drivers/media/platform/exynos-gsc/gsc-core.c @@ -39,8 +39,8 @@ static const struct gsc_fmt gsc_formats[] = { .num_planes = 1, .num_comp = 1, }, { - .name = "XRGB-8-8-8-8, 32 bpp", - .pixelformat = V4L2_PIX_FMT_RGB32, + .name = "BGRX-8-8-8-8, 32 bpp", + .pixelformat = V4L2_PIX_FMT_BGR32, .depth = { 32 }, .color = GSC_RGB, .num_planes = 1, From 5656894d7fb8aef7407b0a6fb45bd661e5e81b30 Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Fri, 30 Sep 2016 18:16:44 -0300 Subject: [PATCH 05/24] exynos-gsc: do proper bytesperline and sizeimage calculation The driver don't take into account the differences between packed, semi planar and multi planar formats when calculating the pixel format bytes per lines and image size values. This makes GStreamer to fail when the following formats are used NV12, NV21, NV16, NV61, YV12, I420 and Y42B: "gst_video_frame_map_id: failed to map video frame plane 1" Nicolas suggested to use the logic found in the Exynos FIMC v4l2 driver since does this correctly. So this patch changes the bytes per line and image size calculation according to what's done in this media driver. After this patch most supported formats work correctly. There are still issues with the NV21 and NV61 formats, but that seems to be a separate problem since NV12 and NV16 work and these formats use the same values. So this can be fixed as a follow-up and shouldn't be a blocker for this change that improves the driver's support. Suggested-by: Nicolas Dufresne Signed-off-by: Javier Martinez Canillas Signed-off-by: Sylwester Nawrocki Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/exynos-gsc/gsc-core.c | 21 ++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c index 8bb1d2be7234..a6c47deba3b7 100644 --- a/drivers/media/platform/exynos-gsc/gsc-core.c +++ b/drivers/media/platform/exynos-gsc/gsc-core.c @@ -451,12 +451,25 @@ int gsc_try_fmt_mplane(struct gsc_ctx *ctx, struct v4l2_format *f) else /* SD */ pix_mp->colorspace = V4L2_COLORSPACE_SMPTE170M; - for (i = 0; i < pix_mp->num_planes; ++i) { - int bpl = (pix_mp->width * fmt->depth[i]) >> 3; - pix_mp->plane_fmt[i].bytesperline = bpl; - pix_mp->plane_fmt[i].sizeimage = bpl * pix_mp->height; + struct v4l2_plane_pix_format *plane_fmt = &pix_mp->plane_fmt[i]; + u32 bpl = plane_fmt->bytesperline; + if (fmt->num_comp == 1 && /* Packed */ + (bpl == 0 || (bpl * 8 / fmt->depth[i]) < pix_mp->width)) + bpl = pix_mp->width * fmt->depth[i] / 8; + + if (fmt->num_comp > 1 && /* Planar */ + (bpl == 0 || bpl < pix_mp->width)) + bpl = pix_mp->width; + + if (i != 0 && fmt->num_comp == 3) + bpl /= 2; + + plane_fmt->bytesperline = bpl; + plane_fmt->sizeimage = max(pix_mp->width * pix_mp->height * + fmt->depth[i] / 8, + plane_fmt->sizeimage); pr_debug("[%d]: bpl: %d, sizeimage: %d", i, bpl, pix_mp->plane_fmt[i].sizeimage); } From 0e3986b585a2885b685cd42d1e35b9141f35866b Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Fri, 7 Oct 2016 17:39:17 -0300 Subject: [PATCH 06/24] exynos-gsc: don't release a non-dynamically allocated video_device The struct v4l2_device instance for the G-Scaler is not dyanmically allocated but a member of the struct gsc_dev. In fact, the assigned .release callback is video_device_release_empty(). But gsc_register_m2m_device() attempts to release the v4l2_device by calling video_device_release() in its error path. This is wrong since the v4l2_device wasn't allocated directly and will be freed once its parent struct gsc_dev is freed. While being there, rename the remaining goto label in the error path to something that better explains the error path cleanup. Signed-off-by: Javier Martinez Canillas Signed-off-by: Sylwester Nawrocki Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/exynos-gsc/gsc-m2m.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c b/drivers/media/platform/exynos-gsc/gsc-m2m.c index e2a16b52f87d..a1cac52ea230 100644 --- a/drivers/media/platform/exynos-gsc/gsc-m2m.c +++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c @@ -760,24 +760,21 @@ int gsc_register_m2m_device(struct gsc_dev *gsc) gsc->m2m.m2m_dev = v4l2_m2m_init(&gsc_m2m_ops); if (IS_ERR(gsc->m2m.m2m_dev)) { dev_err(&pdev->dev, "failed to initialize v4l2-m2m device\n"); - ret = PTR_ERR(gsc->m2m.m2m_dev); - goto err_m2m_r1; + return PTR_ERR(gsc->m2m.m2m_dev); } ret = video_register_device(&gsc->vdev, VFL_TYPE_GRABBER, -1); if (ret) { dev_err(&pdev->dev, "%s(): failed to register video device\n", __func__); - goto err_m2m_r2; + goto err_m2m_release; } pr_debug("gsc m2m driver registered as /dev/video%d", gsc->vdev.num); return 0; -err_m2m_r2: +err_m2m_release: v4l2_m2m_release(gsc->m2m.m2m_dev); -err_m2m_r1: - video_device_release(gsc->m2m.vfd); return ret; } From 728ed2a9317dd4a3b8ebe78f728b6801887d412a Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Fri, 7 Oct 2016 17:39:18 -0300 Subject: [PATCH 07/24] exynos-gsc: unregister video device node on driver removal The driver doesn't unregister the video device node when the driver is removed, this keeps video device nodes that makes the machine to crash with a NULL pointer dereference when nodes are attempted to be opened: [ 36.530006] Unable to handle kernel paging request at virtual address bf1f8200 [ 36.535985] pgd = edbbc000 [ 36.538486] [bf1f8200] *pgd=6d99a811, *pte=00000000, *ppte=00000000 [ 36.544727] Internal error: Oops: 7 [#1] PREEMPT SMP ARM [ 36.550016] Modules linked in: s5p_jpeg s5p_mfc v4l2_mem2mem videobuf2_dma_contig [ 36.566303] CPU: 6 PID: 533 Comm: v4l2-ctl Not tainted 4.8.0 [ 36.574466] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) [ 36.580526] task: ee3cc600 task.stack: ed626000 [ 36.585046] PC is at try_module_get+0x1c/0xac [ 36.589364] LR is at try_module_get+0x1c/0xac [ 36.593698] pc : [] lr : [] psr: 80070013 [ 36.593698] sp : ed627de0 ip : a0070013 fp : 00000000 [ 36.605156] r10: 00000002 r9 : ed627ed0 r8 : 00000000 [ 36.610331] r7 : c01e5f14 r6 : ed57be00 r5 : bf1f8200 r4 : bf1f8200 [ 36.616834] r3 : 00000002 r2 : 00000002 r1 : 01930192 r0 : 00000001 .. [ 36.785004] [] (try_module_get) from [] (cdev_get+0x1c/0x4c) [ 36.792362] [] (cdev_get) from [] (chrdev_open+0x2c/0x178) [ 36.799555] [] (chrdev_open) from [] (do_dentry_open+0x1e0/0x300) [ 36.807360] [] (do_dentry_open) from [] (path_openat+0x35c/0xf58) [ 36.815154] [] (path_openat) from [] (do_filp_open+0x5c/0xc0) [ 36.822606] [] (do_filp_open) from [] (do_sys_open+0x10c/0x1bc) [ 36.830235] [] (do_sys_open) from [] (ret_fast_syscall+0x0/0x3c) [ 36.837942] Code: 0a00001c e1a04000 e3a00001 ebfec92d (e5943000) Signed-off-by: Javier Martinez Canillas Signed-off-by: Sylwester Nawrocki Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/exynos-gsc/gsc-m2m.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c b/drivers/media/platform/exynos-gsc/gsc-m2m.c index a1cac52ea230..c8c0bcec35ed 100644 --- a/drivers/media/platform/exynos-gsc/gsc-m2m.c +++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c @@ -781,6 +781,8 @@ err_m2m_release: void gsc_unregister_m2m_device(struct gsc_dev *gsc) { - if (gsc) + if (gsc) { v4l2_m2m_release(gsc->m2m.m2m_dev); + video_unregister_device(&gsc->vdev); + } } From 20dcb10ccf908dfc455679eae362df958700a378 Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Fri, 7 Oct 2016 17:39:19 -0300 Subject: [PATCH 08/24] exynos-gsc: cleanup m2m src and dst vb2 queues on STREAMOFF Media drivers that use the videobuf2 framework have to give back to vb2 all the buffers that received from vb2 using its .buf_queue callback. But the exynos-gsc driver isn't doing a proper cleanup so vb2 complains that the number of buffers enqueued and received are not balanced: WARNING: CPU: 2 PID: 660 at drivers/media/v4l2-core/videobuf2-core.c:1654 __vb2_queue_cancel+0xec/0x150 [videobuf2_core] Modules linked in: mwifiex_sdio mwifiex uvcvideo exynos_gsc videobuf2_vmalloc s5p_mfc s5p_jpeg CPU: 2 PID: 660 Comm: lt-gst-validate Not tainted 4.8.0 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (dump_stack+0x88/0x9c) [] (dump_stack) from [] (__warn+0xe8/0x100) [] (__warn) from [] (warn_slowpath_null+0x20/0x28) [] (warn_slowpath_null) from [] (__vb2_queue_cancel+0xec/0x150 [videobuf2_core]) [] (__vb2_queue_cancel [videobuf2_core]) from [] (vb2_core_streamoff+0x34/0x9c [videobuf2_core]) [] (vb2_core_streamoff [videobuf2_core]) from [] (v4l2_m2m_streamoff+0x2c/0xe4 [v4l2_mem2mem]) [] (v4l2_m2m_streamoff [v4l2_mem2mem]) from [] (__video_do_ioctl+0x298/0x30c [videodev]) [] (__video_do_ioctl [videodev]) from [] (video_usercopy+0x174/0x4e8 [videodev]) [] (video_usercopy [videodev]) from [] (v4l2_ioctl+0xc4/0xd8 [videodev]) [] (v4l2_ioctl [videodev]) from [] (do_vfs_ioctl+0x9c/0x8f4) [] (do_vfs_ioctl) from [] (SyS_ioctl+0x34/0x5c) [] (SyS_ioctl) from [] (ret_fast_syscall+0x0/0x3c) Fix this by passing back to vb2 all the received buffers that were not processed. Signed-off-by: Javier Martinez Canillas Signed-off-by: Sylwester Nawrocki Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/exynos-gsc/gsc-m2m.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c b/drivers/media/platform/exynos-gsc/gsc-m2m.c index c8c0bcec35ed..f49f24b4462a 100644 --- a/drivers/media/platform/exynos-gsc/gsc-m2m.c +++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c @@ -66,12 +66,29 @@ static int gsc_m2m_start_streaming(struct vb2_queue *q, unsigned int count) return ret > 0 ? 0 : ret; } +static void __gsc_m2m_cleanup_queue(struct gsc_ctx *ctx) +{ + struct vb2_v4l2_buffer *src_vb, *dst_vb; + + while (v4l2_m2m_num_src_bufs_ready(ctx->m2m_ctx) > 0) { + src_vb = v4l2_m2m_src_buf_remove(ctx->m2m_ctx); + v4l2_m2m_buf_done(src_vb, VB2_BUF_STATE_ERROR); + } + + while (v4l2_m2m_num_dst_bufs_ready(ctx->m2m_ctx) > 0) { + dst_vb = v4l2_m2m_dst_buf_remove(ctx->m2m_ctx); + v4l2_m2m_buf_done(dst_vb, VB2_BUF_STATE_ERROR); + } +} + static void gsc_m2m_stop_streaming(struct vb2_queue *q) { struct gsc_ctx *ctx = q->drv_priv; __gsc_m2m_job_abort(ctx); + __gsc_m2m_cleanup_queue(ctx); + pm_runtime_put(&ctx->gsc_dev->pdev->dev); } From 018cad5bc452be9448617615ce224b0ab07bb67a Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Wed, 9 Nov 2016 12:23:50 -0200 Subject: [PATCH 09/24] exynos-gsc: Simplify clock management Instead of having separate functions that fetches, prepares and unprepares the clock, let's encapsulate this code into ->probe(). This makes error handling easier and decreases the lines of code. [mszyprow: rebased onto v4.9-rc4] Signed-off-by: Ulf Hansson Signed-off-by: Marek Szyprowski Tested-by: Javier Martinez Canillas Signed-off-by: Sylwester Nawrocki Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/exynos-gsc/gsc-core.c | 49 ++++++-------------- 1 file changed, 14 insertions(+), 35 deletions(-) diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c index a6c47deba3b7..183c6e4e915f 100644 --- a/drivers/media/platform/exynos-gsc/gsc-core.c +++ b/drivers/media/platform/exynos-gsc/gsc-core.c @@ -1001,36 +1001,6 @@ static void *gsc_get_drv_data(struct platform_device *pdev) return driver_data; } -static void gsc_clk_put(struct gsc_dev *gsc) -{ - if (!IS_ERR(gsc->clock)) - clk_unprepare(gsc->clock); -} - -static int gsc_clk_get(struct gsc_dev *gsc) -{ - int ret; - - dev_dbg(&gsc->pdev->dev, "gsc_clk_get Called\n"); - - gsc->clock = devm_clk_get(&gsc->pdev->dev, GSC_CLOCK_GATE_NAME); - if (IS_ERR(gsc->clock)) { - dev_err(&gsc->pdev->dev, "failed to get clock~~~: %s\n", - GSC_CLOCK_GATE_NAME); - return PTR_ERR(gsc->clock); - } - - ret = clk_prepare(gsc->clock); - if (ret < 0) { - dev_err(&gsc->pdev->dev, "clock prepare failed for clock: %s\n", - GSC_CLOCK_GATE_NAME); - gsc->clock = ERR_PTR(-EINVAL); - return ret; - } - - return 0; -} - static int gsc_m2m_suspend(struct gsc_dev *gsc) { unsigned long flags; @@ -1098,7 +1068,6 @@ static int gsc_probe(struct platform_device *pdev) init_waitqueue_head(&gsc->irq_queue); spin_lock_init(&gsc->slock); mutex_init(&gsc->lock); - gsc->clock = ERR_PTR(-EINVAL); res = platform_get_resource(pdev, IORESOURCE_MEM, 0); gsc->regs = devm_ioremap_resource(dev, res); @@ -1111,9 +1080,19 @@ static int gsc_probe(struct platform_device *pdev) return -ENXIO; } - ret = gsc_clk_get(gsc); - if (ret) + gsc->clock = devm_clk_get(dev, GSC_CLOCK_GATE_NAME); + if (IS_ERR(gsc->clock)) { + dev_err(dev, "failed to get clock~~~: %s\n", + GSC_CLOCK_GATE_NAME); + return PTR_ERR(gsc->clock); + } + + ret = clk_prepare(gsc->clock); + if (ret) { + dev_err(&gsc->pdev->dev, "clock prepare failed for clock: %s\n", + GSC_CLOCK_GATE_NAME); return ret; + } ret = devm_request_irq(dev, res->start, gsc_irq_handler, 0, pdev->name, gsc); @@ -1148,7 +1127,7 @@ err_m2m: err_v4l2: v4l2_device_unregister(&gsc->v4l2_dev); err_clk: - gsc_clk_put(gsc); + clk_unprepare(gsc->clock); return ret; } @@ -1161,7 +1140,7 @@ static int gsc_remove(struct platform_device *pdev) vb2_dma_contig_clear_max_seg_size(&pdev->dev); pm_runtime_disable(&pdev->dev); - gsc_clk_put(gsc); + clk_unprepare(gsc->clock); dev_dbg(&pdev->dev, "%s driver unloaded\n", pdev->name); return 0; From 4fb2c78e3c7aa51b90b65d1e5da4750c81677aba Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Wed, 9 Nov 2016 12:23:51 -0200 Subject: [PATCH 10/24] exynos-gsc: Convert gsc_m2m_resume() from int to void Since gsc_m2m_resume() always returns 0, convert it to a void instead. [mszyprow: rebased onto v4.9-rc4] Signed-off-by: Ulf Hansson Signed-off-by: Marek Szyprowski Tested-by: Javier Martinez Canillas Signed-off-by: Sylwester Nawrocki Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/exynos-gsc/gsc-core.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c index 183c6e4e915f..945042c346e7 100644 --- a/drivers/media/platform/exynos-gsc/gsc-core.c +++ b/drivers/media/platform/exynos-gsc/gsc-core.c @@ -1023,7 +1023,7 @@ static int gsc_m2m_suspend(struct gsc_dev *gsc) return timeout == 0 ? -EAGAIN : 0; } -static int gsc_m2m_resume(struct gsc_dev *gsc) +static void gsc_m2m_resume(struct gsc_dev *gsc) { struct gsc_ctx *ctx; unsigned long flags; @@ -1036,8 +1036,6 @@ static int gsc_m2m_resume(struct gsc_dev *gsc) if (test_and_clear_bit(ST_M2M_SUSPENDED, &gsc->state)) gsc_m2m_job_finish(ctx, VB2_BUF_STATE_ERROR); - - return 0; } static int gsc_probe(struct platform_device *pdev) @@ -1159,8 +1157,9 @@ static int gsc_runtime_resume(struct device *dev) gsc_hw_set_sw_reset(gsc); gsc_wait_reset(gsc); + gsc_m2m_resume(gsc); - return gsc_m2m_resume(gsc); + return 0; } static int gsc_runtime_suspend(struct device *dev) From 67ebf54ec04f95e7983b4f428a2df3aa9275ba3c Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Wed, 9 Nov 2016 12:23:52 -0200 Subject: [PATCH 11/24] exynos-gsc: Make driver functional when CONFIG_PM is unset The driver depended on CONFIG_PM to be functional. Let's remove that dependency, by enable the runtime PM resourses during ->probe() and update the device's runtime PM status to reflect this. [mszyprow: rebased onto v4.9-rc4] Signed-off-by: Ulf Hansson Signed-off-by: Marek Szyprowski Tested-by: Javier Martinez Canillas Signed-off-by: Sylwester Nawrocki Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/exynos-gsc/gsc-core.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c index 945042c346e7..7539bb3e318f 100644 --- a/drivers/media/platform/exynos-gsc/gsc-core.c +++ b/drivers/media/platform/exynos-gsc/gsc-core.c @@ -1085,7 +1085,7 @@ static int gsc_probe(struct platform_device *pdev) return PTR_ERR(gsc->clock); } - ret = clk_prepare(gsc->clock); + ret = clk_prepare_enable(gsc->clock); if (ret) { dev_err(&gsc->pdev->dev, "clock prepare failed for clock: %s\n", GSC_CLOCK_GATE_NAME); @@ -1108,24 +1108,23 @@ static int gsc_probe(struct platform_device *pdev) goto err_v4l2; platform_set_drvdata(pdev, gsc); - pm_runtime_enable(dev); - ret = pm_runtime_get_sync(&pdev->dev); - if (ret < 0) - goto err_m2m; + + gsc_hw_set_sw_reset(gsc); + gsc_wait_reset(gsc); vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32)); dev_dbg(dev, "gsc-%d registered successfully\n", gsc->id); - pm_runtime_put(dev); + pm_runtime_set_active(dev); + pm_runtime_enable(dev); + return 0; -err_m2m: - gsc_unregister_m2m_device(gsc); err_v4l2: v4l2_device_unregister(&gsc->v4l2_dev); err_clk: - clk_unprepare(gsc->clock); + clk_disable_unprepare(gsc->clock); return ret; } From 613fd898baeeeeaadc43f035cb9f4d16a274be22 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Mon, 14 Nov 2016 08:27:32 -0200 Subject: [PATCH 12/24] exynos-gsc: Make PM callbacks available conditionally There are no need to set up the PM callbacks (runtime and system) unless they are being used. It also causes compiler warnings about unused functions. Silence the warnings by making them available for CONFIG_PM (runtime callbacks) and CONFIG_PM_SLEEP (system sleep callbacks). [mszyprow: squashed two patches into one to avoid potential build break, changed patch subject and updated commit message] Signed-off-by: Ulf Hansson Signed-off-by: Marek Szyprowski Signed-off-by: Sylwester Nawrocki Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/exynos-gsc/gsc-core.c | 84 ++++++++++---------- 1 file changed, 43 insertions(+), 41 deletions(-) diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c index 7539bb3e318f..19fcbc7424fc 100644 --- a/drivers/media/platform/exynos-gsc/gsc-core.c +++ b/drivers/media/platform/exynos-gsc/gsc-core.c @@ -1001,43 +1001,6 @@ static void *gsc_get_drv_data(struct platform_device *pdev) return driver_data; } -static int gsc_m2m_suspend(struct gsc_dev *gsc) -{ - unsigned long flags; - int timeout; - - spin_lock_irqsave(&gsc->slock, flags); - if (!gsc_m2m_pending(gsc)) { - spin_unlock_irqrestore(&gsc->slock, flags); - return 0; - } - clear_bit(ST_M2M_SUSPENDED, &gsc->state); - set_bit(ST_M2M_SUSPENDING, &gsc->state); - spin_unlock_irqrestore(&gsc->slock, flags); - - timeout = wait_event_timeout(gsc->irq_queue, - test_bit(ST_M2M_SUSPENDED, &gsc->state), - GSC_SHUTDOWN_TIMEOUT); - - clear_bit(ST_M2M_SUSPENDING, &gsc->state); - return timeout == 0 ? -EAGAIN : 0; -} - -static void gsc_m2m_resume(struct gsc_dev *gsc) -{ - struct gsc_ctx *ctx; - unsigned long flags; - - spin_lock_irqsave(&gsc->slock, flags); - /* Clear for full H/W setup in first run after resume */ - ctx = gsc->m2m.ctx; - gsc->m2m.ctx = NULL; - spin_unlock_irqrestore(&gsc->slock, flags); - - if (test_and_clear_bit(ST_M2M_SUSPENDED, &gsc->state)) - gsc_m2m_job_finish(ctx, VB2_BUF_STATE_ERROR); -} - static int gsc_probe(struct platform_device *pdev) { struct gsc_dev *gsc; @@ -1143,6 +1106,44 @@ static int gsc_remove(struct platform_device *pdev) return 0; } +#ifdef CONFIG_PM +static int gsc_m2m_suspend(struct gsc_dev *gsc) +{ + unsigned long flags; + int timeout; + + spin_lock_irqsave(&gsc->slock, flags); + if (!gsc_m2m_pending(gsc)) { + spin_unlock_irqrestore(&gsc->slock, flags); + return 0; + } + clear_bit(ST_M2M_SUSPENDED, &gsc->state); + set_bit(ST_M2M_SUSPENDING, &gsc->state); + spin_unlock_irqrestore(&gsc->slock, flags); + + timeout = wait_event_timeout(gsc->irq_queue, + test_bit(ST_M2M_SUSPENDED, &gsc->state), + GSC_SHUTDOWN_TIMEOUT); + + clear_bit(ST_M2M_SUSPENDING, &gsc->state); + return timeout == 0 ? -EAGAIN : 0; +} + +static void gsc_m2m_resume(struct gsc_dev *gsc) +{ + struct gsc_ctx *ctx; + unsigned long flags; + + spin_lock_irqsave(&gsc->slock, flags); + /* Clear for full H/W setup in first run after resume */ + ctx = gsc->m2m.ctx; + gsc->m2m.ctx = NULL; + spin_unlock_irqrestore(&gsc->slock, flags); + + if (test_and_clear_bit(ST_M2M_SUSPENDED, &gsc->state)) + gsc_m2m_job_finish(ctx, VB2_BUF_STATE_ERROR); +} + static int gsc_runtime_resume(struct device *dev) { struct gsc_dev *gsc = dev_get_drvdata(dev); @@ -1173,7 +1174,9 @@ static int gsc_runtime_suspend(struct device *dev) pr_debug("gsc%d: state: 0x%lx", gsc->id, gsc->state); return ret; } +#endif +#ifdef CONFIG_PM_SLEEP static int gsc_resume(struct device *dev) { struct gsc_dev *gsc = dev_get_drvdata(dev); @@ -1210,12 +1213,11 @@ static int gsc_suspend(struct device *dev) return 0; } +#endif static const struct dev_pm_ops gsc_pm_ops = { - .suspend = gsc_suspend, - .resume = gsc_resume, - .runtime_suspend = gsc_runtime_suspend, - .runtime_resume = gsc_runtime_resume, + SET_SYSTEM_SLEEP_PM_OPS(gsc_suspend, gsc_resume) + SET_RUNTIME_PM_OPS(gsc_runtime_suspend, gsc_runtime_resume, NULL) }; static struct platform_driver gsc_driver = { From 74c328765bf2f670296a030f4007298d8c15275f Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Wed, 9 Nov 2016 12:23:54 -0200 Subject: [PATCH 13/24] exynos-gsc: Fixup clock management at ->remove() To make sure the clock is fully gated in ->remove(), we first need to to bring the device into full power by invoking pm_runtime_get_sync(). Then, let's both unprepare and disable the clock. [mszyprow: rebased onto v4.9-rc4] Signed-off-by: Ulf Hansson Signed-off-by: Marek Szyprowski Tested-by: Javier Martinez Canillas Signed-off-by: Sylwester Nawrocki Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/exynos-gsc/gsc-core.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c index 19fcbc7424fc..e62765328848 100644 --- a/drivers/media/platform/exynos-gsc/gsc-core.c +++ b/drivers/media/platform/exynos-gsc/gsc-core.c @@ -1095,12 +1095,15 @@ static int gsc_remove(struct platform_device *pdev) { struct gsc_dev *gsc = platform_get_drvdata(pdev); + pm_runtime_get_sync(&pdev->dev); + gsc_unregister_m2m_device(gsc); v4l2_device_unregister(&gsc->v4l2_dev); vb2_dma_contig_clear_max_seg_size(&pdev->dev); - pm_runtime_disable(&pdev->dev); - clk_unprepare(gsc->clock); + clk_disable_unprepare(gsc->clock); + + pm_runtime_put_noidle(&pdev->dev); dev_dbg(&pdev->dev, "%s driver unloaded\n", pdev->name); return 0; From 9a28dde6a81132763904110ea34168a768e0554a Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Wed, 9 Nov 2016 12:23:55 -0200 Subject: [PATCH 14/24] exynos-gsc: Do full clock gating at runtime PM suspend To potentially save more power in runtime PM suspend state, let's also prepare/unprepare the clock from the runtime PM callbacks. [mszyprow: rebased onto v4.9-rc4] Signed-off-by: Ulf Hansson Signed-off-by: Marek Szyprowski Tested-by: Javier Martinez Canillas Signed-off-by: Sylwester Nawrocki Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/exynos-gsc/gsc-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c index e62765328848..3ac588f5c558 100644 --- a/drivers/media/platform/exynos-gsc/gsc-core.c +++ b/drivers/media/platform/exynos-gsc/gsc-core.c @@ -1154,7 +1154,7 @@ static int gsc_runtime_resume(struct device *dev) pr_debug("gsc%d: state: 0x%lx", gsc->id, gsc->state); - ret = clk_enable(gsc->clock); + ret = clk_prepare_enable(gsc->clock); if (ret) return ret; @@ -1172,7 +1172,7 @@ static int gsc_runtime_suspend(struct device *dev) ret = gsc_m2m_suspend(gsc); if (!ret) - clk_disable(gsc->clock); + clk_disable_unprepare(gsc->clock); pr_debug("gsc%d: state: 0x%lx", gsc->id, gsc->state); return ret; From a4e4fb5eb0da69bf50c8ad67010b17bff43998c4 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Wed, 9 Nov 2016 12:23:57 -0200 Subject: [PATCH 15/24] exynos-gsc: Simplify system PM It's not needed to keep a local flag about the current system PM state. Let's just remove that code and the corresponding debug print. [mszyprow: rebased onto v4.9-rc4] Signed-off-by: Ulf Hansson Signed-off-by: Marek Szyprowski Tested-by: Javier Martinez Canillas Signed-off-by: Sylwester Nawrocki Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/exynos-gsc/gsc-core.c | 21 -------------------- drivers/media/platform/exynos-gsc/gsc-core.h | 3 --- 2 files changed, 24 deletions(-) diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c index 3ac588f5c558..5aeb8b44eacd 100644 --- a/drivers/media/platform/exynos-gsc/gsc-core.c +++ b/drivers/media/platform/exynos-gsc/gsc-core.c @@ -1182,20 +1182,6 @@ static int gsc_runtime_suspend(struct device *dev) #ifdef CONFIG_PM_SLEEP static int gsc_resume(struct device *dev) { - struct gsc_dev *gsc = dev_get_drvdata(dev); - unsigned long flags; - - pr_debug("gsc%d: state: 0x%lx", gsc->id, gsc->state); - - /* Do not resume if the device was idle before system suspend */ - spin_lock_irqsave(&gsc->slock, flags); - if (!test_and_clear_bit(ST_SUSPEND, &gsc->state) || - !gsc_m2m_opened(gsc)) { - spin_unlock_irqrestore(&gsc->slock, flags); - return 0; - } - spin_unlock_irqrestore(&gsc->slock, flags); - if (!pm_runtime_suspended(dev)) return gsc_runtime_resume(dev); @@ -1204,13 +1190,6 @@ static int gsc_resume(struct device *dev) static int gsc_suspend(struct device *dev) { - struct gsc_dev *gsc = dev_get_drvdata(dev); - - pr_debug("gsc%d: state: 0x%lx", gsc->id, gsc->state); - - if (test_and_set_bit(ST_SUSPEND, &gsc->state)) - return 0; - if (!pm_runtime_suspended(dev)) return gsc_runtime_suspend(dev); diff --git a/drivers/media/platform/exynos-gsc/gsc-core.h b/drivers/media/platform/exynos-gsc/gsc-core.h index 7ad7b9dc2243..8480aec05441 100644 --- a/drivers/media/platform/exynos-gsc/gsc-core.h +++ b/drivers/media/platform/exynos-gsc/gsc-core.h @@ -48,9 +48,6 @@ #define GSC_CTX_ABORT (1 << 7) enum gsc_dev_flags { - /* for global */ - ST_SUSPEND, - /* for m2m node */ ST_M2M_OPEN, ST_M2M_RUN, From df994d2fe8b47fb211c5e165099b69ddd9bfdf24 Mon Sep 17 00:00:00 2001 From: Marek Szyprowski Date: Wed, 9 Nov 2016 12:23:58 -0200 Subject: [PATCH 16/24] exynos-gsc: Simplify system PM even more System PM callbacks only ensure that device is runtime suspended/resumed, so remove them and use generic pm_runtime_force_suspend/resume helper. Signed-off-by: Marek Szyprowski Reviewed-by: Ulf Hansson Tested-by: Javier Martinez Canillas Signed-off-by: Sylwester Nawrocki Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/exynos-gsc/gsc-core.c | 21 ++------------------ 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c index 5aeb8b44eacd..45a62216385a 100644 --- a/drivers/media/platform/exynos-gsc/gsc-core.c +++ b/drivers/media/platform/exynos-gsc/gsc-core.c @@ -1179,26 +1179,9 @@ static int gsc_runtime_suspend(struct device *dev) } #endif -#ifdef CONFIG_PM_SLEEP -static int gsc_resume(struct device *dev) -{ - if (!pm_runtime_suspended(dev)) - return gsc_runtime_resume(dev); - - return 0; -} - -static int gsc_suspend(struct device *dev) -{ - if (!pm_runtime_suspended(dev)) - return gsc_runtime_suspend(dev); - - return 0; -} -#endif - static const struct dev_pm_ops gsc_pm_ops = { - SET_SYSTEM_SLEEP_PM_OPS(gsc_suspend, gsc_resume) + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, + pm_runtime_force_resume) SET_RUNTIME_PM_OPS(gsc_runtime_suspend, gsc_runtime_resume, NULL) }; From 09cfc5f295d5baa1456f5048092c584fc2cd8174 Mon Sep 17 00:00:00 2001 From: Marek Szyprowski Date: Wed, 9 Nov 2016 12:23:59 -0200 Subject: [PATCH 17/24] exynos-gsc: Remove unused lclk_freqency entry Remove dead, unused code. Signed-off-by: Marek Szyprowski Tested-by: Javier Martinez Canillas Signed-off-by: Sylwester Nawrocki Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/exynos-gsc/gsc-core.c | 1 - drivers/media/platform/exynos-gsc/gsc-core.h | 2 -- 2 files changed, 3 deletions(-) diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c index 45a62216385a..aa4db1dfc438 100644 --- a/drivers/media/platform/exynos-gsc/gsc-core.c +++ b/drivers/media/platform/exynos-gsc/gsc-core.c @@ -977,7 +977,6 @@ static struct gsc_driverdata gsc_v_100_drvdata = { [3] = &gsc_v_100_variant, }, .num_entities = 4, - .lclk_frequency = 266000000UL, }; static const struct of_device_id exynos_gsc_match[] = { diff --git a/drivers/media/platform/exynos-gsc/gsc-core.h b/drivers/media/platform/exynos-gsc/gsc-core.h index 8480aec05441..e5aa8f42f11e 100644 --- a/drivers/media/platform/exynos-gsc/gsc-core.h +++ b/drivers/media/platform/exynos-gsc/gsc-core.h @@ -303,12 +303,10 @@ struct gsc_variant { * struct gsc_driverdata - per device type driver data for init time. * * @variant: the variant information for this driver. - * @lclk_frequency: G-Scaler clock frequency * @num_entities: the number of g-scalers */ struct gsc_driverdata { struct gsc_variant *variant[GSC_MAX_DEVS]; - unsigned long lclk_frequency; int num_entities; }; From 4abc5b06ee7c379d65a8a15d5c4b261378078742 Mon Sep 17 00:00:00 2001 From: Marek Szyprowski Date: Wed, 9 Nov 2016 12:24:00 -0200 Subject: [PATCH 18/24] exynos-gsc: Add missing newline char in debug messages Fix missing newline char in debug messages. Signed-off-by: Marek Szyprowski Tested-by: Javier Martinez Canillas Signed-off-by: Sylwester Nawrocki Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/exynos-gsc/gsc-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c index aa4db1dfc438..c7693c6296bf 100644 --- a/drivers/media/platform/exynos-gsc/gsc-core.c +++ b/drivers/media/platform/exynos-gsc/gsc-core.c @@ -1151,7 +1151,7 @@ static int gsc_runtime_resume(struct device *dev) struct gsc_dev *gsc = dev_get_drvdata(dev); int ret = 0; - pr_debug("gsc%d: state: 0x%lx", gsc->id, gsc->state); + pr_debug("gsc%d: state: 0x%lx\n", gsc->id, gsc->state); ret = clk_prepare_enable(gsc->clock); if (ret) @@ -1173,7 +1173,7 @@ static int gsc_runtime_suspend(struct device *dev) if (!ret) clk_disable_unprepare(gsc->clock); - pr_debug("gsc%d: state: 0x%lx", gsc->id, gsc->state); + pr_debug("gsc%d: state: 0x%lx\n", gsc->id, gsc->state); return ret; } #endif From 8b7b4b04cd804ba5fa2e25fc5e5c05a9592784f0 Mon Sep 17 00:00:00 2001 From: Marek Szyprowski Date: Wed, 9 Nov 2016 12:24:01 -0200 Subject: [PATCH 19/24] exynos-gsc: Use of_device_get_match_data() helper Replace open-coded driver data extraction code with generic helper. Signed-off-by: Marek Szyprowski Tested-by: Javier Martinez Canillas Signed-off-by: Sylwester Nawrocki Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/exynos-gsc/gsc-core.c | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c index c7693c6296bf..f0d8e2433299 100644 --- a/drivers/media/platform/exynos-gsc/gsc-core.c +++ b/drivers/media/platform/exynos-gsc/gsc-core.c @@ -24,6 +24,7 @@ #include #include #include +#include #include #include "gsc-core.h" @@ -988,24 +989,12 @@ static const struct of_device_id exynos_gsc_match[] = { }; MODULE_DEVICE_TABLE(of, exynos_gsc_match); -static void *gsc_get_drv_data(struct platform_device *pdev) -{ - struct gsc_driverdata *driver_data = NULL; - const struct of_device_id *match; - - match = of_match_node(exynos_gsc_match, pdev->dev.of_node); - if (match) - driver_data = (struct gsc_driverdata *)match->data; - - return driver_data; -} - static int gsc_probe(struct platform_device *pdev) { struct gsc_dev *gsc; struct resource *res; - struct gsc_driverdata *drv_data = gsc_get_drv_data(pdev); struct device *dev = &pdev->dev; + const struct gsc_driverdata *drv_data = of_device_get_match_data(dev); int ret; gsc = devm_kzalloc(dev, sizeof(struct gsc_dev), GFP_KERNEL); From c5e87b68cd80b1559478967164cd23e4550657f1 Mon Sep 17 00:00:00 2001 From: Marek Szyprowski Date: Wed, 9 Nov 2016 12:29:38 -0200 Subject: [PATCH 20/24] exynos-gsc: Add support for Exynos5433 specific version This patch adds support for Exynos5433 specific version of the GScaler module. The main difference between Exynos 5433 and earlier is addition of new clocks that have to be controlled. Signed-off-by: Marek Szyprowski Reviewed-by: Javier Martinez Canillas Tested-by: Javier Martinez Canillas Acked-by: Krzysztof Kozlowski Signed-off-by: Sylwester Nawrocki Signed-off-by: Mauro Carvalho Chehab --- .../devicetree/bindings/media/exynos5-gsc.txt | 3 +- drivers/media/platform/exynos-gsc/gsc-core.c | 74 ++++++++++++++----- drivers/media/platform/exynos-gsc/gsc-core.h | 6 +- 3 files changed, 62 insertions(+), 21 deletions(-) diff --git a/Documentation/devicetree/bindings/media/exynos5-gsc.txt b/Documentation/devicetree/bindings/media/exynos5-gsc.txt index 5fe9372abb37..26ca25b6d264 100644 --- a/Documentation/devicetree/bindings/media/exynos5-gsc.txt +++ b/Documentation/devicetree/bindings/media/exynos5-gsc.txt @@ -3,7 +3,8 @@ G-Scaler is used for scaling and color space conversion on EXYNOS5 SoCs. Required properties: -- compatible: should be "samsung,exynos5-gsc" +- compatible: should be "samsung,exynos5-gsc" (for Exynos 5250, 5420 and + 5422 SoCs) or "samsung,exynos5433-gsc" (Exynos 5433) - reg: should contain G-Scaler physical address location and length. - interrupts: should contain G-Scaler interrupt number diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c index f0d8e2433299..cbf75b6194b4 100644 --- a/drivers/media/platform/exynos-gsc/gsc-core.c +++ b/drivers/media/platform/exynos-gsc/gsc-core.c @@ -29,8 +29,6 @@ #include "gsc-core.h" -#define GSC_CLOCK_GATE_NAME "gscl" - static const struct gsc_fmt gsc_formats[] = { { .name = "RGB565", @@ -978,6 +976,19 @@ static struct gsc_driverdata gsc_v_100_drvdata = { [3] = &gsc_v_100_variant, }, .num_entities = 4, + .clk_names = { "gscl" }, + .num_clocks = 1, +}; + +static struct gsc_driverdata gsc_5433_drvdata = { + .variant = { + [0] = &gsc_v_100_variant, + [1] = &gsc_v_100_variant, + [2] = &gsc_v_100_variant, + }, + .num_entities = 3, + .clk_names = { "pclk", "aclk", "aclk_xiu", "aclk_gsclbend" }, + .num_clocks = 4, }; static const struct of_device_id exynos_gsc_match[] = { @@ -985,6 +996,10 @@ static const struct of_device_id exynos_gsc_match[] = { .compatible = "samsung,exynos5-gsc", .data = &gsc_v_100_drvdata, }, + { + .compatible = "samsung,exynos5433-gsc", + .data = &gsc_5433_drvdata, + }, {}, }; MODULE_DEVICE_TABLE(of, exynos_gsc_match); @@ -996,6 +1011,7 @@ static int gsc_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; const struct gsc_driverdata *drv_data = of_device_get_match_data(dev); int ret; + int i; gsc = devm_kzalloc(dev, sizeof(struct gsc_dev), GFP_KERNEL); if (!gsc) @@ -1011,6 +1027,7 @@ static int gsc_probe(struct platform_device *pdev) return -EINVAL; } + gsc->num_clocks = drv_data->num_clocks; gsc->variant = drv_data->variant[gsc->id]; gsc->pdev = pdev; @@ -1029,18 +1046,24 @@ static int gsc_probe(struct platform_device *pdev) return -ENXIO; } - gsc->clock = devm_clk_get(dev, GSC_CLOCK_GATE_NAME); - if (IS_ERR(gsc->clock)) { - dev_err(dev, "failed to get clock~~~: %s\n", - GSC_CLOCK_GATE_NAME); - return PTR_ERR(gsc->clock); + for (i = 0; i < gsc->num_clocks; i++) { + gsc->clock[i] = devm_clk_get(dev, drv_data->clk_names[i]); + if (IS_ERR(gsc->clock[i])) { + dev_err(dev, "failed to get clock: %s\n", + drv_data->clk_names[i]); + return PTR_ERR(gsc->clock[i]); + } } - ret = clk_prepare_enable(gsc->clock); - if (ret) { - dev_err(&gsc->pdev->dev, "clock prepare failed for clock: %s\n", - GSC_CLOCK_GATE_NAME); - return ret; + for (i = 0; i < gsc->num_clocks; i++) { + ret = clk_prepare_enable(gsc->clock[i]); + if (ret) { + dev_err(dev, "clock prepare failed for clock: %s\n", + drv_data->clk_names[i]); + while (--i >= 0) + clk_disable_unprepare(gsc->clock[i]); + return ret; + } } ret = devm_request_irq(dev, res->start, gsc_irq_handler, @@ -1075,13 +1098,15 @@ static int gsc_probe(struct platform_device *pdev) err_v4l2: v4l2_device_unregister(&gsc->v4l2_dev); err_clk: - clk_disable_unprepare(gsc->clock); + for (i = gsc->num_clocks - 1; i >= 0; i--) + clk_disable_unprepare(gsc->clock[i]); return ret; } static int gsc_remove(struct platform_device *pdev) { struct gsc_dev *gsc = platform_get_drvdata(pdev); + int i; pm_runtime_get_sync(&pdev->dev); @@ -1089,7 +1114,8 @@ static int gsc_remove(struct platform_device *pdev) v4l2_device_unregister(&gsc->v4l2_dev); vb2_dma_contig_clear_max_seg_size(&pdev->dev); - clk_disable_unprepare(gsc->clock); + for (i = 0; i < gsc->num_clocks; i++) + clk_disable_unprepare(gsc->clock[i]); pm_runtime_put_noidle(&pdev->dev); @@ -1139,12 +1165,18 @@ static int gsc_runtime_resume(struct device *dev) { struct gsc_dev *gsc = dev_get_drvdata(dev); int ret = 0; + int i; pr_debug("gsc%d: state: 0x%lx\n", gsc->id, gsc->state); - ret = clk_prepare_enable(gsc->clock); - if (ret) - return ret; + for (i = 0; i < gsc->num_clocks; i++) { + ret = clk_prepare_enable(gsc->clock[i]); + if (ret) { + while (--i >= 0) + clk_disable_unprepare(gsc->clock[i]); + return ret; + } + } gsc_hw_set_sw_reset(gsc); gsc_wait_reset(gsc); @@ -1157,10 +1189,14 @@ static int gsc_runtime_suspend(struct device *dev) { struct gsc_dev *gsc = dev_get_drvdata(dev); int ret = 0; + int i; ret = gsc_m2m_suspend(gsc); - if (!ret) - clk_disable_unprepare(gsc->clock); + if (ret) + return ret; + + for (i = gsc->num_clocks - 1; i >= 0; i--) + clk_disable_unprepare(gsc->clock[i]); pr_debug("gsc%d: state: 0x%lx\n", gsc->id, gsc->state); return ret; diff --git a/drivers/media/platform/exynos-gsc/gsc-core.h b/drivers/media/platform/exynos-gsc/gsc-core.h index e5aa8f42f11e..696217e9af66 100644 --- a/drivers/media/platform/exynos-gsc/gsc-core.h +++ b/drivers/media/platform/exynos-gsc/gsc-core.h @@ -33,6 +33,7 @@ #define GSC_SHUTDOWN_TIMEOUT ((100*HZ)/1000) #define GSC_MAX_DEVS 4 +#define GSC_MAX_CLOCKS 4 #define GSC_M2M_BUF_NUM 0 #define GSC_MAX_CTRL_NUM 10 #define GSC_SC_ALIGN_4 4 @@ -307,6 +308,8 @@ struct gsc_variant { */ struct gsc_driverdata { struct gsc_variant *variant[GSC_MAX_DEVS]; + const char *clk_names[GSC_MAX_CLOCKS]; + int num_clocks; int num_entities; }; @@ -330,7 +333,8 @@ struct gsc_dev { struct platform_device *pdev; struct gsc_variant *variant; u16 id; - struct clk *clock; + int num_clocks; + struct clk *clock[GSC_MAX_CLOCKS]; void __iomem *regs; wait_queue_head_t irq_queue; struct gsc_m2m_device m2m; From 7fd1633aea330ce65642e01439d51cd75a7b6472 Mon Sep 17 00:00:00 2001 From: Shailendra Verma Date: Fri, 2 Dec 2016 02:45:27 -0200 Subject: [PATCH 21/24] exynos-gsc: Clean up file handle in open() error path The file handle is not yet added in the vfd list. So no need to call v4l2_fh_del(&ctx->fh) if it fails to create controls. Signed-off-by: Shailendra Verma Signed-off-by: Sylwester Nawrocki Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/exynos-gsc/gsc-m2m.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c b/drivers/media/platform/exynos-gsc/gsc-m2m.c index f49f24b4462a..82505025d96c 100644 --- a/drivers/media/platform/exynos-gsc/gsc-m2m.c +++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c @@ -675,8 +675,8 @@ static int gsc_m2m_open(struct file *file) error_ctrls: gsc_ctrls_delete(ctx); -error_fh: v4l2_fh_del(&ctx->fh); +error_fh: v4l2_fh_exit(&ctx->fh); kfree(ctx); unlock: From 0e5e57d687787bc780de9260501cdffe32485c42 Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Thu, 19 Jan 2017 20:36:19 -0200 Subject: [PATCH 22/24] exynos-gsc: Fix unbalanced pm_runtime_enable() error Commit a006c04e6218 ("[media] exynos-gsc: Fixup clock management at ->remove()") changed the driver's .remove function logic to fist do a pm_runtime_get_sync() to make sure the device is powered before attempting to gate the gsc clock. But the commit also removed a pm_runtime_disable() call that leads to an unbalanced pm_runtime_enable() error if the driver is removed and re-probed: exynos-gsc 13e00000.video-scaler: Unbalanced pm_runtime_enable! exynos-gsc 13e10000.video-scaler: Unbalanced pm_runtime_enable! Fixes: a006c04e6218 ("[media] exynos-gsc: Fixup clock management at ->remove()") Signed-off-by: Javier Martinez Canillas Acked-by: Marek Szyprowski Signed-off-by: Sylwester Nawrocki Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/exynos-gsc/gsc-core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c index cbf75b6194b4..83272f10722d 100644 --- a/drivers/media/platform/exynos-gsc/gsc-core.c +++ b/drivers/media/platform/exynos-gsc/gsc-core.c @@ -1118,6 +1118,7 @@ static int gsc_remove(struct platform_device *pdev) clk_disable_unprepare(gsc->clock[i]); pm_runtime_put_noidle(&pdev->dev); + pm_runtime_disable(&pdev->dev); dev_dbg(&pdev->dev, "%s driver unloaded\n", pdev->name); return 0; From 4dc432d022bf2bb027ba17547cd12292e3d76b19 Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Tue, 24 Jan 2017 19:42:26 -0200 Subject: [PATCH 23/24] exynos-gsc: Avoid spamming the log on VIDIOC_TRY_FMT There isn't an ioctl to enum the supported field orders, so a user-space application can call VIDIOC_TRY_FMT using different field orders to know if one is supported. For example, GStreamer does this so during playback dozens of the following messages appear in the kernel log buffer: [ 442.143393] Not supported field order(4) Instead of printing this as an error, just keep it as debug information. Signed-off-by: Javier Martinez Canillas Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/exynos-gsc/gsc-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c index 83272f10722d..cbb03768f5d7 100644 --- a/drivers/media/platform/exynos-gsc/gsc-core.c +++ b/drivers/media/platform/exynos-gsc/gsc-core.c @@ -408,7 +408,7 @@ int gsc_try_fmt_mplane(struct gsc_ctx *ctx, struct v4l2_format *f) if (pix_mp->field == V4L2_FIELD_ANY) pix_mp->field = V4L2_FIELD_NONE; else if (pix_mp->field != V4L2_FIELD_NONE) { - pr_err("Not supported field order(%d)\n", pix_mp->field); + pr_debug("Not supported field order(%d)\n", pix_mp->field); return -EINVAL; } From 72e0e3302e35a28646c6f4e94a619ade7bfecac0 Mon Sep 17 00:00:00 2001 From: OtherCrashOverride Date: Sun, 5 Mar 2017 07:33:38 +0000 Subject: [PATCH 24/24] ODROID-XU4: Enable G2D --- arch/arm/boot/dts/exynos5420.dtsi | 9 +++++++++ arch/arm/configs/odroidxu3_defconfig | 3 ++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi index f85ed2c2951c..2dc638d0b371 100644 --- a/arch/arm/boot/dts/exynos5420.dtsi +++ b/arch/arm/boot/dts/exynos5420.dtsi @@ -191,6 +191,15 @@ clock-names = "fout_epll", "pll_ref", "pll_in", "sclk_audio", "sclk_pcm_in"; }; + g2d@10850000 { + compatible = "samsung,exynos5250-g2d"; + reg = <0x10850000 0x1000>; + interrupts = <0 91 0>; + clocks = <&clock CLK_G2D>; + clock-names = "fimg2d"; + iommus = <&sysmmu_g2dr>, <&sysmmu_g2dw>; + }; + mfc: codec@11000000 { compatible = "samsung,mfc-v7"; reg = <0x11000000 0x10000>; diff --git a/arch/arm/configs/odroidxu3_defconfig b/arch/arm/configs/odroidxu3_defconfig index ff7d99f01d44..11ec72a52225 100644 --- a/arch/arm/configs/odroidxu3_defconfig +++ b/arch/arm/configs/odroidxu3_defconfig @@ -3229,7 +3229,7 @@ CONFIG_VIDEO_EXYNOS4_ISP_DMA_CAPTURE=y # CONFIG_VIDEO_XILINX is not set CONFIG_V4L_MEM2MEM_DRIVERS=y # CONFIG_VIDEO_MEM2MEM_DEINTERLACE is not set -CONFIG_VIDEO_SAMSUNG_S5P_G2D=y +# CONFIG_VIDEO_SAMSUNG_S5P_G2D is not set CONFIG_VIDEO_SAMSUNG_S5P_JPEG=y CONFIG_VIDEO_SAMSUNG_S5P_MFC=y CONFIG_VIDEO_SAMSUNG_EXYNOS_GSC=y @@ -3545,6 +3545,7 @@ CONFIG_DRM_EXYNOS_HDMI=y # # Sub-drivers # +CONFIG_DRM_EXYNOS_G2D=y CONFIG_DRM_EXYNOS_IPP=y CONFIG_DRM_EXYNOS_FIMC=y CONFIG_DRM_EXYNOS_ROTATOR=y