From mboxrd@z Thu Jan 1 00:00:00 1970 From: Todd Previte Subject: Re: [PATCH 05/11] drm/fb_helper: allow adding/removing connectors later Date: Tue, 17 Jun 2014 08:23:04 -0700 Message-ID: <53A05D58.4090207@gmail.com> References: <1400640904-16847-1-git-send-email-airlied@gmail.com> <1400640904-16847-6-git-send-email-airlied@gmail.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0750702132==" Return-path: In-Reply-To: <1400640904-16847-6-git-send-email-airlied@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Dave Airlie Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org This is a multi-part message in MIME format. --===============0750702132== Content-Type: multipart/alternative; boundary="------------080002000302000701060000" This is a multi-part message in MIME format. --------------080002000302000701060000 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Minor formatting issues - there's a number of lines that exceed 80 characters in length. One other comment inline below. Reviewed-by: Todd Previte > Dave Airlie > Tuesday, May 20, 2014 7:54 PM > From: Dave Airlie > > 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 > --- > 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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Sent using Postbox: http://www.getpostbox.com --------------080002000302000701060000 Content-Type: multipart/related; boundary="------------070701090603050601090200" --------------070701090603050601090200 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit
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>
 
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
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
--------------070701090603050601090200 Content-Type: image/jpeg; x-apple-mail-type=stationery; name="postbox-contact.jpg" Content-Transfer-Encoding: base64 Content-ID: Content-Disposition: inline; filename="postbox-contact.jpg" /9j/4AAQSkZJRgABAQAAAQABAAD/2wBDAAMCAgICAgMCAgIDAwMDBAYEBAQEBAgGBgUGCQgK CgkICQkKDA8MCgsOCwkJDRENDg8QEBEQCgwSExIQEw8QEBD/2wBDAQMDAwQDBAgEBAgQCwkL EBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBD/wAAR CAAZABkDAREAAhEBAxEB/8QAHwAAAQUBAQEBAQEAAAAAAAAAAAECAwQFBgcICQoL/8QAtRAA AgEDAwIEAwUFBAQAAAF9AQIDAAQRBRIhMUEGE1FhByJxFDKBkaEII0KxwRVS0fAkM2JyggkK FhcYGRolJicoKSo0NTY3ODk6Q0RFRkdISUpTVFVWV1hZWmNkZWZnaGlqc3R1dnd4eXqDhIWG h4iJipKTlJWWl5iZmqKjpKWmp6ipqrKztLW2t7i5usLDxMXGx8jJytLT1NXW19jZ2uHi4+Tl 5ufo6erx8vP09fb3+Pn6/8QAHwEAAwEBAQEBAQEBAQAAAAAAAAECAwQFBgcICQoL/8QAtREA AgECBAQDBAcFBAQAAQJ3AAECAxEEBSExBhJBUQdhcRMiMoEIFEKRobHBCSMzUvAVYnLRChYk NOEl8RcYGRomJygpKjU2Nzg5OkNERUZHSElKU1RVVldYWVpjZGVmZ2hpanN0dXZ3eHl6goOE hYaHiImKkpOUlZaXmJmaoqOkpaanqKmqsrO0tba3uLm6wsPExcbHyMnK0tPU1dbX2Nna4uPk 5ebn6Onq8vP09fb3+Pn6/9oADAMBAAIRAxEAPwD4ikvNV8RaufG/jlrTXL/Wre3vtt7YwSvc O8KGSaeVkMhUybwsasvC8FFVA3DjcfDBru30PquGOFcRxHVfK+WnHeX6JdWWHv8AdxHofhaJ O0aeF9NYAem54Gf8SxPvXiPOq97pI/UYeGGVKnyynNvvdflYpx2Nxda7p11o2n6TYXUF1FMU s9Ngi8yNGDOYnCeZDKqguCr87SV2MoDetgcxji/datI/O+KuDK/DyVenLnpPS/VPsyH/AIbY /ao/6LNrX/fuD/43XpnxB9O+BP2LLr4meGzdW/xi0Lw7ruk2WnwT6ZqNqwhkhXT7bZLHOG5B O5SCg5U9eteHicNHF1FPuff5FxbVyHDfUuXRO911v3v9xoXH/BN/4mwafNqn/C6vh+LW2nSC eeS9mWGJ3UMoaQRlRnK85xyMkc4j+yPQ9r/iIf8Ai/AW1/YpuPA0Fh4v1T46+FdaubPVtPEW naJC92JSbyJG3TZVUAViehzx65ohgvqs/a9jz8043qZrhpYLlup6a2089OqPm3/hlW1/6HEf +AP/ANsr0PrnkfB/U/M9R+DHxf8Agf8AFbxKfB37TVveeDdcsrO00vT9WgnNtAJICyyw3QkR xCzFj8zKVUpg4zWiouh8GqMalb2rXMj7Ptf2ZP2VLTwLdaFZ3iXcepXNvdR6nNK0978gbaIr yEi1AIk442c5bdhSuftal7k2Vj5F/aM+KH7PPwCWPwn+z1OPE/jRr+Ce61y7uor+DRreJsm3 iliVYpZpGxv2gqqjbktyNVCVZWqbAp+zfNHc+e/+Ek/al/6FbxR/4Tj/APxqr+r0+w/bz7jv 22f+TqPiJ/2E0/8AREdbmJ5En/ILb/eNSwNn4Tf8lU8G/wDYwad/6Ux1QH77UAf/2Q== --------------070701090603050601090200-- --------------080002000302000701060000-- --===============0750702132== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx --===============0750702132==--