[libcamera-devel] [PATCH V2] libcamera: MediaDevice: Make MediaDevice::setupLink account for existing link flags

[Kieran Bingham]

Hi Dan,

On 27/08/2020 12:54, Dan Scally wrote:
>> I'm still interested to know how you hit this? What platform are
>> you testing on ?
> 
> x86, specifically a Lenovo Miix-510. I'm working with the
> surface-linux guys trying to get webcams working on Microsoft Surface
> and similar devices. We're either patching DSDT or using a bridge
> driver to connect the sensors to the cio2 infrastructure, but trying
> to take images using either cam or qcam threw the error that this
> clears (though media-ctl got past it).
> 
> With this patch in, my webcam's functional.

Oh! That's fantastic news,

Are you able to share some of the information on the work you guys have
done with the DSDT patching or bridge drivers?

We've had plenty of people asking about how to make use of the IPU3 on
platforms we can't otherwise support because of that. There's quite a
few Dell laptops that also use it from what I recall.

--
Kieran


> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Thursday, 27 August 2020 12:48,
> Kieran Bingham <kieran.bingham at ideasonboard.com> wrote:
> 
>> +libcamera-devel
>> 
> 
>> On 27/08/2020 12:11, Dan Scally wrote:
>> 
> 
>>> Hi Kieran
>>> 
> 
>>>> Git commit messages should aim to be wrapped at 72 chars,
>>>> though it can be longer when it makes sense. (the above goes to
>>>> 96).
>>> 
> 
>>>> We can rewrap when applying though.
>>> 
> 
>>> Mea culpa - I'll stick to the limit in future. Happy to do a V3
>>> if you prefer, fixing that and the SoB.
>> 
> 
>> No worries, I think it's ok.
>> 
> 
>> Unless there's any comments from anyone else I have it fixed up
>> locally already.
>> 
> 
>>>> This is missing a Signed-off-by tag from you.
>>> 
> 
>>>> Can you confirm you are happy to add the following please? (as
>>>> per the DCO: https://www.libcamera.org/contributing.html)
>>> 
> 
>>>> Signed-off-by: Dan Scally djrscally at protonmail.com
>>> 
> 
>>> Yes, absolutely. Sorry - first time patching anything, knew I'd
>>> forget something!
>> 
> 
>> no problem, Thank you for contributing!
>> 
> 
>> I'm still interested to know how you hit this? What platform are
>> you testing on ?
>> 
> 
>> -----------------------------------------------------------------------------------------------------------------------------
>>
>
>>  Kieran
>> 
> 
>>> Thanks Dan ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Thursday, 27
>>> August 2020 11:42, Kieran Bingham kieran.bingham at ideasonboard.com
>>> wrote:
>>> 
> 
>>>> Hi Dan,
>>> 
> 
>>>> On 26/08/2020 17:05, Dan Scally wrote:
>>> 
> 
>>>>> The setupLink function fails (ioctl returns EINVAL) when it
>>>>> passes the MEDIA_LNK_FL_ENABLE flag to a link that is already
>>>>> flagged with MEDIA_LNK_FL_ENABLE and MEDIA_LNK_FL_IMMUTABLE.
>>>>> Contrast to media-ctl's equivalent media_setup_link() which
>>>>> ORs the new flags with the existing values. This patch
>>>>> modifies the behaviour of setupLink() to behave the same as
>>>>> media_setup_link() in media-ctl.
>>> 
> 
>>>> Git commit messages should aim to be wrapped at 72 chars,
>>>> though it can be longer when it makes sense. (the above goes to
>>>> 96).
>>> 
> 
>>>> We can rewrap when applying though.
>>> 
> 
>>>>> ref
>>>>> https://git.linuxtv.org/v4l-utils.git/tree/utils/media-ctl/libmediactl.c#n210
>>>
>
>>>>> 
>>>> This is missing a Signed-off-by tag from you.
>>> 
> 
>>>> Can you confirm you are happy to add the following please? (as
>>>> per the DCO: https://www.libcamera.org/contributing.html)
>>> 
> 
>>>> Signed-off-by: Dan Scally djrscally at protonmail.com
>>> 
> 
>>>> With that,
>>> 
> 
>>>> Reviewed-by: Kieran Bingham kieran.bingham at ideasonboard.com
>>> 
> 
>>>> Eeep, git-am had issue applying this mail, possibly due to utf 
>>>> formatting or such, but I've fixed up locally.
>>> 
> 
>>>> With confirmation from Dan on the SoB, and another RB tag, I'll
>>>> handle integration of this patch as I have it locally.
>>> 
> 
>>>> 
> 
>>> 
> 
>>>> Kieran
>>> 
> 
>>>>> Changelog:
>>> 
> 
>>>>> v2 - Simplified by removing the call to link() to fetch a
>>>>> link that's already passed as a parameter to the function.
>>>>> 
> 
>>> 
> 
>>>>> 
> 
>>> 
> 
>>>>> src/libcamera/media_device.cpp | 2 +- 1 file changed, 1
>>>>> insertion(+), 1 deletion(-) diff --git
>>>>> a/src/libcamera/media_device.cpp
>>>>> b/src/libcamera/media_device.cpp index de18d57..007a45b
>>>>> 100644 --- a/src/libcamera/media_device.cpp +++
>>>>> b/src/libcamera/media_device.cpp @@ -794,7 +794,7 @@ int
>>>>> MediaDevice::setupLink(const MediaLink *link, unsigned int
>>>>> flags) linkDesc.sink.index = sink->index(); 
>>>>> linkDesc.sink.flags = MEDIA_PAD_FL_SINK;
>>> 
> 
>>>>> -   linkDesc.flags = flags;
>>> 
> 
>>>>> -   linkDesc.flags = flags | (link->flags() &
>>>>> MEDIA_LNK_FL_IMMUTABLE); int ret = ioctl(fd_,
>>>>> MEDIA_IOC_SETUP_LINK, &linkDesc); if (ret) {
>>>>> 
> 
>>> 
> 
>>>>> 
> 
>>> 
> 
>>>>> libcamera-devel mailing list 
>>>>> libcamera-devel at lists.libcamera.org 
>>>>> https://lists.libcamera.org/listinfo/libcamera-devel
>>> 
> 
>>>> --
>>> 
> 
>>>> Regards
>>> 
> 
>>>> 
> 
>>> 
> 
>>>> Kieran
>> 
> 
>> --
>> 
> 
>> Regards
>> 
> 
>> --------
>> 
> 
>> Kieran
> 

-- 
Regards
--
Kieran