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

2014-01-08 Thread Kristian Høgsberg
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)
 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.
 
 Signed-off-by: Fabien Dessenne fabien.desse...@st.com

That makes perfect sense, that's a good fallback to have if everything
else fails.  Sorry for not picking it up earlier, it got lost in the
noise around the alpha release.  Pestering me with emails or in irc is
right way get the patch applied, thanks for reminding me.

Kristian

 ---
  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@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


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

2014-01-07 Thread 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@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.harring...@samsung.com]
  Sent: mardi 17 décembre 2013 03:26
  To: Fabien DESSENNE
  Cc: wayland-devel@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.desse...@st.com
 
  Reviewed-by: Bryce Harrington b.harring...@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@lists.freedesktop.org
   http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


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

2013-12-17 Thread Fabien DESSENNE
 From: Bryce W. Harrington [mailto:b.harring...@samsung.com]
 Sent: mardi 17 décembre 2013 03:26
 To: Fabien DESSENNE
 Cc: wayland-devel@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.desse...@st.com
 
 Reviewed-by: Bryce Harrington b.harring...@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@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


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

2013-12-16 Thread Bryce W. Harrington
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...
 
 Signed-off-by: Fabien Dessenne fabien.desse...@st.com

Reviewed-by: Bryce Harrington b.harring...@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@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel