[PATCH 00/23] Nokia N950 display support

[PATCH 00/23] Nokia N950 display support

[Sebastian Reichel]

Hi,

This series adds support for the Nokia N950 display.
Since the panel is using DSI command mode, it involves
adding support for manually updated displays to
omapdrm.

The following patches, that are based on 4.5-rc7
were tested with fbcon and Xorg (from Debian sid)
on the N950.

-- Sebastian

Sebastian Reichel (22):
  ARM: dts: n9/n950: regulator configuration
  ARM: dts: n950: add display support
  drm: omapdrm: add DSI mapping
  Revert "drm: omapdrm: Remove manual update display support"
  drm: omapdrm: wait for pending operations before updating plane
  drm: omapdrm: crtc: switch pending variable to atomic bitset
  drm: omapdrm: crtc: add enabled bit to state
  drm: omapdrm: dss: method to get stallmode from lcd config
  drm: omapdrm: crtc: detect manually updated displays
  include: video: omapdss: provide fifo threshold methods
  drm: omapdrm: plane: update fifo size on atomic update
  drm: omapdrm: crtc: update plane fifos on lcd config change
  drm: omapdrm: crtc: save framedone callback from dss
  drm: omapdrm: crtc: add support for manual updated displays
  drm: omapdrm: update manual displays on dirty ioctl
  drm: omapdrm: panel-dsi-cm: add regulator support
  drm: omapdrm: panel-dsi-cm: use threaded irq handler
  drm: omapdrm: panel-dsi-cm: improve DT support
  drm: omapdrm: panel-dsi-cm: add offset support
  drm: omapdrm: panel-dsi-cm: block disable until update completed
  drm: omapdrm: panel-dsi-cm: ratelimit debug output in update path
  drm: omapdrm: panel-dsi-cm: provide timings methods for omapdrm

Tomi Valkeinen (1):
  drm: omapdrm: dss: reset dsi module during initialization

 arch/arm/boot/dts/omap3-n950-n9.dtsi            |  72 +++++++++
 arch/arm/boot/dts/omap3-n950.dts                |  71 +++++++++
 drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c | 163 ++++++++++++++++++---
 drivers/gpu/drm/omapdrm/dss/dsi.c               |  24 +++
 drivers/gpu/drm/omapdrm/dss/output.c            |   6 +
 drivers/gpu/drm/omapdrm/omap_crtc.c             | 185 +++++++++++++++++++++---
 drivers/gpu/drm/omapdrm/omap_drv.c              |   6 +
 drivers/gpu/drm/omapdrm/omap_drv.h              |   6 +
 drivers/gpu/drm/omapdrm/omap_fb.c               |  38 +++++
 drivers/gpu/drm/omapdrm/omap_fbdev.c            |  57 +++++++-
 drivers/gpu/drm/omapdrm/omap_plane.c            |  23 +++
 include/video/omapdss.h                         |   7 +
 12 files changed, 616 insertions(+), 42 deletions(-)

-- 
2.7.0

[PATCH 01/23] ARM: dts: n9/n950: regulator configuration

[Sebastian Reichel]

Add regulator configuration as found in the
board files of Nokia's kernel.

Signed-off-By: Sebastian Reichel <sre@kernel.org>
---
 arch/arm/boot/dts/omap3-n950-n9.dtsi | 72 ++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi b/arch/arm/boot/dts/omap3-n950-n9.dtsi
index a2c2b8d8dd2c..3c7f1d2deb2f 100644
--- a/arch/arm/boot/dts/omap3-n950-n9.dtsi
+++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi
@@ -72,6 +72,30 @@
 	ti,pulldowns	= <0x008106>; /* BIT(1) | BIT(2) | BIT(8) | BIT(15) */
 };
 
+&vdac {
+	regulator-name = "vdac";
+	regulator-min-microvolt = <1800000>;
+	regulator-max-microvolt = <1800000>;
+};
+
+&vpll1 {
+	regulator-name = "vpll1";
+	regulator-min-microvolt = <1800000>;
+	regulator-max-microvolt = <1800000>;
+};
+
+&vpll2 {
+	regulator-name = "vpll2";
+	regulator-min-microvolt = <1800000>;
+	regulator-max-microvolt = <1800000>;
+};
+
+&vaux1 {
+	regulator-name = "vaux1";
+	regulator-min-microvolt = <2800000>;
+	regulator-max-microvolt = <2800000>;
+};
+
 /* CSI-2 receiver */
 &vaux2 {
 	regulator-name = "vaux2";
@@ -86,6 +110,54 @@
 	regulator-max-microvolt = <2800000>;
 };
 
+&vaux4 {
+	regulator-name = "vaux4";
+	regulator-min-microvolt = <2800000>;
+	regulator-max-microvolt = <2800000>;
+};
+
+&vmmc1 {
+	regulator-name = "vmmc1";
+	regulator-min-microvolt = <1850000>;
+	regulator-max-microvolt = <3150000>;
+};
+
+&vmmc2 {
+	regulator-name = "vmmc2";
+	regulator-min-microvolt = <3000000>;
+	regulator-max-microvolt = <3000000>;
+};
+
+&vintana1 {
+	regulator-name = "vintana1";
+	regulator-min-microvolt = <1500000>;
+	regulator-max-microvolt = <1500000>;
+};
+
+&vintana2 {
+	regulator-name = "vintana2";
+	regulator-min-microvolt = <2750000>;
+	regulator-max-microvolt = <2750000>;
+};
+
+&vintdig {
+	regulator-name = "vintdig";
+	regulator-min-microvolt = <1500000>;
+	regulator-max-microvolt = <1500000>;
+};
+
+&vsim {
+	regulator-name = "vsim";
+	regulator-min-microvolt = <1800000>;
+	regulator-max-microvolt = <1800000>;
+};
+
+&vio {
+	regulator-name = "vio";
+	regulator-min-microvolt = <1800000>;
+	regulator-max-microvolt = <1800000>;
+};
+
 &i2c2 {
 	clock-frequency = <400000>;
 };
-- 
2.7.0

[PATCH 02/23] ARM: dts: n950: add display support

[Sebastian Reichel]

Signed-off-By: Sebastian Reichel <sre@kernel.org>
---
 arch/arm/boot/dts/omap3-n950.dts | 71 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/arch/arm/boot/dts/omap3-n950.dts b/arch/arm/boot/dts/omap3-n950.dts
index 0885b34d5d7d..41b8fb585272 100644
--- a/arch/arm/boot/dts/omap3-n950.dts
+++ b/arch/arm/boot/dts/omap3-n950.dts
@@ -17,6 +17,26 @@
 	compatible = "nokia,omap3-n950", "ti,omap36xx", "ti,omap3";
 };
 
+&omap3_pmx_core {
+	dsi_pins: pinmux_dsi_pins {
+		pinctrl-single,pins = <
+			OMAP3_CORE1_IOPAD(0x20dc, PIN_OUTPUT | MUX_MODE1) /* dsi_dx0 - data0+ */
+			OMAP3_CORE1_IOPAD(0x20de, PIN_OUTPUT | MUX_MODE1) /* dsi_dy0 - data0- */
+			OMAP3_CORE1_IOPAD(0x20e0, PIN_OUTPUT | MUX_MODE1) /* dsi_dx1 - clk+   */
+			OMAP3_CORE1_IOPAD(0x20e2, PIN_OUTPUT | MUX_MODE1) /* dsi_dy1 - clk-   */
+			OMAP3_CORE1_IOPAD(0x20e4, PIN_OUTPUT | MUX_MODE1) /* dsi_dx2 - data1+ */
+			OMAP3_CORE1_IOPAD(0x20e6, PIN_OUTPUT | MUX_MODE1) /* dsi_dy2 - data1- */
+		>;
+	};
+
+	display_pins: pinmux_display_pins {
+		pinctrl-single,pins = <
+			OMAP3_CORE1_IOPAD(0x20ca, PIN_INPUT | MUX_MODE4) /* gpio 62 - display te */
+			OMAP3_CORE1_IOPAD(0x20fe, PIN_OUTPUT | MUX_MODE4) /* gpio 87 - display reset */
+		>;
+	};
+};
+
 &i2c2 {
 	smia_1: camera@10 {
 		compatible = "nokia,smia";
@@ -53,3 +73,54 @@
 		};
 	};
 };
+
+&dss {
+	status = "ok";
+
+	vdda_video-supply = <&vdac>;
+};
+
+&dsi {
+	status = "ok";
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&dsi_pins>;
+
+	vdd-supply = <&vpll2>;
+
+	port {
+		dsi_out_ep: endpoint {
+			remote-endpoint = <&lcd0_in>;
+			lanes = <2 3 0 1 4 5>;
+		};
+	};
+
+	lcd0: display {
+		compatible = "nokia,himalaya", "panel-dsi-cm";
+		label = "lcd0";
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&display_pins>;
+
+		vpnl-supply = <&vmmc2>;
+		vddi-supply = <&vio>;
+
+		reset-gpios = <&gpio3 23 GPIO_ACTIVE_HIGH>;	/* 87 */
+		te-gpios = <&gpio2 30 GPIO_ACTIVE_HIGH>;	/* 62 */
+
+		has-dsi-backlight;
+
+		/* panel is 480x464 with top and bottom 5 lines not visible */
+		/* physical dimensions: 48960µm x 88128µm */
+		resolution-x = <480>;
+		resolution-y = <854>;
+		offset-x = <0>;
+		offset-y = <5>;
+
+		port {
+			lcd0_in: endpoint {
+				remote-endpoint = <&dsi_out_ep>;
+			};
+		};
+	};
+};
-- 
2.7.0

Re: [PATCH 02/23] ARM: dts: n950: add display support

[Laurent Pinchart]

Hi Sebastian,

Thank you for the patch.

On Tuesday 08 March 2016 17:39:34 Sebastian Reichel wrote:
> Signed-off-By: Sebastian Reichel <sre@kernel.org>
> ---
>  arch/arm/boot/dts/omap3-n950.dts | 71 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/omap3-n950.dts
> b/arch/arm/boot/dts/omap3-n950.dts index 0885b34d5d7d..41b8fb585272 100644
> --- a/arch/arm/boot/dts/omap3-n950.dts
> +++ b/arch/arm/boot/dts/omap3-n950.dts
> @@ -17,6 +17,26 @@
>  	compatible = "nokia,omap3-n950", "ti,omap36xx", "ti,omap3";
>  };
> 
> +&omap3_pmx_core {
> +	dsi_pins: pinmux_dsi_pins {
> +		pinctrl-single,pins = <
> +			OMAP3_CORE1_IOPAD(0x20dc, PIN_OUTPUT | MUX_MODE1) /* dsi_dx0 -
> data0+ */
> +			OMAP3_CORE1_IOPAD(0x20de, PIN_OUTPUT | MUX_MODE1) /* dsi_dy0 -
> data0- */
> +			OMAP3_CORE1_IOPAD(0x20e0, PIN_OUTPUT | MUX_MODE1) /* dsi_dx1 -
> clk+   */
> +			OMAP3_CORE1_IOPAD(0x20e2, PIN_OUTPUT | MUX_MODE1) /* dsi_dy1 -
> clk-   */
> +			OMAP3_CORE1_IOPAD(0x20e4, PIN_OUTPUT | MUX_MODE1) /* dsi_dx2 -
> data1+ */
> +			OMAP3_CORE1_IOPAD(0x20e6, PIN_OUTPUT | MUX_MODE1) /* dsi_dy2 -
> data1- */
> +		>;
> +	};
> +
> +	display_pins: pinmux_display_pins {
> +		pinctrl-single,pins = <
> +			OMAP3_CORE1_IOPAD(0x20ca, PIN_INPUT | MUX_MODE4) /* gpio 62 -
> display te */
> +			OMAP3_CORE1_IOPAD(0x20fe, PIN_OUTPUT | MUX_MODE4) /* gpio 87 -
> display reset */
> +		>;
> +	};
> +};
> +
>  &i2c2 {
>  	smia_1: camera@10 {
>  		compatible = "nokia,smia";
> @@ -53,3 +73,54 @@
>  		};
>  	};
>  };
> +
> +&dss {
> +	status = "ok";
> +
> +	vdda_video-supply = <&vdac>;
> +};
> +
> +&dsi {
> +	status = "ok";
> +
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&dsi_pins>;
> +
> +	vdd-supply = <&vpll2>;
> +
> +	port {
> +		dsi_out_ep: endpoint {
> +			remote-endpoint = <&lcd0_in>;
> +			lanes = <2 3 0 1 4 5>;
> +		};
> +	};
> +
> +	lcd0: display {
> +		compatible = "nokia,himalaya", "panel-dsi-cm";
> +		label = "lcd0";
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&display_pins>;
> +
> +		vpnl-supply = <&vmmc2>;
> +		vddi-supply = <&vio>;
> +
> +		reset-gpios = <&gpio3 23 GPIO_ACTIVE_HIGH>;	/* 87 */
> +		te-gpios = <&gpio2 30 GPIO_ACTIVE_HIGH>;	/* 62 */
> +
> +		has-dsi-backlight;
> +
> +		/* panel is 480x464 with top and bottom 5 lines not visible */

I assume you mean 480x864 ?

> +		/* physical dimensions: 48960µm x 88128µm */
> +		resolution-x = <480>;
> +		resolution-y = <854>;
> +		offset-x = <0>;
> +		offset-y = <5>;
> +
> +		port {
> +			lcd0_in: endpoint {
> +				remote-endpoint = <&dsi_out_ep>;
> +			};
> +		};
> +	};
> +};

-- 
Regards,

Laurent Pinchart

Re: [PATCH 02/23] ARM: dts: n950: add display support

[Laurent Pinchart]

Hi Sebastian,

Thank you for the patch.

On Tuesday 08 March 2016 17:39:34 Sebastian Reichel wrote:
> Signed-off-By: Sebastian Reichel <sre@kernel.org>
> ---
>  arch/arm/boot/dts/omap3-n950.dts | 71 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/omap3-n950.dts
> b/arch/arm/boot/dts/omap3-n950.dts index 0885b34d5d7d..41b8fb585272 100644
> --- a/arch/arm/boot/dts/omap3-n950.dts
> +++ b/arch/arm/boot/dts/omap3-n950.dts
> @@ -17,6 +17,26 @@
>  	compatible = "nokia,omap3-n950", "ti,omap36xx", "ti,omap3";
>  };
> 
> +&omap3_pmx_core {
> +	dsi_pins: pinmux_dsi_pins {
> +		pinctrl-single,pins = <
> +			OMAP3_CORE1_IOPAD(0x20dc, PIN_OUTPUT | MUX_MODE1) /* dsi_dx0 -
> data0+ */
> +			OMAP3_CORE1_IOPAD(0x20de, PIN_OUTPUT | MUX_MODE1) /* dsi_dy0 -
> data0- */
> +			OMAP3_CORE1_IOPAD(0x20e0, PIN_OUTPUT | MUX_MODE1) /* dsi_dx1 -
> clk+   */
> +			OMAP3_CORE1_IOPAD(0x20e2, PIN_OUTPUT | MUX_MODE1) /* dsi_dy1 -
> clk-   */
> +			OMAP3_CORE1_IOPAD(0x20e4, PIN_OUTPUT | MUX_MODE1) /* dsi_dx2 -
> data1+ */
> +			OMAP3_CORE1_IOPAD(0x20e6, PIN_OUTPUT | MUX_MODE1) /* dsi_dy2 -
> data1- */
> +		>;
> +	};
> +
> +	display_pins: pinmux_display_pins {
> +		pinctrl-single,pins = <
> +			OMAP3_CORE1_IOPAD(0x20ca, PIN_INPUT | MUX_MODE4) /* gpio 62 -
> display te */
> +			OMAP3_CORE1_IOPAD(0x20fe, PIN_OUTPUT | MUX_MODE4) /* gpio 87 -
> display reset */
> +		>;
> +	};
> +};
> +
>  &i2c2 {
>  	smia_1: camera@10 {
>  		compatible = "nokia,smia";
> @@ -53,3 +73,54 @@
>  		};
>  	};
>  };
> +
> +&dss {
> +	status = "ok";
> +
> +	vdda_video-supply = <&vdac>;
> +};
> +
> +&dsi {
> +	status = "ok";
> +
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&dsi_pins>;
> +
> +	vdd-supply = <&vpll2>;
> +
> +	port {
> +		dsi_out_ep: endpoint {
> +			remote-endpoint = <&lcd0_in>;
> +			lanes = <2 3 0 1 4 5>;
> +		};
> +	};
> +
> +	lcd0: display {
> +		compatible = "nokia,himalaya", "panel-dsi-cm";
> +		label = "lcd0";
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&display_pins>;
> +
> +		vpnl-supply = <&vmmc2>;
> +		vddi-supply = <&vio>;
> +
> +		reset-gpios = <&gpio3 23 GPIO_ACTIVE_HIGH>;	/* 87 */
> +		te-gpios = <&gpio2 30 GPIO_ACTIVE_HIGH>;	/* 62 */
> +
> +		has-dsi-backlight;
> +
> +		/* panel is 480x464 with top and bottom 5 lines not visible */

I assume you mean 480x864 ?

> +		/* physical dimensions: 48960µm x 88128µm */
> +		resolution-x = <480>;
> +		resolution-y = <854>;
> +		offset-x = <0>;
> +		offset-y = <5>;
> +
> +		port {
> +			lcd0_in: endpoint {
> +				remote-endpoint = <&dsi_out_ep>;
> +			};
> +		};
> +	};
> +};

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 02/23] ARM: dts: n950: add display support

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 476 bytes --]

Hi Laurent,

On Thu, Mar 17, 2016 at 02:14:26PM +0200, Laurent Pinchart wrote:
> [...]
> > +
> > +		/* panel is 480x464 with top and bottom 5 lines not visible */
> 
> I assume you mean 480x864 ?

Yes, nice catch. Basically the screen is 480x864, but only
480x854 are visible.

> > +		/* physical dimensions: 48960µm x 88128µm */
> > +		resolution-x = <480>;
> > +		resolution-y = <854>;
> > +		offset-x = <0>;
> > +		offset-y = <5>;
> [...]

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 02/23] ARM: dts: n950: add display support

[Jani Nikula]

On Thu, 17 Mar 2016, Sebastian Reichel <sre@kernel.org> wrote:
> On Thu, Mar 17, 2016 at 02:14:26PM +0200, Laurent Pinchart wrote:
>> [...]
>> > +
>> > +		/* panel is 480x464 with top and bottom 5 lines not visible */
>> 
>> I assume you mean 480x864 ?
>
> Yes, nice catch. Basically the screen is 480x864, but only
> 480x854 are visible.

It's been a while, but I thought the full 480x864 was actually usable
and visible.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH 02/23] ARM: dts: n950: add display support

[Jani Nikula]

On Thu, 17 Mar 2016, Sebastian Reichel <sre@kernel.org> wrote:
> On Thu, Mar 17, 2016 at 02:14:26PM +0200, Laurent Pinchart wrote:
>> [...]
>> > +
>> > +		/* panel is 480x464 with top and bottom 5 lines not visible */
>> 
>> I assume you mean 480x864 ?
>
> Yes, nice catch. Basically the screen is 480x864, but only
> 480x854 are visible.

It's been a while, but I thought the full 480x864 was actually usable
and visible.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/23] ARM: dts: n950: add display support

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 803 bytes --]

Hi,

On Wed, Mar 23, 2016 at 02:40:53PM +0200, Jani Nikula wrote:
> On Thu, 17 Mar 2016, Sebastian Reichel <sre@kernel.org> wrote:
> > On Thu, Mar 17, 2016 at 02:14:26PM +0200, Laurent Pinchart wrote:
> >> [...]
> >> > +
> >> > +		/* panel is 480x464 with top and bottom 5 lines not visible */
> >> 
> >> I assume you mean 480x864 ?
> >
> > Yes, nice catch. Basically the screen is 480x864, but only
> > 480x854 are visible.
> 
> It's been a while, but I thought the full 480x864 was actually usable
> and visible.

