Send plymouth mailing list submissions to
        [email protected]

To subscribe or unsubscribe via the World Wide Web, visit
        http://lists.freedesktop.org/mailman/listinfo/plymouth
or, via email, send a message with subject or body 'help' to
        [email protected]

You can reach the person managing the list at
        [email protected]

When replying, please edit your Subject line so it is more specific
than "Re: Contents of plymouth digest..."


Today's Topics:

   1. [PATCH] drm: Don't override active mode. (Forest Bond)
   2. Re: [PATCH] drm: Don't override active mode. (Forest Bond)
   3. [PATCH] drm: Don't override active mode. (Forest Bond)
   4. Re: [PATCH] drm: Don't override active mode. (Forest Bond)
   5. [PATCH] drm: Don't override active mode. (Forest Bond)


----------------------------------------------------------------------

Message: 1
Date: Sun, 13 Jun 2010 12:39:56 -0400
From: Forest Bond <[email protected]>
Subject: [PATCH] drm: Don't override active mode.
To: [email protected]
Message-ID: <[email protected]>
Content-Type: text/plain; charset="us-ascii"

The drm renderer calls drmModeSetCrtc to tell the CRTC to use the
allocated scan out buffer.  This should not result in setting a
new mode for the CRTC, but can do exactly that since plymouth saves
the wrong mode when creating heads, and inadvertently overrides the
initial mode set by the kernel.

The root of the problem is the assumption that the first mode in the
connector's mode list is the current active mode.  This will be true
if the active mode is set by automatic detection, as the kernel will
have selected the first mode in the list as the initial mode, but it
will not be true if the user has selected a different mode via the
video= kernel boot parameter.

To fix this, the saved mode is copied from the controller itself,
unless that mode is for some reason not valid, in which case we fall
back to the old behavior (which could result in a mode switch).

Signed-off-by: Forest Bond <[email protected]>
---
 src/plugins/renderers/drm/plugin.c |   35 ++++++++++++++++++++++++++++++++---
 1 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/src/plugins/renderers/drm/plugin.c 
b/src/plugins/renderers/drm/plugin.c
index 385bd91..7976e21 100644
--- a/src/plugins/renderers/drm/plugin.c
+++ b/src/plugins/renderers/drm/plugin.c
@@ -159,6 +159,7 @@ ply_renderer_head_free (ply_renderer_head_t *head)
   ply_trace ("freeing %ldx%ld renderer head", head->area.width, 
head->area.height);
   ply_pixel_buffer_free (head->pixel_buffer);
   drmModeFreeConnector (head->connector);
+  drmModeFreeModeInfo (head->mode);
   free (head);
 }
 
@@ -547,12 +548,21 @@ close_device (ply_renderer_backend_t *backend)
 }
 
 static drmModeModeInfo *
-get_active_mode_for_connector (ply_renderer_backend_t *backend,
-                               drmModeConnector       *connector)
+get_preferred_mode_for_connector (ply_renderer_backend_t *backend,
+                                  drmModeConnector       *connector)
 {
   return &connector->modes[0];
 }
 
+static drmModeModeInfo *
+get_active_mode_for_controller (ply_renderer_backend_t  *backend,
+                                drmModeCrtc             *controller)
+{
+  if (controller->mode_valid)
+    return &controller->mode;
+  return NULL;
+}
+
 static bool
 controller_is_available (ply_renderer_backend_t *backend,
                          uint32_t                controller_id)
@@ -725,6 +735,8 @@ create_heads_for_active_connectors (ply_renderer_backend_t 
*backend)
       uint32_t encoder_id;
       uint32_t console_buffer_id;
       drmModeModeInfo *mode;
+      drmModeModeInfo *src_mode;
+      drmModeCrtc *controller;
 
       connector = drmModeGetConnector (backend->device_fd,
                                        backend->resources->connectors[i]);
@@ -762,7 +774,24 @@ create_heads_for_active_connectors (ply_renderer_backend_t 
*backend)
           continue;
         }
 
-      mode = get_active_mode_for_connector (backend, connector);
+      mode = malloc (sizeof (drmModeModeInfo));
+      controller = drmModeGetCrtc (backend->device_fd, controller_id);
+      /* Try to preserve the active mode for the controller. */
+      src_mode = get_active_mode_for_controller (backend, controller);
+      if (src_mode)
+        {
+          /* Active mode is set and valid, use it. */
+          memcpy (mode, src_mode, sizeof (drmModeModeInfo));
+        }
+      else
+        {
+          /* Fall back to setting the preferred mode for the connector.
+           * This will result in changing the active mode if one is set.
+           */
+          src_mode = get_preferred_mode_for_connector (backend, connector);
+          memcpy (mode, src_mode, sizeof (drmModeModeInfo));
+        }
+      drmModeFreeCrtc (controller);
 
       console_buffer_id = get_console_buffer_id (backend, controller_id);
 
-- 
1.7.0.4
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: 
<http://lists.freedesktop.org/archives/plymouth/attachments/20100613/e09139be/attachment-0001.pgp>

------------------------------

Message: 2
Date: Sun, 13 Jun 2010 12:58:38 -0400
From: Forest Bond <[email protected]>
Subject: Re: [PATCH] drm: Don't override active mode.
To: [email protected]
Message-ID: <[email protected]>
Content-Type: text/plain; charset="us-ascii"

Hi,

Please disregard this patch, I'll resend with tweaks.

Thanks,
Forest

On Sun, Jun 13, 2010 at 12:39:56PM -0400, Forest Bond wrote:
> The drm renderer calls drmModeSetCrtc to tell the CRTC to use the
> allocated scan out buffer.  This should not result in setting a
> new mode for the CRTC, but can do exactly that since plymouth saves
> the wrong mode when creating heads, and inadvertently overrides the
> initial mode set by the kernel.
> 
> The root of the problem is the assumption that the first mode in the
> connector's mode list is the current active mode.  This will be true
> if the active mode is set by automatic detection, as the kernel will
> have selected the first mode in the list as the initial mode, but it
> will not be true if the user has selected a different mode via the
> video= kernel boot parameter.
> 
> To fix this, the saved mode is copied from the controller itself,
> unless that mode is for some reason not valid, in which case we fall
> back to the old behavior (which could result in a mode switch).
> 
> Signed-off-by: Forest Bond <[email protected]>
> ---
>  src/plugins/renderers/drm/plugin.c |   35 ++++++++++++++++++++++++++++++++---
>  1 files changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/src/plugins/renderers/drm/plugin.c 
> b/src/plugins/renderers/drm/plugin.c
> index 385bd91..7976e21 100644
> --- a/src/plugins/renderers/drm/plugin.c
> +++ b/src/plugins/renderers/drm/plugin.c
> @@ -159,6 +159,7 @@ ply_renderer_head_free (ply_renderer_head_t *head)
>    ply_trace ("freeing %ldx%ld renderer head", head->area.width, 
> head->area.height);
>    ply_pixel_buffer_free (head->pixel_buffer);
>    drmModeFreeConnector (head->connector);
> +  drmModeFreeModeInfo (head->mode);
>    free (head);
>  }
>  
> @@ -547,12 +548,21 @@ close_device (ply_renderer_backend_t *backend)
>  }
>  
>  static drmModeModeInfo *
> -get_active_mode_for_connector (ply_renderer_backend_t *backend,
> -                               drmModeConnector       *connector)
> +get_preferred_mode_for_connector (ply_renderer_backend_t *backend,
> +                                  drmModeConnector       *connector)
>  {
>    return &connector->modes[0];
>  }
>  
> +static drmModeModeInfo *
> +get_active_mode_for_controller (ply_renderer_backend_t  *backend,
> +                                drmModeCrtc             *controller)
> +{
> +  if (controller->mode_valid)
> +    return &controller->mode;
> +  return NULL;
> +}
> +
>  static bool
>  controller_is_available (ply_renderer_backend_t *backend,
>                           uint32_t                controller_id)
> @@ -725,6 +735,8 @@ create_heads_for_active_connectors 
> (ply_renderer_backend_t *backend)
>        uint32_t encoder_id;
>        uint32_t console_buffer_id;
>        drmModeModeInfo *mode;
> +      drmModeModeInfo *src_mode;
> +      drmModeCrtc *controller;
>  
>        connector = drmModeGetConnector (backend->device_fd,
>                                         backend->resources->connectors[i]);
> @@ -762,7 +774,24 @@ create_heads_for_active_connectors 
> (ply_renderer_backend_t *backend)
>            continue;
>          }
>  
> -      mode = get_active_mode_for_connector (backend, connector);
> +      mode = malloc (sizeof (drmModeModeInfo));
> +      controller = drmModeGetCrtc (backend->device_fd, controller_id);
> +      /* Try to preserve the active mode for the controller. */
> +      src_mode = get_active_mode_for_controller (backend, controller);
> +      if (src_mode)
> +        {
> +          /* Active mode is set and valid, use it. */
> +          memcpy (mode, src_mode, sizeof (drmModeModeInfo));
> +        }
> +      else
> +        {
> +          /* Fall back to setting the preferred mode for the connector.
> +           * This will result in changing the active mode if one is set.
> +           */
> +          src_mode = get_preferred_mode_for_connector (backend, connector);
> +          memcpy (mode, src_mode, sizeof (drmModeModeInfo));
> +        }
> +      drmModeFreeCrtc (controller);
>  
>        console_buffer_id = get_console_buffer_id (backend, controller_id);
>  
> -- 
> 1.7.0.4
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: 
<http://lists.freedesktop.org/archives/plymouth/attachments/20100613/fe2ba9b7/attachment-0001.pgp>

