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

[Fabien DESSENNE]

What is the next step to get this patch merged ?
I am asking because I am a new contributor and I am not really aware of the acceptance / merge process.
Fabien

> -----Original Message-----
> From: Fabien DESSENNE
> Sent: mardi 17 décembre 2013 10:01
> To: 'Bryce W. Harrington'
> 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
> 
> > 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