Fixing the udl driver: issues and questions

Fixing the udl driver: issues and questions

[Forest Crossman]

Hi, all,

I recently bough a bunch of udl-compatible DisplayLink adapters and
have been trying to use them in Linux, but have been running into some
issues:

- Output is limited to 16-bit color, but the device is capable of 24-bit color.
- X providers (not sure if that's the right term--it's the output of
`xrandr --listproviders`) often linger when plugging and unplugging
multiple adapters. e.g., I normally have one provider (my main GPU)
with no adapters plugged in, then I'll plug in three DisplayLink
adapters and there will be four (or five!) providers, and then I'll
unplug one and there will still be four (or five), and if I plug it in
again it'll add another provider, etc.
- The driver only uses RLE to compress data, but the devices are
capable of (better?) Huffman coding-based compression.
- Not exactly a udl issue, but the udlfb module has to be blacklisted
to enable the use of udl.
- The entire driver hangs if you plug in the adapter while preventing
the DisplayLink SoC from reading the adapter's internal I2C EEPROM.
Yes, I realize this happened because I was messing around with the
electronics inside the adapter, but a USB device should probably not
be permitted to cause a driver to hang in the first place.
- Not really a functional issue, but the driver is very "chatty" in
its dmesg output, printing a bunch of non-fatal error messages when
things go wrong.
- When things do go wrong, the driver creates the X provider even if
there's a bunch of errors in the kernel log. This is just my opinion,
but I think if a problem is so bad it prints an error message, the
device init should fail and not result in the X provider being
created. If it's really not that bad, then it should probably be just
a warning.
- The driver seems to check for the EDID of connected monitors on all
adapters whenever a new adapter is connected, regardless of whether
there are any monitors connected to any of those adapters, and then it
outputs error messages when it fails to read the EDIDs of the
unconnected monitors.
- The vendor descriptor is output with DRM_INFO when a new device is
connected, which seems unnecessarily verbose.

Since I haven't done any kernel dev before, I thought that fixing some
of these issues might be a good introduction. However, I'm not
entirely sure what I'm doing, and the commit history of the driver
doesn't seem to tell the whole story as to why certain design
decisions were made, so I have a few questions:

- Why only 16-bit color? I realize that the lower 8 bits need to be
stored and transferred in another buffer, but is there a technical
reason why this isn't already implemented? Or was there just not much
interest in adding that functionality at the time?
- Why isn't there support for the Huffman coding compression? Was it
for technical reasons, e.g., more things need to be reverse
engineered, performance, etc.? Or just lack of interest/developer
time?
- Why are there two drivers for these devices (udl and udlfb)? Would
it be possible to merge all the functionality into udl so I don't have
to blacklist udlfb on every computer I want to use these adapters
with?
- What parts of the driver should I be looking into to fix the X
provider duplication issue?
- Why is there so much pointer math? (e.g., `*cmd++ = val;`). Is this
normal for kernel code?
- Why is the driver version still "0.0.1" and the driver date
"20120220"? Should these be updated?

I apologize if any of the questions I'm asking sound silly--as I said
before, I'm new to this and I'm still learning how it all works.

I figured that fixing the 16-bit/24-bit issue wouldn't be too
difficult, so I've already done a little driver hacking and managed to
add a bunch of code to output the lower 8-bits of pixel data to the
device. Unfortunately, it's all very hacky, crashes pretty often, and
doesn't actually work. I know how to use libusb pretty well, but the
kernel USB API is new to me, so I'm pretty sure that's where most of
my problems are right now. Then again, I also just might not fully
understand the hardware yet, either.

If anyone has any answers/comments/advice on how to fix those issues,
I'd love to hear them! Also, if you need more detailed issue
descriptions, I can do some more testing and provide those as well.

Thanks!
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Fixing the udl driver: issues and questions

[Daniel Vetter]

