[PATCH 1/3] drm/panel: Add display_timing support

[PATCH 1/3] drm/panel: Add display_timing support

[Philipp Zabel]

Many panel data sheets additionally to typical values list allowed ranges for
timings such as hsync/vsync lengths, porches, and the pixel clock rate. These
can be stored in a struct display_timing, to be used by an encoder mode_fixup
callback to clamp user provided timing values or to validate workarounds for
clock source limitations.

This patch adds a new drm_panel_funcs callback that returns the panels'
available display_timing entries.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 include/drm/drm_panel.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index 1fbcc96..13ff44b 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -29,6 +29,7 @@
 struct drm_connector;
 struct drm_device;
 struct drm_panel;
+struct display_timing;
 
 /**
  * struct drm_panel_funcs - perform operations on a given panel
@@ -38,6 +39,8 @@ struct drm_panel;
  * @enable: enable panel (turn on back light, etc.)
  * @get_modes: add modes to the connector that the panel is attached to and
  * return the number of modes added
+ * @get_timings: copy display timings into the provided array and return
+ * the number of display timings available
  *
  * The .prepare() function is typically called before the display controller
  * starts to transmit video data. Panel drivers can use this to turn the panel
@@ -68,6 +71,8 @@ struct drm_panel_funcs {
 	int (*prepare)(struct drm_panel *panel);
 	int (*enable)(struct drm_panel *panel);
 	int (*get_modes)(struct drm_panel *panel);
+	int (*get_timings)(struct drm_panel *panel, unsigned int num_timings,
+			   struct display_timing *timings);
 };
 
 struct drm_panel {
-- 
2.1.3

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

[PATCH 2/3] drm/panel: Add display_timing support to simple panel driver

[Philipp Zabel]

Many panel data sheets additionally to typical values list allowed ranges for
timings such as hsync/vsync lengths, porches, and the pixel clock rate. This
patch allows simple panels to store a struct display_timing in place of the
struct drm_display_mode used before.
The get_modes panel_ops callback calculates the drm_display_mode list from
the typical timings and the get_timings callback returns the timings to
the connected encoder for mode fixup and validation.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/gpu/drm/panel/panel-simple.c | 40 ++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index c4b6167..895af09 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -33,9 +33,14 @@
 #include <drm/drm_mipi_dsi.h>
 #include <drm/drm_panel.h>
 
+#include <video/display_timing.h>
+#include <video/videomode.h>
+
 struct panel_desc {
 	const struct drm_display_mode *modes;
 	unsigned int num_modes;
+	const struct display_timing *timings;
+	unsigned int num_timings;
 
 	unsigned int bpc;
 
@@ -92,6 +97,25 @@ static int panel_simple_get_fixed_modes(struct panel_simple *panel)
 	if (!panel->desc)
 		return 0;
 
+	for (i = 0; i < panel->desc->num_timings; i++) {
+		const struct display_timing *dt = &panel->desc->timings[i];
+		struct videomode vm;
+
+		videomode_from_timing(dt, &vm);
+		mode = drm_mode_create(drm);
+		if (!mode) {
+			dev_err(drm->dev, "failed to add mode %ux%u\n",
+				dt->hactive.typ, dt->vactive.typ);
+			continue;
+		}
+
+		drm_display_mode_from_videomode(&vm, mode);
+		drm_mode_set_name(mode);
+
+		drm_mode_probed_add(connector, mode);
+		num++;
+	}
+
 	for (i = 0; i < panel->desc->num_modes; i++) {
 		const struct drm_display_mode *m = &panel->desc->modes[i];
 
@@ -221,12 +245,28 @@ static int panel_simple_get_modes(struct drm_panel *panel)
 	return num;
 }
 
+static int panel_simple_get_timings(struct drm_panel *panel,
+				    unsigned int num_timings,
+				    struct display_timing *timings)
+{
+	struct panel_simple *p = to_panel_simple(panel);
+	int i;
+
+	if (timings) {
+		for (i = 0; i < min(num_timings, p->desc->num_timings); i++)
+			timings[i] = p->desc->timings[i];
+	}
+
+	return p->desc->num_timings;
+}
+
 static const struct drm_panel_funcs panel_simple_funcs = {
 	.disable = panel_simple_disable,
 	.unprepare = panel_simple_unprepare,
 	.prepare = panel_simple_prepare,
 	.enable = panel_simple_enable,
 	.get_modes = panel_simple_get_modes,
+	.get_timings = panel_simple_get_timings,
 };
 
 static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
-- 
2.1.3

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

[PATCH 3/3] drm/panel: Add display_timing entry for the HannStar HSD070PWW1 panel

[Philipp Zabel]

The HannStar HSD070PWW1 LVDS panel data sheet lists allowed ranges additionally
to the typical values for pixel clock rate (64.3 MHz ... 82 MHz) and blanking
intervals (54 to 681 clocks horizontally, 3 to 23 lines vertically).
This patch replaces this panels' drm_display_mode entry with a display_timing
entry to describe acceptable timings.
Since the HSYNC/VSYNC are unused, the distribution between front porches, back
porches, and sync pulse lengths was chosen at will.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/gpu/drm/panel/panel-simple.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 895af09..47ecdca 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -600,22 +600,22 @@ static const struct panel_desc foxlink_fl500wvr00_a0t = {
 	},
 };
 
-static const struct drm_display_mode hannstar_hsd070pww1_mode = {
-	.clock = 71100,
-	.hdisplay = 1280,
-	.hsync_start = 1280 + 1,
-	.hsync_end = 1280 + 1 + 158,
-	.htotal = 1280 + 1 + 158 + 1,
-	.vdisplay = 800,
-	.vsync_start = 800 + 1,
-	.vsync_end = 800 + 1 + 21,
-	.vtotal = 800 + 1 + 21 + 1,
-	.vrefresh = 60,
+static const struct display_timing hannstar_hsd070pww1_timing = {
+	.pixelclock = { 64300000, 71100000, 82000000 },
+	.hactive = { 1280, 1280, 1280 },
+	.hfront_porch = { 1, 1, 10 },
+	.hback_porch = { 1, 1, 10 },
+	.hsync_len = { 52, 158, 661 },
+	.vactive = { 800, 800, 800 },
+	.vfront_porch = { 1, 1, 10 },
+	.vback_porch = { 1, 1, 10 },
+	.vsync_len = { 1, 21, 203 },
+	.flags = DISPLAY_FLAGS_DE_HIGH,
 };
 
 static const struct panel_desc hannstar_hsd070pww1 = {
-	.modes = &hannstar_hsd070pww1_mode,
-	.num_modes = 1,
+	.timings = &hannstar_hsd070pww1_timing,
+	.num_timings = 1,
 	.bpc = 6,
 	.size = {
 		.width = 151,
-- 
2.1.3

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

Re: [PATCH 1/3] drm/panel: Add display_timing support

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 2517 bytes --]

On Thu, Dec 11, 2014 at 06:32:44PM +0100, Philipp Zabel wrote:
> Many panel data sheets additionally to typical values list allowed ranges for
> timings such as hsync/vsync lengths, porches, and the pixel clock rate. These
> can be stored in a struct display_timing, to be used by an encoder mode_fixup
> callback to clamp user provided timing values or to validate workarounds for
> clock source limitations.
> 
> This patch adds a new drm_panel_funcs callback that returns the panels'
> available display_timing entries.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  include/drm/drm_panel.h | 5 +++++
>  1 file changed, 5 insertions(+)

Sorry for taking so long to get back on this. I generally like the idea,
though a couple of things are unclear to me.

In struct display_timing we have:

	struct timing_entry hactive;
	...
	struct timing_entry vactive;

I wonder if that really makes sense. Are there really datasheets that
provide a valid range for the number of horizontal and vertical active
pixels?

> 
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index 1fbcc96..13ff44b 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -29,6 +29,7 @@
>  struct drm_connector;
>  struct drm_device;
>  struct drm_panel;
> +struct display_timing;
>  
>  /**
>   * struct drm_panel_funcs - perform operations on a given panel
> @@ -38,6 +39,8 @@ struct drm_panel;
>   * @enable: enable panel (turn on back light, etc.)
>   * @get_modes: add modes to the connector that the panel is attached to and
>   * return the number of modes added
> + * @get_timings: copy display timings into the provided array and return
> + * the number of display timings available
>   *
>   * The .prepare() function is typically called before the display controller
>   * starts to transmit video data. Panel drivers can use this to turn the panel
> @@ -68,6 +71,8 @@ struct drm_panel_funcs {
>  	int (*prepare)(struct drm_panel *panel);
>  	int (*enable)(struct drm_panel *panel);
>  	int (*get_modes)(struct drm_panel *panel);
> +	int (*get_timings)(struct drm_panel *panel, unsigned int num_timings,
> +			   struct display_timing *timings);

Also are there even panels that support more than one set of timings?
I've never heard of panels that are actually able to do more than one
mode, so I'm wondering if num_timings isn't overdoing it a little here.
I guess it doesn't hurt all that much, though.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 1/3] drm/panel: Add display_timing support

[Philipp Zabel]

Hi Thierry,

Am Dienstag, den 03.02.2015, 14:30 +0100 schrieb Thierry Reding:
> On Thu, Dec 11, 2014 at 06:32:44PM +0100, Philipp Zabel wrote:
> > Many panel data sheets additionally to typical values list allowed ranges for
> > timings such as hsync/vsync lengths, porches, and the pixel clock rate. These
> > can be stored in a struct display_timing, to be used by an encoder mode_fixup
> > callback to clamp user provided timing values or to validate workarounds for
> > clock source limitations.
> > 
> > This patch adds a new drm_panel_funcs callback that returns the panels'
> > available display_timing entries.
> > 
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > ---
> >  include/drm/drm_panel.h | 5 +++++
> >  1 file changed, 5 insertions(+)
> 
> Sorry for taking so long to get back on this. I generally like the idea,
> though a couple of things are unclear to me.
> 
> In struct display_timing we have:
> 
> 	struct timing_entry hactive;
> 	...
> 	struct timing_entry vactive;
> 
> I wonder if that really makes sense. Are there really datasheets that
> provide a valid range for the number of horizontal and vertical active
> pixels?

I often see datasheets that give minimum, typical and maximum values
also for the horizontal and vertical active pixels, but those are
usually the same value. I don't know if there are any panels that really
have documented ranges here. 

[...]
> > @@ -68,6 +71,8 @@ struct drm_panel_funcs {
> >  	int (*prepare)(struct drm_panel *panel);
> >  	int (*enable)(struct drm_panel *panel);
> >  	int (*get_modes)(struct drm_panel *panel);
> > +	int (*get_timings)(struct drm_panel *panel, unsigned int num_timings,
> > +			   struct display_timing *timings);
> 
> Also are there even panels that support more than one set of timings?
> I've never heard of panels that are actually able to do more than one
> mode, so I'm wondering if num_timings isn't overdoing it a little here.
> I guess it doesn't hurt all that much, though.

I have not seen any yet that reasonably should be driven by the
simple-panel driver. I was thinking about the Pixel Qi panel, but it
turns out those just have a single timing and use the subpixels in B/W
reflective mode.

regards
Philipp

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

Re: [PATCH 1/3] drm/panel: Add display_timing support

[Philipp Zabel]

Hi Thierry,

do you have any further thoughts on this?

Am Dienstag, den 03.02.2015, 14:30 +0100 schrieb Thierry Reding:
> On Thu, Dec 11, 2014 at 06:32:44PM +0100, Philipp Zabel wrote:
> > Many panel data sheets additionally to typical values list allowed ranges for
> > timings such as hsync/vsync lengths, porches, and the pixel clock rate. These
> > can be stored in a struct display_timing, to be used by an encoder mode_fixup
> > callback to clamp user provided timing values or to validate workarounds for
> > clock source limitations.
> > 
> > This patch adds a new drm_panel_funcs callback that returns the panels'
> > available display_timing entries.
> > 
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > ---
> >  include/drm/drm_panel.h | 5 +++++
> >  1 file changed, 5 insertions(+)
> 
> Sorry for taking so long to get back on this. I generally like the idea,
> though a couple of things are unclear to me.
> 
> In struct display_timing we have:
> 
> 	struct timing_entry hactive;
> 	...
> 	struct timing_entry vactive;
> 
> I wonder if that really makes sense. Are there really datasheets that
> provide a valid range for the number of horizontal and vertical active
> pixels?

I could send a patch to change hactive/vactive to a single value
and change Documentation/devicetree/bindings/video/display-timing.txt
to clarify ranges are not allowed for these properties until somebody
digs out a panel that actually needs this.
Adding Steffen to Cc: in case there was a reason other than symmetry.

> > diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> > index 1fbcc96..13ff44b 100644
> > --- a/include/drm/drm_panel.h
> > +++ b/include/drm/drm_panel.h
> > @@ -29,6 +29,7 @@
> >  struct drm_connector;
> >  struct drm_device;
> >  struct drm_panel;
> > +struct display_timing;
> >  
> >  /**
> >   * struct drm_panel_funcs - perform operations on a given panel
> > @@ -38,6 +39,8 @@ struct drm_panel;
> >   * @enable: enable panel (turn on back light, etc.)
> >   * @get_modes: add modes to the connector that the panel is attached to and
> >   * return the number of modes added
> > + * @get_timings: copy display timings into the provided array and return
> > + * the number of display timings available
> >   *
> >   * The .prepare() function is typically called before the display controller
> >   * starts to transmit video data. Panel drivers can use this to turn the panel
> > @@ -68,6 +71,8 @@ struct drm_panel_funcs {
> >  	int (*prepare)(struct drm_panel *panel);
> >  	int (*enable)(struct drm_panel *panel);
> >  	int (*get_modes)(struct drm_panel *panel);
> > +	int (*get_timings)(struct drm_panel *panel, unsigned int num_timings,
> > +			   struct display_timing *timings);
> 
> Also are there even panels that support more than one set of timings?
> I've never heard of panels that are actually able to do more than one
> mode, so I'm wondering if num_timings isn't overdoing it a little here.
> I guess it doesn't hurt all that much, though.

Would you prefer
	struct display_timing *(*get_timing)(struct drm_panel *panel);
?

regards
Philipp

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

Re: [PATCH 1/3] drm/panel: Add display_timing support

[Boris Brezillon]

Hi Philipp,

On Mon, 23 Feb 2015 15:04:32 +0100
Philipp Zabel <p.zabel@pengutronix.de> wrote:

> Hi Thierry,
> 
> do you have any further thoughts on this?
> 
> Am Dienstag, den 03.02.2015, 14:30 +0100 schrieb Thierry Reding:
> > On Thu, Dec 11, 2014 at 06:32:44PM +0100, Philipp Zabel wrote:
> > > Many panel data sheets additionally to typical values list allowed ranges for
> > > timings such as hsync/vsync lengths, porches, and the pixel clock rate. These
> > > can be stored in a struct display_timing, to be used by an encoder mode_fixup
> > > callback to clamp user provided timing values or to validate workarounds for
> > > clock source limitations.
> > > 
> > > This patch adds a new drm_panel_funcs callback that returns the panels'
> > > available display_timing entries.

Anthony recently noticed that the edt_etm0700g0dh6 panel he's
connecting on his sama5 board defines typical timings that are not
supported by Atmel's display controller.
This patch could help us deal with such cases.
Could you keep me (and Anthony) in Cc of your future versions ?

Thanks,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/panel: Add display_timing support

[Philipp Zabel]

Hi Boris,

Am Donnerstag, den 26.02.2015, 14:51 +0100 schrieb Boris Brezillon:
> Hi Philipp,
> 
> On Mon, 23 Feb 2015 15:04:32 +0100
> Philipp Zabel <p.zabel@pengutronix.de> wrote:
> 
> > Hi Thierry,
> > 
> > do you have any further thoughts on this?
> > 
> > Am Dienstag, den 03.02.2015, 14:30 +0100 schrieb Thierry Reding:
> > > On Thu, Dec 11, 2014 at 06:32:44PM +0100, Philipp Zabel wrote:
> > > > Many panel data sheets additionally to typical values list allowed ranges for
> > > > timings such as hsync/vsync lengths, porches, and the pixel clock rate. These
> > > > can be stored in a struct display_timing, to be used by an encoder mode_fixup
> > > > callback to clamp user provided timing values or to validate workarounds for
> > > > clock source limitations.
> > > > 
> > > > This patch adds a new drm_panel_funcs callback that returns the panels'
> > > > available display_timing entries.
> 
> Anthony recently noticed that the edt_etm0700g0dh6 panel he's
> connecting on his sama5 board defines typical timings that are not
> supported by Atmel's display controller.
> This patch could help us deal with such cases.
> Could you keep me (and Anthony) in Cc of your future versions ?

Will do.

regards
Philipp

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

Re: [PATCH 1/3] drm/panel: Add display_timing support

[Philipp Zabel]

Hi Thierry,

Am Montag, den 23.02.2015, 15:04 +0100 schrieb Philipp Zabel:
> Hi Thierry,
> 
> do you have any further thoughts on this?

[...]
> > Sorry for taking so long to get back on this. I generally like the idea,
> > though a couple of things are unclear to me.
> > 
> > In struct display_timing we have:
> > 
> > 	struct timing_entry hactive;
> > 	...
> > 	struct timing_entry vactive;
> > 
> > I wonder if that really makes sense. Are there really datasheets that
> > provide a valid range for the number of horizontal and vertical active
> > pixels?
> 
> I could send a patch to change hactive/vactive to a single value
> and change Documentation/devicetree/bindings/video/display-timing.txt
> to clarify ranges are not allowed for these properties until somebody
> digs out a panel that actually needs this.
> Adding Steffen to Cc: in case there was a reason other than symmetry.

That seems not to be the case so far.

[...]
> > >  /**
> > >   * struct drm_panel_funcs - perform operations on a given panel
> > > @@ -38,6 +39,8 @@ struct drm_panel;
> > >   * @enable: enable panel (turn on back light, etc.)
> > >   * @get_modes: add modes to the connector that the panel is attached to and
> > >   * return the number of modes added
> > > + * @get_timings: copy display timings into the provided array and return
> > > + * the number of display timings available
> > >   *
> > >   * The .prepare() function is typically called before the display controller
> > >   * starts to transmit video data. Panel drivers can use this to turn the panel
> > > @@ -68,6 +71,8 @@ struct drm_panel_funcs {
> > >  	int (*prepare)(struct drm_panel *panel);
> > >  	int (*enable)(struct drm_panel *panel);
> > >  	int (*get_modes)(struct drm_panel *panel);
> > > +	int (*get_timings)(struct drm_panel *panel, unsigned int num_timings,
> > > +			   struct display_timing *timings);
> > 
> > Also are there even panels that support more than one set of timings?
> > I've never heard of panels that are actually able to do more than one
> > mode, so I'm wondering if num_timings isn't overdoing it a little here.
> > I guess it doesn't hurt all that much, though.
> 
> Would you prefer
> 	struct display_timing *(*get_timing)(struct drm_panel *panel);
> ?

I'd like to resend this. Please let me know if you want me to change
this function prototype.

regards
Philipp

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

Re: [PATCH 1/3] drm/panel: Add display_timing support

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 2877 bytes --]

On Tue, Mar 03, 2015 at 12:49:43PM +0100, Philipp Zabel wrote:
> Hi Thierry,
> 
> Am Montag, den 23.02.2015, 15:04 +0100 schrieb Philipp Zabel:
> > Hi Thierry,
> > 
> > do you have any further thoughts on this?
> 
> [...]
> > > Sorry for taking so long to get back on this. I generally like the idea,
> > > though a couple of things are unclear to me.
> > > 
> > > In struct display_timing we have:
> > > 
> > > 	struct timing_entry hactive;
> > > 	...
> > > 	struct timing_entry vactive;
> > > 
> > > I wonder if that really makes sense. Are there really datasheets that
> > > provide a valid range for the number of horizontal and vertical active
> > > pixels?
> > 
> > I could send a patch to change hactive/vactive to a single value
> > and change Documentation/devicetree/bindings/video/display-timing.txt
> > to clarify ranges are not allowed for these properties until somebody
> > digs out a panel that actually needs this.
> > Adding Steffen to Cc: in case there was a reason other than symmetry.
> 
> That seems not to be the case so far.
> 
> [...]
> > > >  /**
> > > >   * struct drm_panel_funcs - perform operations on a given panel
> > > > @@ -38,6 +39,8 @@ struct drm_panel;
> > > >   * @enable: enable panel (turn on back light, etc.)
> > > >   * @get_modes: add modes to the connector that the panel is attached to and
> > > >   * return the number of modes added
> > > > + * @get_timings: copy display timings into the provided array and return
> > > > + * the number of display timings available
> > > >   *
> > > >   * The .prepare() function is typically called before the display controller
> > > >   * starts to transmit video data. Panel drivers can use this to turn the panel
> > > > @@ -68,6 +71,8 @@ struct drm_panel_funcs {
> > > >  	int (*prepare)(struct drm_panel *panel);
> > > >  	int (*enable)(struct drm_panel *panel);
> > > >  	int (*get_modes)(struct drm_panel *panel);
> > > > +	int (*get_timings)(struct drm_panel *panel, unsigned int num_timings,
> > > > +			   struct display_timing *timings);
> > > 
> > > Also are there even panels that support more than one set of timings?
> > > I've never heard of panels that are actually able to do more than one
> > > mode, so I'm wondering if num_timings isn't overdoing it a little here.
> > > I guess it doesn't hurt all that much, though.
> > 
> > Would you prefer
> > 	struct display_timing *(*get_timing)(struct drm_panel *panel);
> > ?
> 
> I'd like to resend this. Please let me know if you want me to change
> this function prototype.

I have no objections to keeping the current prototype. It's something we
can always fixup if we want to. Also keeping the symmetry with min/max
values for hactive and vactive is okay in my opinion.

Were there any other remaining points? If not I'll just apply this as
is.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 1/3] drm/panel: Add display_timing support

[Philipp Zabel]

Am Dienstag, den 24.03.2015, 12:34 +0100 schrieb Thierry Reding:
[...]
> > > Would you prefer
> > > 	struct display_timing *(*get_timing)(struct drm_panel *panel);
> > > ?
> > 
> > I'd like to resend this. Please let me know if you want me to change
> > this function prototype.
> 
> I have no objections to keeping the current prototype. It's something we
> can always fixup if we want to. Also keeping the symmetry with min/max
> values for hactive and vactive is okay in my opinion.
> 
> Were there any other remaining points? If not I'll just apply this as
> is.

No, I'm happy if you apply this as is.

thanks
Philipp

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

Re: [PATCH 1/3] drm/panel: Add display_timing support

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1254 bytes --]

On Tue, Mar 24, 2015 at 12:52:44PM +0100, Philipp Zabel wrote:
> Am Dienstag, den 24.03.2015, 12:34 +0100 schrieb Thierry Reding:
> [...]
> > > > Would you prefer
> > > > 	struct display_timing *(*get_timing)(struct drm_panel *panel);
> > > > ?
> > > 
> > > I'd like to resend this. Please let me know if you want me to change
> > > this function prototype.
> > 
> > I have no objections to keeping the current prototype. It's something we
> > can always fixup if we want to. Also keeping the symmetry with min/max
> > values for hactive and vactive is okay in my opinion.
> > 
> > Were there any other remaining points? If not I'll just apply this as
> > is.
> 
> No, I'm happy if you apply this as is.

Done. I touched up a couple of things in the commit messages and made a
minor change to the loop which copies the timings in the simple-panel
driver (extract the min(num_timings, p->desc->num_timings) out of the
for statement). Nothing major, but it might still be good if you could
test, just to make sure I didn't make a mess.

This should be in tomorrow's linux-next, but if you want to take a peek
before that, you can grab it from here:

	git://anongit.freedesktop.org/tegra/linux.git#drm/panel/for-next

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 1/3] drm/panel: Add display_timing support

[Philipp Zabel]

Hi Thierry,

Am Dienstag, den 24.03.2015, 13:40 +0100 schrieb Thierry Reding:
> On Tue, Mar 24, 2015 at 12:52:44PM +0100, Philipp Zabel wrote:
> > Am Dienstag, den 24.03.2015, 12:34 +0100 schrieb Thierry Reding:
> > [...]
> > > > > Would you prefer
> > > > > 	struct display_timing *(*get_timing)(struct drm_panel *panel);
> > > > > ?
> > > > 
> > > > I'd like to resend this. Please let me know if you want me to change
> > > > this function prototype.
> > > 
> > > I have no objections to keeping the current prototype. It's something we
> > > can always fixup if we want to. Also keeping the symmetry with min/max
> > > values for hactive and vactive is okay in my opinion.
> > > 
> > > Were there any other remaining points? If not I'll just apply this as
> > > is.
> > 
> > No, I'm happy if you apply this as is.
> 
> Done. I touched up a couple of things in the commit messages and made a
> minor change to the loop which copies the timings in the simple-panel
> driver (extract the min(num_timings, p->desc->num_timings) out of the
> for statement). Nothing major, but it might still be good if you could
> test, just to make sure I didn't make a mess.

Thanks for the cleanup, it still seems to work well.

regards
Philipp

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