Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753327AbcKHGcv (ORCPT ); Tue, 8 Nov 2016 01:32:51 -0500 Received: from mail-wm0-f67.google.com ([74.125.82.67]:35499 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752549AbcKHGct (ORCPT ); Tue, 8 Nov 2016 01:32:49 -0500 Subject: Re: [PATCH v2] drm/udl: Ensure channel is selected before using the device. To: Jamie Lentin , David Airlie , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org References: <20160815064328.GZ6232@phenom.ffwll.local> <1471904254-26545-1-git-send-email-jm@lentin.co.uk> <20160823055758.GI24290@phenom.ffwll.local> From: poma Message-ID: Date: Tue, 8 Nov 2016 07:32:45 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20160823055758.GI24290@phenom.ffwll.local> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5475 Lines: 143 On 23.08.2016 07:57, Daniel Vetter wrote: > On Mon, Aug 22, 2016 at 11:17:34PM +0100, Jamie Lentin wrote: >> Lift configuration command from udlfb. This appears to be essential for >> at least a Rextron VCUD-60, without which no URB communication occurs. >> >> Signed-off-by: Jamie Lentin >> --- >> udl_encoder_commit() is too late to do this set up in it seems. This >> setup doesn't need to be performed again after a suspend, although this >> is somewhat academic until I send the patch adding suspend and resume >> functions. >> >> Tested with a Rextron VCUD-60 attached to a Thinkpad X201s on Linux 4.7.0 > > Applied to drm-misc, thanks. > -Daniel > >> >> Cheers, >> --- >> drivers/gpu/drm/udl/udl_main.c | 25 +++++++++++++++++++++++++ >> 1 file changed, 25 insertions(+) >> >> diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c >> index 33dbfb2..29f0207 100644 >> --- a/drivers/gpu/drm/udl/udl_main.c >> +++ b/drivers/gpu/drm/udl/udl_main.c >> @@ -16,6 +16,8 @@ >> /* -BULK_SIZE as per usb-skeleton. Can we get full page and avoid overhead? */ >> #define BULK_SIZE 512 >> >> +#define NR_USB_REQUEST_CHANNEL 0x12 >> + >> #define MAX_TRANSFER (PAGE_SIZE*16 - BULK_SIZE) >> #define WRITES_IN_FLIGHT (4) >> #define MAX_VENDOR_DESCRIPTOR_SIZE 256 >> @@ -90,6 +92,26 @@ success: >> return true; >> } >> >> +/* >> + * Need to ensure a channel is selected before submitting URBs >> + */ >> +static int udl_select_std_channel(struct udl_device *udl) >> +{ >> + int ret; >> + u8 set_def_chn[] = {0x57, 0xCD, 0xDC, 0xA7, >> + 0x1C, 0x88, 0x5E, 0x15, >> + 0x60, 0xFE, 0xC6, 0x97, >> + 0x16, 0x3D, 0x47, 0xF2}; >> + >> + ret = usb_control_msg(udl->udev, >> + usb_sndctrlpipe(udl->udev, 0), >> + NR_USB_REQUEST_CHANNEL, >> + (USB_DIR_OUT | USB_TYPE_VENDOR), 0, 0, >> + set_def_chn, sizeof(set_def_chn), >> + USB_CTRL_SET_TIMEOUT); >> + return ret < 0 ? ret : 0; >> +} >> + >> static void udl_release_urb_work(struct work_struct *work) >> { >> struct urb_node *unode = container_of(work, struct urb_node, >> @@ -301,6 +323,9 @@ int udl_driver_load(struct drm_device *dev, unsigned long flags) >> goto err; >> } >> >> + if (udl_select_std_channel(udl)) >> + DRM_ERROR("Selecting channel failed\n"); >> + >> if (!udl_alloc_urb_list(dev, WRITES_IN_FLIGHT, MAX_TRANSFER)) { >> DRM_ERROR("udl_alloc_urb_list failed\n"); >> goto err; >> -- >> 2.8.1 >> > It doesn't breaks the device, however under the hood it is boiling, ------------[ cut here ]------------ WARNING: CPU: 0 PID: 381 at drivers/usb/core/hcd.c:1584 usb_hcd_map_urb_for_dma+0x49f/0x770 transfer buffer not dma capable Modules linked in: ... udl(+) ... drm_kms_helper ... drm ... CPU: 0 PID: 381 Comm: systemd-udevd Not tainted 4.9.0-0.rc4.git0.1.fc26.x86_64+debug #1 ... Call Trace: [] dump_stack+0x86/0xc3 [] __warn+0xcb/0xf0 [] warn_slowpath_fmt+0x5f/0x80 [] ? debug_dma_mapping_error+0x7b/0x90 [] usb_hcd_map_urb_for_dma+0x49f/0x770 [] ? trace_hardirqs_on_caller+0xf5/0x1b0 [] usb_hcd_submit_urb+0x34d/0xb50 [] ? mark_held_locks+0x76/0xa0 [] ? __raw_spin_lock_init+0x21/0x60 [] ? trace_hardirqs_on_caller+0xf5/0x1b0 [] usb_submit_urb+0x2f4/0x560 [] ? lockdep_init_map+0x61/0x210 [] usb_start_wait_urb+0x74/0x180 [] usb_control_msg+0xdc/0x120 [] udl_driver_load+0x141/0x590 [udl] [] ? trace_hardirqs_on_caller+0xd1/0x1b0 [] drm_dev_register+0xa9/0xd0 [drm] [] udl_usb_probe+0x42/0x90 [udl] [] usb_probe_interface+0x15f/0x2d0 [] driver_probe_device+0x223/0x430 [] __driver_attach+0xe3/0xf0 [] ? driver_probe_device+0x430/0x430 [] bus_for_each_dev+0x73/0xc0 [] driver_attach+0x1e/0x20 [] bus_add_driver+0x173/0x270 [] driver_register+0x60/0xe0 [] usb_register_driver+0xaa/0x160 [] ? 0xffffffffc089a000 [] udl_driver_init+0x1e/0x1000 [udl] [] do_one_initcall+0x50/0x180 [] ? rcu_read_lock_sched_held+0x45/0x80 [] ? kmem_cache_alloc_trace+0x277/0x2d0 [] ? do_init_module+0x27/0x1f1 [] do_init_module+0x5f/0x1f1 [] load_module+0x2401/0x2b40 [] ? __symbol_put+0x70/0x70 [] ? sched_clock_cpu+0x90/0xc0 [] SYSC_init_module+0x19b/0x1c0 [] SyS_init_module+0xe/0x10 [] do_syscall_64+0x6c/0x1f0 [] entry_SYSCALL64_slow_path+0x25/0x25 ---[ end trace 1fa5e22a0dcf62da ]--- [drm:udl_driver_load [udl]] *ERROR* Selecting channel failed udl 1-2:1.0: fb1: udldrmfb frame buffer device [drm] Initialized udl on minor 1 usbcore: registered new interface driver udl Is this expected WARN? Ref. https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/udl?id=d1c151dc https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/core/hcd.c?id=refs/tags/v4.9-rc4#n1584