From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 25DA2226529 for ; Thu, 19 Dec 2024 16:29:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.129.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734625748; cv=none; b=auOfmUkS3PHd3hdXSZNQ7gksi+ZsRwGGmpNmn5b1jbEOnLP2R7fsTiTcvbRJue+iJoGhXZOvfQgG7Ltt9AY6c+56JR0hO7heD2l+X+y+S18BH+mE2uOpu4RhId0bGRXkUrYmM9UD0AAF6VWxG3qpAd2dVMWqBJ7V4BtearNjmiI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734625748; c=relaxed/simple; bh=oKIq5vnmx1xQOzVkLsM5Wqe3GsjrviAYVLCXlbjtP04=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=vF8vH96UA0XjB5L2+WGXF1OMfSQzZ+vsLC6UwCFuSKr1zb7j504BWXMfICm+YyKMGwBWrERIVGx5CLOZZXqLVPvEfUJidIvzgKw+ZbwFWVNxPB6je+mDbyE5yj8yqnuVwWeJL0M7zvXEYehv+5a31g/7iaDCVhmJpLqiLiHwmWY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=PLKHnT0o; arc=none smtp.client-ip=170.10.129.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="PLKHnT0o" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1734625745; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=So5R73QcSisDFY3wMbd2J1hSFrs51I/ICzSiUu4MnMg=; b=PLKHnT0ozo33k2aDaSFDIR/D+/sKvzfIsaPo6i/PaXyznbCbQdGOtXnmq3m1iUAiquTkKI D4cNFHzRzbNpuXFgkYhwRqCz6D6xWZCR4qhE2F4egl2tzxfNRF5JrRvOnCFVhvwfTokDMb cq7+78B4OstwiX4kcpm6PVFFtbvJ9Ng= Received: from mail-ed1-f70.google.com (mail-ed1-f70.google.com [209.85.208.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-449-lUnOUcNpPKuoJJOUQzsCAA-1; Thu, 19 Dec 2024 11:29:03 -0500 X-MC-Unique: lUnOUcNpPKuoJJOUQzsCAA-1 X-Mimecast-MFC-AGG-ID: lUnOUcNpPKuoJJOUQzsCAA Received: by mail-ed1-f70.google.com with SMTP id 4fb4d7f45d1cf-5d3d6d924c1so1050031a12.2 for ; Thu, 19 Dec 2024 08:29:03 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1734625742; x=1735230542; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=So5R73QcSisDFY3wMbd2J1hSFrs51I/ICzSiUu4MnMg=; b=UnSnOiFWejpECuYDdrNOaWGIFCrMJ5/gf+IIRoFO27WDTIB8rJBjEo/t6fIs9pjdUc 30Olzr9bjwCkx7IX56Qosn/BPHPWeyPKOx14C8Vo8GaT+hFSS8zNL5ERBvE0JhRXgM9G dGmOida8frlBAeRLIGJg2S5GC6lmXcHECQ6apZmA44BJl+xHUPD0EKzfWv/JPicHpggV BF/zts4Kgj9eaI68BrLa3PFkL8xSYXpcNUnmhoE0mcYgCTibi2ZXNtJ/8TRCn0lv3kjl bDuJrXDbrnzc7akAL4zDYrwIZk0vcy3rjvlIBKjUzdF7daCXCz4igFaKoGK55iCmyHQ4 6org== X-Forwarded-Encrypted: i=1; AJvYcCXXaJpxEgatm8+YcRtkjLEp0T/iZFC3BClegB5Of4eqmYWQxqgCvkssbd2El+jaGHT5zuov7dyI7wFLhxY=@vger.kernel.org X-Gm-Message-State: AOJu0Yxe/MNTKr+CmCriRAhQ8igzVLtzeIE5YLOrxw3+HDM8KPdoH1Pf 5X39KB0ePvR6VM7eFstTElZNZwKD++aObNihYxcqaIEyr4j/4shlHi+f27DP2GItUw4nU/gpc/I W0VeqxxVnPbJiJPnoeJEGvNb5t18ZzIMqhgbFyc+aW0/CaQ3/4npzW76Sz67ABg== X-Gm-Gg: ASbGncusaXpALLfse5rjR61jF9LOC8oPU/6WNl3s/6yelSKpVaywG3ZiH0S8qC6hbl7 w6QBipcuHegGGKoM6QaWJ8l33kHwBU3r7O+9UuJdyxTAsxP1EpR/Th/9zV1jH6ypeHNpM8Wh9a6 z/ECQKT8b2w657TL1Ukv1yChJGLAg2qjEaH87c3jFEST/KdjhKeRtWa0FbieBLgfcOFaktOk872 ZtMBMSenNcc/Skg+2vJakRpX29TbDvZDXOYhNFCJLHgTAoyoa+2ozDLHKQs++xuXiAy4a3lI75v LREWfsFiyL7CxLRLrGLyaGs5jc72VtqpC1Gk27EmW+0X/PxJBtolMNIrT3idG6G2CVLLRDlOu4w b5WtmhVpxJmHSSfAv/EfkDGadALt6qxk= X-Received: by 2002:a05:6402:350d:b0:5d1:fb79:c1b2 with SMTP id 4fb4d7f45d1cf-5d8025c743emr3370225a12.11.1734625742420; Thu, 19 Dec 2024 08:29:02 -0800 (PST) X-Google-Smtp-Source: AGHT+IFg79ItjB5SIzLn6TdKcIl17PUCrEkAPLeQR65C/1Y7SIrhTQkXTpDrepiNCNXWFZyk9vze2w== X-Received: by 2002:a05:6402:350d:b0:5d1:fb79:c1b2 with SMTP id 4fb4d7f45d1cf-5d8025c743emr3370194a12.11.1734625741939; Thu, 19 Dec 2024 08:29:01 -0800 (PST) Received: from ?IPV6:2001:1c00:c32:7800:5bfa:a036:83f0:f9ec? (2001-1c00-0c32-7800-5bfa-a036-83f0-f9ec.cable.dynamic.v6.ziggo.nl. [2001:1c00:c32:7800:5bfa:a036:83f0:f9ec]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-5d80701b2absm783363a12.81.2024.12.19.08.29.01 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 19 Dec 2024 08:29:01 -0800 (PST) Message-ID: <9ea278a7-6ad3-4e3d-ac1d-8df3f7e97d9a@redhat.com> Date: Thu, 19 Dec 2024 17:29:00 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] media: uvcvideo: Filter hw errors while enumerating controls To: Ricardo Ribalda Cc: Laurent Pinchart , Hans Verkuil , Mauro Carvalho Chehab , linux-media@vger.kernel.org, linux-kernel@vger.kernel.org References: <20241213-uvc-eaccess-v1-1-62e0b4fcc634@chromium.org> <20241218232730.GF5518@pendragon.ideasonboard.com> <20241219144124.GB2510@pendragon.ideasonboard.com> <20241219154103.GD19884@pendragon.ideasonboard.com> <372a4cd2-60ea-4ce8-848f-318e34d62cbf@redhat.com> Content-Language: en-US, nl From: Hans de Goede In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Hi, On 19-Dec-24 5:18 PM, Ricardo Ribalda wrote: > On Thu, 19 Dec 2024 at 17:05, Hans de Goede wrote: >> >> Hi, >> >> On 19-Dec-24 4:53 PM, Ricardo Ribalda wrote: >>> On Thu, 19 Dec 2024 at 16:41, Laurent Pinchart >>> wrote: >>>> >>>> On Thu, Dec 19, 2024 at 04:35:37PM +0100, Ricardo Ribalda wrote: >>>>> On Thu, 19 Dec 2024 at 15:41, Laurent Pinchart wrote: >>>>>> On Thu, Dec 19, 2024 at 09:17:31AM +0100, Ricardo Ribalda wrote: >>>>>>> On Thu, 19 Dec 2024 at 00:27, Laurent Pinchart wrote: >>>>>>>> On Fri, Dec 13, 2024 at 11:21:02AM +0000, Ricardo Ribalda wrote: >>>>>>>>> To implement VIDIOC_QUERYCTRL, we need to read from the hardware all the >>>>>>>>> values that were not cached previously. If that read fails, we used to >>>>>>>>> report back the error to the user. >>>>>>>>> >>>>>>>>> Unfortunately this does not play nice with userspace. When they are >>>>>>>>> enumerating the contols, the only expect an error when there are no >>>>>>>>> "next" control. >>>>>>>>> >>>>>>>>> This is probably a corner case, and could be handled in userspace, but >>>>>>>>> both v4l2-ctl and yavta fail to enumerate all the controls if we return >>>>>>>>> then -EIO during VIDIOC_QUERYCTRL. I suspect that there are tons of >>>>>>>>> userspace apps handling this wrongly as well. >>>>>>>>> >>>>>>>>> This patch get around this issue by ignoring the hardware errors and >>>>>>>>> always returning 0 if a control exists. >>>>>>>>> >>>>>>>>> Signed-off-by: Ricardo Ribalda >>>>>>>>> --- >>>>>>>>> Hi 2*Hans and Laurent! >>>>>>>>> >>>>>>>>> I came around a device that was listing just a couple of controls when >>>>>>>>> it should be listing much more. >>>>>>>>> >>>>>>>>> Some debugging latter I found that the device was returning -EIO when >>>>>>>>> all the focal controls were read. >>>>>>>> >>>>>>>> Was it transient and random errors, or does the device always fail for >>>>>>>> those controls ? >>>>>>> >>>>>>> For one of the devices the control is always failing (or I could not >>>>>>> find a combination that made it work). >>>>>>> >>>>>>> For the other it was more or less random. >>>>>> >>>>>> Are there other controls that failed for that device ? And what >>>>>> request(s) fail, is it only GET_CUR or also GET_MIN/GET_MAX/GET_RES ? >>>>> >>>>> It is a mix. >>>>> >>>>>> What's the approximate frequency of those random failures ? >>>>>> >>>>>>>>> This could be solved in userspace.. but I suspect that a lot of people >>>>>>>>> has copied the implementation of v4l-utils or yavta. >>>>>>>>> >>>>>>>>> What do you think of this workaround? >>>>>>>> >>>>>>>> Pretending that the control could be queried is problematic. We'll >>>>>>>> return invalid values to the user, I don't think that's a good idea. If >>>>>>>> the problematic device always returns error for focus controls, we could >>>>>>>> add a quirk, and extend the uvc_device_info structure to list the >>>>>>>> controls to ignore. >>>>>>>> >>>>>>>> Another option would be to skip over controls that return -EIO within >>>>>>>> the kernel, and mark those controls are broken. I think this could be >>>>>>>> done transparently for userspace, the first time we try to populate the >>>>>>>> cache for such controls, a -EIO error would mark the control as broken, >>>>>>>> and from a userspace point of view it wouldn't be visible through as >>>>>>>> ioctl. >>>>>>> >>>>>>> I see a couple of issues with this: >>>>>>> - There are controls that fail randomly. >>>>>>> - There are controls that fail based on the value of other controls >>>>>>> (yeah, I know). >>>>>> >>>>>> I was fearing there would be random (or random-looking) failures, as >>>>>> that can preclude marking the controls as broken and fully hiding them >>>>>> from userspace :-( >>>>>> >>>>>>> - There are controls that do not implement RES, MIN, or MAX, but >>>>>>> besides that, they are completely functional. >>>>>>> In any of those cases we do not want to skip those controls. >>>>>>> >>>>>>> I am not against quirking specific cameras once we detect that they >>>>>>> are broken... >>>>>> >>>>>> Hopefully there won't be too many of those, right ? Righhhht... ? >>>>> >>>>> So far I have identified 4 in a week, and I am not testing obscure >>>>> camera modules.... >>>> >>>> Can you provide more information about those modules ? USB descriptors >>>> maybe, and the list of controls that fail, and how they fail ? >>> >>> These are the ones I can share now: >>> >>> "13d3:5519": Focus value out of range >>> focus_absolute 0x009a090a (int) : min=355 max=790 step=1 default=6 >>> value=500 flags=inactive >> >> Hmm this one looks like min and default are swapped ? >> >> So I guess this one needs a quirk which checks if default < min >> and in that case swaps them (the check is to avoid swapping >> with fixed fw). If these are built into chromebooks how about >> doing a fwupdate for the camera instead ? > > We do fwupdate whenever possible. But some modules are not updateable. > They either: lack DFU, or the flash is read-only, or the update > process has a non acceptable fail rate. Ok, I was just wondering if we could avoid having to add a quirk for this model. > We aim to detect compliance errors early in the development process. > V4L2-compliance now (almost) works with the uvcvideo driver. And that > helps a lot :) > > I plan to add quirks for the cameras that I can test. But we still > need a solution for all the external cameras and modules that are not > in the lab. I agree we need some solution for this, especially the broken controls hiding others is a problem which needs some workaround. I'm not sure what that workaround should look like though. Just returning 0 as your v2 patch does seems less then ideal. Lets continue discussing this after the Christmas break. Regards, Hans >>> "3277:0003": Focus returns -EIO >>> Focus Absolute and Focus, Automatic Continuous: return -EIO for at >>> least one of get_ max/min/res >>> >>> "0408:302f": Error reading AutoExposure Flags >>> UVC_GET_INFO returns invalid flags >> >> Regards, >> >> Hans >> >> >> > >