Re: [Intel-gfx] [PATCH v3 1/4] drm/panel: Add helper for reading DT rotation
On Mon, Jun 24, 2019 at 1:36 PM Sam Ravnborg wrote:
>
> Hi Derek.
>
> On Fri, Jun 21, 2019 at 08:41:02PM -0700, Derek Basehore wrote:
> > This adds a helper function for reading the rotation (panel
> > orientation) from the device tree.
> >
> > Signed-off-by: Derek Basehore
> > ---
> > drivers/gpu/drm/drm_panel.c | 42 +
> > include/drm/drm_panel.h | 7 +++
> > 2 files changed, 49 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> > index dbd5b873e8f2..507099af4b57 100644
> > --- a/drivers/gpu/drm/drm_panel.c
> > +++ b/drivers/gpu/drm/drm_panel.c
> > @@ -172,6 +172,48 @@ struct drm_panel *of_drm_find_panel(const struct
> > device_node *np)
> > return ERR_PTR(-EPROBE_DEFER);
> > }
> > EXPORT_SYMBOL(of_drm_find_panel);
> > +
> > +/**
> > + * of_drm_get_panel_orientation - look up the rotation of the panel using a
> > + * device tree node
> > + * @np: device tree node of the panel
> > + * @orientation: orientation enum to be filled in
> > + *
> > + * Looks up the rotation of a panel in the device tree. The rotation in the
> > + * device tree is counter clockwise.
> > + *
> > + * Return: 0 when a valid rotation value (0, 90, 180, or 270) is read or
> > the
> > + * rotation property doesn't exist. -EERROR otherwise.
> > + */
> This function should better spell out why it talks about rotation versus
> orientation.
>
> It happens that orientation, due to bad design choices is named rotation
> in DT.
> But then this function is all about orientation, that just happens to be
> named rotation in DT.
> And the comments associated to the function should reflect this.
>
> something like:
> /**
> * of_drm_get_panel_orientation - look up the orientation of the panel using a
> * device tree node
> * @np: device tree node of the panel
> * @orientation: orientation enum to be filled in
> *
> * Looks up the rotation property of a panel in the device tree.
> * The orientation of the panel is expressed as a property named
> * "rotation" in the device tree.
> * The rotation in the device tree is counter clockwise.
> *
> * Return: 0 when a valid orientation value (0, 90, 180, or 270) is read or
> the
> * rotation property doesn't exist. -EERROR otherwise.
> */
I'll clear this up in the next patch set.
>
> This would at least remove some of my confusiuon.
> And then maybe add a bit more explanation to the binding property
> description too.
Tried this, yet I got told off for adding kernel details to the DT
documentation (which is frowned upon).
>
> Sam
>
>
>
>
>
>
>
>
>
>
>
>
> > +int of_drm_get_panel_orientation(const struct device_node *np,
> > + enum drm_panel_orientation *orientation)
> > +{
> > + int rotation, ret;
> > +
> > + ret = of_property_read_u32(np, "rotation", );
> > + if (ret == -EINVAL) {
> > + /* Don't return an error if there's no rotation property. */
> > + *orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
> > + return 0;
> > + }
> > +
> > + if (ret < 0)
> > + return ret;
> > +
> > + if (rotation == 0)
> > + *orientation = DRM_MODE_PANEL_ORIENTATION_NORMAL;
> > + else if (rotation == 90)
> > + *orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP;
> > + else if (rotation == 180)
> > + *orientation = DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
> > + else if (rotation == 270)
> > + *orientation = DRM_MODE_PANEL_ORIENTATION_LEFT_UP;
> > + else
> > + return -EINVAL;
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(of_drm_get_panel_orientation);
> > #endif
> >
> > MODULE_AUTHOR("Thierry Reding ");
> > diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> > index 8c738c0e6e9f..3564952f1a4f 100644
> > --- a/include/drm/drm_panel.h
> > +++ b/include/drm/drm_panel.h
> > @@ -197,11 +197,18 @@ int drm_panel_detach(struct drm_panel *panel);
> >
> > #if defined(CONFIG_OF) && defined(CONFIG_DRM_PANEL)
> > struct drm_panel *of_drm_find_panel(const struct device_node *np);
> > +int of_drm_get_panel_orientation(const struct device_node *np,
> > + enum drm_panel_orientation *orientation);
> > #else
> > static inline struct drm_panel *of_drm_find_panel(const struct device_node
> > *np)
> > {
> > return ERR_PTR(-ENODEV);
> > }
> > +int of_drm_get_panel_orientation(const struct device_node *np,
> > + enum drm_panel_orientation *orientation)
> > +{
> > + return -ENODEV;
> > +}
> > #endif
> >
> > #endif
> > --
> > 2.22.0.410.gd8fdbe21b5-goog
Thanks for the review
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 1/4] drm/panel: Add helper for reading DT rotation
Hi Derek.
On Fri, Jun 21, 2019 at 08:41:02PM -0700, Derek Basehore wrote:
> This adds a helper function for reading the rotation (panel
> orientation) from the device tree.
>
> Signed-off-by: Derek Basehore
> ---
> drivers/gpu/drm/drm_panel.c | 42 +
> include/drm/drm_panel.h | 7 +++
> 2 files changed, 49 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> index dbd5b873e8f2..507099af4b57 100644
> --- a/drivers/gpu/drm/drm_panel.c
> +++ b/drivers/gpu/drm/drm_panel.c
> @@ -172,6 +172,48 @@ struct drm_panel *of_drm_find_panel(const struct
> device_node *np)
> return ERR_PTR(-EPROBE_DEFER);
> }
> EXPORT_SYMBOL(of_drm_find_panel);
> +
> +/**
> + * of_drm_get_panel_orientation - look up the rotation of the panel using a
> + * device tree node
> + * @np: device tree node of the panel
> + * @orientation: orientation enum to be filled in
> + *
> + * Looks up the rotation of a panel in the device tree. The rotation in the
> + * device tree is counter clockwise.
> + *
> + * Return: 0 when a valid rotation value (0, 90, 180, or 270) is read or the
> + * rotation property doesn't exist. -EERROR otherwise.
> + */
This function should better spell out why it talks about rotation versus
orientation.
It happens that orientation, due to bad design choices is named rotation
in DT.
But then this function is all about orientation, that just happens to be
named rotation in DT.
And the comments associated to the function should reflect this.
something like:
/**
* of_drm_get_panel_orientation - look up the orientation of the panel using a
* device tree node
* @np: device tree node of the panel
* @orientation: orientation enum to be filled in
*
* Looks up the rotation property of a panel in the device tree.
* The orientation of the panel is expressed as a property named
* "rotation" in the device tree.
* The rotation in the device tree is counter clockwise.
*
* Return: 0 when a valid orientation value (0, 90, 180, or 270) is read or the
* rotation property doesn't exist. -EERROR otherwise.
*/
This would at least remove some of my confusiuon.
And then maybe add a bit more explanation to the binding property
description too.
Sam
> +int of_drm_get_panel_orientation(const struct device_node *np,
> + enum drm_panel_orientation *orientation)
> +{
> + int rotation, ret;
> +
> + ret = of_property_read_u32(np, "rotation", );
> + if (ret == -EINVAL) {
> + /* Don't return an error if there's no rotation property. */
> + *orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
> + return 0;
> + }
> +
> + if (ret < 0)
> + return ret;
> +
> + if (rotation == 0)
> + *orientation = DRM_MODE_PANEL_ORIENTATION_NORMAL;
> + else if (rotation == 90)
> + *orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP;
> + else if (rotation == 180)
> + *orientation = DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
> + else if (rotation == 270)
> + *orientation = DRM_MODE_PANEL_ORIENTATION_LEFT_UP;
> + else
> + return -EINVAL;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(of_drm_get_panel_orientation);
> #endif
>
> MODULE_AUTHOR("Thierry Reding ");
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index 8c738c0e6e9f..3564952f1a4f 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -197,11 +197,18 @@ int drm_panel_detach(struct drm_panel *panel);
>
> #if defined(CONFIG_OF) && defined(CONFIG_DRM_PANEL)
> struct drm_panel *of_drm_find_panel(const struct device_node *np);
> +int of_drm_get_panel_orientation(const struct device_node *np,
> + enum drm_panel_orientation *orientation);
> #else
> static inline struct drm_panel *of_drm_find_panel(const struct device_node
> *np)
> {
> return ERR_PTR(-ENODEV);
> }
> +int of_drm_get_panel_orientation(const struct device_node *np,
> + enum drm_panel_orientation *orientation)
> +{
> + return -ENODEV;
> +}
> #endif
>
> #endif
> --
> 2.22.0.410.gd8fdbe21b5-goog
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 1/4] drm/panel: Add helper for reading DT rotation
This adds a helper function for reading the rotation (panel
orientation) from the device tree.
Signed-off-by: Derek Basehore
---
drivers/gpu/drm/drm_panel.c | 42 +
include/drm/drm_panel.h | 7 +++
2 files changed, 49 insertions(+)
diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index dbd5b873e8f2..507099af4b57 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -172,6 +172,48 @@ struct drm_panel *of_drm_find_panel(const struct
device_node *np)
return ERR_PTR(-EPROBE_DEFER);
}
EXPORT_SYMBOL(of_drm_find_panel);
+
+/**
+ * of_drm_get_panel_orientation - look up the rotation of the panel using a
+ * device tree node
+ * @np: device tree node of the panel
+ * @orientation: orientation enum to be filled in
+ *
+ * Looks up the rotation of a panel in the device tree. The rotation in the
+ * device tree is counter clockwise.
+ *
+ * Return: 0 when a valid rotation value (0, 90, 180, or 270) is read or the
+ * rotation property doesn't exist. -EERROR otherwise.
+ */
+int of_drm_get_panel_orientation(const struct device_node *np,
+enum drm_panel_orientation *orientation)
+{
+ int rotation, ret;
+
+ ret = of_property_read_u32(np, "rotation", );
+ if (ret == -EINVAL) {
+ /* Don't return an error if there's no rotation property. */
+ *orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
+ return 0;
+ }
+
+ if (ret < 0)
+ return ret;
+
+ if (rotation == 0)
+ *orientation = DRM_MODE_PANEL_ORIENTATION_NORMAL;
+ else if (rotation == 90)
+ *orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP;
+ else if (rotation == 180)
+ *orientation = DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
+ else if (rotation == 270)
+ *orientation = DRM_MODE_PANEL_ORIENTATION_LEFT_UP;
+ else
+ return -EINVAL;
+
+ return 0;
+}
+EXPORT_SYMBOL(of_drm_get_panel_orientation);
#endif
MODULE_AUTHOR("Thierry Reding ");
diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index 8c738c0e6e9f..3564952f1a4f 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -197,11 +197,18 @@ int drm_panel_detach(struct drm_panel *panel);
#if defined(CONFIG_OF) && defined(CONFIG_DRM_PANEL)
struct drm_panel *of_drm_find_panel(const struct device_node *np);
+int of_drm_get_panel_orientation(const struct device_node *np,
+enum drm_panel_orientation *orientation);
#else
static inline struct drm_panel *of_drm_find_panel(const struct device_node *np)
{
return ERR_PTR(-ENODEV);
}
+int of_drm_get_panel_orientation(const struct device_node *np,
+enum drm_panel_orientation *orientation)
+{
+ return -ENODEV;
+}
#endif
#endif
--
2.22.0.410.gd8fdbe21b5-goog
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
