[PATCH] compositor-drm: consider the best mode of the mode_list as an option

[Fabien DESSENNE]

> From: Bryce W. Harrington [mailto:b.harrington at samsung.com]
> Sent: mardi 17 décembre 2013 03:26
> To: Fabien DESSENNE
> Cc: wayland-devel at lists.freedesktop.org; Benjamin Gaignard
> Subject: Re: [PATCH] compositor-drm: consider the best mode of the
> mode_list as an option
> 
> On Thu, Dec 12, 2013 at 05:13:56PM +0100, Fabien DESSENNE wrote:
> > This patch fixes an issue where Weston using the DRM backend, cannot
> > start the display. This happens in the following context:
> > - no video mode is set before weston starts (eg no "/dev/fb" set up)
> > - weston is not configured with any default video mode (nothing from
> >   weston.ini nor command line)
> > - the DRM driver provides with a list of supported modes, but none of
> them
> >   is marked as PREFERRED (which is not a usual case, but it happens)
> 
> Good point, I've seen such hardware myself.
> 
> > In that case, according to the current implementation, the DRM
> > compositor fails to set a video mode.
> > This fix lets the DRM compositor selects a video mode (the best one of
> > the list, which is the first) from the ones provided by the driver.
> 
> Is it always guaranteed to be the best, or is it just returning the list in the
> order stored in EDID?  Either way, picking the first in the list is probably
> sensible, however in the latter case I don't know that we can assume it's the
> 'best'.  Maybe the variable should be called 'first'
> rather than 'best'?  But I'm just bikeshedding...

Well, initially the variable was 'first', but I decided to change it to 'best'...
Using 'first' may be confusing as the mode list is being parsed in the reverse order.
The list provided by the DRM driver as implemented by the generic core part (drm_modes.c) is ordered according to the following rule:
- first the preferred mode(s) as defined by the EDID
- then from the highest to the lowest resolution
So the first one is the best one.

> 
> > Signed-off-by: Fabien Dessenne <fabien.dessenne at st.com>
> 
> Reviewed-by: Bryce Harrington <b.harrington at samsung.com>
> 
> > ---
> >  src/compositor-drm.c |    6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/compositor-drm.c b/src/compositor-drm.c index
> > fbf6e49..54caaa9 100644
> > --- a/src/compositor-drm.c
> > +++ b/src/compositor-drm.c
> > @@ -1858,7 +1858,7 @@ create_output_for_connector(struct
> drm_compositor *ec,
> >  			    int x, int y, struct udev_device *drm_device)  {
> >  	struct drm_output *output;
> > -	struct drm_mode *drm_mode, *next, *preferred, *current,
> *configured;
> > +	struct drm_mode *drm_mode, *next, *preferred, *current,
> *configured,
> > +*best;
> >  	struct weston_mode *m;
> >  	struct weston_config_section *section;
> >  	drmModeEncoder *encoder;
> > @@ -1961,6 +1961,7 @@ create_output_for_connector(struct
> drm_compositor *ec,
> >  	preferred = NULL;
> >  	current = NULL;
> >  	configured = NULL;
> > +	best = NULL;
> >
> >  	wl_list_for_each_reverse(drm_mode, &output->base.mode_list,
> base.link) {
> >  		if (config == OUTPUT_CONFIG_MODE && @@ -1971,6
> +1972,7 @@
> > create_output_for_connector(struct drm_compositor *ec,
> >  			current = drm_mode;
> >  		if (drm_mode->base.flags &
> WL_OUTPUT_MODE_PREFERRED)
> >  			preferred = drm_mode;
> > +		best = drm_mode;
> >  	}
> >
> >  	if (config == OUTPUT_CONFIG_MODELINE) { @@ -1996,6 +1998,8
> @@
> > create_output_for_connector(struct drm_compositor *ec,
> >  		output->base.current_mode = &preferred->base;
> >  	else if (current)
> >  		output->base.current_mode = &current->base;
> > +	else if (best)
> > +		output->base.current_mode = &best->base;
> >
> >  	if (output->base.current_mode == NULL) {
> >  		weston_log("no available modes for %s\n", output-
> >base.name);
> > --
> > 1.7.9.5
> >
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel