[PATCH v6 00/14] drm/exynos: support LCD I80 interface display

[Inki Dae]

On 2014? 07? 17? 18:01, YoungJun Cho wrote:
> Hi,
> 
> This series adds LCD I80 interface display support for Exynos DRM driver.
> The FIMD(display controller) specification describes it as "LCD I80 interface"
> and the DSI specification describes it as "Command mode interface".
> 
> This is based on exynos-drm-next branch.
> 
> The previous patches,
> RFC: http://www.spinics.net/lists/dri-devel/msg58898.html
> V1: http://www.spinics.net/lists/dri-devel/msg59291.html
> V2: http://www.spinics.net/lists/dri-devel/msg59867.html
> V3: http://www.spinics.net/lists/dri-devel/msg60708.html
> V4: http://www.spinics.net/lists/dri-devel/msg60943.html
> V5: http://www.spinics.net/lists/dri-devel/msg62956.html
> 
> Changelog v2:
> - Fixes typo and removes unnecessary error log. (commented by Andrzej Hazda)
> - Adds missed pendlig_flip flag clear points. (commented by Daniel Kurtz)
> 
> Changelog v3:
> - Removes generic command mode and command mode display timing interface.
> - Moves I80 interface timings from panel DT to the FIMD(display controller) 
> DT.
> 
> Changelog v4:
> - Removes exynos5 sysreg(syscon) DT bindings and node from dtsi because
>   it was already updated by linux-samsung-soc. (commented by Vivek Gautam)
> 
> Changelog v5:
> - Fixes FIMD vidcon0 register relevant code.
> - Fixes panel gamma table, disable sequence.
> - Slitely updates for code cleanup.
> 
> Changelog v6:
> - Removes pass TE host ops in dsi and exynos dsi uses TE irq handler instead,
>   and it is related with the TE GPIO of panel. (commented by Thierry Reding)
> 
> Patches 1 and 2 fix trivial bugs.
> 
> Patches 3, 4, 5 and 6 implement FIMD(display controller) I80 interface.
> The MIPI DSI command mode based panel generates Tearing Effect synchronization
> signal between MCU and FB to display video image, and FIMD should trigger to
> transfer video image at this signal.
> So the panel generates it and the dsi should receive the TE IRQ and call TE
> handler chains to notify it to the FIMD.
> 
> Patches 7 and 8 implement to use Exynos5410 / 5420 / 5440 SoC DSI driver
> which is different from previous Exynos4 SoCs for some registers control.
> 
> Patches 9 and 10 introduce MIPI DSI command mode based Samsung S6E3FA0 AMOLED
> 5.7" LCD drm panel driver.
> 
> The ohters add DT property nodes to support MIPI DSI command mode.
> 
> I welcome any comments.
> 
> Thank you.
> Best regards YJ
> 
> YoungJun Cho (14):
>   drm/exynos: dsi: move the EoT packets configuration point
>   drm/exynos: use wait_event_timeout() for safety usage
>   ARM: dts: samsung-fimd: add LCD I80 interface specific properties
>   drm/exynos: add TE handler to support LCD I80 interface
>   drm/exynos: dsi: add TE interrupt handler to support LCD I80 interface
>   drm/exynos: fimd: support LCD I80 interface
>   ARM: dts: exynos_dsim: add exynos5410 compatible to DT bindings
>   drm/exynos: dsi: add driver data to support Exynos5410/5420/5440 SoCs
>   ARM: dts: s6e3fa0: add DT bindings
>   drm/panel: add S6E3FA0 driver
>   ARM: dts: exynos4: add system register property
>   ARM: dts: exynos5: add system register property
>   ARM: dts: exynos5420: add mipi-phy node
>   ARM: dts: exynos5420: add dsi node

Piked them up except patch 9 and 10. The two patches are needed for more
review and consensus

Thanks,
Inki Dae

> 
>  .../devicetree/bindings/panel/samsung,s6e3fa0.txt  |  46 ++
>  .../devicetree/bindings/video/exynos_dsim.txt  |   4 +-
>  .../devicetree/bindings/video/samsung-fimd.txt |  28 ++
>  arch/arm/boot/dts/exynos4.dtsi |   1 +
>  arch/arm/boot/dts/exynos5.dtsi |   1 +
>  arch/arm/boot/dts/exynos5420.dtsi  |  20 +
>  drivers/gpu/drm/exynos/Kconfig |   1 +
>  drivers/gpu/drm/exynos/exynos_drm_crtc.c   |  15 +-
>  drivers/gpu/drm/exynos/exynos_drm_crtc.h   |   7 +
>  drivers/gpu/drm/exynos/exynos_drm_drv.h|   3 +
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c| 266 +-
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c   | 276 +--
>  drivers/gpu/drm/panel/Kconfig  |   7 +
>  drivers/gpu/drm/panel/Makefile |   1 +
>  drivers/gpu/drm/panel/panel-s6e3fa0.c  | 541 
> +
>  include/video/samsung_fimd.h   |   3 +-
>  16 files changed, 1146 insertions(+), 74 deletions(-)
>  create mode 100644 
> Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt
>  create mode 100644 drivers/gpu/drm/panel/panel-s6e3fa0.c
> 

[PATCH 00/12] drm/exynos/ipp: image post processing improvements, part three

[Inki Dae]

On 2014? 07? 03? 22:10, Andrzej Hajda wrote:
> This set of independent patches contains various improvement and fixes
> for exynos_drm ipp framework.
> The patchset is based on exynos-drm-next branch.

Applied.

Thanks,
Inki Dae

> 
> Regards
> Andrzej
> 
> 
> Andrzej Hajda (12):
>   drm/exynos/ipp: remove type casting
>   drm/exynos/ipp: remove unused field from exynos_drm_ipp_private
>   drm/exynos/ipp: remove struct exynos_drm_ipp_private
>   drm/exynos/ipp: correct address type
>   drm/exynos/ipp: remove temporary variable
>   drm/exynos/ipp: remove incorrect checks of list_first_entry result
>   drm/exynos/ipp: simplify memory check function
>   drm/exynos/ipp: remove useless registration checks
>   drm/exynos/ipp: simplify ipp_find_obj
>   drm/exynos/ipp: remove redundant messages
>   drm/exynos/ipp: simplify ipp_create_id
>   drm/exynos/ipp: simplify ipp_find_driver
> 
>  drivers/gpu/drm/exynos/exynos_drm_drv.h |   7 +-
>  drivers/gpu/drm/exynos/exynos_drm_ipp.c | 259 
> +---
>  drivers/gpu/drm/exynos/exynos_drm_ipp.h |   4 +-
>  3 files changed, 73 insertions(+), 197 deletions(-)
> 

[Bug 80901] [radeon] loading corrupts lspci entry + unloading crashes kernel

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=80901

--- Comment #11 from Alex Deucher  ---
You can leave the radeon driver as a module and just not unload it.  There's
generally no reason to.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 80901] [radeon] loading corrupts lspci entry + unloading crashes kernel

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=80901

Andrea Patern?  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |INVALID

--- Comment #10 from Andrea Patern?  ---
It turns out that the vgaswitcheroo crash may be avoided by integrating the
radeon module directly into the kernel. Of course, this prevents the user from
unloading it, but, with the power management features on, I see no reason why
should I want to unload it.

I had to compile the radeon extra firmware in the kernel as well, but as of
now, I am not facing any issue, even after suspending/resuming.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[PATCH v6 05/14] drm/exynos: dsi: add TE interrupt handler to support LCD I80 interface

[YoungJun Cho]

Hi Varka,

On 07/22/2014 08:14 PM, Varka Bhadram wrote:
> On 07/22/2014 04:40 PM, YoungJun Cho wrote:
>> Hi Varka,
>>
>> This irq handler should be registered in attach() and unregistered in
>> detach().
>>
>> The devm_* APIs are released(freed) in remove(), right?
>>
>> Logically the panel could be attached and detached several times after
>> dsi is probed and not removed.
>> So I don't use devm_* APIs.
>
> You meant to say that in-case of GPIOs also you are following the same
> thing ..?
>
> Means requesting the GPIOs and Releasing several times ..?
>

Yes, this TE gpio is came from panel.
So it should be refresh whenever a (new) panel is attached.

Thank you.
Best regards YJ

>>
>>>
>>
>
>

[PATCH] drm: shmobile: fix warnings

[Dave Airlie]

On 22 July 2014 18:52, Russell King - ARM Linux  
wrote:
> On Sun, Jul 13, 2014 at 12:19:03PM +0100, Russell King wrote:
>> drivers/gpu/drm/shmobile/shmob_drm_drv.c:300:5: warning: "CONFIG_PM_SLEEP" 
>> is not defined [-Wundef]
>>
>> Always use #ifdef with CONFIG symbols, never just bare #if
>>
>> Signed-off-by: Russell King 
>
> Ping?
>

I merged all these to -next earlier today,

I'm still looking at the other ones that have dependant patches on
Greg on how best to merge.

Dave.

[PATCH v6 05/14] drm/exynos: dsi: add TE interrupt handler to support LCD I80 interface

[YoungJun Cho]

Hi Varka,

This irq handler should be registered in attach() and unregistered in 
detach().

The devm_* APIs are released(freed) in remove(), right?

Logically the panel could be attached and detached several times after 
dsi is probed and not removed.
So I don't use devm_* APIs.

Thank you.
Best regards YJ

On 07/22/2014 07:57 PM, Varka Bhadram wrote:
> On 07/22/2014 04:19 PM, YoungJun Cho wrote:
>
> (...)
>
>> +ret = gpio_request_one(dsi->te_gpio, GPIOF_IN, "te_gpio");
>
> devm_* APIs..?
>
>> +if (ret) {
>> +dev_err(dsi->dev, "gpio request failed with %d\n", ret);
>> +goto out;
>> +}
>> +
>> +/*
>> + * This TE GPIO IRQ should not be set to IRQ_NOAUTOEN, because panel
>> + * calls drm_panel_init() first then calls mipi_dsi_attach() in
>> probe().
>> + * It means that te_gpio is invalid when exynos_dsi_enable_irq() is
>> + * called by drm_panel_init() before panel is attached.
>> + */
>> +ret = request_threaded_irq(gpio_to_irq(dsi->te_gpio),
>> +exynos_dsi_te_irq_handler, NULL,
>> +IRQF_TRIGGER_RISING, "TE", dsi);
>
> why don't we use devm_request_threaded_irq()..?
>
>

[PATCH v4 04/11] pwm: add DT bindings documentation for atmel-hlcdc-pwm driver

[Varka Bhadram]

On Tuesday 22 July 2014 06:41 PM, Boris BREZILLON wrote:
> The HLCDC IP available in some Atmel SoCs (i.e. sam9x5i.e. at91sam9n12,
> at91sam9x5 family or sama5d3 family) provide a PWM device.
>
> The DT bindings used for this PWM device is following the default 3 cells
> bindings described in Documentation/devicetree/bindings/pwm/pwm.txt.
>
> Signed-off-by: Boris BREZILLON 
> ---
>   .../devicetree/bindings/pwm/atmel-hlcdc-pwm.txt| 55 
> ++
>   1 file changed, 55 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt
>
> diff --git a/Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt 
> b/Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt
> new file mode 100644
> index 000..86ad3e2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt
> @@ -0,0 +1,55 @@
> +Device-Tree bindings for Atmel's HLCDC (High LCD Controller) PWM driver
> +
> +The Atmel HLCDC PWM is subdevice of the HLCDC MFD device.
> +See ../mfd/atmel-hlcdc.txt for more details.
> +
> +Required properties:
> + - compatible: value should be one of the following:
> +   "atmel,hlcdc-pwm"
> + - pinctr-names: the pin control state names. Should contain "default".
> + - pinctrl-0: should contain the pinctrl states described by pinctrl
> +   default.
> + - #pwm-cells: should be set to 3. This PWM chip use the default 3 cells
> +   bindings defined in Documentation/devicetree/bindings/pwm/pwm.txt.
> +   The first cell encodes the PWM id (0 is the only acceptable value here,
> +   because the chip only provide one PWM).
> +   The second cell encodes the PWM period in nanoseconds.
> +   The third cell encodes the PWM flags (the only supported flag is
> +   PWM_POLARITY_INVERTED)

It will be readable if:
Required properties:
  - compatible  : value should be one of the following: "atmel,hlcdc-pwm"
  - pinctr-names: the pin control state names. Should contain "default".
  - pinctrl-0   : should contain the pinctrl states described by pinctrl 
default.
  - #pwm-cells  : should be set to 3. This PWM chip use the default 3 cells
  bindings defined in 
Documentation/devicetree/bindings/pwm/pwm.txt.
  The first cell encodes the PWM id (0 is the only acceptable 
value here,
  because the chip only provide one PWM).
  The second cell encodes the PWM period in nanoseconds.
  The third cell encodes the PWM flags (the only supported flag 
is
  PWM_POLARITY_INVERTED)



-- 
Regards,
Varka Bhadram

[PATCH v4 02/11] mfd: add documentation for atmel-hlcdc DT bindings

[Varka Bhadram]

On Tuesday 22 July 2014 06:41 PM, Boris BREZILLON wrote:
> The HLCDC IP available on some Atmel SoCs (i.e. at91sam9n12, at91sam9x5
> family or sama5d3 family) exposes 2 subdevices:
> - a display controller (controlled by a DRM driver)
> - a PWM chip
>
> This patch adds documentation for atmel-hlcdc DT bindings.
>
> Signed-off-by: Boris BREZILLON 
> ---
>   .../devicetree/bindings/mfd/atmel-hlcdc.txt| 50 
> ++
>   1 file changed, 50 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt 
> b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
> new file mode 100644
> index 000..e9cc1b2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
> @@ -0,0 +1,50 @@
> +Device-Tree bindings for Atmel's HLCDC (High LCD Controller) MFD driver
> +
> +Required properties:
> + - compatible: value should be one of the following:
> +   "atmel,sama5d3-hlcdc"
> + - reg: base address and size of the HLCDC device registers.
> + - clock-names: the name of the 3 clocks requested by the HLCDC device.
> +   Should contain "periph_clk", "sys_clk" and "slow_clk".
> + - clocks: should contain the 3 clocks requested by the HLCDC device.
> +

These bindings not clearly readable. It would be readable if

Required properties:
  - compatible  : value should be one of the following:"atmel,sama5d3-hlcdc"
  - reg : base address and size of the HLCDC device registers.
  - clock-names : the name of the 3 clocks requested by the HLCDC device.
  Should contain "periph_clk", "sys_clk" and "slow_clk".
  - clocks  : should contain the 3 clocks requested by the HLCDC device.

..


-- 
Regards,
Varka Bhadram

[PATCH v6 05/14] drm/exynos: dsi: add TE interrupt handler to support LCD I80 interface

[YoungJun Cho]

This is a temporary solution and should be made by more
generic way.

To support LCD I80 interface, the DSI host should register
TE interrupt handler from the TE GPIO of attached panel.
So the panel generates a tearing effect synchronization signal
then the DSI host calls the CRTC device manager to trigger
to transfer video image.

Signed-off-by: YoungJun Cho 
Acked-by: Inki Dae 
Acked-by: Kyungmin Park 
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 97 -
 1 file changed, 95 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c 
b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 58bfb2a..3adad44 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -16,7 +16,9 @@
 #include 

 #include 
+#include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -24,6 +26,7 @@
 #include 
 #include 

+#include "exynos_drm_crtc.h"
 #include "exynos_drm_drv.h"

 /* returns true iff both arguments logically differs */
@@ -247,6 +250,7 @@ struct exynos_dsi {
struct clk *bus_clk;
struct regulator_bulk_data supplies[2];
int irq;
+   int te_gpio;

u32 pll_clk_rate;
u32 burst_clk_rate;
@@ -954,17 +958,89 @@ static irqreturn_t exynos_dsi_irq(int irq, void *dev_id)
return IRQ_HANDLED;
 }

+static irqreturn_t exynos_dsi_te_irq_handler(int irq, void *dev_id)
+{
+   struct exynos_dsi *dsi = (struct exynos_dsi *)dev_id;
+   struct drm_encoder *encoder = dsi->encoder;
+
+   if (dsi->state & DSIM_STATE_ENABLED)
+   exynos_drm_crtc_te_handler(encoder->crtc);
+
+   return IRQ_HANDLED;
+}
+
+static void exynos_dsi_enable_irq(struct exynos_dsi *dsi)
+{
+   enable_irq(dsi->irq);
+
+   if (gpio_is_valid(dsi->te_gpio))
+   enable_irq(gpio_to_irq(dsi->te_gpio));
+}
+
+static void exynos_dsi_disable_irq(struct exynos_dsi *dsi)
+{
+   if (gpio_is_valid(dsi->te_gpio))
+   disable_irq(gpio_to_irq(dsi->te_gpio));
+
+   disable_irq(dsi->irq);
+}
+
 static int exynos_dsi_init(struct exynos_dsi *dsi)
 {
exynos_dsi_enable_clock(dsi);
exynos_dsi_reset(dsi);
-   enable_irq(dsi->irq);
+   exynos_dsi_enable_irq(dsi);
exynos_dsi_wait_for_reset(dsi);
exynos_dsi_init_link(dsi);

return 0;
 }

+static int exynos_dsi_register_te_irq(struct exynos_dsi *dsi)
+{
+   int ret;
+
+   dsi->te_gpio = of_get_named_gpio(dsi->panel_node, "te-gpios", 0);
+   if (!gpio_is_valid(dsi->te_gpio)) {
+   dev_err(dsi->dev, "no te-gpios specified\n");
+   ret = dsi->te_gpio;
+   goto out;
+   }
+
+   ret = gpio_request_one(dsi->te_gpio, GPIOF_IN, "te_gpio");
+   if (ret) {
+   dev_err(dsi->dev, "gpio request failed with %d\n", ret);
+   goto out;
+   }
+
+   /*
+* This TE GPIO IRQ should not be set to IRQ_NOAUTOEN, because panel
+* calls drm_panel_init() first then calls mipi_dsi_attach() in probe().
+* It means that te_gpio is invalid when exynos_dsi_enable_irq() is
+* called by drm_panel_init() before panel is attached.
+*/
+   ret = request_threaded_irq(gpio_to_irq(dsi->te_gpio),
+   exynos_dsi_te_irq_handler, NULL,
+   IRQF_TRIGGER_RISING, "TE", dsi);
+   if (ret) {
+   dev_err(dsi->dev, "request interrupt failed with %d\n", ret);
+   gpio_free(dsi->te_gpio);
+   goto out;
+   }
+
+out:
+   return ret;
+}
+
+static void exynos_dsi_unregister_te_irq(struct exynos_dsi *dsi)
+{
+   if (gpio_is_valid(dsi->te_gpio)) {
+   free_irq(gpio_to_irq(dsi->te_gpio), dsi);
+   gpio_free(dsi->te_gpio);
+   dsi->te_gpio = -ENOENT;
+   }
+}
+
 static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
  struct mipi_dsi_device *device)
 {
@@ -978,6 +1054,18 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host 
*host,
if (dsi->connector.dev)
drm_helper_hpd_irq_event(dsi->connector.dev);

+   /*
+* This is a temporary solution and should be made by more generic way.
+*
+* If attached panel device is for command mode one, dsi should register
+* TE interrupt handler.
+*/
+   if (!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO)) {
+   int ret = exynos_dsi_register_te_irq(dsi);
+   if (ret)
+   return ret;
+   }
+
return 0;
 }

