[PATCH weston v11 12/13] compositor-drm: Track unused connectors and CRTCs

[Pekka Paalanen]

On Tue, 18 Jul 2017 14:14:34 +0100
Daniel Stone <daniels at collabora.com> wrote:

> Rather than a more piecemeal approach at backend creation, explicitly
> track connectors and CRTCs we do not intend to use, so we can ensure
> they are disabled where appropriate.
> 
> Signed-off-by: Daniel Stone <daniels at collabora.com>
> ---
>  libweston/compositor-drm.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index ca4146a5..101f9d69 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -185,6 +185,9 @@ struct drm_backend {
>  
>  	void *repaint_data;
>  
> +	struct wl_array unused_connectors;
> +	struct wl_array unused_crtcs;
> +
>  	int cursors_are_broken;
>  
>  	bool universal_planes;
> @@ -3966,6 +3969,47 @@ drm_output_disable(struct weston_output *base)
>  }
>  
>  /**
> + * Update the list of unused connectors and CRTCs
> + *
> + * This keeps the unused_connectors and unused_crtcs arrays up to date.
> + *
> + * @param b Weston backend structure
> + * @param resources DRM resources for this device
> + */
> +static void
> +drm_backend_update_unused_outputs(struct drm_backend *b, drmModeRes *resources)
> +{
> +	int i;
> +
> +	wl_array_release(&b->unused_connectors);
> +	wl_array_init(&b->unused_connectors);
> +
> +	for (i = 0; i < resources->count_connectors; i++) {
> +		uint32_t *connector_id;
> +
> +		if (drm_output_find_by_connector(b, resources->connectors[i]))
> +			continue;
> +
> +		connector_id = wl_array_add(&b->unused_connectors,
> +					    sizeof(*connector_id));
> +		*connector_id = resources->connectors[i];
> +	}
> +
> +	wl_array_release(&b->unused_crtcs);
> +	wl_array_init(&b->unused_crtcs);
> +
> +	for (i = 0; i < resources->count_crtcs; i++) {
> +		uint32_t *crtc_id;
> +
> +		if (drm_output_find_by_crtc(b, resources->crtcs[i]))
> +			continue;
> +
> +		crtc_id = wl_array_add(&b->unused_crtcs, sizeof(*crtc_id));
> +		*crtc_id = resources->crtcs[i];
> +	}
> +}

Hi,

a slight issue with drm_output_find_by_{connector,crtc}() is that they
include all, both enabled and disabled. Looking at patch 13, I think
you'd want to find only those that are actually enabled.

To be more precise, there are three kinds of connectors (or crtcs by
assignment):
- unconnected; do not have a weston_output
- connected, not enabled or disabled; has a pending weston_output
- connected, enabled; has a weston_output, not pending

ISTR the DRM-backend allocates a CRTC & connector when the connector is
connected, even if it is not enabled. Which is to be fixed one day.
When that is fixed, then drm_output_find_by_{crtc,connector}() will do
the right thing automatically.

So, in that respect, I can give:

Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>

since this code is correct in itself, it just suffers from other bits
being not as good as they should.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20170721/89efaac8/attachment.sig>