[PATCH v4 1/6] drm: add SimpleDRM driver

[David Herrmann]

Hi Tom

On Sat, Sep 21, 2013 at 4:18 PM, Tom Gundersen <teg at jklm.no> wrote:
> Hi David,
>
> On Sun, Sep 1, 2013 at 3:36 PM, David Herrmann <dh.herrmann at gmail.com> wrote:
>> The SimpleDRM driver binds to simple-framebuffer devices and provides a
>> DRM/KMS API. It provides only a single CRTC+encoder+connector combination
>> plus one initial mode.
>>
>> Userspace can create one dumb-buffer and attach it to the CRTC. Only if
>> the buffer is destroyed, a new buffer can be created. The buffer is
>> directly mapped into user-space, so we have only resources for a single
>> buffer. Otherwise, shadow buffers plus damage-request would be needed.
>>
>> Signed-off-by: David Herrmann <dh.herrmann at gmail.com>
>> Tested-by: Stephen Warren <swarren at nvidia.com>
>> ---
>
> [...]
>
>> +static int sdrm_conn_fill_modes(struct drm_connector *conn, uint32_t max_x,
>> +                               uint32_t max_y)
>> +{
>> +       struct sdrm_device *sdrm = conn->dev->dev_private;
>> +       struct drm_display_mode *mode;
>> +       int ret;
>> +
>> +       if (conn->force == DRM_FORCE_ON)
>> +               conn->status = connector_status_connected;
>> +       else if (conn->force)
>> +               conn->status = connector_status_disconnected;
>> +       else
>> +               conn->status = connector_status_connected;
>> +
>> +       list_for_each_entry(mode, &conn->modes, head)
>> +               mode->status = MODE_UNVERIFIED;
>> +
>> +       mode = drm_gtf_mode(sdrm->ddev, sdrm->fb_width, sdrm->fb_height,
>> +                           60, 0, 0);
>> +       if (mode) {
>> +               mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
>> +               drm_mode_probed_add(conn, mode);
>> +               sdrm->mode = mode;
>
> Should you also be setting sdrm->fb_{width,height} to
> mode->{v,h}display here? Otherwise, due to the rounding in
> drm_gtf_mode(), these values won't necessarily match (which I suppose
> they must?).

What the ****. I wasn't aware drm_gtf_mode() modifies the v/hdisplay
values. Hm.. I will probably have to use something else then.

Thanks
David