From efd11cc8fa1a6d8aba16bd6aaa774ecc3d02145f Mon Sep 17 00:00:00 2001 From: Mark yao Date: Sat, 27 May 2017 19:43:36 +0800 Subject: drm/rockchip: Correct vop out_mode configure Force vop output mode on encoder driver seem not a good idea, EDP, HDMI, DisplayPort all have 10bit input on rk3399, On non-10bit vop, vop 8bit output bit[0-7] connect to the encoder high 8bit [2-9]. So force RGB10 to RGB888 on vop driver would be better. And another problem, EDP check crtc id on atomic_check, but encoder maybe NULL, so out_mode configure would fail, it cause edp no display. Signed-off-by: Mark Yao Reviewed-by: Jeffy Chen Link: http://patchwork.freedesktop.org/patch/msgid/1495885416-22216-1-git-send-email-mark.yao@rock-chips.com --- drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 12 ------------ drivers/gpu/drm/rockchip/cdn-dp-core.c | 9 ++------- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 8 ++++++++ drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 3 +++ drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 2 ++ 5 files changed, 15 insertions(+), 19 deletions(-) (limited to 'drivers/gpu/drm') diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c index d8fa7a9c9240..ce5f2d1f9994 100644 --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c @@ -245,8 +245,6 @@ rockchip_dp_drm_encoder_atomic_check(struct drm_encoder *encoder, struct drm_connector_state *conn_state) { struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc_state); - struct rockchip_dp_device *dp = to_dp(encoder); - int ret; /* * The hardware IC designed that VOP must output the RGB10 video @@ -258,16 +256,6 @@ rockchip_dp_drm_encoder_atomic_check(struct drm_encoder *encoder, s->output_mode = ROCKCHIP_OUT_MODE_AAAA; s->output_type = DRM_MODE_CONNECTOR_eDP; - if (dp->data->chip_type == RK3399_EDP) { - /* - * For RK3399, VOP Lit must code the out mode to RGB888, - * VOP Big must code the out mode to RGB10. - */ - ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, - encoder); - if (ret > 0) - s->output_mode = ROCKCHIP_OUT_MODE_P888; - } return 0; } diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c index a2169dd3d26b..14fa1f8351e8 100644 --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c @@ -615,7 +615,6 @@ static void cdn_dp_encoder_enable(struct drm_encoder *encoder) { struct cdn_dp_device *dp = encoder_to_dp(encoder); int ret, val; - struct rockchip_crtc_state *state; ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, encoder); if (ret < 0) { @@ -625,14 +624,10 @@ static void cdn_dp_encoder_enable(struct drm_encoder *encoder) DRM_DEV_DEBUG_KMS(dp->dev, "vop %s output to cdn-dp\n", (ret) ? "LIT" : "BIG"); - state = to_rockchip_crtc_state(encoder->crtc->state); - if (ret) { + if (ret) val = DP_SEL_VOP_LIT | (DP_SEL_VOP_LIT << 16); - state->output_mode = ROCKCHIP_OUT_MODE_P888; - } else { + else val = DP_SEL_VOP_LIT << 16; - state->output_mode = ROCKCHIP_OUT_MODE_AAAA; - } ret = cdn_dp_grf_write(dp, GRF_SOC_CON9, val); if (ret) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index 3f7a82d1e095..45589d6ce65e 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -875,6 +875,7 @@ static bool vop_crtc_mode_fixup(struct drm_crtc *crtc, static void vop_crtc_enable(struct drm_crtc *crtc) { struct vop *vop = to_vop(crtc); + const struct vop_data *vop_data = vop->data; struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc->state); struct drm_display_mode *adjusted_mode = &crtc->state->adjusted_mode; u16 hsync_len = adjusted_mode->hsync_end - adjusted_mode->hsync_start; @@ -967,6 +968,13 @@ static void vop_crtc_enable(struct drm_crtc *crtc) DRM_DEV_ERROR(vop->dev, "unsupported connector_type [%d]\n", s->output_type); } + + /* + * if vop is not support RGB10 output, need force RGB10 to RGB888. + */ + if (s->output_mode == ROCKCHIP_OUT_MODE_AAAA && + !(vop_data->feature & VOP_FEATURE_OUTPUT_RGB10)) + s->output_mode = ROCKCHIP_OUT_MODE_P888; VOP_CTRL_SET(vop, out_mode, s->output_mode); VOP_CTRL_SET(vop, htotal_pw, (htotal << 16) | hsync_len); diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h index 5a4faa85dbd2..9979fd0c2282 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h @@ -142,6 +142,9 @@ struct vop_data { const struct vop_intr *intr; const struct vop_win_data *win; unsigned int win_size; + +#define VOP_FEATURE_OUTPUT_RGB10 BIT(0) + u64 feature; }; /* interrupt define */ diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c index 0da44442aab0..bafd698a28b1 100644 --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c @@ -275,6 +275,7 @@ static const struct vop_intr rk3288_vop_intr = { static const struct vop_data rk3288_vop = { .init_table = rk3288_init_reg_table, .table_size = ARRAY_SIZE(rk3288_init_reg_table), + .feature = VOP_FEATURE_OUTPUT_RGB10, .intr = &rk3288_vop_intr, .ctrl = &rk3288_ctrl_data, .win = rk3288_vop_win_data, @@ -343,6 +344,7 @@ static const struct vop_reg_data rk3399_init_reg_table[] = { static const struct vop_data rk3399_vop_big = { .init_table = rk3399_init_reg_table, .table_size = ARRAY_SIZE(rk3399_init_reg_table), + .feature = VOP_FEATURE_OUTPUT_RGB10, .intr = &rk3399_vop_intr, .ctrl = &rk3399_ctrl_data, /* -- cgit v1.2.3 From 869e188a35c93f1bad7bf16c32f34a6feb5f79f1 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 31 May 2017 10:38:13 +0200 Subject: drm: Fix locking in drm_atomic_helper_resume In the conversion to drop drm_modeset_lock_all and the magic implicit context I failed to realize that _resume starts out with a pile of state copies, but not with the locks. And hence drm_atomic_commit won't grab these for us. v2: Add locking checks in helpers to make sure we catch this in the future. Note we can only require the locks in the atomic_check phase, not in the commit phase. But since any commit is guaranteed to first run the checks (even for the resume stuff where we use stored duplicated old state) this should give us full coverage. Requested by Maarten. Cc: Jyri Sarha Fixes: a5b8444e289c ("drm/atomic-helper: remove modeset_lock_all from helper_resume") Cc: Maarten Lankhorst Reviewed-by: Maarten Lankhorst Cc: Daniel Vetter Cc: Jani Nikula Cc: Sean Paul Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/20170531083813.1390-1-daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_atomic_helper.c | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'drivers/gpu/drm') diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 8be9719284b0..aa885a614e27 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -508,6 +508,8 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, bool has_connectors = !!new_crtc_state->connector_mask; + WARN_ON(!drm_modeset_is_locked(&crtc->mutex)); + if (!drm_mode_equal(&old_crtc_state->mode, &new_crtc_state->mode)) { DRM_DEBUG_ATOMIC("[CRTC:%d:%s] mode changed\n", crtc->base.id, crtc->name); @@ -551,6 +553,8 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, for_each_oldnew_connector_in_state(state, connector, old_connector_state, new_connector_state, i) { const struct drm_connector_helper_funcs *funcs = connector->helper_private; + WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex)); + /* * This only sets crtc->connectors_changed for routing changes, * drivers must set crtc->connectors_changed themselves when @@ -650,6 +654,8 @@ drm_atomic_helper_check_planes(struct drm_device *dev, for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) { const struct drm_plane_helper_funcs *funcs; + WARN_ON(!drm_modeset_is_locked(&plane->mutex)); + funcs = plane->helper_private; drm_atomic_helper_plane_changed(state, old_plane_state, new_plane_state, plane); @@ -2663,7 +2669,12 @@ int drm_atomic_helper_resume(struct drm_device *dev, drm_modeset_acquire_init(&ctx, 0); while (1) { + err = drm_modeset_lock_all_ctx(dev, &ctx); + if (err) + goto out; + err = drm_atomic_helper_commit_duplicated_state(state, &ctx); +out: if (err != -EDEADLK) break; -- cgit v1.2.3 From 75fb636324a839c2c31be9f81644034c6142e469 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 1 Jun 2017 13:54:30 +0200 Subject: drm: Fix oops + Xserver hang when unplugging USB drm devices MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit commit a39be606f99d ("drm: Do a full device unregister when unplugging") causes backtraces like this one when unplugging an usb drm device while it is in use: usb 2-3: USB disconnect, device number 25 ------------[ cut here ]------------ WARNING: CPU: 0 PID: 242 at drivers/gpu/drm/drm_mode_config.c:424 drm_mode_config_cleanup+0x220/0x280 [drm] ... RIP: 0010:drm_mode_config_cleanup+0x220/0x280 [drm] ... Call Trace: gm12u320_modeset_cleanup+0xe/0x10 [gm12u320] gm12u320_driver_unload+0x35/0x70 [gm12u320] drm_dev_unregister+0x3c/0xe0 [drm] drm_unplug_dev+0x12/0x60 [drm] gm12u320_usb_disconnect+0x36/0x40 [gm12u320] usb_unbind_interface+0x72/0x280 device_release_driver_internal+0x158/0x210 device_release_driver+0x12/0x20 bus_remove_device+0x104/0x180 device_del+0x1d2/0x350 usb_disable_device+0x9f/0x270 usb_disconnect+0xc6/0x260 ... [drm:drm_mode_config_cleanup [drm]] *ERROR* connector Unknown-1 leaked! ------------[ cut here ]------------ WARNING: CPU: 0 PID: 242 at drivers/gpu/drm/drm_mode_config.c:458 drm_mode_config_cleanup+0x268/0x280 [drm] ... ---[ end trace 80df975dae439ed6 ]--- general protection fault: 0000 [#1] SMP ... Call Trace: ? __switch_to+0x225/0x450 drm_mode_rmfb_work_fn+0x55/0x70 [drm] process_one_work+0x193/0x3c0 worker_thread+0x4a/0x3a0 ... RIP: drm_framebuffer_remove+0x62/0x3f0 [drm] RSP: ffffb776c39dfd98 ---[ end trace 80df975dae439ed7 ]--- After which the system is unusable this is caused by drm_dev_unregister getting called immediately on unplug, which calls the drivers unload function which calls drm_mode_config_cleanup which removes the framebuffer object while userspace is still holding a reference to it. Reverting commit a39be606f99d ("drm: Do a full device unregister when unplugging") leads to the following oops on unplug instead, when userspace closes the last fd referencing the drm_dev: sysfs group 'power' not found for kobject 'card1-Unknown-1' ------------[ cut here ]------------ WARNING: CPU: 0 PID: 2459 at fs/sysfs/group.c:237 sysfs_remove_group+0x80/0x90 ... RIP: 0010:sysfs_remove_group+0x80/0x90 ... Call Trace: dpm_sysfs_remove+0x57/0x60 device_del+0xfd/0x350 device_unregister+0x1a/0x60 drm_sysfs_connector_remove+0x39/0x50 [drm] drm_connector_unregister+0x5a/0x70 [drm] drm_connector_unregister_all+0x45/0xa0 [drm] drm_modeset_unregister_all+0x12/0x30 [drm] drm_dev_unregister+0xca/0xe0 [drm] drm_put_dev+0x32/0x60 [drm] drm_release+0x2f3/0x380 [drm] __fput+0xdf/0x1e0 ... ---[ end trace ecfb91ac85688bbe ]--- BUG: unable to handle kernel NULL pointer dereference at 00000000000000a8 IP: down_write+0x1f/0x40 ... Call Trace: debugfs_remove_recursive+0x55/0x1b0 drm_debugfs_connector_remove+0x21/0x40 [drm] drm_connector_unregister+0x62/0x70 [drm] drm_connector_unregister_all+0x45/0xa0 [drm] drm_modeset_unregister_all+0x12/0x30 [drm] drm_dev_unregister+0xca/0xe0 [drm] drm_put_dev+0x32/0x60 [drm] drm_release+0x2f3/0x380 [drm] __fput+0xdf/0x1e0 ... ---[ end trace ecfb91ac85688bbf ]--- This is caused by the revert moving back to drm_unplug_dev calling drm_minor_unregister which does: device_del(minor->kdev); dev_set_drvdata(minor->kdev, NULL); /* safety belt */ drm_debugfs_cleanup(minor); Causing the sysfs entries to already be removed even though we still have references to them in e.g. drm_connector. Note we must call drm_minor_unregister to notify userspace of the unplug of the device, so calling drm_dev_unregister is not completely wrong the problem is that drm_dev_unregister does too much. This commit fixes drm_unplug_dev by not only reverting commit a39be606f99d ("drm: Do a full device unregister when unplugging") but by also adding a call to drm_modeset_unregister_all before the drm_minor_unregister calls to make sure all sysfs entries are removed before calling device_del(minor->kdev) thereby also fixing the second set of oopses caused by just reverting the commit. Fixes: a39be606f99d ("drm: Do a full device unregister when unplugging") Cc: stable@vger.kernel.org Cc: Chris Wilson Cc: Jeffy Cc: Marco Diego Aurélio Mesquita Reported-by: Marco Diego Aurélio Mesquita Signed-off-by: Hans de Goede Reviewed-by: Chris Wilson Signed-off-by: Sean Paul Link: http://patchwork.freedesktop.org/patch/msgid/20170601115430.4113-1-hdegoede@redhat.com --- drivers/gpu/drm/drm_drv.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'drivers/gpu/drm') diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index b5c6bb46a425..37b8ad3e30d8 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -358,7 +358,12 @@ EXPORT_SYMBOL(drm_put_dev); void drm_unplug_dev(struct drm_device *dev) { /* for a USB device */ - drm_dev_unregister(dev); + if (drm_core_check_feature(dev, DRIVER_MODESET)) + drm_modeset_unregister_all(dev); + + drm_minor_unregister(dev, DRM_MINOR_PRIMARY); + drm_minor_unregister(dev, DRM_MINOR_RENDER); + drm_minor_unregister(dev, DRM_MINOR_CONTROL); mutex_lock(&drm_global_mutex); -- cgit v1.2.3