I tried that first and the first few lines were missing. The stock
kernel also uses only 854px:

https://github.com/nemomobile/kernel-adaptation-n950-n9/blob/mer-n9-2.6.32-20121301/arch/arm/mach-omap2/board-rm680-video.c

(search for partial_area)

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 02/23] ARM: dts: n950: add display support

[Jani Nikula]

On Wed, 23 Mar 2016, Sebastian Reichel <sre@kernel.org> wrote:
> On Wed, Mar 23, 2016 at 02:40:53PM +0200, Jani Nikula wrote:
>> On Thu, 17 Mar 2016, Sebastian Reichel <sre@kernel.org> wrote:
>> > On Thu, Mar 17, 2016 at 02:14:26PM +0200, Laurent Pinchart wrote:
>> >> [...]
>> >> > +
>> >> > +		/* panel is 480x464 with top and bottom 5 lines not visible */
>> >> 
>> >> I assume you mean 480x864 ?
>> >
>> > Yes, nice catch. Basically the screen is 480x864, but only
>> > 480x854 are visible.
>> 
>> It's been a while, but I thought the full 480x864 was actually usable
>> and visible.
>
> I tried that first and the first few lines were missing. The stock
> kernel also uses only 854px:
>
> https://github.com/nemomobile/kernel-adaptation-n950-n9/blob/mer-n9-2.6.32-20121301/arch/arm/mach-omap2/board-rm680-video.c
>
> (search for partial_area)

Heh, I was reminded by old colleagues that it was actually my commit
back in the day that changed the resolution 864->854 in the stock
kernel. And that I did it reluctantly, because there really was no
technical reason to do the change.

I don't really care all that much either way anymore. I just thought
you'd like to get those 4800 pixels back that you've been missing all
these years. Plus 864 was nicer to deal with because it has 2^5 as a
prime factor while 854 only has 2.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH 02/23] ARM: dts: n950: add display support

[Jani Nikula]

On Wed, 23 Mar 2016, Sebastian Reichel <sre@kernel.org> wrote:
> On Wed, Mar 23, 2016 at 02:40:53PM +0200, Jani Nikula wrote:
>> On Thu, 17 Mar 2016, Sebastian Reichel <sre@kernel.org> wrote:
>> > On Thu, Mar 17, 2016 at 02:14:26PM +0200, Laurent Pinchart wrote:
>> >> [...]
>> >> > +
>> >> > +		/* panel is 480x464 with top and bottom 5 lines not visible */
>> >> 
>> >> I assume you mean 480x864 ?
>> >
>> > Yes, nice catch. Basically the screen is 480x864, but only
>> > 480x854 are visible.
>> 
>> It's been a while, but I thought the full 480x864 was actually usable
>> and visible.
>
> I tried that first and the first few lines were missing. The stock
> kernel also uses only 854px:
>
> https://github.com/nemomobile/kernel-adaptation-n950-n9/blob/mer-n9-2.6.32-20121301/arch/arm/mach-omap2/board-rm680-video.c
>
> (search for partial_area)

Heh, I was reminded by old colleagues that it was actually my commit
back in the day that changed the resolution 864->854 in the stock
kernel. And that I did it reluctantly, because there really was no
technical reason to do the change.

I don't really care all that much either way anymore. I just thought
you'd like to get those 4800 pixels back that you've been missing all
these years. Plus 864 was nicer to deal with because it has 2^5 as a
prime factor while 854 only has 2.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/23] ARM: dts: n950: add display support

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 2215 bytes --]

Hi,

On Thu, Mar 24, 2016 at 12:03:01PM +0200, Jani Nikula wrote:
> On Wed, 23 Mar 2016, Sebastian Reichel <sre@kernel.org> wrote:
> > On Wed, Mar 23, 2016 at 02:40:53PM +0200, Jani Nikula wrote:
> >> On Thu, 17 Mar 2016, Sebastian Reichel <sre@kernel.org> wrote:
> >> > On Thu, Mar 17, 2016 at 02:14:26PM +0200, Laurent Pinchart wrote:
> >> >> [...]
> >> >> > +
> >> >> > +		/* panel is 480x464 with top and bottom 5 lines not visible */
> >> >> 
> >> >> I assume you mean 480x864 ?
> >> >
> >> > Yes, nice catch. Basically the screen is 480x864, but only
> >> > 480x854 are visible.
> >> 
> >> It's been a while, but I thought the full 480x864 was actually usable
> >> and visible.
> >
> > I tried that first and the first few lines were missing. The stock
> > kernel also uses only 854px:
> >
> > https://github.com/nemomobile/kernel-adaptation-n950-n9/blob/mer-n9-2.6.32-20121301/arch/arm/mach-omap2/board-rm680-video.c
> >
> > (search for partial_area)
> 
> Heh, I was reminded by old colleagues that it was actually my commit
> back in the day that changed the resolution 864->854 in the stock
> kernel. And that I did it reluctantly, because there really was no
> technical reason to do the change.
> 
> I don't really care all that much either way anymore. I just thought
> you'd like to get those 4800 pixels back that you've been missing all
> these years. Plus 864 was nicer to deal with because it has 2^5 as a
> prime factor while 854 only has 2.

As I said: I did use 864 initially. That results in missing pixels.
This is what I observed before switching to 854:

In fbcon the first line was rendered half (only the bottom part of
each character was visible). Then, when I rotated fbcon (fbcon has
native rotation support, which does not work with DRM, but just
renders the text differently), the left part of each character was
missing. In my case the "[" prefix of kernel messages was rendered
as two dots. At least the vertical line was not visible at all.

I _think_, that your HW team decided to cover the first and the
last few pixels of the 864 display with plastic. So technically
it's a 864 display, but effectively it's 854.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 02/23] ARM: dts: n950: add display support

[Jani Nikula]

On Thu, 24 Mar 2016, Sebastian Reichel <sre@kernel.org> wrote:
> As I said: I did use 864 initially. That results in missing pixels.

Sorry, I didn't mean to question this. Go with what works, not with some
old fart's ramblings!

> I _think_, that your HW team decided to cover the first and the
> last few pixels of the 864 display with plastic. So technically
> it's a 864 display, but effectively it's 854.

(*shudder* at "your HW team" ;)

It's plausible, the covers did change slightly for the developer
edition.

Good luck with the upstreaming efforts!


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH 02/23] ARM: dts: n950: add display support

[Jani Nikula]

On Thu, 24 Mar 2016, Sebastian Reichel <sre@kernel.org> wrote:
> As I said: I did use 864 initially. That results in missing pixels.

Sorry, I didn't mean to question this. Go with what works, not with some
old fart's ramblings!

> I _think_, that your HW team decided to cover the first and the
> last few pixels of the 864 display with plastic. So technically
> it's a 864 display, but effectively it's 854.

(*shudder* at "your HW team" ;)

It's plausible, the covers did change slightly for the developer
edition.

Good luck with the upstreaming efforts!


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/23] ARM: dts: n950: add display support

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1234 bytes --]

Hi,

On Thu, Mar 24, 2016 at 05:11:15PM +0200, Jani Nikula wrote:
> On Thu, 24 Mar 2016, Sebastian Reichel <sre@kernel.org> wrote:
> > As I said: I did use 864 initially. That results in missing pixels.
> 
> Sorry, I didn't mean to question this. Go with what works, not with some
> old fart's ramblings!

No problem. I also wondered why this has been done. If I understood
Tomi right, the userspace team feared, that their software would not
work with 864, since it was originally written for 854.

Anyways I'm pretty sure, that the first 5 pixels are unusable
from the users point of view.

> > I _think_, that your HW team decided to cover the first and the
> > last few pixels of the 864 display with plastic. So technically
> > it's a 864 display, but effectively it's 854.
> 
> (*shudder* at "your HW team" ;)

;)

> It's plausible, the covers did change slightly for the developer
> edition.

Assuming the non-developer edition can use the full display it
can be "unlocked" with my proposed DT bindings by slight
modifications. I guess there exist just a couple of those, so
maybe we just ignore it for the mainline kernel?

> Good luck with the upstreaming efforts!

Thanks.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 02/23] ARM: dts: n950: add display support

[Tony Lindgren]

* Sebastian Reichel <sre@kernel.org> [160324 17:16]:
> On Thu, Mar 24, 2016 at 05:11:15PM +0200, Jani Nikula wrote:
> > > I _think_, that your HW team decided to cover the first and the
> > > last few pixels of the 864 display with plastic. So technically
> > > it's a 864 display, but effectively it's 854.

Sebastian, should I apply this dts patch as is or are changes
still needed?

Tony

Re: [PATCH 02/23] ARM: dts: n950: add display support

[Tony Lindgren]

* Tony Lindgren <tony@atomide.com> [160412 13:53]:
> * Sebastian Reichel <sre@kernel.org> [160324 17:16]:
> > On Thu, Mar 24, 2016 at 05:11:15PM +0200, Jani Nikula wrote:
> > > > I _think_, that your HW team decided to cover the first and the
> > > > last few pixels of the 864 display with plastic. So technically
> > > > it's a 864 display, but effectively it's 854.
> 
> Sebastian, should I apply this dts patch as is or are changes
> still needed?

Well please repost when ready, I'll untag this old thread here..

Tony

Re: [PATCH 02/23] ARM: dts: n950: add display support

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 771 bytes --]

Hi Tony,

On Tue, Jun 21, 2016 at 04:01:03AM -0700, Tony Lindgren wrote:
> * Tony Lindgren <tony@atomide.com> [160412 13:53]:
> > * Sebastian Reichel <sre@kernel.org> [160324 17:16]:
> > > On Thu, Mar 24, 2016 at 05:11:15PM +0200, Jani Nikula wrote:
> > > > > I _think_, that your HW team decided to cover the first and the
> > > > > last few pixels of the 864 display with plastic. So technically
> > > > > it's a 864 display, but effectively it's 854.
> > 
> > Sebastian, should I apply this dts patch as is or are changes
> > still needed?
> 
> Well please repost when ready, I'll untag this old thread here..

Sounds fine to me. DTS changes should be merged once the driver
changes are accepted and they need at least one more cycle.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH 03/23] drm: omapdrm: dss: reset dsi module during initialization

[Sebastian Reichel]

From: Tomi Valkeinen <tomi.valkeinen@ti.com>

The Nokia N950's bootloader leaves the DSI module
in a non-working state. Work around this by resetting
the DSI module before using it.

Signed-off-By: Tomi Valkeinen <tomi.valkeinen@ti.com>
Reviewed-By: Sebastian Reichel <sre@kernel.org>
Tested-By: Sebastian Reichel <sre@kernel.org>
---
 drivers/gpu/drm/omapdrm/dss/dsi.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c
index 43be4b2a7b05..363a61e6aca7 100644
--- a/drivers/gpu/drm/omapdrm/dss/dsi.c
+++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
@@ -5274,6 +5274,28 @@ static int dsi_init_pll_data(struct platform_device *dsidev)
 	return 0;
 }
 
+static int _dsi_wait_reset(struct platform_device *dsidev)
+{
+	int t = 0;
+
+	while (REG_GET(dsidev, DSI_SYSSTATUS, 0, 0) == 0) {
+		 if (++t > 5) {
+		DSSERR("soft reset failed\n");
+			return -ENODEV;
+		 }
+		 udelay(1);
+	}
+
+	return 0;
+}
+
+static int _dsi_reset(struct platform_device *dsidev)
+{
+	/* Soft reset */
+	dsi_write_reg(dsidev, DSI_SYSCONFIG, 0x2);
+	return _dsi_wait_reset(dsidev);
+}
+
 /* DSI1 HW IP initialisation */
 static int dsi_bind(struct device *dev, struct device *master, void *data)
 {
@@ -5432,6 +5454,8 @@ static int dsi_bind(struct device *dev, struct device *master, void *data)
 	if (r)
 		goto err_runtime_get;
 
+	_dsi_reset(dsidev);
+
 	rev = dsi_read_reg(dsidev, DSI_REVISION);
 	dev_dbg(&dsidev->dev, "OMAP DSI rev %d.%d\n",
 	       FLD_GET(rev, 7, 4), FLD_GET(rev, 3, 0));
-- 
2.7.0

Re: [PATCH 03/23] drm: omapdrm: dss: reset dsi module during initialization

[Tomi Valkeinen]

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



On 08/03/16 18:39, Sebastian Reichel wrote:
> From: Tomi Valkeinen <tomi.valkeinen@ti.com>
> 
> The Nokia N950's bootloader leaves the DSI module
> in a non-working state. Work around this by resetting
> the DSI module before using it.
> 
> Signed-off-By: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Reviewed-By: Sebastian Reichel <sre@kernel.org>
> Tested-By: Sebastian Reichel <sre@kernel.org>
> ---
>  drivers/gpu/drm/omapdrm/dss/dsi.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c
> index 43be4b2a7b05..363a61e6aca7 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dsi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
> @@ -5274,6 +5274,28 @@ static int dsi_init_pll_data(struct platform_device *dsidev)
>  	return 0;
>  }
>  
> +static int _dsi_wait_reset(struct platform_device *dsidev)
> +{
> +	int t = 0;
> +
> +	while (REG_GET(dsidev, DSI_SYSSTATUS, 0, 0) == 0) {
> +		 if (++t > 5) {
> +		DSSERR("soft reset failed\n");
> +			return -ENODEV;
> +		 }
> +		 udelay(1);
> +	}
> +
> +	return 0;
> +}
> +
> +static int _dsi_reset(struct platform_device *dsidev)
> +{
> +	/* Soft reset */
> +	dsi_write_reg(dsidev, DSI_SYSCONFIG, 0x2);
> +	return _dsi_wait_reset(dsidev);
> +}
> +
>  /* DSI1 HW IP initialisation */
>  static int dsi_bind(struct device *dev, struct device *master, void *data)
>  {
> @@ -5432,6 +5454,8 @@ static int dsi_bind(struct device *dev, struct device *master, void *data)
>  	if (r)
>  		goto err_runtime_get;
>  
> +	_dsi_reset(dsidev);
> +
>  	rev = dsi_read_reg(dsidev, DSI_REVISION);
>  	dev_dbg(&dsidev->dev, "OMAP DSI rev %d.%d\n",
>  	       FLD_GET(rev, 7, 4), FLD_GET(rev, 3, 0));
> 

This is good as a prototype, but unfortunately we're not supposed to
reset the IP from the driver. I haven't followed if there's a proper
reset framework yet available, but if not, I think we should do the
reset at the platform code at boot time.

Which actually makes me wonder... We should be doing that reset already
there, in mach-omap2/display.c. I wonder why that's not working...

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 03/23] drm: omapdrm: dss: reset dsi module during initialization

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 2185 bytes --]



On 08/03/16 18:39, Sebastian Reichel wrote:
> From: Tomi Valkeinen <tomi.valkeinen@ti.com>
> 
> The Nokia N950's bootloader leaves the DSI module
> in a non-working state. Work around this by resetting
> the DSI module before using it.
> 
> Signed-off-By: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Reviewed-By: Sebastian Reichel <sre@kernel.org>
> Tested-By: Sebastian Reichel <sre@kernel.org>
> ---
>  drivers/gpu/drm/omapdrm/dss/dsi.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c
> index 43be4b2a7b05..363a61e6aca7 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dsi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
> @@ -5274,6 +5274,28 @@ static int dsi_init_pll_data(struct platform_device *dsidev)
>  	return 0;
>  }
>  
> +static int _dsi_wait_reset(struct platform_device *dsidev)
> +{
> +	int t = 0;
> +
> +	while (REG_GET(dsidev, DSI_SYSSTATUS, 0, 0) == 0) {
> +		 if (++t > 5) {
> +		DSSERR("soft reset failed\n");
> +			return -ENODEV;
> +		 }
> +		 udelay(1);
> +	}
> +
> +	return 0;
> +}
> +
> +static int _dsi_reset(struct platform_device *dsidev)
> +{
> +	/* Soft reset */
> +	dsi_write_reg(dsidev, DSI_SYSCONFIG, 0x2);
> +	return _dsi_wait_reset(dsidev);
> +}
> +
>  /* DSI1 HW IP initialisation */
>  static int dsi_bind(struct device *dev, struct device *master, void *data)
>  {
> @@ -5432,6 +5454,8 @@ static int dsi_bind(struct device *dev, struct device *master, void *data)
>  	if (r)
>  		goto err_runtime_get;
>  
> +	_dsi_reset(dsidev);
> +
>  	rev = dsi_read_reg(dsidev, DSI_REVISION);
>  	dev_dbg(&dsidev->dev, "OMAP DSI rev %d.%d\n",
>  	       FLD_GET(rev, 7, 4), FLD_GET(rev, 3, 0));
> 

This is good as a prototype, but unfortunately we're not supposed to
reset the IP from the driver. I haven't followed if there's a proper
reset framework yet available, but if not, I think we should do the
reset at the platform code at boot time.

Which actually makes me wonder... We should be doing that reset already
there, in mach-omap2/display.c. I wonder why that's not working...

 Tomi


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

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

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

[PATCH 04/23] drm: omapdrm: add DSI mapping

[Sebastian Reichel]

This sets proper connector type for DSI connected panels.

Signed-off-By: Sebastian Reichel <sre@kernel.org>
---
 drivers/gpu/drm/omapdrm/omap_drv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index dfafdb602ad2..a3ff35f5f6cd 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -215,6 +215,8 @@ static int get_connector_type(struct omap_dss_device *dssdev)
 		return DRM_MODE_CONNECTOR_HDMIA;
 	case OMAP_DISPLAY_TYPE_DVI:
 		return DRM_MODE_CONNECTOR_DVID;
+	case OMAP_DISPLAY_TYPE_DSI:
+		return DRM_MODE_CONNECTOR_DSI;
 	default:
 		return DRM_MODE_CONNECTOR_Unknown;
 	}
-- 
2.7.0

Re: [PATCH 04/23] drm: omapdrm: add DSI mapping

[Laurent Pinchart]

Hi Sebastian,

Thank you for the patch.

On Tuesday 08 Mar 2016 17:39:36 Sebastian Reichel wrote:
> This sets proper connector type for DSI connected panels.
> 
> Signed-off-By: Sebastian Reichel <sre@kernel.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/omapdrm/omap_drv.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> b/drivers/gpu/drm/omapdrm/omap_drv.c index dfafdb602ad2..a3ff35f5f6cd
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -215,6 +215,8 @@ static int get_connector_type(struct omap_dss_device
> *dssdev) return DRM_MODE_CONNECTOR_HDMIA;
>  	case OMAP_DISPLAY_TYPE_DVI:
>  		return DRM_MODE_CONNECTOR_DVID;
> +	case OMAP_DISPLAY_TYPE_DSI:
> +		return DRM_MODE_CONNECTOR_DSI;
>  	default:
>  		return DRM_MODE_CONNECTOR_Unknown;
>  	}

-- 
Regards,

Laurent Pinchart

Re: [PATCH 04/23] drm: omapdrm: add DSI mapping

[Laurent Pinchart]

Hi Sebastian,

Thank you for the patch.

On Tuesday 08 Mar 2016 17:39:36 Sebastian Reichel wrote:
> This sets proper connector type for DSI connected panels.
> 
> Signed-off-By: Sebastian Reichel <sre@kernel.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/omapdrm/omap_drv.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> b/drivers/gpu/drm/omapdrm/omap_drv.c index dfafdb602ad2..a3ff35f5f6cd
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -215,6 +215,8 @@ static int get_connector_type(struct omap_dss_device
> *dssdev) return DRM_MODE_CONNECTOR_HDMIA;
>  	case OMAP_DISPLAY_TYPE_DVI:
>  		return DRM_MODE_CONNECTOR_DVID;
> +	case OMAP_DISPLAY_TYPE_DSI:
> +		return DRM_MODE_CONNECTOR_DSI;
>  	default:
>  		return DRM_MODE_CONNECTOR_Unknown;
>  	}

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 04/23] drm: omapdrm: add DSI mapping

[Tomi Valkeinen]

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

On 08/03/16 18:39, Sebastian Reichel wrote:
> This sets proper connector type for DSI connected panels.
> 
> Signed-off-By: Sebastian Reichel <sre@kernel.org>
> ---
>  drivers/gpu/drm/omapdrm/omap_drv.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
> index dfafdb602ad2..a3ff35f5f6cd 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -215,6 +215,8 @@ static int get_connector_type(struct omap_dss_device *dssdev)
>  		return DRM_MODE_CONNECTOR_HDMIA;
>  	case OMAP_DISPLAY_TYPE_DVI:
>  		return DRM_MODE_CONNECTOR_DVID;
> +	case OMAP_DISPLAY_TYPE_DSI:
> +		return DRM_MODE_CONNECTOR_DSI;
>  	default:
>  		return DRM_MODE_CONNECTOR_Unknown;
>  	}
> 

Thanks, I've picked this up for 4.8.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 04/23] drm: omapdrm: add DSI mapping

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 857 bytes --]

On 08/03/16 18:39, Sebastian Reichel wrote:
> This sets proper connector type for DSI connected panels.
> 
> Signed-off-By: Sebastian Reichel <sre@kernel.org>
> ---
>  drivers/gpu/drm/omapdrm/omap_drv.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
> index dfafdb602ad2..a3ff35f5f6cd 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -215,6 +215,8 @@ static int get_connector_type(struct omap_dss_device *dssdev)
>  		return DRM_MODE_CONNECTOR_HDMIA;
>  	case OMAP_DISPLAY_TYPE_DVI:
>  		return DRM_MODE_CONNECTOR_DVID;
> +	case OMAP_DISPLAY_TYPE_DSI:
> +		return DRM_MODE_CONNECTOR_DSI;
>  	default:
>  		return DRM_MODE_CONNECTOR_Unknown;
>  	}
> 

Thanks, I've picked this up for 4.8.

 Tomi


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

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

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

[PATCH 05/23] Revert "drm: omapdrm: Remove manual update display support"

[Sebastian Reichel]

This reverts commit 5a35876e2830511cb8110667fc426c6a6165a593.

Revert the removal of manual update display support in
preparation for DSI command mode panels.

Signed-off-By: Sebastian Reichel <sre@kernel.org>
---
 drivers/gpu/drm/omapdrm/omap_connector.c | 12 +++++++
 drivers/gpu/drm/omapdrm/omap_drv.h       |  4 +++
 drivers/gpu/drm/omapdrm/omap_fb.c        | 38 +++++++++++++++++++++
 drivers/gpu/drm/omapdrm/omap_fbdev.c     | 57 +++++++++++++++++++++++++++++---
 4 files changed, 107 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
index 83f2a9177c14..8db36aa4bd00 100644
--- a/drivers/gpu/drm/omapdrm/omap_connector.c
+++ b/drivers/gpu/drm/omapdrm/omap_connector.c
@@ -275,6 +275,18 @@ static const struct drm_connector_helper_funcs omap_connector_helper_funcs = {
 	.best_encoder = omap_connector_attached_encoder,
 };
 