On Tue, Aug 29, 2017 at 08:20:41PM -0500, Forest Crossman wrote:
> Hi, all,
> 
> I recently bough a bunch of udl-compatible DisplayLink adapters and
> have been trying to use them in Linux, but have been running into some
> issues:
> 
> - Output is limited to 16-bit color, but the device is capable of 24-bit color.
> - X providers (not sure if that's the right term--it's the output of
> `xrandr --listproviders`) often linger when plugging and unplugging
> multiple adapters. e.g., I normally have one provider (my main GPU)
> with no adapters plugged in, then I'll plug in three DisplayLink
> adapters and there will be four (or five!) providers, and then I'll
> unplug one and there will still be four (or five), and if I plug it in
> again it'll add another provider, etc.
> - The driver only uses RLE to compress data, but the devices are
> capable of (better?) Huffman coding-based compression.
> - Not exactly a udl issue, but the udlfb module has to be blacklisted
> to enable the use of udl.
> - The entire driver hangs if you plug in the adapter while preventing
> the DisplayLink SoC from reading the adapter's internal I2C EEPROM.
> Yes, I realize this happened because I was messing around with the
> electronics inside the adapter, but a USB device should probably not
> be permitted to cause a driver to hang in the first place.
> - Not really a functional issue, but the driver is very "chatty" in
> its dmesg output, printing a bunch of non-fatal error messages when
> things go wrong.
> - When things do go wrong, the driver creates the X provider even if
> there's a bunch of errors in the kernel log. This is just my opinion,
> but I think if a problem is so bad it prints an error message, the
> device init should fail and not result in the X provider being
> created. If it's really not that bad, then it should probably be just
> a warning.
> - The driver seems to check for the EDID of connected monitors on all
> adapters whenever a new adapter is connected, regardless of whether
> there are any monitors connected to any of those adapters, and then it
> outputs error messages when it fails to read the EDIDs of the
> unconnected monitors.
> - The vendor descriptor is output with DRM_INFO when a new device is
> connected, which seems unnecessarily verbose.
> 
> Since I haven't done any kernel dev before, I thought that fixing some
> of these issues might be a good introduction. However, I'm not
> entirely sure what I'm doing, and the commit history of the driver
> doesn't seem to tell the whole story as to why certain design
> decisions were made, so I have a few questions:
> 
> - Why only 16-bit color? I realize that the lower 8 bits need to be
> stored and transferred in another buffer, but is there a technical
> reason why this isn't already implemented? Or was there just not much
> interest in adding that functionality at the time?
> - Why isn't there support for the Huffman coding compression? Was it
> for technical reasons, e.g., more things need to be reverse
> engineered, performance, etc.? Or just lack of interest/developer
> time?
> - Why are there two drivers for these devices (udl and udlfb)? Would
> it be possible to merge all the functionality into udl so I don't have
> to blacklist udlfb on every computer I want to use these adapters
> with?

We have a bunch of other such cases, the solution is to just not compile
_any_ of the fbdev drivers. You still need fbdev itself, plus fbcon, for
the kernel console, but the fbdev drivers should all be drm drivers which
provide fbdev support through the fbdev emulation layer we have.

If your distro enables udlfb, your distro is doing it wrong.

> - What parts of the driver should I be looking into to fix the X
> provider duplication issue?
> - Why is there so much pointer math? (e.g., `*cmd++ = val;`). Is this
> normal for kernel code?
> - Why is the driver version still "0.0.1" and the driver date
> "20120220"? Should these be updated?
> 
> I apologize if any of the questions I'm asking sound silly--as I said
> before, I'm new to this and I'm still learning how it all works.
> 
> I figured that fixing the 16-bit/24-bit issue wouldn't be too
> difficult, so I've already done a little driver hacking and managed to
> add a bunch of code to output the lower 8-bits of pixel data to the
> device. Unfortunately, it's all very hacky, crashes pretty often, and
> doesn't actually work. I know how to use libusb pretty well, but the
> kernel USB API is new to me, so I'm pretty sure that's where most of
> my problems are right now. Then again, I also just might not fully
> understand the hardware yet, either.
> 
> If anyone has any answers/comments/advice on how to fix those issues,
> I'd love to hear them! Also, if you need more detailed issue
> descriptions, I can do some more testing and provide those as well.

UDL is defacto unmaintained, so if you have such a huge issue list I
recommend you dive into the code and have fun. There's some patches
floating around on dri-devel already, but because unmaintained, no one
really looks at them.

For small drm drivers we have drm-misc with commit rights, so barrier to
get this all going is super low (by kernel standards, it's still nowhere
near opening pull requests on github or similar).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Fixing the udl driver: issues and questions

[Dave Airlie]

On 30 August 2017 at 11:20, Forest Crossman <cyrozap@gmail.com> wrote:
> Hi, all,
>
> I recently bough a bunch of udl-compatible DisplayLink adapters and
> have been trying to use them in Linux, but have been running into some
> issues:
>
> - Output is limited to 16-bit color, but the device is capable of 24-bit color.
> - X providers (not sure if that's the right term--it's the output of
> `xrandr --listproviders`) often linger when plugging and unplugging
> multiple adapters. e.g., I normally have one provider (my main GPU)
> with no adapters plugged in, then I'll plug in three DisplayLink
> adapters and there will be four (or five!) providers, and then I'll
> unplug one and there will still be four (or five), and if I plug it in
> again it'll add another provider, etc.

Yeah reference counting works, you'd have to trace what delays
these. If the underlying device goes away the X server should get
a udev message to remove the provider.

> - The driver only uses RLE to compress data, but the devices are
> capable of (better?) Huffman coding-based compression.

AFAIK Displaylink have never revealed how to use that stuff, so you'd
have to reverse engineer it, also Displaylink the company are pretty
open source hostile.

> - The entire driver hangs if you plug in the adapter while preventing
> the DisplayLink SoC from reading the adapter's internal I2C EEPROM.
> Yes, I realize this happened because I was messing around with the
> electronics inside the adapter, but a USB device should probably not
> be permitted to cause a driver to hang in the first place.

Maybe add a timeout somewhere, not sure what the hang is there.

> - Not really a functional issue, but the driver is very "chatty" in
> its dmesg output, printing a bunch of non-fatal error messages when
> things go wrong.
> - When things do go wrong, the driver creates the X provider even if
> there's a bunch of errors in the kernel log. This is just my opinion,
> but I think if a problem is so bad it prints an error message, the
> device init should fail and not result in the X provider being
> created. If it's really not that bad, then it should probably be just
> a warning.
> - The driver seems to check for the EDID of connected monitors on all
> adapters whenever a new adapter is connected, regardless of whether
> there are any monitors connected to any of those adapters, and then it
> outputs error messages when it fails to read the EDIDs of the
> unconnected monitors.
> - The vendor descriptor is output with DRM_INFO when a new device is
> connected, which seems unnecessarily verbose.

Yeah patches for these would be good.

>
> Since I haven't done any kernel dev before, I thought that fixing some
> of these issues might be a good introduction. However, I'm not
> entirely sure what I'm doing, and the commit history of the driver
> doesn't seem to tell the whole story as to why certain design
> decisions were made, so I have a few questions:
>
> - Why only 16-bit color? I realize that the lower 8 bits need to be
> stored and transferred in another buffer, but is there a technical
> reason why this isn't already implemented? Or was there just not much
> interest in adding that functionality at the time?

The devices are USB2, saving bandwidth seemed like a good idea, and
doing the wierd 24-bit layering just didn't seem like a great use of time.

> - Why isn't there support for the Huffman coding compression? Was it
> for technical reasons, e.g., more things need to be reverse
> engineered, performance, etc.? Or just lack of interest/developer
> time?

It wasn't documented and required reverse engineering. Displaylink
aren't very helpful.

> - Why are there two drivers for these devices (udl and udlfb)? Would
> it be possible to merge all the functionality into udl so I don't have
> to blacklist udlfb on every computer I want to use these adapters
> with?

Distro problem, ask them not to enable udlfb, udl should cover them all.

> - What parts of the driver should I be looking into to fix the X
> provider duplication issue?

udl doesn't do much here, it's pretty much the X sever listening for udev
events to add/remove providers.

> - Why is there so much pointer math? (e.g., `*cmd++ = val;`). Is this
> normal for kernel code?

Yes.

> - Why is the driver version still "0.0.1" and the driver date
> "20120220"? Should these be updated?

Not really, these are useful for drivers that see a lot of development,
that isn't this.

As I wrote udl I feel partly responsible, but getting USB3 support
reverse engineered pretty much sapped most of my will to care about
Displaylink and their products (the company isn't interested in open
source support). So it is mostly working, and I'll fix things if I get patches
arrive, but I'm not sure investing too much time in it is worth it.

Dave.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel