Re: [PATCH v11 07/11] drm: Add helper functions to handle aspect-ratio flag bits

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 4/23/2018 3:43 PM, Ville Syrjälä wrote:
On Mon, Apr 23, 2018 at 10:55:54AM +0530, Nautiyal, Ankit K wrote:

On 4/20/2018 7:42 PM, Ville Syrjälä wrote:
On Fri, Apr 20, 2018 at 07:01:47PM +0530, Nautiyal, Ankit K wrote:
From: Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx>

This patch adds helper functions for determining if aspect-ratio is
expected in user-mode and for allowing/disallowing the aspect-ratio,
if its not expected.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx>
---
   drivers/gpu/drm/drm_modes.c | 47 +++++++++++++++++++++++++++++++++++++++++++++
   include/drm/drm_modes.h     |  4 ++++
   2 files changed, 51 insertions(+)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index c395a24..d6133e8 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1759,3 +1759,50 @@ bool drm_mode_is_420(const struct drm_display_info *display,
            drm_mode_is_420_also(display, mode);
   }
   EXPORT_SYMBOL(drm_mode_is_420);
+
+/**
+ * drm_mode_aspect_ratio_allowed - checks if the aspect-ratio information
+ * is expected from the user-mode.
+ *
+ * If the user has set aspect-ratio cap, then the flag of the user-mode is
+ * allowed to contain aspect-ratio value.
+ * If the user does not set aspect-ratio cap, then the only value allowed in the
+ * flags bits is aspect-ratio NONE.
+ *
+ * @file_priv: file private structure to get the user capabilities
+ * @umode: drm_mode_modeinfo struct, whose flag carry the aspect ratio
+ * information.
+ *
+ * Returns:
+ * true if the aspect-ratio info is allowed in the user-mode flags.
+ * false, otherwise.
+ */
+bool
+drm_mode_aspect_ratio_allowed(const struct drm_file *file_priv,
+                        struct drm_mode_modeinfo *umode)
+{
+  return file_priv->aspect_ratio_allowed || (umode->flags &
+          DRM_MODE_FLAG_PIC_AR_MASK) == DRM_MODE_FLAG_PIC_AR_NONE;
Odd line split here. Makes this a bit hard to read.
I would split after the ||
Agreed. I wasn't sure how to let it have better readability and less
than 80 char length
at the same time. Will fix this.

+}
+EXPORT_SYMBOL(drm_mode_aspect_ratio_allowed);
Do we actually need to export these? I don't think so.

But I might be wrong. It's a bit hard to see with the way
you split this patch with the actual users in a different patch.
These helper functions are used in drm_mode_atomic.c, drm_mode_crtc.c,
and drm_mode_get_connector.c
The patches are split as to have modes-set handling separate and the
exposing of connector separate.
Do you suggest this patch to be merged with the patch for handling
aspect-ratio in modeset?
Usually if you add a function you should do it in the same patch where
you add the user(s) for said function. Otherwise it's impossibly to
judge whether the new function is any good.

That makes sense. I will merge this patch with the patch for handling aspect ratio in modeset path,
where its actually used.


+
+/**
+ * drm_mode_filter_aspect_ratio_flags - filters the aspect-ratio bits in the
+ * user-mode flags.
+ *
+ * Checks if the aspect-ratio information is allowed. Resets the aspect-ratio
+ * bits in the user-mode flags, if aspect-ratio info is not allowed.
+ *
+ * @file_priv: file private structure to get the user capabilities.
+ * @umode: drm_mode_modeinfo struct, whose flags' aspect-ratio bits needs to
+ * be filtered.
+ *
+ */
+void
+drm_mode_filter_aspect_ratio_flags(const struct drm_file *file_priv,
+                             struct drm_mode_modeinfo *umode)
+{
+  if (!drm_mode_aspect_ratio_allowed(file_priv, umode))
+          umode->flags &= ~DRM_MODE_FLAG_PIC_AR_MASK;
+}
+EXPORT_SYMBOL(drm_mode_filter_aspect_ratio_flags);
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index 2f78b7e..e0b060d 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -461,6 +461,10 @@ bool drm_mode_is_420_also(const struct drm_display_info *display,
                      const struct drm_display_mode *mode);
   bool drm_mode_is_420(const struct drm_display_info *display,
                 const struct drm_display_mode *mode);
+bool drm_mode_aspect_ratio_allowed(const struct drm_file *file_priv,
+                             struct drm_mode_modeinfo *umode);
+void drm_mode_filter_aspect_ratio_flags(const struct drm_file *file_priv,
+                                  struct drm_mode_modeinfo *umode);

   struct drm_display_mode *drm_cvt_mode(struct drm_device *dev,
                                  int hdisplay, int vdisplay, int vrefresh,
--
2.7.4
--
Ville Syrjälä
Intel

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux