dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: "Noralf Trønnes" <noralf@tronnes.org>
To: "Marco Diego Aurélio Mesquita" <marcodiegomesquita@gmail.com>
Cc: Hans de Goede <hdegoede@redhat.com>,
	Emil Velikov <emil.l.velikov@gmail.com>,
	ML dri-devel <dri-devel@lists.freedesktop.org>,
	Daniel Vetter <daniel.vetter@intel.com>
Subject: Re: [RFC 0/1] drm: Add Grain Media GM12U320 kms driver
Date: Wed, 7 Jun 2017 23:14:24 +0200	[thread overview]
Message-ID: <ef65267e-7fc6-dbac-9ee5-fd4e72e16a35@tronnes.org> (raw)
In-Reply-To: <e8b4c6be-8923-c5a7-c77b-879314263ab9@tronnes.org>


Den 07.06.2017 22.19, skrev Noralf Trønnes:
>
> Den 07.06.2017 21.50, skrev Marco Diego Aurélio Mesquita:
>> On Wed, Jun 7, 2017 at 4:38 PM, Noralf Trønnes <noralf@tronnes.org> 
>> wrote:
>>> Den 07.06.2017 20.46, skrev Marco Diego Aurélio Mesquita:
>>>> Hi Noralf!
>>>>
>>>> On Wed, Jun 7, 2017 at 1:56 PM, Noralf Trønnes <noralf@tronnes.org> 
>>>> wrote:
>>>>> tinydrm is targeted at displays on slow busses where userspace tells
>>>>> the driver which part of the framebuffer it has changed to minimize
>>>>> the transfer.
>>>>>
>>>>> The PL111 driver uses drm_simple_display_pipe and might be a better
>>>>> example for your use case:
>>>>> https://cgit.freedesktop.org/drm/drm-misc/tree/drivers/gpu/drm/pl111
>>>> I'll investigate how to modify the pl111 driver to fit my needs then.
>>>> Do you think is there any change that must be done to the
>>>> pl111_get_panel function regarding the call to
>>>> of_graph_get_next_endpoint? I mean, the udl driver (which the gm12u320
>>>> driver is based on) does not have a panel; would any change be needed
>>>> because of this?
>>>
>>> I guess you can keep your get_modes and do something like this:
>>>
>>> static const struct drm_connector_helper_funcs gm12u320_helper_funcs 
>>> = {
>>>      .get_modes = gm12u320_get_modes,
>>> };
>>>
>>> .best_encoder has a default that fits your 1 encoder case.
>>>
>>> static const struct drm_connector_funcs gm12u320_connector_funcs = {
>>>      .fill_modes = drm_helper_probe_single_connector_modes,
>>>      .destroy = gm12u320_connector_destroy,
>>>      .detect = gm12u320_detect,
>>>      .dpms = drm_atomic_helper_connector_dpms,
>>>      .reset = drm_atomic_helper_connector_reset,
>>>      .atomic_duplicate_state = 
>>> drm_atomic_helper_connector_duplicate_state,
>>>      .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>>> };
>>>
>>> Then you init and add helper:
>>>      drm_connector_init(dev, connector, &gm12u320_connector_funcs,
>>>                 DRM_MODE_CONNECTOR_Unknown);
>>>      drm_connector_helper_add(connector, &gm12u320_helper_funcs);
>>>
>>> And finally attach the connector to the pipe:
>>>      drm_simple_display_pipe_init(..., connector);
>>>
>>> The connector is registered when the drm device is registered.
>>>
>> Thanks Noralf! That's exactly what I did and I'll test it soon. The
>> parts that scare me the most are the calls to
>> of_graph_get_next_endpoint, of_property_read_u32_array and of_node_put
>> (in display.c, for example). I'm still not sure those will work.
>
> You don't need those since you don't have a panel nor those pads:
> https://cgit.freedesktop.org/drm/drm-misc/tree/Documentation/devicetree/bindings/display/arm,pl11x.txt 
>
>
> The reason I pointed you to the PL111 driver is because it's very simple,
> so it's easy to see how the simple pipeline works.
> It uses CMA memory and if you can use that all is well, but if you need
> to access the framebuffer from the cpu, it's not so good, because cma
> memory has uncached reads.
> The udl driver handles memory in a different way.
>
> I'm out of my depths here with memory and GEM and stuff.
>

I see that you access the framebuffer in gm12u320_copy_fb_to_blocks().
That will be be very slow with CMA memory. I have worked around this in
tinydrm by caching one line at a time to speed it up. I don't know if
this will work for you. tinydrm displays are very small so the overhead
isn't big compared to the transfer speed.
See http://elixir.free-electrons.com/linux/latest/ident/tinydrm_swab16

Maybe you can cache one cacheline size at a time? After all, you have to
pull it through the cpu cache. But as I said, memory isn't my expertise.

It greatly simplifies your driver if you can use the cma library though.
If so, maybe you even can use tinydrm with a connector of your own. You
can just make your own version of tinydrm_display_pipe_init().

Noralf.

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

  reply	other threads:[~2017-06-07 21:14 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-01 11:59 [RFC 0/1] drm: Add Grain Media GM12U320 kms driver Hans de Goede
2017-06-01 11:59 ` [RFC] " Hans de Goede
2017-06-20  8:57   ` Daniel Vetter
2017-06-01 22:46 ` [RFC 0/1] " Marco Diego Aurélio Mesquita
2017-06-02 16:14   ` Emil Velikov
2017-06-02 16:39     ` Marco Diego Aurélio Mesquita
2017-06-07 10:30       ` Emil Velikov
2017-06-07 15:35         ` Marco Diego Aurélio Mesquita
2017-06-07 16:56           ` Noralf Trønnes
2017-06-07 18:46             ` Marco Diego Aurélio Mesquita
2017-06-07 19:38               ` Noralf Trønnes
2017-06-07 19:50                 ` Marco Diego Aurélio Mesquita
2017-06-07 20:19                   ` Noralf Trønnes
2017-06-07 21:14                     ` Noralf Trønnes [this message]
2017-06-08  7:08                       ` Hans de Goede
2017-06-09 20:59                         ` Marco Diego Aurélio Mesquita
2017-06-09 22:31                           ` Noralf Trønnes
2017-06-10 17:14                             ` Marco Diego Aurélio Mesquita
2017-06-10 18:39                               ` Hans de Goede
2017-06-11 17:20                                 ` Marco Diego Aurélio Mesquita
2017-06-11 19:22                                   ` Hans de Goede
2017-06-11 20:24                                   ` Noralf Trønnes
2017-06-10  9:49                           ` Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ef65267e-7fc6-dbac-9ee5-fd4e72e16a35@tronnes.org \
    --to=noralf@tronnes.org \
    --cc=daniel.vetter@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=emil.l.velikov@gmail.com \
    --cc=hdegoede@redhat.com \
    --cc=marcodiegomesquita@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).