+/* flush an area of the framebuffer (in case of manual update display that
+ * is not automatically flushed)
+ */
+void omap_connector_flush(struct drm_connector *connector,
+		int x, int y, int w, int h)
+{
+	struct omap_connector *omap_connector = to_omap_connector(connector);
+
+	/* TODO: enable when supported in dss */
+	VERB("%s: %d,%d, %dx%d", omap_connector->dssdev->name, x, y, w, h);
+}
+
 /* initialize connector */
 struct drm_connector *omap_connector_init(struct drm_device *dev,
 		int connector_type, struct omap_dss_device *dssdev,
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h
index 9e0030731c37..5dfa93a3b505 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -170,6 +170,8 @@ struct drm_connector *omap_connector_init(struct drm_device *dev,
 struct drm_encoder *omap_connector_attached_encoder(
 		struct drm_connector *connector);
 bool omap_connector_get_hdmi_mode(struct drm_connector *connector);
+void omap_connector_flush(struct drm_connector *connector,
+		int x, int y, int w, int h);
 
 void copy_timings_omap_to_drm(struct drm_display_mode *mode,
 		struct omap_video_timings *timings);
@@ -189,6 +191,8 @@ void omap_framebuffer_update_scanout(struct drm_framebuffer *fb,
 		struct omap_drm_window *win, struct omap_overlay_info *info);
 struct drm_connector *omap_framebuffer_get_next_connector(
 		struct drm_framebuffer *fb, struct drm_connector *from);
+void omap_framebuffer_flush(struct drm_framebuffer *fb,
+		int x, int y, int w, int h);
 
 void omap_gem_init(struct drm_device *dev);
 void omap_gem_deinit(struct drm_device *dev);
diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c
index ad202dfc1a49..b51d2243f356 100644
--- a/drivers/gpu/drm/omapdrm/omap_fb.c
+++ b/drivers/gpu/drm/omapdrm/omap_fb.c
@@ -124,6 +124,16 @@ static int omap_framebuffer_dirty(struct drm_framebuffer *fb,
 		struct drm_file *file_priv, unsigned flags, unsigned color,
 		struct drm_clip_rect *clips, unsigned num_clips)
 {
+	int i;
+
+	drm_modeset_lock_all(fb->dev);
+	for (i = 0; i < num_clips; i++) {
+		omap_framebuffer_flush(fb, clips[i].x1, clips[i].y1,
+					clips[i].x2 - clips[i].x1,
+					clips[i].y2 - clips[i].y1);
+	}
+	drm_modeset_unlock_all(fb->dev);
+
 	return 0;
 }
 
@@ -345,6 +355,34 @@ struct drm_connector *omap_framebuffer_get_next_connector(
 	return NULL;
 }
 
+/* flush an area of the framebuffer (in case of manual update display that
+ * is not automatically flushed)
+ */
+void omap_framebuffer_flush(struct drm_framebuffer *fb,
+		int x, int y, int w, int h)
+{
+	struct drm_connector *connector = NULL;
+
+	VERB("flush: %d,%d %dx%d, fb=%p", x, y, w, h, fb);
+
+	/* FIXME: This is racy - no protection against modeset config changes. */
+	while ((connector = omap_framebuffer_get_next_connector(fb, connector))) {
+		/* only consider connectors that are part of a chain */
+		if (connector->encoder && connector->encoder->crtc) {
+			/* TODO: maybe this should propagate thru the crtc who
+			 * could do the coordinate translation..
+			 */
+			struct drm_crtc *crtc = connector->encoder->crtc;
+			int cx = max(0, x - crtc->x);
+			int cy = max(0, y - crtc->y);
+			int cw = w + (x - crtc->x) - cx;
+			int ch = h + (y - crtc->y) - cy;
+
+			omap_connector_flush(connector, cx, cy, cw, ch);
+		}
+	}
+}
+
 #ifdef CONFIG_DEBUG_FS
 void omap_framebuffer_describe(struct drm_framebuffer *fb, struct seq_file *m)
 {
diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c b/drivers/gpu/drm/omapdrm/omap_fbdev.c
index 3cb16f0cf381..47ab7426c501 100644
--- a/drivers/gpu/drm/omapdrm/omap_fbdev.c
+++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c
@@ -42,8 +42,42 @@ struct omap_fbdev {
 	struct work_struct work;
 };
 
+static void omap_fbdev_flush(struct fb_info *fbi, int x, int y, int w, int h);
 static struct drm_fb_helper *get_fb(struct fb_info *fbi);
 
+static ssize_t omap_fbdev_write(struct fb_info *fbi, const char __user *buf,
+		size_t count, loff_t *ppos)
+{
+	ssize_t res;
+
+	res = fb_sys_write(fbi, buf, count, ppos);
+	omap_fbdev_flush(fbi, 0, 0, fbi->var.xres, fbi->var.yres);
+
+	return res;
+}
+
+static void omap_fbdev_fillrect(struct fb_info *fbi,
+		const struct fb_fillrect *rect)
+{
+	sys_fillrect(fbi, rect);
+	omap_fbdev_flush(fbi, rect->dx, rect->dy, rect->width, rect->height);
+}
+
+static void omap_fbdev_copyarea(struct fb_info *fbi,
+		const struct fb_copyarea *area)
+{
+	sys_copyarea(fbi, area);
+	omap_fbdev_flush(fbi, area->dx, area->dy, area->width, area->height);
+}
+
+static void omap_fbdev_imageblit(struct fb_info *fbi,
+		const struct fb_image *image)
+{
+	sys_imageblit(fbi, image);
+	omap_fbdev_flush(fbi, image->dx, image->dy,
+				image->width, image->height);
+}
+
 static void pan_worker(struct work_struct *work)
 {
 	struct omap_fbdev *fbdev = container_of(work, struct omap_fbdev, work);
@@ -87,10 +121,10 @@ static struct fb_ops omap_fb_ops = {
 	 * basic fbdev ops which write to the framebuffer
 	 */
 	.fb_read = drm_fb_helper_sys_read,
-	.fb_write = drm_fb_helper_sys_write,
-	.fb_fillrect = drm_fb_helper_sys_fillrect,
-	.fb_copyarea = drm_fb_helper_sys_copyarea,
-	.fb_imageblit = drm_fb_helper_sys_imageblit,
+	.fb_write = omap_fbdev_write,
+	.fb_fillrect = omap_fbdev_fillrect,
+	.fb_copyarea = omap_fbdev_copyarea,
+	.fb_imageblit = omap_fbdev_imageblit,
 
 	.fb_check_var = drm_fb_helper_check_var,
 	.fb_set_par = drm_fb_helper_set_par,
@@ -251,6 +285,21 @@ static struct drm_fb_helper *get_fb(struct fb_info *fbi)
 	return fbi->par;
 }
 
+/* flush an area of the framebuffer (in case of manual update display that
+ * is not automatically flushed)
+ */
+static void omap_fbdev_flush(struct fb_info *fbi, int x, int y, int w, int h)
+{
+	struct drm_fb_helper *helper = get_fb(fbi);
+
+	if (!helper)
+		return;
+
+	VERB("flush fbdev: %d,%d %dx%d, fbi=%p", x, y, w, h, fbi);
+
+	omap_framebuffer_flush(helper->fb, x, y, w, h);
+}
+
 /* initialize fbdev helper */
 struct drm_fb_helper *omap_fbdev_init(struct drm_device *dev)
 {
-- 
2.7.0

[PATCH 06/23] drm: omapdrm: wait for pending operations before updating plane

[Sebastian Reichel]

Updating the plane may interrupt ongoing display
updates, so wait for any pending operations.

Signed-off-By: Sebastian Reichel <sre@kernel.org>
---
 drivers/gpu/drm/omapdrm/omap_drv.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index a3ff35f5f6cd..e142a4245766 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -95,6 +95,10 @@ static void omap_atomic_complete(struct omap_atomic_state_commit *commit)
 	/* Apply the atomic update. */
 	dispc_runtime_get();
 
+	dev_dbg(dev->dev, "omap_atomic_complete");
+
+	omap_atomic_wait_for_completion(dev, old_state);
+
 	drm_atomic_helper_commit_modeset_disables(dev, old_state);
 	drm_atomic_helper_commit_planes(dev, old_state, false);
 	drm_atomic_helper_commit_modeset_enables(dev, old_state);
-- 
2.7.0

Re: [PATCH 06/23] drm: omapdrm: wait for pending operations before updating plane

[Laurent Pinchart]

Hi Sabastian,

Thank you for the patch.

On Tuesday 08 Mar 2016 17:39:38 Sebastian Reichel wrote:
> Updating the plane may interrupt ongoing display
> updates, so wait for any pending operations.

There's already an omap_atomic_wait_for_completion() call a couple of lines 
below, do we need two of them ? Why can display update be ongoing there, given 
that the previous omap_atomic_complete() call did wait for completion before 
returning ?

> Signed-off-By: Sebastian Reichel <sre@kernel.org>
> ---
>  drivers/gpu/drm/omapdrm/omap_drv.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> b/drivers/gpu/drm/omapdrm/omap_drv.c index a3ff35f5f6cd..e142a4245766
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -95,6 +95,10 @@ static void omap_atomic_complete(struct
> omap_atomic_state_commit *commit) /* Apply the atomic update. */
>  	dispc_runtime_get();
> 
> +	dev_dbg(dev->dev, "omap_atomic_complete");
> +
> +	omap_atomic_wait_for_completion(dev, old_state);
> +
>  	drm_atomic_helper_commit_modeset_disables(dev, old_state);
>  	drm_atomic_helper_commit_planes(dev, old_state, false);
>  	drm_atomic_helper_commit_modeset_enables(dev, old_state);

-- 
Regards,

Laurent Pinchart

Re: [PATCH 06/23] drm: omapdrm: wait for pending operations before updating plane

[Laurent Pinchart]

Hi Sabastian,

Thank you for the patch.

On Tuesday 08 Mar 2016 17:39:38 Sebastian Reichel wrote:
> Updating the plane may interrupt ongoing display
> updates, so wait for any pending operations.

There's already an omap_atomic_wait_for_completion() call a couple of lines 
below, do we need two of them ? Why can display update be ongoing there, given 
that the previous omap_atomic_complete() call did wait for completion before 
returning ?

> Signed-off-By: Sebastian Reichel <sre@kernel.org>
> ---
>  drivers/gpu/drm/omapdrm/omap_drv.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> b/drivers/gpu/drm/omapdrm/omap_drv.c index a3ff35f5f6cd..e142a4245766
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -95,6 +95,10 @@ static void omap_atomic_complete(struct
> omap_atomic_state_commit *commit) /* Apply the atomic update. */
>  	dispc_runtime_get();
> 
> +	dev_dbg(dev->dev, "omap_atomic_complete");
> +
> +	omap_atomic_wait_for_completion(dev, old_state);
> +
>  	drm_atomic_helper_commit_modeset_disables(dev, old_state);
>  	drm_atomic_helper_commit_planes(dev, old_state, false);
>  	drm_atomic_helper_commit_modeset_enables(dev, old_state);

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 06/23] drm: omapdrm: wait for pending operations before updating plane

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1913 bytes --]

Hi Laurent,

On Sat, Mar 26, 2016 at 11:20:00AM +0200, Laurent Pinchart wrote:
> On Tuesday 08 Mar 2016 17:39:38 Sebastian Reichel wrote:
> > Updating the plane may interrupt ongoing display
> > updates, so wait for any pending operations.
> 
> There's already an omap_atomic_wait_for_completion() call a couple of lines 
> below, do we need two of them ? Why can display update be ongoing there, given 
> that the previous omap_atomic_complete() call did wait for completion before 
> returning?

This is a preparation for the manual display update. The planes
should not be touched while a manual display update is in progress.

I only checked the patches for the N950's manually updated DSI
panel, but none of the other supported panel types. Since I wasn't
sure what is triggered by the plane update for the other panel
types, I kept the second wait_for_completion.

I guess I will drop the second wait in the next revision and see
what happens.

-- Sebastian

> > Signed-off-By: Sebastian Reichel <sre@kernel.org>
> > ---
> >  drivers/gpu/drm/omapdrm/omap_drv.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> > b/drivers/gpu/drm/omapdrm/omap_drv.c index a3ff35f5f6cd..e142a4245766
> > 100644
> > --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> > +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> > @@ -95,6 +95,10 @@ static void omap_atomic_complete(struct
> > omap_atomic_state_commit *commit) /* Apply the atomic update. */
> >  	dispc_runtime_get();
> > 
> > +	dev_dbg(dev->dev, "omap_atomic_complete");
> > +
> > +	omap_atomic_wait_for_completion(dev, old_state);
> > +
> >  	drm_atomic_helper_commit_modeset_disables(dev, old_state);
> >  	drm_atomic_helper_commit_planes(dev, old_state, false);
> >  	drm_atomic_helper_commit_modeset_enables(dev, old_state);
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH 07/23] drm: omapdrm: crtc: switch pending variable to atomic bitset

[Sebastian Reichel]

Having the pending variable available as atomic bit helps
with the later addition of manually updated display support.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 2ed0754ed19e..5ef27664bcfa 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -28,6 +28,11 @@
 
 #define to_omap_crtc(x) container_of(x, struct omap_crtc, base)
 
+enum omap_crtc_state {
+	crtc_enabled	= 0,
+	crtc_pending	= 1
+};
+
 struct omap_crtc {
 	struct drm_crtc base;
 
@@ -49,7 +54,7 @@ struct omap_crtc {
 
 	bool ignore_digit_sync_lost;
 
-	bool pending;
+	unsigned long state;
 	wait_queue_head_t pending_wait;
 };
 
@@ -81,7 +86,7 @@ int omap_crtc_wait_pending(struct drm_crtc *crtc)
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
 
 	return wait_event_timeout(omap_crtc->pending_wait,
-				  !omap_crtc->pending,
+				  !test_bit(crtc_pending, &omap_crtc->state),
 				  msecs_to_jiffies(50));
 }
 
@@ -311,10 +316,8 @@ static void omap_crtc_vblank_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
 
 	__omap_irq_unregister(dev, &omap_crtc->vblank_irq);
 
-	rmb();
-	WARN_ON(!omap_crtc->pending);
-	omap_crtc->pending = false;
-	wmb();
+	if (!test_and_clear_bit(crtc_pending, &omap_crtc->state))
+		dev_warn(dev->dev, "pending bit was not set in vblank irq");
 
 	/* wake up userspace */
 	omap_crtc_complete_page_flip(&omap_crtc->base);
@@ -351,13 +354,12 @@ static bool omap_crtc_mode_fixup(struct drm_crtc *crtc,
 static void omap_crtc_enable(struct drm_crtc *crtc)
 {
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct drm_device *dev = crtc->dev;
 
 	DBG("%s", omap_crtc->name);
 
-	rmb();
-	WARN_ON(omap_crtc->pending);
-	omap_crtc->pending = true;
-	wmb();
+	if (test_and_set_bit(crtc_pending, &omap_crtc->state))
+		dev_warn(dev->dev, "crtc enable while pending bit set!");
 
 	omap_irq_register(crtc->dev, &omap_crtc->vblank_irq);
 
@@ -397,6 +399,7 @@ static void omap_crtc_atomic_flush(struct drm_crtc *crtc,
                                   struct drm_crtc_state *old_crtc_state)
 {
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct drm_device *dev = crtc->dev;
 
 	WARN_ON(omap_crtc->vblank_irq.registered);
 
@@ -404,10 +407,8 @@ static void omap_crtc_atomic_flush(struct drm_crtc *crtc,
 
 		DBG("%s: GO", omap_crtc->name);
 
-		rmb();
-		WARN_ON(omap_crtc->pending);
-		omap_crtc->pending = true;
-		wmb();
+		if (test_and_set_bit(crtc_pending, &omap_crtc->state))
+			dev_warn(dev->dev, "atomic flush while pending bit set!");
 
 		dispc_mgr_go(omap_crtc->channel);
 		omap_irq_register(crtc->dev, &omap_crtc->vblank_irq);
@@ -509,6 +510,8 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 
 	init_waitqueue_head(&omap_crtc->pending_wait);
 
+	omap_crtc->state = 0;
+
 	omap_crtc->channel = channel;
 	omap_crtc->name = channel_names[channel];
 
-- 
2.7.0

Re: [PATCH 07/23] drm: omapdrm: crtc: switch pending variable to atomic bitset

[Laurent Pinchart]

Hi Sebastian,

Thank you for the patch.

On Tuesday 08 Mar 2016 17:39:39 Sebastian Reichel wrote:
> Having the pending variable available as atomic bit helps
> with the later addition of manually updated display support.
> 
> Signed-off-by: Sebastian Reichel <sre@kernel.org>
> ---
>  drivers/gpu/drm/omapdrm/omap_crtc.c | 31 +++++++++++++++++--------------
>  1 file changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c index 2ed0754ed19e..5ef27664bcfa
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -28,6 +28,11 @@
> 
>  #define to_omap_crtc(x) container_of(x, struct omap_crtc, base)
> 
> +enum omap_crtc_state {

I find that using an enum and calling it omap_crtc_state is confusing, it 
seems to imply that the CRTC state will be one of the enumerated values, while 
the values are actually non-exclusive bits in a bitmask.

> +	crtc_enabled	= 0,

You don't seem to be using this bit in this patch, you can define it in patch 
08/23.

> +	crtc_pending	= 1

Please name this OMAP_CRTC_STATE_PENDING.

> +};

Please add a short description of each bit in a comment above the enum.

> +
>  struct omap_crtc {
>  	struct drm_crtc base;
> 
> @@ -49,7 +54,7 @@ struct omap_crtc {
> 
>  	bool ignore_digit_sync_lost;
> 
> -	bool pending;
> +	unsigned long state;
>  	wait_queue_head_t pending_wait;
>  };
> 
> @@ -81,7 +86,7 @@ int omap_crtc_wait_pending(struct drm_crtc *crtc)
>  	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> 
>  	return wait_event_timeout(omap_crtc->pending_wait,
> -				  !omap_crtc->pending,
> +				  !test_bit(crtc_pending, &omap_crtc->state),
>  				  msecs_to_jiffies(50));
>  }
> 
> @@ -311,10 +316,8 @@ static void omap_crtc_vblank_irq(struct omap_drm_irq
> *irq, uint32_t irqstatus)
> 
>  	__omap_irq_unregister(dev, &omap_crtc->vblank_irq);
> 
> -	rmb();
> -	WARN_ON(!omap_crtc->pending);
> -	omap_crtc->pending = false;
> -	wmb();
> +	if (!test_and_clear_bit(crtc_pending, &omap_crtc->state))
> +		dev_warn(dev->dev, "pending bit was not set in vblank irq");

Documentation/atomic_ops.txt states that the atomic bit ops must be atomic and 
include memory barriers, but I'm confused by the ARM implementation.

The constant bit number version ____atomic_test_and_clear_bit() uses 
raw_local_irq_save() and raw_low_irq_restore() for synchronization, which 
expand to arch_local_irq_save() and arch_local_irq_restore(). On ARMv6 and 
newer, the functions are defined as

static inline unsigned long arch_local_irq_save(void)
{
        unsigned long flags;

        asm volatile(
                "       mrs     %0, " IRQMASK_REG_NAME_R "      @ 
arch_local_irq_save\n"
                "       cpsid   i"
                : "=r" (flags) : : "memory", "cc");
        return flags;
}

and

static inline void arch_local_irq_restore(unsigned long flags)
{
        asm volatile(
                "       msr     " IRQMASK_REG_NAME_W ", %0      @ 
local_irq_restore"
                :
                : "r" (flags)
                : "memory", "cc");
}

I'm probably missing something obvious, but I don't see the memory barriers 
:-/

>  	/* wake up userspace */
>  	omap_crtc_complete_page_flip(&omap_crtc->base);
> @@ -351,13 +354,12 @@ static bool omap_crtc_mode_fixup(struct drm_crtc
> *crtc, static void omap_crtc_enable(struct drm_crtc *crtc)
>  {
>  	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> +	struct drm_device *dev = crtc->dev;
> 
>  	DBG("%s", omap_crtc->name);
> 
> -	rmb();
> -	WARN_ON(omap_crtc->pending);
> -	omap_crtc->pending = true;
> -	wmb();
> +	if (test_and_set_bit(crtc_pending, &omap_crtc->state))
> +		dev_warn(dev->dev, "crtc enable while pending bit set!");
> 
>  	omap_irq_register(crtc->dev, &omap_crtc->vblank_irq);
> 
> @@ -397,6 +399,7 @@ static void omap_crtc_atomic_flush(struct drm_crtc
> *crtc, struct drm_crtc_state *old_crtc_state) {
>  	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> +	struct drm_device *dev = crtc->dev;
> 
>  	WARN_ON(omap_crtc->vblank_irq.registered);
> 
> @@ -404,10 +407,8 @@ static void omap_crtc_atomic_flush(struct drm_crtc
> *crtc,
> 
>  		DBG("%s: GO", omap_crtc->name);
> 
> -		rmb();
> -		WARN_ON(omap_crtc->pending);
> -		omap_crtc->pending = true;
> -		wmb();
> +		if (test_and_set_bit(crtc_pending, &omap_crtc->state))
> +			dev_warn(dev->dev, "atomic flush while pending bit set!");
> 
>  		dispc_mgr_go(omap_crtc->channel);
>  		omap_irq_register(crtc->dev, &omap_crtc->vblank_irq);
> @@ -509,6 +510,8 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
> 
>  	init_waitqueue_head(&omap_crtc->pending_wait);
> 
> +	omap_crtc->state = 0;
> +
>  	omap_crtc->channel = channel;
>  	omap_crtc->name = channel_names[channel];

-- 
Regards,

Laurent Pinchart

[PATCH 08/23] drm: omapdrm: crtc: add enabled bit to state

[Sebastian Reichel]

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 5ef27664bcfa..78ef9773cca1 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -207,6 +207,8 @@ static int omap_crtc_dss_enable(struct omap_overlay_manager *mgr)
 			&omap_crtc->timings);
 	omap_crtc_set_enabled(&omap_crtc->base, true);
 
+	set_bit(crtc_enabled, &omap_crtc->state);
+
 	return 0;
 }
 
@@ -214,6 +216,8 @@ static void omap_crtc_dss_disable(struct omap_overlay_manager *mgr)
 {
 	struct omap_crtc *omap_crtc = omap_crtcs[mgr->id];
 
+	clear_bit(crtc_enabled, &omap_crtc->state);
+
 	omap_crtc_set_enabled(&omap_crtc->base, false);
 }
 
-- 
2.7.0

Re: [PATCH 08/23] drm: omapdrm: crtc: add enabled bit to state

[Laurent Pinchart]

Hi Sebastian,

Thank you for the patch.

Given that you only add four lines of code here, and given that the enabled 
bit is unused here, I'd just squash this patch with the one that makes use of 
the enabled bit.

On Tuesday 08 Mar 2016 17:39:40 Sebastian Reichel wrote:
> Signed-off-by: Sebastian Reichel <sre@kernel.org>
> ---
>  drivers/gpu/drm/omapdrm/omap_crtc.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c index 5ef27664bcfa..78ef9773cca1
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -207,6 +207,8 @@ static int omap_crtc_dss_enable(struct
> omap_overlay_manager *mgr) &omap_crtc->timings);
>  	omap_crtc_set_enabled(&omap_crtc->base, true);
> 
> +	set_bit(crtc_enabled, &omap_crtc->state);
> +

Atomic KMS drivers should store state in the state structure, not the KMS 
objects themselves. Beside, how does this differ from the CRTC state enable 
and active fields ?

>  	return 0;
>  }
> 
> @@ -214,6 +216,8 @@ static void omap_crtc_dss_disable(struct
> omap_overlay_manager *mgr) {
>  	struct omap_crtc *omap_crtc = omap_crtcs[mgr->id];
> 
> +	clear_bit(crtc_enabled, &omap_crtc->state);
> +
>  	omap_crtc_set_enabled(&omap_crtc->base, false);
>  }

-- 
Regards,

Laurent Pinchart

[PATCH 09/23] drm: omapdrm: dss: method to get stallmode from lcd config

[Sebastian Reichel]

Introduce a new dss method for reading the stallmode
status from the private lcd config structure.

Signed-off-By: Sebastian Reichel <sre@kernel.org>
---
 drivers/gpu/drm/omapdrm/dss/output.c | 6 ++++++
 include/video/omapdss.h              | 1 +
 2 files changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/dss/output.c b/drivers/gpu/drm/omapdrm/dss/output.c
index 16072159bd24..b82e45938c9e 100644
--- a/drivers/gpu/drm/omapdrm/dss/output.c
+++ b/drivers/gpu/drm/omapdrm/dss/output.c
@@ -234,6 +234,12 @@ void dss_mgr_set_lcd_config(struct omap_overlay_manager *mgr,
 }
 EXPORT_SYMBOL(dss_mgr_set_lcd_config);
 
+bool dss_lcd_mgr_config_get_stallmode(const struct dss_lcd_mgr_config *config)
+{
+	return config->stallmode;
+}
+EXPORT_SYMBOL(dss_lcd_mgr_config_get_stallmode);
+
 int dss_mgr_enable(struct omap_overlay_manager *mgr)
 {
 	return dss_mgr_ops->enable(mgr);
diff --git a/include/video/omapdss.h b/include/video/omapdss.h
index 295b41e20d8e..9bde65b79220 100644
--- a/include/video/omapdss.h
+++ b/include/video/omapdss.h
@@ -975,6 +975,7 @@ int dss_mgr_register_framedone_handler(struct omap_overlay_manager *mgr,
 		void (*handler)(void *), void *data);
 void dss_mgr_unregister_framedone_handler(struct omap_overlay_manager *mgr,
 		void (*handler)(void *), void *data);
+bool dss_lcd_mgr_config_get_stallmode(const struct dss_lcd_mgr_config *config);
 
 static inline bool omapdss_device_is_connected(struct omap_dss_device *dssdev)
 {
-- 
2.7.0

[PATCH 10/23] drm: omapdrm: crtc: detect manually updated displays

[Sebastian Reichel]

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 31 ++++++++++++++++++++++++++++++-
 drivers/gpu/drm/omapdrm/omap_drv.h  |  1 +
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 78ef9773cca1..8967013c1fb5 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -48,6 +48,7 @@ struct omap_crtc {
 	struct omap_overlay_manager *mgr;
 
 	struct omap_video_timings timings;
+	bool manually_updated;
 
 	struct omap_drm_irq vblank_irq;
 	struct omap_drm_irq error_irq;
@@ -90,6 +91,12 @@ int omap_crtc_wait_pending(struct drm_crtc *crtc)
 				  msecs_to_jiffies(50));
 }
 
+bool omap_crtc_is_manual_updated(struct drm_crtc *crtc)
+{
+	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	return omap_crtc->manually_updated;
+}
+
 /* -----------------------------------------------------------------------------
  * DSS Manager Functions
  */
@@ -154,6 +161,11 @@ static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
 		omap_crtc->ignore_digit_sync_lost = true;
 	}
 
+	if (omap_crtc->manually_updated) {
+		dev_dbg(dev->dev, "stallmode detected, not waiting for irq");
+		return;
+	}
+
 	framedone_irq = dispc_mgr_get_framedone_irq(channel);
 	vsync_irq = dispc_mgr_get_vsync_irq(channel);
 
@@ -233,7 +245,13 @@ static void omap_crtc_dss_set_lcd_config(struct omap_overlay_manager *mgr,
 		const struct dss_lcd_mgr_config *config)
 {
 	struct omap_crtc *omap_crtc = omap_crtcs[mgr->id];
-	DBG("%s", omap_crtc->name);
+	struct drm_device *dev = omap_crtc->base.dev;
+	struct drm_plane *plane;
+
+	dev_dbg(dev->dev, "set lcd config for %s", omap_crtc->name);
+
+	omap_crtc->manually_updated = dss_lcd_mgr_config_get_stallmode(config);
+
 	dispc_mgr_set_lcd_config(omap_crtc->channel, config);
 }
 
@@ -358,10 +376,18 @@ static bool omap_crtc_mode_fixup(struct drm_crtc *crtc,
 static void omap_crtc_enable(struct drm_crtc *crtc)
 {
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct omap_dss_device *display = omap_crtc->mgr->output->dst;
 	struct drm_device *dev = crtc->dev;
 
 	DBG("%s", omap_crtc->name);
 
+	/* manual updated display will not trigger vsync irq */
+	/* omap_crtc->manually_updated is not yet set */
+	if (display->caps & OMAP_DSS_DISPLAY_CAP_MANUAL_UPDATE) {
+		dev_dbg(dev->dev, "manual update display detected!");
+		return;
+	}
+
 	if (test_and_set_bit(crtc_pending, &omap_crtc->state))
 		dev_warn(dev->dev, "crtc enable while pending bit set!");
 
@@ -407,6 +433,9 @@ static void omap_crtc_atomic_flush(struct drm_crtc *crtc,
 
 	WARN_ON(omap_crtc->vblank_irq.registered);
 
+	if (omap_crtc->manually_updated)
+		return;
+
 	if (dispc_mgr_is_enabled(omap_crtc->channel)) {
 
 		DBG("%s: GO", omap_crtc->name);
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h
index 5dfa93a3b505..71e2c2284b86 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -155,6 +155,7 @@ void omap_crtc_pre_uninit(void);
 struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 		struct drm_plane *plane, enum omap_channel channel, int id);
 int omap_crtc_wait_pending(struct drm_crtc *crtc);
+bool omap_crtc_is_manual_updated(struct drm_crtc *crtc);
 
 struct drm_plane *omap_plane_init(struct drm_device *dev,
 		int id, enum drm_plane_type type);
-- 
2.7.0

Re: [PATCH 10/23] drm: omapdrm: crtc: detect manually updated displays

[Laurent Pinchart]

Hi Sebastian,

Thank you for the patch.

On Tuesday 08 Mar 2016 17:39:42 Sebastian Reichel wrote:
> Signed-off-by: Sebastian Reichel <sre@kernel.org>
> ---
>  drivers/gpu/drm/omapdrm/omap_crtc.c | 31 ++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/omapdrm/omap_drv.h  |  1 +
>  2 files changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c index 78ef9773cca1..8967013c1fb5
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -48,6 +48,7 @@ struct omap_crtc {
>  	struct omap_overlay_manager *mgr;
> 
>  	struct omap_video_timings timings;
> +	bool manually_updated;
> 
>  	struct omap_drm_irq vblank_irq;
>  	struct omap_drm_irq error_irq;
> @@ -90,6 +91,12 @@ int omap_crtc_wait_pending(struct drm_crtc *crtc)
>  				  msecs_to_jiffies(50));
>  }
> 
> +bool omap_crtc_is_manual_updated(struct drm_crtc *crtc)
> +{
> +	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> +	return omap_crtc->manually_updated;
> +}
> +
>  /*
> ---------------------------------------------------------------------------
> -- * DSS Manager Functions
>   */
> @@ -154,6 +161,11 @@ static void omap_crtc_set_enabled(struct drm_crtc
> *crtc, bool enable) omap_crtc->ignore_digit_sync_lost = true;
>  	}
> 
> +	if (omap_crtc->manually_updated) {
> +		dev_dbg(dev->dev, "stallmode detected, not waiting for irq");
> +		return;
> +	}
> +
>  	framedone_irq = dispc_mgr_get_framedone_irq(channel);
>  	vsync_irq = dispc_mgr_get_vsync_irq(channel);
> 
> @@ -233,7 +245,13 @@ static void omap_crtc_dss_set_lcd_config(struct
> omap_overlay_manager *mgr, const struct dss_lcd_mgr_config *config)
>  {
>  	struct omap_crtc *omap_crtc = omap_crtcs[mgr->id];
> -	DBG("%s", omap_crtc->name);
> +	struct drm_device *dev = omap_crtc->base.dev;
> +	struct drm_plane *plane;
> +
> +	dev_dbg(dev->dev, "set lcd config for %s", omap_crtc->name);
> +
> +	omap_crtc->manually_updated = dss_lcd_mgr_config_get_stallmode(config);
> +
>  	dispc_mgr_set_lcd_config(omap_crtc->channel, config);
>  }
> 
> @@ -358,10 +376,18 @@ static bool omap_crtc_mode_fixup(struct drm_crtc
> *crtc, static void omap_crtc_enable(struct drm_crtc *crtc)
>  {
>  	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> +	struct omap_dss_device *display = omap_crtc->mgr->output->dst;
>  	struct drm_device *dev = crtc->dev;
> 
>  	DBG("%s", omap_crtc->name);
> 
> +	/* manual updated display will not trigger vsync irq */
> +	/* omap_crtc->manually_updated is not yet set */

Can't you figure out whether the display requires manual update earlier, 
preferably during atomic_check, and store the value in the CRTC state ?

> +	if (display->caps & OMAP_DSS_DISPLAY_CAP_MANUAL_UPDATE) {
> +		dev_dbg(dev->dev, "manual update display detected!");
> +		return;
> +	}
> +
>  	if (test_and_set_bit(crtc_pending, &omap_crtc->state))
>  		dev_warn(dev->dev, "crtc enable while pending bit set!");
> 
> @@ -407,6 +433,9 @@ static void omap_crtc_atomic_flush(struct drm_crtc
> *crtc,
> 
>  	WARN_ON(omap_crtc->vblank_irq.registered);
> 
> +	if (omap_crtc->manually_updated)
> +		return;
> +
>  	if (dispc_mgr_is_enabled(omap_crtc->channel)) {
> 
>  		DBG("%s: GO", omap_crtc->name);
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h
> b/drivers/gpu/drm/omapdrm/omap_drv.h index 5dfa93a3b505..71e2c2284b86
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.h
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.h
> @@ -155,6 +155,7 @@ void omap_crtc_pre_uninit(void);
>  struct drm_crtc *omap_crtc_init(struct drm_device *dev,
>  		struct drm_plane *plane, enum omap_channel channel, int id);
>  int omap_crtc_wait_pending(struct drm_crtc *crtc);
> +bool omap_crtc_is_manual_updated(struct drm_crtc *crtc);
> 
>  struct drm_plane *omap_plane_init(struct drm_device *dev,
>  		int id, enum drm_plane_type type);

-- 
Regards,

Laurent Pinchart

[PATCH 11/23] include: video: omapdss: provide fifo threshold methods

[Sebastian Reichel]

The FIFO thresholds must be configured by omapdrm for
manually updated DSI panels due to a hardware bug.

Signed-off-By: Sebastian Reichel <sre@kernel.org>
---
 include/video/omapdss.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/video/omapdss.h b/include/video/omapdss.h
index 9bde65b79220..f6cdd809ae5c 100644
--- a/include/video/omapdss.h
+++ b/include/video/omapdss.h
@@ -935,6 +935,12 @@ int dispc_ovl_setup(enum omap_plane plane, const struct omap_overlay_info *oi,
 		bool replication, const struct omap_video_timings *mgr_timings,
 		bool mem_to_mem);
 
+void dispc_ovl_compute_fifo_thresholds(enum omap_plane plane,
+		u32 *fifo_low, u32 *fifo_high, bool use_fifomerge,
+		bool manual_update);
+void dispc_ovl_set_fifo_threshold(enum omap_plane plane, u32 low, u32 high);
+
+
 int omapdss_compat_init(void);
 void omapdss_compat_uninit(void);
 
-- 
2.7.0

Re: [PATCH 11/23] include: video: omapdss: provide fifo threshold methods

[Laurent Pinchart]

Hi Sebastian,

Thank you for the patch.

On Tuesday 08 Mar 2016 17:39:43 Sebastian Reichel wrote:
> The FIFO thresholds must be configured by omapdrm for
> manually updated DSI panels due to a hardware bug.

I find the subject a bit misleading, it made me think that the patch added new 
methods, while it only declares them as part of the DSS public API.

> Signed-off-By: Sebastian Reichel <sre@kernel.org>
> ---
>  include/video/omapdss.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/include/video/omapdss.h b/include/video/omapdss.h
> index 9bde65b79220..f6cdd809ae5c 100644
> --- a/include/video/omapdss.h
> +++ b/include/video/omapdss.h
> @@ -935,6 +935,12 @@ int dispc_ovl_setup(enum omap_plane plane, const struct
> omap_overlay_info *oi, bool replication, const struct omap_video_timings
> *mgr_timings, bool mem_to_mem);
> 
> +void dispc_ovl_compute_fifo_thresholds(enum omap_plane plane,
> +		u32 *fifo_low, u32 *fifo_high, bool use_fifomerge,
> +		bool manual_update);
> +void dispc_ovl_set_fifo_threshold(enum omap_plane plane, u32 low, u32
> high);

Should you then remove the function declarations from 
drivers/gpu/drm/omapdrm/dss/dss.h ?

> +
> +

One blank line is enough.

>  int omapdss_compat_init(void);
>  void omapdss_compat_uninit(void);

-- 
Regards,

Laurent Pinchart

[PATCH 12/23] drm: omapdrm: plane: update fifo size on atomic update

[Sebastian Reichel]

This is a workaround for a hardware bug occuring
on OMAP3 with manually updated panels.

Signed-off-By: Sebastian Reichel <sre@kernel.org>
---
 drivers/gpu/drm/omapdrm/omap_drv.h   |  1 +
 drivers/gpu/drm/omapdrm/omap_plane.c | 23 +++++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h
index 71e2c2284b86..3ab4919aff4b 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -161,6 +161,7 @@ struct drm_plane *omap_plane_init(struct drm_device *dev,
 		int id, enum drm_plane_type type);
 void omap_plane_install_properties(struct drm_plane *plane,
 		struct drm_mode_object *obj);
+void omap_plane_update_fifo(struct drm_plane *plane);
 
 struct drm_encoder *omap_encoder_init(struct drm_device *dev,
 		struct omap_dss_device *dssdev);
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index d75b197eff46..0147e416140c 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -75,6 +75,28 @@ static void omap_plane_cleanup_fb(struct drm_plane *plane,
 		omap_framebuffer_unpin(old_state->fb);
 }
 
+void omap_plane_update_fifo(struct drm_plane *plane)
+{
+	struct omap_plane *omap_plane = to_omap_plane(plane);
+	struct drm_plane_state *state = plane->state;
+	struct drm_device *dev = plane->dev;
+	bool use_fifo_merge = false;
+	u32 fifo_low, fifo_high;
+	bool use_manual_update;
+
+	if (!dispc_ovl_enabled(omap_plane->id))
+		return;
+
+	use_manual_update = omap_crtc_is_manual_updated(state->crtc);
+
+	dispc_ovl_compute_fifo_thresholds(omap_plane->id, &fifo_low, &fifo_high,
+			use_fifo_merge, use_manual_update);
+
+	dev_dbg(dev->dev, "update fifo: %d %d", fifo_low, fifo_high);
+
+	dispc_ovl_set_fifo_threshold(omap_plane->id, fifo_low, fifo_high);
+}
+
 static void omap_plane_atomic_update(struct drm_plane *plane,
 				     struct drm_plane_state *old_state)
 {
@@ -141,6 +163,7 @@ static void omap_plane_atomic_update(struct drm_plane *plane,
 	}
 
 	dispc_ovl_enable(omap_plane->id, true);
+	omap_plane_update_fifo(plane);
 }
 
 static void omap_plane_atomic_disable(struct drm_plane *plane,
-- 
2.7.0

Re: [PATCH 12/23] drm: omapdrm: plane: update fifo size on atomic update

[Laurent Pinchart]

Hi Sebastian,

Thank you for the patch.

On Tuesday 08 Mar 2016 17:39:44 Sebastian Reichel wrote:
> This is a workaround for a hardware bug occuring
> on OMAP3 with manually updated panels.

Could you please explain what the bug is and how the workaround operates ? Do 
you have a reference to an errata document ?

> Signed-off-By: Sebastian Reichel <sre@kernel.org>
> ---
>  drivers/gpu/drm/omapdrm/omap_drv.h   |  1 +
>  drivers/gpu/drm/omapdrm/omap_plane.c | 23 +++++++++++++++++++++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h
> b/drivers/gpu/drm/omapdrm/omap_drv.h index 71e2c2284b86..3ab4919aff4b
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.h
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.h
> @@ -161,6 +161,7 @@ struct drm_plane *omap_plane_init(struct drm_device
> *dev, int id, enum drm_plane_type type);
>  void omap_plane_install_properties(struct drm_plane *plane,
>  		struct drm_mode_object *obj);
> +void omap_plane_update_fifo(struct drm_plane *plane);
> 
>  struct drm_encoder *omap_encoder_init(struct drm_device *dev,
>  		struct omap_dss_device *dssdev);
> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c
> b/drivers/gpu/drm/omapdrm/omap_plane.c index d75b197eff46..0147e416140c
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
> @@ -75,6 +75,28 @@ static void omap_plane_cleanup_fb(struct drm_plane
> *plane, omap_framebuffer_unpin(old_state->fb);
>  }
> 
> +void omap_plane_update_fifo(struct drm_plane *plane)
> +{
> +	struct omap_plane *omap_plane = to_omap_plane(plane);
> +	struct drm_plane_state *state = plane->state;
> +	struct drm_device *dev = plane->dev;
> +	bool use_fifo_merge = false;
> +	u32 fifo_low, fifo_high;
> +	bool use_manual_update;
> +
> +	if (!dispc_ovl_enabled(omap_plane->id))
> +		return;

Given that this function is called right after dispc_ovl_enable(omap_plane-
>id, true), can this condition be true ?

> +	use_manual_update = omap_crtc_is_manual_updated(state->crtc);
> +
> +	dispc_ovl_compute_fifo_thresholds(omap_plane->id, &fifo_low, 
&fifo_high,
> +			use_fifo_merge, use_manual_update);

You can remove the use_fifo_merge variable and set the argument to false 
directly.

> +
> +	dev_dbg(dev->dev, "update fifo: %d %d", fifo_low, fifo_high);

The two variables are unsigned, you should use %u.

> +	dispc_ovl_set_fifo_threshold(omap_plane->id, fifo_low, fifo_high);

On a side note, shouldn't the dispc_ovl_compute_fifo_thresholds() and 
dispc_ovl_set_fifo_threshold() functions be merged into a single one as 
they're always called together ?

> +}
> +
>  static void omap_plane_atomic_update(struct drm_plane *plane,
>  				     struct drm_plane_state *old_state)
>  {
> @@ -141,6 +163,7 @@ static void omap_plane_atomic_update(struct drm_plane
> *plane, }
> 
>  	dispc_ovl_enable(omap_plane->id, true);
> +	omap_plane_update_fifo(plane);
>  }
> 
>  static void omap_plane_atomic_disable(struct drm_plane *plane,

-- 
Regards,

Laurent Pinchart

Re: [PATCH 12/23] drm: omapdrm: plane: update fifo size on atomic update

[Laurent Pinchart]

Hi Sebastian,

Thank you for the patch.

On Tuesday 08 Mar 2016 17:39:44 Sebastian Reichel wrote:
> This is a workaround for a hardware bug occuring
> on OMAP3 with manually updated panels.

Could you please explain what the bug is and how the workaround operates ? Do 
you have a reference to an errata document ?

> Signed-off-By: Sebastian Reichel <sre@kernel.org>
> ---
>  drivers/gpu/drm/omapdrm/omap_drv.h   |  1 +
>  drivers/gpu/drm/omapdrm/omap_plane.c | 23 +++++++++++++++++++++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h
> b/drivers/gpu/drm/omapdrm/omap_drv.h index 71e2c2284b86..3ab4919aff4b
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.h
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.h
> @@ -161,6 +161,7 @@ struct drm_plane *omap_plane_init(struct drm_device
> *dev, int id, enum drm_plane_type type);
>  void omap_plane_install_properties(struct drm_plane *plane,
>  		struct drm_mode_object *obj);
> +void omap_plane_update_fifo(struct drm_plane *plane);
> 
>  struct drm_encoder *omap_encoder_init(struct drm_device *dev,
>  		struct omap_dss_device *dssdev);
> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c
> b/drivers/gpu/drm/omapdrm/omap_plane.c index d75b197eff46..0147e416140c
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
> @@ -75,6 +75,28 @@ static void omap_plane_cleanup_fb(struct drm_plane
> *plane, omap_framebuffer_unpin(old_state->fb);
>  }
> 
> +void omap_plane_update_fifo(struct drm_plane *plane)
> +{
> +	struct omap_plane *omap_plane = to_omap_plane(plane);
> +	struct drm_plane_state *state = plane->state;
> +	struct drm_device *dev = plane->dev;
> +	bool use_fifo_merge = false;
> +	u32 fifo_low, fifo_high;
> +	bool use_manual_update;
> +
> +	if (!dispc_ovl_enabled(omap_plane->id))
> +		return;

Given that this function is called right after dispc_ovl_enable(omap_plane-
>id, true), can this condition be true ?

> +	use_manual_update = omap_crtc_is_manual_updated(state->crtc);
> +
> +	dispc_ovl_compute_fifo_thresholds(omap_plane->id, &fifo_low, 
&fifo_high,
> +			use_fifo_merge, use_manual_update);

You can remove the use_fifo_merge variable and set the argument to false 
directly.

> +
> +	dev_dbg(dev->dev, "update fifo: %d %d", fifo_low, fifo_high);

The two variables are unsigned, you should use %u.

> +	dispc_ovl_set_fifo_threshold(omap_plane->id, fifo_low, fifo_high);

On a side note, shouldn't the dispc_ovl_compute_fifo_thresholds() and 
dispc_ovl_set_fifo_threshold() functions be merged into a single one as 
they're always called together ?

> +}
> +
>  static void omap_plane_atomic_update(struct drm_plane *plane,
>  				     struct drm_plane_state *old_state)
>  {
> @@ -141,6 +163,7 @@ static void omap_plane_atomic_update(struct drm_plane
> *plane, }
> 
>  	dispc_ovl_enable(omap_plane->id, true);
> +	omap_plane_update_fifo(plane);
>  }
> 
>  static void omap_plane_atomic_disable(struct drm_plane *plane,

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 12/23] drm: omapdrm: plane: update fifo size on atomic update

[Tomi Valkeinen]

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

On 13/12/16 19:35, Laurent Pinchart wrote:
> Hi Sebastian,
> 
> Thank you for the patch.
> 
> On Tuesday 08 Mar 2016 17:39:44 Sebastian Reichel wrote:
>> This is a workaround for a hardware bug occuring
>> on OMAP3 with manually updated panels.
> 
> Could you please explain what the bug is and how the workaround operates ? Do 
> you have a reference to an errata document ?

I don't think I ever found out exactly why the problem happens. But on
OMAP3 DSI, the fifo thresholds had to be tuned slightly, otherwise DISPC
would stop. dispc_ovl_compute_fifo_thresholds() does that tuning if
"manual_update" parameter is set on OMAP3.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 12/23] drm: omapdrm: plane: update fifo size on atomic update

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 659 bytes --]

On 13/12/16 19:35, Laurent Pinchart wrote:
> Hi Sebastian,
> 
> Thank you for the patch.
> 
> On Tuesday 08 Mar 2016 17:39:44 Sebastian Reichel wrote:
>> This is a workaround for a hardware bug occuring
>> on OMAP3 with manually updated panels.
> 
> Could you please explain what the bug is and how the workaround operates ? Do 
> you have a reference to an errata document ?

I don't think I ever found out exactly why the problem happens. But on
OMAP3 DSI, the fifo thresholds had to be tuned slightly, otherwise DISPC
would stop. dispc_ovl_compute_fifo_thresholds() does that tuning if
"manual_update" parameter is set on OMAP3.

 Tomi


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

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

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

Re: [PATCH 12/23] drm: omapdrm: plane: update fifo size on atomic update

[Laurent Pinchart]

Hi Tomi,

On Wednesday 14 Dec 2016 10:43:18 Tomi Valkeinen wrote:
> On 13/12/16 19:35, Laurent Pinchart wrote:
> > On Tuesday 08 Mar 2016 17:39:44 Sebastian Reichel wrote:
> >> This is a workaround for a hardware bug occuring
> >> on OMAP3 with manually updated panels.
> > 
> > Could you please explain what the bug is and how the workaround operates ?
> > Do you have a reference to an errata document ?
> 
> I don't think I ever found out exactly why the problem happens. But on
> OMAP3 DSI, the fifo thresholds had to be tuned slightly, otherwise DISPC
> would stop. dispc_ovl_compute_fifo_thresholds() does that tuning if
> "manual_update" parameter is set on OMAP3.

I've had a look at dispc_ovl_compute_fifo_thresholds() and the patch makes 
sense to me. If Sebastian could address the small issues I pointed out, we 
could then merge this. Alternatively I can take care of addressing them.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 12/23] drm: omapdrm: plane: update fifo size on atomic update

[Laurent Pinchart]

Hi Tomi,

On Wednesday 14 Dec 2016 10:43:18 Tomi Valkeinen wrote:
> On 13/12/16 19:35, Laurent Pinchart wrote:
> > On Tuesday 08 Mar 2016 17:39:44 Sebastian Reichel wrote:
> >> This is a workaround for a hardware bug occuring
> >> on OMAP3 with manually updated panels.
> > 
> > Could you please explain what the bug is and how the workaround operates ?
> > Do you have a reference to an errata document ?
> 
> I don't think I ever found out exactly why the problem happens. But on
> OMAP3 DSI, the fifo thresholds had to be tuned slightly, otherwise DISPC
> would stop. dispc_ovl_compute_fifo_thresholds() does that tuning if
> "manual_update" parameter is set on OMAP3.

I've had a look at dispc_ovl_compute_fifo_thresholds() and the patch makes 
sense to me. If Sebastian could address the small issues I pointed out, we 
could then merge this. Alternatively I can take care of addressing them.

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 12/23] drm: omapdrm: plane: update fifo size on atomic update

[Tomi Valkeinen]

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

On 14/12/16 11:10, Laurent Pinchart wrote:
> Hi Tomi,
> 
> On Wednesday 14 Dec 2016 10:43:18 Tomi Valkeinen wrote:
>> On 13/12/16 19:35, Laurent Pinchart wrote:
>>> On Tuesday 08 Mar 2016 17:39:44 Sebastian Reichel wrote:
>>>> This is a workaround for a hardware bug occuring
>>>> on OMAP3 with manually updated panels.
>>>
>>> Could you please explain what the bug is and how the workaround operates ?
>>> Do you have a reference to an errata document ?
>>
>> I don't think I ever found out exactly why the problem happens. But on
>> OMAP3 DSI, the fifo thresholds had to be tuned slightly, otherwise DISPC
>> would stop. dispc_ovl_compute_fifo_thresholds() does that tuning if
>> "manual_update" parameter is set on OMAP3.
> 
> I've had a look at dispc_ovl_compute_fifo_thresholds() and the patch makes 
> sense to me. If Sebastian could address the small issues I pointed out, we 
> could then merge this. Alternatively I can take care of addressing them.

It's only needed with the rest of the DSI manual update series, so I'd
rather keep it as part of that series.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 12/23] drm: omapdrm: plane: update fifo size on atomic update

[Tomi Valkeinen]

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

On 14/12/16 11:10, Laurent Pinchart wrote:
> Hi Tomi,
> 
> On Wednesday 14 Dec 2016 10:43:18 Tomi Valkeinen wrote:
>> On 13/12/16 19:35, Laurent Pinchart wrote:
>>> On Tuesday 08 Mar 2016 17:39:44 Sebastian Reichel wrote:
>>>> This is a workaround for a hardware bug occuring
>>>> on OMAP3 with manually updated panels.
>>>
>>> Could you please explain what the bug is and how the workaround operates ?
>>> Do you have a reference to an errata document ?
>>
>> I don't think I ever found out exactly why the problem happens. But on
>> OMAP3 DSI, the fifo thresholds had to be tuned slightly, otherwise DISPC
>> would stop. dispc_ovl_compute_fifo_thresholds() does that tuning if
>> "manual_update" parameter is set on OMAP3.
> 
> I've had a look at dispc_ovl_compute_fifo_thresholds() and the patch makes 
> sense to me. If Sebastian could address the small issues I pointed out, we 
> could then merge this. Alternatively I can take care of addressing them.

It's only needed with the rest of the DSI manual update series, so I'd
rather keep it as part of that series.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 12/23] drm: omapdrm: plane: update fifo size on atomic update

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1736 bytes --]

Hi,

On Wed, Dec 14, 2016 at 11:14:32AM +0200, Tomi Valkeinen wrote:
> On 14/12/16 11:10, Laurent Pinchart wrote:
> > Hi Tomi,
> > 
> > On Wednesday 14 Dec 2016 10:43:18 Tomi Valkeinen wrote:
> >> On 13/12/16 19:35, Laurent Pinchart wrote:
> >>> On Tuesday 08 Mar 2016 17:39:44 Sebastian Reichel wrote:
> >>>> This is a workaround for a hardware bug occuring
> >>>> on OMAP3 with manually updated panels.
> >>>
> >>> Could you please explain what the bug is and how the workaround operates ?
> >>> Do you have a reference to an errata document ?

FWIW I don't know anything about this bug. I just hit it while
getting omapdrm working on n950 and ported this over from omapfb.

> >> I don't think I ever found out exactly why the problem happens. But on
> >> OMAP3 DSI, the fifo thresholds had to be tuned slightly, otherwise DISPC
> >> would stop. dispc_ovl_compute_fifo_thresholds() does that tuning if
> >> "manual_update" parameter is set on OMAP3.
> > 
> > I've had a look at dispc_ovl_compute_fifo_thresholds() and the patch makes 
> > sense to me. If Sebastian could address the small issues I pointed out, we 
> > could then merge this. Alternatively I can take care of addressing them.
> 
> It's only needed with the rest of the DSI manual update series, so I'd
> rather keep it as part of that series.

To be honest I haven't worked on this for some time. From some
comments on the patchset I think the biggest issue is, that omapdrm
does not use generic panel drivers and cannot easily use the
mipi_dsi_driver_register. I simply did not have enough time to
implement such a huge change.

I guess a first step is Peter Ujfalusi's series:
https://lkml.org/lkml/2016/9/1/267

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 12/23] drm: omapdrm: plane: update fifo size on atomic update

[Laurent Pinchart]

Hi Sebastian,

(CC'ing Peter Ujfalusi)

On Wednesday 14 Dec 2016 15:03:08 Sebastian Reichel wrote:
> On Wed, Dec 14, 2016 at 11:14:32AM +0200, Tomi Valkeinen wrote:
> > On 14/12/16 11:10, Laurent Pinchart wrote:
> >> On Wednesday 14 Dec 2016 10:43:18 Tomi Valkeinen wrote:
> >>> On 13/12/16 19:35, Laurent Pinchart wrote:
> >>>> On Tuesday 08 Mar 2016 17:39:44 Sebastian Reichel wrote:
> >>>>> This is a workaround for a hardware bug occuring
> >>>>> on OMAP3 with manually updated panels.
> >>>> 
> >>>> Could you please explain what the bug is and how the workaround
> >>>> operates ? Do you have a reference to an errata document ?
> 
> FWIW I don't know anything about this bug. I just hit it while
> getting omapdrm working on n950 and ported this over from omapfb.

I wonder if it's a bug or an expected behaviour. In any case, looking at the 
FIFO thresholds computation function it's quite clear that we need to 
recompute and set the values when the panel mode changes. Maybe you should 
explain this in the commit message.

> >>> I don't think I ever found out exactly why the problem happens. But on
> >>> OMAP3 DSI, the fifo thresholds had to be tuned slightly, otherwise
> >>> DISPC would stop. dispc_ovl_compute_fifo_thresholds() does that tuning
> >>> if "manual_update" parameter is set on OMAP3.
> >> 
> >> I've had a look at dispc_ovl_compute_fifo_thresholds() and the patch
> >> makes sense to me. If Sebastian could address the small issues I pointed
> >> out, we could then merge this. Alternatively I can take care of
> >> addressing them.
> > 
> > It's only needed with the rest of the DSI manual update series, so I'd
> > rather keep it as part of that series.
> 
> To be honest I haven't worked on this for some time. From some
> comments on the patchset I think the biggest issue is, that omapdrm
> does not use generic panel drivers and cannot easily use the
> mipi_dsi_driver_register. I simply did not have enough time to
> implement such a huge change.

I'll probably give it a go at some point, but it will take time.

> I guess a first step is Peter Ujfalusi's series:
> https://lkml.org/lkml/2016/9/1/267

Peter, do you plan to respin that patch series ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH 12/23] drm: omapdrm: plane: update fifo size on atomic update

[Tomi Valkeinen]

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

On 14/12/16 23:36, Laurent Pinchart wrote:

>> I guess a first step is Peter Ujfalusi's series:
>> https://lkml.org/lkml/2016/9/1/267
> 
> Peter, do you plan to respin that patch series ?

That series has been merged already.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 12/23] drm: omapdrm: plane: update fifo size on atomic update

[Tomi Valkeinen]

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

On 14/12/16 23:36, Laurent Pinchart wrote:

>> I guess a first step is Peter Ujfalusi's series:
>> https://lkml.org/lkml/2016/9/1/267
> 
> Peter, do you plan to respin that patch series ?

That series has been merged already.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH 13/23] drm: omapdrm: crtc: update plane fifos on lcd config change

[Sebastian Reichel]

Due to a hardware bug, FIFOs thresholds must be
configured very carefully for manually updated
displays.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 8967013c1fb5..094e89a2fa94 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -253,6 +253,10 @@ static void omap_crtc_dss_set_lcd_config(struct omap_overlay_manager *mgr,
 	omap_crtc->manually_updated = dss_lcd_mgr_config_get_stallmode(config);
 
 	dispc_mgr_set_lcd_config(omap_crtc->channel, config);
+
+	drm_for_each_plane(plane, dev) {
+		omap_plane_update_fifo(plane);
+	}
 }
 
 static int omap_crtc_dss_register_framedone(
-- 
2.7.0

Re: [PATCH 13/23] drm: omapdrm: crtc: update plane fifos on lcd config change

[Laurent Pinchart]

Hi Sebastian,

Thank you for the patch.

On Tuesday 08 Mar 2016 17:39:45 Sebastian Reichel wrote:
> Due to a hardware bug, FIFOs thresholds must be
> configured very carefully for manually updated
> displays.

Could you please provide more information about the bug and how the code 
handles it ?

> Signed-off-by: Sebastian Reichel <sre@kernel.org>
> ---
>  drivers/gpu/drm/omapdrm/omap_crtc.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c index 8967013c1fb5..094e89a2fa94
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -253,6 +253,10 @@ static void omap_crtc_dss_set_lcd_config(struct
> omap_overlay_manager *mgr, omap_crtc->manually_updated =
> dss_lcd_mgr_config_get_stallmode(config);
> 
>  	dispc_mgr_set_lcd_config(omap_crtc->channel, config);
> +
> +	drm_for_each_plane(plane, dev) {
> +		omap_plane_update_fifo(plane);
> +	}

This seems fishy :-/ To start with you shouldn't touch planes that don't 
belong to this CRTC. Then, updating the FIFO thresholds here in addition to 
omap_plane_atomic_update() makes me wonder if both are needed, and if so, why.

>  }
> 
>  static int omap_crtc_dss_register_framedone(

-- 
Regards,

Laurent Pinchart

Re: [PATCH 13/23] drm: omapdrm: crtc: update plane fifos on lcd config change

[Laurent Pinchart]

Hi Sebastian,

Thank you for the patch.

On Tuesday 08 Mar 2016 17:39:45 Sebastian Reichel wrote:
> Due to a hardware bug, FIFOs thresholds must be
> configured very carefully for manually updated
> displays.

Could you please provide more information about the bug and how the code 
handles it ?

> Signed-off-by: Sebastian Reichel <sre@kernel.org>
> ---
>  drivers/gpu/drm/omapdrm/omap_crtc.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c index 8967013c1fb5..094e89a2fa94
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -253,6 +253,10 @@ static void omap_crtc_dss_set_lcd_config(struct
> omap_overlay_manager *mgr, omap_crtc->manually_updated =
> dss_lcd_mgr_config_get_stallmode(config);
> 
>  	dispc_mgr_set_lcd_config(omap_crtc->channel, config);
> +
> +	drm_for_each_plane(plane, dev) {
> +		omap_plane_update_fifo(plane);
> +	}

This seems fishy :-/ To start with you shouldn't touch planes that don't 
belong to this CRTC. Then, updating the FIFO thresholds here in addition to 
omap_plane_atomic_update() makes me wonder if both are needed, and if so, why.

>  }
> 
>  static int omap_crtc_dss_register_framedone(

-- 
Regards,

Laurent Pinchart

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

[PATCH 14/23] drm: omapdrm: crtc: save framedone callback from dss

[Sebastian Reichel]

Save the framedone callback supplied by dss for later
usage.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 094e89a2fa94..3ce7143e5a5f 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -57,6 +57,9 @@ struct omap_crtc {
 
 	unsigned long state;
 	wait_queue_head_t pending_wait;
+
+	void (*framedone_handler)(void *);
+	void *framedone_handler_data;
 };
 
 /* -----------------------------------------------------------------------------
@@ -263,6 +266,17 @@ static int omap_crtc_dss_register_framedone(
 		struct omap_overlay_manager *mgr,
 		void (*handler)(void *), void *data)
 {
+	struct omap_crtc *omap_crtc = omap_crtcs[mgr->id];
+	struct drm_device *dev = omap_crtc->base.dev;
+
+	if (omap_crtc->framedone_handler)
+		return -EBUSY;
+
+	dev_dbg(dev->dev, "register framedone %s", omap_crtc->name);
+
+	omap_crtc->framedone_handler = handler;
+	omap_crtc->framedone_handler_data = data;
+
 	return 0;
 }
 
@@ -270,6 +284,16 @@ static void omap_crtc_dss_unregister_framedone(
 		struct omap_overlay_manager *mgr,
 		void (*handler)(void *), void *data)
 {
+	struct omap_crtc *omap_crtc = omap_crtcs[mgr->id];
+	struct drm_device *dev = omap_crtc->base.dev;
+
+	dev_dbg(dev->dev, "unregister framedone %s", omap_crtc->name);
+
+	WARN_ON(omap_crtc->framedone_handler != handler);
+	WARN_ON(omap_crtc->framedone_handler_data != data);
+
+	omap_crtc->framedone_handler = NULL;
+	omap_crtc->framedone_handler_data = NULL;
 }
 
 static const struct dss_mgr_ops mgr_ops = {
-- 
2.7.0

Re: [PATCH 14/23] drm: omapdrm: crtc: save framedone callback from dss

[Laurent Pinchart]

Hi Sebastian,

Thank you for the patch.

On Tuesday 08 Mar 2016 17:39:46 Sebastian Reichel wrote:
> Save the framedone callback supplied by dss for later
> usage.

We already have too many callbacks in the driver, making the code difficult to 
understand. Wouldn't it be possible to cal directly from the omapdrm driver 
into the dss code without using callbacks ?

> Signed-off-by: Sebastian Reichel <sre@kernel.org>
> ---
>  drivers/gpu/drm/omapdrm/omap_crtc.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c index 094e89a2fa94..3ce7143e5a5f
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -57,6 +57,9 @@ struct omap_crtc {
> 
>  	unsigned long state;
>  	wait_queue_head_t pending_wait;
> +
> +	void (*framedone_handler)(void *);
> +	void *framedone_handler_data;
>  };
> 
>  /*
> ---------------------------------------------------------------------------
> -- @@ -263,6 +266,17 @@ static int omap_crtc_dss_register_framedone(
>  		struct omap_overlay_manager *mgr,
>  		void (*handler)(void *), void *data)
>  {
> +	struct omap_crtc *omap_crtc = omap_crtcs[mgr->id];
> +	struct drm_device *dev = omap_crtc->base.dev;
> +
> +	if (omap_crtc->framedone_handler)
> +		return -EBUSY;
> +
> +	dev_dbg(dev->dev, "register framedone %s", omap_crtc->name);
> +
> +	omap_crtc->framedone_handler = handler;
> +	omap_crtc->framedone_handler_data = data;
> +
>  	return 0;
>  }
> 
> @@ -270,6 +284,16 @@ static void omap_crtc_dss_unregister_framedone(
>  		struct omap_overlay_manager *mgr,
>  		void (*handler)(void *), void *data)
>  {
> +	struct omap_crtc *omap_crtc = omap_crtcs[mgr->id];
> +	struct drm_device *dev = omap_crtc->base.dev;
> +
> +	dev_dbg(dev->dev, "unregister framedone %s", omap_crtc->name);
> +
> +	WARN_ON(omap_crtc->framedone_handler != handler);
> +	WARN_ON(omap_crtc->framedone_handler_data != data);
> +
> +	omap_crtc->framedone_handler = NULL;
> +	omap_crtc->framedone_handler_data = NULL;
>  }
> 
>  static const struct dss_mgr_ops mgr_ops = {

-- 
Regards,

Laurent Pinchart

Re: [PATCH 14/23] drm: omapdrm: crtc: save framedone callback from dss

[Laurent Pinchart]

Hi Sebastian,

Thank you for the patch.

On Tuesday 08 Mar 2016 17:39:46 Sebastian Reichel wrote:
> Save the framedone callback supplied by dss for later
> usage.

We already have too many callbacks in the driver, making the code difficult to 
understand. Wouldn't it be possible to cal directly from the omapdrm driver 
into the dss code without using callbacks ?

> Signed-off-by: Sebastian Reichel <sre@kernel.org>
> ---
>  drivers/gpu/drm/omapdrm/omap_crtc.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c index 094e89a2fa94..3ce7143e5a5f
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -57,6 +57,9 @@ struct omap_crtc {
> 
>  	unsigned long state;
>  	wait_queue_head_t pending_wait;
> +
> +	void (*framedone_handler)(void *);
> +	void *framedone_handler_data;
>  };
> 
>  /*
> ---------------------------------------------------------------------------
> -- @@ -263,6 +266,17 @@ static int omap_crtc_dss_register_framedone(
>  		struct omap_overlay_manager *mgr,
>  		void (*handler)(void *), void *data)
>  {
> +	struct omap_crtc *omap_crtc = omap_crtcs[mgr->id];
> +	struct drm_device *dev = omap_crtc->base.dev;
> +
> +	if (omap_crtc->framedone_handler)
> +		return -EBUSY;
> +
> +	dev_dbg(dev->dev, "register framedone %s", omap_crtc->name);
> +
> +	omap_crtc->framedone_handler = handler;
> +	omap_crtc->framedone_handler_data = data;
> +
>  	return 0;
>  }
> 
> @@ -270,6 +284,16 @@ static void omap_crtc_dss_unregister_framedone(
>  		struct omap_overlay_manager *mgr,
>  		void (*handler)(void *), void *data)
>  {
> +	struct omap_crtc *omap_crtc = omap_crtcs[mgr->id];
> +	struct drm_device *dev = omap_crtc->base.dev;
> +
> +	dev_dbg(dev->dev, "unregister framedone %s", omap_crtc->name);
> +
> +	WARN_ON(omap_crtc->framedone_handler != handler);
> +	WARN_ON(omap_crtc->framedone_handler_data != data);
> +
> +	omap_crtc->framedone_handler = NULL;
> +	omap_crtc->framedone_handler_data = NULL;
>  }
> 
>  static const struct dss_mgr_ops mgr_ops = {

-- 
Regards,

Laurent Pinchart

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

[PATCH 15/23] drm: omapdrm: crtc: add support for manual updated displays

[Sebastian Reichel]

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 91 +++++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/omapdrm/omap_drv.h  |  2 +
 2 files changed, 90 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 3ce7143e5a5f..10d2647965c2 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -51,12 +51,14 @@ struct omap_crtc {
 	bool manually_updated;
 
 	struct omap_drm_irq vblank_irq;
+	struct omap_drm_irq framedone_irq;
 	struct omap_drm_irq error_irq;
 
 	bool ignore_digit_sync_lost;
 
 	unsigned long state;
 	wait_queue_head_t pending_wait;
+	struct delayed_work update_work;
 
 	void (*framedone_handler)(void *);
 	void *framedone_handler_data;
@@ -91,7 +93,7 @@ int omap_crtc_wait_pending(struct drm_crtc *crtc)
 
 	return wait_event_timeout(omap_crtc->pending_wait,
 				  !test_bit(crtc_pending, &omap_crtc->state),
-				  msecs_to_jiffies(50));
+				  msecs_to_jiffies(250));
 }
 
 bool omap_crtc_is_manual_updated(struct drm_crtc *crtc)
@@ -141,6 +143,15 @@ static void omap_crtc_dss_disconnect(struct omap_overlay_manager *mgr,
 
 static void omap_crtc_dss_start_update(struct omap_overlay_manager *mgr)
 {
+	struct omap_crtc *omap_crtc = omap_crtcs[mgr->id];
+	struct drm_device *dev = omap_crtc->base.dev;
+	enum omap_channel channel = omap_crtc->channel;
+
+	WARN_ON(dispc_mgr_is_enabled(channel));
+
+	omap_irq_register(dev, &omap_crtc->framedone_irq);
+
+	dispc_mgr_enable(channel, true);
 }
 
 /* Called only from the encoder enable/disable and suspend/resume handlers. */
@@ -209,8 +220,11 @@ static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
 static int omap_crtc_dss_enable(struct omap_overlay_manager *mgr)
 {
 	struct omap_crtc *omap_crtc = omap_crtcs[mgr->id];
+	struct drm_device *dev = omap_crtc->base.dev;
 	struct omap_overlay_manager_info info;
 
+	dev_dbg(dev->dev, "crtc dss enable %s", omap_crtc->name);
+
 	memset(&info, 0, sizeof(info));
 	info.default_color = 0x00000000;
 	info.trans_key = 0x00000000;
@@ -230,6 +244,9 @@ static int omap_crtc_dss_enable(struct omap_overlay_manager *mgr)
 static void omap_crtc_dss_disable(struct omap_overlay_manager *mgr)
 {
 	struct omap_crtc *omap_crtc = omap_crtcs[mgr->id];
+	struct drm_device *dev = omap_crtc->base.dev;
+
+	dev_dbg(dev->dev, "crtc dss disable %s", omap_crtc->name);
 
 	clear_bit(crtc_enabled, &omap_crtc->state);
 
@@ -376,6 +393,63 @@ static void omap_crtc_vblank_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
 	wake_up(&omap_crtc->pending_wait);
 }
 
+static void omap_crtc_framedone_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
+{
+	struct omap_crtc *omap_crtc =
+			container_of(irq, struct omap_crtc, framedone_irq);
+	struct drm_device *dev = omap_crtc->base.dev;
+
+	if (omap_crtc->framedone_handler)
+		omap_crtc->framedone_handler(omap_crtc->framedone_handler_data);
+
+	__omap_irq_unregister(dev, &omap_crtc->framedone_irq);
+
+	clear_bit(crtc_pending, &omap_crtc->state);
+	wake_up(&omap_crtc->pending_wait);
+}
+
+void omap_crtc_flush(struct drm_crtc *crtc,
+		int x, int y, int w, int h)
+{
+	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+
+	if (!omap_crtc->manually_updated)
+		return;
+
+	if (!test_bit(crtc_enabled, &omap_crtc->state))
+		return;
+
+	if (!delayed_work_pending(&omap_crtc->update_work))
+		schedule_delayed_work(&omap_crtc->update_work, 0);
+}
+
+static void omap_crtc_manual_display_update(struct work_struct *data)
+{
+	struct omap_crtc *omap_crtc = container_of(data, struct omap_crtc,
+					update_work.work);
+	struct omap_dss_device *dssdev = omap_crtc->mgr->output->dst;
+	struct omap_dss_driver *dssdrv = dssdev->driver;
+	int ret;
+
+	if (!dssdrv || !dssdrv->update)
+		return;
+
+	if (!test_bit(crtc_enabled, &omap_crtc->state))
+		return;
+
+	if (test_and_set_bit(crtc_pending, &omap_crtc->state))
+		return;
+
+	if (dssdrv->sync)
+		dssdrv->sync(dssdev);
+
+	ret = dssdrv->update(dssdev, 0, 0, omap_crtc->timings.x_res, omap_crtc->timings.y_res);
+	if (ret < 0) {
+		clear_bit(crtc_pending, &omap_crtc->state);
+		wake_up(&omap_crtc->pending_wait);
+	}
+}
+
 /* -----------------------------------------------------------------------------
  * CRTC Functions
  */
@@ -407,7 +481,7 @@ static void omap_crtc_enable(struct drm_crtc *crtc)
 	struct omap_dss_device *display = omap_crtc->mgr->output->dst;
 	struct drm_device *dev = crtc->dev;
 
-	DBG("%s", omap_crtc->name);
+	dev_dbg(dev->dev, "enable crtc %s", omap_crtc->name);
 
 	/* manual updated display will not trigger vsync irq */
 	/* omap_crtc->manually_updated is not yet set */
@@ -427,8 +501,14 @@ static void omap_crtc_enable(struct drm_crtc *crtc)
 static void omap_crtc_disable(struct drm_crtc *crtc)
 {
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct drm_device *dev = crtc->dev;
 
-	DBG("%s", omap_crtc->name);
+	dev_dbg(dev->dev, "disable crtc %s", omap_crtc->name);
+
+	cancel_delayed_work(&omap_crtc->update_work);
+
+	if (!omap_crtc_wait_pending(crtc))
+		dev_warn(dev->dev, "manual display update did not finish!");
 
 	drm_crtc_vblank_off(crtc);
 }
@@ -571,6 +651,8 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 
 	init_waitqueue_head(&omap_crtc->pending_wait);
 
+	INIT_DELAYED_WORK(&omap_crtc->update_work, omap_crtc_manual_display_update);
+
 	omap_crtc->state = 0;
 
 	omap_crtc->channel = channel;
@@ -579,6 +661,9 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 	omap_crtc->vblank_irq.irqmask = pipe2vbl(crtc);
 	omap_crtc->vblank_irq.irq = omap_crtc_vblank_irq;
 
+	omap_crtc->framedone_irq.irqmask = dispc_mgr_get_framedone_irq(omap_crtc->channel);
+	omap_crtc->framedone_irq.irq = omap_crtc_framedone_irq;
+
 	omap_crtc->error_irq.irqmask =
 			dispc_mgr_get_sync_lost_irq(channel);
 	omap_crtc->error_irq.irq = omap_crtc_error_irq;
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h
index 3ab4919aff4b..e08f4ef1bf3c 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -174,6 +174,8 @@ struct drm_encoder *omap_connector_attached_encoder(
 bool omap_connector_get_hdmi_mode(struct drm_connector *connector);
 void omap_connector_flush(struct drm_connector *connector,
 		int x, int y, int w, int h);
+void omap_crtc_flush(struct drm_crtc *crtc,
+		int x, int y, int w, int h);
 
 void copy_timings_omap_to_drm(struct drm_display_mode *mode,
 		struct omap_video_timings *timings);
-- 
2.7.0

[PATCH 16/23] drm: omapdrm: update manual displays on dirty ioctl

[Sebastian Reichel]

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/gpu/drm/omapdrm/omap_connector.c | 12 ------------
 drivers/gpu/drm/omapdrm/omap_drv.h       |  2 --
 drivers/gpu/drm/omapdrm/omap_fb.c        |  2 +-
 3 files changed, 1 insertion(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
index 8db36aa4bd00..83f2a9177c14 100644
--- a/drivers/gpu/drm/omapdrm/omap_connector.c
+++ b/drivers/gpu/drm/omapdrm/omap_connector.c
@@ -275,18 +275,6 @@ static const struct drm_connector_helper_funcs omap_connector_helper_funcs = {
 	.best_encoder = omap_connector_attached_encoder,
 };
 
-/* flush an area of the framebuffer (in case of manual update display that
- * is not automatically flushed)
- */
-void omap_connector_flush(struct drm_connector *connector,
-		int x, int y, int w, int h)
-{
-	struct omap_connector *omap_connector = to_omap_connector(connector);
-
-	/* TODO: enable when supported in dss */
-	VERB("%s: %d,%d, %dx%d", omap_connector->dssdev->name, x, y, w, h);
-}
-
 /* initialize connector */
 struct drm_connector *omap_connector_init(struct drm_device *dev,
 		int connector_type, struct omap_dss_device *dssdev,
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h
index e08f4ef1bf3c..e536a8dbb51b 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -172,8 +172,6 @@ struct drm_connector *omap_connector_init(struct drm_device *dev,
 struct drm_encoder *omap_connector_attached_encoder(
 		struct drm_connector *connector);
 bool omap_connector_get_hdmi_mode(struct drm_connector *connector);
-void omap_connector_flush(struct drm_connector *connector,
-		int x, int y, int w, int h);
 void omap_crtc_flush(struct drm_crtc *crtc,
 		int x, int y, int w, int h);
 
diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c
index b51d2243f356..ee61a34bab26 100644
--- a/drivers/gpu/drm/omapdrm/omap_fb.c
+++ b/drivers/gpu/drm/omapdrm/omap_fb.c
@@ -378,7 +378,7 @@ void omap_framebuffer_flush(struct drm_framebuffer *fb,
 			int cw = w + (x - crtc->x) - cx;
 			int ch = h + (y - crtc->y) - cy;
 
-			omap_connector_flush(connector, cx, cy, cw, ch);
+			omap_crtc_flush(crtc, cx, cy, cw, ch);
 		}
 	}
 }
-- 
2.7.0

[PATCH 17/23] drm: omapdrm: panel-dsi-cm: add regulator support

[Sebastian Reichel]

The N950's display requires two regulators.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c | 57 +++++++++++++++++++++++--
 1 file changed, 53 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
index 3414c2609320..768e92d1ec8b 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
@@ -24,6 +24,7 @@
 #include <linux/workqueue.h>
 #include <linux/of_device.h>
 #include <linux/of_gpio.h>
+#include <linux/regulator/consumer.h>
 
 #include <video/omapdss.h>
 #include <video/omap-panel-data.h>
@@ -60,6 +61,9 @@ struct panel_drv_data {
 	int reset_gpio;
 	int ext_te_gpio;
 
+	struct regulator *vpnl;
+	struct regulator *vddi;
+
 	bool use_dsi_backlight;
 
 	struct omap_dsi_pin_config pin_config;
@@ -597,25 +601,43 @@ static int dsicm_power_on(struct panel_drv_data *ddata)
 		.lp_clk_max = 10000000,
 	};
 
+	if (ddata->vpnl) {
+		r = regulator_enable(ddata->vpnl);
+		if (r) {
+			dev_err(&ddata->pdev->dev,
+				"failed to enable VPNL: %d\n", r);
+			goto err0;
+		}
+	}
+
+	if (ddata->vddi) {
+		r = regulator_enable(ddata->vddi);
+		if (r) {
+			dev_err(&ddata->pdev->dev,
+				"failed to enable VDDI: %d\n", r);
+			goto err1;
+		}
+	}
+
 	if (ddata->pin_config.num_pins > 0) {
 		r = in->ops.dsi->configure_pins(in, &ddata->pin_config);
 		if (r) {
 			dev_err(&ddata->pdev->dev,
 				"failed to configure DSI pins\n");
-			goto err0;
+			goto err2;
 		}
 	}
 
 	r = in->ops.dsi->set_config(in, &dsi_config);
 	if (r) {
 		dev_err(&ddata->pdev->dev, "failed to configure DSI\n");
-		goto err0;
+		goto err2;
 	}
 
 	r = in->ops.dsi->enable(in);
 	if (r) {
 		dev_err(&ddata->pdev->dev, "failed to enable DSI\n");
-		goto err0;
+		goto err2;
 	}
 
 	dsicm_hw_reset(ddata);
@@ -673,6 +695,12 @@ err:
 	dsicm_hw_reset(ddata);
 
 	in->ops.dsi->disable(in, true, false);
+err2:
+	if (ddata->vddi)
+		regulator_disable(ddata->vddi);
+err1:
+	if (ddata->vpnl)
+		regulator_disable(ddata->vpnl);
 err0:
 	return r;
 }
@@ -696,6 +724,11 @@ static void dsicm_power_off(struct panel_drv_data *ddata)
 
 	in->ops.dsi->disable(in, true, false);
 
+	if (ddata->vddi)
+		regulator_disable(ddata->vddi);
+	if (ddata->vpnl)
+		regulator_disable(ddata->vpnl);
+
 	ddata->enabled = 0;
 }
 
@@ -1166,7 +1199,7 @@ static int dsicm_probe_of(struct platform_device *pdev)
 	struct device_node *node = pdev->dev.of_node;
 	struct panel_drv_data *ddata = platform_get_drvdata(pdev);
 	struct omap_dss_device *in;
-	int gpio;
+	int gpio, err;
 
 	gpio = of_get_named_gpio(node, "reset-gpios", 0);
 	if (!gpio_is_valid(gpio)) {
@@ -1189,6 +1222,22 @@ static int dsicm_probe_of(struct platform_device *pdev)
 		return PTR_ERR(in);
 	}
 
+	ddata->vpnl = devm_regulator_get_optional(&pdev->dev, "vpnl");
+	if (IS_ERR(ddata->vpnl)) {
+		err = PTR_ERR(ddata->vpnl);
+		if (err == -EPROBE_DEFER)
+			return err;
+		ddata->vpnl = NULL;
+	}
+
+	ddata->vddi = devm_regulator_get_optional(&pdev->dev, "vddi");
+	if (IS_ERR(ddata->vddi)) {
+		err = PTR_ERR(ddata->vddi);
+		if (err == -EPROBE_DEFER)
+			return err;
+		ddata->vddi = NULL;
+	}
+
 	ddata->in = in;
 
 	/* TODO: ulps, backlight */
-- 
2.7.0

[PATCH 18/23] drm: omapdrm: panel-dsi-cm: use threaded irq handler

[Sebastian Reichel]

Use threaded irq handler for the tearing effect gpio,
since it updates the display content, which requires
too much time for a fastpath irq.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
index 768e92d1ec8b..8316b6c2d8aa 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
@@ -1317,9 +1317,9 @@ static int dsicm_probe(struct platform_device *pdev)
 			return r;
 		}
 
-		r = devm_request_irq(dev, gpio_to_irq(ddata->ext_te_gpio),
-				dsicm_te_isr,
-				IRQF_TRIGGER_RISING,
+		r = devm_request_threaded_irq(dev, gpio_to_irq(ddata->ext_te_gpio),
+				NULL, dsicm_te_isr,
+				IRQF_TRIGGER_RISING | IRQF_ONESHOT,
 				"taal vsync", ddata);
 
 		if (r) {
-- 
2.7.0

Re: [PATCH 18/23] drm: omapdrm: panel-dsi-cm: use threaded irq handler

[Tomi Valkeinen]

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


On 08/03/16 18:39, Sebastian Reichel wrote:
> Use threaded irq handler for the tearing effect gpio,
> since it updates the display content, which requires
> too much time for a fastpath irq.
> 
> Signed-off-by: Sebastian Reichel <sre@kernel.org>
> ---
>  drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> index 768e92d1ec8b..8316b6c2d8aa 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> @@ -1317,9 +1317,9 @@ static int dsicm_probe(struct platform_device *pdev)
>  			return r;
>  		}
>  
> -		r = devm_request_irq(dev, gpio_to_irq(ddata->ext_te_gpio),
> -				dsicm_te_isr,
> -				IRQF_TRIGGER_RISING,
> +		r = devm_request_threaded_irq(dev, gpio_to_irq(ddata->ext_te_gpio),
> +				NULL, dsicm_te_isr,
> +				IRQF_TRIGGER_RISING | IRQF_ONESHOT,
>  				"taal vsync", ddata);
>  
>  		if (r) {
> 

I don't have a problem with moving to threaded irq, but it does make me
wonder, what's happening there that's taking so much time? I think it
should be just writing the configs to the registers and starting the
transfer.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 18/23] drm: omapdrm: panel-dsi-cm: use threaded irq handler

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 1313 bytes --]


On 08/03/16 18:39, Sebastian Reichel wrote:
> Use threaded irq handler for the tearing effect gpio,
> since it updates the display content, which requires
> too much time for a fastpath irq.
> 
> Signed-off-by: Sebastian Reichel <sre@kernel.org>
> ---
>  drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> index 768e92d1ec8b..8316b6c2d8aa 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> @@ -1317,9 +1317,9 @@ static int dsicm_probe(struct platform_device *pdev)
>  			return r;
>  		}
>  
> -		r = devm_request_irq(dev, gpio_to_irq(ddata->ext_te_gpio),
> -				dsicm_te_isr,
> -				IRQF_TRIGGER_RISING,
> +		r = devm_request_threaded_irq(dev, gpio_to_irq(ddata->ext_te_gpio),
> +				NULL, dsicm_te_isr,
> +				IRQF_TRIGGER_RISING | IRQF_ONESHOT,
>  				"taal vsync", ddata);
>  
>  		if (r) {
> 

I don't have a problem with moving to threaded irq, but it does make me
wonder, what's happening there that's taking so much time? I think it
should be just writing the configs to the registers and starting the
transfer.

 Tomi


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

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

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

[PATCH 19/23] drm: omapdrm: panel-dsi-cm: improve DT support

[Sebastian Reichel]

Introduce new DT properties, so that it's possible
to use different panels with the driver.

 * resolution-x (default: 864)
 * resolution-y (default: 480)
 * has-dsi-backlight (default: no backlight)

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
index 8316b6c2d8aa..4c1e76da68cd 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
@@ -1240,7 +1240,12 @@ static int dsicm_probe_of(struct platform_device *pdev)
 
 	ddata->in = in;
 
-	/* TODO: ulps, backlight */
+	ddata->use_dsi_backlight = of_property_read_bool(node, "has-dsi-backlight");
+
+	of_property_read_u32(node, "resolution-x", (u32*) &ddata->timings.x_res);
+	of_property_read_u32(node, "resolution-y", (u32*) &ddata->timings.y_res);
+
+	ddata->timings.pixelclock = ddata->timings.x_res * ddata->timings.y_res * 60;
 
 	return 0;
 }
@@ -1263,6 +1268,12 @@ static int dsicm_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, ddata);
 	ddata->pdev = pdev;
 
+	/* setup defaults */
+	ddata->ulps_timeout = 500;
+	ddata->timings.x_res = 864;
+	ddata->timings.y_res = 480;
+	ddata->timings.pixelclock = 864 * 480 * 60;
+
 	if (dev_get_platdata(dev)) {
 		r = dsicm_probe_pdata(pdev);
 		if (r)
@@ -1275,10 +1286,6 @@ static int dsicm_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	ddata->timings.x_res = 864;
-	ddata->timings.y_res = 480;
-	ddata->timings.pixelclock = 864 * 480 * 60;
-
 	dssdev = &ddata->dssdev;
 	dssdev->dev = dev;
 	dssdev->driver = &dsicm_ops;
-- 
2.7.0

[PATCH 20/23] drm: omapdrm: panel-dsi-cm: add offset support

[Sebastian Reichel]

The Nokia N950 contains a panel, that is partially
covered, so that not the whole display is visible.
Thus the image must be displayed with an offset.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
index 4c1e76da68cd..c60bc3013ff4 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
@@ -45,6 +45,8 @@ struct panel_drv_data {
 	struct omap_dss_device *in;
 
 	struct omap_video_timings timings;
+	int offset_x;
+	int offset_y;
 
 	struct platform_device *pdev;
 
@@ -203,12 +205,17 @@ static int dsicm_set_update_window(struct panel_drv_data *ddata,
 {
 	struct omap_dss_device *in = ddata->in;
 	int r;
-	u16 x1 = x;
-	u16 x2 = x + w - 1;
-	u16 y1 = y;
-	u16 y2 = y + h - 1;
-
 	u8 buf[5];
+	u16 x1, x2, y1, y2;
+
+	x += ddata->offset_x;
+	y += ddata->offset_y;
+
+	x1 = x;
+	x2 = x + w - 1;
+	y1 = y;
+	y2 = y + h - 1;
+
 	buf[0] = MIPI_DCS_SET_COLUMN_ADDRESS;
 	buf[1] = (x1 >> 8) & 0xff;
 	buf[2] = (x1 >> 0) & 0xff;
@@ -1244,6 +1251,8 @@ static int dsicm_probe_of(struct platform_device *pdev)
 
 	of_property_read_u32(node, "resolution-x", (u32*) &ddata->timings.x_res);
 	of_property_read_u32(node, "resolution-y", (u32*) &ddata->timings.y_res);
+	of_property_read_u32(node, "offset-x", &ddata->offset_x);
+	of_property_read_u32(node, "offset-y", &ddata->offset_y);
 
 	ddata->timings.pixelclock = ddata->timings.x_res * ddata->timings.y_res * 60;
 
@@ -1273,6 +1282,8 @@ static int dsicm_probe(struct platform_device *pdev)
 	ddata->timings.x_res = 864;
 	ddata->timings.y_res = 480;
 	ddata->timings.pixelclock = 864 * 480 * 60;
+	ddata->offset_x = 0;
+	ddata->offset_y = 0;
 
 	if (dev_get_platdata(dev)) {
 		r = dsicm_probe_pdata(pdev);
-- 
2.7.0

[PATCH 21/23] drm: omapdrm: panel-dsi-cm: block disable until update completed

[Sebastian Reichel]

DSI hardware IPs seem to be a bit unhappy about
incomplete image transfers, so try to wait for
an update to finish before disabling the panel.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
index c60bc3013ff4..ca7fba59c760 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
@@ -78,6 +78,9 @@ struct panel_drv_data {
 	atomic_t do_update;
 	int channel;
 
+	wait_queue_head_t update_wait;
+	atomic_t updating;
+
 	struct delayed_work te_timeout_work;
 
 	bool intro_printed;
@@ -845,6 +848,13 @@ static void dsicm_disable(struct omap_dss_device *dssdev)
 
 	dev_dbg(&ddata->pdev->dev, "disable\n");
 
+	r = wait_event_timeout(ddata->update_wait,
+				!atomic_read(&ddata->updating),
+				msecs_to_jiffies(250));
+	if (!r) {
+		dev_warn(&ddata->pdev->dev, "update did not finish, force disable!");
+	}
+
 	mutex_lock(&ddata->lock);
 
 	dsicm_cancel_ulps_work(ddata);
@@ -871,6 +881,9 @@ static void dsicm_framedone_cb(int err, void *data)
 
 	dev_dbg(&ddata->pdev->dev, "framedone, err %d\n", err);
 	in->ops.dsi->bus_unlock(ddata->in);
+
+	atomic_set(&ddata->updating, 0);
+	wake_up(&ddata->update_wait);
 }
 
 static irqreturn_t dsicm_te_isr(int irq, void *data)
@@ -885,6 +898,7 @@ static irqreturn_t dsicm_te_isr(int irq, void *data)
 	if (old) {
 		cancel_delayed_work(&ddata->te_timeout_work);
 
+		atomic_set(&ddata->updating, 1);
 		r = in->ops.dsi->update(in, ddata->channel, dsicm_framedone_cb,
 				ddata);
 		if (r)
@@ -943,6 +957,7 @@ static int dsicm_update(struct omap_dss_device *dssdev,
 				msecs_to_jiffies(250));
 		atomic_set(&ddata->do_update, 1);
 	} else {
+		atomic_set(&ddata->updating, 1);
 		r = in->ops.dsi->update(in, ddata->channel, dsicm_framedone_cb,
 				ddata);
 		if (r)
@@ -1317,6 +1332,9 @@ static int dsicm_probe(struct platform_device *pdev)
 	mutex_init(&ddata->lock);
 
 	atomic_set(&ddata->do_update, 0);
+	atomic_set(&ddata->updating, 0);
+
+	init_waitqueue_head(&ddata->update_wait);
 
 	if (gpio_is_valid(ddata->reset_gpio)) {
 		r = devm_gpio_request_one(dev, ddata->reset_gpio,
-- 
2.7.0

[PATCH 22/23] drm: omapdrm: panel-dsi-cm: ratelimit debug output in update path

[Sebastian Reichel]

Generating debug messages during display update results
in lot's of messages, if the display shows the console.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
index ca7fba59c760..196fe8d18ff4 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
@@ -931,7 +931,7 @@ static int dsicm_update(struct omap_dss_device *dssdev,
 	struct omap_dss_device *in = ddata->in;
 	int r;
 
-	dev_dbg(&ddata->pdev->dev, "update %d, %d, %d x %d\n", x, y, w, h);
+	dev_dbg_ratelimited(&ddata->pdev->dev, "update %d, %d, %d x %d\n", x, y, w, h);
 
 	mutex_lock(&ddata->lock);
 	in->ops.dsi->bus_lock(in);
@@ -978,14 +978,14 @@ static int dsicm_sync(struct omap_dss_device *dssdev)
 	struct panel_drv_data *ddata = to_panel_data(dssdev);
 	struct omap_dss_device *in = ddata->in;
 
-	dev_dbg(&ddata->pdev->dev, "sync\n");
+	dev_dbg_ratelimited(&ddata->pdev->dev, "sync\n");
 
 	mutex_lock(&ddata->lock);
 	in->ops.dsi->bus_lock(in);
 	in->ops.dsi->bus_unlock(in);
 	mutex_unlock(&ddata->lock);
 
-	dev_dbg(&ddata->pdev->dev, "sync done\n");
+	dev_dbg_ratelimited(&ddata->pdev->dev, "sync done\n");
 
 	return 0;
 }
-- 
2.7.0

[PATCH 23/23] drm: omapdrm: panel-dsi-cm: provide timings methods for omapdrm

[Sebastian Reichel]

Provide omapdrm with methods to acquire timings
and check them later, so that it can properly
initialize the display.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c | 38 +++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
index 196fe8d18ff4..edd1298b3a71 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
@@ -1163,6 +1163,41 @@ static void dsicm_ulps_work(struct work_struct *work)
 	mutex_unlock(&ddata->lock);
 }
 
+static void dsicm_get_timings(struct omap_dss_device *dssdev,
+		struct omap_video_timings *timings)
+{
+	struct panel_drv_data *ddata = to_panel_data(dssdev);
+
+	*timings = ddata->timings;
+}
+
+static int dsicm_check_timings(struct omap_dss_device *dssdev,
+		struct omap_video_timings *timings)
+{
+	struct panel_drv_data *ddata = to_panel_data(dssdev);
+	int ret = 0;
+
+	if (timings->x_res != ddata->timings.x_res)
+		ret = -EINVAL;
+	else if (timings->y_res != ddata->timings.y_res)
+		ret = -EINVAL;
+
+	/* pixelclock is modified by dsi, so not easily checkable */
+
+	/* vfp, vbp, hfp, hbp are (re-)configured automatically by dsi */
+
+	if (ret) {
+		dev_warn(dssdev->dev, "wrong resolution: %d x %d",
+			timings->x_res, timings->y_res);
+		dev_warn(dssdev->dev, "panel resolution: %d x %d",
+			ddata->timings.x_res, ddata->timings.y_res);
+	} else {
+		dev_dbg(dssdev->dev, "mode check: ok");
+	}
+
+	return ret;
+}
+
 static struct omap_dss_driver dsicm_ops = {
 	.connect	= dsicm_connect,
 	.disconnect	= dsicm_disconnect,
@@ -1180,6 +1215,9 @@ static struct omap_dss_driver dsicm_ops = {
 	.get_te		= dsicm_get_te,
 
 	.memory_read	= dsicm_memory_read,
+
+	.get_timings	= dsicm_get_timings,
+	.check_timings	= dsicm_check_timings,
 };
 
 static int dsicm_probe_pdata(struct platform_device *pdev)
-- 
2.7.0

Re: [PATCH 00/23] Nokia N950 display support

[Aaro Koskinen]

Hi,

On Tue, Mar 08, 2016 at 05:39:32PM +0100, Sebastian Reichel wrote:
> This series adds support for the Nokia N950 display.
> Since the panel is using DSI command mode, it involves
> adding support for manually updated displays to
> omapdrm.

Works OK, but the picture seems to be upside down? Also shouldn't the
default orientation be landscape?

A.

Re: [PATCH 00/23] Nokia N950 display support

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1361 bytes --]

Hi,

On Tue, Mar 08, 2016 at 08:39:08PM +0200, Aaro Koskinen wrote:
> On Tue, Mar 08, 2016 at 05:39:32PM +0100, Sebastian Reichel wrote:
> > This series adds support for the Nokia N950 display.
> > Since the panel is using DSI command mode, it involves
> > adding support for manually updated displays to
> > omapdrm.
> 
> Works OK, but the picture seems to be upside down?

vertical, upside down is the native panel orientation.

> Also shouldn't the default orientation be landscape?

The N950 vendor kernel contains some code adding DSI
rotation support with half-frame update mechnism to
avoid tearing. It's quite complex and as far as I
understand it also error-prone. Tomi knows more about
that.

I have a simpler patch without the half-frame update
stuff, which works fine for me. I didn't notice any
tearing, but I haven't done any really fast image
updating. Also omapdrm has rotation support using
the DSS hardware, which also seems to work ok. I'm
still checking out what method is most suitable for
mainline.

But yeah, we probably want to change the default
rotation. Especially since touchscreen should have
the same default rotation as the screen. (TS is
horizontal, correct orientation for keyboard usage)

I will have a look at the (default-)rotation when
basic panel support has been ACKed.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 00/23] Nokia N950 display support

[Tomi Valkeinen]

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

On 08/03/16 22:45, Sebastian Reichel wrote:
> Hi,
> 
> On Tue, Mar 08, 2016 at 08:39:08PM +0200, Aaro Koskinen wrote:
>> On Tue, Mar 08, 2016 at 05:39:32PM +0100, Sebastian Reichel wrote:
>>> This series adds support for the Nokia N950 display.
>>> Since the panel is using DSI command mode, it involves
>>> adding support for manually updated displays to
>>> omapdrm.
>>
>> Works OK, but the picture seems to be upside down?
> 
> vertical, upside down is the native panel orientation.
> 
>> Also shouldn't the default orientation be landscape?
> 
> The N950 vendor kernel contains some code adding DSI
> rotation support with half-frame update mechnism to
> avoid tearing. It's quite complex and as far as I
> understand it also error-prone. Tomi knows more about
> that.

It needs support in both the panel driver and the dispc driver, and is
quite intrusive. Or, at least it was with omapfb, I can't say if it
could somehow be implemented more cleanly with omapdrm. It's definitely
not something I will be working on.

> I have a simpler patch without the half-frame update
> stuff, which works fine for me. I didn't notice any
> tearing, but I haven't done any really fast image

You will see diagonal tearing with that rotation. But maybe that's not
an issue. I think it's the best option available if landscape mode is
required.

> updating. Also omapdrm has rotation support using
> the DSS hardware, which also seems to work ok. I'm
> still checking out what method is most suitable for
> mainline.

Hmm there's only so called DMA rotation, which shouldn't work. It's only
meant for really small displays, when the framebuffer is in SRAM. So I'm
a bit baffled as to what rotation you are using and why is it working =).

There is also VRFB rotation on omap3, but that's not supported by omapdrm.

> But yeah, we probably want to change the default
> rotation. Especially since touchscreen should have
> the same default rotation as the screen. (TS is
> horizontal, correct orientation for keyboard usage)

I don't know much about touchscreens, but I'm guessing that it's easier
to rotate the coordinates from touch than achieve good panel rotation on
N950.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 00/23] Nokia N950 display support

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 2230 bytes --]

On 08/03/16 22:45, Sebastian Reichel wrote:
> Hi,
> 
> On Tue, Mar 08, 2016 at 08:39:08PM +0200, Aaro Koskinen wrote:
>> On Tue, Mar 08, 2016 at 05:39:32PM +0100, Sebastian Reichel wrote:
>>> This series adds support for the Nokia N950 display.
>>> Since the panel is using DSI command mode, it involves
>>> adding support for manually updated displays to
>>> omapdrm.
>>
>> Works OK, but the picture seems to be upside down?
> 
> vertical, upside down is the native panel orientation.
> 
>> Also shouldn't the default orientation be landscape?
> 
> The N950 vendor kernel contains some code adding DSI
> rotation support with half-frame update mechnism to
> avoid tearing. It's quite complex and as far as I
> understand it also error-prone. Tomi knows more about
> that.

It needs support in both the panel driver and the dispc driver, and is
quite intrusive. Or, at least it was with omapfb, I can't say if it
could somehow be implemented more cleanly with omapdrm. It's definitely
not something I will be working on.

> I have a simpler patch without the half-frame update
> stuff, which works fine for me. I didn't notice any
> tearing, but I haven't done any really fast image

You will see diagonal tearing with that rotation. But maybe that's not
an issue. I think it's the best option available if landscape mode is
required.

> updating. Also omapdrm has rotation support using
> the DSS hardware, which also seems to work ok. I'm
> still checking out what method is most suitable for
> mainline.

Hmm there's only so called DMA rotation, which shouldn't work. It's only
meant for really small displays, when the framebuffer is in SRAM. So I'm
a bit baffled as to what rotation you are using and why is it working =).

There is also VRFB rotation on omap3, but that's not supported by omapdrm.

> But yeah, we probably want to change the default
> rotation. Especially since touchscreen should have
> the same default rotation as the screen. (TS is
> horizontal, correct orientation for keyboard usage)

I don't know much about touchscreens, but I'm guessing that it's easier
to rotate the coordinates from touch than achieve good panel rotation on
N950.

 Tomi


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

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

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

Re: [PATCH 00/23] Nokia N950 display support

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1242 bytes --]

Hi,

On Wed, Mar 09, 2016 at 09:10:28AM +0200, Tomi Valkeinen wrote:
> Hmm there's only so called DMA rotation, which shouldn't work. It's only
> meant for really small displays, when the framebuffer is in SRAM. So I'm
> a bit baffled as to what rotation you are using and why is it working =).
>
> There is also VRFB rotation on omap3, but that's not supported by omapdrm.

yeah, I was also wondering why it's working. I just did "xrandr -o
left" and my display was rotated unexpectedly :) I have not yet
checked what happens in that case.

> > But yeah, we probably want to change the default
> > rotation. Especially since touchscreen should have
> > the same default rotation as the screen. (TS is
> > horizontal, correct orientation for keyboard usage)
> 
> I don't know much about touchscreens, but I'm guessing that it's easier
> to rotate the coordinates from touch than achieve good panel rotation on
> N950.

Yes, rotating the TS coordinates is simpler than panel rotation,
since much less information must be rotated. I just mentioned
TS, since the _default_ orientation should be the same for both.
In the end both must be rotated at the same time (and thus both
must support rotation).

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 00/23] Nokia N950 display support

[Aaro Koskinen]

Hi,

On Tue, Mar 08, 2016 at 09:45:11PM +0100, Sebastian Reichel wrote:
> On Tue, Mar 08, 2016 at 08:39:08PM +0200, Aaro Koskinen wrote:
> > On Tue, Mar 08, 2016 at 05:39:32PM +0100, Sebastian Reichel wrote:
> > > This series adds support for the Nokia N950 display.
> > > Since the panel is using DSI command mode, it involves
> > > adding support for manually updated displays to
> > > omapdrm.
> > 
> > Works OK, but the picture seems to be upside down?
> 
> vertical, upside down is the native panel orientation.
> 
> > Also shouldn't the default orientation be landscape?
> 
> The N950 vendor kernel contains some code adding DSI
> rotation support with half-frame update mechnism to
> avoid tearing. It's quite complex and as far as I
> understand it also error-prone. Tomi knows more about
> that.

Ok, my comment was actually about fbcon (which doesn't need driver support
for rotation). But I guess there is no way to tell it what the default
should be, instead user needs to pass the correct rotation manually...

Anyway, for this series:

Tested-by: Aaro Koskinen <aaro.koskinen@iki.fi>

A.

Re: [PATCH 00/23] Nokia N950 display support

[Emil Velikov]

Hi Sebastian,

On 8 March 2016 at 16:39, Sebastian Reichel <sre@kernel.org> wrote:

>  arch/arm/boot/dts/omap3-n950-n9.dtsi            |  72 +++++++++
>  arch/arm/boot/dts/omap3-n950.dts                |  71 +++++++++
Just a friendly reminder that updating these and one will have to keep
the driver backwards compatible forever.

Tomi, has been in the process of removing all the unneeded cruft,
although omapdrm it is still using the custom panel, dsi and other
code as opposed to the ones provided by DRM. As he gets to reusing
those he might have some fun keeping things compatible.

It's up-to Tomi to decide, just thought I point it out.

Regards,
Emil

Re: [PATCH 00/23] Nokia N950 display support

[Tomi Valkeinen]

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



On 09/03/16 18:19, Emil Velikov wrote:
> Hi Sebastian,
> 
> On 8 March 2016 at 16:39, Sebastian Reichel <sre@kernel.org> wrote:
> 
>>  arch/arm/boot/dts/omap3-n950-n9.dtsi            |  72 +++++++++
>>  arch/arm/boot/dts/omap3-n950.dts                |  71 +++++++++
> Just a friendly reminder that updating these and one will have to keep
> the driver backwards compatible forever.
> 
> Tomi, has been in the process of removing all the unneeded cruft,
> although omapdrm it is still using the custom panel, dsi and other
> code as opposed to the ones provided by DRM. As he gets to reusing
> those he might have some fun keeping things compatible.
> 
> It's up-to Tomi to decide, just thought I point it out.

I haven't looked at this series yet, but all the omap display DT
bindings have been designed to be correct, and while omapdrm will
receive lots of changes, I don't see any need to touch anything in the
DT side.

Of course, if we will be using some existing common driver which
implements bindings in some other way, it'll cause some complexities...

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 00/23] Nokia N950 display support

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 1100 bytes --]



On 09/03/16 18:19, Emil Velikov wrote:
> Hi Sebastian,
> 
> On 8 March 2016 at 16:39, Sebastian Reichel <sre@kernel.org> wrote:
> 
>>  arch/arm/boot/dts/omap3-n950-n9.dtsi            |  72 +++++++++
>>  arch/arm/boot/dts/omap3-n950.dts                |  71 +++++++++
> Just a friendly reminder that updating these and one will have to keep
> the driver backwards compatible forever.
> 
> Tomi, has been in the process of removing all the unneeded cruft,
> although omapdrm it is still using the custom panel, dsi and other
> code as opposed to the ones provided by DRM. As he gets to reusing
> those he might have some fun keeping things compatible.
> 
> It's up-to Tomi to decide, just thought I point it out.

I haven't looked at this series yet, but all the omap display DT
bindings have been designed to be correct, and while omapdrm will
receive lots of changes, I don't see any need to touch anything in the
DT side.

Of course, if we will be using some existing common driver which
implements bindings in some other way, it'll cause some complexities...

 Tomi


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

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

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

Re: [PATCH 00/23] Nokia N950 display support

[Emil Velikov]

On 9 March 2016 at 16:24, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On 09/03/16 18:19, Emil Velikov wrote:
>> Hi Sebastian,
>>
>> On 8 March 2016 at 16:39, Sebastian Reichel <sre@kernel.org> wrote:
>>
>>>  arch/arm/boot/dts/omap3-n950-n9.dtsi            |  72 +++++++++
>>>  arch/arm/boot/dts/omap3-n950.dts                |  71 +++++++++
>> Just a friendly reminder that updating these and one will have to keep
>> the driver backwards compatible forever.
>>
>> Tomi, has been in the process of removing all the unneeded cruft,
>> although omapdrm it is still using the custom panel, dsi and other
>> code as opposed to the ones provided by DRM. As he gets to reusing
>> those he might have some fun keeping things compatible.
>>
>> It's up-to Tomi to decide, just thought I point it out.
>
> I haven't looked at this series yet, but all the omap display DT
> bindings have been designed to be correct, and while omapdrm will
> receive lots of changes, I don't see any need to touch anything in the
> DT side.
>
> Of course, if we will be using some existing common driver which
> implements bindings in some other way, it'll cause some complexities...
>
I'm leaning that you want to use the existing the DRM_PANEL driver
(gpu/drm/drm_panel.c). It has enough infrastructure to support 25+
'simple' panels (gpu/drm/panel/panel-simple.c) plus 5+ more complex
ones (gpu/drm/panel/).

>From a quick look the binding, they do differ although do take a look
for yourself:
Documentation/devicetree/bindings/display/panel/{display-timing,panel-dpi,panel-dsi-cm,simple-panel}.txt

Regards,
Emil

Re: [PATCH 00/23] Nokia N950 display support

[Emil Velikov]

On 9 March 2016 at 16:24, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On 09/03/16 18:19, Emil Velikov wrote:
>> Hi Sebastian,
>>
>> On 8 March 2016 at 16:39, Sebastian Reichel <sre@kernel.org> wrote:
>>
>>>  arch/arm/boot/dts/omap3-n950-n9.dtsi            |  72 +++++++++
>>>  arch/arm/boot/dts/omap3-n950.dts                |  71 +++++++++
>> Just a friendly reminder that updating these and one will have to keep
>> the driver backwards compatible forever.
>>
>> Tomi, has been in the process of removing all the unneeded cruft,
>> although omapdrm it is still using the custom panel, dsi and other
>> code as opposed to the ones provided by DRM. As he gets to reusing
>> those he might have some fun keeping things compatible.
>>
>> It's up-to Tomi to decide, just thought I point it out.
>
> I haven't looked at this series yet, but all the omap display DT
> bindings have been designed to be correct, and while omapdrm will
> receive lots of changes, I don't see any need to touch anything in the
> DT side.
>
> Of course, if we will be using some existing common driver which
> implements bindings in some other way, it'll cause some complexities...
>
I'm leaning that you want to use the existing the DRM_PANEL driver
(gpu/drm/drm_panel.c). It has enough infrastructure to support 25+
'simple' panels (gpu/drm/panel/panel-simple.c) plus 5+ more complex
ones (gpu/drm/panel/).

From a quick look the binding, they do differ although do take a look
for yourself:
Documentation/devicetree/bindings/display/panel/{display-timing,panel-dpi,panel-dsi-cm,simple-panel}.txt

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

Re: [PATCH 00/23] Nokia N950 display support

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1293 bytes --]

Hi Emil,

On Wed, Mar 09, 2016 at 04:19:48PM +0000, Emil Velikov wrote:
> Hi Sebastian,
> 
> On 8 March 2016 at 16:39, Sebastian Reichel <sre@kernel.org> wrote:
> 
> >  arch/arm/boot/dts/omap3-n950-n9.dtsi            |  72 +++++++++
> >  arch/arm/boot/dts/omap3-n950.dts                |  71 +++++++++
> Just a friendly reminder that updating these and one will have to keep
> the driver backwards compatible forever.

Of course. Basically the new DT properties are:

 * optional regulators (vpnl & vddi)
 * boolean "has-dsi-backlight"
 * resolution-x/y
 * offset-x/y

So probably not that much of a problem even if other API is
invented. I just noticed, that I forgot to actually add documentation
in Documentation/devicetree/bindings/display/panel/panel-dsi-cm.txt

I will fix that in the next patchset revision.

> Tomi, has been in the process of removing all the unneeded cruft,
> although omapdrm it is still using the custom panel, dsi and other
> code as opposed to the ones provided by DRM. As he gets to reusing
> those he might have some fun keeping things compatible.

I could not find "generic" panel binding documentation. Can you
give me a pointer? I can just stick to the same property names
to avoid potential future problems.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 00/23] Nokia N950 display support

[Emil Velikov]

Hi Sebastian,

On 9 March 2016 at 17:09, Sebastian Reichel <sre@kernel.org> wrote:
> Hi Emil,
>
> On Wed, Mar 09, 2016 at 04:19:48PM +0000, Emil Velikov wrote:
>> Hi Sebastian,
>>
>> On 8 March 2016 at 16:39, Sebastian Reichel <sre@kernel.org> wrote:
>>
>> >  arch/arm/boot/dts/omap3-n950-n9.dtsi            |  72 +++++++++
>> >  arch/arm/boot/dts/omap3-n950.dts                |  71 +++++++++
>> Just a friendly reminder that updating these and one will have to keep
>> the driver backwards compatible forever.
>
> Of course. Basically the new DT properties are:
>
>  * optional regulators (vpnl & vddi)
>  * boolean "has-dsi-backlight"
>  * resolution-x/y
>  * offset-x/y
>
> So probably not that much of a problem even if other API is
> invented. I just noticed, that I forgot to actually add documentation
> in Documentation/devicetree/bindings/display/panel/panel-dsi-cm.txt
>
> I will fix that in the next patchset revision.
>
>> Tomi, has been in the process of removing all the unneeded cruft,
>> although omapdrm it is still using the custom panel, dsi and other
>> code as opposed to the ones provided by DRM. As he gets to reusing
>> those he might have some fun keeping things compatible.
>
> I could not find "generic" panel binding documentation. Can you
> give me a pointer? I can just stick to the same property names
> to avoid potential future problems.
>
Pardon for the late reply.

As mentioned in another email, if you're interested in reusing the drm
panel infrastructure you should be looking in
Documentation/devicetree/bindings/display/panel/. With display-timing
panel-dpi panel-dsi-cm and (optional) simple-panel as a start.

Would reshuffling/documenting things make it more obvious ? Feel free
to let Rob Herring, Thierry Reding know.

Regards,
Emil

Re: [PATCH 00/23] Nokia N950 display support

[Emil Velikov]

Hi Sebastian,

On 9 March 2016 at 17:09, Sebastian Reichel <sre@kernel.org> wrote:
> Hi Emil,
>
> On Wed, Mar 09, 2016 at 04:19:48PM +0000, Emil Velikov wrote:
>> Hi Sebastian,
>>
>> On 8 March 2016 at 16:39, Sebastian Reichel <sre@kernel.org> wrote:
>>
>> >  arch/arm/boot/dts/omap3-n950-n9.dtsi            |  72 +++++++++
>> >  arch/arm/boot/dts/omap3-n950.dts                |  71 +++++++++
>> Just a friendly reminder that updating these and one will have to keep
>> the driver backwards compatible forever.
>
> Of course. Basically the new DT properties are:
>
>  * optional regulators (vpnl & vddi)
>  * boolean "has-dsi-backlight"
>  * resolution-x/y
>  * offset-x/y
>
> So probably not that much of a problem even if other API is
> invented. I just noticed, that I forgot to actually add documentation
> in Documentation/devicetree/bindings/display/panel/panel-dsi-cm.txt
>
> I will fix that in the next patchset revision.
>
>> Tomi, has been in the process of removing all the unneeded cruft,
>> although omapdrm it is still using the custom panel, dsi and other
>> code as opposed to the ones provided by DRM. As he gets to reusing
>> those he might have some fun keeping things compatible.
>
> I could not find "generic" panel binding documentation. Can you
> give me a pointer? I can just stick to the same property names
> to avoid potential future problems.
>
Pardon for the late reply.

As mentioned in another email, if you're interested in reusing the drm
panel infrastructure you should be looking in
Documentation/devicetree/bindings/display/panel/. With display-timing
panel-dpi panel-dsi-cm and (optional) simple-panel as a start.

Would reshuffling/documenting things make it more obvious ? Feel free
to let Rob Herring, Thierry Reding know.

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