* [PATCH] drm/panel: simple: Doxygenize 'struct panel_desc'; rename a few functions
@ 2019-07-12 16:33 Douglas Anderson
2019-07-17 17:33 ` Sam Ravnborg
0 siblings, 1 reply; 4+ messages in thread
From: Douglas Anderson @ 2019-07-12 16:33 UTC (permalink / raw)
To: Thierry Reding, Sam Ravnborg
Cc: Sean Paul, Boris Brezillon, Douglas Anderson, linux-kernel,
dri-devel, David Airlie, Daniel Vetter
This attempts to address outstanding review feedback from commit
b8a2948fa2b3 ("drm/panel: simple: Add ability to override typical
timing"). Specifically:
* It was requested that I document (in the structure definition) that
the device tree override had no effect if 'struct drm_display_mode'
was used in the panel description. I have provided full Doxygen
comments for 'struct panel_desc' to accomplish that.
* panel_simple_get_fixed_modes() was thought to be a confusing name,
so it has been renamed to panel_simple_get_display_modes().
* panel_simple_parse_override_mode() was thought to be better named as
panel_simple_parse_panel_timing_node().
Suggested-by: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
NOTES:
- I have not addressed the suggestion of doing 'bool has_override =
panel->override_mode.type != 0;'. As per my reply on the mailing
list I think the convention in this file is to rely on implicit
conversions from int to bool.
- Sam said that there was still something that he didn't understand
with regards to the flags. Sam: if this is something that needs to
be addressed, please yell.
drivers/gpu/drm/panel/panel-simple.c | 28 ++++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index a50871c6836b..f3c0e203f40f 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -38,6 +38,22 @@
#include <drm/drm_mipi_dsi.h>
#include <drm/drm_panel.h>
+/**
+ * @modes: Pointer to array of fixed modes appropriate for this panel. If
+ * only one mode then this can just be the address of this the mode.
+ * NOTE: cannot be used with "timings" and also if this is specified
+ * then you cannot override the mode in the device tree.
+ * @num_modes: Number of elements in modes array.
+ * @timings: Pointer to array of display timings. NOTE: cannot be used with
+ * "modes" and also these will be used to validate a device tree
+ * override if one is present.
+ * @num_timings: Number of elements in timings array.
+ * @bpc: Bits per color.
+ * @size: Structure containing the physical size of this panel.
+ * @delay: Structure containing various delay values for this panel.
+ * @bus_format: See MEDIA_BUS_FMT_... defines.
+ * @bus_flags: See DRM_BUS_FLAG_... defines.
+ */
struct panel_desc {
const struct drm_display_mode *modes;
unsigned int num_modes;
@@ -135,7 +151,7 @@ static unsigned int panel_simple_get_timings_modes(struct panel_simple *panel)
return num;
}
-static unsigned int panel_simple_get_fixed_modes(struct panel_simple *panel)
+static unsigned int panel_simple_get_display_modes(struct panel_simple *panel)
{
struct drm_connector *connector = panel->base.connector;
struct drm_device *drm = panel->base.drm;
@@ -199,7 +215,7 @@ static int panel_simple_get_non_edid_modes(struct panel_simple *panel)
*/
WARN_ON(panel->desc->num_timings && panel->desc->num_modes);
if (num == 0)
- num = panel_simple_get_fixed_modes(panel);
+ num = panel_simple_get_display_modes(panel);
connector->display_info.bpc = panel->desc->bpc;
connector->display_info.width_mm = panel->desc->size.width;
@@ -351,9 +367,9 @@ static const struct drm_panel_funcs panel_simple_funcs = {
#define PANEL_SIMPLE_BOUNDS_CHECK(to_check, bounds, field) \
(to_check->field.typ >= bounds->field.min && \
to_check->field.typ <= bounds->field.max)
-static void panel_simple_parse_override_mode(struct device *dev,
- struct panel_simple *panel,
- const struct display_timing *ot)
+static void panel_simple_parse_panel_timing_node(struct device *dev,
+ struct panel_simple *panel,
+ const struct display_timing *ot)
{
const struct panel_desc *desc = panel->desc;
struct videomode vm;
@@ -446,7 +462,7 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
}
if (!of_get_display_timing(dev->of_node, "panel-timing", &dt))
- panel_simple_parse_override_mode(dev, panel, &dt);
+ panel_simple_parse_panel_timing_node(dev, panel, &dt);
drm_panel_init(&panel->base);
panel->base.dev = dev;
--
2.22.0.510.g264f2c817a-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/panel: simple: Doxygenize 'struct panel_desc'; rename a few functions
2019-07-12 16:33 [PATCH] drm/panel: simple: Doxygenize 'struct panel_desc'; rename a few functions Douglas Anderson
@ 2019-07-17 17:33 ` Sam Ravnborg
2019-07-21 9:38 ` Sam Ravnborg
0 siblings, 1 reply; 4+ messages in thread
From: Sam Ravnborg @ 2019-07-17 17:33 UTC (permalink / raw)
To: Douglas Anderson
Cc: Thierry Reding, Sean Paul, Boris Brezillon, linux-kernel,
dri-devel, David Airlie, Daniel Vetter
Hi Doug.
On Fri, Jul 12, 2019 at 09:33:33AM -0700, Douglas Anderson wrote:
> This attempts to address outstanding review feedback from commit
> b8a2948fa2b3 ("drm/panel: simple: Add ability to override typical
> timing"). Specifically:
>
> * It was requested that I document (in the structure definition) that
> the device tree override had no effect if 'struct drm_display_mode'
> was used in the panel description. I have provided full Doxygen
> comments for 'struct panel_desc' to accomplish that.
> * panel_simple_get_fixed_modes() was thought to be a confusing name,
> so it has been renamed to panel_simple_get_display_modes().
> * panel_simple_parse_override_mode() was thought to be better named as
> panel_simple_parse_panel_timing_node().
>
> Suggested-by: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
Thanks.
I updated the $subject to:
drm/panel: simple: document panel_desc; rename a few functions
And pushed out to drm-misc-next.
> - Sam said that there was still something that he didn't understand
> with regards to the flags. Sam: if this is something that needs to
> be addressed, please yell.
Need to re-visit this later when I have familiarized myself with
the new yaml syntax and what impact any potential changes may have on
the panel drivers.
So for now we leave it as is.
Sam
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/panel: simple: Doxygenize 'struct panel_desc'; rename a few functions
2019-07-17 17:33 ` Sam Ravnborg
@ 2019-07-21 9:38 ` Sam Ravnborg
2019-07-22 18:26 ` Doug Anderson
0 siblings, 1 reply; 4+ messages in thread
From: Sam Ravnborg @ 2019-07-21 9:38 UTC (permalink / raw)
To: Douglas Anderson
Cc: David Airlie, linux-kernel, dri-devel, Thierry Reding, Sean Paul,
Boris Brezillon
Hi Doug.
On Wed, Jul 17, 2019 at 07:33:17PM +0200, Sam Ravnborg wrote:
> Hi Doug.
>
> On Fri, Jul 12, 2019 at 09:33:33AM -0700, Douglas Anderson wrote:
> > This attempts to address outstanding review feedback from commit
> > b8a2948fa2b3 ("drm/panel: simple: Add ability to override typical
> > timing"). Specifically:
> >
> > * It was requested that I document (in the structure definition) that
> > the device tree override had no effect if 'struct drm_display_mode'
> > was used in the panel description. I have provided full Doxygen
> > comments for 'struct panel_desc' to accomplish that.
> > * panel_simple_get_fixed_modes() was thought to be a confusing name,
> > so it has been renamed to panel_simple_get_display_modes().
> > * panel_simple_parse_override_mode() was thought to be better named as
> > panel_simple_parse_panel_timing_node().
> >
> > Suggested-by: Sam Ravnborg <sam@ravnborg.org>
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
>
> Thanks.
>
> I updated the $subject to:
> drm/panel: simple: document panel_desc; rename a few functions
>
> And pushed out to drm-misc-next.
I see the following error printed at each boot:
/panel: could not find node 'panel-timing'
The error originates from this snip (from panel-simple.c):
if (!of_get_display_timing(dev->of_node, "panel-timing", &dt))
panel_simple_parse_panel_timing_node(dev, panel, &dt);
of_get_display_timing() returns -2 (-ENOENT).
In the setup I use there is no panel-timing node as the timing specified
in panel-simple.c is fine.
This is the typical setup - and there should not in the normal case
be printed error messages like this during boot.
Will you please take a look at this.
Sam
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/panel: simple: Doxygenize 'struct panel_desc'; rename a few functions
2019-07-21 9:38 ` Sam Ravnborg
@ 2019-07-22 18:26 ` Doug Anderson
0 siblings, 0 replies; 4+ messages in thread
From: Doug Anderson @ 2019-07-22 18:26 UTC (permalink / raw)
To: Sam Ravnborg
Cc: David Airlie, LKML, dri-devel, Thierry Reding, Sean Paul,
Boris Brezillon
Hi,
On Sun, Jul 21, 2019 at 2:38 AM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Doug.
>
> On Wed, Jul 17, 2019 at 07:33:17PM +0200, Sam Ravnborg wrote:
> > Hi Doug.
> >
> > On Fri, Jul 12, 2019 at 09:33:33AM -0700, Douglas Anderson wrote:
> > > This attempts to address outstanding review feedback from commit
> > > b8a2948fa2b3 ("drm/panel: simple: Add ability to override typical
> > > timing"). Specifically:
> > >
> > > * It was requested that I document (in the structure definition) that
> > > the device tree override had no effect if 'struct drm_display_mode'
> > > was used in the panel description. I have provided full Doxygen
> > > comments for 'struct panel_desc' to accomplish that.
> > > * panel_simple_get_fixed_modes() was thought to be a confusing name,
> > > so it has been renamed to panel_simple_get_display_modes().
> > > * panel_simple_parse_override_mode() was thought to be better named as
> > > panel_simple_parse_panel_timing_node().
> > >
> > > Suggested-by: Sam Ravnborg <sam@ravnborg.org>
> > > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> >
> > Thanks.
> >
> > I updated the $subject to:
> > drm/panel: simple: document panel_desc; rename a few functions
> >
> > And pushed out to drm-misc-next.
>
> I see the following error printed at each boot:
>
> /panel: could not find node 'panel-timing'
>
> The error originates from this snip (from panel-simple.c):
>
> if (!of_get_display_timing(dev->of_node, "panel-timing", &dt))
> panel_simple_parse_panel_timing_node(dev, panel, &dt);
>
> of_get_display_timing() returns -2 (-ENOENT).
>
> In the setup I use there is no panel-timing node as the timing specified
> in panel-simple.c is fine.
> This is the typical setup - and there should not in the normal case
> be printed error messages like this during boot.
>
> Will you please take a look at this.
Breadcrumbs: series has been posted to address this. PTAL.
https://lkml.kernel.org/r/20190722182439.44844-1-dianders@chromium.org
-Doug
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-07-22 18:26 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-12 16:33 [PATCH] drm/panel: simple: Doxygenize 'struct panel_desc'; rename a few functions Douglas Anderson
2019-07-17 17:33 ` Sam Ravnborg
2019-07-21 9:38 ` Sam Ravnborg
2019-07-22 18:26 ` Doug Anderson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).