On 22/06/15 11:25 AM, Daniel Stone wrote:
> From: Pekka Paalanen <pekka.paala...@collabora.co.uk>
> 
> Refactor the code for choosing the initial mode for an output from
> create_output_for_connector() to drm_output_choose_initial_mode().
> 
> This makes create_output_for_connector() slightly easier to read.
> 
> v2: Document everything.
> 
> Signed-off-by: Pekka Paalanen <pekka.paala...@collabora.co.uk>
> Signed-off-by: Daniel Stone <dani...@collabora.com>
> ---
>  src/compositor-drm.c | 168 
> ++++++++++++++++++++++++++++++++++++---------------
>  1 file changed, 119 insertions(+), 49 deletions(-)
> 
> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> index 6d8684d..60f07f1 100644
> --- a/src/compositor-drm.c
> +++ b/src/compositor-drm.c
> @@ -1211,6 +1211,17 @@ drm_output_destroy(struct weston_output *output_base)
>       free(output);
>  }
>  
> +/**
> + * Find the closest-matching mode for a given target
> + *
> + * Given a target mode, find the most suitable mode amongst the output's
> + * current mode list to use, preferring the current mode if possible, to
> + * avoid an expensive mode switch.
> + *
> + * @param output DRM output
> + * @param target_mode Mode to attempt to match
> + * @returns Pointer to a mode from the output's mode list
> + */

I guess the pedant in me would rather see the "unrelated" doc hunks in a
separate patch.

Either way,
Reviewed-By: Derek Foreman <der...@osg.samsung.com

