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

From: "Noralf Trønnes" <noralf@tronnes.org>
To: Jyri Sarha <jsarha@ti.com>, Daniel Vetter <daniel@ffwll.ch>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>,
	laurent.pinchart@ideasonboard.com,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH RFC 00/11] drm/tilcdc: Atomic modeset support
Date: Tue, 10 May 2016 16:04:45 +0200	[thread overview]
Message-ID: <120916d3-69d2-dc7a-98ba-9ccd5bff7880@tronnes.org> (raw)
In-Reply-To: <5731A68B.5010405@ti.com>

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

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