------------------------------

Message: 3
Date: Sun, 13 Jun 2010 12:59:46 -0400
From: Forest Bond <[email protected]>
Subject: [PATCH] drm: Don't override active mode.
To: [email protected]
Message-ID: <[email protected]>
Content-Type: text/plain; charset="us-ascii"

The drm renderer calls drmModeSetCrtc to tell the CRTC to use the
allocated scan out buffer.  This should not result in setting a
new mode for the CRTC, but can do exactly that since plymouth saves
the wrong mode when creating heads, and inadvertently overrides the
initial mode set by the kernel.

The root of the problem is the assumption that the first mode in the
connector's mode list is the current active mode.  This will be true
if the active mode is set by automatic detection, as the kernel will
have selected the first mode in the list as the initial mode, but it
will not be true if the user has selected a different mode via the
video= kernel boot parameter.

To fix this, the saved mode is copied from the controller itself,
unless that mode is for some reason not valid, in which case we fall
back to the old behavior (which could result in a mode switch).

Signed-off-by: Forest Bond <[email protected]>
---
 src/plugins/renderers/drm/plugin.c |   31 ++++++++++++++++++++++++++++---
 1 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/src/plugins/renderers/drm/plugin.c 
b/src/plugins/renderers/drm/plugin.c
index 385bd91..11b68b2 100644
--- a/src/plugins/renderers/drm/plugin.c
+++ b/src/plugins/renderers/drm/plugin.c
@@ -159,6 +159,7 @@ ply_renderer_head_free (ply_renderer_head_t *head)
   ply_trace ("freeing %ldx%ld renderer head", head->area.width, 
head->area.height);
   ply_pixel_buffer_free (head->pixel_buffer);
   drmModeFreeConnector (head->connector);
+  drmModeFreeModeInfo (head->mode);
   free (head);
 }
 
@@ -547,12 +548,21 @@ close_device (ply_renderer_backend_t *backend)
 }
 
 static drmModeModeInfo *
-get_active_mode_for_connector (ply_renderer_backend_t *backend,
-                               drmModeConnector       *connector)
+get_preferred_mode_for_connector (ply_renderer_backend_t *backend,
+                                  drmModeConnector       *connector)
 {
   return &connector->modes[0];
 }
 
+static drmModeModeInfo *
+get_active_mode_for_controller (ply_renderer_backend_t  *backend,
+                                drmModeCrtc             *controller)
+{
+  if (controller->mode_valid)
+    return &controller->mode;
+  return NULL;
+}
+
 static bool
 controller_is_available (ply_renderer_backend_t *backend,
                          uint32_t                controller_id)
@@ -725,6 +735,8 @@ create_heads_for_active_connectors (ply_renderer_backend_t 
*backend)
       uint32_t encoder_id;
       uint32_t console_buffer_id;
       drmModeModeInfo *mode;
+      drmModeModeInfo *src_mode;
+      drmModeCrtc *controller;
 
       connector = drmModeGetConnector (backend->device_fd,
                                        backend->resources->connectors[i]);
@@ -762,7 +774,20 @@ create_heads_for_active_connectors (ply_renderer_backend_t 
*backend)
           continue;
         }
 
-      mode = get_active_mode_for_connector (backend, connector);
+      controller = drmModeGetCrtc (backend->device_fd, controller_id);
+
+      /* Try to preserve the active mode for the controller. */
+      src_mode = get_active_mode_for_controller (backend, controller);
+
+      /* Fall back to setting the preferred mode for the connector.
+       * This will result in changing the active mode if one is set.
+       */
+      if (!src_mode)
+        src_mode = get_preferred_mode_for_connector (backend, connector);
+
+      mode = malloc (sizeof (drmModeModeInfo));
+      memcpy (mode, src_mode, sizeof (drmModeModeInfo));
+      drmModeFreeCrtc (controller);
 
       console_buffer_id = get_console_buffer_id (backend, controller_id);
 
-- 
1.7.0.4
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: 
<http://lists.freedesktop.org/archives/plymouth/attachments/20100613/3cb79d4c/attachment-0001.pgp>

------------------------------

Message: 4
Date: Sun, 13 Jun 2010 13:21:52 -0400
From: Forest Bond <[email protected]>
Subject: Re: [PATCH] drm: Don't override active mode.
To: [email protected]
Message-ID: <[email protected]>
Content-Type: text/plain; charset="us-ascii"

Hi,

Agh, I guess I should be checking for NULL return values.  I'll resend again.
Sorry for the noise.

Thanks,
Forest

On Sun, Jun 13, 2010 at 12:59:46PM -0400, Forest Bond wrote:
> The drm renderer calls drmModeSetCrtc to tell the CRTC to use the
> allocated scan out buffer.  This should not result in setting a
> new mode for the CRTC, but can do exactly that since plymouth saves
> the wrong mode when creating heads, and inadvertently overrides the
> initial mode set by the kernel.
> 
> The root of the problem is the assumption that the first mode in the
> connector's mode list is the current active mode.  This will be true
> if the active mode is set by automatic detection, as the kernel will
> have selected the first mode in the list as the initial mode, but it
> will not be true if the user has selected a different mode via the
> video= kernel boot parameter.
> 
> To fix this, the saved mode is copied from the controller itself,
> unless that mode is for some reason not valid, in which case we fall
> back to the old behavior (which could result in a mode switch).
> 
> Signed-off-by: Forest Bond <[email protected]>
> ---
>  src/plugins/renderers/drm/plugin.c |   31 ++++++++++++++++++++++++++++---
>  1 files changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/src/plugins/renderers/drm/plugin.c 
> b/src/plugins/renderers/drm/plugin.c
> index 385bd91..11b68b2 100644
> --- a/src/plugins/renderers/drm/plugin.c
> +++ b/src/plugins/renderers/drm/plugin.c
> @@ -159,6 +159,7 @@ ply_renderer_head_free (ply_renderer_head_t *head)
>    ply_trace ("freeing %ldx%ld renderer head", head->area.width, 
> head->area.height);
>    ply_pixel_buffer_free (head->pixel_buffer);
>    drmModeFreeConnector (head->connector);
> +  drmModeFreeModeInfo (head->mode);
>    free (head);
>  }
>  
> @@ -547,12 +548,21 @@ close_device (ply_renderer_backend_t *backend)
>  }
>  
>  static drmModeModeInfo *
> -get_active_mode_for_connector (ply_renderer_backend_t *backend,
> -                               drmModeConnector       *connector)
> +get_preferred_mode_for_connector (ply_renderer_backend_t *backend,
> +                                  drmModeConnector       *connector)
>  {
>    return &connector->modes[0];
>  }
>  
> +static drmModeModeInfo *
> +get_active_mode_for_controller (ply_renderer_backend_t  *backend,
> +                                drmModeCrtc             *controller)
> +{
> +  if (controller->mode_valid)
> +    return &controller->mode;
> +  return NULL;
> +}
> +
>  static bool
>  controller_is_available (ply_renderer_backend_t *backend,
>                           uint32_t                controller_id)
> @@ -725,6 +735,8 @@ create_heads_for_active_connectors 
> (ply_renderer_backend_t *backend)
>        uint32_t encoder_id;
>        uint32_t console_buffer_id;
>        drmModeModeInfo *mode;
> +      drmModeModeInfo *src_mode;
> +      drmModeCrtc *controller;
>  
>        connector = drmModeGetConnector (backend->device_fd,
>                                         backend->resources->connectors[i]);
> @@ -762,7 +774,20 @@ create_heads_for_active_connectors 
> (ply_renderer_backend_t *backend)
>            continue;
>          }
>  
> -      mode = get_active_mode_for_connector (backend, connector);
> +      controller = drmModeGetCrtc (backend->device_fd, controller_id);
> +
> +      /* Try to preserve the active mode for the controller. */
> +      src_mode = get_active_mode_for_controller (backend, controller);
> +
> +      /* Fall back to setting the preferred mode for the connector.
> +       * This will result in changing the active mode if one is set.
> +       */
> +      if (!src_mode)
> +        src_mode = get_preferred_mode_for_connector (backend, connector);
> +
> +      mode = malloc (sizeof (drmModeModeInfo));
> +      memcpy (mode, src_mode, sizeof (drmModeModeInfo));
> +      drmModeFreeCrtc (controller);
>  
>        console_buffer_id = get_console_buffer_id (backend, controller_id);
>  
> -- 
> 1.7.0.4
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: 
<http://lists.freedesktop.org/archives/plymouth/attachments/20100613/0c2e8eba/attachment-0001.pgp>

------------------------------

Message: 5
Date: Sun, 13 Jun 2010 13:27:13 -0400
From: Forest Bond <[email protected]>
Subject: [PATCH] drm: Don't override active mode.
To: [email protected]
Message-ID: <[email protected]>
Content-Type: text/plain; charset="us-ascii"

The drm renderer calls drmModeSetCrtc to tell the CRTC to use the
allocated scan out buffer.  This should not result in setting a
new mode for the CRTC, but can do exactly that since plymouth saves
the wrong mode when creating heads, and inadvertently overrides the
initial mode set by the kernel.

The root of the problem is the assumption that the first mode in the
connector's mode list is the current active mode.  This will be true
if the active mode is set by automatic detection, as the kernel will
have selected the first mode in the list as the initial mode, but it
will not be true if the user has selected a different mode via the
video= kernel boot parameter.

To fix this, the saved mode is copied from the controller itself,
unless that mode is for some reason not valid, in which case we fall
back to the old behavior (which could result in a mode switch).

Signed-off-by: Forest Bond <[email protected]>
---
 src/plugins/renderers/drm/plugin.c |   43 +++++++++++++++++++++++++++++++++--
 1 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/src/plugins/renderers/drm/plugin.c 
b/src/plugins/renderers/drm/plugin.c
index 385bd91..993cf77 100644
--- a/src/plugins/renderers/drm/plugin.c
+++ b/src/plugins/renderers/drm/plugin.c
@@ -159,6 +159,7 @@ ply_renderer_head_free (ply_renderer_head_t *head)
   ply_trace ("freeing %ldx%ld renderer head", head->area.width, 
head->area.height);
   ply_pixel_buffer_free (head->pixel_buffer);
   drmModeFreeConnector (head->connector);
+  drmModeFreeModeInfo (head->mode);
   free (head);
 }
 
@@ -547,12 +548,21 @@ close_device (ply_renderer_backend_t *backend)
 }
 
 static drmModeModeInfo *
-get_active_mode_for_connector (ply_renderer_backend_t *backend,
-                               drmModeConnector       *connector)
+get_preferred_mode_for_connector (ply_renderer_backend_t *backend,
+                                  drmModeConnector       *connector)
 {
   return &connector->modes[0];
 }
 
+static drmModeModeInfo *
+get_active_mode_for_controller (ply_renderer_backend_t  *backend,
+                                drmModeCrtc             *controller)
+{
+  if (controller->mode_valid)
+    return &controller->mode;
+  return NULL;
+}
+
 static bool
 controller_is_available (ply_renderer_backend_t *backend,
                          uint32_t                controller_id)
@@ -725,6 +735,8 @@ create_heads_for_active_connectors (ply_renderer_backend_t 
*backend)
       uint32_t encoder_id;
       uint32_t console_buffer_id;
       drmModeModeInfo *mode;
+      drmModeModeInfo *src_mode = NULL;
+      drmModeCrtc *controller;
 
       connector = drmModeGetConnector (backend->device_fd,
                                        backend->resources->connectors[i]);
@@ -762,7 +774,32 @@ create_heads_for_active_connectors (ply_renderer_backend_t 
*backend)
           continue;
         }
 
-      mode = get_active_mode_for_connector (backend, connector);
+      controller = drmModeGetCrtc (backend->device_fd, controller_id);
+
+      /* Try to preserve the active mode for the controller. */
+      if (controller != NULL)
+        src_mode = get_active_mode_for_controller (backend, controller);
+
+      /* Fall back to setting the preferred mode for the connector.
+       * This may result in changing the active mode if one is set.
+       */
+      if (src_mode == NULL)
+        src_mode = get_preferred_mode_for_connector (backend, connector);
+
+      mode = malloc (sizeof (drmModeModeInfo));
+
+      if (mode == NULL)
+        {
+          if (controller != NULL)
+            drmModeFreeCrtc (controller);
+          drmModeFreeConnector (connector);
+          continue;
+        }
+
+      memcpy (mode, src_mode, sizeof (drmModeModeInfo));
+
+      if (controller != NULL)
+        drmModeFreeCrtc (controller);
 
       console_buffer_id = get_console_buffer_id (backend, controller_id);
 
-- 
1.7.0.4
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: 
<http://lists.freedesktop.org/archives/plymouth/attachments/20100613/ecb6f4c1/attachment.pgp>

------------------------------

_______________________________________________
plymouth mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/plymouth


End of plymouth Digest, Vol 20, Issue 4
***************************************

Reply via email to