Re: [PATCH 05/11] drm/fb_helper: allow adding/removing connectors later

[Todd Previte <tprevite@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 4987 bytes --]


Minor formatting issues - there's a number of lines that exceed 80 
characters in length. One other comment inline below.

Reviewed-by: Todd Previte <tprevite@gmail.com>

> Dave Airlie <mailto:airlied@gmail.com>
> Tuesday, May 20, 2014 7:54 PM
> From: Dave Airlie <airlied@redhat.com>
>
> This is required to get fbcon probing to work on new connectors,
> callers should acquire the mode config lock before calling these.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
> drivers/gpu/drm/drm_fb_helper.c | 53 
> +++++++++++++++++++++++++++++++++++++++++
> include/drm/drm_fb_helper.h | 4 ++++
> 2 files changed, 57 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c 
> b/drivers/gpu/drm/drm_fb_helper.c
> index 04d3fd3..a184204 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -105,6 +105,58 @@ fail:
> }
> EXPORT_SYMBOL(drm_fb_helper_single_add_all_connectors);
>
> +int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, 
> struct drm_connector *connector)
> +{
> + struct drm_fb_helper_connector **temp;
> + struct drm_fb_helper_connector *fb_helper_connector;
> +
> + WARN_ON(!mutex_is_locked(&fb_helper->dev->mode_config.mutex));
> + if (fb_helper->connector_count + 1 > 
> fb_helper->connector_info_alloc_count) {
> + temp = krealloc(fb_helper->connector_info, sizeof(struct 
> drm_fb_helper_connector) * (fb_helper->connector_count + 1), GFP_KERNEL);
> + if (!temp)
> + return -ENOMEM;
> +
> + fb_helper->connector_info_alloc_count = fb_helper->connector_count + 1;
> + fb_helper->connector_info = temp;
> + }
> +
> +
> + fb_helper_connector = kzalloc(sizeof(struct 
> drm_fb_helper_connector), GFP_KERNEL);
> + if (!fb_helper_connector)
> + return -ENOMEM;
> +
> + fb_helper_connector->connector = connector;
> + fb_helper->connector_info[fb_helper->connector_count++] = 
> fb_helper_connector;
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_fb_helper_add_one_connector);
> +
> +int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
> + struct drm_connector *connector)
> +{
> + struct drm_fb_helper_connector *fb_helper_connector;
> + int i, j;
> +
> + WARN_ON(!mutex_is_locked(&fb_helper->dev->mode_config.mutex));
> +
> + for (i = 0; i < fb_helper->connector_count; i++) {
> + if (fb_helper->connector_info[i]->connector == connector)
> + break;
> + }
> +
> + if (i == fb_helper->connector_count)
> + return -EINVAL;
> + fb_helper_connector = fb_helper->connector_info[i];
> +
> + for (j = i + 1; j < fb_helper->connector_count; j++) {
> + fb_helper->connector_info[j - 1] = fb_helper->connector_info[j];
> + }
Why switch to using a different index variable here? Seems like you 
could just increment i and keep going...
> + fb_helper->connector_count--;
> + kfree(fb_helper_connector);
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_fb_helper_remove_one_connector);
> +
> static int drm_fb_helper_parse_command_line(struct drm_fb_helper 
> *fb_helper)
> {
> struct drm_fb_helper_connector *fb_helper_conn;
> @@ -534,6 +586,7 @@ int drm_fb_helper_init(struct drm_device *dev,
> kfree(fb_helper->crtc_info);
> return -ENOMEM;
> }
> + fb_helper->connector_info_alloc_count = dev->mode_config.num_connector;
> fb_helper->connector_count = 0;
>
> for (i = 0; i < crtc_count; i++) {
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 6e622f7..4abb415 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -86,6 +86,7 @@ struct drm_fb_helper {
> int crtc_count;
> struct drm_fb_helper_crtc *crtc_info;
> int connector_count;
> + int connector_info_alloc_count;
> struct drm_fb_helper_connector **connector_info;
> struct drm_fb_helper_funcs *funcs;
> struct fb_info *fbdev;
> @@ -128,4 +129,7 @@ struct drm_display_mode *
> drm_pick_cmdline_mode(struct drm_fb_helper_connector *fb_helper_conn,
> int width, int height);
>
> +int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, 
> struct drm_connector *connector);
> +int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
> + struct drm_connector *connector);
> #endif
> Dave Airlie <mailto:airlied@gmail.com>
> Tuesday, May 20, 2014 7:54 PM
> Hey,
>
> So this set is pretty close to what I think we should be merging 
> initially,
>
> Since the last set, it makes fbcon and suspend/resume work a lot better,
>
> I've also fixed a couple of bugs in -intel that make things work a lot
> better.
>
> I've bashed on this a bit using kms-flip from intel-gpu-tools, hacked
> to add 3 monitor support.
>
> It still generates a fair few i915 state checker backtraces, and some
> of them are fairly hard to work out, it might be we should just tone
> down the state checker for encoders/connectors with no actual hw backing
> them.
>
> Dave.
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Sent using Postbox:
http://www.getpostbox.com

[-- Attachment #1.2.1: Type: text/html, Size: 8377 bytes --]

[-- Attachment #1.2.2: postbox-contact.jpg --]
[-- Type: image/jpeg, Size: 1291 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx