From: Ville Syrjala <ville.syrjala@linux.intel.com> To: dri-devel@lists.freedesktop.org Cc: Daniel Vetter <daniel.vetter@ffwll.ch>, intel-gfx@lists.freedesktop.org, Sam Ravnborg <sam@ravnborg.org>, Emil Velikov <emil.l.velikov@gmail.com> Subject: [PATCH v3 16/16] drm: Replace mode->export_head with a boolean Date: Tue, 28 Apr 2020 20:19:40 +0300 [thread overview] Message-ID: <20200428171940.19552-17-ville.syrjala@linux.intel.com> (raw) In-Reply-To: <20200428171940.19552-1-ville.syrjala@linux.intel.com> From: Ville Syrjälä <ville.syrjala@linux.intel.com> In order to shrink drm_display_mode below the magic two cacheline mark in 64bit we need to shrink it by another 8 bytes. The easiest thing to eliminate is the 'export_head' list head which is only used during the getconnector ioctl to temporarly track which modes on the connector's mode list are to be exposed and which are to remain hidden. We can simply replace the list head with a boolean which we use to tag the modes that are to be exposed. If we make sure to clear the tags after we're done with them we don't even need an extra loop over the modes to reset the tags at the start of the getconnector ioctl. Conveniently we already have a hole for the boolean left behind by the removal of mode->private_flags. The final size of the struct is now 112 bytes on 32bit and 120 bytes on 64bit. The downside is that drm_mode_expose_to_userspace() gets to iterate a few more modes. It already was O(n^2), now it's a slightly worse O(n^2). Another alternative would be a temp bitmask so we wouldn't have to have anything in the mode struct itself. The main issue is how large of a bitmask do we need? I guess we could allocate it dynamically but that means an extra kcalloc() and an extra loop through the modes to count them first (or grow the bitmask with krealloc() as needed). CC: Sam Ravnborg <sam@ravnborg.org> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Emil Velikov <emil.l.velikov@gmail.com> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/drm_connector.c | 45 +++++++++++++++++++++++---------- include/drm/drm_modes.h | 24 ++++++++---------- 2 files changed, 43 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index b1099e1251a2..7e719b08564d 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -2198,7 +2198,7 @@ static struct drm_encoder *drm_connector_get_encoder(struct drm_connector *conne static bool drm_mode_expose_to_userspace(const struct drm_display_mode *mode, - const struct list_head *export_list, + const struct list_head *modes, const struct drm_file *file_priv) { /* @@ -2214,15 +2214,17 @@ drm_mode_expose_to_userspace(const struct drm_display_mode *mode, * while preparing the list of user-modes. */ if (!file_priv->aspect_ratio_allowed) { - struct drm_display_mode *mode_itr; + const struct drm_display_mode *mode_itr; - list_for_each_entry(mode_itr, export_list, export_head) - if (drm_mode_match(mode_itr, mode, + list_for_each_entry(mode_itr, modes, head) { + if (mode_itr->expose_to_userspace && + drm_mode_match(mode_itr, mode, DRM_MODE_MATCH_TIMINGS | DRM_MODE_MATCH_CLOCK | DRM_MODE_MATCH_FLAGS | DRM_MODE_MATCH_3D_FLAGS)) return false; + } } return true; @@ -2242,7 +2244,6 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, struct drm_mode_modeinfo u_mode; struct drm_mode_modeinfo __user *mode_ptr; uint32_t __user *encoder_ptr; - LIST_HEAD(export_list); if (!drm_core_check_feature(dev, DRIVER_MODESET)) return -EOPNOTSUPP; @@ -2286,25 +2287,30 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, out_resp->connection = connector->status; /* delayed so we get modes regardless of pre-fill_modes state */ - list_for_each_entry(mode, &connector->modes, head) - if (drm_mode_expose_to_userspace(mode, &export_list, + list_for_each_entry(mode, &connector->modes, head) { + WARN_ON(mode->expose_to_userspace); + + if (drm_mode_expose_to_userspace(mode, &connector->modes, file_priv)) { - list_add_tail(&mode->export_head, &export_list); + mode->expose_to_userspace = true; mode_count++; } + } /* * This ioctl is called twice, once to determine how much space is * needed, and the 2nd time to fill it. - * The modes that need to be exposed to the user are maintained in the - * 'export_list'. When the ioctl is called first time to determine the, - * space, the export_list gets filled, to find the no.of modes. In the - * 2nd time, the user modes are filled, one by one from the export_list. */ if ((out_resp->count_modes >= mode_count) && mode_count) { copied = 0; mode_ptr = (struct drm_mode_modeinfo __user *)(unsigned long)out_resp->modes_ptr; - list_for_each_entry(mode, &export_list, export_head) { + list_for_each_entry(mode, &connector->modes, head) { + if (!mode->expose_to_userspace) + continue; + + /* Clear the tag for the next time around */ + mode->expose_to_userspace = false; + drm_mode_convert_to_umode(&u_mode, mode); /* * Reset aspect ratio flags of user-mode, if modes with @@ -2315,13 +2321,26 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, if (copy_to_user(mode_ptr + copied, &u_mode, sizeof(u_mode))) { ret = -EFAULT; + + /* + * Clear the tag for the rest of + * the modes for the next time around. + */ + list_for_each_entry_continue(mode, &connector->modes, head) + mode->expose_to_userspace = false; + mutex_unlock(&dev->mode_config.mutex); goto out; } copied++; } + } else { + /* Clear the tag for the next time around */ + list_for_each_entry(mode, &connector->modes, head) + mode->expose_to_userspace = false; } + out_resp->count_modes = mode_count; mutex_unlock(&dev->mode_config.mutex); diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h index 1e97138a9b8c..ac0589aab23e 100644 --- a/include/drm/drm_modes.h +++ b/include/drm/drm_modes.h @@ -348,6 +348,17 @@ struct drm_display_mode { */ u8 type; + /** + * @expose_to_userspace: + * + * Indicates whether the mode is to be exposed to the userspace. + * This is to maintain a set of exposed modes while preparing + * user-mode's list in drm_mode_getconnector ioctl. The purpose of + * this only lies in the ioctl function, and is not to be used + * outside the function. + */ + bool expose_to_userspace; + /** * @head: * @@ -355,19 +366,6 @@ struct drm_display_mode { */ struct list_head head; - /** - * @export_head: - * - * struct list_head for modes to be exposed to the userspace. - * This is to maintain a list of exposed modes while preparing - * user-mode's list in drm_mode_getconnector ioctl. The purpose of this - * list_head only lies in the ioctl function, and is not expected to be - * used outside the function. - * Once used, the stale pointers are not reset, but left as it is, to - * avoid overhead of protecting it by mode_config.mutex. - */ - struct list_head export_head; - /** * @name: * -- 2.24.1 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
WARNING: multiple messages have this Message-ID (diff)
From: Ville Syrjala <ville.syrjala@linux.intel.com> To: dri-devel@lists.freedesktop.org Cc: Daniel Vetter <daniel.vetter@ffwll.ch>, intel-gfx@lists.freedesktop.org, Sam Ravnborg <sam@ravnborg.org> Subject: [Intel-gfx] [PATCH v3 16/16] drm: Replace mode->export_head with a boolean Date: Tue, 28 Apr 2020 20:19:40 +0300 [thread overview] Message-ID: <20200428171940.19552-17-ville.syrjala@linux.intel.com> (raw) In-Reply-To: <20200428171940.19552-1-ville.syrjala@linux.intel.com> From: Ville Syrjälä <ville.syrjala@linux.intel.com> In order to shrink drm_display_mode below the magic two cacheline mark in 64bit we need to shrink it by another 8 bytes. The easiest thing to eliminate is the 'export_head' list head which is only used during the getconnector ioctl to temporarly track which modes on the connector's mode list are to be exposed and which are to remain hidden. We can simply replace the list head with a boolean which we use to tag the modes that are to be exposed. If we make sure to clear the tags after we're done with them we don't even need an extra loop over the modes to reset the tags at the start of the getconnector ioctl. Conveniently we already have a hole for the boolean left behind by the removal of mode->private_flags. The final size of the struct is now 112 bytes on 32bit and 120 bytes on 64bit. The downside is that drm_mode_expose_to_userspace() gets to iterate a few more modes. It already was O(n^2), now it's a slightly worse O(n^2). Another alternative would be a temp bitmask so we wouldn't have to have anything in the mode struct itself. The main issue is how large of a bitmask do we need? I guess we could allocate it dynamically but that means an extra kcalloc() and an extra loop through the modes to count them first (or grow the bitmask with krealloc() as needed). CC: Sam Ravnborg <sam@ravnborg.org> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Emil Velikov <emil.l.velikov@gmail.com> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/drm_connector.c | 45 +++++++++++++++++++++++---------- include/drm/drm_modes.h | 24 ++++++++---------- 2 files changed, 43 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index b1099e1251a2..7e719b08564d 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -2198,7 +2198,7 @@ static struct drm_encoder *drm_connector_get_encoder(struct drm_connector *conne static bool drm_mode_expose_to_userspace(const struct drm_display_mode *mode, - const struct list_head *export_list, + const struct list_head *modes, const struct drm_file *file_priv) { /* @@ -2214,15 +2214,17 @@ drm_mode_expose_to_userspace(const struct drm_display_mode *mode, * while preparing the list of user-modes. */ if (!file_priv->aspect_ratio_allowed) { - struct drm_display_mode *mode_itr; + const struct drm_display_mode *mode_itr; - list_for_each_entry(mode_itr, export_list, export_head) - if (drm_mode_match(mode_itr, mode, + list_for_each_entry(mode_itr, modes, head) { + if (mode_itr->expose_to_userspace && + drm_mode_match(mode_itr, mode, DRM_MODE_MATCH_TIMINGS | DRM_MODE_MATCH_CLOCK | DRM_MODE_MATCH_FLAGS | DRM_MODE_MATCH_3D_FLAGS)) return false; + } } return true; @@ -2242,7 +2244,6 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, struct drm_mode_modeinfo u_mode; struct drm_mode_modeinfo __user *mode_ptr; uint32_t __user *encoder_ptr; - LIST_HEAD(export_list); if (!drm_core_check_feature(dev, DRIVER_MODESET)) return -EOPNOTSUPP; @@ -2286,25 +2287,30 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, out_resp->connection = connector->status; /* delayed so we get modes regardless of pre-fill_modes state */ - list_for_each_entry(mode, &connector->modes, head) - if (drm_mode_expose_to_userspace(mode, &export_list, + list_for_each_entry(mode, &connector->modes, head) { + WARN_ON(mode->expose_to_userspace); + + if (drm_mode_expose_to_userspace(mode, &connector->modes, file_priv)) { - list_add_tail(&mode->export_head, &export_list); + mode->expose_to_userspace = true; mode_count++; } + } /* * This ioctl is called twice, once to determine how much space is * needed, and the 2nd time to fill it. - * The modes that need to be exposed to the user are maintained in the - * 'export_list'. When the ioctl is called first time to determine the, - * space, the export_list gets filled, to find the no.of modes. In the - * 2nd time, the user modes are filled, one by one from the export_list. */ if ((out_resp->count_modes >= mode_count) && mode_count) { copied = 0; mode_ptr = (struct drm_mode_modeinfo __user *)(unsigned long)out_resp->modes_ptr; - list_for_each_entry(mode, &export_list, export_head) { + list_for_each_entry(mode, &connector->modes, head) { + if (!mode->expose_to_userspace) + continue; + + /* Clear the tag for the next time around */ + mode->expose_to_userspace = false; + drm_mode_convert_to_umode(&u_mode, mode); /* * Reset aspect ratio flags of user-mode, if modes with @@ -2315,13 +2321,26 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, if (copy_to_user(mode_ptr + copied, &u_mode, sizeof(u_mode))) { ret = -EFAULT; + + /* + * Clear the tag for the rest of + * the modes for the next time around. + */ + list_for_each_entry_continue(mode, &connector->modes, head) + mode->expose_to_userspace = false; + mutex_unlock(&dev->mode_config.mutex); goto out; } copied++; } + } else { + /* Clear the tag for the next time around */ + list_for_each_entry(mode, &connector->modes, head) + mode->expose_to_userspace = false; } + out_resp->count_modes = mode_count; mutex_unlock(&dev->mode_config.mutex); diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h index 1e97138a9b8c..ac0589aab23e 100644 --- a/include/drm/drm_modes.h +++ b/include/drm/drm_modes.h @@ -348,6 +348,17 @@ struct drm_display_mode { */ u8 type; + /** + * @expose_to_userspace: + * + * Indicates whether the mode is to be exposed to the userspace. + * This is to maintain a set of exposed modes while preparing + * user-mode's list in drm_mode_getconnector ioctl. The purpose of + * this only lies in the ioctl function, and is not to be used + * outside the function. + */ + bool expose_to_userspace; + /** * @head: * @@ -355,19 +366,6 @@ struct drm_display_mode { */ struct list_head head; - /** - * @export_head: - * - * struct list_head for modes to be exposed to the userspace. - * This is to maintain a list of exposed modes while preparing - * user-mode's list in drm_mode_getconnector ioctl. The purpose of this - * list_head only lies in the ioctl function, and is not expected to be - * used outside the function. - * Once used, the stale pointers are not reset, but left as it is, to - * avoid overhead of protecting it by mode_config.mutex. - */ - struct list_head export_head; - /** * @name: * -- 2.24.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2020-04-28 17:20 UTC|newest] Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-04-28 17:19 [PATCH v3 00/16] drm: Put drm_display_mode on diet Ville Syrjala 2020-04-28 17:19 ` [Intel-gfx] " Ville Syrjala 2020-04-28 17:19 ` [PATCH v3 01/16] drm: Nuke mode->hsync Ville Syrjala 2020-04-28 17:19 ` [Intel-gfx] " Ville Syrjala 2020-04-28 17:19 ` [PATCH v3 02/16] drm/i915: Introduce some local intel_dp variables Ville Syrjala 2020-04-28 17:19 ` [Intel-gfx] " Ville Syrjala 2020-04-28 17:19 ` [PATCH v3 03/16] drm: Nuke mode->vrefresh Ville Syrjala 2020-04-28 17:19 ` Ville Syrjala 2020-04-28 17:19 ` [Intel-gfx] " Ville Syrjala 2020-04-28 17:19 ` Ville Syrjala 2020-04-28 17:19 ` [PATCH v3 04/16] drm/msm/dpu: Stop copying around mode->private_flags Ville Syrjala 2020-04-28 17:19 ` [Intel-gfx] " Ville Syrjala 2020-04-28 17:19 ` Ville Syrjala 2020-04-28 17:19 ` [PATCH v3 05/16] drm: Shrink {width,height}_mm to u16 Ville Syrjala 2020-04-28 17:19 ` [Intel-gfx] " Ville Syrjala 2020-04-28 17:19 ` [PATCH v3 06/16] drm: Shrink mode->type to u8 Ville Syrjala 2020-04-28 17:19 ` [Intel-gfx] " Ville Syrjala 2020-04-28 17:19 ` [PATCH v3 07/16] drm: Make mode->flags u32 Ville Syrjala 2020-04-28 17:19 ` [Intel-gfx] " Ville Syrjala 2020-04-28 17:19 ` [PATCH v3 08/16] drm: Shrink drm_display_mode timings Ville Syrjala 2020-04-28 17:19 ` [Intel-gfx] " Ville Syrjala 2020-04-28 17:19 ` [PATCH v3 09/16] drm: Flatten drm_mode_vrefresh() Ville Syrjala 2020-04-28 17:19 ` [Intel-gfx] " Ville Syrjala 2020-04-28 17:19 ` [PATCH v3 10/16] drm: pahole struct drm_display_mode Ville Syrjala 2020-04-28 17:19 ` [Intel-gfx] " Ville Syrjala 2020-04-28 17:19 ` [PATCH v3 11/16] drm/mcde: Use mode->clock instead of reverse calculating it from the vrefresh Ville Syrjala 2020-04-28 17:19 ` [Intel-gfx] " Ville Syrjala 2020-04-28 17:19 ` [PATCH v3 12/16] drm/i915: Stop using mode->private_flags Ville Syrjala 2020-04-28 17:19 ` [Intel-gfx] " Ville Syrjala 2020-04-29 10:39 ` [PATCH v4 " Ville Syrjala 2020-04-29 10:39 ` [Intel-gfx] " Ville Syrjala 2020-04-28 17:19 ` [PATCH v3 13/16] drm/i915: Replace I915_MODE_FLAG_INHERITED with a boolean Ville Syrjala 2020-04-28 17:19 ` [Intel-gfx] " Ville Syrjala 2020-04-29 10:39 ` [PATCH v4 " Ville Syrjala 2020-04-29 10:39 ` [Intel-gfx] " Ville Syrjala 2020-04-28 17:19 ` [PATCH v3 14/16] drm/gma500: Stop using mode->private_flags Ville Syrjala 2020-04-28 17:19 ` [Intel-gfx] " Ville Syrjala 2020-04-28 17:19 ` [PATCH v3 15/16] drm: Nuke mode->private_flags Ville Syrjala 2020-04-28 17:19 ` [Intel-gfx] " Ville Syrjala 2020-04-28 17:19 ` Ville Syrjala [this message] 2020-04-28 17:19 ` [Intel-gfx] [PATCH v3 16/16] drm: Replace mode->export_head with a boolean Ville Syrjala 2020-04-30 13:50 ` Emil Velikov 2020-04-30 13:50 ` [Intel-gfx] " Emil Velikov 2020-09-01 10:58 ` Ville Syrjälä 2020-09-01 10:58 ` [Intel-gfx] " Ville Syrjälä 2020-04-28 19:59 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm: Put drm_display_mode on diet (rev4) Patchwork 2020-04-28 20:26 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork 2020-04-29 11:38 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm: Put drm_display_mode on diet (rev6) Patchwork 2020-04-29 12:04 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork 2020-04-29 15:27 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200428171940.19552-17-ville.syrjala@linux.intel.com \ --to=ville.syrjala@linux.intel.com \ --cc=daniel.vetter@ffwll.ch \ --cc=dri-devel@lists.freedesktop.org \ --cc=emil.l.velikov@gmail.com \ --cc=intel-gfx@lists.freedesktop.org \ --cc=sam@ravnborg.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.