diff mbox

[1/5] drm: Have the crtc code only reference master from legacy nodes

Message ID 1394708244-6565-2-git-send-email-thellstrom@vmware.com (mailing list archive)
State New, archived
Headers show

Commit Message

Thomas Hellstrom March 13, 2014, 10:57 a.m. UTC 
control- and render nodes are intended to be master-less.

Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
---
 drivers/gpu/drm/drm_crtc.c |   15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

Comments

David Herrmann March 13, 2014, 11:12 a.m. UTC | #1 
Hi

On Thu, Mar 13, 2014 at 11:57 AM, Thomas Hellstrom
<thellstrom@vmware.com> wrote:
> control- and render nodes are intended to be master-less.
>
> Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
> ---
>  drivers/gpu/drm/drm_crtc.c |   15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 3b7d32d..c9d895a 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1398,7 +1398,7 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
>         uint32_t __user *crtc_id;
>         uint32_t __user *connector_id;
>         uint32_t __user *encoder_id;
> -       struct drm_mode_group *mode_group;
> +       struct drm_mode_group *mode_group = NULL;

Why not move this mode_group=NULL; into the if() condition below?
Avoids global initialization and makes the "if() - else" parts easier
to read as you directly see that mode_group is set to NULL for
non-legacy nodes.

>
>         if (!drm_core_check_feature(dev, DRIVER_MODESET))
>                 return -EINVAL;
> @@ -1429,8 +1429,7 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
>         mutex_unlock(&file_priv->fbs_lock);
>
>         drm_modeset_lock_all(dev);
> -       mode_group = &file_priv->master->minor->mode_group;
> -       if (file_priv->master->minor->type == DRM_MINOR_CONTROL) {
> +       if (file_priv->minor->type != DRM_MINOR_LEGACY) {
>
>                 list_for_each(lh, &dev->mode_config.crtc_list)
>                         crtc_count++;
> @@ -1442,6 +1441,7 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
>                         encoder_count++;
>         } else {
>
> +               mode_group = &file_priv->master->minor->mode_group;
>                 crtc_count = mode_group->num_crtcs;
>                 connector_count = mode_group->num_connectors;
>                 encoder_count = mode_group->num_encoders;
> @@ -1456,7 +1456,7 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
>         if (card_res->count_crtcs >= crtc_count) {
>                 copied = 0;
>                 crtc_id = (uint32_t __user *)(unsigned long)card_res->crtc_id_ptr;
> -               if (file_priv->master->minor->type == DRM_MINOR_CONTROL) {
> +               if (file_priv->minor->type != DRM_MINOR_LEGACY) {

if (!mode_group) {

You set mode_group=NULL with your patch, so I think it's much easier
to read if you replace all these minor-id-tests with "if
(!mode_group)". This moves the group-selection to the head of the
function and makes the remaining parts just work on the selected group
(or global if NULL).

Same for the two conditions below..

Apart from that:
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>

Thanks
David

>                         list_for_each_entry(crtc, &dev->mode_config.crtc_list,
>                                             head) {
>                                 DRM_DEBUG_KMS("[CRTC:%d]\n", crtc->base.id);
> @@ -1483,7 +1483,7 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
>         if (card_res->count_encoders >= encoder_count) {
>                 copied = 0;
>                 encoder_id = (uint32_t __user *)(unsigned long)card_res->encoder_id_ptr;
> -               if (file_priv->master->minor->type == DRM_MINOR_CONTROL) {
> +               if (file_priv->minor->type != DRM_MINOR_LEGACY) {

if (!mode_group) {

>                         list_for_each_entry(encoder,
>                                             &dev->mode_config.encoder_list,
>                                             head) {
> @@ -1514,7 +1514,7 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
>         if (card_res->count_connectors >= connector_count) {
>                 copied = 0;
>                 connector_id = (uint32_t __user *)(unsigned long)card_res->connector_id_ptr;
> -               if (file_priv->master->minor->type == DRM_MINOR_CONTROL) {
> +               if (file_priv->minor->type != DRM_MINOR_LEGACY) {

if (!mode_group) {

>                         list_for_each_entry(connector,
>                                             &dev->mode_config.connector_list,
>                                             head) {
> @@ -2715,7 +2715,8 @@ int drm_mode_getfb(struct drm_device *dev,
>         r->bpp = fb->bits_per_pixel;
>         r->pitch = fb->pitches[0];
>         if (fb->funcs->create_handle) {
> -               if (file_priv->is_master || capable(CAP_SYS_ADMIN)) {
> +               if (file_priv->is_master || capable(CAP_SYS_ADMIN) ||
> +                   file_priv->minor->type == DRM_MINOR_CONTROL) {
>                         ret = fb->funcs->create_handle(fb, file_priv,
>                                                        &r->handle);
>                 } else {
> --
> 1.7.10.4
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 3b7d32d..c9d895a 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1398,7 +1398,7 @@  int drm_mode_getresources(struct drm_device *dev, void *data,
 	uint32_t __user *crtc_id;
 	uint32_t __user *connector_id;
 	uint32_t __user *encoder_id;
-	struct drm_mode_group *mode_group;
+	struct drm_mode_group *mode_group = NULL;
 
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		return -EINVAL;
@@ -1429,8 +1429,7 @@  int drm_mode_getresources(struct drm_device *dev, void *data,
 	mutex_unlock(&file_priv->fbs_lock);
 
 	drm_modeset_lock_all(dev);
-	mode_group = &file_priv->master->minor->mode_group;
-	if (file_priv->master->minor->type == DRM_MINOR_CONTROL) {
+	if (file_priv->minor->type != DRM_MINOR_LEGACY) {
 
 		list_for_each(lh, &dev->mode_config.crtc_list)
 			crtc_count++;
@@ -1442,6 +1441,7 @@  int drm_mode_getresources(struct drm_device *dev, void *data,
 			encoder_count++;
 	} else {
 
+		mode_group = &file_priv->master->minor->mode_group;
 		crtc_count = mode_group->num_crtcs;
 		connector_count = mode_group->num_connectors;
 		encoder_count = mode_group->num_encoders;
@@ -1456,7 +1456,7 @@  int drm_mode_getresources(struct drm_device *dev, void *data,
 	if (card_res->count_crtcs >= crtc_count) {
 		copied = 0;
 		crtc_id = (uint32_t __user *)(unsigned long)card_res->crtc_id_ptr;
-		if (file_priv->master->minor->type == DRM_MINOR_CONTROL) {
+		if (file_priv->minor->type != DRM_MINOR_LEGACY) {
 			list_for_each_entry(crtc, &dev->mode_config.crtc_list,
 					    head) {
 				DRM_DEBUG_KMS("[CRTC:%d]\n", crtc->base.id);
@@ -1483,7 +1483,7 @@  int drm_mode_getresources(struct drm_device *dev, void *data,
 	if (card_res->count_encoders >= encoder_count) {
 		copied = 0;
 		encoder_id = (uint32_t __user *)(unsigned long)card_res->encoder_id_ptr;
-		if (file_priv->master->minor->type == DRM_MINOR_CONTROL) {
+		if (file_priv->minor->type != DRM_MINOR_LEGACY) {
 			list_for_each_entry(encoder,
 					    &dev->mode_config.encoder_list,
 					    head) {
@@ -1514,7 +1514,7 @@  int drm_mode_getresources(struct drm_device *dev, void *data,
 	if (card_res->count_connectors >= connector_count) {
 		copied = 0;
 		connector_id = (uint32_t __user *)(unsigned long)card_res->connector_id_ptr;
-		if (file_priv->master->minor->type == DRM_MINOR_CONTROL) {
+		if (file_priv->minor->type != DRM_MINOR_LEGACY) {
 			list_for_each_entry(connector,
 					    &dev->mode_config.connector_list,
 					    head) {
@@ -2715,7 +2715,8 @@  int drm_mode_getfb(struct drm_device *dev,
 	r->bpp = fb->bits_per_pixel;
 	r->pitch = fb->pitches[0];
 	if (fb->funcs->create_handle) {
-		if (file_priv->is_master || capable(CAP_SYS_ADMIN)) {
+		if (file_priv->is_master || capable(CAP_SYS_ADMIN) ||
+		    file_priv->minor->type == DRM_MINOR_CONTROL) {
 			ret = fb->funcs->create_handle(fb, file_priv,
 						       &r->handle);
 		} else {