>  static struct drm_mode *
>  choose_mode (struct drm_output *output, struct weston_mode *target_mode)
>  {
> @@ -1463,8 +1474,18 @@ init_pixman(struct drm_compositor *ec)
>       return pixman_renderer_init(&ec->base);
>  }
>  
> +/**
> + * Add a mode to output's mode list
> + *
> + * Copy the supplied DRM mode into a Weston mode structure, and add it to the
> + * output's mode list.
> + *
> + * @param output DRM output to add mode to
> + * @param info DRM mode structure to add
> + * @returns Newly-allocated Weston/DRM mode structure
> + */
>  static struct drm_mode *
> -drm_output_add_mode(struct drm_output *output, drmModeModeInfo *info)
> +drm_output_add_mode(struct drm_output *output, const drmModeModeInfo *info)
>  {
>       struct drm_mode *mode;
>       uint64_t refresh;
> @@ -1977,6 +1998,97 @@ get_gbm_format_from_section(struct 
> weston_config_section *section,
>       return ret;
>  }
>  
> +/**
> + * Choose suitable mode for an output
> + *
> + * Find the most suitable mode to use for initial setup (or reconfiguration 
> on
> + * hotplug etc) for a DRM output.
> + *
> + * @param output DRM output to choose mode for
> + * @param kind Strategy and preference to use when choosing mode
> + * @param width Desired width for this output
> + * @param height Desired height for this output
> + * @param current_mode Mode currently being displayed on this output
> + * @param modeline Manually-entered mode (may be NULL)
> + * @returns A mode from the output's mode list, or NULL if none available
> + */
> +static struct drm_mode *
> +drm_output_choose_initial_mode(struct drm_output *output,
> +                            enum output_config kind,
> +                            int width, int height,
> +                            const drmModeModeInfo *current_mode,
> +                            const drmModeModeInfo *modeline)
> +{
> +     struct drm_mode *preferred = NULL;
> +     struct drm_mode *current = NULL;
> +     struct drm_mode *configured = NULL;
> +     struct drm_mode *best = NULL;
> +     struct drm_mode *drm_mode;
> +
> +     wl_list_for_each_reverse(drm_mode, &output->base.mode_list, base.link) {
> +             if (kind == OUTPUT_CONFIG_MODE &&
> +                 width == drm_mode->base.width &&
> +                 height == drm_mode->base.height)
> +                     configured = drm_mode;
> +
> +             if (memcmp(&current_mode, &drm_mode->mode_info,
> +                        sizeof *current_mode) == 0)
> +                     current = drm_mode;
> +
> +             if (drm_mode->base.flags & WL_OUTPUT_MODE_PREFERRED)
> +                     preferred = drm_mode;
> +
> +             best = drm_mode;
> +     }
> +
> +     if (kind == OUTPUT_CONFIG_MODELINE) {
> +             configured = drm_output_add_mode(output, modeline);
> +             if (!configured)
> +                     return NULL;
> +     }
> +
> +     if (current == NULL && current_mode->clock != 0) {
> +             current = drm_output_add_mode(output, current_mode);
> +             if (!current)
> +                     return NULL;
> +     }
> +
> +     if (kind == OUTPUT_CONFIG_CURRENT)
> +             configured = current;
> +
> +     if (option_current_mode && current)
> +             return current;
> +
> +     if (configured)
> +             return configured;
> +
> +     if (preferred)
> +             return preferred;
> +
> +     if (current)
> +             return current;
> +
> +     if (best)
> +             return best;
> +
> +     weston_log("no available modes for %s\n", output->base.name);
> +     return NULL;
> +}
> +
> +/**
> + * Create and configure a Weston output structure
> + *
> + * Given a DRM connector, create a matching drm_output structure and add it
> + * to Weston's output list.
> + *
> + * @param ec DRM compositor structure
> + * @param resources DRM resources for this device
> + * @param connector DRM connector to use for this new output
> + * @param x Horizontal offset to use into global co-ordinate space
> + * @param y Vertical offset to use into global co-ordinate space
> + * @param drm_device udev device pointer
> + * @returns 0 on success, or -1 on failure
> + */
>  static int
>  create_output_for_connector(struct drm_compositor *ec,
>                           drmModeRes *resources,
> @@ -1984,7 +2096,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, 
> *best;
> +     struct drm_mode *drm_mode, *next, *current;
>       struct weston_mode *m;
>       struct weston_config_section *section;
>       drmModeEncoder *encoder;
> @@ -2092,54 +2204,12 @@ create_output_for_connector(struct drm_compositor *ec,
>               goto err_free;
>       }
>  
> -     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 &&
> -                 width == drm_mode->base.width &&
> -                 height == drm_mode->base.height)
> -                     configured = drm_mode;
> -             if (!memcmp(&crtc_mode, &drm_mode->mode_info, sizeof crtc_mode))
> -                     current = drm_mode;
> -             if (drm_mode->base.flags & WL_OUTPUT_MODE_PREFERRED)
> -                     preferred = drm_mode;
> -             best = drm_mode;
> -     }
> -
> -     if (config == OUTPUT_CONFIG_MODELINE) {
> -             configured = drm_output_add_mode(output, &modeline);
> -             if (!configured)
> -                     goto err_free;
> -     }
> -
> -     if (current == NULL && crtc_mode.clock != 0) {
> -             current = drm_output_add_mode(output, &crtc_mode);
> -             if (!current)
> -                     goto err_free;
> -     }
> -
> -     if (config == OUTPUT_CONFIG_CURRENT)
> -             configured = current;
> -
> -     if (option_current_mode && current)
> -             output->base.current_mode = &current->base;
> -     else if (configured)
> -             output->base.current_mode = &configured->base;
> -     else if (preferred)
> -             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);
> +     current = drm_output_choose_initial_mode(output, config,
> +                                              width, height,
> +                                              &crtc_mode, &modeline);
> +     if (!current)
>               goto err_free;
> -     }
> -
> +     output->base.current_mode = &current->base;
>       output->base.current_mode->flags |= WL_OUTPUT_MODE_CURRENT;
>  
>       weston_output_init(&output->base, &ec->base, x, y,
> 

_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to