[PATCH RFC 00/11] drm/tilcdc: Atomic modeset support

2016-05-11 Thread Daniel Vetter
On Tue, May 10, 2016 at 7:30 PM, Noralf Trønnes  wrote:
>> Hm, my idea with external transcoders was to pull them in as a
>> drm_bridge. That's of course more work, and we already have a
>> proliferation of different transcoder driver standards in drm
>> unfortunately (there's drm_bridge, but als to drm_encoder_slave).
>
>
> Does this mean that you want the drm_bridge_*() functions in
> disable_outputs(), crtc_set_mode() and
> drm_atomic_helper_commit_modeset_enables() to run for
> drm_simple_display_pipe?
>
> Because "drm: Make drm_encoder_helper_funcs optional" skips those bridge
> functions if encoder->helper_private is not set (I found this pattern in
> mode_fixup() which skips drm_bridge_mode_fixup() on !funcs).
> So if that's the case, I have to add an empty drm_encoder_helper_funcs to
> the
> simple pipe.

Oops you're right. I think the correct fix would be to always run the
bridge functions though, there's really no need to break that when
there's no encoder vtable. Since I merged your initial patch already,
care to create a follow-up patch to rectify this?

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch


[PATCH RFC 00/11] drm/tilcdc: Atomic modeset support

2016-05-10 Thread Noralf Trønnes

Den 10.05.2016 16:18, skrev Daniel Vetter:
> On Tue, May 10, 2016 at 4:04 PM, Noralf Trønnes  
> wrote:
>> Den 10.05.2016 11:14, skrev Jyri Sarha:
>>> On 05/10/16 09:34, Daniel Vetter wrote:
>> There's a drm_simple_display_pipe floating around which seems perfectly
 suited to tilcdc. It's meant for the case where you have 1 plane, 1
 crtc
 and 1 encoder maybe linking to different connectors. And it takes
 care of
 all the small bits for you, with a grand total of 5 callbacks, all of
 them
 optional.

 Might indeed be useful to rebase tilcdc on top of that, should be
 possible
 to nuke piles of code.
>>
>> Looks interesting. Does it look like it is getting ready to be merged
>> soon?
 Should be landind soon, yes. Probably not for 4.7, that's closed now, but
 I can still pick it up into drm-misc right away when it's ready. Open
 review comments are all just small things, you could pick the latest
 version to start prototyping a conversion and there shouldn't be any
 surprises when you rebase onto the merged version.
>>> Hmmm, too bad it wants to own its encoder. LCDC on Beaglebone-Black
>>> needs the componentized external tda998x driver. So at least in its
>>> current form the drm_simple_display_pipe wont work for tilcdc.
>>>
>>> It may not be too big a job to add an external encoder support, but
>>> would it complicate currently so nice and simple driver structure too
>>> much?
>>
>> How about we add an argument for encoder and if it is NULL, then the no-op
>> encoder is used:
> Hm, my idea with external transcoders was to pull them in as a
> drm_bridge. That's of course more work, and we already have a
> proliferation of different transcoder driver standards in drm
> unfortunately (there's drm_bridge, but als to drm_encoder_slave).

Does this mean that you want the drm_bridge_*() functions in
disable_outputs(), crtc_set_mode() and
drm_atomic_helper_commit_modeset_enables() to run for 
drm_simple_display_pipe?

Because "drm: Make drm_encoder_helper_funcs optional" skips those bridge
functions if encoder->helper_private is not set (I found this pattern in
mode_fixup() which skips drm_bridge_mode_fixup() on !funcs).
So if that's the case, I have to add an empty drm_encoder_helper_funcs 
to the
simple pipe.

Noralf.



[PATCH RFC 00/11] drm/tilcdc: Atomic modeset support

2016-05-10 Thread Laurent Pinchart
On Tuesday 10 May 2016 16:18:54 Daniel Vetter wrote:
> On Tue, May 10, 2016 at 4:04 PM, Noralf Trønnes  
> wrote:
> > Den 10.05.2016 11:14, skrev Jyri Sarha:
> >> On 05/10/16 09:34, Daniel Vetter wrote:
> >>> There's a drm_simple_display_pipe floating around which seems
> >>> perfectly suited to tilcdc. It's meant for the case where you have 1
> >>> plane, 1 crtc and 1 encoder maybe linking to different connectors.
> >>> And it takes care of all the small bits for you, with a grand total
> >>> of 5 callbacks, all of them optional.
> >>
> >> Might indeed be useful to rebase tilcdc on top of that, should be
> >> possible to nuke piles of code.
> > 
> > Looks interesting. Does it look like it is getting ready to be merged
> > soon?
> >>> 
> >>> Should be landind soon, yes. Probably not for 4.7, that's closed now,
> >>> but
> >>> I can still pick it up into drm-misc right away when it's ready. Open
> >>> review comments are all just small things, you could pick the latest
> >>> version to start prototyping a conversion and there shouldn't be any
> >>> surprises when you rebase onto the merged version.
> >> 
> >> Hmmm, too bad it wants to own its encoder. LCDC on Beaglebone-Black
> >> needs the componentized external tda998x driver. So at least in its
> >> current form the drm_simple_display_pipe wont work for tilcdc.
> >> 
> >> It may not be too big a job to add an external encoder support, but
> >> would it complicate currently so nice and simple driver structure too
> >> much?
> > 
> > How about we add an argument for encoder and if it is NULL, then the no-op
> > encoder is used:
>
> Hm, my idea with external transcoders was to pull them in as a
> drm_bridge. That's of course more work, and we already have a
> proliferation of different transcoder driver standards in drm
> unfortunately (there's drm_bridge, but als to drm_encoder_slave).

drm_encoder_slave has to go. As a first step the adv7511 driver is being 
converted to a drm_bridge by Archit. Any volunteer for the three other drivers 
? We also need to clearly state that new drivers must use drm_bridge.

-- 
Regards,

Laurent Pinchart



[PATCH RFC 00/11] drm/tilcdc: Atomic modeset support

2016-05-10 Thread Daniel Vetter
On Tue, May 10, 2016 at 5:11 PM, Laurent Pinchart
 wrote:
>> >> Hmmm, too bad it wants to own its encoder. LCDC on Beaglebone-Black
>> >> needs the componentized external tda998x driver. So at least in its
>> >> current form the drm_simple_display_pipe wont work for tilcdc.
>> >>
>> >> It may not be too big a job to add an external encoder support, but
>> >> would it complicate currently so nice and simple driver structure too
>> >> much?
>> >
>> > How about we add an argument for encoder and if it is NULL, then the no-op
>> > encoder is used:
>>
>> Hm, my idea with external transcoders was to pull them in as a
>> drm_bridge. That's of course more work, and we already have a
>> proliferation of different transcoder driver standards in drm
>> unfortunately (there's drm_bridge, but als to drm_encoder_slave).
>
> drm_encoder_slave has to go. As a first step the adv7511 driver is being
> converted to a drm_bridge by Archit. Any volunteer for the three other drivers
> ? We also need to clearly state that new drivers must use drm_bridge.

Hm, just realized that tda998x _is_ an encoder slave driver. But since
it's not automatically wire up like drm_bridge we can't keep the dummy
encoder of drm_simple_display_pipe. Would be good indeed to have a
minimal drm_bridge conversion of tda998x so that we don't need to add
hacks to the simple pipe helpers.

A small function to set the first drm_bridge for a
drm_simple_display_pipe should be all that's really needed here. A bit
more work, but hopefully not much. And really should be worth it to
use the simple helpers for tilcdc, it matches the use-case perfectly I
think.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch


[PATCH RFC 00/11] drm/tilcdc: Atomic modeset support

2016-05-10 Thread Daniel Vetter
On Tue, May 10, 2016 at 4:04 PM, Noralf Trønnes  wrote:
> Den 10.05.2016 11:14, skrev Jyri Sarha:
>>
>> On 05/10/16 09:34, Daniel Vetter wrote:
>
> There's a drm_simple_display_pipe floating around which seems perfectly
>>>
>>> suited to tilcdc. It's meant for the case where you have 1 plane, 1
>>> crtc
>>> and 1 encoder maybe linking to different connectors. And it takes
>>> care of
>>> all the small bits for you, with a grand total of 5 callbacks, all of
>>> them
>>> optional.
>>>
>>> Might indeed be useful to rebase tilcdc on top of that, should be
>>> possible
>>> to nuke piles of code.
>
>
> Looks interesting. Does it look like it is getting ready to be merged
> soon?
>>>
>>> Should be landind soon, yes. Probably not for 4.7, that's closed now, but
>>> I can still pick it up into drm-misc right away when it's ready. Open
>>> review comments are all just small things, you could pick the latest
>>> version to start prototyping a conversion and there shouldn't be any
>>> surprises when you rebase onto the merged version.
>>
>> Hmmm, too bad it wants to own its encoder. LCDC on Beaglebone-Black
>> needs the componentized external tda998x driver. So at least in its
>> current form the drm_simple_display_pipe wont work for tilcdc.
>>
>> It may not be too big a job to add an external encoder support, but
>> would it complicate currently so nice and simple driver structure too
>> much?
>
>
> How about we add an argument for encoder and if it is NULL, then the no-op
> encoder is used:

Hm, my idea with external transcoders was to pull them in as a
drm_bridge. That's of course more work, and we already have a
proliferation of different transcoder driver standards in drm
unfortunately (there's drm_bridge, but als to drm_encoder_slave).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch


[PATCH RFC 00/11] drm/tilcdc: Atomic modeset support

2016-05-10 Thread Noralf Trønnes
Den 10.05.2016 11:14, skrev Jyri Sarha:
> On 05/10/16 09:34, Daniel Vetter wrote:
 There's a drm_simple_display_pipe floating around which seems perfectly
>> suited to tilcdc. It's meant for the case where you have 1 plane, 1 crtc
>> and 1 encoder maybe linking to different connectors. And it takes care of
>> all the small bits for you, with a grand total of 5 callbacks, all of 
>> them
>> optional.
>>
>> Might indeed be useful to rebase tilcdc on top of that, should be 
>> possible
>> to nuke piles of code.

 Looks interesting. Does it look like it is getting ready to be merged soon?
>> Should be landind soon, yes. Probably not for 4.7, that's closed now, but
>> I can still pick it up into drm-misc right away when it's ready. Open
>> review comments are all just small things, you could pick the latest
>> version to start prototyping a conversion and there shouldn't be any
>> surprises when you rebase onto the merged version.
> Hmmm, too bad it wants to own its encoder. LCDC on Beaglebone-Black
> needs the componentized external tda998x driver. So at least in its
> current form the drm_simple_display_pipe wont work for tilcdc.
>
> It may not be too big a job to add an external encoder support, but
> would it complicate currently so nice and simple driver structure too much?

How about we add an argument for encoder and if it is NULL, then the no-op
encoder is used:

int drm_simple_display_pipe_init(struct drm_device *dev,
  struct drm_simple_display_pipe *pipe,
  struct drm_simple_display_pipe_funcs *funcs,
  const uint32_t *formats,
  unsigned int format_count,
  struct drm_encoder *encoder,
  struct drm_connector *connector)
{
 struct drm_plane *plane = &pipe->plane;
 struct drm_crtc *crtc = &pipe->crtc;
 int ret;

 pipe->funcs = funcs;

 drm_plane_helper_add(plane, &drm_simple_kms_plane_helper_funcs);
 ret = drm_universal_plane_init(dev, plane, 0,
&drm_simple_kms_plane_funcs,
formats, format_count,
DRM_PLANE_TYPE_PRIMARY, NULL);
 if (ret)
 return ret;

 drm_crtc_helper_add(crtc, &drm_simple_kms_crtc_helper_funcs);
 ret = drm_crtc_init_with_planes(dev, crtc, plane, NULL,
 &drm_simple_kms_crtc_funcs, NULL);
 if (ret)
 return ret;

 if (!encoder) {
 encoder = kzalloc(sizeof(*encoder), GFP_KERNEL);
 if (!encoder)
 return -ENOMEM;

 ret = drm_encoder_init(dev, encoder,
&drm_simple_kms_encoder_funcs,
DRM_MODE_ENCODER_NONE, NULL);
 if (ret)
 return ret;
 }

 encoder->possible_crtcs = 1 << drm_crtc_index(crtc);
 ret = drm_mode_connector_attach_encoder(connector, encoder);
 if (ret)
 return ret;

 return drm_connector_register(connector);
}

static void drm_simple_kms_encoder_cleanup(struct drm_encoder *encoder)
{
 drm_encoder_cleanup(encoder);
 kfree(encoder);
}

static const struct drm_encoder_funcs drm_simple_kms_encoder_funcs = {
 .destroy = drm_simple_kms_encoder_cleanup,
};


Noralf.

>   Jyri



[PATCH RFC 00/11] drm/tilcdc: Atomic modeset support

2016-05-10 Thread Jyri Sarha
On 05/10/16 09:34, Daniel Vetter wrote:
>>> There's a drm_simple_display_pipe floating around which seems perfectly
>>> > > suited to tilcdc. It's meant for the case where you have 1 plane, 1 crtc
>>> > > and 1 encoder maybe linking to different connectors. And it takes care 
>>> > > of
>>> > > all the small bits for you, with a grand total of 5 callbacks, all of 
>>> > > them
>>> > > optional.
>>> > > 
>>> > > Might indeed be useful to rebase tilcdc on top of that, should be 
>>> > > possible
>>> > > to nuke piles of code.
>> > 
>> > 
>> > Looks interesting. Does it look like it is getting ready to be merged soon?
> Should be landind soon, yes. Probably not for 4.7, that's closed now, but
> I can still pick it up into drm-misc right away when it's ready. Open
> review comments are all just small things, you could pick the latest
> version to start prototyping a conversion and there shouldn't be any
> surprises when you rebase onto the merged version.

Hmmm, too bad it wants to own its encoder. LCDC on Beaglebone-Black
needs the componentized external tda998x driver. So at least in its
current form the drm_simple_display_pipe wont work for tilcdc.

It may not be too big a job to add an external encoder support, but
would it complicate currently so nice and simple driver structure too much?

Jyri


[PATCH RFC 00/11] drm/tilcdc: Atomic modeset support

2016-05-10 Thread Daniel Vetter
On Tue, May 10, 2016 at 12:29:23AM +0300, Jyri Sarha wrote:
> On 05/09/16 17:42, Daniel Vetter wrote:
> >> > It's not clear to me if a (primary) plane is required with atomic
> >> > universal planes and modesetting or not... I guess it is, when thinking
> >> > of how e.g. a framebuffer is configured to be shown on a screen when
> >> > using the atomic modesetting uapi.
> > You need a primary plane, but atomic doesn't require that it's enabled.
> > Which this simple display controller probably wont like, so seems like
> > this implementation of a primary plane is a bit too simple. You also need
> 
> So I do what I can, by checking in crtc check that the plane is part of
> the new state. What more should I do?g

that's not enough, since if you disable the primary plane it'll still be
part of the state, but disabled. You need to check that it actually is
enabled, and placed correctly (i.e. fullscreen). There's a helper for
that. But if you switch over to drm_simple_display_pipe that helper will
take care of things. At least in the final revision, current patch version
also lacks that check.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


[PATCH RFC 00/11] drm/tilcdc: Atomic modeset support

2016-05-10 Thread Daniel Vetter
On Tue, May 10, 2016 at 12:29:23AM +0300, Jyri Sarha wrote:
> On 05/09/16 17:42, Daniel Vetter wrote:
> >> > It's not clear to me if a (primary) plane is required with atomic
> >> > universal planes and modesetting or not... I guess it is, when thinking
> >> > of how e.g. a framebuffer is configured to be shown on a screen when
> >> > using the atomic modesetting uapi.
> > You need a primary plane, but atomic doesn't require that it's enabled.
> > Which this simple display controller probably wont like, so seems like
> > this implementation of a primary plane is a bit too simple. You also need
> 
> So I do what I can, by checking in crtc check that the plane is part of
> the new state. What more should I do?g
> 
> > a real plane for the cursor, if you want to support that with atomic.
> > 
> 
> Well, there is no such thing in LCDC.
> 
> >> > But if it is required, it makes me wonder, are there other HWs out there
> >> > without any planes? The dummy plane implementation you added is not
> >> > complex, but is it something that should be implemented with DRM helper
> >> > funcs?
> > There's a drm_simple_display_pipe floating around which seems perfectly
> > suited to tilcdc. It's meant for the case where you have 1 plane, 1 crtc
> > and 1 encoder maybe linking to different connectors. And it takes care of
> > all the small bits for you, with a grand total of 5 callbacks, all of them
> > optional.
> > 
> > Might indeed be useful to rebase tilcdc on top of that, should be possible
> > to nuke piles of code.
> 
> 
> Looks interesting. Does it look like it is getting ready to be merged soon?

Should be landind soon, yes. Probably not for 4.7, that's closed now, but
I can still pick it up into drm-misc right away when it's ready. Open
review comments are all just small things, you could pick the latest
version to start prototyping a conversion and there shouldn't be any
surprises when you rebase onto the merged version.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


[PATCH RFC 00/11] drm/tilcdc: Atomic modeset support

2016-05-10 Thread Jyri Sarha
On 05/09/16 17:42, Daniel Vetter wrote:
>> > It's not clear to me if a (primary) plane is required with atomic
>> > universal planes and modesetting or not... I guess it is, when thinking
>> > of how e.g. a framebuffer is configured to be shown on a screen when
>> > using the atomic modesetting uapi.
> You need a primary plane, but atomic doesn't require that it's enabled.
> Which this simple display controller probably wont like, so seems like
> this implementation of a primary plane is a bit too simple. You also need

So I do what I can, by checking in crtc check that the plane is part of
the new state. What more should I do?g

> a real plane for the cursor, if you want to support that with atomic.
> 

Well, there is no such thing in LCDC.

>> > But if it is required, it makes me wonder, are there other HWs out there
>> > without any planes? The dummy plane implementation you added is not
>> > complex, but is it something that should be implemented with DRM helper
>> > funcs?
> There's a drm_simple_display_pipe floating around which seems perfectly
> suited to tilcdc. It's meant for the case where you have 1 plane, 1 crtc
> and 1 encoder maybe linking to different connectors. And it takes care of
> all the small bits for you, with a grand total of 5 callbacks, all of them
> optional.
> 
> Might indeed be useful to rebase tilcdc on top of that, should be possible
> to nuke piles of code.


Looks interesting. Does it look like it is getting ready to be merged soon?

Jyri


[PATCH RFC 00/11] drm/tilcdc: Atomic modeset support

2016-05-10 Thread Jyri Sarha
On 05/09/16 17:10, Tomi Valkeinen wrote:
> Hi Jyri,
> 
> On 11/04/16 19:46, Jyri Sarha wrote:
>> The LCDC in its simplicity does not fit too well into DRM atomic
>> modeset abstractions. I wonder if I am doing the right thing in
>> implementing the dummy primary plane and in implementing
>> mode_set_nofb() crtc helper when the crtc actually needs the
>> framebuffer to be there when configuring it. See individual patch
>> descriptions for details. There is still lot of room for cleaning up
>> but I would first like to know if I am moving at all to the right
>> direction.
> 
> I'm no expert on atomic modesetting, but here are my comments/questions:
> 
> You add drm_crtc_vblank_off() to crtc_destroy() callback. Why is that? I
> think you should call drm_crtc_vblank_on() in crtc_enable(), and
> vblank_off in disable.
> 

I did not really think of that. I'll make the change.

> You remove the setting of tilcdc_crtc_helper_funcs.dpms, but leave the
> tilcdc_crtc_dpms, which you use elsewhere. I think all dpms stuff should
> be removed from crtc, as it's all either "enable" or "disable". Git grep
> shows that in omapdrm, there's just one reference to dpms, in
> omap_connector.c: .dpms = drm_atomic_helper_connector_dpms
> 

I left that for subsequent clean up after general approach is accepted.

> It's not clear to me if a (primary) plane is required with atomic
> universal planes and modesetting or not... I guess it is, when thinking
> of how e.g. a framebuffer is configured to be shown on a screen when
> using the atomic modesetting uapi.
> 

As modeset_nofb is the preferred call back for setting the mode, and it
does not require any fb or plane, I needed something to express the
mandatory framebuffer. The primatry plane just wast the first solution
that came to my mind and it appeared to work.

> But if it is required, it makes me wonder, are there other HWs out there
> without any planes? The dummy plane implementation you added is not
> complex, but is it something that should be implemented with DRM helper
> funcs?
> 
>  Tomi
> 


-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: 



[PATCH RFC 00/11] drm/tilcdc: Atomic modeset support

2016-05-09 Thread Tomi Valkeinen
Hi Jyri,

On 11/04/16 19:46, Jyri Sarha wrote:
> The LCDC in its simplicity does not fit too well into DRM atomic
> modeset abstractions. I wonder if I am doing the right thing in
> implementing the dummy primary plane and in implementing
> mode_set_nofb() crtc helper when the crtc actually needs the
> framebuffer to be there when configuring it. See individual patch
> descriptions for details. There is still lot of room for cleaning up
> but I would first like to know if I am moving at all to the right
> direction.

I'm no expert on atomic modesetting, but here are my comments/questions:

You add drm_crtc_vblank_off() to crtc_destroy() callback. Why is that? I
think you should call drm_crtc_vblank_on() in crtc_enable(), and
vblank_off in disable.

You remove the setting of tilcdc_crtc_helper_funcs.dpms, but leave the
tilcdc_crtc_dpms, which you use elsewhere. I think all dpms stuff should
be removed from crtc, as it's all either "enable" or "disable". Git grep
shows that in omapdrm, there's just one reference to dpms, in
omap_connector.c: .dpms = drm_atomic_helper_connector_dpms

It's not clear to me if a (primary) plane is required with atomic
universal planes and modesetting or not... I guess it is, when thinking
of how e.g. a framebuffer is configured to be shown on a screen when
using the atomic modesetting uapi.

But if it is required, it makes me wonder, are there other HWs out there
without any planes? The dummy plane implementation you added is not
complex, but is it something that should be implemented with DRM helper
funcs?

 Tomi

-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: 



[PATCH RFC 00/11] drm/tilcdc: Atomic modeset support

2016-05-09 Thread Daniel Vetter
On Mon, May 09, 2016 at 05:10:00PM +0300, Tomi Valkeinen wrote:
> Hi Jyri,
> 
> On 11/04/16 19:46, Jyri Sarha wrote:
> > The LCDC in its simplicity does not fit too well into DRM atomic
> > modeset abstractions. I wonder if I am doing the right thing in
> > implementing the dummy primary plane and in implementing
> > mode_set_nofb() crtc helper when the crtc actually needs the
> > framebuffer to be there when configuring it. See individual patch
> > descriptions for details. There is still lot of room for cleaning up
> > but I would first like to know if I am moving at all to the right
> > direction.
> 
> I'm no expert on atomic modesetting, but here are my comments/questions:
> 
> You add drm_crtc_vblank_off() to crtc_destroy() callback. Why is that? I
> think you should call drm_crtc_vblank_on() in crtc_enable(), and
> vblank_off in disable.
> 
> You remove the setting of tilcdc_crtc_helper_funcs.dpms, but leave the
> tilcdc_crtc_dpms, which you use elsewhere. I think all dpms stuff should
> be removed from crtc, as it's all either "enable" or "disable". Git grep
> shows that in omapdrm, there's just one reference to dpms, in
> omap_connector.c: .dpms = drm_atomic_helper_connector_dpms
> 
> It's not clear to me if a (primary) plane is required with atomic
> universal planes and modesetting or not... I guess it is, when thinking
> of how e.g. a framebuffer is configured to be shown on a screen when
> using the atomic modesetting uapi.

You need a primary plane, but atomic doesn't require that it's enabled.
Which this simple display controller probably wont like, so seems like
this implementation of a primary plane is a bit too simple. You also need
a real plane for the cursor, if you want to support that with atomic.

> But if it is required, it makes me wonder, are there other HWs out there
> without any planes? The dummy plane implementation you added is not
> complex, but is it something that should be implemented with DRM helper
> funcs?

There's a drm_simple_display_pipe floating around which seems perfectly
suited to tilcdc. It's meant for the case where you have 1 plane, 1 crtc
and 1 encoder maybe linking to different connectors. And it takes care of
all the small bits for you, with a grand total of 5 callbacks, all of them
optional.

Might indeed be useful to rebase tilcdc on top of that, should be possible
to nuke piles of code.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


[PATCH RFC 00/11] drm/tilcdc: Atomic modeset support

2016-04-11 Thread Jyri Sarha
The LCDC in its simplicity does not fit too well into DRM atomic
modeset abstractions. I wonder if I am doing the right thing in
implementing the dummy primary plane and in implementing
mode_set_nofb() crtc helper when the crtc actually needs the
framebuffer to be there when configuring it. See individual patch
descriptions for details. There is still lot of room for cleaning up
but I would first like to know if I am moving at all to the right
direction.

Jyri Sarha (11):
  drm/tilcdc: Make tilcdc_crtc_page_flip() public
  drm/tilcdc: Add dummy primary plane implementation
  drm/tilcdc: Initialize dummy primary plane from crtc init
  drm/tilcdc: Add tilcdc_crtc_mode_set_nofb()
  drm/tilcdc: Add tilcdc_crtc_atomic_check()
  drm/tilcdc: Add atomic mode config funcs
  drm/tilcdc: Add drm_mode_config_reset() call to tilcdc_load()
  drm/tilcdc: Call drm_crtc_vblank_off() in tilcdc_crtc_destroy()
  drm/tilcdc: Set DRIVER_ATOMIC and use atomic crtc helpers
  drm/tilcdc: Remove obsolete crtc helper functions
  drm/tilcdc: Remove tilcdc_verify_fb()

 drivers/gpu/drm/tilcdc/Makefile   |   1 +
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c  | 142 +++---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c   |  52 -
 drivers/gpu/drm/tilcdc/tilcdc_drv.h   |   6 ++
 drivers/gpu/drm/tilcdc/tilcdc_plane.c | 122 +
 5 files changed, 244 insertions(+), 79 deletions(-)
 create mode 100644 drivers/gpu/drm/tilcdc/tilcdc_plane.c

-- 
1.9.1