Re: [PATCH/RFC 0/5] HDMI driver for Samsung S5PV310 platform

[Alex Deucher <alexdeucher@gmail.com>]

On Wed, Feb 9, 2011 at 3:59 AM, Hans Verkuil <hansverk@cisco.com> wrote:
> On Tuesday, February 08, 2011 16:28:32 Alex Deucher wrote:
>> On Tue, Feb 8, 2011 at 4:47 AM, Hans Verkuil <hansverk@cisco.com> wrote:
>
> <snip>
>
>> >>   The driver supports an interrupt. It is used to detect plug/unplug
> events
>> > in
>> >> kernel debugs.  The API for detection of such an events in V4L2 API is to
> be
>> >> defined.
>> >
>> > Cisco (i.e. a few colleagues and myself) are working on this. We hope to
> post
>> > an RFC by the end of this month. We also have a proposal for CEC support
> in
>> > the pipeline.
>>
>> Any reason to not use the drm kms APIs for modesetting, display
>> configuration, and hotplug support?  We already have the
>> infrastructure in place for complex display configurations and
>> generating events for hotplug interrupts.  It would seem to make more
>> sense to me to fix any deficiencies in the KMS APIs than to spin a new
>> API.  Things like CEC would be a natural fit since a lot of desktop
>> GPUs support hdmi audio/3d/etc. and are already using kms.
>
> There are various reasons for not going down that road. The most important one
> is that mixing APIs is actually a bad idea. I've done that once in the past
> and I've regretted ever since. The problem with doing that is that it is
> pretty hard on applications who have to mix two different styles of API,
> somehow know where to find the documentation for each and know that both APIs
> can in fact be used on the same device.
>
> Now, if there was a lot of code that could be shared, then that might be
> enough reason to go that way, but in practice there is very little overlap.
> Take CEC: all the V4L API will do is to pass the CEC packets from kernel to
> userspace and vice versa. There is no parsing at all. This is typically used
> by embedded apps that want to do their own CEC processing.
>
> An exception might be a PCI(e) card with HDMI input/output that wants to
> handle CEC internally. At that point we might look at sharing CEC parsing
> code. A similar story is true for EDID handling.
>
> One area that might be nice to look at would be to share drivers for HDMI
> receivers and transmitters. However, the infrastructure for such drivers is
> wildly different between how it is used for GPUs versus V4L and has been for
> 10 years or so. I also suspect that most GPUs have there own HDMI internal
> implementation so code sharing will probably be quite limited.
>

You don't need to worry about the rest of the 3D and acceleration
stuff to use the kms modesetting API.  For video output, you have a
timing generator, an encoder that translates a bitstream into
voltages, and an connector that you plug into a monitor.  Additionally
you may want to read an edid or generate a hotplug event and use some
modeline handling helpers.  The kms api provides core modesetting code
and a set of modesetting driver callbacks for crtcs, encoders, and
connectors.  The hardware implementations will vary, but modesetting
is the same.  From drm_crtc_helper.h:

The driver provides the following callbacks for the crtc.  The crtc
loosely refers to the part of the display pipe that generates timing
and framebuffer scanout position.

struct drm_crtc_helper_funcs {
        /*
         * Control power levels on the CRTC.  If the mode passed in is
         * unsupported, the provider must use the next lowest power
level.
         */
        void (*dpms)(struct drm_crtc *crtc, int mode);
        void (*prepare)(struct drm_crtc *crtc);
        void (*commit)(struct drm_crtc *crtc);

        /* Provider can fixup or change mode timings before modeset occurs */
        bool (*mode_fixup)(struct drm_crtc *crtc,
                           struct drm_display_mode *mode,
                           struct drm_display_mode *adjusted_mode);
        /* Actually set the mode */
        int (*mode_set)(struct drm_crtc *crtc, struct drm_display_mode *mode,
                        struct drm_display_mode *adjusted_mode, int x, int y,
                        struct drm_framebuffer *old_fb);

        /* Move the crtc on the current fb to the given position *optional* */
        int (*mode_set_base)(struct drm_crtc *crtc, int x, int y,
                             struct drm_framebuffer *old_fb);
        int (*mode_set_base_atomic)(struct drm_crtc *crtc,
                                    struct drm_framebuffer *fb, int x, int y,
                                    enum mode_set_atomic);

        /* reload the current crtc LUT */
        void (*load_lut)(struct drm_crtc *crtc);

        /* disable crtc when not in use - more explicit than dpms off */
        void (*disable)(struct drm_crtc *crtc);
};

encoders take the bitstream from the crtc and convert it into a set of
voltages understood by the monitor, e.g., TMDS or LVDS encoders.  The
callbacks follow a similar pattern to crtcs.

struct drm_encoder_helper_funcs {
        void (*dpms)(struct drm_encoder *encoder, int mode);
        void (*save)(struct drm_encoder *encoder);
        void (*restore)(struct drm_encoder *encoder);

	bool (*mode_fixup)(struct drm_encoder *encoder,
                           struct drm_display_mode *mode,
                           struct drm_display_mode *adjusted_mode);
        void (*prepare)(struct drm_encoder *encoder);
        void (*commit)(struct drm_encoder *encoder);
	void (*mode_set)(struct drm_encoder *encoder,
                         struct drm_display_mode *mode,
                         struct drm_display_mode *adjusted_mode);
        struct drm_crtc *(*get_crtc)(struct drm_encoder *encoder);
 	/* detect for DAC style encoders */
        enum drm_connector_status (*detect)(struct drm_encoder *encoder,
                                            struct drm_connector *connector);
        /* disable encoder when not in use - more explicit than dpms off */
        void (*disable)(struct drm_encoder *encoder);
};


And finally connectors.  These are the actual physical connectors on
the board (DVI-I, HDMI-A, VGA, S-video, etc.).  Things like ddc lines
are generally tied to a connector so functions relevant to getting
modelines are associated with connectors.

struct drm_connector_helper_funcs {
        int (*get_modes)(struct drm_connector *connector);
        int (*mode_valid)(struct drm_connector *connector,
                          struct drm_display_mode *mode);
        struct drm_encoder *(*best_encoder)(struct drm_connector *connector);
};

See drm_crtc_helper.c to see how the callbacks are used.  The code can
handle crtcs that can be routed to different encoders dynamically or
crtcs that are hardcoded to specific encoders.  Additionally, it can
handle encoders that are shared between multiple connectors (e.g., a
DAC shared between VGA and S-video), or multiple encoders tied to a
single connector (e.g., DAC and TMDS encoders tied to a single DVI-I
connector).

Let's look at two scenarios where you want a system to display an
interactive RGB desktop and provide video capture and video output.
You'd need to use different sets of APIs depending on what hardware
you use:

1. SoC with an LVDS panel and HDMI output and a capture unit
2. PC with a GPU with an LVDS panel and an HDMI output and a capture unit.

As I understand it, to use the scenario 1 would use the following:

- LVDS uses some new v4l interface or a hacked up kernel fb interface
to support multiple displays
- HDMI uses some new v4l interface or a hacked up kernel fb interface
to support multiple displays
- capture unit uses V4L

Scenario 2 would use:

- LVDS uses KMS API
- HDMI uses KMS API
- capture unit uses V4L

Why not use KMS?  A modesetting API is a huge amount of work.  I
haven't seen anything in these SoC platforms that makes them so
radically different that they need their own API.  If there are any,
we'd love to hear about them so they can be addressed.

Alex

> So, no, there are no plans to share anything between the two (except perhaps
> EDID and CEC parsing should that become relevant).
>
> Oh, and let me join Andy in saying that the drm/kms/whatever API documentation
> *really* needs a lot of work.
>
> Regards,
>
>        Hans
>