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