| Message ID | 20190622034105.188454-2-dbasehore@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Panel rotation patches | expand |
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 <dbasehore@chromium.org> > --- > 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", &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 <treding@nvidia.com>"); > 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
On Mon, Jun 24, 2019 at 1:36 PM Sam Ravnborg <sam@ravnborg.org> 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 <dbasehore@chromium.org> > > --- > > 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", &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 <treding@nvidia.com>"); > > 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
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", &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 <treding@nvidia.com>"); 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
This adds a helper function for reading the rotation (panel orientation) from the device tree. Signed-off-by: Derek Basehore <dbasehore@chromium.org> --- drivers/gpu/drm/drm_panel.c | 42 +++++++++++++++++++++++++++++++++++++ include/drm/drm_panel.h | 7 +++++++ 2 files changed, 49 insertions(+)