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

[Todd Previte <tprevite@xxxxxxxxx>]

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@xxxxxxxxx>
 

Dave Airlie

Tuesday, May 20, 2014 7:54 PM

From: Dave Airlie <airlied@xxxxxxxxxx>

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@xxxxxxxxxx>
---
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

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@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--

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

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