CRTC properties patches

[Paulo Zanoni]

Hi

Some time ago I posted a patch series to implement Kernel CRTC
properties and a "rotation" property for the i915 driver. No one
objected those patches, so now I'm resending the rebased versions, with
some minor changes and also "overscan compensation" properties. The
patches apply to the ~danvet/drm-intel tree, drm-intel-next-queued
branch.

Summary (i.e., tl;dr)
---------------------

- CRTC properties seem to be the most correct way to solve some problems
- "Rotation" property solves problems with Intel AMT KVM
- "Overscan Compensation" properties solve problems with TVs/monitors
  that do overscan
- patches for kernel, libdrm, and xf86-video-intel

Definition of the problem
-------------------------

Some of the "features" of the Intel hardware have to be configured per
pipe. In the DRM world, the pipe is abstracted as a CRTC, so the thing
that makes most sense is to implement these features as CRTC properties.

Why not connector properties? In theory, we can have more than one
connector per each pipe, so we would have to keep synchronizing the
properties with all the connectors associated with a pipe, and also
update everything each time a connector changes pipe. If we can have the
"correct" (IMHO) CRTC properties implementation, why make hackish
connector properties?

So far, there are two features that the i915 driver can/should implement
as CRTC properties: "rotation" and "overscan compensation".

Rotation
--------

(Disclaimer: this description is purely based on my own experiences with
the feature and it represents only my own opinion, not Intel's opinion.
The description might even be wrong.)

Intel AMT has a feature called KVM (Keyboard, Video, Mouse). AFAIK, this
is available for all machines labeled as "vPro" (like my Lenovo
laptops). Once you enable this feature on your machine, you can
remotely connect to the machine using standard VNC protocol
(additionally, there are also non-standard features for a closed-source
VNC client, but I've never used them). This is a very cool thing since
you don't need to install any software on the KVM machine, and you can
even reboot the machine and browse the BIOS using the VNC.

The KVM implementation reads the "screen" directly from the video
graphics registers (it's OS independent, remember?). The problem is:
whenever the screen is rotated by X, the KVM implementation needs to
know this, otherwise the mouse pointer will be moved to the wrong
direction. How do we tell the KVM that the screen is rotated? We need to
set some bits on the PIPECONF registers.

So my implementation creates a CRTC property called "rotation" and then
whenever X rotates the screen, xf86-video-intel updates the property,
and everyone gets happy. The property is not exposed to the "users".

This problem is very easy to reproduce:
- acquire a vPro machine (bro tip: use magnets for faster acquiring)
- configure AMT KVM (there's a "ctrl+p" menu somewhere when you boot)
- boot the machine, use xrandr to rotate the screen
- connect to the KVM through VNC (any client will do)
- have fun trying to use the mouse

Overscan Compensation (a.k.a. "Underscan")
------------------------------------------

Recently we have enabled Interlaced modes in our driver. One of the
problems remaining is that some TVs and monitors will enable overscan
when you use Interlaced modes (actually, some enable overscan even on
progressive modes, like my own TV). One possible solution to this
problem would be to send an AVI infoframe that asks the TV to not
overscan (this patch is on my TODO list), but most TVs/monitors don't
repect the infoframes, so another solution is to use the panel fitters
to compensate for overscan, and since this is independent from the
TV/monitor, it should work with every device (even LVDS).

I already have submitted to intel-gpu-tools a tool called
"intel_panel_fitter" that does overscan compensation using the panel
fitter (so people that don't have future kernels can already have
overscan compensation). The tool works but requires you to be root, so
now I am proposing two new CRTC properties: "underscan x" and "underscan
y".

When I implemented CRTC properties I followed the examples and made the
ioctls require you to be DRM master, so we can't just rely on having our
users calling the libdrm functions: we need to make X (or
xf86-video-intel) expose the properties to the user. How do we expose
those properties? I imagined 3 solutions:

1 - Change the xrandr protocol to include crtc properties. This would
    require a lot of releases, the code would take many months to arrive
    the distros.
2 - Implement a xorg.conf option to enable/disable underscan. At first
    this seems simple, but when you start thinking that the user wants
    underscan for just some modes on a specific output, and that
    sometimes compensation must be on and sometimes it must be off, it
    gets complicated: how do we specify this in xorg.conf?
3 - Do the hack I wanted to avoid inside the Kernel: expose the
    underscan properties as connector properties, and whenever these
    connector properties change, change the Kernel CRTC properties.

So I chose option 3 and, if people show interest, I can also implement
option 1 for the "long-term", and then deprecate option 3 once option 1
gets into the X server.

Q: But you kept pushing CRTC properties as the "right" solution and now
you expose overscan properties to the user as connector properties! Why
don't you just implement everything inside the kernel as connector
properties?

A: First of all, don't forget the "rotation" property that is used
correctly, and also all the other properties that we might create in the
future. For the overscan compensation properties specifically, having
the Kernel implement the correct solution and then xf86-video-intel
implement the "hackish" solution is better than having the Kernel
implement the "hackish" solution and force every single  app (like
Wayland) also implement a hackish solution. Also, adding CRTC properties
to RandR in the future would solve the problem.


The patches
-----------

-> Kernel

- 0001: drm: add drm_property_change_is_valid
Preparatory work to avoid code duplication in the future.

- 0002: drm: WARN() when drm_connector_attach_property fails
When I was implementing CRTC properties I was looking at how connector
properties were implemented and found this. The WARN is a must (IMHO), but
changing from void to int could be reverted if you want (then I'll also
change drm_crtc_attach_property to return int, since I want both the
connector and crtc property functions to have the same usage).

- 0003: drm: add CRTC properties
Adds the CRTC properties interface, but no properties.

- 0004: drm/i915: add 'rotation' CRTC property
First i915 property.

- 0005: drm/i915: add overscan compensation CRTC properties
Second and third i915 properties.


-> libdrm

- 0001: modetest: fix some compiler warnings
This patch is independent from CRTC properties. While learning
everything I was using the modetest tool, so I decided to fix some of
its trivial warnings.

- 0002: modetest: fix memory leak
Same as 0001: independent from CRTC properties.

- 0003: modetest: print more about our properties
Same as 0002: independent from CRTC properties.

- 0004: Add support for CRTC properties
This function adds the ioctl wrappers.

- 0005: modetest: print CRTC properties
Use the ioctls.


-> xf86-video-intel

- 0001: Avoid duplicated code with intel_output_create_ranged_atom
This is just a change that reduces code duplication. It seems worth
committing even if we reject all the other pathces. The next patches use
the new function.

- 0002: Update the rotation property whenever we change the rotation.
This patch uses the "rotation" drm property to fix the Intel AMT bugs.

- 0003: Add underscan properties
This is the patch that implements underscan properties as connector
properties instead of crtc properties.

We still lack changes inside the sna/ directory. I'm waiting for the
reviews on the non-sna code before writing the sna code. We also need to
add a dependency on the "future" libdrm release that will include the
new functions.

-- 
Paulo Zanoni