[RFC 0/1] drm: Add Grain Media GM12U320 kms driver

[Hans de Goede]

Hi,

On 10-06-17 19:14, Marco Diego Aurélio Mesquita wrote:
> Hi Devs!
> 
> On Fri, Jun 9, 2017 at 7:31 PM, Noralf Trønnes <noralf at tronnes.org> wrote:
>>
>> Den 09.06.2017 22.59, skrev Marco Diego Aurélio Mesquita:
>>>
>>> Hi Devs!
>>>
>>> On Thu, Jun 8, 2017 at 4:08 AM, Hans de Goede <hdegoede at redhat.com> wrote:
>>>>
>>>> I don't think that using cma for the gm12u320 is a good idea, it will
>>>> typically be used as a secondary GPU output together with a real GPU
>>>> extending the desktop by being a prime display output. So for the memory
>>>> management stuff I would keep the code copied from the udl driver (which
>>>> we may later split out in a separate helper lib for devices where
>>>> the framebuffer is in normal system memory and we have some
>>>> scather-gather
>>>> capable process copying it to the real device over e.g. USB)
>>>>
>>> I got the PL111 driver and stripped all device specific code. Also, I
>>> added the get_modes and driver usb probe functions from gm12u320. The
>>> resulting code is available in
>>> https://gitlab.com/marcodiego/dummy-display-driver .
>>>
>>> The driver compiles, loads, identifies the device when I plug it,
>>> /dev/fb1 is created adequately but it stops there. Gnome monitors tool
>>> does not see it as a new monitor. What I expected was that it would be
>>> seen as a new monitor and that I could even activate it. Instead,
>>> syslog complains: "Cannot find any crtc or sizes".
>>>
>>> Anyway, I think this is a first step. My first doubt: where should I
>>> go from now? What should I change in the driver so that gnome monitors
>>> tool sees it as a new monitor and I could activate it, even if it
>>> works as a mere dummy device?
>>>
>>> Second doubt is: is this really the simplest/best way? I mean, the
>>> repaper driver that Emil pointed seems a lot simpler, wouldn't it be
>>> better to mimic it? How complicated is it to modify the repaper driver
>>> to build a dummy driver?
>>>
>>> By dummy driver, I mean something that gnome monitors tools can
>>> identify as a new monitor and I can activate it. I hope to reach a
>>> point where the update or dirty callback is called. From that point on
>>> it is just a matter of sending the framebuffer through USB the way
>>> gm12u320 does currently.
>>
>>
>> I guess you have forgotten to replace connector detect, because I
>> didn't see you use drm_panel in your RFC.
>> (gm12u320_connector->panel is NULL afaict, so it's always disconnected)
>>
>> static enum drm_connector_status gm12u320_connector_detect(struct
>> drm_connector
>>                              *connector, bool force)
>> {
>> -    struct gm12u320_drm_connector *gm12u320_connector =
>> -        to_gm12u320_connector(connector);
>> -
>> -    return (gm12u320_connector->panel ?
>> -        connector_status_connected :
>> -        connector_status_disconnected);
>> +    if (drm_device_is_unplugged(connector->dev))
>> +        return connector_status_disconnected;
>> +
>> +    return connector_status_connected;
>> }
>>
>>
> 
> I applied this change and now gnome monitors tool identifies it as
> another monitor, thanks Noralf! There are still some problems: when I
> activate it, gnome complains "could not set configuration for CRTC
> 284". I still don't know what causes this. Any idea?
> 
> Other problem I get is that the device is spontaneously disconnected
> after some time. I'm not sure what causes this, but I think it happens
> because it receives no data after an amount of time.

Correct, there is no way to blank (think DPMS off) the device, but
it will turn itself of it does not get any data for more then x seconds,
I don't remember what X was.

> On the udl-based
> driver, a workqueue is used to periodically send some data to the
> device. Is there any problem using the same approach here? I mean,
> won't using a workqueue to periodically send some data to the device
> bring any conflict? If not, I'll start porting the code related to the
> workqueue from the udl-based driver to the pl111-based driver.

Yes that is the right thing to do, notice that the workqueue is not
just there to fix the timeout issue, the workqueue is there to send a
full copy of the framebuffer (partial updates are not supported, but
USB-3 is fast enough to not worry about this) to the device, it will
this whenever it gets woken because either userspace gives us a new
framebuffer (flip) or userspace has marked some area dirty marking.

If the workqueue is not woken it will resend the last framebuffer after
a timeout to avoid the device turning off, but its primary use is
sending the framebuffer when updated.

> That will probably improve the behavior of the device. Once I get it
> behaving properly, my next step will be to finally try to update the
> framebuffer of the device and get it to show something.
> 
> Does this approach seem sound?

This sounds good to me.

Regards,

Hans