@@ -986,6 +1074,8 @@ static int exynos_dsi_host_detach(struct mipi_dsi_host 
*host,
 {
struct exynos_dsi *dsi = host_to_dsi(host);

+   exynos_dsi_unregister_te_irq(dsi);
+
dsi->panel_node = NULL;

if (dsi->connector.dev)
@@ -1099,7 +1189,7 @@ static void exynos_dsi_poweroff(struct exynos_dsi *dsi)

[PATCH v6 05/14] drm/exynos: dsi: add TE interrupt handler to support LCD I80 interface

[YoungJun Cho]

Hi Andrzej,

On 07/22/2014 07:12 PM, Andrzej Hajda wrote:
> On 07/22/2014 03:23 AM, Inki Dae wrote:
>> On 2014? 07? 21? 23:01, Andrzej Hajda wrote:
>>> On 07/17/2014 11:01 AM, YoungJun Cho wrote:
 To support LCD I80 interface, the DSI host should register
 TE interrupt handler from the TE GPIO of attached panel.
 So the panel generates a tearing effect synchronization signal
 then the DSI host calls the CRTC device manager to trigger
 to transfer video image.

>>> This whole patch seems to be a hack.
>>>
>>> I think it is not a good idea to parse property of one device by another
>>> device's driver.
>>>
>>> Especially here TE GPIO belongs to the panel. The panel driver should
>>> know how to configure it and how to use it or not. The panel driver will
>>> generate TE signal based on this GPIO, DSI/BTA mechanism or any other
>>> mechanism provided by the panel.
>>> TE signal should be delivered to the display controller. The only role
>>> of DSIM here is that it is between the panel and the display controller
>>> so it should be used to route this signal to DC.
>>>
>>> According to specs TE signal should/can be generated by DBI and DSI
>>> command mode panels, so its handling should be more generic, not Exynos
>>> specific.
>>>
>> Right. However, it seems that there are no much users using command mode
>> panel so we would need more times to discuss the generic way. I think we
>> can have this feature in specific to Exynos drm - it doesn't affect
>> other SoC -.  Actually, I know OMAP people handle this feature in a way
>> specific to OMAP SoC. If other users need more generic way to this
>> feature then we could have a discussion about the generic way at that time.
>>
>> That is why Mr. Cho implemented TE feature like this. Do you have more
>> good idea? I wish the idea would be specific so that Mr. Cho can
>> implement it.
>>
>> P.s. Thierry already opposed the use of mipi_dsi_host_ops, I agree with
>> him. And also it seems not good to use crtc and encoder/connector
>> because these frameworks are common to all architecture including x86 so
>> other architectures wouldn't need TE feature.
>
> The good thing is that DT bindings in this case are OK, TE GPIO is in
> panel node.
> Maybe we can leave it this way for now, but at least lets add a comment to
> the code describing that it is temporary solution and should be make
> more generic in the future.
>

Okay, I'll leave this comment at exynos_dsi_host_attach() before current 
exynos_dsi_register_te_irq() relevant comment.

Thank you.
Best regards YJ

> Regards
> Andrzej
>>
>> Thanks,
>> Inki Dae
>>
>>> Regards
>>> Andrzej
>>>
 Signed-off-by: YoungJun Cho 
 Acked-by: Inki Dae 
 Acked-by: Kyungmin Park 
 ---
   drivers/gpu/drm/exynos/exynos_drm_dsi.c | 95 
 -
   1 file changed, 93 insertions(+), 2 deletions(-)

 diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c 
 b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
 index 58bfb2a..4997bfe 100644
 --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
 +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
 @@ -16,7 +16,9 @@
   #include 

   #include 
 +#include 
   #include 
 +#include 
   #include 
   #include 
   #include 
 @@ -24,6 +26,7 @@
   #include 
   #include 

 +#include "exynos_drm_crtc.h"
   #include "exynos_drm_drv.h"

   /* returns true iff both arguments logically differs */
 @@ -247,6 +250,7 @@ struct exynos_dsi {
struct clk *bus_clk;
struct regulator_bulk_data supplies[2];
int irq;
 +  int te_gpio;

u32 pll_clk_rate;
u32 burst_clk_rate;
 @@ -954,17 +958,89 @@ static irqreturn_t exynos_dsi_irq(int irq, void 
 *dev_id)
return IRQ_HANDLED;
   }

 +static irqreturn_t exynos_dsi_te_irq_handler(int irq, void *dev_id)
 +{
 +  struct exynos_dsi *dsi = (struct exynos_dsi *)dev_id;
 +  struct drm_encoder *encoder = dsi->encoder;
 +
 +  if (dsi->state & DSIM_STATE_ENABLED)
 +  exynos_drm_crtc_te_handler(encoder->crtc);
 +
 +  return IRQ_HANDLED;
 +}
 +
 +static void exynos_dsi_enable_irq(struct exynos_dsi *dsi)
 +{
 +  enable_irq(dsi->irq);
 +
 +  if (gpio_is_valid(dsi->te_gpio))
 +  enable_irq(gpio_to_irq(dsi->te_gpio));
 +}
 +
 +static void exynos_dsi_disable_irq(struct exynos_dsi *dsi)
 +{
 +  if (gpio_is_valid(dsi->te_gpio))
 +  disable_irq(gpio_to_irq(dsi->te_gpio));
 +
 +  disable_irq(dsi->irq);
 +}
 +
   static int exynos_dsi_init(struct exynos_dsi *dsi)
   {
exynos_dsi_enable_clock(dsi);
exynos_dsi_reset(dsi);
 -  enable_irq(dsi->irq);
 +  exynos_dsi_enable_irq(dsi);
exynos_dsi_wait_for_reset(dsi);
exynos_dsi_init_link(dsi);

return 0;
   }

 +static int 

[PATCH v6 10/14] drm/panel: add S6E3FA0 driver

[YoungJun Cho]

Hi Thierry,

On 07/22/2014 04:49 PM, Thierry Reding wrote:
> On Tue, Jul 22, 2014 at 12:41:21PM +0900, YoungJun Cho wrote:
>> On 07/21/2014 08:18 PM, Andrzej Hajda wrote:
>>> On 07/21/2014 11:19 AM, Thierry Reding wrote:
 On Mon, Jul 21, 2014 at 10:56:08AM +0200, Andrzej Hajda wrote:
> On 07/18/2014 03:49 AM, YoungJun Cho wrote:
>> On 07/17/2014 07:36 PM, Thierry Reding wrote:
>>> On Thu, Jul 17, 2014 at 06:01:25PM +0900, YoungJun Cho wrote:
> [...]
 +static void s6e3fa0_read_mtp_id(struct s6e3fa0 *ctx)
 +{
 +  unsigned char id[MTP_ID_LEN];
 +  int ret;
 +
 +  s6e3fa0_set_maximum_return_packet_size(ctx, MTP_ID_LEN);
 +  ret = s6e3fa0_dcs_read(ctx, MIPI_DCS_GET_DISPLAY_ID, id, 
 MTP_ID_LEN);
>>> This also looks like a standard DCS command. I can't find it in either
>>> v1.01 nor v1.02 of the specification, though. Do you know where it's
>>> specified?
>>>
>> Yes, I also can't find it in DCS specification and there is no special
>> description in panel datasheet.
>> But as it is declared in "include/video/mipi_display.h", so I think it
>> is a standard one.
>
> On page 9 of the "Introduction of MIPI by Renesas" [2] there is info
> it is a part of "Nokia I/F command list", I guess it is kind of alpha
> version of MIPI DCS.
>
> [2]: http://wenku.baidu.com/view/658fab68af1ffc4ffe47acbe.html

 That link is the only one which contains "Nokia I/F command list" on the
 internet (that is, according to Google). But since this is already part
 of the mipi_display.h header file we may as well use it.

 I wonder if perhaps the READ_DDB_START and READ_DDB_CONTINUE commands
 could be used as a replacement for this display ID.

>>
>> There is a simple description for "Read DDB Start(A1H)" like below.
>> - This command returns supplier identification and display module model /
>> revision information.
>> - NOTE: This information is not the same what Read IDs(04H) command is
>> returning.
>>
>> For reference, Read IDs(04H) description is like below.
>> - This read command returns 24-bit display identification information.
>> - The first read byte identifies the OLED module's manufacturer.
>> - The Second read byte is used to track the OLED module/driver version.
>> - The third read byte identifies the OLED module/driver.
>
> Okay, that explains things a little better. Can you point me to the
> document that this is from?

Sorry, I forgot to write specification name.
This specification is s6e3fa0 data sheet and it is confidential.
So I quoted only that portion.

>
> But what I was trying to say is that if the Read IDs command isn't an
> official DCS command, maybe it would be a better idea to use the DDB
> instead. I assume that even if it isn't the same information it would
> at least be a superset and therefore a suitable replacement.

This panel has several versions and each should set specific tuning 
value especially for AID, ELVSS and etc.
(Current is suitable for id[2] == 0x23).

I'll check READ_DDB_START & READ_DDB_CONTINUE result and try to use them 
if it is possible.

Thank you.
Best regards YJ

>
> Thierry
>

[PATCH 0/25] Replace DEFINE_PCI_DEVICE_TABLE macro use

[Benoit Taine]

On 21/07/2014 17:16, Bjorn Helgaas wrote:
> [+cc Jingoo]
> 
> On Fri, Jul 18, 2014 at 12:50 PM, James Bottomley
> > Anyway, it's PCI code ... let the PCI maintainer decide.  However, if he
> > does want this do it as one big bang patch via either the PCI or Trivial
> > tree (latter because Ji?? has experience doing this, but the former
> > might be useful so the decider feels the pain ...)
> 
> I don't feel strongly either way, so I guess I'm OK with this, and in
> the spirit of feeling the pain, I'm willing to handle it.  Jingoo
> proposed similar patches, so it might be nice to give him some credit.
> 
> Benoit, how about if you wait until about half-way through the merge
> window after v3.16 releases, generate an up-to-date single patch, and
> post that?  Then we can try to get it in before v3.17-rc1 to minimize
> merge hassles.

Sure, I will do this.

-- 
Beno?t Taine
Master cycle intern
Regal Team. LIP6

[Nouveau] [PATCH 09/17] drm/radeon: use common fence implementation for fences

[Daniel Vetter]

On Tue, Jul 22, 2014 at 6:39 PM, Christian K?nig
 wrote:
>> Maybe I've mixed things up a bit in my description. There is
>> fence_signal which the implementor/exporter of a fence must call when
>> the fence is completed. If the exporter has an ->enable_signaling
>> callback it can delay that call to fence_signal for as long as it
>> wishes as long as enable_signaling isn't called yet. But that's just
>> the optimization to not required irqs to be turned on all the time.
>>
>> The other function is fence_is_signaled, which is used by code that is
>> interested in the fence state, together with fence_wait if it wants to
>> block and not just wants to know the momentary fence state. All the
>> other functions (the stuff that adds callbacks and the various _locked
>> and other versions) are just for fancy special cases.
>
> Well that's rather bad, cause IRQs aren't reliable enough on Radeon HW for
> such a thing. Especially on Prime systems and Macs.
>
> That's why we have this fancy HZ/2 timeout on all fence wait operations to
> manually check if the fence is signaled or not.
>
> To guarantee that a fence is signaled after enable_signaling is called we
> would need to fire up a kernel thread which periodically calls
> fence->signaled.

We actually have seen similar fun on some i915 platforms. I wonder
whether we shouldn't have something in the fence core for this given
how common it is. Currently we have the same trick with regular wakups
on platforms with unreliable interrupts, but I haven't yet looked at
how we'll do this with callbacks once we add the scheduler and fences.
It might be though that we've finally fixed these coherency issues
between the interrupt and the fence write for real.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 1/4] drm/dsi: Make mipi_dsi_dcs_write() return ssize_t

[YoungJun Cho]

Hi,

On 07/22/2014 04:28 PM, Andrzej Hajda wrote:
> Hi Thierry,
>
> Thanks for the patch.
>
> On 07/22/2014 09:12 AM, Thierry Reding wrote:
>> From: Thierry Reding 
>>
>> This function returns the value of the struct mipi_dsi_host_ops'
>> .transfer() so make sure the return types are consistent.
>>
>> Signed-off-by: Thierry Reding 
>
> Acked-by: Andrzej Hajda 
> --
> Regards
> Andrzej
>> ---
>>   drivers/gpu/drm/drm_mipi_dsi.c| 4 ++--
>>   drivers/gpu/drm/panel/panel-s6e8aa0.c | 4 ++--
>>   include/drm/drm_mipi_dsi.h| 4 ++--
>>   3 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
>> index e633df2f68d8..6d2fd2077dae 100644
>> --- a/drivers/gpu/drm/drm_mipi_dsi.c
>> +++ b/drivers/gpu/drm/drm_mipi_dsi.c
>> @@ -205,8 +205,8 @@ EXPORT_SYMBOL(mipi_dsi_detach);
>>* @data: pointer to the command followed by parameters
>>* @len: length of @data
>>*/
>> -int mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, unsigned int channel,
>> -   const void *data, size_t len)
>> +ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, unsigned int 
>> channel,
>> +   const void *data, size_t len)
>>   {
>>  const struct mipi_dsi_host_ops *ops = dsi->host->ops;
>>  struct mipi_dsi_msg msg = {
>> diff --git a/drivers/gpu/drm/panel/panel-s6e8aa0.c 
>> b/drivers/gpu/drm/panel/panel-s6e8aa0.c
>> index 06e57a26db7a..beb43492b649 100644
>> --- a/drivers/gpu/drm/panel/panel-s6e8aa0.c
>> +++ b/drivers/gpu/drm/panel/panel-s6e8aa0.c
>> @@ -133,14 +133,14 @@ static int s6e8aa0_clear_error(struct s6e8aa0 *ctx)
>>   static void s6e8aa0_dcs_write(struct s6e8aa0 *ctx, const void *data, 
>> size_t len)
>>   {
>>  struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
>> -int ret;
>> +ssize_t ret;
>>
>>  if (ctx->error < 0)
>>  return;
>>
>>  ret = mipi_dsi_dcs_write(dsi, dsi->channel, data, len);
>>  if (ret < 0) {
>> -dev_err(ctx->dev, "error %d writing dcs seq: %*ph\n", ret, len,
>> +dev_err(ctx->dev, "error %zd writing dcs seq: %*ph\n", ret, len,
>>  data);
>>  ctx->error = ret;

One more thing!
This 'ctx->error' type is 'int'. So it should be changed from int to 
ssize_t in struct s6e8aa0.

Thank you.
Best regards YJ

>>  }
>> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
>> index efa1b552adc5..4b0112781910 100644
>> --- a/include/drm/drm_mipi_dsi.h
>> +++ b/include/drm/drm_mipi_dsi.h
>> @@ -127,8 +127,8 @@ struct mipi_dsi_device {
>>
>>   int mipi_dsi_attach(struct mipi_dsi_device *dsi);
>>   int mipi_dsi_detach(struct mipi_dsi_device *dsi);
>> -int mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, unsigned int channel,
>> -   const void *data, size_t len);
>> +ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, unsigned int 
>> channel,
>> +   const void *data, size_t len);
>>   ssize_t mipi_dsi_dcs_read(struct mipi_dsi_device *dsi, unsigned int 
>> channel,
>>u8 cmd, void *data, size_t len);
>>
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>

[Nouveau] [PATCH 09/17] drm/radeon: use common fence implementation for fences

[Daniel Vetter]

On Tue, Jul 22, 2014 at 6:21 PM, Daniel Vetter  
wrote:
> But like I've said fence->signaled is optional so you don't need this
> necessarily, as long as radeon eventually calls fence_signaled once
> the fence has completed.

Actually I've chatted a bit with Maarten about the different ways we
could restrict both the calling context and the implementations for
fence callbacks to avoid surprises. One is certainyl that we need
WARN_ON(in_interrupt) around the wait, enable_singaling and add
callback stuff.

But we also talked about ensure that the ->signaled callback never
sleeps by wrapping it in a preempt_enable/disable section. Currently
sleeping is allowed in ->signaled, which the radeon implementation
here does. I think it would be reasonable to forbid this and drop
__radeon_fence_signaled.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[Bug 80901] [radeon] loading corrupts lspci entry + unloading crashes kernel

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=80901

--- Comment #9 from Andrea Patern?  ---
That is.. well. I feel quite dumb, because it totally makes sense. It totally
works when the card is powered up: both lspci and radeontop work like a charm!

Now I only have to figure out what may cause the vgaswitcheroo problem

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[Nouveau] [PATCH 09/17] drm/radeon: use common fence implementation for fences

[Christian König]

> Maybe I've mixed things up a bit in my description. There is
> fence_signal which the implementor/exporter of a fence must call when
> the fence is completed. If the exporter has an ->enable_signaling
> callback it can delay that call to fence_signal for as long as it
> wishes as long as enable_signaling isn't called yet. But that's just
> the optimization to not required irqs to be turned on all the time.
>
> The other function is fence_is_signaled, which is used by code that is
> interested in the fence state, together with fence_wait if it wants to
> block and not just wants to know the momentary fence state. All the
> other functions (the stuff that adds callbacks and the various _locked
> and other versions) are just for fancy special cases.
Well that's rather bad, cause IRQs aren't reliable enough on Radeon HW 
for such a thing. Especially on Prime systems and Macs.

That's why we have this fancy HZ/2 timeout on all fence wait operations 
to manually check if the fence is signaled or not.

To guarantee that a fence is signaled after enable_signaling is called 
we would need to fire up a kernel thread which periodically calls 
fence->signaled.

Christian.

Am 22.07.2014 18:21, schrieb Daniel Vetter:
> On Tue, Jul 22, 2014 at 5:59 PM, Christian K?nig
>  wrote:
>> Am 22.07.2014 17:42, schrieb Daniel Vetter:
>>
>>> On Tue, Jul 22, 2014 at 5:35 PM, Christian K?nig
>>>  wrote:
 Drivers exporting fences need to provide a fence->signaled and a
 fence->wait
 function, everything else like fence->enable_signaling or calling
 fence_signaled() from the driver is optional.

 Drivers wanting to use exported fences don't call fence->signaled or
 fence->wait in atomic or interrupt context, and not with holding any
 global
 locking primitives (like mmap_sem etc...). Holding locking primitives
 local
 to the driver is ok, as long as they don't conflict with anything
 possible
 used by their own fence implementation.
>>> Well that's almost what we have right now with the exception that
>>> drivers are allowed (actually must for correctness when updating
>>> fences) the ww_mutexes for dma-bufs (or other buffer objects).
>>
>> In this case sorry for so much noise. I really haven't looked in so much
>> detail into anything but Maarten's Radeon patches.
>>
>> But how does that then work right now? My impression was that it's mandatory
>> for drivers to call fence_signaled()?
> Maybe I've mixed things up a bit in my description. There is
> fence_signal which the implementor/exporter of a fence must call when
> the fence is completed. If the exporter has an ->enable_signaling
> callback it can delay that call to fence_signal for as long as it
> wishes as long as enable_signaling isn't called yet. But that's just
> the optimization to not required irqs to be turned on all the time.
>
> The other function is fence_is_signaled, which is used by code that is
> interested in the fence state, together with fence_wait if it wants to
> block and not just wants to know the momentary fence state. All the
> other functions (the stuff that adds callbacks and the various _locked
> and other versions) are just for fancy special cases.
>
>>> Locking
>>> correctness is enforced with some extremely nasty lockdep annotations
>>> + additional debugging infrastructure enabled with
>>> CONFIG_DEBUG_WW_MUTEX_SLOWPATH. We really need to be able to hold
>>> dma-buf ww_mutexes while updating fences or waiting for them. And
>>> obviously for ->wait we need non-atomic context, not just
>>> non-interrupt.
>>
>> Sounds mostly reasonable, but for holding the dma-buf ww_mutex, wouldn't be
>> an RCU be more appropriate here? E.g. aren't we just interested that the
>> current assigned fence at some point is signaled?
> Yeah, as an optimization you can get the set of currently attached
> fences to a dma-buf with just rcu. But if you update the set of fences
> attached to a dma-buf (e.g. radeon blits the newly rendered frame to a
> dma-buf exported by i915 for scanout on i915) then you need a write
> lock on that buffer. Which is what the ww_mutex is for, to make sure
> that you don't deadlock with i915 doing concurrent ops on the same
> underlying buffer.
>
>> Something like grab ww_mutexes, grab a reference to the current fence
>> object, release ww_mutex, wait for fence, release reference to the fence
>> object.
> Yeah, if the only thing you want to do is wait for fences, then the
> rcu-protected fence ref grabbing + lockless waiting is all you need.
> But e.g. in an execbuf you also need to update fences and maybe deep
> down in the reservation code you notice that you need to evict some
> stuff and so need to wait on some other guy to finish, and it's too
> complicated to drop and reacquire all the locks. Or you simply need to
> do a blocking wait on other gpus (because there's no direct hw sync
> mechanism) and again dropping locks would needlessly complicate the
> code. So I think we should allow 

[Bug 80901] [radeon] loading corrupts lspci entry + unloading crashes kernel

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=80901

--- Comment #8 from Alex Deucher  ---
(In reply to Andrea Patern? from comment #7)
> Makes sense! Nonetheless, the "unknown header type 7f" problem remains, as
> well as the unloading problem. I managed to capture the journal log of the
> system just before the crash, which is attached.

The lspci -vvv problem is that lspci doesn't power up the GPU so it just reads
back garbage since the GPU is powered down.  It should work if you run lspci
when the GPU is powered up (e.g., when rendering with DRM_PRIME=1 or if you
disable runtime power management (boot with radeon.runpm=0 on the kernel
command line in grub).  The crash is a bug in vgaswitcheroo I think rather than
the radeon driver.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[Nouveau] [PATCH 09/17] drm/radeon: use common fence implementation for fences

[Daniel Vetter]

On Tue, Jul 22, 2014 at 5:59 PM, Christian K?nig
 wrote:
> Am 22.07.2014 17:42, schrieb Daniel Vetter:
>
>> On Tue, Jul 22, 2014 at 5:35 PM, Christian K?nig
>>  wrote:
>>>
>>> Drivers exporting fences need to provide a fence->signaled and a
>>> fence->wait
>>> function, everything else like fence->enable_signaling or calling
>>> fence_signaled() from the driver is optional.
>>>
>>> Drivers wanting to use exported fences don't call fence->signaled or
>>> fence->wait in atomic or interrupt context, and not with holding any
>>> global
>>> locking primitives (like mmap_sem etc...). Holding locking primitives
>>> local
>>> to the driver is ok, as long as they don't conflict with anything
>>> possible
>>> used by their own fence implementation.
>>
>> Well that's almost what we have right now with the exception that
>> drivers are allowed (actually must for correctness when updating
>> fences) the ww_mutexes for dma-bufs (or other buffer objects).
>
>
> In this case sorry for so much noise. I really haven't looked in so much
> detail into anything but Maarten's Radeon patches.
>
> But how does that then work right now? My impression was that it's mandatory
> for drivers to call fence_signaled()?

Maybe I've mixed things up a bit in my description. There is
fence_signal which the implementor/exporter of a fence must call when
the fence is completed. If the exporter has an ->enable_signaling
callback it can delay that call to fence_signal for as long as it
wishes as long as enable_signaling isn't called yet. But that's just
the optimization to not required irqs to be turned on all the time.

The other function is fence_is_signaled, which is used by code that is
interested in the fence state, together with fence_wait if it wants to
block and not just wants to know the momentary fence state. All the
other functions (the stuff that adds callbacks and the various _locked
and other versions) are just for fancy special cases.

>> Locking
>> correctness is enforced with some extremely nasty lockdep annotations
>> + additional debugging infrastructure enabled with
>> CONFIG_DEBUG_WW_MUTEX_SLOWPATH. We really need to be able to hold
>> dma-buf ww_mutexes while updating fences or waiting for them. And
>> obviously for ->wait we need non-atomic context, not just
>> non-interrupt.
>
>
> Sounds mostly reasonable, but for holding the dma-buf ww_mutex, wouldn't be
> an RCU be more appropriate here? E.g. aren't we just interested that the
> current assigned fence at some point is signaled?

Yeah, as an optimization you can get the set of currently attached
fences to a dma-buf with just rcu. But if you update the set of fences
attached to a dma-buf (e.g. radeon blits the newly rendered frame to a
dma-buf exported by i915 for scanout on i915) then you need a write
lock on that buffer. Which is what the ww_mutex is for, to make sure
that you don't deadlock with i915 doing concurrent ops on the same
underlying buffer.

> Something like grab ww_mutexes, grab a reference to the current fence
> object, release ww_mutex, wait for fence, release reference to the fence
> object.

Yeah, if the only thing you want to do is wait for fences, then the
rcu-protected fence ref grabbing + lockless waiting is all you need.
But e.g. in an execbuf you also need to update fences and maybe deep
down in the reservation code you notice that you need to evict some
stuff and so need to wait on some other guy to finish, and it's too
complicated to drop and reacquire all the locks. Or you simply need to
do a blocking wait on other gpus (because there's no direct hw sync
mechanism) and again dropping locks would needlessly complicate the
code. So I think we should allow this just to avoid too hairy/brittle
(and almost definitely little tested code) in drivers.

Afaik this is also the same way ttm currently handles things wrt
buffer reservation and eviction.

>> Agreed that any shared locks are out of the way (especially stuff like
>> dev->struct_mutex or other non-strictly driver-private stuff, i915 is
>> really bad here still).
>
>
> Yeah that's also an point I've wanted to note on Maartens patch. Radeon
> grabs the read side of it's exclusive semaphore while waiting for fences
> (because it assumes that the fence it waits for is a Radeon fence).
>
> Assuming that we need to wait in both directions with Prime (e.g. Intel
> driver needs to wait for Radeon to finish rendering and Radeon needs to wait
> for Intel to finish displaying), this might become a perfect example of
> locking inversion.

fence updates are atomic on a dma-buf, protected by ww_mutex. The neat
trick of ww_mutex is that they enforce a global ordering, so in your
scenario either i915 or radeon would be first and you can't deadlock.
There is no way to interleave anything even if you have lots of
buffers shared between i915/radeon. Wrt deadlocking it's exactly the
same guarantees as the magic ttm provides for just one driver with
concurrent command submission since it's the same idea.

>> So 

[PATCH 4/4] drm/radeon: remove visible vram size limit on bo allocation (v3)

[Michel Dänzer]

On 19.07.2014 00:09, Alex Deucher wrote:
> Now that fallback to gtt is fixed for cpu access, we can
> remove this limit.
> 
> bug:
> https://bugs.freedesktop.org/show_bug.cgi?id=78717
> 
> v2: use new gart_pin_size to accurately track available gtt.
> v3: fix comment

[...]

> @@ -55,10 +55,14 @@ int radeon_gem_object_create(struct radeon_device *rdev, 
> int size,
>   alignment = PAGE_SIZE;
>   }
>  
> - /* maximun bo size is the minimun btw visible vram and gtt size */
> - max_size = min(rdev->mc.visible_vram_size, rdev->mc.gtt_size);
> + /* Maximum bo size is the unpinned gtt size since we use the gtt to
> +  * handle vram to system pool migrations.  We could probably remove
> +  * this check altogether with a little additional work to support
> +  * splitting vram <-> system transfers into multiple steps.
> +  */
> + max_size = rdev->mc.gtt_size - rdev->gart_pin_size;

Actually, the the check couldn't be removed even then, but would need to
be replaced by a check against the VRAM size or something like that.
Maybe just drop the second sentence of the comment?

Either way though, the series is

Reviewed-by: Michel D?nzer 


-- 
Earthling Michel D?nzer|  http://www.amd.com
Libre software enthusiast  |Mesa and X developer

[Bug 80901] [radeon] loading corrupts lspci entry + unloading crashes kernel

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=80901

--- Comment #7 from Andrea Patern?  ---
Created attachment 143941
  --> https://bugzilla.kernel.org/attachment.cgi?id=143941=edit
System Journal log after module unloading

Makes sense! Nonetheless, the "unknown header type 7f" problem remains, as well
as the unloading problem. I managed to capture the journal log of the system
just before the crash, which is attached.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[Nouveau] [PATCH 09/17] drm/radeon: use common fence implementation for fences

[Christian König]

Am 22.07.2014 17:42, schrieb Daniel Vetter:
> On Tue, Jul 22, 2014 at 5:35 PM, Christian K?nig
>  wrote:
>> Drivers exporting fences need to provide a fence->signaled and a fence->wait
>> function, everything else like fence->enable_signaling or calling
>> fence_signaled() from the driver is optional.
>>
>> Drivers wanting to use exported fences don't call fence->signaled or
>> fence->wait in atomic or interrupt context, and not with holding any global
>> locking primitives (like mmap_sem etc...). Holding locking primitives local
>> to the driver is ok, as long as they don't conflict with anything possible
>> used by their own fence implementation.
> Well that's almost what we have right now with the exception that
> drivers are allowed (actually must for correctness when updating
> fences) the ww_mutexes for dma-bufs (or other buffer objects).

In this case sorry for so much noise. I really haven't looked in so much 
detail into anything but Maarten's Radeon patches.

But how does that then work right now? My impression was that it's 
mandatory for drivers to call fence_signaled()?

> Locking
> correctness is enforced with some extremely nasty lockdep annotations
> + additional debugging infrastructure enabled with
> CONFIG_DEBUG_WW_MUTEX_SLOWPATH. We really need to be able to hold
> dma-buf ww_mutexes while updating fences or waiting for them. And
> obviously for ->wait we need non-atomic context, not just
> non-interrupt.

Sounds mostly reasonable, but for holding the dma-buf ww_mutex, wouldn't 
be an RCU be more appropriate here? E.g. aren't we just interested that 
the current assigned fence at some point is signaled?

Something like grab ww_mutexes, grab a reference to the current fence 
object, release ww_mutex, wait for fence, release reference to the fence 
object.


> Agreed that any shared locks are out of the way (especially stuff like
> dev->struct_mutex or other non-strictly driver-private stuff, i915 is
> really bad here still).

Yeah that's also an point I've wanted to note on Maartens patch. Radeon 
grabs the read side of it's exclusive semaphore while waiting for fences 
(because it assumes that the fence it waits for is a Radeon fence).

Assuming that we need to wait in both directions with Prime (e.g. Intel 
driver needs to wait for Radeon to finish rendering and Radeon needs to 
wait for Intel to finish displaying), this might become a perfect 
example of locking inversion.

> So from the core fence framework I think we already have exactly this,
> and we only need to adjust the radeon implementation a bit to make it
> less risky and invasive to the radeon driver logic.

Agree. Well the biggest problem I see is that exclusive semaphore I need 
to take when anything calls into the driver. For the fence code I need 
to move that down into the fence->signaled handler, cause that now can 
be called from outside the driver.

Maarten solved this by telling the driver in the lockup handler (where 
we grab the write side of the exclusive lock) that all interrupts are 
already enabled, so that fence->signaled hopefully wouldn't mess with 
the hardware at all. While this probably works, it just leaves me with a 
feeling that we are doing something wrong here.

Christian.

> -Daniel

[PATCH 09/17] drm/radeon: use common fence implementation for fences

[Daniel Vetter]

On Tue, Jul 22, 2014 at 5:42 PM, Alex Deucher  wrote:
>> Fence-based syncing between userspace queues submitted stuff through
>> doorbells and anything submitted by the general simply wont work.
>> Which is why I think the doorbell is a stupid interface since I just
>> don't see cameras and v4l devices implementing all that complexity to
>> get a pure userspace side sync solution.
>>
>
> Like it or not this is what a lot of application writers want (look at
> mantle and metal and similar new APIs or android synpts).  Having
> queues and fences in userspace allows the application to structure
> things to best fit their own task graphs.  The app can decide how to
> deal with dependencies and synchronization explicitly instead of
> blocking the queues in the kernel for everyone.  Anyway, this is
> getting off topic.

Well there's explicit fences as used in opencl and android syncpts. My
plan is actually to support that in i915 using Maarten's struct fence
stuff (and there's just a very trivial patch for the android stuff in
merging needed to get there). What doesn't work is fences created
behind the kernel's back purely in userspace by giving shared memory
locations special meaning. Those get the kernel completely out of the
picture (as opposed to android syncpts, which just make sync
explicit).

I guess long-term we might need something like gpu futexes to make
that pure userspace syncing integrate a bit better, but imo that's (at
least for now) out of scope. For fences here I have the goal of one
internally representation used by both implicit syncing (dma-buf on
classic linux, e.g. prime) and explicit fencing on android or opencl
or something like that.

We don't have the code yet ready, but that's the direction i915 will
move towards for the near future. Jesse is working on some patches
already.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[Nouveau] [PATCH 09/17] drm/radeon: use common fence implementation for fences

[Daniel Vetter]

On Tue, Jul 22, 2014 at 5:35 PM, Christian K?nig
 wrote:
> Drivers exporting fences need to provide a fence->signaled and a fence->wait
> function, everything else like fence->enable_signaling or calling
> fence_signaled() from the driver is optional.
>
> Drivers wanting to use exported fences don't call fence->signaled or
> fence->wait in atomic or interrupt context, and not with holding any global
> locking primitives (like mmap_sem etc...). Holding locking primitives local
> to the driver is ok, as long as they don't conflict with anything possible
> used by their own fence implementation.

Well that's almost what we have right now with the exception that
drivers are allowed (actually must for correctness when updating
fences) the ww_mutexes for dma-bufs (or other buffer objects). Locking
correctness is enforced with some extremely nasty lockdep annotations
+ additional debugging infrastructure enabled with
CONFIG_DEBUG_WW_MUTEX_SLOWPATH. We really need to be able to hold
dma-buf ww_mutexes while updating fences or waiting for them. And
obviously for ->wait we need non-atomic context, not just
non-interrupt.

Agreed that any shared locks are out of the way (especially stuff like
dev->struct_mutex or other non-strictly driver-private stuff, i915 is
really bad here still).

So from the core fence framework I think we already have exactly this,
and we only need to adjust the radeon implementation a bit to make it
less risky and invasive to the radeon driver logic.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] drm/radeon: add trace_radeon_vm_flush

[Christian König]

From: Christian K?nig 

Signed-off-by: Christian K?nig 
---
 drivers/gpu/drm/radeon/radeon_trace.h | 18 ++
 drivers/gpu/drm/radeon/radeon_vm.c|  1 +
 2 files changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/radeon/radeon_trace.h 
b/drivers/gpu/drm/radeon/radeon_trace.h
index f749f2c..cd781f3 100644
--- a/drivers/gpu/drm/radeon/radeon_trace.h
+++ b/drivers/gpu/drm/radeon/radeon_trace.h
@@ -104,6 +104,24 @@ TRACE_EVENT(radeon_vm_set_page,
  __entry->flags, __entry->count)
 );

+TRACE_EVENT(radeon_vm_flush,
+   TP_PROTO(uint64_t pd_addr, unsigned ring, unsigned id),
+   TP_ARGS(pd_addr, ring, id),
+   TP_STRUCT__entry(
+__field(u64, pd_addr)
+__field(u32, ring)
+__field(u32, id)
+),
+
+   TP_fast_assign(
+  __entry->pd_addr = pd_addr;
+  __entry->ring = ring;
+  __entry->id = id;
+  ),
+   TP_printk("pd_addr=%010Lx, ring=%u, id=%u",
+ __entry->pd_addr, __entry->ring, __entry->id)
+);
+
 DECLARE_EVENT_CLASS(radeon_fence_request,

TP_PROTO(struct drm_device *dev, int ring, u32 seqno),
diff --git a/drivers/gpu/drm/radeon/radeon_vm.c 
b/drivers/gpu/drm/radeon/radeon_vm.c
index 4b7a080..d488a87 100644
--- a/drivers/gpu/drm/radeon/radeon_vm.c
+++ b/drivers/gpu/drm/radeon/radeon_vm.c
@@ -240,6 +240,7 @@ void radeon_vm_flush(struct radeon_device *rdev,
/* if we can't remember our last VM flush then flush now! */
/* XXX figure out why we have to flush all the time */
if (!vm->last_flush || true || pd_addr != vm->pd_gpu_addr) {
+   trace_radeon_vm_flush(pd_addr, ring, vm->id);
vm->pd_gpu_addr = pd_addr;
radeon_ring_vm_flush(rdev, ring, vm);
}
-- 
1.9.1

[PATCH] drm/radeon: fix R600_PTE_GART handling

[Christian König]

From: Christian K?nig 

That didn't worked correctly any more and opened up a security problem.

Signed-off-by: Christian K?nig 
---
 drivers/gpu/drm/radeon/cik_sdma.c | 3 +--
 drivers/gpu/drm/radeon/radeon.h   | 6 +++---
 drivers/gpu/drm/radeon/si_dma.c   | 3 +--
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/radeon/cik_sdma.c 
b/drivers/gpu/drm/radeon/cik_sdma.c
index 8534068..28ca3cb 100644
--- a/drivers/gpu/drm/radeon/cik_sdma.c
+++ b/drivers/gpu/drm/radeon/cik_sdma.c
@@ -775,8 +775,7 @@ void cik_sdma_vm_set_page(struct radeon_device *rdev,

trace_radeon_vm_set_page(pe, addr, count, incr, flags);

-   /* XXX: How to distinguish between GART and other system memory pages? 
*/
-   if (flags & R600_PTE_SYSTEM) {
+   if ((flags & R600_PTE_GART_MASK) == R600_PTE_GART_MASK) {
uint64_t src = rdev->gart.table_addr + (addr >> 12) * 8;
while (count) {
unsigned bytes = count * 8;
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 89b63b9..fd878c7 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -864,9 +864,9 @@ struct radeon_mec {
 #define R600_PTE_FRAG_64KB (4 << 7)
 #define R600_PTE_FRAG_256KB(6 << 7)

-/* flags used for GART page table entries on R600+ */
-#define R600_PTE_GART  ( R600_PTE_VALID | R600_PTE_SYSTEM | R600_PTE_SNOOPED \
-   | R600_PTE_READABLE | R600_PTE_WRITEABLE)
+/* flags needed to be set so we can copy directly from the GART table */
+#define R600_PTE_GART_MASK ( R600_PTE_READABLE | R600_PTE_WRITEABLE | \
+ R600_PTE_SYSTEM | R600_PTE_VALID )

 struct radeon_vm_pt {
struct radeon_bo*bo;
diff --git a/drivers/gpu/drm/radeon/si_dma.c b/drivers/gpu/drm/radeon/si_dma.c
index c9da341..a26e842 100644
--- a/drivers/gpu/drm/radeon/si_dma.c
+++ b/drivers/gpu/drm/radeon/si_dma.c
@@ -79,8 +79,7 @@ void si_dma_vm_set_page(struct radeon_device *rdev,

trace_radeon_vm_set_page(pe, addr, count, incr, flags);

-   /* XXX: How to distinguish between GART and other system memory pages? 
*/
-   if (flags & R600_PTE_SYSTEM) {
+   if ((flags & R600_PTE_GART_MASK) == R600_PTE_GART_MASK) {
uint64_t src = rdev->gart.table_addr + (addr >> 12) * 8;
while (count) {
unsigned bytes = count * 8;
-- 
1.9.1

[PATCH] drm/crtc-helper: use drm_framebuffer flags

[Benjamin Gaignard]

Adding LKML and David in diffusion list to get an opinion on this patch

2014-07-10 10:01 GMT+02:00 Fabien DESSENNE :
> Hi,
> Can anyone review this patch ?
> Thanks for your time.
> Fabien
>
>> -Original Message-
>> From: Fabien DESSENNE [mailto:fabien.dessenne at st.com]
>> Sent: mardi 1 juillet 2014 14:41
>> To: dri-devel at lists.freedesktop.org
>> Cc: Benjamin Gaignard; Vincent ABRIOU; Fabien DESSENNE
>> Subject: [PATCH] drm/crtc-helper: use drm_framebuffer flags
>>
>> The "flags" parameter of the DRM_IOCTL_MODE_ADDFB2 ioctl must be
>> propagated and used by the driver.
>> The only possible value of flags is DRM_MODE_FB_INTERLACED.
>>
>> Signed-off-by: Fabien Dessenne 
>> Reviewed-by: Benjamin GAIGNARD 
>> ---
>>  drivers/gpu/drm/drm_crtc_helper.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpu/drm/drm_crtc_helper.c
>> b/drivers/gpu/drm/drm_crtc_helper.c
>> index 23500c0..5974489 100644
>> --- a/drivers/gpu/drm/drm_crtc_helper.c
>> +++ b/drivers/gpu/drm/drm_crtc_helper.c
>> @@ -966,6 +966,7 @@ int drm_helper_mode_fill_fb_struct(struct
>> drm_framebuffer *fb,
>>   drm_fb_get_bpp_depth(mode_cmd->pixel_format, >depth,
>>   >bits_per_pixel);
>>   fb->pixel_format = mode_cmd->pixel_format;
>> + fb->flags = mode_cmd->flags;
>>
>>   return 0;
>>  }
>> --
>> 1.9.1
>



-- 
Benjamin Gaignard

Graphic Working Group

Linaro.org ? Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog

[Nouveau] [PATCH 09/17] drm/radeon: use common fence implementation for fences

[Christian König]

Am 22.07.2014 17:17, schrieb Daniel Vetter:
> On Tue, Jul 22, 2014 at 3:45 PM, Christian K?nig
>  wrote:
>>> Would that be something you can agree to?
>>
>> No, the whole enable_signaling stuff should go away. No callback from the
>> driver into the fence code, only the other way around.
>>
>> fence->signaled as well as fence->wait should become mandatory and only
>> called from process context without holding any locks, neither atomic nor
>> any mutex/semaphore (rcu might be ok).
> So for the enable_signaling, that's optional already. It's only for
> drivers that don't want to keep interrupts enabled all the time. You
> can opt out of that easily.
>
> Wrt holding no locks at all while calling into any fence functions,
> that's just not going to work out. The point here is to make different
> drivers work together and we can rework all the ttm and i915 code to
> work locklessly in all cases where they need to wait for someone to
> complete rendering. Or at least I don't think that's feasible. So if
> you insist that no one might call into radeon code then we simply need
> to exclude radeon from participating in any shared fencing. But that's
> a bit pointless.
>
>>> Like I've said I think restricting the insanity other people are willing
>>> to live with just because you don't like it isn't right. But it is
>>> certainly right for you to insist on not being forced into any such
>>> design. I think the above would achieve this.
>>
>> I don't think so. If it's just me I would say that I'm just to cautious and
>> the idea is still save to apply to the whole kernel.
>>
>> But since Dave, Jerome and Ben seems to have similar concerns I think we
>> need to agree to a minimum and save interface for all drivers.
> Well I haven't yet seen a proposal that actually works.

How about this:

Drivers exporting fences need to provide a fence->signaled and a 
fence->wait function, everything else like fence->enable_signaling or 
calling fence_signaled() from the driver is optional.

Drivers wanting to use exported fences don't call fence->signaled or 
fence->wait in atomic or interrupt context, and not with holding any 
global locking primitives (like mmap_sem etc...). Holding locking 
primitives local to the driver is ok, as long as they don't conflict 
with anything possible used by their own fence implementation.

Christian.

>   From an intel
> pov I don't care that much since we don't care about desktop prime, so
> if radeon/nouveau don't want to do that, meh. Imo the design as-is is
> fairly sound, and as simple as it can get given the requirements. I
> haven't heard an  argument convincing me otherwise, so I guess we
> won't have prime support on linux that actually works, ever.
> -Daniel

[PATCH v6 05/14] drm/exynos: dsi: add TE interrupt handler to support LCD I80 interface

[Varka Bhadram]

On 07/22/2014 05:23 PM, YoungJun Cho wrote:
> Hi Varka,
>
> On 07/22/2014 08:14 PM, Varka Bhadram wrote:
>> On 07/22/2014 04:40 PM, YoungJun Cho wrote:
>>> Hi Varka,
>>>
>>> This irq handler should be registered in attach() and unregistered in
>>> detach().
>>>
>>> The devm_* APIs are released(freed) in remove(), right?
>>>
>>> Logically the panel could be attached and detached several times after
>>> dsi is probed and not removed.
>>> So I don't use devm_* APIs.
>>
>> You meant to say that in-case of GPIOs also you are following the same
>> thing ..?
>>
>> Means requesting the GPIOs and Releasing several times ..?
>>
>
> Yes, this TE gpio is came from panel.
> So it should be refresh whenever a (new) panel is attached.
>
In this case it would be fine. Thanks for the clarity.

-- 
Regards,
Varka Bhadram.

[PATCH 09/17] drm/radeon: use common fence implementation for fences

[Daniel Vetter]

On Tue, Jul 22, 2014 at 4:39 PM, Christian K?nig
 wrote:
> Am 22.07.2014 16:27, schrieb Maarten Lankhorst:
>
>> op 22-07-14 16:24, Christian K?nig schreef:

 No, you really shouldn't be doing much in the check anyway, it's meant
 to be a lightweight check. If you're not ready yet because of a lockup
 simply return not signaled yet.
>>>
>>> It's not only the lockup case from radeon I have in mind here. For
>>> userspace queues it might be necessary to call copy_from_user to figure out
>>> if a fence is signaled or not.
>>>
>>> Returning false all the time is probably not a good idea either.
>>
>> Having userspace implement a fence sounds like an awful idea, why would
>> you want to do that?
>
>
> Marketing moves in mysterious ways. Don't ask me, but that the direction it
> currently moves with userspace queues and IOMMU etc...

Fence-based syncing between userspace queues submitted stuff through
doorbells and anything submitted by the general simply wont work.
Which is why I think the doorbell is a stupid interface since I just
don't see cameras and v4l devices implementing all that complexity to
get a pure userspace side sync solution.

But that's a different problem really, and I guess marketing will
eventually figure this one out, too.
-Daniel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[Nouveau] [PATCH 09/17] drm/radeon: use common fence implementation for fences

[Maarten Lankhorst]

Hey,

op 22-07-14 17:02, Christian K?nig schreef:
> Am 22.07.2014 16:44, schrieb Maarten Lankhorst:
>> op 22-07-14 15:45, Christian K?nig schreef:
>>> Am 22.07.2014 15:26, schrieb Daniel Vetter:
 On Tue, Jul 22, 2014 at 02:19:57PM +0200, Christian K?nig wrote:
> Am 22.07.2014 13:57, schrieb Daniel Vetter:
>> On Tue, Jul 22, 2014 at 01:46:07PM +0200, Daniel Vetter wrote:
>>> On Tue, Jul 22, 2014 at 10:43:13AM +0200, Christian K?nig wrote:
 Am 22.07.2014 06:05, schrieb Dave Airlie:
> On 9 July 2014 22:29, Maarten Lankhorst  canonical.com> wrote:
>> Signed-off-by: Maarten Lankhorst 
>> ---
>>drivers/gpu/drm/radeon/radeon.h|   15 +-
>>drivers/gpu/drm/radeon/radeon_device.c |   60 -
>>drivers/gpu/drm/radeon/radeon_fence.c  |  223 
>> ++--
>>3 files changed, 248 insertions(+), 50 deletions(-)
>>
>   From what I can see this is still suffering from the problem that we
> need to find a proper solution to,
>
> My summary of the issues after talking to Jerome and Ben and
> re-reading things is:
>
> We really need to work out a better interface into the drivers to be
> able to avoid random atomic entrypoints,
 Which is exactly what I criticized from the very first beginning. Good 
 to
 know that I'm not the only one thinking that this isn't such a good 
 idea.
>>> I guess I've lost context a bit, but which atomic entry point are we
>>> talking about? Afaics the only one that's mandatory is the is
>>> fence->signaled callback to check whether a fence really has been
>>> signalled. It's used internally by the fence code to avoid spurious
>>> wakeups. Afaik that should be doable already on any hardware. If that's
>>> not the case then we can always track the signalled state in software 
>>> and
>>> double-check in a worker thread before updating the sw state. And wrap
>>> this all up into a special fence class if there's more than one driver
>>> needing this.
>> One thing I've forgotten: The i915 scheduler that's floating around runs
>> its bottom half from irq context. So I really want to be able to check
>> fence state from irq context and I also want to make it possible
>> (possible! not mandatory) to register callbacks which are run from any
>> context asap after the fence is signalled.
> NAK, that's just the bad design I've talked about. Checking fence state
> inside the same driver from interrupt context is OK, because it's the
> drivers interrupt that we are talking about here.
>
> Checking fence status from another drivers interrupt context is what 
> really
> concerns me here, cause your driver doesn't have the slightest idea if the
> called driver is really capable of checking the fence right now.
 I guess my mail hasn't been clear then. If you don't like it we could add
 a bit of glue to insulate the madness and bad design i915 might do from
 radeon. That imo doesn't invalidate the overall fence interfaces.

 So what about the following:
 - fence->enabling_signaling is restricted to be called from process
 context. We don't use any different yet, so would boild down to adding 
 a
 WARN_ON(in_interrupt) or so to fence_enable_sw_signalling.

 - Make fence->signaled optional (already the case) and don't implement it
 in readon (i.e. reduce this patch here). Only downside is that radeon
 needs to correctly (i.e. without races or so) call fence_signal. And 
 the
 cross-driver synchronization might be a bit less efficient. Note that
 you can call fence_signal from wherever you want to, so hopefully that
 doesn't restrict your implementation.

 End result: No one calls into radeon from interrupt context, and this is
 guaranteed.

 Would that be something you can agree to?
>>> No, the whole enable_signaling stuff should go away. No callback from the 
>>> driver into the fence code, only the other way around.
>>>
>>> fence->signaled as well as fence->wait should become mandatory and only 
>>> called from process context without holding any locks, neither atomic nor 
>>> any mutex/semaphore (rcu might be ok).
>> fence->wait is mandatory, and already requires sleeping.
>>
>> If .signaled is not implemented there is no guarantee the fence will be
>> signaled sometime soon, this is also why enable_signaling exists, to
>> allow the driver to flush. I get it that it doesn't apply to radeon and 
>> nouveau,
>> but for other drivers that could be necessary, like vmwgfx.
>>
>> Ironically that is also a part of the ttm fence, except it was called flush 
>> there.
>
> Then call it flush again and make it optional like in TTM.
You've posted a lot of concerns, but I 

[Nouveau] [PATCH 09/17] drm/radeon: use common fence implementation for fences

[Daniel Vetter]

On Tue, Jul 22, 2014 at 3:45 PM, Christian K?nig
 wrote:
>> Would that be something you can agree to?
>
>
> No, the whole enable_signaling stuff should go away. No callback from the
> driver into the fence code, only the other way around.
>
> fence->signaled as well as fence->wait should become mandatory and only
> called from process context without holding any locks, neither atomic nor
> any mutex/semaphore (rcu might be ok).

So for the enable_signaling, that's optional already. It's only for
drivers that don't want to keep interrupts enabled all the time. You
can opt out of that easily.

Wrt holding no locks at all while calling into any fence functions,
that's just not going to work out. The point here is to make different
drivers work together and we can rework all the ttm and i915 code to
work locklessly in all cases where they need to wait for someone to
complete rendering. Or at least I don't think that's feasible. So if
you insist that no one might call into radeon code then we simply need
to exclude radeon from participating in any shared fencing. But that's
a bit pointless.

>> Like I've said I think restricting the insanity other people are willing
>> to live with just because you don't like it isn't right. But it is
>> certainly right for you to insist on not being forced into any such
>> design. I think the above would achieve this.
>
>
> I don't think so. If it's just me I would say that I'm just to cautious and
> the idea is still save to apply to the whole kernel.
>
> But since Dave, Jerome and Ben seems to have similar concerns I think we
> need to agree to a minimum and save interface for all drivers.

Well I haven't yet seen a proposal that actually works. From an intel
pov I don't care that much since we don't care about desktop prime, so
if radeon/nouveau don't want to do that, meh. Imo the design as-is is
fairly sound, and as simple as it can get given the requirements. I
haven't heard an  argument convincing me otherwise, so I guess we
won't have prime support on linux that actually works, ever.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 09/17] drm/radeon: use common fence implementation for fences

[Christian König]

Am 22.07.2014 16:47, schrieb Maarten Lankhorst:
> op 22-07-14 16:39, Christian K?nig schreef:
>> Am 22.07.2014 16:27, schrieb Maarten Lankhorst:
>>> op 22-07-14 16:24, Christian K?nig schreef:
> No, you really shouldn't be doing much in the check anyway, it's meant to 
> be a lightweight check. If you're not ready yet because of a lockup 
> simply return not signaled yet.
 It's not only the lockup case from radeon I have in mind here. For 
 userspace queues it might be necessary to call copy_from_user to figure 
 out if a fence is signaled or not.

 Returning false all the time is probably not a good idea either.
>>> Having userspace implement a fence sounds like an awful idea, why would you 
>>> want to do that?
>> Marketing moves in mysterious ways. Don't ask me, but that the direction it 
>> currently moves with userspace queues and IOMMU etc...
>>
>>> A fence could be exported to userspace, but that would only mean it can 
>>> wait for it to be signaled with an interface like poll..
>> Yeah agree totally, but the point for the fence interface is that I can't 
>> predict what's necessary to check if a fence is signaled or not on future 
>> hardware.
>>
>> For the currently available radeon hardware I can say that reading a value 
>> from a kernel page is pretty much all you need. But for older hardware that 
>> was reading from a register which might become very tricky if the hardware 
>> is power off or currently inside a reset cycle.
>>
>> Because off this I would avoid any such interface if it's not absolutely 
>> required by some use case, and currently I don't see this requirement 
>> because the functionality you want to archive could be implemented without 
>> this.
> Oh? I've already done that in radeon_fence, there is no way enable_signaling 
> will fiddle with hardware registers during a reset cycle.
> I've also made sure that __radeon_fence_is_signaled grabs exclusive_lock in 
> read mode before touching any hw state.
>
> Older hardware also doesn't implement optimus, so I think power off is not 
> much of a worry for them, if you could point me at the checking done for that 
> I could make sure that this is the case.

I'm not talking about any specific radeon hardware or use case here. As 
far as I can see you indeed solved all driver problems with the current 
interface design.

The question I'm raising is if the current interface design needs as 
complex as it is. And my answer to this is a clear *no*, so why do you 
want to stick with this design? I still haven't understood that.

If it's just to support a further feature of direct synchronization in 
interrupt context between different drivers then I must clearly say that 
this is a NAK, cause you add complexity to the kernel that isn't necessary.

Christian.

>
> ~Maarten
>

[Intel-gfx] [PATCH 09/11] i915: add DP 1.2 MST support (v0.6)

[Paulo Zanoni]

2014-06-05 1:01 GMT-03:00 Dave Airlie :
> From: Dave Airlie 
>
> This adds DP 1.2 MST support on Haswell systems.

Hi

It looks like drm-intel-nightly now includes this patch. It actually
includes v7, but I couldn't find it on my mail dirs.

Just by booting the machine with this patch, I get:

[   11.013593] [ cut here ]
[   11.013627] WARNING: CPU: 1 PID: 389 at
drivers/gpu/drm/i915/intel_fbdev.c:383
intel_fb_initial_config+0x3ca/0x660 [i915]()
[   11.013629] Modules linked in: i2c_designware_platform i915(+)
i2c_designware_core i2c_algo_bit drm_kms_helper drm sg sd_mod ahci
ehci_pci libahci ehci_hcd e1000e xhci_hcd sdhci_acpi sdhci
[   11.013651] CPU: 1 PID: 389 Comm: udevd Not tainted
3.16.0-rc6.1407221626+ #673
[   11.013654] Hardware name: Intel Corporation Shark Bay Client
platform/WhiteTip Mountain 1, BIOS HSWLPTU1.86C.0137.R00.1403031632
03/03/2014
[   11.013656]  0009 88009d14b790 816b6a61

[   11.013662]  88009d14b7c8 81075d88 8801483db10b
880148279400
[   11.013667]  88009d0caf00 0003 0003
88009d14b7d8
[   11.013672] Call Trace:
[   11.013680]  [] dump_stack+0x4d/0x66
[   11.013686]  [] warn_slowpath_common+0x78/0xa0
[   11.013690]  [] warn_slowpath_null+0x15/0x20
[   11.013714]  [] intel_fb_initial_config+0x3ca/0x660 [i915]
[   11.013724]  [] drm_setup_crtcs+0x17f/0x830
[drm_kms_helper]
[   11.013729]  [] ? trace_hardirqs_on_caller+0x15d/0x200
[   11.013736]  []
drm_fb_helper_initial_config+0x19a/0x4d0 [drm_kms_helper]
[   11.013740]  [] ? trace_hardirqs_on_caller+0x15d/0x200
[   11.013762]  [] intel_fbdev_initial_config+0x1a/0x20 [i915]
[   11.013789]  [] i915_driver_load+0x111b/0x1130 [i915]
[   11.013803]  [] drm_dev_register+0xa5/0x100 [drm]
[   11.013815]  [] drm_get_pci_dev+0x88/0x1f0 [drm]
[   11.013834]  [] i915_pci_probe+0x36/0x50 [i915]
[   11.013839]  [] local_pci_probe+0x3d/0xa0
[   11.013843]  [] ? pci_match_device+0xe5/0x110
[   11.013847]  [] pci_device_probe+0xc9/0x120
[   11.013853]  [] driver_probe_device+0x89/0x250
[   11.013857]  [] __driver_attach+0x8b/0x90
[   11.013879]  [] ? __device_attach+0x40/0x40
[   11.013883]  [] bus_for_each_dev+0x63/0xa0
[   11.013888]  [] driver_attach+0x19/0x20
[   11.013892]  [] bus_add_driver+0x178/0x230
[   11.013896]  [] driver_register+0x5f/0xf0
[   11.013899]  [] __pci_register_driver+0x58/0x60
[   11.013911]  [] drm_pci_init+0xfa/0x130 [drm]
[   11.013915]  [] ? trace_hardirqs_on+0xd/0x10
[   11.013918]  [] ? 0xa0221fff
[   11.013937]  [] i915_init+0x89/0x90 [i915]
[   11.013942]  [] do_one_initcall+0x98/0x1f0
[   11.013947]  [] ? __vunmap+0xa2/0x100
[   11.013952]  [] load_module+0x1bea/0x22d0
[   11.013956]  [] ? symbol_put_addr+0x40/0x40
[   11.013960]  [] ? copy_module_from_fd.isra.49+0x119/0x170
[   11.013965]  [] SyS_finit_module+0x76/0x80
[   11.013970]  [] system_call_fastpath+0x16/0x1b
[   11.013973] ---[ end trace e4d35cd76b2fc39f ]---

This is the WARN that makes the code print
[drm:intel_fb_initial_config] connector HDMI-A-2 has no encoder or
crtc, skipping


Then, if I do a normal S3 suspend/resume cycle with the desktop
enabled and everything running, I get:

[   32.431306] [ cut here ]
[   32.431341] WARNING: CPU: 2 PID: 3200 at
drivers/gpu/drm/i915/intel_pm.c:4979
intel_suspend_gt_powersave+0x49/0x50 [i915]()
[   32.431370] Modules linked in: fuse intel_rapl x86_pkg_temp_thermal
intel_powerclamp serio_raw i2c_i801 i2c_designware_platform
i2c_designware_core i915 i2c_algo_bit drm_kms_helper drm mei_me mei sg
sd_mod ehci_pci ehci_hcd ahci libahci e1000e xhci_hcd sdhci_acpi sdhci
[   32.431373] CPU: 2 PID: 3200 Comm: kworker/u16:14 Tainted: G
W 3.16.0-rc6.1407221626+ #673
[   32.431375] Hardware name: Intel Corporation Shark Bay Client
platform/WhiteTip Mountain 1, BIOS HSWLPTU1.86C.0137.R00.1403031632
03/03/2014
[   32.431379] Workqueue: events_unbound async_run_entry_fn
[   32.431383]  0009 88009d1f3be8 816b6a61

[   32.431386]  88009d1f3c20 81075d88 88014861

[   32.431390]  88014861 88009d86e758 81a5d47a
88009d1f3c30
[   32.431390] Call Trace:
[   32.431396]  [] dump_stack+0x4d/0x66
[   32.431399]  [] warn_slowpath_common+0x78/0xa0
[   32.431402]  [] warn_slowpath_null+0x15/0x20
[   32.431413]  [] intel_suspend_gt_powersave+0x49/0x50 [i915]
[   32.431422]  [] i915_drm_freeze+0x96/0x190 [i915]
[   32.431430]  [] i915_pm_suspend+0x2a/0x50 [i915]
[   32.431434]  [] pci_pm_suspend+0x71/0x160
[   32.431437]  [] ? pci_pm_freeze+0xe0/0xe0
[   32.431440]  [] dpm_run_callback+0x3a/0x130
[   32.431443]  [] __device_suspend+0xf0/0x2f0
[   32.431445]  [] async_suspend+0x1a/0xa0
[   32.431448]  [] async_run_entry_fn+0x34/0x120
[   32.431451]  [] process_one_work+0x1cf/0x550
[   32.431454]  [] ? process_one_work+0x16f/0x550
[   32.431457]  [] worker_thread+0x63/0x540
[   32.431461]  [] 

[Nouveau] [PATCH 09/17] drm/radeon: use common fence implementation for fences

[Christian König]

Am 22.07.2014 16:44, schrieb Maarten Lankhorst:
> op 22-07-14 15:45, Christian K?nig schreef:
>> Am 22.07.2014 15:26, schrieb Daniel Vetter:
>>> On Tue, Jul 22, 2014 at 02:19:57PM +0200, Christian K?nig wrote:
 Am 22.07.2014 13:57, schrieb Daniel Vetter:
> On Tue, Jul 22, 2014 at 01:46:07PM +0200, Daniel Vetter wrote:
>> On Tue, Jul 22, 2014 at 10:43:13AM +0200, Christian K?nig wrote:
>>> Am 22.07.2014 06:05, schrieb Dave Airlie:
 On 9 July 2014 22:29, Maarten Lankhorst >>> canonical.com> wrote:
> Signed-off-by: Maarten Lankhorst 
> ---
>drivers/gpu/drm/radeon/radeon.h|   15 +-
>drivers/gpu/drm/radeon/radeon_device.c |   60 -
>drivers/gpu/drm/radeon/radeon_fence.c  |  223 
> ++--
>3 files changed, 248 insertions(+), 50 deletions(-)
>
   From what I can see this is still suffering from the problem that we
 need to find a proper solution to,

 My summary of the issues after talking to Jerome and Ben and
 re-reading things is:

 We really need to work out a better interface into the drivers to be
 able to avoid random atomic entrypoints,
>>> Which is exactly what I criticized from the very first beginning. Good 
>>> to
>>> know that I'm not the only one thinking that this isn't such a good 
>>> idea.
>> I guess I've lost context a bit, but which atomic entry point are we
>> talking about? Afaics the only one that's mandatory is the is
>> fence->signaled callback to check whether a fence really has been
>> signalled. It's used internally by the fence code to avoid spurious
>> wakeups. Afaik that should be doable already on any hardware. If that's
>> not the case then we can always track the signalled state in software and
>> double-check in a worker thread before updating the sw state. And wrap
>> this all up into a special fence class if there's more than one driver
>> needing this.
> One thing I've forgotten: The i915 scheduler that's floating around runs
> its bottom half from irq context. So I really want to be able to check
> fence state from irq context and I also want to make it possible
> (possible! not mandatory) to register callbacks which are run from any
> context asap after the fence is signalled.
 NAK, that's just the bad design I've talked about. Checking fence state
 inside the same driver from interrupt context is OK, because it's the
 drivers interrupt that we are talking about here.

 Checking fence status from another drivers interrupt context is what really
 concerns me here, cause your driver doesn't have the slightest idea if the
 called driver is really capable of checking the fence right now.
>>> I guess my mail hasn't been clear then. If you don't like it we could add
>>> a bit of glue to insulate the madness and bad design i915 might do from
>>> radeon. That imo doesn't invalidate the overall fence interfaces.
>>>
>>> So what about the following:
>>> - fence->enabling_signaling is restricted to be called from process
>>> context. We don't use any different yet, so would boild down to adding a
>>> WARN_ON(in_interrupt) or so to fence_enable_sw_signalling.
>>>
>>> - Make fence->signaled optional (already the case) and don't implement it
>>> in readon (i.e. reduce this patch here). Only downside is that radeon
>>> needs to correctly (i.e. without races or so) call fence_signal. And the
>>> cross-driver synchronization might be a bit less efficient. Note that
>>> you can call fence_signal from wherever you want to, so hopefully that
>>> doesn't restrict your implementation.
>>>
>>> End result: No one calls into radeon from interrupt context, and this is
>>> guaranteed.
>>>
>>> Would that be something you can agree to?
>> No, the whole enable_signaling stuff should go away. No callback from the 
>> driver into the fence code, only the other way around.
>>
>> fence->signaled as well as fence->wait should become mandatory and only 
>> called from process context without holding any locks, neither atomic nor 
>> any mutex/semaphore (rcu might be ok).
> fence->wait is mandatory, and already requires sleeping.
>
> If .signaled is not implemented there is no guarantee the fence will be
> signaled sometime soon, this is also why enable_signaling exists, to
> allow the driver to flush. I get it that it doesn't apply to radeon and 
> nouveau,
> but for other drivers that could be necessary, like vmwgfx.
>
> Ironically that is also a part of the ttm fence, except it was called flush 
> there.

Then call it flush again and make it optional like in TTM.

> I would also like to note that ttm_bo_wait currently is also a function that 
> currently uses is_signaled from atomic_context...

I know, but TTM is only called from inside a single driver, no inter 
driver 

[PATCH 09/17] drm/radeon: use common fence implementation for fences

[Maarten Lankhorst]

op 22-07-14 16:39, Christian K?nig schreef:
> Am 22.07.2014 16:27, schrieb Maarten Lankhorst:
>> op 22-07-14 16:24, Christian K?nig schreef:
 No, you really shouldn't be doing much in the check anyway, it's meant to 
 be a lightweight check. If you're not ready yet because of a lockup simply 
 return not signaled yet.
>>> It's not only the lockup case from radeon I have in mind here. For 
>>> userspace queues it might be necessary to call copy_from_user to figure out 
>>> if a fence is signaled or not.
>>>
>>> Returning false all the time is probably not a good idea either.
>> Having userspace implement a fence sounds like an awful idea, why would you 
>> want to do that?
>
> Marketing moves in mysterious ways. Don't ask me, but that the direction it 
> currently moves with userspace queues and IOMMU etc...
>
>> A fence could be exported to userspace, but that would only mean it can wait 
>> for it to be signaled with an interface like poll..
>
> Yeah agree totally, but the point for the fence interface is that I can't 
> predict what's necessary to check if a fence is signaled or not on future 
> hardware.
>
> For the currently available radeon hardware I can say that reading a value 
> from a kernel page is pretty much all you need. But for older hardware that 
> was reading from a register which might become very tricky if the hardware is 
> power off or currently inside a reset cycle.
>
> Because off this I would avoid any such interface if it's not absolutely 
> required by some use case, and currently I don't see this requirement because 
> the functionality you want to archive could be implemented without this.
Oh? I've already done that in radeon_fence, there is no way enable_signaling 
will fiddle with hardware registers during a reset cycle.
I've also made sure that __radeon_fence_is_signaled grabs exclusive_lock in 
read mode before touching any hw state.

Older hardware also doesn't implement optimus, so I think power off is not much 
of a worry for them, if you could point me at the checking done for that I 
could make sure that this is the case.

~Maarten

[PATCH v6 05/14] drm/exynos: dsi: add TE interrupt handler to support LCD I80 interface

[Varka Bhadram]

On 07/22/2014 04:40 PM, YoungJun Cho wrote:
> Hi Varka,
>
> This irq handler should be registered in attach() and unregistered in 
> detach().
>
> The devm_* APIs are released(freed) in remove(), right?
>
> Logically the panel could be attached and detached several times after 
> dsi is probed and not removed.
> So I don't use devm_* APIs.

You meant to say that in-case of GPIOs also you are following the same thing ..?

Means requesting the GPIOs and Releasing several times ..?

>
>>
>


-- 
Regards,
Varka Bhadram.

[Nouveau] [PATCH 09/17] drm/radeon: use common fence implementation for fences

[Maarten Lankhorst]

op 22-07-14 15:45, Christian K?nig schreef:
> Am 22.07.2014 15:26, schrieb Daniel Vetter:
>> On Tue, Jul 22, 2014 at 02:19:57PM +0200, Christian K?nig wrote:
>>> Am 22.07.2014 13:57, schrieb Daniel Vetter:
 On Tue, Jul 22, 2014 at 01:46:07PM +0200, Daniel Vetter wrote:
> On Tue, Jul 22, 2014 at 10:43:13AM +0200, Christian K?nig wrote:
>> Am 22.07.2014 06:05, schrieb Dave Airlie:
>>> On 9 July 2014 22:29, Maarten Lankhorst >> canonical.com> wrote:
 Signed-off-by: Maarten Lankhorst 
 ---
   drivers/gpu/drm/radeon/radeon.h|   15 +-
   drivers/gpu/drm/radeon/radeon_device.c |   60 -
   drivers/gpu/drm/radeon/radeon_fence.c  |  223 
 ++--
   3 files changed, 248 insertions(+), 50 deletions(-)

>>>  From what I can see this is still suffering from the problem that we
>>> need to find a proper solution to,
>>>
>>> My summary of the issues after talking to Jerome and Ben and
>>> re-reading things is:
>>>
>>> We really need to work out a better interface into the drivers to be
>>> able to avoid random atomic entrypoints,
>> Which is exactly what I criticized from the very first beginning. Good to
>> know that I'm not the only one thinking that this isn't such a good idea.
> I guess I've lost context a bit, but which atomic entry point are we
> talking about? Afaics the only one that's mandatory is the is
> fence->signaled callback to check whether a fence really has been
> signalled. It's used internally by the fence code to avoid spurious
> wakeups. Afaik that should be doable already on any hardware. If that's
> not the case then we can always track the signalled state in software and
> double-check in a worker thread before updating the sw state. And wrap
> this all up into a special fence class if there's more than one driver
> needing this.
 One thing I've forgotten: The i915 scheduler that's floating around runs
 its bottom half from irq context. So I really want to be able to check
 fence state from irq context and I also want to make it possible
 (possible! not mandatory) to register callbacks which are run from any
 context asap after the fence is signalled.
>>> NAK, that's just the bad design I've talked about. Checking fence state
>>> inside the same driver from interrupt context is OK, because it's the
>>> drivers interrupt that we are talking about here.
>>>
>>> Checking fence status from another drivers interrupt context is what really
>>> concerns me here, cause your driver doesn't have the slightest idea if the
>>> called driver is really capable of checking the fence right now.
>> I guess my mail hasn't been clear then. If you don't like it we could add
>> a bit of glue to insulate the madness and bad design i915 might do from
>> radeon. That imo doesn't invalidate the overall fence interfaces.
>>
>> So what about the following:
>> - fence->enabling_signaling is restricted to be called from process
>>context. We don't use any different yet, so would boild down to adding a
>>WARN_ON(in_interrupt) or so to fence_enable_sw_signalling.
>>
>> - Make fence->signaled optional (already the case) and don't implement it
>>in readon (i.e. reduce this patch here). Only downside is that radeon
>>needs to correctly (i.e. without races or so) call fence_signal. And the
>>cross-driver synchronization might be a bit less efficient. Note that
>>you can call fence_signal from wherever you want to, so hopefully that
>>doesn't restrict your implementation.
>>
>> End result: No one calls into radeon from interrupt context, and this is
>> guaranteed.
>>
>> Would that be something you can agree to?
>
> No, the whole enable_signaling stuff should go away. No callback from the 
> driver into the fence code, only the other way around.
>
> fence->signaled as well as fence->wait should become mandatory and only 
> called from process context without holding any locks, neither atomic nor any 
> mutex/semaphore (rcu might be ok).
fence->wait is mandatory, and already requires sleeping.

If .signaled is not implemented there is no guarantee the fence will be
signaled sometime soon, this is also why enable_signaling exists, to
allow the driver to flush. I get it that it doesn't apply to radeon and nouveau,
but for other drivers that could be necessary, like vmwgfx.

Ironically that is also a part of the ttm fence, except it was called flush 
there.

I would also like to note that ttm_bo_wait currently is also a function that 
currently uses is_signaled from atomic_context...

For the more complicated locking worries: Lockdep is your friend, use 
PROVE_LOCKING and find bugs before they trigger. ;-)

>> Like I've said I think restricting the insanity other people are willing
>> to live with just because you don't like it isn't right. But it is
>> certainly right for you to insist on 

[PATCH 09/17] drm/radeon: use common fence implementation for fences

[Christian König]

Am 22.07.2014 16:27, schrieb Maarten Lankhorst:
> op 22-07-14 16:24, Christian K?nig schreef:
>>> No, you really shouldn't be doing much in the check anyway, it's meant to 
>>> be a lightweight check. If you're not ready yet because of a lockup simply 
>>> return not signaled yet.
>> It's not only the lockup case from radeon I have in mind here. For userspace 
>> queues it might be necessary to call copy_from_user to figure out if a fence 
>> is signaled or not.
>>
>> Returning false all the time is probably not a good idea either.
> Having userspace implement a fence sounds like an awful idea, why would you 
> want to do that?

Marketing moves in mysterious ways. Don't ask me, but that the direction 
it currently moves with userspace queues and IOMMU etc...

> A fence could be exported to userspace, but that would only mean it can wait 
> for it to be signaled with an interface like poll..

Yeah agree totally, but the point for the fence interface is that I 
can't predict what's necessary to check if a fence is signaled or not on 
future hardware.

For the currently available radeon hardware I can say that reading a 
value from a kernel page is pretty much all you need. But for older 
hardware that was reading from a register which might become very tricky 
if the hardware is power off or currently inside a reset cycle.

Because off this I would avoid any such interface if it's not absolutely 
required by some use case, and currently I don't see this requirement 
because the functionality you want to archive could be implemented 
without this.

Christian.

>
> ~Maarten
>

Looking for a start point for fixing a bug

[Адонай Элохим]

Hello all!

I have some spare time and knowledge in C to try to fix some bugs I am
seeing on my machine.
So I've checked out and compiled all git trees that I may need and now I'm
beginning to read articles.

And this is the point from where I don't know where to go. I want to fix
particular bug #79806 [1].
For me there are many places where this bug can hide - mesa? dri? radeon
kernel module? and I just don't know whether should I start reading
articles about mesa hacking or about dri architecture or about kernel
module development.

Now I think the best thing for me is to start looking through radeon kernel
module code (I've got ingenious idea that power management resides there)
and read more about its architecture. Is this right? I mean, I just want to
find out, is this a right place to start looking at for this bug?

P.S. Sorry for my English in case it's not good, I'm learning it now

P.P.S. And thanks for your hard work!

---
[1] https://bugs.freedesktop.org/show_bug.cgi?id=79806
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140722/f4fb0688/attachment.html>

[PATCH v6 05/14] drm/exynos: dsi: add TE interrupt handler to support LCD I80 interface

[Varka Bhadram]

On 07/22/2014 04:19 PM, YoungJun Cho wrote:

(...)

> + ret = gpio_request_one(dsi->te_gpio, GPIOF_IN, "te_gpio");

devm_* APIs..?

> + if (ret) {
> + dev_err(dsi->dev, "gpio request failed with %d\n", ret);
> + goto out;
> + }
> +
> + /*
> +  * This TE GPIO IRQ should not be set to IRQ_NOAUTOEN, because panel
> +  * calls drm_panel_init() first then calls mipi_dsi_attach() in probe().
> +  * It means that te_gpio is invalid when exynos_dsi_enable_irq() is
> +  * called by drm_panel_init() before panel is attached.
> +  */
> + ret = request_threaded_irq(gpio_to_irq(dsi->te_gpio),
> + exynos_dsi_te_irq_handler, NULL,
> + IRQF_TRIGGER_RISING, "TE", dsi);

why don't we use devm_request_threaded_irq()..?


-- 
Regards,
Varka Bhadram.

[PATCH 09/17] drm/radeon: use common fence implementation for fences

[Maarten Lankhorst]

op 22-07-14 16:24, Christian K?nig schreef:
>> No, you really shouldn't be doing much in the check anyway, it's meant to be 
>> a lightweight check. If you're not ready yet because of a lockup simply 
>> return not signaled yet.
> It's not only the lockup case from radeon I have in mind here. For userspace 
> queues it might be necessary to call copy_from_user to figure out if a fence 
> is signaled or not.
>
> Returning false all the time is probably not a good idea either.
Having userspace implement a fence sounds like an awful idea, why would you 
want to do that?

A fence could be exported to userspace, but that would only mean it can wait 
for it to be signaled with an interface like poll..

~Maarten

[PATCH 09/17] drm/radeon: use common fence implementation for fences

[Christian König]

> No, you really shouldn't be doing much in the check anyway, it's meant to be 
> a lightweight check. If you're not ready yet because of a lockup simply 
> return not signaled yet.
It's not only the lockup case from radeon I have in mind here. For 
userspace queues it might be necessary to call copy_from_user to figure 
out if a fence is signaled or not.

Returning false all the time is probably not a good idea either.

Christian.

Am 22.07.2014 16:05, schrieb Maarten Lankhorst:
> op 22-07-14 14:19, Christian K?nig schreef:
>> Am 22.07.2014 13:57, schrieb Daniel Vetter:
>>> On Tue, Jul 22, 2014 at 01:46:07PM +0200, Daniel Vetter wrote:
 On Tue, Jul 22, 2014 at 10:43:13AM +0200, Christian K?nig wrote:
> Am 22.07.2014 06:05, schrieb Dave Airlie:
>> On 9 July 2014 22:29, Maarten Lankhorst > canonical.com> wrote:
>>> Signed-off-by: Maarten Lankhorst 
>>> ---
>>>drivers/gpu/drm/radeon/radeon.h|   15 +-
>>>drivers/gpu/drm/radeon/radeon_device.c |   60 -
>>>drivers/gpu/drm/radeon/radeon_fence.c  |  223 
>>> ++--
>>>3 files changed, 248 insertions(+), 50 deletions(-)
>>>
>>   From what I can see this is still suffering from the problem that we
>> need to find a proper solution to,
>>
>> My summary of the issues after talking to Jerome and Ben and
>> re-reading things is:
>>
>> We really need to work out a better interface into the drivers to be
>> able to avoid random atomic entrypoints,
> Which is exactly what I criticized from the very first beginning. Good to
> know that I'm not the only one thinking that this isn't such a good idea.
 I guess I've lost context a bit, but which atomic entry point are we
 talking about? Afaics the only one that's mandatory is the is
 fence->signaled callback to check whether a fence really has been
 signalled. It's used internally by the fence code to avoid spurious
 wakeups. Afaik that should be doable already on any hardware. If that's
 not the case then we can always track the signalled state in software and
 double-check in a worker thread before updating the sw state. And wrap
 this all up into a special fence class if there's more than one driver
 needing this.
>>> One thing I've forgotten: The i915 scheduler that's floating around runs
>>> its bottom half from irq context. So I really want to be able to check
>>> fence state from irq context and I also want to make it possible
>>> (possible! not mandatory) to register callbacks which are run from any
>>> context asap after the fence is signalled.
>> NAK, that's just the bad design I've talked about. Checking fence state 
>> inside the same driver from interrupt context is OK, because it's the 
>> drivers interrupt that we are talking about here.
>>
>> Checking fence status from another drivers interrupt context is what really 
>> concerns me here, cause your driver doesn't have the slightest idea if the 
>> called driver is really capable of checking the fence right now.
> I think there is a usecase for having atomic context allowed with 
> fence_is_signaled, but I don't think there is one for interrupt context, so 
> it's good with me if fence_is_signaled cannot be called in interrupt context, 
> or with irqs disabled.
>
> fence_enable_sw_signaling disables interrupts because it holds fence->lock, 
> so in theory it could be called from any context including interrupts. But no 
> sane driver author does that, or at least I hope not..
>
> Would a sanity check like the one below be enough to allay your fears?
> 8<---
>
> diff --git a/include/linux/fence.h b/include/linux/fence.h
> index d174585b874b..c1a4519ba2f5 100644
> --- a/include/linux/fence.h
> +++ b/include/linux/fence.h
> @@ -143,6 +143,7 @@ struct fence_cb {
>* the second time will be a noop since it was already signaled.
>*
>* Notes on signaled:
> + * Called with interrupts enabled, and never from interrupt context.
>* May set fence->status if returning true.
>*
>* Notes on wait:
> @@ -268,15 +269,29 @@ fence_is_signaled_locked(struct fence *fence)
>   static inline bool
>   fence_is_signaled(struct fence *fence)
>   {
> + bool ret;
> +
>   if (test_bit(FENCE_FLAG_SIGNALED_BIT, >flags))
>   return true;
>   
> - if (fence->ops->signaled && fence->ops->signaled(fence)) {
> + if (!fence->ops->signaled)
> + return false;
> +
> + if (config_enabled(CONFIG_PROVE_LOCKING))
> + WARN_ON(in_interrupt() || irqs_disabled());
> +
> + if (config_enabled(CONFIG_DEBUG_ATOMIC_SLEEP))
> + preempt_disable();
> +
> + ret = fence->ops->signaled(fence);
> +
> + if (config_enabled(CONFIG_DEBUG_ATOMIC_SLEEP))
> + preempt_enable();
> +
> + if (ret)
>   fence_signal(fence);
> - return true;
> - }
>   
> - return false;
> + return ret;
>   }
>   
>  

[Bug 80901] [radeon] loading corrupts lspci entry + unloading crashes kernel

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=80901

--- Comment #6 from Alex Deucher  ---
(In reply to Andrea Patern? from comment #4)
> It does.
> 
> gandalf at the_shire ~ ? LIBGL_DEBUG=1 DRI_PRIME=1 glxinfo | grep "renderer
> string"
> libGL: Can't open configuration file /home/gandalf/.drirc: No such file or
> directory.
> libGL: Can't open configuration file /home/gandalf/.drirc: No such file or
> directory.
> OpenGL renderer string: Gallium 0.4 on AMD CAPE VERDE
> 
> Also, I just noticed that everytime I lauch anithing with PRIME, it takes a
> few seconds before returning the output. If you check the dmesg output I
> just attached, you'll see that it shows several driver-related outputs.
> Basically It prints them out everytime I lauch the program with PRIME

Everything appears to be working fine.  The delay and messages are due to the
card being powered down by default to save power and then powered back up when
you use it.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[PATCH 09/17] drm/radeon: use common fence implementation for fences

[Maarten Lankhorst]

op 22-07-14 14:19, Christian K?nig schreef:
> Am 22.07.2014 13:57, schrieb Daniel Vetter:
>> On Tue, Jul 22, 2014 at 01:46:07PM +0200, Daniel Vetter wrote:
>>> On Tue, Jul 22, 2014 at 10:43:13AM +0200, Christian K?nig wrote:
 Am 22.07.2014 06:05, schrieb Dave Airlie:
> On 9 July 2014 22:29, Maarten Lankhorst  canonical.com> wrote:
>> Signed-off-by: Maarten Lankhorst 
>> ---
>>   drivers/gpu/drm/radeon/radeon.h|   15 +-
>>   drivers/gpu/drm/radeon/radeon_device.c |   60 -
>>   drivers/gpu/drm/radeon/radeon_fence.c  |  223 
>> ++--
>>   3 files changed, 248 insertions(+), 50 deletions(-)
>>
>  From what I can see this is still suffering from the problem that we
> need to find a proper solution to,
>
> My summary of the issues after talking to Jerome and Ben and
> re-reading things is:
>
> We really need to work out a better interface into the drivers to be
> able to avoid random atomic entrypoints,
 Which is exactly what I criticized from the very first beginning. Good to
 know that I'm not the only one thinking that this isn't such a good idea.
>>> I guess I've lost context a bit, but which atomic entry point are we
>>> talking about? Afaics the only one that's mandatory is the is
>>> fence->signaled callback to check whether a fence really has been
>>> signalled. It's used internally by the fence code to avoid spurious
>>> wakeups. Afaik that should be doable already on any hardware. If that's
>>> not the case then we can always track the signalled state in software and
>>> double-check in a worker thread before updating the sw state. And wrap
>>> this all up into a special fence class if there's more than one driver
>>> needing this.
>> One thing I've forgotten: The i915 scheduler that's floating around runs
>> its bottom half from irq context. So I really want to be able to check
>> fence state from irq context and I also want to make it possible
>> (possible! not mandatory) to register callbacks which are run from any
>> context asap after the fence is signalled.
>
> NAK, that's just the bad design I've talked about. Checking fence state 
> inside the same driver from interrupt context is OK, because it's the drivers 
> interrupt that we are talking about here.
>
> Checking fence status from another drivers interrupt context is what really 
> concerns me here, cause your driver doesn't have the slightest idea if the 
> called driver is really capable of checking the fence right now.
I think there is a usecase for having atomic context allowed with 
fence_is_signaled, but I don't think there is one for interrupt context, so 
it's good with me if fence_is_signaled cannot be called in interrupt context, 
or with irqs disabled.

fence_enable_sw_signaling disables interrupts because it holds fence->lock, so 
in theory it could be called from any context including interrupts. But no sane 
driver author does that, or at least I hope not..

Would a sanity check like the one below be enough to allay your fears?
8<---

diff --git a/include/linux/fence.h b/include/linux/fence.h
index d174585b874b..c1a4519ba2f5 100644
--- a/include/linux/fence.h
+++ b/include/linux/fence.h
@@ -143,6 +143,7 @@ struct fence_cb {
  * the second time will be a noop since it was already signaled.
  *
  * Notes on signaled:
+ * Called with interrupts enabled, and never from interrupt context.
  * May set fence->status if returning true.
  *
  * Notes on wait:
@@ -268,15 +269,29 @@ fence_is_signaled_locked(struct fence *fence)
 static inline bool
 fence_is_signaled(struct fence *fence)
 {
+   bool ret;
+
if (test_bit(FENCE_FLAG_SIGNALED_BIT, >flags))
return true;

-   if (fence->ops->signaled && fence->ops->signaled(fence)) {
+   if (!fence->ops->signaled)
+   return false;
+
+   if (config_enabled(CONFIG_PROVE_LOCKING))
+   WARN_ON(in_interrupt() || irqs_disabled());
+
+   if (config_enabled(CONFIG_DEBUG_ATOMIC_SLEEP))
+   preempt_disable();
+
+   ret = fence->ops->signaled(fence);
+
+   if (config_enabled(CONFIG_DEBUG_ATOMIC_SLEEP))
+   preempt_enable();
+
+   if (ret)
fence_signal(fence);
-   return true;
-   }

-   return false;
+   return ret;
 }

 /**
8<

>> If the radeon hw/driver doesn't want to cope with that complexity we can
>> fully insolate it with the sw tracked fence state if you don't like
>> Maarten's radeon implementation. But forcing everyone to forgoe this just
>> because you don't like it and don't want to use it in radeon doesn't sound
>> right.
>
> While it's clearly a hack Maarten's solution for radeon would indeed work, 
> but that's not really the point here.
>
> It's just that I think leaking interrupt context from one driver into another 
> driver is just a really really bad idea from a design point of view.
>
> And calling into a 

[Bug 81644] Random crashes on RadeonSI with Chromium.

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=81644

Aaron B  changed:

   What|Removed |Added

   Hardware|Other   |x86-64 (AMD64)
   Priority|medium  |high

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140722/5ba01c21/attachment.html>

[Bug 81644] New: Random crashes on RadeonSI with Chromium.

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=81644

  Priority: medium
Bug ID: 81644
  Assignee: dri-devel at lists.freedesktop.org
   Summary: Random crashes on RadeonSI with Chromium.
  Severity: normal
Classification: Unclassified
OS: All
  Reporter: aaronbottegal at gmail.com
  Hardware: Other
Status: NEW
   Version: git
 Component: Drivers/Gallium/radeonsi
   Product: Mesa

Chromium randomly crashes with RadeonSI driver when using Chromium. Most
usually with Youtube videos. Everything is current git from Oibaf PPA using
Mint 17 Cinnamon, although I'm about to move to Arch Linux so I don't know how
long I'll be able to provide logs as the problem doesn't exist in Arch's
currents AFAIK. But there are logs in Bug #77980 from my crash, which was close
sounding, but apparently a different bug.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140722/0433eac4/attachment.html>

[Bug 77876] Performance degredation shown when running glmark2

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=77876

Aaron B  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140722/74ac72fe/attachment.html>

[Nouveau] [PATCH 09/17] drm/radeon: use common fence implementation for fences

[Christian König]

Am 22.07.2014 15:26, schrieb Daniel Vetter:
> On Tue, Jul 22, 2014 at 02:19:57PM +0200, Christian K?nig wrote:
>> Am 22.07.2014 13:57, schrieb Daniel Vetter:
>>> On Tue, Jul 22, 2014 at 01:46:07PM +0200, Daniel Vetter wrote:
 On Tue, Jul 22, 2014 at 10:43:13AM +0200, Christian K?nig wrote:
> Am 22.07.2014 06:05, schrieb Dave Airlie:
>> On 9 July 2014 22:29, Maarten Lankhorst > canonical.com> wrote:
>>> Signed-off-by: Maarten Lankhorst 
>>> ---
>>>   drivers/gpu/drm/radeon/radeon.h|   15 +-
>>>   drivers/gpu/drm/radeon/radeon_device.c |   60 -
>>>   drivers/gpu/drm/radeon/radeon_fence.c  |  223 
>>> ++--
>>>   3 files changed, 248 insertions(+), 50 deletions(-)
>>>
>>  From what I can see this is still suffering from the problem that we
>> need to find a proper solution to,
>>
>> My summary of the issues after talking to Jerome and Ben and
>> re-reading things is:
>>
>> We really need to work out a better interface into the drivers to be
>> able to avoid random atomic entrypoints,
> Which is exactly what I criticized from the very first beginning. Good to
> know that I'm not the only one thinking that this isn't such a good idea.
 I guess I've lost context a bit, but which atomic entry point are we
 talking about? Afaics the only one that's mandatory is the is
 fence->signaled callback to check whether a fence really has been
 signalled. It's used internally by the fence code to avoid spurious
 wakeups. Afaik that should be doable already on any hardware. If that's
 not the case then we can always track the signalled state in software and
 double-check in a worker thread before updating the sw state. And wrap
 this all up into a special fence class if there's more than one driver
 needing this.
>>> One thing I've forgotten: The i915 scheduler that's floating around runs
>>> its bottom half from irq context. So I really want to be able to check
>>> fence state from irq context and I also want to make it possible
>>> (possible! not mandatory) to register callbacks which are run from any
>>> context asap after the fence is signalled.
>> NAK, that's just the bad design I've talked about. Checking fence state
>> inside the same driver from interrupt context is OK, because it's the
>> drivers interrupt that we are talking about here.
>>
>> Checking fence status from another drivers interrupt context is what really
>> concerns me here, cause your driver doesn't have the slightest idea if the
>> called driver is really capable of checking the fence right now.
> I guess my mail hasn't been clear then. If you don't like it we could add
> a bit of glue to insulate the madness and bad design i915 might do from
> radeon. That imo doesn't invalidate the overall fence interfaces.
>
> So what about the following:
> - fence->enabling_signaling is restricted to be called from process
>context. We don't use any different yet, so would boild down to adding a
>WARN_ON(in_interrupt) or so to fence_enable_sw_signalling.
>
> - Make fence->signaled optional (already the case) and don't implement it
>in readon (i.e. reduce this patch here). Only downside is that radeon
>needs to correctly (i.e. without races or so) call fence_signal. And the
>cross-driver synchronization might be a bit less efficient. Note that
>you can call fence_signal from wherever you want to, so hopefully that
>doesn't restrict your implementation.
>
> End result: No one calls into radeon from interrupt context, and this is
> guaranteed.
>
> Would that be something you can agree to?

No, the whole enable_signaling stuff should go away. No callback from 
the driver into the fence code, only the other way around.

fence->signaled as well as fence->wait should become mandatory and only 
called from process context without holding any locks, neither atomic 
nor any mutex/semaphore (rcu might be ok).

> Like I've said I think restricting the insanity other people are willing
> to live with just because you don't like it isn't right. But it is
> certainly right for you to insist on not being forced into any such
> design. I think the above would achieve this.

I don't think so. If it's just me I would say that I'm just to cautious 
and the idea is still save to apply to the whole kernel.

But since Dave, Jerome and Ben seems to have similar concerns I think we 
need to agree to a minimum and save interface for all drivers.

Christian.

[Nouveau] [PATCH 09/17] drm/radeon: use common fence implementation for fences

[Daniel Vetter]

On Tue, Jul 22, 2014 at 02:19:57PM +0200, Christian K?nig wrote:
> Am 22.07.2014 13:57, schrieb Daniel Vetter:
> >On Tue, Jul 22, 2014 at 01:46:07PM +0200, Daniel Vetter wrote:
> >>On Tue, Jul 22, 2014 at 10:43:13AM +0200, Christian K?nig wrote:
> >>>Am 22.07.2014 06:05, schrieb Dave Airlie:
> On 9 July 2014 22:29, Maarten Lankhorst  canonical.com> wrote:
> >Signed-off-by: Maarten Lankhorst 
> >---
> >  drivers/gpu/drm/radeon/radeon.h|   15 +-
> >  drivers/gpu/drm/radeon/radeon_device.c |   60 -
> >  drivers/gpu/drm/radeon/radeon_fence.c  |  223 
> > ++--
> >  3 files changed, 248 insertions(+), 50 deletions(-)
> >
>  From what I can see this is still suffering from the problem that we
> need to find a proper solution to,
> 
> My summary of the issues after talking to Jerome and Ben and
> re-reading things is:
> 
> We really need to work out a better interface into the drivers to be
> able to avoid random atomic entrypoints,
> >>>Which is exactly what I criticized from the very first beginning. Good to
> >>>know that I'm not the only one thinking that this isn't such a good idea.
> >>I guess I've lost context a bit, but which atomic entry point are we
> >>talking about? Afaics the only one that's mandatory is the is
> >>fence->signaled callback to check whether a fence really has been
> >>signalled. It's used internally by the fence code to avoid spurious
> >>wakeups. Afaik that should be doable already on any hardware. If that's
> >>not the case then we can always track the signalled state in software and
> >>double-check in a worker thread before updating the sw state. And wrap
> >>this all up into a special fence class if there's more than one driver
> >>needing this.
> >One thing I've forgotten: The i915 scheduler that's floating around runs
> >its bottom half from irq context. So I really want to be able to check
> >fence state from irq context and I also want to make it possible
> >(possible! not mandatory) to register callbacks which are run from any
> >context asap after the fence is signalled.
> 
> NAK, that's just the bad design I've talked about. Checking fence state
> inside the same driver from interrupt context is OK, because it's the
> drivers interrupt that we are talking about here.
> 
> Checking fence status from another drivers interrupt context is what really
> concerns me here, cause your driver doesn't have the slightest idea if the
> called driver is really capable of checking the fence right now.

I guess my mail hasn't been clear then. If you don't like it we could add
a bit of glue to insulate the madness and bad design i915 might do from
radeon. That imo doesn't invalidate the overall fence interfaces.

So what about the following:
- fence->enabling_signaling is restricted to be called from process
  context. We don't use any different yet, so would boild down to adding a
  WARN_ON(in_interrupt) or so to fence_enable_sw_signalling.

- Make fence->signaled optional (already the case) and don't implement it
  in readon (i.e. reduce this patch here). Only downside is that radeon
  needs to correctly (i.e. without races or so) call fence_signal. And the
  cross-driver synchronization might be a bit less efficient. Note that
  you can call fence_signal from wherever you want to, so hopefully that
  doesn't restrict your implementation. 

End result: No one calls into radeon from interrupt context, and this is
guaranteed.

Would that be something you can agree to?

Like I've said I think restricting the insanity other people are willing
to live with just because you don't like it isn't right. But it is
certainly right for you to insist on not being forced into any such
design. I think the above would achieve this.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH v4 11/11] ARM: at91/dt: enable the LCD panel on sama5d3xek boards

[Boris BREZILLON]

Enable LCD related nodes.

Signed-off-by: Boris BREZILLON 
---
 arch/arm/boot/dts/sama5d31ek.dts | 20 
 arch/arm/boot/dts/sama5d33ek.dts | 20 
 arch/arm/boot/dts/sama5d34ek.dts | 20 
 arch/arm/boot/dts/sama5d36ek.dts | 20 
 4 files changed, 80 insertions(+)

diff --git a/arch/arm/boot/dts/sama5d31ek.dts b/arch/arm/boot/dts/sama5d31ek.dts
index 04eec0d..6e605fe 100644
--- a/arch/arm/boot/dts/sama5d31ek.dts
+++ b/arch/arm/boot/dts/sama5d31ek.dts
@@ -33,6 +33,10 @@
status = "okay";
};

+   hlcdc: hlcdc at f003 {
+   status = "okay";
+   };
+
macb1: ethernet at f802c000 {
status = "okay";
};
@@ -46,6 +50,22 @@
};
};

+   bl_reg: backlight_regulator {
+   status = "okay";
+   };
+
+   panel_reg: panel_regulator {
+   status = "okay";
+   };
+
+   backlight: backlight {
+   status = "okay";
+   };
+
+   panel: panel {
+   status = "okay";
+   };
+
sound {
status = "okay";
};
diff --git a/arch/arm/boot/dts/sama5d33ek.dts b/arch/arm/boot/dts/sama5d33ek.dts
index cbd6a3f..0400641 100644
--- a/arch/arm/boot/dts/sama5d33ek.dts
+++ b/arch/arm/boot/dts/sama5d33ek.dts
@@ -36,9 +36,29 @@
macb0: ethernet at f0028000 {
status = "okay";
};
+
+   hlcdc: hlcdc at f003 {
+   status = "okay";
+   };
};
};

+   bl_reg: backlight_regulator {
+   status = "okay";
+   };
+
+   panel_reg: panel_regulator {
+   status = "okay";
+   };
+
+   backlight: backlight {
+   status = "okay";
+   };
+
+   panel: panel {
+   status = "okay";
+   };
+
sound {
status = "okay";
};
diff --git a/arch/arm/boot/dts/sama5d34ek.dts b/arch/arm/boot/dts/sama5d34ek.dts
index 878aa16..9cf473e 100644
--- a/arch/arm/boot/dts/sama5d34ek.dts
+++ b/arch/arm/boot/dts/sama5d34ek.dts
@@ -46,6 +46,10 @@
macb0: ethernet at f0028000 {
status = "okay";
};
+
+   hlcdc: hlcdc at f003 {
+   status = "okay";
+   };
};
};

@@ -56,6 +60,22 @@
};
};

+   bl_reg: backlight_regulator {
+   status = "okay";
+   };
+
+   panel_reg: panel_regulator {
+   status = "okay";
+   };
+
+   backlight: backlight {
+   status = "okay";
+   };
+
+   panel: panel {
+   status = "okay";
+   };
+
sound {
status = "okay";
};
diff --git a/arch/arm/boot/dts/sama5d36ek.dts b/arch/arm/boot/dts/sama5d36ek.dts
index 59576c6..1c65741 100644
--- a/arch/arm/boot/dts/sama5d36ek.dts
+++ b/arch/arm/boot/dts/sama5d36ek.dts
@@ -41,12 +41,32 @@
status = "okay";
};

+   hlcdc: hlcdc at f003 {
+   status = "okay";
+   };
+
macb1: ethernet at f802c000 {
status = "okay";
};
};
};

+   bl_reg: backlight_regulator {
+   status = "okay";
+   };
+
+   panel_reg: panel_regulator {
+   status = "okay";
+   };
+
+   backlight: backlight {
+   status = "okay";
+   };
+
+   panel: panel {
+   status = "okay";
+   };
+
sound {
status = "okay";
};
-- 
1.8.3.2

[PATCH v4 10/11] ARM: at91/dt: add LCD panel description to sama5d3xdm.dtsi

[Boris BREZILLON]

Add LCD panel related nodes (backlight, regulators and panel) to sama5d3
Display Module dtsi.

Reference LCD pin muxing used by sama5d3xek boards.

Signed-off-by: Boris BREZILLON 
---
 arch/arm/boot/dts/sama5d3xdm.dtsi | 58 +++
 1 file changed, 58 insertions(+)

diff --git a/arch/arm/boot/dts/sama5d3xdm.dtsi 
b/arch/arm/boot/dts/sama5d3xdm.dtsi
index 035ab72..91975eb 100644
--- a/arch/arm/boot/dts/sama5d3xdm.dtsi
+++ b/arch/arm/boot/dts/sama5d3xdm.dtsi
@@ -36,6 +36,64 @@
};
};
};
+
+   hlcdc: hlcdc at f003 {
+   hlcdc-display-controller {
+   pinctrl-names = "default";
+   pinctrl-0 = <_lcd_base 
_lcd_rgb888_alt>;
+
+   port at 0 {
+   hlcdc_panel_output: endpoint at 
0 {
+   reg = <0>;
+   remote-endpoint = 
<_input>;
+   };
+   };
+   };
+   };
+   };
+   };
+
+   bl_reg: backlight_regulator {
+   compatible = "regulator-fixed";
+   regulator-name = "backlight-power-supply";
+   regulator-min-microvolt = <500>;
+   regulator-max-microvolt = <500>;
+   status = "disabled";
+   };
+
+   panel_reg: panel_regulator {
+   compatible = "regulator-fixed";
+   regulator-name = "panel-power-supply";
+   regulator-min-microvolt = <330>;
+   regulator-max-microvolt = <330>;
+   status = "disabled";
+   };
+
+   backlight: backlight {
+   compatible = "pwm-backlight";
+   pwms = <_pwm 0 5 0>;
+   brightness-levels = <0 4 8 16 32 64 128 255>;
+   default-brightness-level = <6>;
+   power-supply = <_reg>;
+   status = "disabled";
+   };
+
+   panel: panel {
+   compatible = "foxlink,fl500wvr00-a0t", "simple-panel";
+   backlight = <>;
+   power-supply = <_reg>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+   status = "disabled";
+
+   port at 0 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   panel_input: endpoint at 0 {
+   reg = <0>;
+   remote-endpoint = <_panel_output>;
+   };
};
};
 };
-- 
1.8.3.2

[PATCH v4 09/11] ARM: at91/dt: define the HLCDC node available on sama5d3 SoCs

[Boris BREZILLON]

Define the HLCDC (HLCD Controller) IP available on some sama5d3 SoCs
(i.e. sama5d31, sama5d33, sama5d34 and sama5d36) in sama5d3 dtsi file.

Signed-off-by: Boris BREZILLON 
---
 arch/arm/boot/dts/sama5d3_lcd.dtsi | 28 
 1 file changed, 28 insertions(+)

diff --git a/arch/arm/boot/dts/sama5d3_lcd.dtsi 
b/arch/arm/boot/dts/sama5d3_lcd.dtsi
index e7581f6..1874cf7 100644
--- a/arch/arm/boot/dts/sama5d3_lcd.dtsi
+++ b/arch/arm/boot/dts/sama5d3_lcd.dtsi
@@ -166,6 +166,34 @@
};
};

+   hlcdc: hlcdc at f003 {
+   compatible = "atmel,sama5d3-hlcdc";
+   reg = <0xf003 0x2000>;
+   clocks = <_clk>, <>, <>;
+   clock-names = "periph_clk","sys_clk", 
"slow_clk";
+   status = "disabled";
+
+   hlcdc-display-controller {
+   compatible = 
"atmel,hlcdc-display-controller";
+   interrupts = <36 IRQ_TYPE_LEVEL_HIGH 0>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port at 0 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <0>;
+   };
+   };
+
+   hlcdc_pwm: hlcdc-pwm {
+   compatible = "atmel,hlcdc-pwm";
+   pinctrl-names = "default";
+   pinctrl-0 = <_lcd_pwm>;
+   #pwm-cells = <3>;
+   };
+   };
+
pmc: pmc at fc00 {
periphck {
lcdc_clk: lcdc_clk {
-- 
1.8.3.2

[PATCH v4 08/11] ARM: AT91/dt: add alternative pin muxing for sama5d3 lcd pins

[Boris BREZILLON]

Define alternative pin muxing for the LCDC pins.

Signed-off-by: Boris BREZILLON 
---
 arch/arm/boot/dts/sama5d3_lcd.dtsi | 50 ++
 1 file changed, 50 insertions(+)

diff --git a/arch/arm/boot/dts/sama5d3_lcd.dtsi 
b/arch/arm/boot/dts/sama5d3_lcd.dtsi
index 2186b89..e7581f6 100644
--- a/arch/arm/boot/dts/sama5d3_lcd.dtsi
+++ b/arch/arm/boot/dts/sama5d3_lcd.dtsi
@@ -82,6 +82,28 @@
 AT91_PIOA 13 
AT91_PERIPH_A AT91_PINCTRL_NONE   /* LCDD13 pin */
 AT91_PIOA 14 
AT91_PERIPH_A AT91_PINCTRL_NONE   /* LCDD14 pin */
 AT91_PIOA 15 
AT91_PERIPH_A AT91_PINCTRL_NONE   /* LCDD15 pin */
+AT91_PIOA 16 
AT91_PERIPH_A AT91_PINCTRL_NONE   /* LCDD16 pin */
+AT91_PIOA 17 
AT91_PERIPH_A AT91_PINCTRL_NONE>; /* LCDD17 pin */
+   };
+
+   pinctrl_lcd_rgb666_alt: lcd-rgb-2-alt {
+   atmel,pins =
+   ; /* LCDD17 pin */
};
@@ -104,6 +126,34 @@
 AT91_PIOA 13 
AT91_PERIPH_A AT91_PINCTRL_NONE   /* LCDD13 pin */
 AT91_PIOA 14 
AT91_PERIPH_A AT91_PINCTRL_NONE   /* LCDD14 pin */
 AT91_PIOA 15 
AT91_PERIPH_A AT91_PINCTRL_NONE   /* LCDD15 pin */
+AT91_PIOA 16 
AT91_PERIPH_A AT91_PINCTRL_NONE   /* LCDD16 pin */
+AT91_PIOA 17 
AT91_PERIPH_A AT91_PINCTRL_NONE   /* LCDD17 pin */
+AT91_PIOA 18 
AT91_PERIPH_A AT91_PINCTRL_NONE   /* LCDD18 pin */
+AT91_PIOA 19 
AT91_PERIPH_A AT91_PINCTRL_NONE   /* LCDD19 pin */
+AT91_PIOA 20 
AT91_PERIPH_A AT91_PINCTRL_NONE   /* LCDD20 pin */
+AT91_PIOA 21 
AT91_PERIPH_A AT91_PINCTRL_NONE   /* LCDD21 pin */
+AT91_PIOA 22 
AT91_PERIPH_A AT91_PINCTRL_NONE   /* LCDD22 pin */
+AT91_PIOA 23 
AT91_PERIPH_A AT91_PINCTRL_NONE>; /* LCDD23 pin */
+   };
+
+   pinctrl_lcd_rgb888_alt: lcd-rgb-3-alt {
+   atmel,pins =
+   

[PATCH v4 07/11] ARM: AT91/dt: split sama5d3 lcd pin definitions to match RGB mode configs

[Boris BREZILLON]

The HLCDC (HLCD Controller) IP supports 4 different output mode (RGB444,
RGB565, RGB666 and RGB888) and the pin muxing will depend on the chosen
RGB mode.

Split pin definitions to be able to set pin config according to the
selected mode.

Signed-off-by: Boris BREZILLON 
---
 arch/arm/boot/dts/sama5d3_lcd.dtsi | 127 -
 1 file changed, 96 insertions(+), 31 deletions(-)

diff --git a/arch/arm/boot/dts/sama5d3_lcd.dtsi 
b/arch/arm/boot/dts/sama5d3_lcd.dtsi
index 85d3027..2186b89 100644
--- a/arch/arm/boot/dts/sama5d3_lcd.dtsi
+++ b/arch/arm/boot/dts/sama5d3_lcd.dtsi
@@ -15,38 +15,103 @@
apb {
pinctrl at f200 {
lcd {
-   pinctrl_lcd: lcd-0 {
+   pinctrl_lcd_pwm: lcd-pwm-0 {
+   atmel,pins = ;/* LCDPWM */
+   };
+
+   pinctrl_lcd_base: lcd-base-0 {
+   atmel,pins =
+   ; /* LCDPCK */
+   };
+
+   pinctrl_lcd_rgb444: lcd-rgb-0 {
+   atmel,pins =
+   ; /* LCDD11 pin */
+   };
+
+   pinctrl_lcd_rgb565: lcd-rgb-1 {
+   atmel,pins =
+   ; /* LCDD15 pin */
+   };
+
+   pinctrl_lcd_rgb666: lcd-rgb-2 {
+   atmel,pins =
+   ; /* LCDD17 pin */
+   };
+
+   pinctrl_lcd_rgb888: lcd-rgb-3 {
atmel,pins =
-   ; /* PE28 periph C LCDD23 pin */
+   ; /* LCDD23 pin */
};
};
};
-- 
1.8.3.2

[PATCH v4 06/11] drm: add DT bindings documentation for atmel-hlcdc-dc driver

[Boris BREZILLON]

The Atmel HLCDC (HLCD Controller) IP available on some Atmel SoCs (i.e.
at91sam9n12, at91sam9x5 family or sama5d3 family) provides a display
controller device.

The HLCDC block provides a single RGB output port, and only supports LCD
panels connection to LCD panels for now.

The atmel,panel property link the HLCDC RGB output with the LCD panel
connected on this port (note that the HLCDC RGB connector implementation
makes use of the DRM panel framework).

Connection to other external devices (DRM bridges) might be added later by
mean of a new atmel,xxx (atmel,bridge) property.

Signed-off-by: Boris BREZILLON 
---
 .../devicetree/bindings/drm/atmel-hlcdc-dc.txt | 54 ++
 1 file changed, 54 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/drm/atmel-hlcdc-dc.txt

diff --git a/Documentation/devicetree/bindings/drm/atmel-hlcdc-dc.txt 
b/Documentation/devicetree/bindings/drm/atmel-hlcdc-dc.txt
new file mode 100644
index 000..73b9860
--- /dev/null
+++ b/Documentation/devicetree/bindings/drm/atmel-hlcdc-dc.txt
@@ -0,0 +1,54 @@
+Device-Tree bindings for Atmel's HLCDC (High LCD Controller) DRM driver
+
+The Atmel HLCDC Display Controller is subdevice of the HLCDC MFD device.
+See ../mfd/atmel-hlcdc.txt for more details.
+
+Required properties:
+ - compatible: value should be "atmel,hlcdc-display-controller"
+ - interrupts: the HLCDC interrupt definition
+ - pinctrl-names: the pin control state names. Should contain "default".
+ - pinctrl-0: should contain the default pinctrl states.
+ - #address-cells: should be set to 1.
+ - #size-cells: should be set to 0.
+
+Required children nodes:
+ Children nodes are encoding available output ports and their connections
+ to external devices using the OF graph reprensentation (see ../graph.txt).
+ At least one port node is required.
+
+Example:
+
+   hlcdc: hlcdc at f003 {
+   compatible = "atmel,sama5d3-hlcdc";
+   reg = <0xf003 0x2000>;
+   clocks = <_clk>, <>, <>;
+   clock-names = "periph_clk","sys_clk", "slow_clk";
+   status = "disabled";
+
+   hlcdc-display-controller {
+   compatible = "atmel,hlcdc-display-controller";
+   interrupts = <36 IRQ_TYPE_LEVEL_HIGH 0>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_lcd_base _lcd_rgb888>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port at 0 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <0>;
+
+   hlcdc_panel_output: endpoint at 0 {
+   reg = <0>;
+   remote-endpoint = <_input>;
+   };
+   };
+   };
+
+   hlcdc_pwm: hlcdc-pwm {
+   compatible = "atmel,hlcdc-pwm";
+   pinctrl-names = "default";
+   pinctrl-0 = <_lcd_pwm>;
+   #pwm-cells = <3>;
+   };
+   };
-- 
1.8.3.2

[PATCH v4 05/11] drm: add Atmel HLCDC Display Controller support

[Boris BREZILLON]

The Atmel HLCDC (HLCD Controller) IP available on some Atmel SoCs (i.e.
at91sam9n12, at91sam9x5 family or sama5d3 family) provides a display
controller device.

This display controller supports at least one primary plane and might
provide several overlays and an hardware cursor depending on the IP
version.

At the moment, this driver only implements an RGB connector to interface
with LCD panels, but support for other kind of external devices might be
added later.

Signed-off-by: Boris BREZILLON 
---
 drivers/gpu/drm/Kconfig  |   2 +
 drivers/gpu/drm/Makefile |   1 +
 drivers/gpu/drm/atmel-hlcdc/Kconfig  |  11 +
 drivers/gpu/drm/atmel-hlcdc/Makefile |   7 +
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c   | 286 
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 488 ++
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 224 +++
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_layer.c  | 635 ++
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_layer.h  | 396 +++
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c | 478 ++
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c  | 804 +++
 11 files changed, 3332 insertions(+)
 create mode 100644 drivers/gpu/drm/atmel-hlcdc/Kconfig
 create mode 100644 drivers/gpu/drm/atmel-hlcdc/Makefile
 create mode 100644 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
 create mode 100644 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
 create mode 100644 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
 create mode 100644 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_layer.c
 create mode 100644 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_layer.h
 create mode 100644 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
 create mode 100644 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index f512004..9183a78 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -184,6 +184,8 @@ source "drivers/gpu/drm/cirrus/Kconfig"

 source "drivers/gpu/drm/armada/Kconfig"

+source "drivers/gpu/drm/atmel-hlcdc/Kconfig"
+
 source "drivers/gpu/drm/rcar-du/Kconfig"

 source "drivers/gpu/drm/shmobile/Kconfig"
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index af9a609..07d388c 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_DRM_GMA500) += gma500/
 obj-$(CONFIG_DRM_UDL) += udl/
 obj-$(CONFIG_DRM_AST) += ast/
 obj-$(CONFIG_DRM_ARMADA) += armada/
+obj-$(CONFIG_DRM_ATMEL_HLCDC)  += atmel-hlcdc/
 obj-$(CONFIG_DRM_RCAR_DU) += rcar-du/
 obj-$(CONFIG_DRM_SHMOBILE) +=shmobile/
 obj-$(CONFIG_DRM_OMAP) += omapdrm/
diff --git a/drivers/gpu/drm/atmel-hlcdc/Kconfig 
b/drivers/gpu/drm/atmel-hlcdc/Kconfig
new file mode 100644
index 000..bc07315
--- /dev/null
+++ b/drivers/gpu/drm/atmel-hlcdc/Kconfig
@@ -0,0 +1,11 @@
+config DRM_ATMEL_HLCDC
+   tristate "DRM Support for ATMEL HLCDC Display Controller"
+   depends on DRM && OF && MFD_ATMEL_HLCDC && COMMON_CLK
+   select DRM_GEM_CMA_HELPER
+   select DRM_KMS_HELPER
+   select DRM_KMS_FB_HELPER
+   select DRM_KMS_CMA_HELPER
+   select DRM_PANEL
+   help
+ Choose this option if you have an ATMEL SoC with an HLCDC display
+ controller (i.e. at91sam9n12, at91sam9x5 family or sama5d3 family).
diff --git a/drivers/gpu/drm/atmel-hlcdc/Makefile 
b/drivers/gpu/drm/atmel-hlcdc/Makefile
new file mode 100644
index 000..10ae426
--- /dev/null
+++ b/drivers/gpu/drm/atmel-hlcdc/Makefile
@@ -0,0 +1,7 @@
+atmel-hlcdc-dc-y := atmel_hlcdc_crtc.o \
+   atmel_hlcdc_dc.o \
+   atmel_hlcdc_layer.o \
+   atmel_hlcdc_output.o \
+   atmel_hlcdc_plane.o
+
+obj-$(CONFIG_DRM_ATMEL_HLCDC)  += atmel-hlcdc-dc.o
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c 
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
new file mode 100644
index 000..2186830
--- /dev/null
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -0,0 +1,286 @@
+/*
+ * Copyright (C) 2014 Traphandler
+ * Copyright (C) 2014 Free Electrons
+ *
+ * Author: Jean-Jacques Hiblot 
+ * Author: Boris BREZILLON 
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see .
+ */
+
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#include 
+
+#include "atmel_hlcdc_dc.h"
+
+/**
+ * Atmel HLCDC CRTC structure
+ *
+ * @base: 

[PATCH v4 04/11] pwm: add DT bindings documentation for atmel-hlcdc-pwm driver

[Boris BREZILLON]

The HLCDC IP available in some Atmel SoCs (i.e. sam9x5i.e. at91sam9n12,
at91sam9x5 family or sama5d3 family) provide a PWM device.

The DT bindings used for this PWM device is following the default 3 cells
bindings described in Documentation/devicetree/bindings/pwm/pwm.txt.

Signed-off-by: Boris BREZILLON 
---
 .../devicetree/bindings/pwm/atmel-hlcdc-pwm.txt| 55 ++
 1 file changed, 55 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt

diff --git a/Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt 
b/Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt
new file mode 100644
index 000..86ad3e2
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt
@@ -0,0 +1,55 @@
+Device-Tree bindings for Atmel's HLCDC (High LCD Controller) PWM driver
+
+The Atmel HLCDC PWM is subdevice of the HLCDC MFD device.
+See ../mfd/atmel-hlcdc.txt for more details.
+
+Required properties:
+ - compatible: value should be one of the following:
+   "atmel,hlcdc-pwm"
+ - pinctr-names: the pin control state names. Should contain "default".
+ - pinctrl-0: should contain the pinctrl states described by pinctrl
+   default.
+ - #pwm-cells: should be set to 3. This PWM chip use the default 3 cells
+   bindings defined in Documentation/devicetree/bindings/pwm/pwm.txt.
+   The first cell encodes the PWM id (0 is the only acceptable value here,
+   because the chip only provide one PWM).
+   The second cell encodes the PWM period in nanoseconds.
+   The third cell encodes the PWM flags (the only supported flag is
+   PWM_POLARITY_INVERTED)
+
+Example:
+
+   hlcdc: hlcdc at f003 {
+   compatible = "atmel,sama5d3-hlcdc";
+   reg = <0xf003 0x2000>;
+   clocks = <_clk>, <>, <>;
+   clock-names = "periph_clk","sys_clk", "slow_clk";
+   status = "disabled";
+
+   hlcdc-display-controller {
+   compatible = "atmel,hlcdc-display-controller";
+   interrupts = <36 IRQ_TYPE_LEVEL_HIGH 0>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_lcd_base _lcd_rgb888>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port at 0 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <0>;
+
+   hlcdc_panel_output: endpoint at 0 {
+   reg = <0>;
+   remote-endpoint = <_input>;
+   };
+   };
+   };
+
+   hlcdc_pwm: hlcdc-pwm {
+   compatible = "atmel,hlcdc-pwm";
+   pinctrl-names = "default";
+   pinctrl-0 = <_lcd_pwm>;
+   #pwm-cells = <3>;
+   };
+   };
-- 
1.8.3.2

[PATCH v4 03/11] pwm: add support for atmel-hlcdc-pwm device

[Boris BREZILLON]

The HLCDC IP available in some Atmel SoCs (i.e. sam9x5i.e. at91sam9n12,
at91sam9x5 family or sama5d3 family) provide a PWM device.

This driver add support for a PWM chip exposing a single PWM device (which
will most likely be used to drive a backlight device).

Signed-off-by: Boris BREZILLON 
---
 drivers/pwm/Kconfig   |   9 ++
 drivers/pwm/Makefile  |   1 +
 drivers/pwm/pwm-atmel-hlcdc.c | 229 ++
 3 files changed, 239 insertions(+)
 create mode 100644 drivers/pwm/pwm-atmel-hlcdc.c

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 4ad7b89..3c8b7d0 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -50,6 +50,15 @@ config PWM_ATMEL
  To compile this driver as a module, choose M here: the module
  will be called pwm-atmel.

+config PWM_ATMEL_HLCDC_PWM
+   tristate "Atmel HLCDC PWM support"
+   depends on MFD_ATMEL_HLCDC
+   help
+ Generic PWM framework driver for Atmel HLCDC PWM.
+
+ To compile this driver as a module, choose M here: the module
+ will be called pwm-atmel.
+
 config PWM_ATMEL_TCB
tristate "Atmel TC Block PWM support"
depends on ATMEL_TCLIB && OF
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 5c86a19..26ae965 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -2,6 +2,7 @@ obj-$(CONFIG_PWM)   += core.o
 obj-$(CONFIG_PWM_SYSFS)+= sysfs.o
 obj-$(CONFIG_PWM_AB8500)   += pwm-ab8500.o
 obj-$(CONFIG_PWM_ATMEL)+= pwm-atmel.o
+obj-$(CONFIG_PWM_ATMEL_HLCDC_PWM)  += pwm-atmel-hlcdc.o
 obj-$(CONFIG_PWM_ATMEL_TCB)+= pwm-atmel-tcb.o
 obj-$(CONFIG_PWM_BCM_KONA) += pwm-bcm-kona.o
 obj-$(CONFIG_PWM_BFIN) += pwm-bfin.o
diff --git a/drivers/pwm/pwm-atmel-hlcdc.c b/drivers/pwm/pwm-atmel-hlcdc.c
new file mode 100644
index 000..7f25197
--- /dev/null
+++ b/drivers/pwm/pwm-atmel-hlcdc.c
@@ -0,0 +1,229 @@
+/*
+ * Copyright (C) 2014 Free Electrons
+ * Copyright (C) 2014 Atmel
+ *
+ * Author: Boris BREZILLON 
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define ATMEL_HLCDC_PWMCVAL_MASK   GENMASK(15, 8)
+#define ATMEL_HLCDC_PWMCVAL(x) ((x << 8) & ATMEL_HLCDC_PWMCVAL_MASK)
+#define ATMEL_HLCDC_PWMPOL BIT(4)
+#define ATMEL_HLCDC_PWMPS_MASK GENMASK(2, 0)
+#define ATMEL_HLCDC_PWMPS_MAX  0x6
+#define ATMEL_HLCDC_PWMPS(x)   ((x) & ATMEL_HLCDC_PWMPS_MASK)
+
+struct atmel_hlcdc_pwm_chip {
+   struct pwm_chip chip;
+   struct atmel_hlcdc *hlcdc;
+   struct clk *cur_clk;
+};
+
+static inline struct atmel_hlcdc_pwm_chip *
+pwm_chip_to_atmel_hlcdc_pwm_chip(struct pwm_chip *chip)
+{
+   return container_of(chip, struct atmel_hlcdc_pwm_chip, chip);
+}
+
+static int atmel_hlcdc_pwm_config(struct pwm_chip *c,
+ struct pwm_device *pwm,
+ int duty_ns, int period_ns)
+{
+   struct atmel_hlcdc_pwm_chip *chip =
+   pwm_chip_to_atmel_hlcdc_pwm_chip(c);
+   struct atmel_hlcdc *hlcdc = chip->hlcdc;
+   struct clk *new_clk = hlcdc->slow_clk;
+   u64 pwmcval = duty_ns * 256;
+   unsigned long clk_freq;
+   u64 clk_period_ns;
+   u32 pwmcfg;
+   int pres;
+
+   clk_freq = clk_get_rate(new_clk);
+   clk_period_ns = 10;
+   clk_period_ns *= 256;
+   do_div(clk_period_ns, clk_freq);
+
+   if (clk_period_ns > period_ns) {
+   new_clk = hlcdc->sys_clk;
+   clk_freq = clk_get_rate(new_clk);
+   clk_period_ns = 10;
+   clk_period_ns *= 256;
+   do_div(clk_period_ns, clk_freq);
+   }
+
+   for (pres = ATMEL_HLCDC_PWMPS_MAX; pres >= 0; pres--) {
+   if ((clk_period_ns << pres) <= period_ns)
+   break;
+   }
+
+   if (pres > ATMEL_HLCDC_PWMPS_MAX)
+   return -EINVAL;
+
+   pwmcfg = ATMEL_HLCDC_PWMPS(pres);
+
+   if (new_clk != chip->cur_clk) {
+   u32 gencfg = 0;
+
+   clk_prepare_enable(new_clk);
+   clk_disable_unprepare(chip->cur_clk);
+   chip->cur_clk = new_clk;
+
+   if (new_clk != hlcdc->slow_clk)
+   gencfg = ATMEL_HLCDC_CLKPWMSEL;
+   

[PATCH v4 02/11] mfd: add documentation for atmel-hlcdc DT bindings

[Boris BREZILLON]

The HLCDC IP available on some Atmel SoCs (i.e. at91sam9n12, at91sam9x5
family or sama5d3 family) exposes 2 subdevices:
- a display controller (controlled by a DRM driver)
- a PWM chip

This patch adds documentation for atmel-hlcdc DT bindings.

Signed-off-by: Boris BREZILLON 
---
 .../devicetree/bindings/mfd/atmel-hlcdc.txt| 50 ++
 1 file changed, 50 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt

diff --git a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt 
b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
new file mode 100644
index 000..e9cc1b2
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
@@ -0,0 +1,50 @@
+Device-Tree bindings for Atmel's HLCDC (High LCD Controller) MFD driver
+
+Required properties:
+ - compatible: value should be one of the following:
+   "atmel,sama5d3-hlcdc"
+ - reg: base address and size of the HLCDC device registers.
+ - clock-names: the name of the 3 clocks requested by the HLCDC device.
+   Should contain "periph_clk", "sys_clk" and "slow_clk".
+ - clocks: should contain the 3 clocks requested by the HLCDC device.
+
+The HLCDC IP exposes two subdevices:
+ - a PWM chip: see ../pwm/atmel-hlcdc-pwm.txt
+ - a Display Controller: see ../drm/atmel-hlcdc-dc.txt
+
+Example:
+
+   hlcdc: hlcdc at f003 {
+   compatible = "atmel,sama5d3-hlcdc";
+   reg = <0xf003 0x2000>;
+   clocks = <_clk>, <>, <>;
+   clock-names = "periph_clk","sys_clk", "slow_clk";
+   status = "disabled";
+
+   hlcdc-display-controller {
+   compatible = "atmel,hlcdc-display-controller";
+   interrupts = <36 IRQ_TYPE_LEVEL_HIGH 0>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_lcd_base _lcd_rgb888>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port at 0 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <0>;
+
+   hlcdc_panel_output: endpoint at 0 {
+   reg = <0>;
+   remote-endpoint = <_input>;
+   };
+   };
+   };
+
+   hlcdc_pwm: hlcdc-pwm {
+   compatible = "atmel,hlcdc-pwm";
+   pinctrl-names = "default";
+   pinctrl-0 = <_lcd_pwm>;
+   #pwm-cells = <3>;
+   };
+   };
-- 
1.8.3.2

[PATCH v4 01/11] mfd: add atmel-hlcdc driver

[Boris BREZILLON]

The HLCDC IP available on some Atmel SoCs (i.e. at91sam9n12, at91sam9x5
family or sama5d3 family) exposes 2 subdevices:
- a display controller (controlled by a DRM driver)
- a PWM chip

The MFD device provides a regmap and several clocks (those connected
to this hardware block) to its subdevices.

This way concurrent accesses to the iomem range are handled by the regmap
framework, and each subdevice can safely access HLCDC registers.

Signed-off-by: Boris BREZILLON 
Acked-by: Lee Jones 
---
 drivers/mfd/Kconfig |  12 
 drivers/mfd/Makefile|   1 +
 drivers/mfd/atmel-hlcdc.c   | 118 
 include/linux/mfd/atmel-hlcdc.h |  78 ++
 4 files changed, 209 insertions(+)
 create mode 100644 drivers/mfd/atmel-hlcdc.c
 create mode 100644 include/linux/mfd/atmel-hlcdc.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 6cc4b6a..c3c007e 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -59,6 +59,18 @@ config MFD_AAT2870_CORE
  additional drivers must be enabled in order to use the
  functionality of the device.

+config MFD_ATMEL_HLCDC
+   tristate "Atmel HLCDC (HLCD Controller)"
+   select MFD_CORE
+   select REGMAP_MMIO
+   depends on OF
+   help
+ Choose this option if you have an ATMEL SoC with an HLCDC (HLCD
+ Controller) IP (i.e. at91sam9n12, at91sam9x5 family or sama5d3
+ family).
+ This MFD device exposes two subdevices: a PWM chip and a Display
+ Controller.
+
 config MFD_BCM590XX
tristate "Broadcom BCM590xx PMUs"
select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 8afedba..5f25b0d 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -156,6 +156,7 @@ obj-$(CONFIG_MFD_PM8921_CORE)   += pm8921-core.o ssbi.o
 obj-$(CONFIG_TPS65911_COMPARATOR)  += tps65911-comparator.o
 obj-$(CONFIG_MFD_TPS65090) += tps65090.o
 obj-$(CONFIG_MFD_AAT2870_CORE) += aat2870-core.o
+obj-$(CONFIG_MFD_ATMEL_HLCDC)  += atmel-hlcdc.o
 obj-$(CONFIG_MFD_INTEL_MSIC)   += intel_msic.o
 obj-$(CONFIG_MFD_PALMAS)   += palmas.o
 obj-$(CONFIG_MFD_VIPERBOARD)+= viperboard.o
diff --git a/drivers/mfd/atmel-hlcdc.c b/drivers/mfd/atmel-hlcdc.c
new file mode 100644
index 000..aadb371
--- /dev/null
+++ b/drivers/mfd/atmel-hlcdc.c
@@ -0,0 +1,118 @@
+/*
+ * Copyright (C) 2014 Free Electrons
+ * Copyright (C) 2014 Atmel
+ *
+ * Author: Boris BREZILLON 
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define ATMEL_HLCDC_REG_MAX(0x4000 - 0x4)
+
+static const struct mfd_cell atmel_hlcdc_cells[] = {
+   {
+   .name = "atmel-hlcdc-pwm",
+   .of_compatible = "atmel,hlcdc-pwm",
+   },
+   {
+   .name = "atmel-hlcdc-dc",
+   .of_compatible = "atmel,hlcdc-display-controller",
+   },
+};
+
+static const struct regmap_config atmel_hlcdc_regmap_config = {
+   .reg_bits = 32,
+   .val_bits = 32,
+   .reg_stride = 4,
+   .max_register = ATMEL_HLCDC_REG_MAX,
+};
+
+static int atmel_hlcdc_probe(struct platform_device *pdev)
+{
+   struct device *dev = >dev;
+   struct atmel_hlcdc *hlcdc;
+   struct resource *res;
+   void __iomem *regs;
+
+   hlcdc = devm_kzalloc(dev, sizeof(*hlcdc), GFP_KERNEL);
+   if (!hlcdc)
+   return -ENOMEM;
+
+   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+   regs = devm_ioremap_resource(dev, res);
+   if (IS_ERR(regs))
+   return PTR_ERR(regs);
+
+   hlcdc->periph_clk = devm_clk_get(dev, "periph_clk");
+   if (IS_ERR(hlcdc->periph_clk)) {
+   dev_err(dev, "failed to get peripheral clock\n");
+   return PTR_ERR(hlcdc->periph_clk);
+   }
+
+   hlcdc->sys_clk = devm_clk_get(dev, "sys_clk");
+   if (IS_ERR(hlcdc->sys_clk)) {
+   dev_err(dev, "failed to get system clock\n");
+   return PTR_ERR(hlcdc->sys_clk);
+   }
+
+   hlcdc->slow_clk = devm_clk_get(dev, "slow_clk");
+   if (IS_ERR(hlcdc->slow_clk)) {
+   dev_err(dev, "failed to get slow clock\n");
+   return PTR_ERR(hlcdc->slow_clk);
+   }
+
+   hlcdc->regmap = devm_regmap_init_mmio(dev, regs,
+ 

[PATCH v4 00/11] drm: add support for Atmel HLCDC Display Controller

[Boris BREZILLON]

Hello,

This patch series adds support for Atmel HLCDC (HLCD Controller) available
on some Atmel SoCs (i.e. the sama5d3 family).

The HLCDC actually provides a Display Controller and a PWM device, hence I
decided to declare an MFD device exposing 2 subdevices: a display
controller and a PWM chip.
This also solves a circular dependency issue preventing HLCDC driver from
unloading.
The HLCDC request a drm_panel device, which request a backlight device
(a PWM backlight), which depends on a PWM which is provided by the HLCDC
driver (hlcdc -> panel -> backlight -> hlcdc (pwm part)).

The current implementation only supports sama5d3 SoCs but other SoCs should
be easily ported by defining new compatible strings and adding HLCDC
description structures for these SoCs.

The drivers supports basic CRTC functionalities, several overlays and an
hardware cursor.

At the moment, it only supports connection to LCD panels through an RGB
connector (defined as an LVDS connector in my implementation), though
connection to other kind of devices (like DRM bridges) could be added later.

It also supports several RGB formats on all planes and some YUV formats on
the HEO overlay plane.

This series depends on 2 other series currently under review: [1] and [2].

Best Regards,

Boris

[1]http://lkml.iu.edu/hypermail/linux/kernel/1407.1/04171.html
[2]http://www.spinics.net/lists/kernel/msg1791681.html

Changes since v3:
- rework the layer code to simplify several parts (locking and layer
  disabling)
- make use of the drm_flip_work infrastructure
- rely on default HW cursor implementation using on the cursor plane
- rework the display controller DT bindings (based on OF graph
  representation)
- add rotation support
- retrive RGB bus format from drm_display_info
- drop the dynamic pinctrl state selection
- rework HLCDC output handling (previously specialized to interface
  with LCD panels)
- drop ".module = THIS_MODULE" lines
- change display controller compatible string

Changes since v2:
- fix coding style issues (macro indentation)
- make use of GENMASK in several places
- declare regmap config as a static structure
- rework hlcdc plane update API
- rework cursor handling to make use of the new plane update API
- fix backporch config
- do not use devm_regmap_init_mmio_clk to avoid extra clk_enable
  clk disable calls when accessing registers
- explicitely include regmap and clk headers instead of relying on
  atmel-hlcdc.h inclusions
- make the atmel-hlcdc driver depends on CONFIG_OF
- separate DT bindings documentation from driver implementation
- support several pin muxing for HLCDC pins on sama5d3 SoCs

Changes since v1:
- replace the backlight driver by a PWM driver
- make use of drm_panel infrastructure
- split driver code in several subsystem: MFD, PWM and DRM
- add support for overlays
- add support for hardware cursor


Boris BREZILLON (11):
  mfd: add atmel-hlcdc driver
  mfd: add documentation for atmel-hlcdc DT bindings
  pwm: add support for atmel-hlcdc-pwm device
  pwm: add DT bindings documentation for atmel-hlcdc-pwm driver
  drm: add Atmel HLCDC Display Controller support
  drm: add DT bindings documentation for atmel-hlcdc-dc driver
  ARM: AT91/dt: split sama5d3 lcd pin definitions to match RGB mode
configs
  ARM: AT91/dt: add alternative pin muxing for sama5d3 lcd pins
  ARM: at91/dt: define the HLCDC node available on sama5d3 SoCs
  ARM: at91/dt: add LCD panel description to sama5d3xdm.dtsi
  ARM: at91/dt: enable the LCD panel on sama5d3xek boards

 .../devicetree/bindings/drm/atmel-hlcdc-dc.txt |  54 ++
 .../devicetree/bindings/mfd/atmel-hlcdc.txt|  50 ++
 .../devicetree/bindings/pwm/atmel-hlcdc-pwm.txt|  55 ++
 arch/arm/boot/dts/sama5d31ek.dts   |  20 +
 arch/arm/boot/dts/sama5d33ek.dts   |  20 +
 arch/arm/boot/dts/sama5d34ek.dts   |  20 +
 arch/arm/boot/dts/sama5d36ek.dts   |  20 +
 arch/arm/boot/dts/sama5d3_lcd.dtsi | 205 +-
 arch/arm/boot/dts/sama5d3xdm.dtsi  |  58 ++
 drivers/gpu/drm/Kconfig|   2 +
 drivers/gpu/drm/Makefile   |   1 +
 drivers/gpu/drm/atmel-hlcdc/Kconfig|  11 +
 drivers/gpu/drm/atmel-hlcdc/Makefile   |   7 +
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 286 
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c   | 488 +
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h   | 224 ++
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_layer.c| 635 
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_layer.h| 396 ++
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c   | 478 
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c| 804 +
 drivers/mfd/Kconfig|  12 +
 drivers/mfd/Makefile   |   1 +
 drivers/mfd/atmel-hlcdc.c  | 118 +++
 

[PATCH RESEND] drm/ttm: expose CPU address of DMA-allocated pages

[Dave Airlie]

On 22 July 2014 14:21, Alexandre Courbot  wrote:
> DRM maintainers, could I have a comment about this patch? A bunch of
> Nouveau changes depend on it.

I'm not sure we really have anyone who is in a great position to comment,

my major issue would be its allocate a large chunk of RAM that might
not be needed
in all cases, and if we could avoid that when we don't need it, then
it would be good.

Or maybe we could join some allocations together, but with the Linux
mm subsystem,
who knows maybe separate small allocs have a better hope of success.

Dave.

[PATCH RESEND] drm/ttm: expose CPU address of DMA-allocated pages

[Alexandre Courbot]

On Tue, Jul 22, 2014 at 2:07 PM, Dave Airlie  wrote:
> On 22 July 2014 14:21, Alexandre Courbot  wrote:
>> DRM maintainers, could I have a comment about this patch? A bunch of
>> Nouveau changes depend on it.
>
> I'm not sure we really have anyone who is in a great position to comment,
>
> my major issue would be its allocate a large chunk of RAM that might
> not be needed
> in all cases, and if we could avoid that when we don't need it, then
> it would be good.

Strictly speaking memory allocated using dma_alloc_coherent() should
only be accessed by the CPU through the returned mapping, so having
this extra information is probably as legitimate as the current
dma_address array.

Now I agree that this results in more memory being used, which is
especially sad since this information is already known in the dma_page
internal structure. Maybe we could expose the whole dma_pages instead
of just the dma address? That way both addresses would be accessible
for the same memory cost (we will need an array to store the adresses
to the dma_pages).

>
> Or maybe we could join some allocations together, but with the Linux
> mm subsystem,
> who knows maybe separate small allocs have a better hope of success.
>
> Dave.

[Bug 79980] Random radeonsi crashes

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=79980

--- Comment #57 from Michel D?nzer  ---
(In reply to comment #56)
> Any idea where I should file the bug report? Would it be the Cinnamon back
> end, or glamour?

The first candidate is the Mesa radeonsi driver.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140722/137c6c65/attachment.html>

[Bug 79980] Random radeonsi crashes

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=79980

--- Comment #56 from Aaron B  ---
(In reply to comment #54)
> (In reply to comment #53)
> > (In reply to comment #52)
> > > I can run youtube for a while, but now Chromium seems to crash it more 
> > > often
> > > in general. Been running for a few days, have had at least 4 crashes. All 
> > > with
> > > about the same fail logs as before.
> > 
> > Your Youtube / Chromium issue is probably separate and should be tracked
> > somewhere else. This report is about a stability regression in 3.15/6-rc
> > kernels, which seems to be addressed by Christian's fixes.
> 
> Yeah, agree. Your log doesn't show any VM faults at all.
> 
> That looks more like a userspace problem triggered by some Chromium
> operations.

Any idea where I should file the bug report? Would it be the Cinnamon back end,
or glamour?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140722/ecae35a4/attachment.html>

[Bug 80901] [radeon] loading corrupts lspci entry + unloading crashes kernel

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=80901

--- Comment #5 from Andrea Patern?  ---
Created attachment 143931
  --> https://bugzilla.kernel.org/attachment.cgi?id=143931=edit
dmesg

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 80901] [radeon] loading corrupts lspci entry + unloading crashes kernel

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=80901

--- Comment #4 from Andrea Patern?  ---
It does.

gandalf at the_shire ~ ? LIBGL_DEBUG=1 DRI_PRIME=1 glxinfo | grep "renderer
string"
libGL: Can't open configuration file /home/gandalf/.drirc: No such file or
directory.
libGL: Can't open configuration file /home/gandalf/.drirc: No such file or
directory.
OpenGL renderer string: Gallium 0.4 on AMD CAPE VERDE

Also, I just noticed that everytime I lauch anithing with PRIME, it takes a few
seconds before returning the output. If you check the dmesg output I just
attached, you'll see that it shows several driver-related outputs. Basically It
prints them out everytime I lauch the program with PRIME

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

No information on valleyview

[Nick Krause]

After doing some research I didn't find any information on valley view.
I am wondering valleyview_get_display_clock_speed what this should
return as I can't find any information on it?
Nick

[Bug 80901] [radeon] loading corrupts lspci entry + unloading crashes kernel

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=80901

Alex Deucher  changed:

   What|Removed |Added

 CC||alexdeucher at gmail.com

--- Comment #3 from Alex Deucher  ---
Does rendering with PRIME work?  E.g., DRM_PRIME=1 glxinfo

I'm not sure if radeontop supports all cards or not.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[PATCH 5/5] drm: panel: simple-panel: add bus format information for foxlink panel

[Boris BREZILLON]

Foxlink's fl500wvr00-a0t supports RGB888 format.

Signed-off-by: Boris BREZILLON 
---
 drivers/gpu/drm/panel/panel-simple.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index 42fd6d1..f1e49fd 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -428,6 +428,7 @@ static const struct panel_desc foxlink_fl500wvr00_a0t = {
.width = 108,
.height = 65,
},
+   .bus_format = VIDEO_BUS_FMT_RGB888_1X24,
 };

 static const struct drm_display_mode lg_lp129qe_mode = {
-- 
1.8.3.2

[PATCH 4/5] drm: panel: simple-panel: add support for bus_format retrieval

[Boris BREZILLON]

Provide a way to specify panel requirement in terms of supported media bus
format (particularly useful for panels connected to an RGB or LVDS bus).

Signed-off-by: Boris BREZILLON 
---
 drivers/gpu/drm/panel/panel-simple.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index 3f76944..42fd6d1 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -41,6 +41,8 @@ struct panel_desc {
unsigned int width;
unsigned int height;
} size;
+
+   enum video_bus_format bus_format;
 };

 struct panel_simple {
@@ -89,6 +91,9 @@ static int panel_simple_get_fixed_modes(struct panel_simple 
*panel)

connector->display_info.width_mm = panel->desc->size.width;
connector->display_info.height_mm = panel->desc->size.height;
+   if (panel->desc->bus_format)
+   drm_display_info_set_bus_formats(>display_info,
+>desc->bus_format, 1);

return num;
 }
-- 
1.8.3.2

[PATCH 3/5] drm: add bus_formats and nbus_formats fields to drm_display_info

[Boris BREZILLON]

Add bus_formats and nbus_formats fields and
drm_display_info_set_bus_formats helper function to specify the bus
formats supported by a given display.

This information can be used by display controller drivers to configure
the output interface appropriately (i.e. RGB565, RGB666 or RGB888 on raw
RGB or LVDS busses).

Signed-off-by: Boris BREZILLON 
---
 drivers/gpu/drm/drm_crtc.c | 28 
 include/drm/drm_crtc.h |  8 
 2 files changed, 36 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index c808a09..50c8395 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -825,6 +825,34 @@ static void drm_mode_remove(struct drm_connector 
*connector,
drm_mode_destroy(connector->dev, mode);
 }

+/*
+ * drm_display_info_set_bus_formats - set the supported bus formats
+ * @info: display info to store bus formats in
+ * @fmts: array containing the supported bus formats
+ * @nfmts: the number of entries in the fmts array
+ *
+ * Store the suppported bus formats in display info structure.
+ */
+int drm_display_info_set_bus_formats(struct drm_display_info *info,
+const enum video_bus_format *fmts,
+int nfmts)
+{
+   enum video_bus_format *formats = NULL;
+
+   if (fmts && nfmts) {
+   formats = kmemdup(fmts, sizeof(*fmts) * nfmts, GFP_KERNEL);
+   if (!formats)
+   return -ENOMEM;
+   }
+
+   kfree(info->bus_formats);
+   info->bus_formats = formats;
+   info->nbus_formats = formats ? nfmts : 0;
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_display_info_set_bus_formats);
+
 /**
  * drm_connector_init - Init a preallocated connector
  * @dev: DRM device
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index e529b68..957729b 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -121,6 +122,9 @@ struct drm_display_info {
enum subpixel_order subpixel_order;
u32 color_formats;

+   const enum video_bus_format *bus_formats;
+   int nbus_formats;
+
/* Mask of supported hdmi deep color modes */
u8 edid_hdmi_dc_modes;

@@ -964,6 +968,10 @@ extern int drm_mode_connector_set_path_property(struct 
drm_connector *connector,
 extern int drm_mode_connector_update_edid_property(struct drm_connector 
*connector,
struct edid *edid);

+extern int drm_display_info_set_bus_formats(struct drm_display_info *info,
+   const enum video_bus_format *fmts,
+   int nfmts);
+
 static inline bool drm_property_type_is(struct drm_property *property,
uint32_t type)
 {
-- 
1.8.3.2

[PATCH 2/5] video: add RGB444_1X12 and RGB565_1X16 bus formats

[Boris BREZILLON]

Add RGB444 format using a 12 bits bus and RGB565 using a 16 bits bus.

These formats will later be used by atmel-hlcdc driver.

Signed-off-by: Boris BREZILLON 
---
 include/uapi/linux/v4l2-mediabus.h| 2 ++
 include/uapi/linux/video-bus-format.h | 4 +++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/v4l2-mediabus.h 
b/include/uapi/linux/v4l2-mediabus.h
index 8c31f11..319f860 100644
--- a/include/uapi/linux/v4l2-mediabus.h
+++ b/include/uapi/linux/v4l2-mediabus.h
@@ -30,6 +30,8 @@
 #define V4L2_MBUS_FMT_RGB888_2X12_BE   VIDEO_BUS_FMT_RGB888_2X12_BE
 #define V4L2_MBUS_FMT_RGB888_2X12_LE   VIDEO_BUS_FMT_RGB888_2X12_LE
 #define V4L2_MBUS_FMT_ARGB_1X32VIDEO_BUS_FMT_ARGB_1X32
+#define V4L2_BUS_FMT_RGB444_1X12   VIDEO_BUS_FMT_RGB444_1X12
+#define V4L2_BUS_FMT_RGB565_1X16   VIDEO_BUS_FMT_RGB565_1X16

 #define V4L2_MBUS_FMT_Y8_1X8   VIDEO_BUS_FMT_Y8_1X8
 #define V4L2_MBUS_FMT_UV8_1X8  VIDEO_BUS_FMT_UV8_1X8
diff --git a/include/uapi/linux/video-bus-format.h 
b/include/uapi/linux/video-bus-format.h
index 4abbd5d..f85f7ee 100644
--- a/include/uapi/linux/video-bus-format.h
+++ b/include/uapi/linux/video-bus-format.h
@@ -34,7 +34,7 @@
 enum video_bus_format {
VIDEO_BUS_FMT_FIXED = 0x0001,

-   /* RGB - next is 0x100e */
+   /* RGB - next is 0x1010 */
VIDEO_BUS_FMT_RGB444_2X8_PADHI_BE = 0x1001,
VIDEO_BUS_FMT_RGB444_2X8_PADHI_LE = 0x1002,
VIDEO_BUS_FMT_RGB555_2X8_PADHI_BE = 0x1003,
@@ -48,6 +48,8 @@ enum video_bus_format {
VIDEO_BUS_FMT_RGB888_2X12_BE = 0x100b,
VIDEO_BUS_FMT_RGB888_2X12_LE = 0x100c,
VIDEO_BUS_FMT_ARGB_1X32 = 0x100d,
+   VIDEO_BUS_FMT_RGB444_1X12 = 0x100e,
+   VIDEO_BUS_FMT_RGB565_1X16 = 0x100f,

/* YUV (including grey) - next is 0x2024 */
VIDEO_BUS_FMT_Y8_1X8 = 0x2001,
-- 
1.8.3.2

[PATCH 1/5] video: move mediabus format definition to a more standard place

[Boris BREZILLON]

Rename mediabus formats and move the enum into a separate header file so
that it can be used by DRM/KMS subsystem without any reference to the V4L2
subsystem.

Old V4L2_MBUS_FMT_ definitions are now macros that points to VIDEO_BUS_FMT_
definitions.

Signed-off-by: Boris BREZILLON 
---
 include/uapi/linux/Kbuild |   1 +
 include/uapi/linux/v4l2-mediabus.h| 183 ++
 include/uapi/linux/video-bus-format.h | 127 +++
 3 files changed, 205 insertions(+), 106 deletions(-)
 create mode 100644 include/uapi/linux/video-bus-format.h

diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index 24e9033..371874b 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -408,6 +408,7 @@ header-y += veth.h
 header-y += vfio.h
 header-y += vhost.h
 header-y += videodev2.h
+header-y += video-bus-format.h
 header-y += virtio_9p.h
 header-y += virtio_balloon.h
 header-y += virtio_blk.h
diff --git a/include/uapi/linux/v4l2-mediabus.h 
b/include/uapi/linux/v4l2-mediabus.h
index 1445e85..8c31f11 100644
--- a/include/uapi/linux/v4l2-mediabus.h
+++ b/include/uapi/linux/v4l2-mediabus.h
@@ -13,119 +13,90 @@

 #include 
 #include 
+#include 

-/*
- * These pixel codes uniquely identify data formats on the media bus. Mostly
- * they correspond to similarly named V4L2_PIX_FMT_* formats, format 0 is
- * reserved, V4L2_MBUS_FMT_FIXED shall be used by host-client pairs, where the
- * data format is fixed. Additionally, "2X8" means that one pixel is 
transferred
- * in two 8-bit samples, "BE" or "LE" specify in which order those samples are
- * transferred over the bus: "LE" means that the least significant bits are
- * transferred first, "BE" means that the most significant bits are transferred
- * first, and "PADHI" and "PADLO" define which bits - low or high, in the
- * incomplete high byte, are filled with padding bits.
- *
- * The pixel codes are grouped by type, bus_width, bits per component, samples
- * per pixel and order of subsamples. Numerical values are sorted using generic
- * numerical sort order (8 thus comes before 10).
- *
- * As their value can't change when a new pixel code is inserted in the
- * enumeration, the pixel codes are explicitly given a numerical value. The 
next
- * free values for each category are listed below, update them when inserting
- * new pixel codes.
- */
-enum v4l2_mbus_pixelcode {
-   V4L2_MBUS_FMT_FIXED = 0x0001,
-
-   /* RGB - next is 0x100e */
-   V4L2_MBUS_FMT_RGB444_2X8_PADHI_BE = 0x1001,
-   V4L2_MBUS_FMT_RGB444_2X8_PADHI_LE = 0x1002,
-   V4L2_MBUS_FMT_RGB555_2X8_PADHI_BE = 0x1003,
-   V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE = 0x1004,
-   V4L2_MBUS_FMT_BGR565_2X8_BE = 0x1005,
-   V4L2_MBUS_FMT_BGR565_2X8_LE = 0x1006,
-   V4L2_MBUS_FMT_RGB565_2X8_BE = 0x1007,
-   V4L2_MBUS_FMT_RGB565_2X8_LE = 0x1008,
-   V4L2_MBUS_FMT_RGB666_1X18 = 0x1009,
-   V4L2_MBUS_FMT_RGB888_1X24 = 0x100a,
-   V4L2_MBUS_FMT_RGB888_2X12_BE = 0x100b,
-   V4L2_MBUS_FMT_RGB888_2X12_LE = 0x100c,
-   V4L2_MBUS_FMT_ARGB_1X32 = 0x100d,
+#define V4L2_MBUS_FMT_FIXEDVIDEO_BUS_FMT_FIXED

-   /* YUV (including grey) - next is 0x2024 */
-   V4L2_MBUS_FMT_Y8_1X8 = 0x2001,
-   V4L2_MBUS_FMT_UV8_1X8 = 0x2015,
-   V4L2_MBUS_FMT_UYVY8_1_5X8 = 0x2002,
-   V4L2_MBUS_FMT_VYUY8_1_5X8 = 0x2003,
-   V4L2_MBUS_FMT_YUYV8_1_5X8 = 0x2004,
-   V4L2_MBUS_FMT_YVYU8_1_5X8 = 0x2005,
-   V4L2_MBUS_FMT_UYVY8_2X8 = 0x2006,
-   V4L2_MBUS_FMT_VYUY8_2X8 = 0x2007,
-   V4L2_MBUS_FMT_YUYV8_2X8 = 0x2008,
-   V4L2_MBUS_FMT_YVYU8_2X8 = 0x2009,
-   V4L2_MBUS_FMT_Y10_1X10 = 0x200a,
-   V4L2_MBUS_FMT_UYVY10_2X10 = 0x2018,
-   V4L2_MBUS_FMT_VYUY10_2X10 = 0x2019,
-   V4L2_MBUS_FMT_YUYV10_2X10 = 0x200b,
-   V4L2_MBUS_FMT_YVYU10_2X10 = 0x200c,
-   V4L2_MBUS_FMT_Y12_1X12 = 0x2013,
-   V4L2_MBUS_FMT_UYVY8_1X16 = 0x200f,
-   V4L2_MBUS_FMT_VYUY8_1X16 = 0x2010,
-   V4L2_MBUS_FMT_YUYV8_1X16 = 0x2011,
-   V4L2_MBUS_FMT_YVYU8_1X16 = 0x2012,
-   V4L2_MBUS_FMT_YDYUYDYV8_1X16 = 0x2014,
-   V4L2_MBUS_FMT_UYVY10_1X20 = 0x201a,
-   V4L2_MBUS_FMT_VYUY10_1X20 = 0x201b,
-   V4L2_MBUS_FMT_YUYV10_1X20 = 0x200d,
-   V4L2_MBUS_FMT_YVYU10_1X20 = 0x200e,
-   V4L2_MBUS_FMT_YUV10_1X30 = 0x2016,
-   V4L2_MBUS_FMT_AYUV8_1X32 = 0x2017,
-   V4L2_MBUS_FMT_UYVY12_2X12 = 0x201c,
-   V4L2_MBUS_FMT_VYUY12_2X12 = 0x201d,
-   V4L2_MBUS_FMT_YUYV12_2X12 = 0x201e,
-   V4L2_MBUS_FMT_YVYU12_2X12 = 0x201f,
-   V4L2_MBUS_FMT_UYVY12_1X24 = 0x2020,
-   V4L2_MBUS_FMT_VYUY12_1X24 = 0x2021,
-   V4L2_MBUS_FMT_YUYV12_1X24 = 0x2022,
-   V4L2_MBUS_FMT_YVYU12_1X24 = 0x2023,
+#define V4L2_MBUS_FMT_RGB444_2X8_PADHI_BE  
VIDEO_BUS_FMT_RGB444_2X8_PADHI_BE
+#define V4L2_MBUS_FMT_RGB444_2X8_PADHI_LE  
VIDEO_BUS_FMT_RGB444_2X8_PADHI_LE
+#define V4L2_MBUS_FMT_RGB555_2X8_PADHI_BE  

[PATCH 0/5] video: describe data bus formats

[Boris BREZILLON]

Hello,

This patch series is a proposal to describe the different data formats used
by HW components to connect with each other.

This is just a copy of the existing V4L2_MBUS_FMT defintions with a neutral
name so that it can be used by V4L2 and DRM/KMS subsystem.

This series also makes use of this video_bus_format enum in the DRM/KMS
subsystem to define the data fomats supported on the connector <-> device
link.

The video bus formats are not documented yet (and I don't know where this doc
should be stored), but I'm pretty sure this version won't be the last one ;-).

Best Regards,

Boris

Boris BREZILLON (5):
  video: move mediabus format definition to a more standard place
  video: add RGB444_1X12 and RGB565_1X16 bus formats
  drm: add bus_formats and nbus_formats fields to drm_display_info
  drm: panel: simple-panel: add support for bus_format retrieval
  drm: panel: simple-panel: add bus format information for foxlink panel

 drivers/gpu/drm/drm_crtc.c|  28 +
 drivers/gpu/drm/panel/panel-simple.c  |   6 ++
 include/drm/drm_crtc.h|   8 ++
 include/uapi/linux/Kbuild |   1 +
 include/uapi/linux/v4l2-mediabus.h| 185 +++---
 include/uapi/linux/video-bus-format.h | 129 
 6 files changed, 251 insertions(+), 106 deletions(-)
 create mode 100644 include/uapi/linux/video-bus-format.h

-- 
1.8.3.2

[PATCH 09/17] drm/radeon: use common fence implementation for fences

[Christian König]

Am 22.07.2014 13:57, schrieb Daniel Vetter:
> On Tue, Jul 22, 2014 at 01:46:07PM +0200, Daniel Vetter wrote:
>> On Tue, Jul 22, 2014 at 10:43:13AM +0200, Christian K?nig wrote:
>>> Am 22.07.2014 06:05, schrieb Dave Airlie:
 On 9 July 2014 22:29, Maarten Lankhorst >>> canonical.com> wrote:
> Signed-off-by: Maarten Lankhorst 
> ---
>   drivers/gpu/drm/radeon/radeon.h|   15 +-
>   drivers/gpu/drm/radeon/radeon_device.c |   60 -
>   drivers/gpu/drm/radeon/radeon_fence.c  |  223 
> ++--
>   3 files changed, 248 insertions(+), 50 deletions(-)
>
  From what I can see this is still suffering from the problem that we
 need to find a proper solution to,

 My summary of the issues after talking to Jerome and Ben and
 re-reading things is:

 We really need to work out a better interface into the drivers to be
 able to avoid random atomic entrypoints,
>>> Which is exactly what I criticized from the very first beginning. Good to
>>> know that I'm not the only one thinking that this isn't such a good idea.
>> I guess I've lost context a bit, but which atomic entry point are we
>> talking about? Afaics the only one that's mandatory is the is
>> fence->signaled callback to check whether a fence really has been
>> signalled. It's used internally by the fence code to avoid spurious
>> wakeups. Afaik that should be doable already on any hardware. If that's
>> not the case then we can always track the signalled state in software and
>> double-check in a worker thread before updating the sw state. And wrap
>> this all up into a special fence class if there's more than one driver
>> needing this.
> One thing I've forgotten: The i915 scheduler that's floating around runs
> its bottom half from irq context. So I really want to be able to check
> fence state from irq context and I also want to make it possible
> (possible! not mandatory) to register callbacks which are run from any
> context asap after the fence is signalled.

NAK, that's just the bad design I've talked about. Checking fence state 
inside the same driver from interrupt context is OK, because it's the 
drivers interrupt that we are talking about here.

Checking fence status from another drivers interrupt context is what 
really concerns me here, cause your driver doesn't have the slightest 
idea if the called driver is really capable of checking the fence right now.

> If the radeon hw/driver doesn't want to cope with that complexity we can
> fully insolate it with the sw tracked fence state if you don't like
> Maarten's radeon implementation. But forcing everyone to forgoe this just
> because you don't like it and don't want to use it in radeon doesn't sound
> right.

While it's clearly a hack Maarten's solution for radeon would indeed 
work, but that's not really the point here.

It's just that I think leaking interrupt context from one driver into 
another driver is just a really really bad idea from a design point of view.

And calling into a driver while in atomic context to check for a fence 
being signaled doesn't sounds like a good idea either, cause that limits 
way to much what the called driver can do for checking the status of a 
fence.

Christian.

[Bug 80901] [radeon] loading corrupts lspci entry + unloading crashes kernel

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=80901

Andrea Patern?  changed:

   What|Removed |Added

 Attachment #143921|application/octet-stream|text/plain
  mime type||

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[PATCH 09/17] drm/radeon: use common fence implementation for fences

[Dave Airlie]

On 9 July 2014 22:29, Maarten Lankhorst  
wrote:
> Signed-off-by: Maarten Lankhorst 
> ---
>  drivers/gpu/drm/radeon/radeon.h|   15 +-
>  drivers/gpu/drm/radeon/radeon_device.c |   60 -
>  drivers/gpu/drm/radeon/radeon_fence.c  |  223 
> ++--
>  3 files changed, 248 insertions(+), 50 deletions(-)
>

>From what I can see this is still suffering from the problem that we
need to find a proper solution to,

My summary of the issues after talking to Jerome and Ben and
re-reading things is:

We really need to work out a better interface into the drivers to be
able to avoid random atomic entrypoints,

I'm sure you have some ideas and I think you really need to
investigate them to move this thing forward,
even it if means some issues with android sync pts.

but none of the two major drivers seem to want the interface as-is so
something needs to give

My major question is why we need an atomic callback here at all, what
scenario does it cover?

Surely we can use a workqueue based callback to ask a driver to check
its signalling, is it really
that urgent?

Dave.

[Bug 80901] [radeon] loading corrupts lspci entry + unloading crashes kernel

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=80901

Andrea Patern?  changed:

   What|Removed |Added

 Attachment #143911|application/octet-stream|text/plain
  mime type||

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 80901] [radeon] loading corrupts lspci entry + unloading crashes kernel

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=80901

Andrea Patern?  changed:

   What|Removed |Added

 Attachment #143901|application/octet-stream|text/plain
  mime type||

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 80901] [radeon] loading corrupts lspci entry + unloading crashes kernel

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=80901

--- Comment #2 from Andrea Patern?  ---
Created attachment 143921
  --> https://bugzilla.kernel.org/attachment.cgi?id=143921=edit
Kernel config

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 80901] [radeon] loading corrupts lspci entry + unloading crashes kernel

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=80901

--- Comment #1 from Andrea Patern?  ---
Created attachment 143911
  --> https://bugzilla.kernel.org/attachment.cgi?id=143911=edit
System Journal log

System log on failure

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 80901] New: [radeon] loading corrupts lspci entry + unloading crashes kernel

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=80901

Bug ID: 80901
   Summary: [radeon] loading corrupts lspci entry + unloading
crashes kernel
   Product: Drivers
   Version: 2.5
Kernel Version: 3.16.0
  Hardware: IA-64
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: normal
  Priority: P1
 Component: Video(DRI - non Intel)
  Assignee: drivers_video-dri at kernel-bugs.osdl.org
  Reporter: amerryfellow at gmail.com
Regression: No

Created attachment 143901
  --> https://bugzilla.kernel.org/attachment.cgi?id=143901=edit
lspci w/ radeon

Hi all. I've been searching the internet for quite a while now, but couldn't
find any solution to my problem. I currently am on a laptop Dell Inspiron 15R
SE, with a i7 3632QM cpu ( which embeds a i915 intel card ) and a discrete
AMD/ATI Radeon HD 773M.

I know this kind of hybrid graphics setup's support is getting better more
recently, but I still am facing some problems with the open source driver.

The main problem is that, I don't really know if ( with PRIME ) my card is
working at all. This is because when I tried the tool radeontop, it couldn't
find my card. This is due, I suspect, to the fact that whenever I load my
"radeon" module, I think it messes up the device. This seems confirmed by the
fact that I get a clean lspci -vvv when the module is unloaded, and a "unknown
header type 7f" when the module is loaded. See attachments please.

Also, it takes a few seconds to load the module itself, and when I unload it (
modprobe -r radeon ) it crashes my machine, right after a few seconds. The
system log shows a dereference problem, but "live" I can only see the stack
trace the kernel outputs and nothing more.


Steps to Reproduce:
modprobe radeon
modprobe -r radeon

Actual Results:
Radeon's LSPCI entry is messed up and card is not recognized by tools. On
unloading, crashes.

Expected Results:
Radeon's LSPCI entry is ok, radeon card is recognized and on unloading,
nothing.

Build Date & Hardware:
Laptop Dell Inspiron 15R SE
Intel i7 3632QM
AMD Radeon HD 7730M Cape Verde

Software:
Arch Linux
Custom Kernel 3.16.0-rc6
Xorg-server 1.16.0-2
xf86-video-intel 2.99.912-4
xf86-video-ati 1:7.4.0-3

Additional Builds and Platforms:
Problem encountered also with stock kernel config and with kernel version 3.13.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[PATCH 09/17] drm/radeon: use common fence implementation for fences

[Daniel Vetter]

On Tue, Jul 22, 2014 at 01:46:07PM +0200, Daniel Vetter wrote:
> On Tue, Jul 22, 2014 at 10:43:13AM +0200, Christian K?nig wrote:
> > Am 22.07.2014 06:05, schrieb Dave Airlie:
> > >On 9 July 2014 22:29, Maarten Lankhorst  > >canonical.com> wrote:
> > >>Signed-off-by: Maarten Lankhorst 
> > >>---
> > >>  drivers/gpu/drm/radeon/radeon.h|   15 +-
> > >>  drivers/gpu/drm/radeon/radeon_device.c |   60 -
> > >>  drivers/gpu/drm/radeon/radeon_fence.c  |  223 
> > >> ++--
> > >>  3 files changed, 248 insertions(+), 50 deletions(-)
> > >>
> > > From what I can see this is still suffering from the problem that we
> > >need to find a proper solution to,
> > >
> > >My summary of the issues after talking to Jerome and Ben and
> > >re-reading things is:
> > >
> > >We really need to work out a better interface into the drivers to be
> > >able to avoid random atomic entrypoints,
> > 
> > Which is exactly what I criticized from the very first beginning. Good to
> > know that I'm not the only one thinking that this isn't such a good idea.
> 
> I guess I've lost context a bit, but which atomic entry point are we
> talking about? Afaics the only one that's mandatory is the is
> fence->signaled callback to check whether a fence really has been
> signalled. It's used internally by the fence code to avoid spurious
> wakeups. Afaik that should be doable already on any hardware. If that's
> not the case then we can always track the signalled state in software and
> double-check in a worker thread before updating the sw state. And wrap
> this all up into a special fence class if there's more than one driver
> needing this.

One thing I've forgotten: The i915 scheduler that's floating around runs
its bottom half from irq context. So I really want to be able to check
fence state from irq context and I also want to make it possible
(possible! not mandatory) to register callbacks which are run from any
context asap after the fence is signalled.

If the radeon hw/driver doesn't want to cope with that complexity we can
fully insolate it with the sw tracked fence state if you don't like
Maarten's radeon implementation. But forcing everyone to forgoe this just
because you don't like it and don't want to use it in radeon doesn't sound
right.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 09/17] drm/radeon: use common fence implementation for fences

[Daniel Vetter]

On Tue, Jul 22, 2014 at 01:46:07PM +0200, Daniel Vetter wrote:
> On Tue, Jul 22, 2014 at 10:43:13AM +0200, Christian K?nig wrote:
> > Am 22.07.2014 06:05, schrieb Dave Airlie:
> > >On 9 July 2014 22:29, Maarten Lankhorst  > >canonical.com> wrote:
> > >>Signed-off-by: Maarten Lankhorst 
> > >>---
> > >>  drivers/gpu/drm/radeon/radeon.h|   15 +-
> > >>  drivers/gpu/drm/radeon/radeon_device.c |   60 -
> > >>  drivers/gpu/drm/radeon/radeon_fence.c  |  223 
> > >> ++--
> > >>  3 files changed, 248 insertions(+), 50 deletions(-)
> > >>
> > > From what I can see this is still suffering from the problem that we
> > >need to find a proper solution to,
> > >
> > >My summary of the issues after talking to Jerome and Ben and
> > >re-reading things is:
> > >
> > >We really need to work out a better interface into the drivers to be
> > >able to avoid random atomic entrypoints,
> > 
> > Which is exactly what I criticized from the very first beginning. Good to
> > know that I'm not the only one thinking that this isn't such a good idea.
> 
> I guess I've lost context a bit, but which atomic entry point are we
> talking about? Afaics the only one that's mandatory is the is
> fence->signaled callback to check whether a fence really has been
> signalled. It's used internally by the fence code to avoid spurious
> wakeups. Afaik that should be doable already on any hardware. If that's
> not the case then we can always track the signalled state in software and
> double-check in a worker thread before updating the sw state. And wrap
> this all up into a special fence class if there's more than one driver
> needing this.
> 
> There is nothing else that forces callbacks from atomic contexts upon you.
> You can use them if you see it fit, but really if it doesn't suit your
> driver you can just ignore that part and do process based waits
> everywhere.

Aside: The fence-process-callback has already been implemented by nouveau
with the struct fence_work in nouveau_fence.c. Would make loads of sense
to move that code into the driver core and adapat it to Maarten's struct
fence once this has all landed.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 09/17] drm/radeon: use common fence implementation for fences

[Maarten Lankhorst]

Hey,

op 22-07-14 06:05, Dave Airlie schreef:
> On 9 July 2014 22:29, Maarten Lankhorst  
> wrote:
>> Signed-off-by: Maarten Lankhorst 
>> ---
>>  drivers/gpu/drm/radeon/radeon.h|   15 +-
>>  drivers/gpu/drm/radeon/radeon_device.c |   60 -
>>  drivers/gpu/drm/radeon/radeon_fence.c  |  223 
>> ++--
>>  3 files changed, 248 insertions(+), 50 deletions(-)
>>
> From what I can see this is still suffering from the problem that we
> need to find a proper solution to,
>
> My summary of the issues after talking to Jerome and Ben and
> re-reading things is:
>
> We really need to work out a better interface into the drivers to be
> able to avoid random atomic entrypoints,
> I'm sure you have some ideas and I think you really need to
> investigate them to move this thing forward,
> even it if means some issues with android sync pts.
>
> but none of the two major drivers seem to want the interface as-is so
> something needs to give
wait_queue_t (which radeon uses for fence_queue) uses atomic entrypoints too, 
the most common
one being autoremove_wake_function, which wakes up the thread it was 
initialized from, and removes
itself from the wait_queue_t list, in atomic fashion. It's used by 
__wait_event_interruptible_locked,
if something internally wants to add some arbitrary callback it could already 
happen...

> My major question is why we need an atomic callback here at all, what
> scenario does it cover?
A atomic callback could do something like schedule_work() (like 
nouveau_fence_work already does right now).

I've also added some more experimental things in my unsubmitted branch, in a 
codepath that's taken when synchronization is used with multiple GPU's:

Nouveau: I write the new seqno to the GART fence, which I added a GPU wait for 
using SEMAPHORE_TRIGGER.ACQUIRE_GE.
radeon: I write to a memory location to unblock the execution ring, this will 
probably be replaced by a call to the GPU scheduler.
i915: write to the EXCC (condition code) register to unblock the ring operation 
when it's waiting for the condition code.

But I want to emphasize that this is a hack, and driver maintainers will 
probably NACK it, I think I will only submit the one for nouveau, which is sane 
there because it schedules contexts in hardware.
Even so that part is not final and will probably go through a few iterations 
before submission.


> Surely we can use a workqueue based callback to ask a driver to check
> its signalling, is it really
> that urgent?
Nothing prevents a driver from using that approach, even with those changes.

Driver maintainers can still NACK the use of fence_add_callback if they want to,
or choose not to export fences to outside the driver. Because fences are still
not exporting, nothing will change for them compared to the current situation.

~Maarten

[PATCH 09/17] drm/radeon: use common fence implementation for fences

[Daniel Vetter]

On Tue, Jul 22, 2014 at 10:43:13AM +0200, Christian K?nig wrote:
> Am 22.07.2014 06:05, schrieb Dave Airlie:
> >On 9 July 2014 22:29, Maarten Lankhorst  
> >wrote:
> >>Signed-off-by: Maarten Lankhorst 
> >>---
> >>  drivers/gpu/drm/radeon/radeon.h|   15 +-
> >>  drivers/gpu/drm/radeon/radeon_device.c |   60 -
> >>  drivers/gpu/drm/radeon/radeon_fence.c  |  223 
> >> ++--
> >>  3 files changed, 248 insertions(+), 50 deletions(-)
> >>
> > From what I can see this is still suffering from the problem that we
> >need to find a proper solution to,
> >
> >My summary of the issues after talking to Jerome and Ben and
> >re-reading things is:
> >
> >We really need to work out a better interface into the drivers to be
> >able to avoid random atomic entrypoints,
> 
> Which is exactly what I criticized from the very first beginning. Good to
> know that I'm not the only one thinking that this isn't such a good idea.

I guess I've lost context a bit, but which atomic entry point are we
talking about? Afaics the only one that's mandatory is the is
fence->signaled callback to check whether a fence really has been
signalled. It's used internally by the fence code to avoid spurious
wakeups. Afaik that should be doable already on any hardware. If that's
not the case then we can always track the signalled state in software and
double-check in a worker thread before updating the sw state. And wrap
this all up into a special fence class if there's more than one driver
needing this.

There is nothing else that forces callbacks from atomic contexts upon you.
You can use them if you see it fit, but really if it doesn't suit your
driver you can just ignore that part and do process based waits
everywhere.

> >I'm sure you have some ideas and I think you really need to
> >investigate them to move this thing forward,
> >even it if means some issues with android sync pts.
> 
> Actually I think that TTMs fence interface already gave quite a good hint
> how it might look like. I can only guess that this won't fit with the
> Android stuff, otherwise I can't see a good reason why we didn't stick with
> that.

Well the current plan for i915<->radeon sync from Maarten is to use these
atomic callbacks on the i915 side. So android didn't figure into this at
all. Actually with android the entire implementation is kinda the
platforms problem, the generic parts just give you a userspace interface
and some means to stack up fences.

> >but none of the two major drivers seem to want the interface as-is so
> >something needs to give
> >
> >My major question is why we need an atomic callback here at all, what
> >scenario does it cover?
> 
> Agree totally. As far as I can see all current uses of the interface are of
> the kind of waiting for a fence to signal.
> 
> No need for any callback from one driver into another, especially not in
> atomic context. If a driver needs such a functionality it should just start
> up a kernel thread and do it's waiting there.
> 
> This obviously shouldn't be an obstacle for pure hardware implementations
> where one driver signals a semaphore another driver is waiting for, or a
> high signal on an interrupt line directly wired between two chips. And I
> think this is a completely different topic and not necessarily part of the
> common fence interface we should currently focus on.

It's for mixed hw/sw stuff where we want to poke the hw from the irq
context (if possible) since someone forgot the wire. At least on the i915
side it boils down to one mmio write, and it's fairly pointless to launch
a thread for that.

So I haven't dug into ttm details but from the i915 side the current stuff
and atomic semantics makes sense. Maybe we just need to wrap a bit more
insulation around ttm-based drivers.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 4/4] ARM: tegra: roth: add display DT node

[Alexandre Courbot]

On Tue, Jul 22, 2014 at 7:51 AM, Mark Brown  wrote:
> On Mon, Jul 21, 2014 at 11:16:32PM +0200, Thierry Reding wrote:
>> On Mon, Jul 21, 2014 at 09:35:27AM -0600, Stephen Warren wrote:
>
>> > > vdd-2v8-display needs to remain always-on however. Here we may hit a
>> > > limitation of the simple-panel driver, where only one power supply can
>> > > be provided.
>
>> > Can't we fix the simple-panel driver to allow a list of regulators in
>> > the property?
>
>> I have no objection to allowing that. But I don't think there's a way to
>> do that with the current regulator API. You can use the bulk API, but
>> that requires separate properties, not multiple regulators in one
>> property.
>
>> Perhaps one idea to solve this would be to make the regulator API return
>> a regulator handle that in fact controls an array of regulators? Adding
>> Mark.
>
> I'm really not comfortable with that idea, it seems like most of the
> users would be abusing it - one of the biggest issues is always getting
> people to understand that their driver may be used in other systems with
> the supplies mapped differently.  If you were going to do something
> along those lines you'd need to do something that enumerated all the
> supply properties on the device.

I also don't think that would do it - for many displays the power-on
sequence is not as simple as "switch all the regulators on".

Maybe what we would want is to have panel-simple allow panels to
provide a "probe" callback so they can request and manage any extra
resource they need. This would of course imply that custom
enable/disable and suspend/resume callbacks. Then the driver might not
be "simple" anymore, but IMHO it would not be that bad.

[PATCH RESEND] drm/ttm: expose CPU address of DMA-allocated pages

[Alexandre Courbot]

DRM maintainers, could I have a comment about this patch? A bunch of
Nouveau changes depend on it.

Thanks,
Alex.

On Tue, Jul 15, 2014 at 11:10 AM, Alexandre Courbot  
wrote:
> Pages allocated using the DMA API have a coherent memory mapping. Make
> this mapping visible to drivers so they can decide to use it instead of
> creating their own redundant one.
>
> This is not a mere optimization: for instance, on ARM it is illegal to
> have several memory mappings to the same memory with different protection.
> The mapping provided by dma_alloc_coherent() and exposed by this patch is
> guaranteed to be safe, but subsequent mappings performed by drivers are
> not. Thus drivers using the DMA page allocator should use this mapping
> instead of creating their own.
>
> Signed-off-by: Alexandre Courbot 
> ---
> This patch was previously part of a series but I figured it would make more
> sense to send it separately. It is to be used by Nouveau (and hopefully
> other drivers) on ARM.
>
>  drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 2 ++
>  drivers/gpu/drm/ttm/ttm_tt.c | 6 +-
>  include/drm/ttm/ttm_bo_driver.h  | 2 ++
>  3 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c 
> b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> index fb8259f69839..0301fac5badd 100644
> --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> @@ -847,6 +847,7 @@ static int ttm_dma_pool_get_pages(struct dma_pool *pool,
> if (count) {
> d_page = list_first_entry(>free_list, struct dma_page, 
> page_list);
> ttm->pages[index] = d_page->p;
> +   ttm_dma->cpu_address[index] = d_page->vaddr;
> ttm_dma->dma_address[index] = d_page->dma;
> list_move_tail(_page->page_list, _dma->pages_list);
> r = 0;
> @@ -978,6 +979,7 @@ void ttm_dma_unpopulate(struct ttm_dma_tt *ttm_dma, 
> struct device *dev)
> INIT_LIST_HEAD(_dma->pages_list);
> for (i = 0; i < ttm->num_pages; i++) {
> ttm->pages[i] = NULL;
> +   ttm_dma->cpu_address[i] = 0;
> ttm_dma->dma_address[i] = 0;
> }
>
> diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
> index 75f319090043..341594ede596 100644
> --- a/drivers/gpu/drm/ttm/ttm_tt.c
> +++ b/drivers/gpu/drm/ttm/ttm_tt.c
> @@ -58,6 +58,8 @@ static void ttm_dma_tt_alloc_page_directory(struct 
> ttm_dma_tt *ttm)
> ttm->ttm.pages = drm_calloc_large(ttm->ttm.num_pages, sizeof(void*));
> ttm->dma_address = drm_calloc_large(ttm->ttm.num_pages,
> sizeof(*ttm->dma_address));
> +   ttm->cpu_address = drm_calloc_large(ttm->ttm.num_pages,
> +   sizeof(*ttm->cpu_address));
>  }
>
>  #ifdef CONFIG_X86
> @@ -228,7 +230,7 @@ int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma, struct 
> ttm_bo_device *bdev,
>
> INIT_LIST_HEAD(_dma->pages_list);
> ttm_dma_tt_alloc_page_directory(ttm_dma);
> -   if (!ttm->pages || !ttm_dma->dma_address) {
> +   if (!ttm->pages || !ttm_dma->dma_address || !ttm_dma->cpu_address) {
> ttm_tt_destroy(ttm);
> pr_err("Failed allocating page table\n");
> return -ENOMEM;
> @@ -243,6 +245,8 @@ void ttm_dma_tt_fini(struct ttm_dma_tt *ttm_dma)
>
> drm_free_large(ttm->pages);
> ttm->pages = NULL;
> +   drm_free_large(ttm_dma->cpu_address);
> +   ttm_dma->cpu_address = NULL;
> drm_free_large(ttm_dma->dma_address);
> ttm_dma->dma_address = NULL;
>  }
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 202f0a7171e8..1d9f0f1ff52d 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -133,6 +133,7 @@ struct ttm_tt {
>   * struct ttm_dma_tt
>   *
>   * @ttm: Base ttm_tt struct.
> + * @cpu_address: The CPU address of the pages
>   * @dma_address: The DMA (bus) addresses of the pages
>   * @pages_list: used by some page allocation backend
>   *
> @@ -142,6 +143,7 @@ struct ttm_tt {
>   */
>  struct ttm_dma_tt {
> struct ttm_tt ttm;
> +   void **cpu_address;
> dma_addr_t *dma_address;
> struct list_head pages_list;
>  };
> --
> 2.0.1
>

[PATCH 3/4] drm/dsi: Make mipi_dsi_dcs_{read, write}() symmetrical

[Daniel Vetter]

On Tue, Jul 22, 2014 at 09:12:20AM +0200, Thierry Reding wrote:
> From: Thierry Reding 
> 
> Currently the mipi_dsi_dcs_write() function requires the DCS command
> byte to be embedded within the write buffer whereas mipi_dsi_dcs_read()
> has a separate parameter. Make them more symmetrical by adding an extra
> command parameter to mipi_dsi_dcs_write().
> 
> Also update the S6E8AA0 panel driver (the only user of this API) to
> adapt to this new API.
> 
> Signed-off-by: Thierry Reding 

Given the patterns established by other sideband subsystesm like i2c or
the dp aux helpers we have in drm I think this is going into the right
direction. Also, this seems to match somewhat the style we have in our
hand-rolled intel dsi implementation. So I think this makes sense.

Reviewed-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_mipi_dsi.c| 45 
> ---
>  drivers/gpu/drm/panel/panel-s6e8aa0.c |  4 ++--
>  include/drm/drm_mipi_dsi.h|  4 ++--
>  3 files changed, 35 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> index 6aa6a9e95570..bbab06ef80c9 100644
> --- a/drivers/gpu/drm/drm_mipi_dsi.c
> +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> @@ -201,29 +201,41 @@ EXPORT_SYMBOL(mipi_dsi_detach);
>  /**
>   * mipi_dsi_dcs_write - send DCS write command
>   * @dsi: DSI device
> - * @data: pointer to the command followed by parameters
> - * @len: length of @data
> + * @cmd: DCS command
> + * @data: pointer to the command payload
> + * @len: payload length
>   */
> -ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, const void *data,
> - size_t len)
> +ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, u8 cmd,
> +const void *data, size_t len)
>  {
>   const struct mipi_dsi_host_ops *ops = dsi->host->ops;
> - struct mipi_dsi_msg msg = {
> - .channel = dsi->channel,
> - .tx_buf = data,
> - .tx_len = len
> - };
> + struct mipi_dsi_msg msg;
> + ssize_t err;
> + u8 *tx;
>  
>   if (!ops || !ops->transfer)
>   return -ENOSYS;
>  
> + if (len > 0) {
> + tx = kmalloc(1 + len, GFP_KERNEL);
> + if (!tx)
> + return -ENOMEM;
> +
> + tx[0] = cmd;
> + memcpy(tx + 1, data, len);
> + } else {
> + tx = 
> + }
> +
> + msg.channel = dsi->channel;
> + msg.tx_len = 1 + len;
> + msg.tx_buf = tx;
> +
>   switch (len) {
>   case 0:
> - return -EINVAL;
> - case 1:
>   msg.type = MIPI_DSI_DCS_SHORT_WRITE;
>   break;
> - case 2:
> + case 1:
>   msg.type = MIPI_DSI_DCS_SHORT_WRITE_PARAM;
>   break;
>   default:
> @@ -231,14 +243,19 @@ ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, 
> const void *data,
>   break;
>   }
>  
> - return ops->transfer(dsi->host, );
> + err = ops->transfer(dsi->host, );
> +
> + if (len > 0)
> + kfree(tx);
> +
> + return err;
>  }
>  EXPORT_SYMBOL(mipi_dsi_dcs_write);
>  
>  /**
>   * mipi_dsi_dcs_read - send DCS read request command
>   * @dsi: DSI device
> - * @cmd: DCS read command
> + * @cmd: DCS command
>   * @data: pointer to read buffer
>   * @len: length of @data
>   *
> diff --git a/drivers/gpu/drm/panel/panel-s6e8aa0.c 
> b/drivers/gpu/drm/panel/panel-s6e8aa0.c
> index 5502ef6bc074..d39bee36816c 100644
> --- a/drivers/gpu/drm/panel/panel-s6e8aa0.c
> +++ b/drivers/gpu/drm/panel/panel-s6e8aa0.c
> @@ -130,7 +130,7 @@ static int s6e8aa0_clear_error(struct s6e8aa0 *ctx)
>   return ret;
>  }
>  
> -static void s6e8aa0_dcs_write(struct s6e8aa0 *ctx, const void *data, size_t 
> len)
> +static void s6e8aa0_dcs_write(struct s6e8aa0 *ctx, const u8 *data, size_t 
> len)
>  {
>   struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
>   ssize_t ret;
> @@ -138,7 +138,7 @@ static void s6e8aa0_dcs_write(struct s6e8aa0 *ctx, const 
> void *data, size_t len)
>   if (ctx->error < 0)
>   return;
>  
> - ret = mipi_dsi_dcs_write(dsi, data, len);
> + ret = mipi_dsi_dcs_write(dsi, data[0], data + 1, len - 1);
>   if (ret < 0) {
>   dev_err(ctx->dev, "error %zd writing dcs seq: %*ph\n", ret, len,
>   data);
> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> index 7b5e1a9244e1..bea181121e8b 100644
> --- a/include/drm/drm_mipi_dsi.h
> +++ b/include/drm/drm_mipi_dsi.h
> @@ -127,8 +127,8 @@ struct mipi_dsi_device {
>  
>  int mipi_dsi_attach(struct mipi_dsi_device *dsi);
>  int mipi_dsi_detach(struct mipi_dsi_device *dsi);
> -ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, const void *data,
> - size_t len);
> +ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, u8 cmd,
> +const void *data, size_t len);
>  ssize_t 

[PATCH v2 00/25] AMDKFD kernel driver

[Daniel Vetter]

On Tue, Jul 22, 2014 at 12:52:43PM +0300, Oded Gabbay wrote:
> On 22/07/14 12:21, Daniel Vetter wrote:
> >On Tue, Jul 22, 2014 at 10:19 AM, Oded Gabbay  wrote:
> >>>Exactly, just prevent userspace from submitting more. And if you have
> >>>misbehaving userspace that submits too much, reset the gpu and tell it
> >>>that you're sorry but won't schedule any more work.
> >>
> >>I'm not sure how you intend to know if a userspace misbehaves or not. Can
> >>you elaborate ?
> >
> >Well that's mostly policy, currently in i915 we only have a check for
> >hangs, and if userspace hangs a bit too often then we stop it. I guess
> >you can do that with the queue unmapping you've describe in reply to
> >Jerome's mail.
> >-Daniel
> >
> What do you mean by hang ? Like the tdr mechanism in Windows (checks if a
> gpu job takes more than 2 seconds, I think, and if so, terminates the job).

Essentially yes. But we also have some hw features to kill jobs quicker,
e.g. for media workloads.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH v6 10/14] drm/panel: add S6E3FA0 driver

[YoungJun Cho]

Hi Thierry,

Now I understand what you mean.

I'll implement common DSI helper functions.

Thank you.
Best regards YJ

On 07/21/2014 06:35 PM, Thierry Reding wrote:
> On Fri, Jul 18, 2014 at 10:49:35AM +0900, YoungJun Cho wrote:
>> Hi Thierry,
>>
>> Thank you a lot for kind comments.
>>
>> On 07/17/2014 07:36 PM, Thierry Reding wrote:
>>> On Thu, Jul 17, 2014 at 06:01:25PM +0900, YoungJun Cho wrote:
> [...]
 diff --git a/drivers/gpu/drm/panel/panel-s6e3fa0.c 
 b/drivers/gpu/drm/panel/panel-s6e3fa0.c
> [...]
 +static void s6e3fa0_set_maximum_return_packet_size(struct s6e3fa0 *ctx,
 +  unsigned int size)
 +{
 +  struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
 +  const struct mipi_dsi_host_ops *ops = dsi->host->ops;
 +
 +  if (ops && ops->transfer) {
 +  unsigned char buf[] = {size, 0};
 +  struct mipi_dsi_msg msg = {
 +  .channel = dsi->channel,
 +  .type = MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE,
 +  .tx_len = sizeof(buf),
 +  .tx_buf = buf
 +  };
 +
 +  ops->transfer(dsi->host, );
 +  }
 +}
>>>
>>> The Set Maximum Return Packet Size command is a standard command, so
>>> please turn that into a generic function exposed by the DSI core.
>>>
>>
>> For this and below standard DCS commands, you want to use generic functions,
>> but I have no idea for that.
>> Could you explain more detail?
>
> The goal should be to make these standard DCS commands available to all
> DSI peripherals, so the implementation should look something like this:
>
> static int mipi_dsi_set_maximum_return_packet_size(struct mipi_dsi_device 
> *dsi,
>  u16 size)
> {
>   struct mipi_dsi_msg msg;
>
>   if (!dsi->ops || !dsi->ops->transfer)
>   return -ENOSYS;
>
>   memset(, 0, sizeof(msg));
>   msg.channel = dsi->channel;
>   msg.type = MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE;
>   msg.tx_len = sizeof(size);
>   msg.tx_buf = 
>
>   return dsi->ops->transfer(dsi->host, );
> }
>
> The above is somewhat special, since it isn't DCS. For DCS I'd suggest a
> common prefix, like so:
>
> enum mipi_dcs_tear_mode {
>   MIPI_DCS_TEAR_VBLANK,
>   MIPI_DCS_TEAR_BLANK,
> };
>
> static int mipi_dcs_set_tear_on(struct mipi_dsi_device *dsi,
>   enum mipi_dcs_tear_mode mode)
> {
>   u8 data[2] = { MIPI_DSI_DCS_SET_TEAR_ON, mode };
>   struct mipi_dsi_msg msg;
>
>   if (!dsi->ops || !dsi->ops->transfer)
>   return -ENOSYS;
>
>   memset(, 0, sizeof(msg));
>   msg.channel = dsi->channel;
>   msg.type = MIPI_DSI_DCS_SHORT_WRITE_PARAM;
>   msg.tx_len = sizeof(data);
>   msg.tx_buf = data;
>
>   return dsi->ops->transfer(dsi->host, );
> }
>
> Thierry
>

[PATCH v2 00/25] AMDKFD kernel driver

[Oded Gabbay]

On 22/07/14 12:21, Daniel Vetter wrote:
> On Tue, Jul 22, 2014 at 10:19 AM, Oded Gabbay  wrote:
>>> Exactly, just prevent userspace from submitting more. And if you have
>>> misbehaving userspace that submits too much, reset the gpu and tell it
>>> that you're sorry but won't schedule any more work.
>>
>> I'm not sure how you intend to know if a userspace misbehaves or not. Can
>> you elaborate ?
>
> Well that's mostly policy, currently in i915 we only have a check for
> hangs, and if userspace hangs a bit too often then we stop it. I guess
> you can do that with the queue unmapping you've describe in reply to
> Jerome's mail.
> -Daniel
>
What do you mean by hang ? Like the tdr mechanism in Windows (checks if a gpu 
job takes more than 2 seconds, I think, and if so, terminates the job).

Oded

[PATCH v6 10/14] drm/panel: add S6E3FA0 driver

[YoungJun Cho]

Hi,

On 07/21/2014 08:18 PM, Andrzej Hajda wrote:
> On 07/21/2014 11:19 AM, Thierry Reding wrote:
>> On Mon, Jul 21, 2014 at 10:56:08AM +0200, Andrzej Hajda wrote:
>>> On 07/18/2014 03:49 AM, YoungJun Cho wrote:
 Hi Thierry,

 Thank you a lot for kind comments.

 On 07/17/2014 07:36 PM, Thierry Reding wrote:
> On Thu, Jul 17, 2014 at 06:01:25PM +0900, YoungJun Cho wrote:
> [...]
>> diff --git a/drivers/gpu/drm/panel/panel-s6e3fa0.c 
>> b/drivers/gpu/drm/panel/panel-s6e3fa0.c
> [...]
>> +/* Manufacturer Command Set */
>> +#define MCS_GLOBAL_PARAMETER0xb0
>> +#define MCS_AID 0xb2
>> +#define MCS_ELVSSOPT0xb6
>> +#define MCS_TEMPERATURE_SET 0xb8
>> +#define MCS_PENTILE_CTRL0xc0
>> +#define MCS_GAMMA_MODE  0xca
>> +#define MCS_VDDM0xd7
>> +#define MCS_ALS 0xe3
>> +#define MCS_ERR_FG  0xed
>> +#define MCS_KEY_LEV10xf0
>> +#define MCS_GAMMA_UPDATE0xf7
>> +#define MCS_KEY_LEV20xfc
>> +#define MCS_RE  0xfe
>> +#define MCS_TOUT2_HSYNC 0xff
>> +
>> +/* Content Adaptive Brightness Control */
>> +#define DCS_WRITE_CABC  0x55
> Is this not a manufacturer specific command? I couldn't find it in the
> DSI or DCS specifications, but it sounds like something standard (also
> indicated by the DCS_ prefix). Can you point out the specification for
> this?
>
 Andrzej commented before and decided it as DCS one because if the value
 is less than 0xb0, it is DCS one and the others are MCS one.
 But still I'm not sure it is correct.
>>> I would not say 'decided'. I have just stated that according to DCS
>>> specification
>>> it should be DCS command (below 0xb0), but it is not present in mipi dcs
>>> specs.
>>> On the other side many panels have it [1]:
>>>
>>> [1]:
>>> https://www.google.com/search?q=%22Write+Content+Adaptive+Brightness+Control%22
>>
>> Yeah, my search yielded similar results. I wonder if this is perhaps
>> part of a draft future specification. I'll try to ask around some more
>> if anybody knows what the status of this is.
>>
>> +static void s6e3fa0_read_mtp_id(struct s6e3fa0 *ctx)
>> +{
>> +unsigned char id[MTP_ID_LEN];
>> +int ret;
>> +
>> +s6e3fa0_set_maximum_return_packet_size(ctx, MTP_ID_LEN);
>> +ret = s6e3fa0_dcs_read(ctx, MIPI_DCS_GET_DISPLAY_ID, id, 
>> MTP_ID_LEN);
> This also looks like a standard DCS command. I can't find it in either
> v1.01 nor v1.02 of the specification, though. Do you know where it's
> specified?
>
 Yes, I also can't find it in DCS specification and there is no special
 description in panel datasheet.
 But as it is declared in "include/video/mipi_display.h", so I think it
 is a standard one.
>>>
>>> On page 9 of the "Introduction of MIPI by Renesas" [2] there is info
>>> it is a part of "Nokia I/F command list", I guess it is kind of alpha
>>> version of MIPI DCS.
>>>
>>> [2]: http://wenku.baidu.com/view/658fab68af1ffc4ffe47acbe.html
>>
>> That link is the only one which contains "Nokia I/F command list" on the
>> internet (that is, according to Google). But since this is already part
>> of the mipi_display.h header file we may as well use it.
>>
>> I wonder if perhaps the READ_DDB_START and READ_DDB_CONTINUE commands
>> could be used as a replacement for this display ID.
>>

There is a simple description for "Read DDB Start(A1H)" like below.
- This command returns supplier identification and display module model 
/ revision information.
- NOTE: This information is not the same what Read IDs(04H) command is 
returning.

For reference, Read IDs(04H) description is like below.
- This read command returns 24-bit display identification information.
- The first read byte identifies the OLED module's manufacturer.
- The Second read byte is used to track the OLED module/driver version.
- The third read byte identifies the OLED module/driver.

>> Adding Guennadi, Tomi, Paul and Imre on Cc since they were involved with
>> the original patch that added mipi_display.h. Perhaps they remember what
>> the origin of this command is.
>
>
> I guess it was PCF8833 used in Nokia 6100 [1][2].
>
> [1]: http://www.vintagecomputercables.com/datasheet/PCF8833_1.pdf
> [2]:
> http://www.elecfreaks.com/store/download/datasheet/shield/6100_Display_Driver.pdf
>

Yes, this command is related with Nokia.

Last Friday, I met panel vendor guy for some issues.
At the break time, I asked him for about this Read IDs(04H), why it is 
not included in DCS specification.
He said that this command was originated by Nokia.
In feature phone times, most of panel vendors wanted their panels to be 
used by Nokia and Nokia wanted this command, so most of panel vendors 
still provide this command traditionally.

Thank you.
Best 

[PATCH 4/4] drm/radeon: remove visible vram size limit on bo allocation (v3)

[Alex Deucher]

On Tue, Jul 22, 2014 at 5:14 AM, Michel D?nzer  wrote:
> On 19.07.2014 00:09, Alex Deucher wrote:
>> Now that fallback to gtt is fixed for cpu access, we can
>> remove this limit.
>>
>> bug:
>> https://bugs.freedesktop.org/show_bug.cgi?id=78717
>>
>> v2: use new gart_pin_size to accurately track available gtt.
>> v3: fix comment
>
> [...]
>
>> @@ -55,10 +55,14 @@ int radeon_gem_object_create(struct radeon_device *rdev, 
>> int size,
>>   alignment = PAGE_SIZE;
>>   }
>>
>> - /* maximun bo size is the minimun btw visible vram and gtt size */
>> - max_size = min(rdev->mc.visible_vram_size, rdev->mc.gtt_size);
>> + /* Maximum bo size is the unpinned gtt size since we use the gtt to
>> +  * handle vram to system pool migrations.  We could probably remove
>> +  * this check altogether with a little additional work to support
>> +  * splitting vram <-> system transfers into multiple steps.
>> +  */
>> + max_size = rdev->mc.gtt_size - rdev->gart_pin_size;
>
> Actually, the the check couldn't be removed even then, but would need to
> be replaced by a check against the VRAM size or something like that.
> Maybe just drop the second sentence of the comment?

Done.

Thanks!

>
> Either way though, the series is
>
> Reviewed-by: Michel D?nzer 
>
>
> --
> Earthling Michel D?nzer|  http://www.amd.com
> Libre software enthusiast  |Mesa and X developer

[PATCH] drm/radeon: fix R600_PTE_GART handling

[Alex Deucher]

On Tue, Jul 22, 2014 at 11:42 AM, Christian K?nig
 wrote:
> From: Christian K?nig 
>
> That didn't worked correctly any more and opened up a security problem.
>
> Signed-off-by: Christian K?nig 

Applied to my 3.17 tree.

Alex

> ---
>  drivers/gpu/drm/radeon/cik_sdma.c | 3 +--
>  drivers/gpu/drm/radeon/radeon.h   | 6 +++---
>  drivers/gpu/drm/radeon/si_dma.c   | 3 +--
>  3 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/cik_sdma.c 
> b/drivers/gpu/drm/radeon/cik_sdma.c
> index 8534068..28ca3cb 100644
> --- a/drivers/gpu/drm/radeon/cik_sdma.c
> +++ b/drivers/gpu/drm/radeon/cik_sdma.c
> @@ -775,8 +775,7 @@ void cik_sdma_vm_set_page(struct radeon_device *rdev,
>
> trace_radeon_vm_set_page(pe, addr, count, incr, flags);
>
> -   /* XXX: How to distinguish between GART and other system memory 
> pages? */
> -   if (flags & R600_PTE_SYSTEM) {
> +   if ((flags & R600_PTE_GART_MASK) == R600_PTE_GART_MASK) {
> uint64_t src = rdev->gart.table_addr + (addr >> 12) * 8;
> while (count) {
> unsigned bytes = count * 8;
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 89b63b9..fd878c7 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -864,9 +864,9 @@ struct radeon_mec {
>  #define R600_PTE_FRAG_64KB (4 << 7)
>  #define R600_PTE_FRAG_256KB(6 << 7)
>
> -/* flags used for GART page table entries on R600+ */
> -#define R600_PTE_GART  ( R600_PTE_VALID | R600_PTE_SYSTEM | R600_PTE_SNOOPED 
> \
> -   | R600_PTE_READABLE | R600_PTE_WRITEABLE)
> +/* flags needed to be set so we can copy directly from the GART table */
> +#define R600_PTE_GART_MASK ( R600_PTE_READABLE | R600_PTE_WRITEABLE | \
> + R600_PTE_SYSTEM | R600_PTE_VALID )
>
>  struct radeon_vm_pt {
> struct radeon_bo*bo;
> diff --git a/drivers/gpu/drm/radeon/si_dma.c b/drivers/gpu/drm/radeon/si_dma.c
> index c9da341..a26e842 100644
> --- a/drivers/gpu/drm/radeon/si_dma.c
> +++ b/drivers/gpu/drm/radeon/si_dma.c
> @@ -79,8 +79,7 @@ void si_dma_vm_set_page(struct radeon_device *rdev,
>
> trace_radeon_vm_set_page(pe, addr, count, incr, flags);
>
> -   /* XXX: How to distinguish between GART and other system memory 
> pages? */
> -   if (flags & R600_PTE_SYSTEM) {
> +   if ((flags & R600_PTE_GART_MASK) == R600_PTE_GART_MASK) {
> uint64_t src = rdev->gart.table_addr + (addr >> 12) * 8;
> while (count) {
> unsigned bytes = count * 8;
> --
> 1.9.1
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/radeon: add trace_radeon_vm_flush

[Alex Deucher]

On Tue, Jul 22, 2014 at 11:42 AM, Christian K?nig
 wrote:
> From: Christian K?nig 
>
> Signed-off-by: Christian K?nig 

Applied to my 3.17 tree.

Alex

> ---
>  drivers/gpu/drm/radeon/radeon_trace.h | 18 ++
>  drivers/gpu/drm/radeon/radeon_vm.c|  1 +
>  2 files changed, 19 insertions(+)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_trace.h 
> b/drivers/gpu/drm/radeon/radeon_trace.h
> index f749f2c..cd781f3 100644
> --- a/drivers/gpu/drm/radeon/radeon_trace.h
> +++ b/drivers/gpu/drm/radeon/radeon_trace.h
> @@ -104,6 +104,24 @@ TRACE_EVENT(radeon_vm_set_page,
>   __entry->flags, __entry->count)
>  );
>
> +TRACE_EVENT(radeon_vm_flush,
> +   TP_PROTO(uint64_t pd_addr, unsigned ring, unsigned id),
> +   TP_ARGS(pd_addr, ring, id),
> +   TP_STRUCT__entry(
> +__field(u64, pd_addr)
> +__field(u32, ring)
> +__field(u32, id)
> +),
> +
> +   TP_fast_assign(
> +  __entry->pd_addr = pd_addr;
> +  __entry->ring = ring;
> +  __entry->id = id;
> +  ),
> +   TP_printk("pd_addr=%010Lx, ring=%u, id=%u",
> + __entry->pd_addr, __entry->ring, __entry->id)
> +);
> +
>  DECLARE_EVENT_CLASS(radeon_fence_request,
>
> TP_PROTO(struct drm_device *dev, int ring, u32 seqno),
> diff --git a/drivers/gpu/drm/radeon/radeon_vm.c 
> b/drivers/gpu/drm/radeon/radeon_vm.c
> index 4b7a080..d488a87 100644
> --- a/drivers/gpu/drm/radeon/radeon_vm.c
> +++ b/drivers/gpu/drm/radeon/radeon_vm.c
> @@ -240,6 +240,7 @@ void radeon_vm_flush(struct radeon_device *rdev,
> /* if we can't remember our last VM flush then flush now! */
> /* XXX figure out why we have to flush all the time */
> if (!vm->last_flush || true || pd_addr != vm->pd_gpu_addr) {
> +   trace_radeon_vm_flush(pd_addr, ring, vm->id);
> vm->pd_gpu_addr = pd_addr;
> radeon_ring_vm_flush(rdev, ring, vm);
> }
> --
> 1.9.1
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/4] drm/dsi: Make mipi_dsi_dcs_write() return ssize_t

[Andrzej Hajda]

Hi Thierry,

YoungJun's comment refreshed my memory about mipi_dsi_dcs_write return
value. It should be rather int than ssize_t. Why?
.transfer() returns the number of read bytes or error, but in case
of dcs write no bytes are read, so it in fact returns error or 0.
This is why return value was implemented originally as int.
So I do not think this patch is necessary.

Regards
Andrzej


On 07/22/2014 12:05 PM, Andrzej Hajda wrote:
> On 07/22/2014 11:50 AM, YoungJun Cho wrote:
>> Hi,
>>
>> On 07/22/2014 04:28 PM, Andrzej Hajda wrote:
>>> Hi Thierry,
>>>
>>> Thanks for the patch.
>>>
>>> On 07/22/2014 09:12 AM, Thierry Reding wrote:
 From: Thierry Reding 

 This function returns the value of the struct mipi_dsi_host_ops'
 .transfer() so make sure the return types are consistent.

 Signed-off-by: Thierry Reding 
>>> Acked-by: Andrzej Hajda 
>>> --
>>> Regards
>>> Andrzej
 ---
   drivers/gpu/drm/drm_mipi_dsi.c| 4 ++--
   drivers/gpu/drm/panel/panel-s6e8aa0.c | 4 ++--
   include/drm/drm_mipi_dsi.h| 4 ++--
   3 files changed, 6 insertions(+), 6 deletions(-)

 diff --git a/drivers/gpu/drm/drm_mipi_dsi.c 
 b/drivers/gpu/drm/drm_mipi_dsi.c
 index e633df2f68d8..6d2fd2077dae 100644
 --- a/drivers/gpu/drm/drm_mipi_dsi.c
 +++ b/drivers/gpu/drm/drm_mipi_dsi.c
 @@ -205,8 +205,8 @@ EXPORT_SYMBOL(mipi_dsi_detach);
* @data: pointer to the command followed by parameters
* @len: length of @data
*/
 -int mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, unsigned int channel,
 - const void *data, size_t len)
 +ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, unsigned int 
 channel,
 + const void *data, size_t len)
   {
const struct mipi_dsi_host_ops *ops = dsi->host->ops;
struct mipi_dsi_msg msg = {
 diff --git a/drivers/gpu/drm/panel/panel-s6e8aa0.c 
 b/drivers/gpu/drm/panel/panel-s6e8aa0.c
 index 06e57a26db7a..beb43492b649 100644
 --- a/drivers/gpu/drm/panel/panel-s6e8aa0.c
 +++ b/drivers/gpu/drm/panel/panel-s6e8aa0.c
 @@ -133,14 +133,14 @@ static int s6e8aa0_clear_error(struct s6e8aa0 *ctx)
   static void s6e8aa0_dcs_write(struct s6e8aa0 *ctx, const void *data, 
 size_t len)
   {
struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
 -  int ret;
 +  ssize_t ret;

if (ctx->error < 0)
return;

ret = mipi_dsi_dcs_write(dsi, dsi->channel, data, len);
if (ret < 0) {
 -  dev_err(ctx->dev, "error %d writing dcs seq: %*ph\n", ret, len,
 +  dev_err(ctx->dev, "error %zd writing dcs seq: %*ph\n", ret, len,
data);
ctx->error = ret;
>> One more thing!
>> This 'ctx->error' type is 'int'. So it should be changed from int to 
>> ssize_t in struct s6e8aa0.
> I do not think so. ctx->error contains always error code, and this is
> guarded here
> by 'if (ret < 0)'  clause.
> 
> Regards
> Andrzej
>>
>> Thank you.
>> Best regards YJ
>>
}
 diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
 index efa1b552adc5..4b0112781910 100644
 --- a/include/drm/drm_mipi_dsi.h
 +++ b/include/drm/drm_mipi_dsi.h
 @@ -127,8 +127,8 @@ struct mipi_dsi_device {

   int mipi_dsi_attach(struct mipi_dsi_device *dsi);
   int mipi_dsi_detach(struct mipi_dsi_device *dsi);
 -int mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, unsigned int channel,
 - const void *data, size_t len);
 +ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, unsigned int 
 channel,
 + const void *data, size_t len);
   ssize_t mipi_dsi_dcs_read(struct mipi_dsi_device *dsi, unsigned int 
 channel,
  u8 cmd, void *data, size_t len);

>>> ___
>>> dri-devel mailing list
>>> dri-devel at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>>