Re: [alsa-devel] [PATCH v2 0/3] Make the audio component binding more generic

[Takashi Iwai]

On Wed, 18 Jul 2018 22:54:35 +0200,
Pierre-Louis Bossart wrote:
> 
> 
> 
> On 07/17/2018 04:26 AM, Takashi Iwai wrote:
> > Hi,
> >
> > this is a preliminiary patch set to convert the existing i915 /
> > HD-audio component binding to be applicable to other drivers like
> > radeon / amdgpu.  This patchset itself doesn't change the
> > functionality but only renames and split to a new drm_audio_component
> > stuff from i915_audio_component.
> >
> > The actual usage of the new API will follow once after this one gets
> > reviewed / accepted.  The whole patches (including this patchset) are
> > found in topic/hda-acomp branch of sound.git tree.
> >
> > BTW, since the whole stuff is about the audio binding, I suppose these
> > will go through sound git tree.  Let me know if anyone has concerns.
> No objections but a slight concern that this will conflict with the
> HDAudio+DSP patches that I was about to resubmit on top of your
> topic/hda-core-intel branch. the two series touch the same files so
> it'd be a miracle if there is no issue.
> How do you want to deal with this?

Does it conflict severely?  If it's trivial, it can be resolved at
merge time, too.  The changes in my patchset are fairly trivial, so it
shouldn't be too hard.


thanks,

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

Re: [PATCH v14 1/2] drm/bridge: add support for sn65dsi86 bridge driver

[Andrzej Hajda]

On 16-Jul-18 17:43, Sandeep Panda wrote:
> Add support for TI's sn65dsi86 dsi2edp bridge chip.
> The chip converts DSI transmitted signal to eDP signal,
> which is fed to the connected eDP panel.
>
> This chip can be controlled via either i2c interface or
> dsi interface. Currently in driver all the control registers
> are being accessed through i2c interface only.
> Also as of now HPD support has not been added to bridge
> chip driver.
>
> Changes in v1:
>   - Split the dt-bindings and the driver support into separate patches
> (Andrzej Hajda).
>   - Use of gpiod APIs to parse and configure gpios instead of obsolete ones
> (Andrzej Hajda).
>   - Use macros to define the register offsets (Andrzej Hajda).
>
> Changes in v2:
>   - Separate out edp panel specific HW resource handling from bridge
> driver and create a separate edp panel drivers to handle panel
> specific mode information and HW resources (Sean Paul).
>   - Replace pr_* APIs to DRM_* APIs to log error or debug information
> (Sean Paul).
>   - Remove some of the unnecessary structure/variable from driver (Sean
> Paul).
>   - Rename the function and structure prefix "sn65dsi86" to "ti_sn_bridge"
> (Sean Paul / Rob Herring).
>   - Remove most of the hard-coding and modified the bridge init sequence
> based on current mode (Sean Paul).
>   - Remove the existing function to retrieve the EDID data and
> implemented this as an i2c_adapter and use drm_get_edid() (Sean Paul).
>   - Remove the dummy irq handler implementation, will add back the
> proper irq handling later (Sean Paul).
>   - Capture the required enable gpios in a single array based on dt entry
> instead of having individual descriptor for each gpio (Sean Paul).
>
> Changes in v3:
>   - Remove usage of irq_gpio and replace it as "interrupts" property (Rob
> Herring).
>   - Remove the unnecessary header file inclusions (Sean Paul).
>   - Rearrange the header files in alphabetical order (Sean Paul).
>   - Use regmap interface to perform i2c transactions.
>   - Update Copyright/License field and address other review comments
> (Jordan Crouse).
>
> Changes in v4:
>   - Update License/Copyright (Sean Paul).
>   - Add Kconfig and Makefile changes (Sean Paul).
>   - Drop i2c gpio handling from this bridge driver, since i2c sda/scl gpios
> will be handled by i2c master.
>   - Update required supplies names.
>   - Remove unnecessary goto statements (Sean Paul).
>   - Add mutex lock to power_ctrl API to avoid race conditions (Sean
> Paul).
>   - Add support to parse reference clk frequency from dt(optional).
>   - Update the bridge chip enable/disable sequence.
>
> Changes in v5:
>   - Fixed Kbuild test service reported warnings.
>
> Changes in v6:
>   - Use PM runtime based ref-counting instead of local ref_count mechanism
> (Stephen Boyd).
>   - Clean up some debug logs and indentations (Sean Paul).
>   - Simplify dp rate calculation (Sean Paul).
>   - Add support to configure refclk based on input REFCLK pin or DACP/N
> pin (Stephen Boyd).
>
> Changes in v7:
>   - Use static supply entries instead of dynamic allocation (Andrzej
> Hajda).
>   - Defer bridge driver probe if panel is not probed (Andrzej Hajda).
>   - Update of_graph APIs for correct node reference management. (Andrzej
> Hajda).
>   - Remove local display_mode object (Andrzej Hajda).
>   - Remove version id check function from driver.
>
> Changes in v8:
>   - Move dsi register/attach function to bridge driver probe (Andrzej
> Hajda).
>   - Introduce a new helper function to write 16bit words into consecutive
> registers (Andrzej Hajda).
>   - Remove unnecessary macros (Andrzej Hajda).
>
> Changes in v9:
>   - Remove dsi register/attach from bridge probe, since dsi dev register
> completion also waits for any panel or bridge to get added. This creates
> deadlock situation when bridge driver calls dsi dev register and
> attach before bridge add, in its probe function.
>   - Fix issues faced during testing of bridge driver on actual HW.
>   - Remove unnecessary initializations (Stephen Boyd).
>   - Use local refclk lut size instead of global macro (Sean Paul).
>
> Changes in v10:
>   - Use refclk to determine if continuous dsi clock is needed or not.
>
> Changes in v11:
>   - Read DPPLL_SRC register to determine continuous clock instead of
> using refclk handle (Stephen Boyd).
>
> Changes in v12:
>   - Explain in comment as in why dsi dev registration is done in
> bridge_attach (Andrzej Hajda).
>   - Move HPD disable to bridge_pre_enable (Andrzej Hajda).
>   - Make panel/DDC exclusive until HPD support is added (Andrzej Hajda).
>
> Changes in v13:
>   - eDP panels report EDID via DP-AUX channel, so remove support for
> dedicated DDC line (Andrzej Hajda).
>
> Signed-off-by: Sandeep Panda 
> ---
>   drivers/gpu/drm/bridge/Kconfig|   9 +
>   drivers/gpu/drm/bridge/Makefile   |   1 +
>   

linux-next: manual merge of the mfd tree with the drm tree

[Stephen Rothwell]

Hi Lee,

Today's linux-next merge of the mfd tree got a conflict in:

  drivers/gpu/drm/i915/intel_display.h

between commit:

  ac213c1b45f7 ("drm/i915/icl: introduce tc_port")

from the drm tree and commit:

  9c229127aee2 ("drm/i915: hdmi: add CEC notifier to intel_hdmi")

from the mfd tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/gpu/drm/i915/intel_display.h
index 9292001cdd14,1f176a71e081..
--- a/drivers/gpu/drm/i915/intel_display.h
+++ b/drivers/gpu/drm/i915/intel_display.h
@@@ -126,17 -126,30 +126,41 @@@ enum port 
  
  #define port_name(p) ((p) + 'A')
  
+ /*
+  * Ports identifier referenced from other drivers.
+  * Expected to remain stable over time
+  */
+ static inline const char *port_identifier(enum port port)
+ {
+   switch (port) {
+   case PORT_A:
+   return "Port A";
+   case PORT_B:
+   return "Port B";
+   case PORT_C:
+   return "Port C";
+   case PORT_D:
+   return "Port D";
+   case PORT_E:
+   return "Port E";
+   case PORT_F:
+   return "Port F";
+   default:
+   return "";
+   }
+ }
+ 
 +enum tc_port {
 +  PORT_TC_NONE = -1,
 +
 +  PORT_TC1 = 0,
 +  PORT_TC2,
 +  PORT_TC3,
 +  PORT_TC4,
 +
 +  I915_MAX_TC_PORTS
 +};
 +
  enum dpio_channel {
DPIO_CH0,
DPIO_CH1


pgpU_Qtpt7Boe.pgp
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/zte: Replace drm_dev_unref with drm_dev_put

[Shawn Guo]

On Tue, Jul 17, 2018 at 01:11:14PM +0200, Thomas Zimmermann wrote:
> This patch unifies the naming of DRM functions for reference counting
> of struct drm_device. The resulting code is more aligned with the rest
> of the Linux kernel interfaces.
> 
> Signed-off-by: Thomas Zimmermann 

Acked-by: Shawn Guo 

> ---
>  drivers/gpu/drm/zte/zx_drm_drv.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/zte/zx_drm_drv.c 
> b/drivers/gpu/drm/zte/zx_drm_drv.c
> index 6f4205e80378..02ae1caf6e8a 100644
> --- a/drivers/gpu/drm/zte/zx_drm_drv.c
> +++ b/drivers/gpu/drm/zte/zx_drm_drv.c
> @@ -122,7 +122,7 @@ static int zx_drm_bind(struct device *dev)
>   component_unbind_all(dev, drm);
>  out_unregister:
>   dev_set_drvdata(dev, NULL);
> - drm_dev_unref(drm);
> + drm_dev_put(drm);
>   return ret;
>  }
>  
> @@ -136,7 +136,7 @@ static void zx_drm_unbind(struct device *dev)
>   drm_mode_config_cleanup(drm);
>   component_unbind_all(dev, drm);
>   dev_set_drvdata(dev, NULL);
> - drm_dev_unref(drm);
> + drm_dev_put(drm);
>  }
>  
>  static const struct component_master_ops zx_drm_master_ops = {
> -- 
> 2.18.0
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/vkms: Fix connector leak at the module removal

[Rodrigo Siqueira]

Currently, vkms shows an error message if the following steps occur: (1)
load vkms, (2) perform any specific operation in the vkms (e.g., run an
IGT test), and (3) unload the module. The following error message
emerges:

[drm:drm_mode_config_cleanup [drm]] *ERROR* connector Virtual-1 leaked!

This commit fixes this error by calling drm_atomic_helper_shutdown()
before drm_mode_config_cleanup, which turns off the whole display
pipeline and remove a reference related to any connector.

Signed-off-by: Rodrigo Siqueira 
---
 drivers/gpu/drm/vkms/vkms_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index 37aa2ef33b21..6e728b825259 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -44,6 +44,7 @@ static void vkms_release(struct drm_device *dev)
struct vkms_device *vkms = container_of(dev, struct vkms_device, drm);
 
platform_device_unregister(vkms->platform);
+   drm_atomic_helper_shutdown(>drm);
drm_mode_config_cleanup(>drm);
drm_dev_fini(>drm);
 }
-- 
2.18.0

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

[Bug 107224] Incorrect Rendering in Deus Ex: Mankind Divided in-game menu

[bugzilla-daemon]

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

Timothy Arceri  changed:

   What|Removed |Added

 QA Contact|dri-devel@lists.freedesktop |mesa-dev@lists.freedesktop.
   |.org|org
   Assignee|dri-devel@lists.freedesktop |mesa-dev@lists.freedesktop.
   |.org|org
  Component|Drivers/Gallium/radeonsi|Mesa core

--- Comment #1 from Timothy Arceri  ---
Di(In reply to network723 from comment #0)
> Created attachment 140628 [details]
> A screenshot of in-game menu
> 
> With recent Mesa versions some parts of the game Deus Ex: Mankind Divided
> are rendered not the way they are supposed to. 
> To reproduce the bug, max out all graphics settings (didn't test with other
> settings) and while in-game, go to "database" menu. It will show bizarre
> artifacts instead of readable text. The game used to work correctly with
> older Mesa/LLVM.

Are you sure it used to work. I've tested all the way back to Mesa
17.0-branchpoint (d1efa09d342bff) on Intels i965 driver and I still see the
issue. 

For now moving this to Mesa core as it seems to be a general Mesa issue (or
game bug ?) rather than a radeonsi problem.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/print: Fix DRM_DEBUG_DP macro

[Lyude Paul]

On Wed, 2018-07-18 at 16:07 -0700, Rodrigo Vivi wrote:
> On Wed, Jul 18, 2018 at 05:57:16PM -0400, Lyude Paul wrote:
> > This isn't supposed to take dev as an argument, I guess no one noticed!
> 
> Apparently no one using it... Why don't we just kill it?

Yeah-I've got plans to hopefully use this at some point in the future
> 
> > 
> > Signed-off-by: Lyude Paul 
> 
> anyways, if you prefer to keep or have plans to use:
> 
> Reviewed-by: Rodrigo Vivi 
> 
> 
> > ---
> >  include/drm/drm_print.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> > index 767c90b654c5..fc08584a5101 100644
> > --- a/include/drm/drm_print.h
> > +++ b/include/drm/drm_print.h
> > @@ -310,7 +310,7 @@ void drm_err(const char *format, ...);
> >  
> >  #defineDRM_DEV_DEBUG_DP(dev, fmt, ...) 
> > \
> > drm_dev_dbg(dev, DRM_UT_DP, fmt, ## __VA_ARGS__)
> > -#define DRM_DEBUG_DP(dev, fmt, ...)
> > \
> > +#define DRM_DEBUG_DP(fmt, ...) 
> > \
> > drm_dbg(DRM_UT_DP, fmt, ## __VA_ARGS__)
> >  
> >  #define _DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, category, fmt, ...) 
> > \
> > -- 
> > 2.17.1
> > 
> > ___
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- 
Cheers,
Lyude Paul
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/print: Fix DRM_DEBUG_DP macro

[Rodrigo Vivi]

On Wed, Jul 18, 2018 at 05:57:16PM -0400, Lyude Paul wrote:
> This isn't supposed to take dev as an argument, I guess no one noticed!

Apparently no one using it... Why don't we just kill it?

> 
> Signed-off-by: Lyude Paul 

anyways, if you prefer to keep or have plans to use:

Reviewed-by: Rodrigo Vivi 


> ---
>  include/drm/drm_print.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index 767c90b654c5..fc08584a5101 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -310,7 +310,7 @@ void drm_err(const char *format, ...);
>  
>  #define  DRM_DEV_DEBUG_DP(dev, fmt, ...) 
> \
>   drm_dev_dbg(dev, DRM_UT_DP, fmt, ## __VA_ARGS__)
> -#define DRM_DEBUG_DP(dev, fmt, ...)  \
> +#define DRM_DEBUG_DP(fmt, ...)   
> \
>   drm_dbg(DRM_UT_DP, fmt, ## __VA_ARGS__)
>  
>  #define _DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, category, fmt, ...)   \
> -- 
> 2.17.1
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 1/2] drm/dp: add extended receiver capability field present bit

[Rodrigo Vivi]

On Tue, Jul 17, 2018 at 02:49:38PM -0700, matthew.s.atw...@intel.com wrote:
> From: Matt Atwood 
> 
> This bit was added to DP Training Aux RD interval sometime between DP
> 1.2 and DP 1.3. Via description of the spec this field indicates the
> panels true capabilities are described in DPCD address space 02200h
> through 022FFh.
> 
> Signed-off-by: Matt Atwood 
> ---
>  include/drm/drm_dp_helper.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index c01564991a9f..757bd5913f3d 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -123,8 +123,9 @@
>  # define DP_FRAMING_CHANGE_CAP   (1 << 1)
>  # define DP_DPCD_DISPLAY_CONTROL_CAPABLE (1 << 3) /* edp v1.2 or higher 
> */
>  
> -#define DP_TRAINING_AUX_RD_INTERVAL 0x00e   /* XXX 1.2? */
> -# define DP_TRAINING_AUX_RD_MASK0x7F/* XXX 1.2? */
> +#define DP_TRAINING_AUX_RD_INTERVAL 0x00e   /* XXX 1.2? */
> +# define DP_TRAINING_AUX_RD_MASK0x7F/* XXX 1.2? */
> +# define DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT (1 << 7)/* XXX 1.2? */

I couldn't find this on 1.2 version. So 1.3?!

>  
>  #define DP_ADAPTER_CAP   0x00f   /* 1.2 */
>  # define DP_FORCE_LOAD_SENSE_CAP (1 << 0)
> -- 
> 2.17.1
> 
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/print: Fix DRM_DEBUG_DP macro

[Lyude Paul]

This isn't supposed to take dev as an argument, I guess no one noticed!

Signed-off-by: Lyude Paul 
---
 include/drm/drm_print.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 767c90b654c5..fc08584a5101 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -310,7 +310,7 @@ void drm_err(const char *format, ...);
 
 #defineDRM_DEV_DEBUG_DP(dev, fmt, ...) 
\
drm_dev_dbg(dev, DRM_UT_DP, fmt, ## __VA_ARGS__)
-#define DRM_DEBUG_DP(dev, fmt, ...)\
+#define DRM_DEBUG_DP(fmt, ...) \
drm_dbg(DRM_UT_DP, fmt, ## __VA_ARGS__)
 
 #define _DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, category, fmt, ...) \
-- 
2.17.1

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

Re: [Nouveau] [PATCH 1/5] drm/nouveau: Prevent RPM callback recursion in suspend/resume paths

[Rafael J. Wysocki]

On Wed, Jul 18, 2018 at 10:11 PM, Lyude Paul  wrote:
> On Wed, 2018-07-18 at 10:36 +0200, Lukas Wunner wrote:
>> On Wed, Jul 18, 2018 at 10:25:05AM +0200, Lukas Wunner wrote:
>> > The GPU contains an i2c subdevice for each connector with DDC lines.
>> > I believe those are modelled as children of the GPU's PCI device as
>> > they're accessed via mmio of the PCI device.
>> >
>> > The problem here is that when the GPU's PCI device runtime suspends,
>> > its i2c child device needs to be runtime active to suspend the MST
>> > topology.  Catch-22.
>> >
>> > I don't know whether or not it's necessary to suspend the MST topology.
>> > I'm not an expert on DisplayPort MultiStream transport.
>> >
>> > BTW Lyude, in patch 4 and 5 of this series, you're runtime resuming
>> > pad->i2c->subdev.device->dev.  Is this the PCI device or is it the i2c
>> > device?  I'm always confused by nouveau's structs.  In nvkm_i2c_bus_ctor()
>> > I can see that the device you're runtime resuming is the parent of the
>> > i2c_adapter:
>> >
>> > struct nvkm_device *device = pad->i2c->subdev.device;
>> > [...]
>> > bus->i2c.dev.parent = device->dev;
>> >
>> > If the i2c_adapter is a child of the PCI device, it's sufficient
>> > to runtime resume the i2c_adapter, i.e. bus->i2c.dev, and this will
>> > implicitly runtime resume its parent.
>>
>> Actually, having written all this I just remembered that we have this
>> in the documentation:
>>
>> 8. "No-Callback" Devices
>>
>> Some "devices" are only logical sub-devices of their parent and cannot
>> be
>> power-managed on their own. [...]
>>
>> Subsystems can tell the PM core about these devices by calling
>> pm_runtime_no_callbacks().
>>
>> So it might actually be sufficient to just call pm_runtime_no_callbacks()
>
> I would have hoped so, but unfortunately it seems that
> pm_runtime_no_callbacks() is already called by default for i2c adapters in
> i2c_register_adapter(). Unfortunately this really can't fix the problem
> though, because it will still try to runtime resume the parent device of the
> i2c adapter, which still leads to deadlocking in the runtime suspend/resume
> path.

Well, there has to be a way to suspend all that thing without
recursion or similar.

If the adapter has no callbacks, then how is it possible for those
callbacks to invoke any runtime PM helpers for any other devices?

> Additionally; I did play around with ignore_children, but unfortunately this
> isn't good enough either as it just means that our i2c devices won't wake the
> GPU up on access.

So on the one hand you want them to stay active over a suspend of the
parent and on the other hand you want  the parent to resume before
them.  Are these requirements really consistent with each other?

> I'm pretty stumped here on trying to figure out any clean way to handle this
> in the PM core if recursive resume calls are off the table. The only possible
> solution I could see to this is if we could disable execution of runtime
> callbacks in the context of a certain task (while all other tasks have to
> honor the runtime PM callbacks), do what we need to do in suspend, then re-
> enable them
>> for the i2c devices...

This sounds completely broken to me, sorry.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Nouveau] [PATCH] drm/nouveau: Fix bogus indenting in nouveau_hwmon.c

[Lyude Paul]

I think that is an issue with your client? It seems to come up fine if I apply
it locally and look at it with gvim

On Tue, 2018-07-17 at 12:25 +0200, Karol Herbst wrote:
> isn't there like 1 space missing for each change? Or maybe my client
> is messed up, but please align it with the first letter of the
> parameters not the "(".
> 
> With that fixed: Reviewed-by: Karol Herbst 
> 
> On Tue, Jul 17, 2018 at 2:07 AM, Lyude Paul  wrote:
> > Signed-off-by: Lyude Paul 
> > ---
> >  drivers/gpu/drm/nouveau/nouveau_hwmon.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_hwmon.c
> > b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
> > index 44178b4c3599..d556f24c6c48 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_hwmon.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
> > @@ -655,7 +655,7 @@ nouveau_read_string(struct device *dev, enum
> > hwmon_sensor_types type, u32 attr,
> > 
> >  static int
> >  nouveau_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> > -   int channel, long
> > *val)
> > +int channel, long *val)
> >  {
> > switch (type) {
> > case hwmon_chip:
> > @@ -677,7 +677,7 @@ nouveau_read(struct device *dev, enum
> > hwmon_sensor_types type, u32 attr,
> > 
> >  static int
> >  nouveau_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> > -   int channel, long
> > val)
> > + int channel, long val)
> >  {
> > switch (type) {
> > case hwmon_temp:
> > --
> > 2.17.1
> > 
> > ___
> > Nouveau mailing list
> > nouv...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/nouveau
-- 
Cheers,
Lyude Paul
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/sun4i: sun4i: Introduce a quirk for lowest plane alpha support

[kbuild test robot]

Hi Paul,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on sunxi/sunxi/for-next]
[also build test ERROR on v4.18-rc5 next-20180718]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Paul-Kocialkowski/drm-sun4i-sun4i-Introduce-a-quirk-for-lowest-plane-alpha-support/20180719-030611
base:   https://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git 
sunxi/for-next
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/sun4i/sun4i_backend.c: In function 
'sun4i_backend_atomic_check':
>> drivers/gpu/drm/sun4i/sun4i_backend.c:560:13: error: 'struct sun4i_backend' 
>> has no member named 'quirks'
 if (backend->quirks->supports_lowest_plane_alpha)
^~
   drivers/gpu/drm/sun4i/sun4i_backend.c:569:14: error: 'struct sun4i_backend' 
has no member named 'quirks'
 if (!backend->quirks->supports_lowest_plane_alpha &&
 ^~

vim +560 drivers/gpu/drm/sun4i/sun4i_backend.c

   463  
   464  static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
   465struct drm_crtc_state *crtc_state)
   466  {
   467  struct drm_plane_state *plane_states[SUN4I_BACKEND_NUM_LAYERS] 
= { 0 };
   468  struct sun4i_backend *backend = engine_to_sun4i_backend(engine);
   469  struct drm_atomic_state *state = crtc_state->state;
   470  struct drm_device *drm = state->dev;
   471  struct drm_plane *plane;
   472  unsigned int num_planes = 0;
   473  unsigned int num_alpha_planes = 0;
   474  unsigned int num_frontend_planes = 0;
   475  unsigned int num_alpha_planes_max = 1;
   476  unsigned int num_yuv_planes = 0;
   477  unsigned int current_pipe = 0;
   478  unsigned int i;
   479  
   480  DRM_DEBUG_DRIVER("Starting checking our planes\n");
   481  
   482  if (!crtc_state->planes_changed)
   483  return 0;
   484  
   485  drm_for_each_plane_mask(plane, drm, crtc_state->plane_mask) {
   486  struct drm_plane_state *plane_state =
   487  drm_atomic_get_plane_state(state, plane);
   488  struct sun4i_layer_state *layer_state =
   489  state_to_sun4i_layer_state(plane_state);
   490  struct drm_framebuffer *fb = plane_state->fb;
   491  struct drm_format_name_buf format_name;
   492  
   493  if (sun4i_backend_plane_uses_frontend(plane_state)) {
   494  DRM_DEBUG_DRIVER("Using the frontend for plane 
%d\n",
   495   plane->index);
   496  
   497  layer_state->uses_frontend = true;
   498  num_frontend_planes++;
   499  } else {
   500  layer_state->uses_frontend = false;
   501  }
   502  
   503  DRM_DEBUG_DRIVER("Plane FB format is %s\n",
   504   drm_get_format_name(fb->format->format,
   505   _name));
   506  if (fb->format->has_alpha || (plane_state->alpha != 
DRM_BLEND_ALPHA_OPAQUE))
   507  num_alpha_planes++;
   508  
   509  if (sun4i_backend_format_is_yuv(fb->format->format)) {
   510  DRM_DEBUG_DRIVER("Plane FB format is YUV\n");
   511  num_yuv_planes++;
   512  }
   513  
   514  DRM_DEBUG_DRIVER("Plane zpos is %d\n",
   515   plane_state->normalized_zpos);
   516  
   517  /* Sort our planes by Zpos */
   518  plane_states[plane_state->normalized_zpos] = 
plane_state;
   519  
   520  num_planes++;
   521  }
   522  
   523  /* All our planes were disabled, bail out */
   524  if (!num_planes)
   525  return 0;
   526  
   527  /*
   528   * The hardware is a bit unusual here.
   529   *
   530   * Even though it supports 4 layers, it does the composition
   531   * in two separate steps.
   532   *
   533   * The first one is assigning a layer to one of its two
   534   * pi

[PATCH 1/2] drm/fb_helper: Add drm_fb_helper_output_poll_changed_with_rpm()

[Lyude Paul]

When DP MST hubs get confused, they can occasionally stop responding for
a good bit of time up until the point where the DRM driver manages to
do the right DPCD accesses to get it to start responding again. In a
worst case scenario however, this process can take upwards of 10+
seconds.

Currently we use the default output_poll_changed handler
drm_fb_helper_output_poll_changed() to handle output polling, which
doesn't happen to grab any power references on the device when polling.
If we're unlucky enough to have a hub (such as Lenovo's infamous laptop
docks for the P5x/P7x series) that's easily startled and confused, this
can lead to a pretty nasty deadlock situation that looks like this:

- Hotplug event from hub happens, we enter
  drm_fb_helper_output_poll_changed() and start communicating with the
  hub
- While we're in drm_fb_helper_output_poll_changed() and attempting to
  communicate with the hub, we end up confusing it and cause it to stop
  responding for at least 10 seconds
- After 5 seconds of being in drm_fb_helper_output_poll_changed(), the
  pm core attempts to put the GPU into autosuspend, which ends up
  calling drm_kms_helper_poll_disable()
- While the runtime PM core is waiting in drm_kms_helper_poll_disable()
  for the output poll to finish, we end up finally detecting an MST
  display
- We notice the new display and tries to enable it, which triggers
  an atomic commit which triggers a call to pm_runtime_get_sync()
- the output poll thread deadlocks the pm core waiting for the pm core
  to finish the autosuspend request while the pm core waits for the
  output poll thread to finish

Sample:
[  246.669625] INFO: task kworker/4:0:37 blocked for more than 120 seconds.
[  246.673398]   Not tainted 4.18.0-rc5Lyude-Test+ #2
[  246.675271] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this 
message.
[  246.676527] kworker/4:0 D037  2 0x8000
[  246.677580] Workqueue: events output_poll_execute [drm_kms_helper]
[  246.678704] Call Trace:
[  246.679753]  __schedule+0x322/0xaf0
[  246.680916]  schedule+0x33/0x90
[  246.681924]  schedule_preempt_disabled+0x15/0x20
[  246.683023]  __mutex_lock+0x569/0x9a0
[  246.684035]  ? kobject_uevent_env+0x117/0x7b0
[  246.685132]  ? drm_fb_helper_hotplug_event.part.28+0x20/0xb0 [drm_kms_helper]
[  246.686179]  mutex_lock_nested+0x1b/0x20
[  246.687278]  ? mutex_lock_nested+0x1b/0x20
[  246.688307]  drm_fb_helper_hotplug_event.part.28+0x20/0xb0 [drm_kms_helper]
[  246.689420]  drm_fb_helper_output_poll_changed+0x23/0x30 [drm_kms_helper]
[  246.690462]  drm_kms_helper_hotplug_event+0x2a/0x30 [drm_kms_helper]
[  246.691570]  output_poll_execute+0x198/0x1c0 [drm_kms_helper]
[  246.692611]  process_one_work+0x231/0x620
[  246.693725]  worker_thread+0x214/0x3a0
[  246.694756]  kthread+0x12b/0x150
[  246.695856]  ? wq_pool_ids_show+0x140/0x140
[  246.696888]  ? kthread_create_worker_on_cpu+0x70/0x70
[  246.697998]  ret_from_fork+0x3a/0x50
[  246.699034] INFO: task kworker/0:1:60 blocked for more than 120 seconds.
[  246.700153]   Not tainted 4.18.0-rc5Lyude-Test+ #2
[  246.701182] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this 
message.
[  246.702278] kworker/0:1 D060  2 0x8000
[  246.703293] Workqueue: pm pm_runtime_work
[  246.704393] Call Trace:
[  246.705403]  __schedule+0x322/0xaf0
[  246.706439]  ? wait_for_completion+0x104/0x190
[  246.707393]  schedule+0x33/0x90
[  246.708375]  schedule_timeout+0x3a5/0x590
[  246.709289]  ? mark_held_locks+0x58/0x80
[  246.710208]  ? _raw_spin_unlock_irq+0x2c/0x40
[  246.711222]  ? wait_for_completion+0x104/0x190
[  246.712134]  ? trace_hardirqs_on_caller+0xf4/0x190
[  246.713094]  ? wait_for_completion+0x104/0x190
[  246.713964]  wait_for_completion+0x12c/0x190
[  246.714895]  ? wake_up_q+0x80/0x80
[  246.715727]  ? get_work_pool+0x90/0x90
[  246.716649]  flush_work+0x1c9/0x280
[  246.717483]  ? flush_workqueue_prep_pwqs+0x1b0/0x1b0
[  246.718442]  __cancel_work_timer+0x146/0x1d0
[  246.719247]  cancel_delayed_work_sync+0x13/0x20
[  246.720043]  drm_kms_helper_poll_disable+0x1f/0x30 [drm_kms_helper]
[  246.721123]  nouveau_pmops_runtime_suspend+0x3d/0xb0 [nouveau]
[  246.721897]  pci_pm_runtime_suspend+0x6b/0x190
[  246.722825]  ? pci_has_legacy_pm_support+0x70/0x70
[  246.723737]  __rpm_callback+0x7a/0x1d0
[  246.724721]  ? pci_has_legacy_pm_support+0x70/0x70
[  246.725607]  rpm_callback+0x24/0x80
[  246.726553]  ? pci_has_legacy_pm_support+0x70/0x70
[  246.727376]  rpm_suspend+0x142/0x6b0
[  246.728185]  pm_runtime_work+0x97/0xc0
[  246.728938]  process_one_work+0x231/0x620
[  246.729796]  worker_thread+0x44/0x3a0
[  246.730614]  kthread+0x12b/0x150
[  246.731395]  ? wq_pool_ids_show+0x140/0x140
[  246.732202]  ? kthread_create_worker_on_cpu+0x70/0x70
[  246.732878]  ret_from_fork+0x3a/0x50
[  246.733768] INFO: task kworker/4:2:422 blocked for more than 120 seconds.
[  246.734587]   Not tainted 4.18.0-rc5Lyude-Test+ #2
[  246.735393] "echo 0 

[PATCH 2/2] drm/probe_helper: Add drm_helper_probe_single_connector_modes_with_rpm()

[Lyude Paul]

For nouveau, while the GPU is guaranteed to be on when a hotplug has
been received, the same assertion does not hold true if a connector
probe has been started by userspace without having had received a sysfs
event.

So ensure that any connector probing keeps the GPU alive for the
duration of the probe by introducing
drm_helper_probe_single_connector_modes_with_rpm(). It's the same as
drm_helper_probe_single_connector_modes, but it handles holding a power
reference to the device for the duration of the connector probe.

Signed-off-by: Lyude Paul 
Reviewed-by: Karol Herbst 
Cc: Lukas Wunner 
Cc: sta...@vger.kernel.org
---
Changes since v1:
 - Add a generic helper to DRM to handle this

 drivers/gpu/drm/drm_probe_helper.c  | 31 +
 drivers/gpu/drm/nouveau/dispnv50/disp.c |  2 +-
 drivers/gpu/drm/nouveau/nouveau_connector.c |  4 +--
 include/drm/drm_crtc_helper.h   |  7 +++--
 4 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_probe_helper.c 
b/drivers/gpu/drm/drm_probe_helper.c
index 527743394150..0a9d6748b854 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -31,6 +31,7 @@
 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -541,6 +542,36 @@ int drm_helper_probe_single_connector_modes(struct 
drm_connector *connector,
 }
 EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
 
+/**
+ * drm_helper_probe_single_connector_modes_with_rpm - get complete set of
+ *display modes
+ * @connector: connector to probe
+ * @maxX: max width for modes
+ * @maxY: max height for modes
+ *
+ * Same as drm_helper_probe_single_connector_modes, except that it makes sure
+ * that the device is active by synchronously grabbing a runtime power
+ * reference while probing.
+ *
+ * Returns:
+ * The number of modes found on @connector.
+ */
+int drm_helper_probe_single_connector_modes_with_rpm(struct drm_connector 
*connector,
+uint32_t maxX, uint32_t 
maxY)
+{
+   int ret;
+
+   ret = pm_runtime_get_sync(connector->dev->dev);
+   if (ret < 0 && ret != -EACCES)
+   return ret;
+
+   ret = drm_helper_probe_single_connector_modes(connector, maxX, maxY);
+
+   pm_runtime_put(connector->dev->dev);
+   return ret;
+}
+EXPORT_SYMBOL(drm_helper_probe_single_connector_modes_with_rpm);
+
 /**
  * drm_kms_helper_hotplug_event - fire off KMS hotplug events
  * @dev: drm_device whose connector state changed
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index fa3ab618a0f9..c54767b50fd8 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -858,7 +858,7 @@ static const struct drm_connector_funcs
 nv50_mstc = {
.reset = nouveau_conn_reset,
.detect = nv50_mstc_detect,
-   .fill_modes = drm_helper_probe_single_connector_modes,
+   .fill_modes = drm_helper_probe_single_connector_modes_with_rpm,
.destroy = nv50_mstc_destroy,
.atomic_duplicate_state = nouveau_conn_atomic_duplicate_state,
.atomic_destroy_state = nouveau_conn_atomic_destroy_state,
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c 
b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 2a45b4c2ceb0..8d9070779261 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -1088,7 +1088,7 @@ nouveau_connector_funcs = {
.reset = nouveau_conn_reset,
.detect = nouveau_connector_detect,
.force = nouveau_connector_force,
-   .fill_modes = drm_helper_probe_single_connector_modes,
+   .fill_modes = drm_helper_probe_single_connector_modes_with_rpm,
.set_property = nouveau_connector_set_property,
.destroy = nouveau_connector_destroy,
.atomic_duplicate_state = nouveau_conn_atomic_duplicate_state,
@@ -1103,7 +1103,7 @@ nouveau_connector_funcs_lvds = {
.reset = nouveau_conn_reset,
.detect = nouveau_connector_detect_lvds,
.force = nouveau_connector_force,
-   .fill_modes = drm_helper_probe_single_connector_modes,
+   .fill_modes = drm_helper_probe_single_connector_modes_with_rpm,
.set_property = nouveau_connector_set_property,
.destroy = nouveau_connector_destroy,
.atomic_duplicate_state = nouveau_conn_atomic_duplicate_state,
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index 6914633037a5..8f3f6d6fcc8c 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -64,9 +64,10 @@ int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int 
x, int y,
  struct drm_framebuffer *old_fb);
 
 /* drm_probe_helper.c */
-int drm_helper_probe_single_connector_modes(struct drm_connector
-   *connector, uint32_t maxX,
-  

[PATCH 0/2] Fix connector probing deadlocks from RPM bugs

[Lyude Paul]

This is a trimmed down version of

https://patchwork.freedesktop.org/series/46637/

with all of the review comments addressed.

The last version of this series had fixes for the i2c and DP aux busses
to ensure that the GPU would be turned on whenever anything tried to
access the i2c/aux busses. Unfortunately: one of the fixes apparently
contained some very incorrect usage of Linux's runtime PM interface in
order to prevent runtime suspend/resume requests coming from within
nouveau's suspend/resume callbacks from deadlocking the system.

Turns out: fixing the i2c/dp aux problem is a lot harder then I
originally anticipated. We either need to come up with helpers for DRM
that work around the PM core, which is really not ideal, or come up with
a new feature for the RPM core that allows us to inhibit suspend/resume
callbacks. The former seems to be somewhat trivial to implement, but
coming up with a decent interface for that is going to take a bit more
time and I'd much rather have issues causing deadlocks get fixed first.
This means that drm_dp_aux_dev is going to be broken on nouveau for
laptops with hybrid GPUs using RPM, but it was already broken before
anyhow.

So: this just contains the seriously important fixes that will stop
people's machines from crashing very hard. Hopefully I can come up with
a solution to the i2c/aux problem soon after fixing some other glaring
MST bugs nouveau currently has.

Lyude Paul (2):
  drm/fb_helper: Add drm_fb_helper_output_poll_changed_with_rpm()
  drm/probe_helper: Add
drm_helper_probe_single_connector_modes_with_rpm()

 drivers/gpu/drm/drm_fb_helper.c | 23 +++
 drivers/gpu/drm/drm_probe_helper.c  | 31 +
 drivers/gpu/drm/nouveau/dispnv50/disp.c |  4 +--
 drivers/gpu/drm/nouveau/nouveau_connector.c |  4 +--
 include/drm/drm_crtc_helper.h   |  7 +++--
 include/drm/drm_fb_helper.h |  5 
 6 files changed, 67 insertions(+), 7 deletions(-)

-- 
2.17.1

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

Re: [PATCH] drm/exynos/mixer: Remove unused local variable priv

[Tobias Jakobi]

Hello Krzysztof,

looks good to me!

Reviewed-by: Tobias Jakobi 

- Tobias


Krzysztof Kozlowski wrote:
> Remove local variable 'priv' to fix GCC warning:
> 
> drivers/gpu/drm/exynos/exynos_mixer.c: In function 'mixer_initialize':
> drivers/gpu/drm/exynos/exynos_mixer.c:840:29: warning: variable 'priv' 
> set but not used [-Wunused-but-set-variable]
> 
> Signed-off-by: Krzysztof Kozlowski 
> ---
>  drivers/gpu/drm/exynos/exynos_mixer.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
> b/drivers/gpu/drm/exynos/exynos_mixer.c
> index 272c79f5f5bf..b86ae2ab9590 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -837,8 +837,6 @@ static int mixer_initialize(struct mixer_context 
> *mixer_ctx,
>   struct drm_device *drm_dev)
>  {
>   int ret;
> - struct exynos_drm_private *priv;
> - priv = drm_dev->dev_private;
>  
>   mixer_ctx->drm_dev = drm_dev;
>  
> 

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

Re: [Nouveau] [PATCH 1/5] drm/nouveau: Prevent RPM callback recursion in suspend/resume paths

[Lyude Paul]

On Wed, 2018-07-18 at 10:36 +0200, Lukas Wunner wrote:
> On Wed, Jul 18, 2018 at 10:25:05AM +0200, Lukas Wunner wrote:
> > The GPU contains an i2c subdevice for each connector with DDC lines.
> > I believe those are modelled as children of the GPU's PCI device as
> > they're accessed via mmio of the PCI device.
> > 
> > The problem here is that when the GPU's PCI device runtime suspends,
> > its i2c child device needs to be runtime active to suspend the MST
> > topology.  Catch-22.
> > 
> > I don't know whether or not it's necessary to suspend the MST topology.
> > I'm not an expert on DisplayPort MultiStream transport.
> > 
> > BTW Lyude, in patch 4 and 5 of this series, you're runtime resuming
> > pad->i2c->subdev.device->dev.  Is this the PCI device or is it the i2c
> > device?  I'm always confused by nouveau's structs.  In nvkm_i2c_bus_ctor()
> > I can see that the device you're runtime resuming is the parent of the
> > i2c_adapter:
> > 
> > struct nvkm_device *device = pad->i2c->subdev.device;
> > [...]
> > bus->i2c.dev.parent = device->dev;
> > 
> > If the i2c_adapter is a child of the PCI device, it's sufficient
> > to runtime resume the i2c_adapter, i.e. bus->i2c.dev, and this will
> > implicitly runtime resume its parent.
> 
> Actually, having written all this I just remembered that we have this
> in the documentation:
> 
> 8. "No-Callback" Devices
> 
> Some "devices" are only logical sub-devices of their parent and cannot
> be
> power-managed on their own. [...]
> 
> Subsystems can tell the PM core about these devices by calling
> pm_runtime_no_callbacks().
> 
> So it might actually be sufficient to just call pm_runtime_no_callbacks()

I would have hoped so, but unfortunately it seems that
pm_runtime_no_callbacks() is already called by default for i2c adapters in
i2c_register_adapter(). Unfortunately this really can't fix the problem
though, because it will still try to runtime resume the parent device of the
i2c adapter, which still leads to deadlocking in the runtime suspend/resume
path.

Additionally; I did play around with ignore_children, but unfortunately this
isn't good enough either as it just means that our i2c devices won't wake the
GPU up on access.

I'm pretty stumped here on trying to figure out any clean way to handle this
in the PM core if recursive resume calls are off the table. The only possible
solution I could see to this is if we could disable execution of runtime
callbacks in the context of a certain task (while all other tasks have to
honor the runtime PM callbacks), do what we need to do in suspend, then re-
enable them
> for the i2c devices...
> 
> Lukas
-- 
Cheers,
Lyude Paul
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PULL] drm-misc-next

[Gustavo Padovan]

Hi Dave,

Another round for 4.19. Fixes the build issue for sun4i. Many fixes and
improvements, most interesting thing is probably the DisplayPort
CEC-Tunneling-over-AUX support. Please pull, thanks.

drm-misc-next-2018-07-18:
drm-misc-next for 4.19:

Core Changes:
- add support for DisplayPort CEC-Tunneling-over-AUX (Hans Verkuil)
- more doc updates (Daniel Vetter)
- fourcc: Add is_yuv field to drm_format_info (Ayan Kumar Halder)
- dma-buf: correctly place BUG_ON (Michel Dänzer)

Driver Changes:
- more vkms support(Rodrigo Siqueira)
- many fixes and small improments to all drivers
The following changes since commit ae61f61fa802c829fa8d505587f9b337e63ea586:

  drm/client: Fix: drm_client_new: Don't require DRM to be registered 
(2018-07-11 22:25:51 +0200)

are available in the Git repository at:

  git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-2018-07-18

for you to fetch changes up to 979c11ef39cee79d6f556091a357890962be2580:

  drm/sun4i: Substitute sun4i_backend_format_is_yuv() with format->is_yuv 
(2018-07-18 17:00:29 +0100)


drm-misc-next for 4.19:

Core Changes:
- add support for DisplayPort CEC-Tunneling-over-AUX (Hans Verkuil)
- more doc updates (Daniel Vetter)
- fourcc: Add is_yuv field to drm_format_info (Ayan Kumar Halder)
- dma-buf: correctly place BUG_ON (Michel Dänzer)

Driver Changes:
- more vkms support(Rodrigo Siqueira)
- many fixes and small improments to all drivers


Alexandru Gheorghe (1):
  drm: writeback: Fix doc that says connector should be disconnected

Arnd Bergmann (2):
  drm/sun4i: fix build failure with CONFIG_DRM_SUN8I_MIXER=m
  drm/tinydrm: add backlight dependency for ili9341

Ayan Kumar Halder (5):
  drm/arm/malidp: Add modifier definitions for describing Arm Framebuffer 
Compression (AFBC).
  drm/fourcc: Add is_yuv field to drm_format_info to denote if the format 
is yuv
  drm/i915: Substitute intel_format_is_yuv() with format->is_yuv
  drm/rockchip: Substitute is_yuv_support() with format->is_yuv
  drm/sun4i: Substitute sun4i_backend_format_is_yuv() with format->is_yuv

Daniel Vetter (12):
  drm: move drv test macros out of drmP.h
  drm/doc: switch drm_connector_state to inline comments
  drm/doc: polish for sturct drm_connector
  drm: drop _mode_ from update_edit_property()
  drm: drop _mode_ from drm_mode_connector_attach_encoder
  drm: drop _mode_ from remaining connector functions
  drm: Switch drm_plane_state to inline kerneldoc style
  drm: switch drm_plane to inline comments
  drm/doc: move struct drm_crtc to in-line comments
  drm/doc: Group the fb gem helpers better
  drm/doc: Include drm_of.c helpers
  drm/doc: use inline kerneldoc style for drm_crtc_state

Eames Trinh (1):
  drm: gma500: Changed __attribute__((packed)) to __packed

Gustavo A. R. Silva (1):
  drm/pl111: Use 64-bit arithmetic instead of 32-bit

Hans Verkuil (3):
  drm: add support for DisplayPort CEC-Tunneling-over-AUX
  drm-kms-helpers.rst: document the DP CEC helpers
  drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

Jernej Skrabec (2):
  drm/sun4i: mixer: Read id from DT
  drm/sun4i: Implement zpos for DE2

Lyude Paul (2):
  drm/dp_helper: Add DP aux channel tracing
  drm/connector: Fix typo in drm_connector_list_iter_next()

Maxime Ripard (1):
  drm/sun4i: tcon-top: Fix return type warning

Michel Dänzer (1):
  dma-buf: Move BUG_ON from _add_shared_fence to _add_shared_inplace

Neil Armstrong (1):
  drm/meson: Make DMT timings parameters and pixel clock generic

Noralf Trønnes (1):
  drm/client: Fix double free in error path

Paul Kocialkowski (1):
  drm/sun4i: sun8i: Avoid clearing blending order at each atomic commit

Rodrigo Siqueira (4):
  drm/vkms: Add dumb operations
  drm/vkms: Add connectors helpers
  drm/vkms: Add vblank events simulated by hrtimers
  drm/vkms: Add framebuffer and plane helpers

Sean Paul (1):
  drm: Fix kerneldoc for DRM_MODE_PROP_IMMUTABLE

Thomas Zimmermann (5):
  drm/stm: Replace drm_dev_unref with drm_dev_put
  drm/sti: Replace drm_dev_unref with drm_dev_put
  drm/sun4i: Replace drm_dev_unref with drm_dev_put
  drm/vc4: Replace drm_dev_unref with drm_dev_put
  drm/pl111: Replace drm_dev_unref with drm_dev_put

Ville Syrjälä (3):
  drm: Extract __setplane_check()
  drm: Introduce __setplane_atomic()
  drm: Skip __drm_mode_set_config_internal() on atomic drivers

Wei Yongjun (1):
  drm/sun4i: DW HDMI: Make symbol sun8i_dw_hdmi_pltfm_driver static

 Documentation/gpu/drm-kms-helpers.rst  |  26 +-
 Documentation/gpu/drm-kms.rst  |  13 +-
 drivers/dma-buf/reservation.c  |   6 +-
 drivers/gpu/drm/Kconfig|  10 +
 drivers/gpu/drm/Makefile 

[PATCH] drm/exynos/mixer: Remove unused local variable priv

[Krzysztof Kozlowski]

Remove local variable 'priv' to fix GCC warning:

drivers/gpu/drm/exynos/exynos_mixer.c: In function 'mixer_initialize':
drivers/gpu/drm/exynos/exynos_mixer.c:840:29: warning: variable 'priv' set 
but not used [-Wunused-but-set-variable]

Signed-off-by: Krzysztof Kozlowski 
---
 drivers/gpu/drm/exynos/exynos_mixer.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
b/drivers/gpu/drm/exynos/exynos_mixer.c
index 272c79f5f5bf..b86ae2ab9590 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -837,8 +837,6 @@ static int mixer_initialize(struct mixer_context *mixer_ctx,
struct drm_device *drm_dev)
 {
int ret;
-   struct exynos_drm_private *priv;
-   priv = drm_dev->dev_private;
 
mixer_ctx->drm_dev = drm_dev;
 
-- 
2.14.1

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

Re: [PATCH v2 1/1] fbdev/core: Disable console-lock warnings when fb.lockless_register_fb is set

[kbuild test robot]

Hi Thomas,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on sof-driver-fuweitax/master]
[also build test ERROR on v4.18-rc5 next-20180718]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/fbdev-core-Disable-console-lock-warnings-when-fb-lockless_register_fb-is-set/20180719-023522
base:   https://github.com/fuweitax/linux master
config: i386-randconfig-s1-07181402 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/video/fbdev/core/fbmem.c: In function 'do_register_framebuffer':
>> drivers/video/fbdev/core/fbmem.c:1642:43: error: 
>> 'ignore_console_lock_warning' undeclared (first use in this function)
 bool saved_ignore_console_lock_warning = ignore_console_lock_warning;
  ^~~
   drivers/video/fbdev/core/fbmem.c:1642:43: note: each undeclared identifier 
is reported only once for each function it appears in

vim +/ignore_console_lock_warning +1642 drivers/video/fbdev/core/fbmem.c

  1631  
  1632  static bool lockless_register_fb;
  1633  module_param_named_unsafe(lockless_register_fb, lockless_register_fb, 
bool, 0400);
  1634  MODULE_PARM_DESC(lockless_register_fb,
  1635  "Lockless framebuffer registration for debugging 
[default=off]");
  1636  
  1637  static int do_register_framebuffer(struct fb_info *fb_info)
  1638  {
  1639  int i, ret;
  1640  struct fb_event event;
  1641  struct fb_videomode mode;
> 1642  bool saved_ignore_console_lock_warning = 
> ignore_console_lock_warning;
  1643  
  1644  if (fb_check_foreignness(fb_info))
  1645  return -ENOSYS;
  1646  
  1647  ret = do_remove_conflicting_framebuffers(fb_info->apertures,
  1648   fb_info->fix.id,
  1649   
fb_is_primary_device(fb_info));
  1650  if (ret)
  1651  return ret;
  1652  
  1653  if (num_registered_fb == FB_MAX)
  1654  return -ENXIO;
  1655  
  1656  num_registered_fb++;
  1657  for (i = 0 ; i < FB_MAX; i++)
  1658  if (!registered_fb[i])
  1659  break;
  1660  fb_info->node = i;
  1661  atomic_set(_info->count, 1);
  1662  mutex_init(_info->lock);
  1663  mutex_init(_info->mm_lock);
  1664  
  1665  fb_info->dev = device_create(fb_class, fb_info->device,
  1666   MKDEV(FB_MAJOR, i), NULL, "fb%d", 
i);
  1667  if (IS_ERR(fb_info->dev)) {
  1668  /* Not fatal */
  1669  printk(KERN_WARNING "Unable to create device for 
framebuffer %d; errno = %ld\n", i, PTR_ERR(fb_info->dev));
  1670  fb_info->dev = NULL;
  1671  } else
  1672  fb_init_device(fb_info);
  1673  
  1674  if (fb_info->pixmap.addr == NULL) {
  1675  fb_info->pixmap.addr = kmalloc(FBPIXMAPSIZE, 
GFP_KERNEL);
  1676  if (fb_info->pixmap.addr) {
  1677  fb_info->pixmap.size = FBPIXMAPSIZE;
  1678  fb_info->pixmap.buf_align = 1;
  1679  fb_info->pixmap.scan_align = 1;
  1680  fb_info->pixmap.access_align = 32;
  1681  fb_info->pixmap.flags = FB_PIXMAP_DEFAULT;
  1682  }
  1683  }   
  1684  fb_info->pixmap.offset = 0;
  1685  
  1686  if (!fb_info->pixmap.blit_x)
  1687  fb_info->pixmap.blit_x = ~(u32)0;
  1688  
  1689  if (!fb_info->pixmap.blit_y)
  1690  fb_info->pixmap.blit_y = ~(u32)0;
  1691  
  1692  if (!fb_info->modelist.prev || !fb_info->modelist.next)
  1693  INIT_LIST_HEAD(_info->modelist);
  1694  
  1695  if (fb_info->skip_vt_switch)
  1696  pm_vt_switch_required(fb_info->dev, false);
  1697  else
  1698  pm_vt_switch_required(fb_info->dev, true);
  1699  
  1700  fb_var_to_videomode(, _info->var);
  1701  fb_add_videomode(, _info->modelist);
  1702  registered_fb[i] = fb_info;
  1703  
  1704  event.info = fb_info;
  1705  if (!lockless_register_fb)
  1706  console_lock();
  1707  else
  1708  ignore_console_lock_warning = true;
  1709  if (!lock_fb_info(fb_info)) {
  1710  ret = -ENODEV;
  171

[PATCH v2 1/2] meson.build: fix intel atomics detection

[Peter Seiderer]

Use the stronger compiler.link() test (instead of the weaker
compiler.compile()) to fix the intel atomics detection.

Fixes false positive in case of sparc compile (buildroot toolchain).

Signed-off-by: Peter Seiderer 
---
Changes v1 -> v2:
  - link test needs main() method
---
 meson.build | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 9b443a50..3c8afb6d 100644
--- a/meson.build
+++ b/meson.build
@@ -49,9 +49,10 @@ intel_atomics = false
 lib_atomics = false
 
 dep_atomic_ops = dependency('atomic_ops', required : false)
-if cc.compiles('''
+if cc.links('''
 int atomic_add(int *i) { return __sync_add_and_fetch (i, 1); }
 int atomic_cmpxchg(int *i, int j, int k) { return 
__sync_val_compare_and_swap (i, j, k); }
+int main() { }
 ''',
 name : 'Intel Atomics')
   intel_atomics = true
-- 
2.18.0

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

[PATCH v2 2/2] meson.build: enable static build

[Peter Seiderer]

Use meson library instead of shared_library to enable static build.

Signed-off-by: Peter Seiderer 
---
Changes v1 -> v2:
  - new patch
---
 amdgpu/meson.build| 2 +-
 etnaviv/meson.build   | 2 +-
 exynos/meson.build| 2 +-
 freedreno/meson.build | 2 +-
 intel/meson.build | 2 +-
 libkms/meson.build| 2 +-
 meson.build   | 2 +-
 nouveau/meson.build   | 2 +-
 omap/meson.build  | 2 +-
 radeon/meson.build| 2 +-
 tegra/meson.build | 2 +-
 11 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/amdgpu/meson.build b/amdgpu/meson.build
index f39d7bf6..1b4b0be6 100644
--- a/amdgpu/meson.build
+++ b/amdgpu/meson.build
@@ -21,7 +21,7 @@
 
 datadir_amdgpu = join_paths(get_option('prefix'), get_option('datadir'), 
'libdrm')
 
-libdrm_amdgpu = shared_library(
+libdrm_amdgpu = library(
   'drm_amdgpu',
   [
 files(
diff --git a/etnaviv/meson.build b/etnaviv/meson.build
index ca2aa544..a0d994ea 100644
--- a/etnaviv/meson.build
+++ b/etnaviv/meson.build
@@ -19,7 +19,7 @@
 # SOFTWARE.
 
 
-libdrm_etnaviv = shared_library(
+libdrm_etnaviv = library(
   'drm_etnaviv',
   [
 files(
diff --git a/exynos/meson.build b/exynos/meson.build
index 30d36405..fd14f3a6 100644
--- a/exynos/meson.build
+++ b/exynos/meson.build
@@ -18,7 +18,7 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
 
-libdrm_exynos = shared_library(
+libdrm_exynos = library(
   'drm_exynos',
   [files('exynos_drm.c', 'exynos_fimg2d.c'), config_file],
   c_args : warn_c_args,
diff --git a/freedreno/meson.build b/freedreno/meson.build
index 015b7fb1..6c8a6a7b 100644
--- a/freedreno/meson.build
+++ b/freedreno/meson.build
@@ -39,7 +39,7 @@ if with_freedreno_kgsl
   )
 endif
 
-libdrm_freedreno = shared_library(
+libdrm_freedreno = library(
   'drm_freedreno',
   [files_freedreno, config_file],
   c_args : warn_c_args,
diff --git a/intel/meson.build b/intel/meson.build
index 53c7fce4..14cabd37 100644
--- a/intel/meson.build
+++ b/intel/meson.build
@@ -18,7 +18,7 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
 
-libdrm_intel = shared_library(
+libdrm_intel = library(
   'drm_intel',
   [
 files(
diff --git a/libkms/meson.build b/libkms/meson.build
index 86d1a4ee..9d21e4ca 100644
--- a/libkms/meson.build
+++ b/libkms/meson.build
@@ -41,7 +41,7 @@ if with_exynos
   libkms_include += include_directories('../exynos')
 endif
 
-libkms = shared_library(
+libkms = library(
   'kms',
   [files_libkms, config_file],
   c_args : warn_c_args,
diff --git a/meson.build b/meson.build
index 3c8afb6d..7aa5f8c6 100644
--- a/meson.build
+++ b/meson.build
@@ -279,7 +279,7 @@ add_project_arguments('-include', 'config.h', language : 
'c')
 inc_root = include_directories('.')
 inc_drm = include_directories('include/drm')
 
-libdrm = shared_library(
+libdrm = library(
   'drm',
   [files(
  'xf86drm.c', 'xf86drmHash.c', 'xf86drmRandom.c', 'xf86drmSL.c',
diff --git a/nouveau/meson.build b/nouveau/meson.build
index 51c9a712..acba048e 100644
--- a/nouveau/meson.build
+++ b/nouveau/meson.build
@@ -19,7 +19,7 @@
 # SOFTWARE.
 
 
-libdrm_nouveau = shared_library(
+libdrm_nouveau = library(
   'drm_nouveau',
   [files( 'nouveau.c', 'pushbuf.c', 'bufctx.c', 'abi16.c'), config_file],
   c_args : warn_c_args,
diff --git a/omap/meson.build b/omap/meson.build
index e57b8f5d..6cffb992 100644
--- a/omap/meson.build
+++ b/omap/meson.build
@@ -18,7 +18,7 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
 
-libdrm_omap = shared_library(
+libdrm_omap = library(
   'drm_omap',
   [files('omap_drm.c'), config_file],
   include_directories : [inc_root, inc_drm],
diff --git a/radeon/meson.build b/radeon/meson.build
index b08c7442..2f45ff79 100644
--- a/radeon/meson.build
+++ b/radeon/meson.build
@@ -19,7 +19,7 @@
 # SOFTWARE.
 
 
-libdrm_radeon = shared_library(
+libdrm_radeon = library(
   'drm_radeon',
   [
 files(
diff --git a/tegra/meson.build b/tegra/meson.build
index 1f5c74b3..8d0cfa92 100644
--- a/tegra/meson.build
+++ b/tegra/meson.build
@@ -18,7 +18,7 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
 
-libdrm_tegra = shared_library(
+libdrm_tegra = library(
   'drm_tegra',
   [files('tegra.c'), config_file],
   include_directories : [inc_root, inc_drm],
-- 
2.18.0

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

Re: [PATCH 1/1] fbdev/core: Disable console-lock warnings when fb.lockless_register_fb is set

[kbuild test robot]

Hi Thomas,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on sof-driver-fuweitax/master]
[also build test ERROR on v4.18-rc5 next-20180718]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/fbdev-core-Disable-console-lock-warnings-when-fb-lockless_register_fb-is-set/20180719-023035
base:   https://github.com/fuweitax/linux master
config: x86_64-randconfig-x018-201828 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/video/fbdev/core/fbmem.c: In function 'do_register_framebuffer':
>> drivers/video/fbdev/core/fbmem.c:1642:43: error: 
>> 'ignore_console_lock_warning' undeclared (first use in this function); did 
>> you mean 'saved_ignore_console_lock_warning'?
 bool saved_ignore_console_lock_warning = ignore_console_lock_warning;
  ^~~
  saved_ignore_console_lock_warning
   drivers/video/fbdev/core/fbmem.c:1642:43: note: each undeclared identifier 
is reported only once for each function it appears in

vim +1642 drivers/video/fbdev/core/fbmem.c

  1631  
  1632  static bool lockless_register_fb;
  1633  module_param_named_unsafe(lockless_register_fb, lockless_register_fb, 
bool, 0400);
  1634  MODULE_PARM_DESC(lockless_register_fb,
  1635  "Lockless framebuffer registration for debugging 
[default=off]");
  1636  
  1637  static int do_register_framebuffer(struct fb_info *fb_info)
  1638  {
  1639  int i, ret;
  1640  struct fb_event event;
  1641  struct fb_videomode mode;
> 1642  bool saved_ignore_console_lock_warning = 
> ignore_console_lock_warning;
  1643  
  1644  if (fb_check_foreignness(fb_info))
  1645  return -ENOSYS;
  1646  
  1647  ret = do_remove_conflicting_framebuffers(fb_info->apertures,
  1648   fb_info->fix.id,
  1649   
fb_is_primary_device(fb_info));
  1650  if (ret)
  1651  return ret;
  1652  
  1653  if (num_registered_fb == FB_MAX)
  1654  return -ENXIO;
  1655  
  1656  num_registered_fb++;
  1657  for (i = 0 ; i < FB_MAX; i++)
  1658  if (!registered_fb[i])
  1659  break;
  1660  fb_info->node = i;
  1661  atomic_set(_info->count, 1);
  1662  mutex_init(_info->lock);
  1663  mutex_init(_info->mm_lock);
  1664  
  1665  fb_info->dev = device_create(fb_class, fb_info->device,
  1666   MKDEV(FB_MAJOR, i), NULL, "fb%d", 
i);
  1667  if (IS_ERR(fb_info->dev)) {
  1668  /* Not fatal */
  1669  printk(KERN_WARNING "Unable to create device for 
framebuffer %d; errno = %ld\n", i, PTR_ERR(fb_info->dev));
  1670  fb_info->dev = NULL;
  1671  } else
  1672  fb_init_device(fb_info);
  1673  
  1674  if (fb_info->pixmap.addr == NULL) {
  1675  fb_info->pixmap.addr = kmalloc(FBPIXMAPSIZE, 
GFP_KERNEL);
  1676  if (fb_info->pixmap.addr) {
  1677  fb_info->pixmap.size = FBPIXMAPSIZE;
  1678  fb_info->pixmap.buf_align = 1;
  1679  fb_info->pixmap.scan_align = 1;
  1680  fb_info->pixmap.access_align = 32;
  1681  fb_info->pixmap.flags = FB_PIXMAP_DEFAULT;
  1682  }
  1683  }   
  1684  fb_info->pixmap.offset = 0;
  1685  
  1686  if (!fb_info->pixmap.blit_x)
  1687  fb_info->pixmap.blit_x = ~(u32)0;
  1688  
  1689  if (!fb_info->pixmap.blit_y)
  1690  fb_info->pixmap.blit_y = ~(u32)0;
  1691  
  1692  if (!fb_info->modelist.prev || !fb_info->modelist.next)
  1693  INIT_LIST_HEAD(_info->modelist);
  1694  
  1695  if (fb_info->skip_vt_switch)
  1696  pm_vt_switch_required(fb_info->dev, false);
  1697  else
  1698  pm_vt_switch_required(fb_info->dev, true);
  1699  
  1700  fb_var_to_videomode(, _info->var);
  1701  fb_add_videomode(, _info->modelist);
  1702  registered_fb[i] = fb_info;
  1703  
  1704  event.info = fb_info;
  1705  if (!lockless_register_fb)
  1706  console_lock();
  1707  else
  1708  ignore_console_lock_

Re: [PATCH v2 5/5] drm/vkms: Implement CRC debugfs API

[Sean Paul]

On Wed, Jul 18, 2018 at 07:24:34PM +0300, Haneen Mohammed wrote:
> First, just so you have the complete view, this is the missing part in this 
> patch:
> -- vkms_crc.c
> static uint32_t _vkms_get_crc(struct vkms_crc_data *crc_data)
> {
>   struct drm_framebuffer *fb = _data->fb;
>   struct drm_gem_object *gem_obj = drm_gem_fb_get_obj(fb, 0);
>   struct vkms_gem_object *vkms_obj = drm_gem_to_vkms_gem(gem_obj);
>   u32 crc = 0;
>   int i = 0;
>   unsigned int x = crc_data->src.x1 >> 16;
>   unsigned int y = crc_data->src.y1 >> 16;
>   unsigned int height = drm_rect_height(_data->src) >> 16;
>   unsigned int width = drm_rect_width(_data->src) >> 16;
>   unsigned int cpp = fb->format->cpp[0];
>   unsigned int src_offset;
>   unsigned int size_byte = width * cpp;
>   void *vaddr = vkms_obj->vaddr;
> 
>   if (WARN_ON(!vaddr))
>   return crc;
> 
>   for (i = y; i < y + height; i++) {
>   src_offset = fb->offsets[0] + (i * fb->pitches[0]) + (x * cpp);
>   crc = crc32_le(crc, vaddr + src_offset, size_byte);
>   }
> 
>   return crc;
> }
> 
> void vkms_crc_work_handle(struct work_struct *work)
> {
>   struct vkms_crtc_state *crtc_state = container_of(work,
>   struct vkms_crtc_state,
>   crc_work);
>   struct drm_crtc *crtc = crtc_state->base.crtc;
>   struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
> 
>   if (crtc_state && out->crc_enabled) {
>   u32 crc32 = _vkms_get_crc(_state->data);
>   unsigned int n_frame = crtc_state->n_frame;
> 
>   drm_crtc_add_crc_entry(crtc, true, n_frame, );
>   }
> }
> 
> int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
>   size_t *values_cnt)
> {
>   struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
> 
>   if (!src_name) {
>   out->crc_enabled = false;
>   } else if (strcmp(src_name, "auto") == 0) {
>   out->crc_enabled = true;
>   } else {
>   out->crc_enabled = false;
>   return -EINVAL;
>   }
> 
>   *values_cnt = 1;
> 
>   return 0;
> }
> --- end vkms_crc.c

/snip

> > > > > > +static void vkms_crtc_atomic_begin(struct drm_crtc *crtc,
> > > > > > +  struct drm_crtc_state 
> > > > > > *old_crtc_state)
> > > > > > +{
> > > > > > +   struct vkms_output *vkms_output = drm_crtc_to_vkms_output(crtc);
> > > > > > +
> > > > > > +   spin_lock_irq(_output->lock);
> > > > > 
> > > > > Hmm, you can't lock across atomic_begin/flush. What is this 
> > > > > protecting against?
> > > > > 
> > > > 
> > > > I did this per Daniel recommendation:
> > > > 1- spin_lock_irq -> vkms_crtc_atomic_begin
> > > > 2- update vkms_crc_data and get ref to fb -> vkms_primary_plane_update
> > > > 3- spin_unlock_irq after vblank event -> vkms_crtc_atomic_flush
> > > 
> > > I think I'm interpreting his mail a little differently. The point of the
> > > spinlock is to protect the pointer to the crc_data in vkms_crtc (right 
> > > now it's
> > > not a pointer, but it should be).
> > > 
> > > So I think you need to do the following:
> > > - Change crc_data to a pointer
> > > - In plane_update, allocate new memory to hold crc_data and, under the 
> > > spinlock,
> > >   update the crc_data to point to the new memory (add a WARN_ON if the 
> > > pointer
> > >   is NULL)
> > > - In the hrtimer, while holding the spinlock, take the pointer from 
> > > crc_data
> > >   into a local variable and clear the crtc->crc_data pointer
> > > - Pass the pointer (from the local variable) to the crc_worker
> > > 
> 
> The problem with this approach is managing the pointer allocation and
> deallocation. If I grab it in the hrtimer, then where/when should I free it?
> I can't after the crc worker is done with it because what I noticed is
> that the crc computation is expected to run multiple times after one
> plane_atomic_update.
> 
> > > I don't think you need to hold the spinlock across the atomic hooks, just 
> > > grab
> > > it in plane_update.
> > 
> 
> I asked Daniel in irc about releasing the vkms_output->lock in the begginning 
> of atomic_flush
> to avoid modifying the event_lock and he mentioned that "output->lock must 
> wrap the event
> handling code completely since we need to make sure that the flip event
> and the crc all match up".
> 
> So maybe I can grab the spinlock in plane_atomic_update and release it at
> the end of crtc_atomic_flush? Is plane_atomic_update always called
> before crtc_atomic_flush?
> 
> Otherwise if I grab the spinlock in atomic_begin I won't be able to
> use pointer for crc_data and allocate it in plane_atomic_update since
> I'll be holding a spinlock already. 
> 
> I have based this patch on Daniel's second feedback to my email
> "[PATCH] drm/vkms: Update CRC support after lock review" which I 

[Bug 102962] GPU crash running Overwatch in wine-staging

[bugzilla-daemon]

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

--- Comment #12 from Tobias Auerochs  ---
(In reply to Timothy Arceri from comment #11)
> (In reply to gloriouseggroll from comment #9)
> > Created attachment 138466 [details]
> > broken colors with mesa override
> > 
> > This was originally able to be resolved using
> > MESA_GL_VERSION_OVERRIDE=4.0COMPAT, however commit
> > 72de747c6a6cdd0be84a24932a4452f453861dbe broke that functionality, so that
> > MESA-GL_VERSION_OVERRIDE no longer renders properly, at least on my system.
> > See attached.
> 
> radeonsi now has compat 4.4 enabled by default (in git and upcoming 18.2
> release). Does the game now work for you with a recent Mesa from git?

Tried launching the game with mesa-git 103535.88e56804a7 (unofficial Arch
repo), rendering in menus (including backgrounds) worked correctly, however the
system still hangs the same way as before (e.g. when selecting Symmetra).

Trying MESA_GL_VERSION_OVERRIDE=4.0COMPAT on mesa-git appears to have no effect
at all anymore, or any other version for that matter. (With the override it
does not freeze on mesa 18.1.4, but graphics glitches occur with it, without
the override it renders correctly and freezes as well, just like mesa-git does)

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 107277] Raven: pci_pm_suspend takes over 1 second

[bugzilla-daemon]

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

Paul Menzel  changed:

   What|Removed |Added

 Status|NEEDINFO|NEW

--- Comment #6 from Paul Menzel  ---
(In reply to Paul Menzel from comment #3)
> (In reply to Christian König from comment #2)

[…]

> > > amdgpu_device_ip_suspend [amdgpu] (694.390 ms @ 75.000977)
> > 
> > This is hardware teardown and rather interesting and the only point we could
> > actually do something. Can you figure out what takes so long here?
> 
> I’ll try to figure that out.

I increased the maximum depth to 10, and according to the trace the loop in
`gfx_v9_0_enter_rlc_safe_mode()` is the culprit. Also, in all the function is
called three times.

static void gfx_v9_0_enter_rlc_safe_mode(struct amdgpu_device *adev)
{
uint32_t rlc_setting, data;
unsigned i;

if (adev->gfx.rlc.in_safe_mode)
return;

/* if RLC is not enabled, do nothing */
rlc_setting = RREG32_SOC15(GC, 0, mmRLC_CNTL);
if (!(rlc_setting & RLC_CNTL__RLC_ENABLE_F32_MASK))
return;

if (adev->cg_flags &
(AMD_CG_SUPPORT_GFX_CGCG | AMD_CG_SUPPORT_GFX_MGCG |
 AMD_CG_SUPPORT_GFX_3D_CGCG)) {
data = RLC_SAFE_MODE__CMD_MASK;
data |= (1 << RLC_SAFE_MODE__MESSAGE__SHIFT);
WREG32_SOC15(GC, 0, mmRLC_SAFE_MODE, data);

/* wait for RLC_SAFE_MODE */
for (i = 0; i < adev->usec_timeout; i++) {
if (!REG_GET_FIELD(SOC15_REG_OFFSET(GC, 0,
mmRLC_SAFE_MODE), RLC_SAFE_MODE, CMD))
break;
udelay(1);
}
adev->gfx.rlc.in_safe_mode = true;
}
}

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCHv3] lib/ratelimit: Lockless ratelimiting

[Andy Shevchenko]

On Tue, Jul 17, 2018 at 3:59 AM, Dmitry Safonov  wrote:
> I would be glad if someone helps/bothers to review the change :C
>

Perhaps Petr and / or Steven can help you.

> Thanks,
> Dmitry
>
> On Tue, 2018-07-03 at 23:56 +0100, Dmitry Safonov wrote:
>> Currently ratelimit_state is protected with spin_lock. If the .lock
>> is
>> taken at the moment of ___ratelimit() call, the message is suppressed
>> to
>> make ratelimiting robust.
>>
>> That results in the following issue issue:
>>   CPU0  CPU1
>> --   --
>> printk_ratelimit()   printk_ratelimit()
>> | |
>>   try_spin_lock()  try_spin_lock()
>> | |
>> time_is_before_jiffies()  return 0; // suppress
>>
>> So, concurrent call of ___ratelimit() results in silently suppressing
>> one of the messages, regardless if the limit is reached or not.
>> And rc->missed is not increased in such case so the issue is covered
>> from user.
>>
>> Convert ratelimiting to use atomic counters and drop spin_lock.
>>
>> Note: That might be unexpected, but with the first interval of
>> messages
>> storm one can print up to burst*2 messages. So, it doesn't guarantee
>> that in *any* interval amount of messages is lesser than burst.
>> But, that differs lightly from previous behavior where one can start
>> burst=5 interval and print 4 messages on the last milliseconds of
>> interval + new 5 messages from new interval (totally 9 messages in
>> lesser than interval value):
>>
>>msg0  msg1-msg4 msg0-msg4
>> | |   |
>> | |   |
>>  |--o-o-|-o|--> (t)
>>   <--->
>>Lesser than burst
>>
>> Dropped dev/random patch since v1 version:
>> lkml.kernel.org/r/<20180510125211.12583-1-d...@arista.com>
>>
>> Dropped `name' in as it's unused in RATELIMIT_STATE_INIT()
>>
>> Cc: Andy Shevchenko 
>> Cc: Arnd Bergmann 
>> Cc: David Airlie 
>> Cc: Greg Kroah-Hartman 
>> Cc: Jani Nikula 
>> Cc: Joonas Lahtinen 
>> Cc: Rodrigo Vivi 
>> Cc: "Theodore Ts'o" 
>> Cc: Thomas Gleixner 
>> Cc: intel-...@lists.freedesktop.org
>> Cc: dri-devel@lists.freedesktop.org
>> Signed-off-by: Dmitry Safonov 
>> ---
>>  drivers/char/random.c| 28 ---
>>  drivers/gpu/drm/i915/i915_perf.c |  8 --
>>  fs/btrfs/super.c | 16 +--
>>  fs/xfs/scrub/scrub.c |  2 +-
>>  include/linux/ratelimit.h| 31 -
>>  kernel/user.c|  2 +-
>>  lib/ratelimit.c  | 59 +++---
>> --
>>  7 files changed, 73 insertions(+), 73 deletions(-)
>>
>> diff --git a/drivers/char/random.c b/drivers/char/random.c
>> index cd888d4ee605..0be31b3eadab 100644
>> --- a/drivers/char/random.c
>> +++ b/drivers/char/random.c
>> @@ -439,10 +439,8 @@ static void _crng_backtrack_protect(struct
>> crng_state *crng,
>>  static void process_random_ready_list(void);
>>  static void _get_random_bytes(void *buf, int nbytes);
>>
>> -static struct ratelimit_state unseeded_warning =
>> - RATELIMIT_STATE_INIT("warn_unseeded_randomness", HZ, 3);
>> -static struct ratelimit_state urandom_warning =
>> - RATELIMIT_STATE_INIT("warn_urandom_randomness", HZ, 3);
>> +static struct ratelimit_state unseeded_warning =
>> RATELIMIT_STATE_INIT(HZ, 3);
>> +static struct ratelimit_state urandom_warning =
>> RATELIMIT_STATE_INIT(HZ, 3);
>>
>>  static int ratelimit_disable __read_mostly;
>>
>> @@ -937,24 +935,22 @@ static void crng_reseed(struct crng_state
>> *crng, struct entropy_store *r)
>>   crng->init_time = jiffies;
>>   spin_unlock_irqrestore(>lock, flags);
>>   if (crng == _crng && crng_init < 2) {
>> + unsigned int unseeded_miss, urandom_miss;
>> +
>>   invalidate_batched_entropy();
>>   numa_crng_init();
>>   crng_init = 2;
>>   process_random_ready_list();
>>   wake_up_interruptible(_init_wait);
>>   pr_notice("random: crng init done\n");
>> - if (unseeded_warning.missed) {
>> - pr_notice("random: %d get_random_xx
>> warning(s) missed "
>> -   "due to ratelimiting\n",
>> -   unseeded_warning.missed);
>> - unseeded_warning.missed = 0;
>> - }
>> - if (urandom_warning.missed) {
>> - pr_notice("random: %d urandom warning(s)
>> missed "
>> -   "due to ratelimiting\n",
>> -   urandom_warning.missed);
>> - urandom_warning.missed = 0;
>> - }
>> + unseeded_miss =
>> atomic_xchg(_warning.missed, 0);
>> + if (unseeded_miss)
>> + 

Re: [Freedreno] [PATCH 02/13] drm: drm_printer: Add printer for devcoredump

[Jordan Crouse]

On Thu, Jul 12, 2018 at 08:40:55PM +0100, Chris Wilson wrote:
> Quoting Jordan Crouse (2018-07-12 19:59:19)
> > Add a drm printer suitable for use with the read callback for
> > devcoredump or other suitable buffer based output format that
> > isn't otherwise covered by seq_file.
> > 
> > Signed-off-by: Jordan Crouse 
> > ---
> >  drivers/gpu/drm/drm_print.c | 74 +
> >  include/drm/drm_print.h | 27 ++
> >  2 files changed, 101 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> > index b25f98f33f6c..03d1f98e5ac7 100644
> > --- a/drivers/gpu/drm/drm_print.c
> > +++ b/drivers/gpu/drm/drm_print.c
> > @@ -30,6 +30,80 @@
> >  #include 
> >  #include 
> >  
> > +void __drm_printfn_coredump(struct drm_printer *p, struct va_format *vaf)
> > +{
> > +   struct drm_print_iterator *iterator = p->arg;
> > +   ssize_t len;
> > +
> > +   if (!iterator->remain)
> > +   return;
> > +
> > +   /* Figure out how big the string will be */
> > +   len = snprintf(NULL, 0, "%pV", vaf);
> 
> I was thinking there's some duplication here (kmalloc + snprintf) that
> could be reduced to kasprintf here. Is avoiding that allocation
> important or frequent enough to merit open coding?

> It's pity the kernel's printk doesn't support %n, so that leaves with
> 
> buf = kasprintf(GFP_... , "%pV", vaf);
> if (!buf)
>   return;
> 
> len = strlen(buf);
> 
> and even the copy + increment looks like it can then be factored to share
> more code.

I did a quick test - using kasprintf() unconditionally increased the total
time in my use case by about 4x.  I think this was mainly due to this case:

if (iterator->offset + len <= iterator->start) {

return;
}

That said, I was able to organize the code so that we can reuse much of the
same code for both the printf and puts funcs which saves us a bit of churn
later.

Jordan

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] backlight: pwm_bl: Fix uninitialized variable

[Daniel Thompson]

On Wed, Jul 18, 2018 at 04:55:44PM +0100, Lee Jones wrote:
> On Wed, 18 Jul 2018, Daniel Thompson wrote:
> 
> > On Wed, Jul 18, 2018 at 02:08:53PM +0100, Lee Jones wrote:
> > > > > > > No, then we are back to the initial issue of num_steps
> > > > > > > potentially not
> > > > > > > being initialised. We really want both of_property_read_u32() to
> > > > > > > succeed AND num_steps to actually be set.
> > > > > > 
> > > > > > I also think num_steps should be pre-initialised.
> > > > 
> > > > Yes, I guess it definitely does not hurt.
> > > > 
> > > > > > Then it will only be set if of_property_read_u32() succeeds.
> > > > 
> > > > Yes, but we still need to check for both, the function not failing and
> > > > num_steps to actually be non zero.
> > > 
> > > Why?  You don't do anything differently if it fails.
> > 
> > Only if you initialize num_steps...
> > 
> > We should either initialize to zero and not worry about the return
> > code[1] or we check the return code and not worry about
> > initialization[2]. I don't think both are worthwhile.
> > 
> > Whilst initialization can fix this specific instance we generally avoid
> > overusing it since it messes up static analysis and, in this instance,
> > distance from declaration to use is >25 lines, hence current patchset.
> > 
> > 
> > Daniel.
> > 
> > 
> > [1] https://lkml.org/lkml/2018/7/16/399
> > [2] https://lkml.org/lkml/2018/7/16/1042
> > 
> > Or...
> > 
> > We check the return code and leave number
> > 
> > num_steps is uninitialized and stack allocated so it only has a valid
> > value if of_property_read_u32() succeeds.
> > 
> > We can (and I originally did) fix the bug by initializing num_steps to 0
> > but its quite some distance between declaration and use so I accepted
> > Marcel's counter proposal to check the return code instead.
> 
> Only checking the return value of of_property_read_u32() is also
> suitable.

I did think about that case... I concluded that it isn't wrong for a
DT to set to this property to 0 (effectively meaning "no interpolated
steps please").

If we take the branch when num_steps is zero we get a bunch of pointless
housekeeping that amounts to no more than an extremely elaborate
malloc/memcpy/free.


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

[pull] amdgpu drm-fixes-4.18

[Alex Deucher]

Hi Dave,

Fixes for 4.18.  The ACP patch is a bit bigger than I would like
at this point, but it should have gone in long ago, it just fell
through the cracks.  The others are pretty small and straight-forward.
- ACP fix for boards with 2 I2S instances
- DP fix for CZ, vega
- Fix for a hybrid graphics laptop
- Fix a resume regression

The following changes since commit bf642e3a1996f1ed8f083c5ecd4b51270a9e11bc:

  Merge tag 'drm-intel-fixes-2018-07-12' of 
git://anongit.freedesktop.org/drm/drm-intel into drm-fixes (2018-07-16 10:32:28 
+1000)

are available in the git repository at:

  git://people.freedesktop.org/~agd5f/linux drm-fixes-4.18

for you to fetch changes up to 2d95ceb45459357288058c646022019d257ae04b:

  drm/amd/amdgpu: creating two I2S instances for stoney/cz (v2) (2018-07-18 
09:03:07 -0500)


Alex Deucher (1):
  drm/amdgpu: add another ATPX quirk for TOPAZ

Hersen Wu (1):
  drm/amd/display: Fix DP HBR2 Eye Diagram Pattern on Carrizo

Leo Liu (1):
  drm/amdgpu: Make sure IB tests flushed after IP resume

Vijendar Mukunda (1):
  drm/amd/amdgpu: creating two I2S instances for stoney/cz (v2)

 drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c| 47 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c   |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  3 ++
 drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c   |  8 ++--
 drivers/gpu/drm/amd/display/dc/dc.h|  1 +
 .../gpu/drm/amd/display/dc/dcn10/dcn10_resource.c  |  2 +
 6 files changed, 47 insertions(+), 15 deletions(-)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] backlight: pwm_bl: Fix uninitialized variable

[Lee Jones]

On Wed, 18 Jul 2018, Daniel Thompson wrote:

> On Wed, Jul 18, 2018 at 02:08:53PM +0100, Lee Jones wrote:
> > > > > > No, then we are back to the initial issue of num_steps
> > > > > > potentially not
> > > > > > being initialised. We really want both of_property_read_u32() to
> > > > > > succeed AND num_steps to actually be set.
> > > > > 
> > > > > I also think num_steps should be pre-initialised.
> > > 
> > > Yes, I guess it definitely does not hurt.
> > > 
> > > > > Then it will only be set if of_property_read_u32() succeeds.
> > > 
> > > Yes, but we still need to check for both, the function not failing and
> > > num_steps to actually be non zero.
> > 
> > Why?  You don't do anything differently if it fails.
> 
> Only if you initialize num_steps...
> 
> We should either initialize to zero and not worry about the return
> code[1] or we check the return code and not worry about
> initialization[2]. I don't think both are worthwhile.
> 
> Whilst initialization can fix this specific instance we generally avoid
> overusing it since it messes up static analysis and, in this instance,
> distance from declaration to use is >25 lines, hence current patchset.
> 
> 
> Daniel.
> 
> 
> [1] https://lkml.org/lkml/2018/7/16/399
> [2] https://lkml.org/lkml/2018/7/16/1042
> 
> Or...
> 
> We check the return code and leave number
> 
> num_steps is uninitialized and stack allocated so it only has a valid
> value if of_property_read_u32() succeeds.
> 
> We can (and I originally did) fix the bug by initializing num_steps to 0
> but its quite some distance between declaration and use so I accepted
> Marcel's counter proposal to check the return code instead.

Only checking the return value of of_property_read_u32() is also
suitable.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 107261] [drm:generic_reg_wait [amdgpu]] *ERROR* REG_WAIT timeout 1us * 10 tries - optc1_lock line:628

[bugzilla-daemon]

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

Paul Menzel  changed:

   What|Removed |Added

 CC||pmenzel+bugs.freedesktop@mo
   ||lgen.mpg.de

--- Comment #1 from Paul Menzel  ---
Created attachment 140703
  --> https://bugs.freedesktop.org/attachment.cgi?id=140703=edit
Linux 4.18-rc5+ messages with trace from `power_down_all_hw_blocks`

Here is another trace from `power_down_all_hw_blocks()`. Please note, the UEFI
firmware didn’t initialize the DP connected monitor correctly and the
framebuffer was just black until amdgpu loaded. Please tell me, if I should
create a separate bug for that.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 107277] Raven: pci_pm_suspend takes over 1 second

[bugzilla-daemon]

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

--- Comment #5 from Alex Deucher  ---
Created attachment 140702
  --> https://bugs.freedesktop.org/attachment.cgi?id=140702=edit
quick hack for s#

Here's a quick hack for S3, but it will need more work to not break S4 support
as well.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 107277] Raven: pci_pm_suspend takes over 1 second

[bugzilla-daemon]

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

Alex Deucher  changed:

   What|Removed |Added

 Attachment #140702|quick hack for s#   |quick hack for S3
description||

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 107277] Raven: pci_pm_suspend takes over 1 second

[bugzilla-daemon]

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

--- Comment #4 from Alex Deucher  ---
(In reply to Paul Menzel from comment #3)
> (In reply to Christian König from comment #2)
> > There isn't much you can do here:
> > > amdgpu_bo_evict_vram [amdgpu] (306.331 ms @ 74.694620)
> > 
> > This is evacuating the content of VRAM to RAM/disk to make sure we don't
> > lose screen content while suspended.
> 
> I do not understand that. The integrated graphics device uses the system RAM
> as VRAM doesn’t it? So why does it have to be evicted at all? Also, I
> believe it’s 1 GB of VRAM. That means the speed would be 3 GB/s, where it
> should be much higher with DDR4 shouldn’t it?

It's not a problem with S3 (suspend to ram), but it is for S4 (suspend to disk)
because power will be lost and the vram carve out area is not managed by the
OS.  Currently S3 and S4 share the same code paths, so they would need to be
reworked to handle S3 and S4 differently.  As for the time it takes, depending
on how much system memory is in use, some stuff may have to be swapped out to
disk to make room for all of the buffers in vram, so a lot of shuffling may
need to take place.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 107277] Raven: pci_pm_suspend takes over 1 second

[bugzilla-daemon]

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

Alex Deucher  changed:

   What|Removed |Added

   Severity|normal  |enhancement

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 107278] Raven: pci_pm_resume takes over 1 second

[bugzilla-daemon]

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

Alex Deucher  changed:

   What|Removed |Added

   Severity|normal  |enhancement

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/sun4i: sun4i: Introduce a quirk for lowest plane alpha support

[Maxime Ripard]

On Wed, Jul 18, 2018 at 11:35:23AM +0200, Paul Kocialkowski wrote:
> On Wed, 2018-07-18 at 11:29 +0200, Paul Kocialkowski wrote:
> > Not all sunxi platforms with the first version of the Display Engine
> > support an alpha component on the plane with the lowest z position
> > (as in: lowest z-pos), that gets blended with the background color.
> > 
> > In particular, the A13 is known to have this limitation. However, it was
> > recently discovered that the A20 and A33 are capable of having alpha on
> > their lowest plane.
> > 
> > Thus, this introduces a specific quirk to indicate such support,
> > per-platform. Since this was not tested on sun4i and sun6i platforms, a
> > conservative approach is kept and this feature is not supported.
> 
> Woops, I forgot to include PATCH 1/2 from v1, which did not change.
> 
> Would it be preferable for me to send v2 again (and maybe call it v3)
> for convenience?

Yeah, resend it, and call it v2 resend (explaining why a resend was
needed).

Maxime


-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 107277] Raven: pci_pm_suspend takes over 1 second

[bugzilla-daemon]

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

--- Comment #3 from Paul Menzel  ---
(In reply to Christian König from comment #2)
> There isn't much you can do here:
> > amdgpu_bo_evict_vram [amdgpu] (306.331 ms @ 74.694620)
> 
> This is evacuating the content of VRAM to RAM/disk to make sure we don't
> lose screen content while suspended.

I do not understand that. The integrated graphics device uses the system RAM 
as VRAM doesn’t it? So why does it have to be evicted at all? Also, I believe
it’s 1 GB of VRAM. That means the speed would be 3 GB/s, where it should be
much higher with DDR4 shouldn’t it?

> > amdgpu_fence_driver_suspend [amdgpu] (0.023 ms @ 75.000953)
> 
> Waiting for the evacuation to be completed.
> 
> > amdgpu_device_ip_suspend [amdgpu] (694.390 ms @ 75.000977)
> 
> This is hardware teardown and rather interesting and the only point we could
> actually do something. Can you figure out what takes so long here?

I’ll try to figure that out.

> > amdgpu_bo_evict_vram [amdgpu] (24.217 ms @ 75.695369)
> 
> Again evacuating VRAM which was locked before because the hardware was still
> using it.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 200517] Vega 8/Radeon 535 hybrid graphics - amdgpu crash on modesetting

[bugzilla-daemon]

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

--- Comment #14 from Alex Deucher (alexdeuc...@gmail.com) ---
On the GPU side, we are working on enabling gfxoff and stutter mode which
should save some additional power.  They are not enabled by default yet, but
should be in the near future.  For the platform side (CPU, FCH, etc.), you may
want to run something like powertop and see what can be done to save power on
things like USB, SATA, etc.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 107277] Raven: pci_pm_suspend takes over 1 second

[bugzilla-daemon]

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

Christian König  changed:

   What|Removed |Added

 Status|NEW |NEEDINFO

--- Comment #2 from Christian König  ---
There isn't much you can do here:
> amdgpu_bo_evict_vram [amdgpu] (306.331 ms @ 74.694620)

This is evacuating the content of VRAM to RAM/disk to make sure we don't lose
screen content while suspended.

> amdgpu_fence_driver_suspend [amdgpu] (0.023 ms @ 75.000953)

Waiting for the evacuation to be completed.

> amdgpu_device_ip_suspend [amdgpu] (694.390 ms @ 75.000977)

This is hardware teardown and rather interesting and the only point we could
actually do something. Can you figure out what takes so long here?

> amdgpu_bo_evict_vram [amdgpu] (24.217 ms @ 75.695369)

Again evacuating VRAM which was locked before because the hardware was still
using it.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [resend PATCH v4 3/5] clk: mediatek: mt8173: switch mmsys to platform device probing (fwd)

[Julia Lawall]

Hello,

Please check on whether the kfree in the remove function is still needed.
Also the platform_driver structure has two probe fields.

julia

-- Forwarded message --
Date: Wed, 18 Jul 2018 22:31:13 +0800
From: kbuild test robot 
To: kbu...@01.org
Cc: Julia Lawall 
Subject: Re: [resend PATCH v4 3/5] clk: mediatek: mt8173: switch mmsys to
platform device probing

In-Reply-To: <20180717220328.792-4-matthias@kernel.org>
References: <20180717220328.792-4-matthias@kernel.org>
TO: matthias@kernel.org

Hi Matthias,

I love your patch! Perhaps something to improve:

[auto build test WARNING on sof-driver-fuweitax/master]
[also build test WARNING on v4.18-rc5 next-20180718]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/matthias-bgg-kernel-org/arm-arm64-mediatek-Fix-mmsys-device-probing/20180718-191916
base:   https://github.com/fuweitax/linux master
:: branch date: 3 hours ago
:: commit date: 3 hours ago

>> drivers/clk/mediatek/clk-mt8173.c:1195:1-6: WARNING: invalid free of devm_ 
>> allocated data
--
>> drivers/clk/mediatek/clk-mt8173.c:1200:48-49: probe: first occurrence line 
>> 1201, second occurrence line 1202

# 
https://github.com/0day-ci/linux/commit/112857d0f58413a87875dfc9e1b43fdfde3b55eb
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 112857d0f58413a87875dfc9e1b43fdfde3b55eb
vim +1195 drivers/clk/mediatek/clk-mt8173.c

112857d0 Matthias Brugger 2018-07-18  1189
112857d0 Matthias Brugger 2018-07-18  1190  static int mtk_mmsys_remove(struct 
platform_device *pdev)
112857d0 Matthias Brugger 2018-07-18  1191  {
112857d0 Matthias Brugger 2018-07-18  1192  struct mtk_mmsys_priv *private 
= platform_get_drvdata(pdev);
112857d0 Matthias Brugger 2018-07-18  1193
112857d0 Matthias Brugger 2018-07-18  1194  kfree(private->clk_data);
112857d0 Matthias Brugger 2018-07-18 @1195  kfree(private);
112857d0 Matthias Brugger 2018-07-18  1196
112857d0 Matthias Brugger 2018-07-18  1197  return 0;
29859d93 James Liao   2015-05-20  1198  }
112857d0 Matthias Brugger 2018-07-18  1199
112857d0 Matthias Brugger 2018-07-18 @1200  static struct platform_driver 
clk_mt8173_mm_drv = {
112857d0 Matthias Brugger 2018-07-18 @1201  .probe = mtk_mmsys_probe,
112857d0 Matthias Brugger 2018-07-18 @1202  .probe = mtk_mmsys_remove,
112857d0 Matthias Brugger 2018-07-18  1203  .driver = {
112857d0 Matthias Brugger 2018-07-18  1204  .name = "clk-mt8173-mm",
112857d0 Matthias Brugger 2018-07-18  1205  },
112857d0 Matthias Brugger 2018-07-18  1206  };
112857d0 Matthias Brugger 2018-07-18  1207  
module_platform_driver(clk_mt8173_mm_drv);
29859d93 James Liao   2015-05-20  1208

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 200517] Vega 8/Radeon 535 hybrid graphics - amdgpu crash on modesetting

[bugzilla-daemon]

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

--- Comment #13 from bakarichar...@gmail.com ---
That is clear, thanks.

Another topic, you seems to be competent to ask your opinion: what do you think
why are my gpu and cpu core temps ~5-8C higher than on Windows 10. This is the
best what open/free drivers can produce? Tlp is in use of course.
If I know well, my Acer notebook's power management is totally acpi controlled
however the DSDT table was broken in Linux and I had to manually addressing two
pci/apic controllers to even start the kernel.
Do you have any suggestion what I should do not to be my legs burnt.
Thanks.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [resend PATCH v4 2/5] clk: mediatek: mt2701-mmsys: switch to platform device probing (fwd)

[Julia Lawall]

Please check on whether the kfree is still needed.

julia

-- Forwarded message --
Date: Wed, 18 Jul 2018 22:03:48 +0800
From: kbuild test robot 
To: kbu...@01.org
Cc: Julia Lawall 
Subject: Re: [resend PATCH v4 2/5] clk: mediatek: mt2701-mmsys: switch to
platform device probing

In-Reply-To: <20180717220328.792-3-matthias@kernel.org>
References: <20180717220328.792-3-matthias@kernel.org>
TO: matthias@kernel.org

Hi Matthias,

I love your patch! Perhaps something to improve:

[auto build test WARNING on sof-driver-fuweitax/master]
[also build test WARNING on v4.18-rc5 next-20180718]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/matthias-bgg-kernel-org/arm-arm64-mediatek-Fix-mmsys-device-probing/20180718-191916
base:   https://github.com/fuweitax/linux master
:: branch date: 3 hours ago
:: commit date: 3 hours ago

>> drivers/clk/mediatek/clk-mt2701-mm.c:128:1-6: WARNING: invalid free of devm_ 
>> allocated data

# 
https://github.com/0day-ci/linux/commit/555e11caac5f7729e8927939046adbfa2264f9f5
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 555e11caac5f7729e8927939046adbfa2264f9f5
vim +128 drivers/clk/mediatek/clk-mt2701-mm.c

e98621182 Shunli Wang  2016-11-04  122
555e11caa Matthias Brugger 2018-07-18  123  static int 
clk_mt2701_mm_remove(struct platform_device *pdev)
555e11caa Matthias Brugger 2018-07-18  124  {
555e11caa Matthias Brugger 2018-07-18  125  struct clk_mt2701_mm_priv 
*private = platform_get_drvdata(pdev);
555e11caa Matthias Brugger 2018-07-18  126
555e11caa Matthias Brugger 2018-07-18  127  kfree(private->clk_data);
555e11caa Matthias Brugger 2018-07-18 @128  kfree(private);
555e11caa Matthias Brugger 2018-07-18  129
555e11caa Matthias Brugger 2018-07-18  130  return 0;
555e11caa Matthias Brugger 2018-07-18  131  }
555e11caa Matthias Brugger 2018-07-18  132

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH RFC] drm: Add ASPEED GFX driver

[Joel Stanley]

Hi Eric,

Thanks for your reply back in April. I have finally found some time to
get back to this.

On 18 April 2018 at 03:43, Eric Anholt  wrote:
> Joel Stanley  writes:
>
>> This driver is for the ASPEED BMC SoC's GFX display hardware. This
>> driver runs on the ARM based BMC systems, unlike the ast driver which
>> runs on a host CPU and is is for a PCIe graphics device that happens to
>> live in the BMC's silicon, but is otherwise available for use by the BMC.
>>
>> The AST2500 supports a total of 3 output paths:
>>
>>   1. VGA output (aka host PCIe graphics device), the output target can
>>   choose either or both to the DAC or DVO interface.
>>
>>   2. Graphics CRT output, the output target can choose either or both to
>>   the DAC or DVO interface.
>>
>>   3. Video input from DVO, the video input can be used for video engine
>>   capture or DAC display output.
>>
>> Output options for are selected in SCU2C. This must be toggled by the
>> BMC in order to select between the host and BMC's display output.
>>
>> The "VGA mode" device is the PCI attached controller. The "Graphics CRT"
>> is the BMC's internal display controller.
>>
>> This driver only supports a simple configuration consisting of a 40MHz
>> pixel clock (fixed by hardware limitations) and the VGA output path.
>
> The confusing part of this driver to me is understanding where this
> display output is going -- if you're going out a VGA connector to a
> monitor (is that what "DAC" meant?), we ought to have
> DRM_MODE_CONNECTOR_VGA and a .get_modes that does I2C to get the EDID.
> If it's fed into the input of some other display controller, then I'm
> not sure what's the right thing to do.

Yes, DAC is what the datasheet calls the VGA connector. The hardware
has optional i2c lines, but they are not hooked up on any of the
systems I have access to.

I think most systems hard code a single mode, and this is the approach
we have taken when integrating this driver into OpenBMC.

>
> I'd recommend moving some of this commit message into kerneldoc in the
> _drv.c explaining what the HW does and what parts of the HW the driver
> exposes.

Good idea.

>
>> Signed-off-by: Joel Stanley 
>> ---
>>
>> Hello!
>>
>> This driver is working on hardware, with a few oddities. The things that
>> I know need cleaning up are:
>>
>>  - clocks: The device can source a clock from three different sources,
>>but they depend on the configuration of other devices in the system
>>(the USB PHY and the 'host' display controller). For this reason we
>>limit the driver to the 40MHz pixel clock that comes from the USB
>>PHY.
>
> You may want to look into setting up a little mux clock provider that
> can source from whatever parents are available to get you the best,
> faster-than-requested clock.  Then, if the rate it gives you is too far
> off from what you wanted, maybe stretch the hblank intervals of your
> modeline to get as close to the target vrefresh as possible (check out
> vc4_dsi.c:vc4_dsi_encoder_mode_fixup() for an example of doing this)

>
>>  - Setting an initial mode. I can only get the system to output when
>>running X. fbset and fb-test can't seem to do this on their own. The
>>system this is being developed for intends to run a simple fbterm
>>based application, so this needs fixing.
>
> Not sure what would be going on here.  You do have all of the DRM FB
> emulation bits enabled, right?

I sorted this out in the end. I had the enable_vblank/disable_vblank
callbacks hooked up to struct drm_driver.

As I was using the drm_simple stuff, it adds it's own enable_vblank on
struct drm_crtc_funcs (which then pokes into struct
drm_simple_display_pipe to grab the real vblank, if it exists).

The fb enable code checks to see if drm_crtc_funcs->enable_vblank
exists, and if it does, calls that and returns without calling the one
on struct drm_driver.

Once I discovered that having enable_vblank on drm_driver is
depreciated, the solution became obvious.

>
>> I wanted to get feedback on the way the driver is structured, and if
>> possible a some advice on why fb tools can't set a mode themselves.
>
> drm_simple_display_pipe seems like a good way to get started.
> Eventually to merge you'll need a DT binding document
> (Documentation/devicetree/bindings/display)
>

>> diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c 
>> b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c

>> +#include "aspeed_gfx.h"
>> +
>> +static const struct drm_mode_config_funcs aspeed_gfx_mode_config_funcs = {
>> + .fb_create  = drm_gem_fb_create,
>> + .atomic_check   = drm_atomic_helper_check,
>> + .atomic_commit  = drm_atomic_helper_commit,
>> +};
>
> I wonder if .output_poll_changed = drm_fb_helper_output_poll_changed,
> might help your fbdev setup?

fwiw this did not help.

>
>> +static void aspeed_gfx_setup_mode_config(struct drm_device *drm)
>> +{
>> + drm_mode_config_init(drm);
>> +
>> + drm->mode_config.min_width = 0;
>> +  

[Bug 107278] Raven: pci_pm_resume takes over 1 second

[bugzilla-daemon]

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

Paul Menzel  changed:

   What|Removed |Added

 CC||pmenzel+bugs.freedesktop@mo
   ||lgen.mpg.de

--- Comment #1 from Paul Menzel  ---
Created attachment 140701
  --> https://bugs.freedesktop.org/attachment.cgi?id=140701=edit
HTML output of `sudo ./sleepgraph.py -config config/suspend-callgraph.cfg
-maxdepth=5`

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 107278] Raven: pci_pm_resume takes over 1 second

[bugzilla-daemon]

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

Bug ID: 107278
   Summary: Raven: pci_pm_resume takes over 1 second
   Product: DRI
   Version: DRI git
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/AMDgpu
  Assignee: dri-devel@lists.freedesktop.org
  Reporter: pmenzel+bugs.freedesk...@molgen.mpg.de

Created attachment 140700
  --> https://bugs.freedesktop.org/attachment.cgi?id=140700=edit
Linux 4.18-rc5+ messages

Profiling the suspend and resume time [1] on a MSI B350M MORTAR (MS-7A37) with
AMD Ryzen 3 2200G with Radeon Vega Graphics with Linux 4.18-rc5+,
`pci_pm_suspend()` takes over one second, which is too long.

[   76.483494] amdgpu :38:00.0: calling pci_pm_resume+0x0/0xa0 @ 1731,
parent: :00:08.1
[   76.483499] pcieport :16:01.0: pci_pm_resume+0x0/0xa0 returned 0 after
14 usecs
[   76.483514] pci :38:00.2: calling pci_pm_resume+0x0/0xa0 @ 1672, parent:
:00:08.1
[   76.483528] reg-dummy reg-dummy: calling platform_pm_resume+0x0/0x40 @ 1653,
parent: platform
[   76.483530] pci :38:00.2: pci_pm_resume+0x0/0xa0 returned 0 after 9
usecs
[   76.483540] xhci_hcd :38:00.3: calling pci_pm_resume+0x0/0xa0 @ 1672,
parent: :00:08.1
[   76.483543] reg-dummy reg-dummy: platform_pm_resume+0x0/0x40 returned 0
after 5 usecs
[   76.483667] button PNP0C0C:00: calling acpi_button_resume+0x0/0x40 [button]
@ 1653, parent: LNXSYBUS:00
[   76.483679] button PNP0C0C:00: acpi_button_resume+0x0/0x40 [button] returned
0 after 5 usecs
[   76.483724] xhci_hcd :38:00.3: pci_pm_resume+0x0/0xa0 returned 0 after
173 usecs
[   76.483727] platform PNP0800:00: calling platform_pm_resume+0x0/0x40 @ 1653,
parent: :00:14.3
[   76.483733] xhci_hcd :38:00.4: calling pci_pm_resume+0x0/0xa0 @ 1672,
parent: :00:08.1
[   76.483739] platform PNP0800:00: platform_pm_resume+0x0/0x40 returned 0
after 5 usecs
[   76.483748] platform PNP0C0C:00: calling platform_pm_resume+0x0/0x40 @ 1653,
parent: platform
[   76.483759] platform PNP0C0C:00: platform_pm_resume+0x0/0x40 returned 0
after 4 usecs
[   76.483767] platform AMDI0030:00: calling platform_pm_resume+0x0/0x40 @
1653, parent: platform
[   76.483778] platform AMDI0030:00: platform_pm_resume+0x0/0x40 returned 0
after 4 usecs
[   76.483791] platform PNP0103:00: calling platform_pm_resume+0x0/0x40 @ 1653,
parent: platform
[   76.483802] platform PNP0103:00: platform_pm_resume+0x0/0x40 returned 0
after 4 usecs
[   76.483813] button LNXPWRBN:00: calling acpi_button_resume+0x0/0x40 [button]
@ 1653, parent: LNXSYSTM:00
[   76.483825] button LNXPWRBN:00: acpi_button_resume+0x0/0x40 [button]
returned 0 after 5 usecs
[   76.483841] system 00:00: calling pnp_bus_resume+0x0/0x90 @ 1653, parent:
pnp0
[   76.483854] system 00:00: pnp_bus_resume+0x0/0x90 returned 0 after 6 usecs
[   76.483862] system 00:01: calling pnp_bus_resume+0x0/0x90 @ 1653, parent:
pnp0
[   76.483875] system 00:01: pnp_bus_resume+0x0/0x90 returned 0 after 6 usecs
[   76.483883] rtc_cmos 00:02: calling pnp_bus_resume+0x0/0x90 @ 1653, parent:
pnp0
[   76.483934] xhci_hcd :38:00.4: pci_pm_resume+0x0/0xa0 returned 0 after
190 usecs
[   76.483943] snd_hda_intel :38:00.6: calling pci_pm_resume+0x0/0xa0 @
1672, parent: :00:08.1
[   76.484150] ahci :39:00.0: calling pci_pm_resume+0x0/0xa0 @ 1730,
parent: :00:08.2
[   76.484222] ahci :39:00.0: pci_pm_resume+0x0/0xa0 returned 0 after 66
usecs
[   76.484234] r8169 :18:00.0: calling pci_pm_resume+0x0/0xa0 @ 1729,
parent: :16:01.0
[   76.484291] rtc_cmos 00:02: pnp_bus_resume+0x0/0x90 returned 0 after 393
usecs
[   76.484296] system 00:03: calling pnp_bus_resume+0x0/0x90 @ 1653, parent:
pnp0
[   76.484302] system 00:03: pnp_bus_resume+0x0/0x90 returned 0 after 3 usecs
[   76.484307] parport_pc 00:04: calling pnp_bus_resume+0x0/0x90 @ 1653,
parent: pnp0
[   76.485185] [drm] PCIE GART of 1024M enabled (table at 0x00F40090).
[   76.485220] [drm] PSP is resuming...
[   76.486099] parport_pc 00:04: activated
[   76.486105] parport_pc 00:04: pnp_bus_resume+0x0/0x90 returned 0 after 1751
usecs
[   76.486111] serial 00:05: calling pnp_bus_resume+0x0/0x90 @ 1653, parent:
pnp0
[   76.487493] serial 00:05: activated
[   76.487975] snd_hda_intel :38:00.6: pci_pm_resume+0x0/0xa0 returned 0
after 3933 usecs
[   76.487995]  ata1: calling ata_port_pm_resume+0x0/0x50 [libata] @ 1672,
parent: :15:00.1
[   76.488016]  ata1: ata_port_pm_resume+0x0/0x50 [libata] returned 0 after 12
usecs
[   76.488025]  ata2: calling ata_port_pm_resume+0x0/0x50 [libata] @ 1672,
parent: :15:00.1
[   76.488043]  ata2: ata_port_pm_resume+0x0/0x50 [libata] returned 0 after 10
usecs
[   76.488052]  ata3: calling ata_port_pm_resume+0x0/0x50 [libata] @ 1672,
parent: :15:00.1
[   76.488070]  ata3: ata_port_pm_resume+0x0/0x50 [libata] returned 0 after 9
usecs
[   76.488079]  ata4: 

[Bug 107277] Raven: pci_pm_suspend takes over 1 second

[bugzilla-daemon]

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

Paul Menzel  changed:

   What|Removed |Added

 CC||pmenzel+bugs.freedesktop@mo
   ||lgen.mpg.de

--- Comment #1 from Paul Menzel  ---
Created attachment 140699
  --> https://bugs.freedesktop.org/attachment.cgi?id=140699=edit
HTML output of `sudo ./sleepgraph.py -config config/suspend-callgraph.cfg
-maxdepth=5`

According to the trace, most of the time is spent in the functions below.

> amdgpu_bo_evict_vram [amdgpu] (306.331 ms @ 74.694620)
> amdgpu_fence_driver_suspend [amdgpu] (0.023 ms @ 75.000953)
> amdgpu_device_ip_suspend [amdgpu] (694.390 ms @ 75.000977)
> amdgpu_bo_evict_vram [amdgpu] (24.217 ms @ 75.695369)

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 107277] Raven: pci_pm_suspend takes over 1 second

[bugzilla-daemon]

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

Bug ID: 107277
   Summary: Raven: pci_pm_suspend takes over 1 second
   Product: DRI
   Version: DRI git
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/AMDgpu
  Assignee: dri-devel@lists.freedesktop.org
  Reporter: pmenzel+bugs.freedesk...@molgen.mpg.de

Created attachment 140698
  --> https://bugs.freedesktop.org/attachment.cgi?id=140698=edit
Linux 4.18-rc5+ messages

Profiling the suspend and resume time [1] on a MSI B350M MORTAR (MS-7A37) with
AMD Ryzen 3 2200G with Radeon Vega Graphics with Linux 4.18-rc5+,
`pci_pm_suspend()` takes over one second, which is too long.

[   74.694600] amdgpu :38:00.0: calling pci_pm_suspend+0x0/0x120 @ 1664,
parent: :00:08.1
[   74.700292] sd 8:0:0:0: scsi_bus_suspend+0x0/0x20 [scsi_mod] returned 0
after 25514 usecs
[   74.700326] scsi target8:0:0: calling scsi_bus_suspend+0x0/0x20 [scsi_mod] @
175, parent: host8
[   74.700346] scsi target8:0:0: scsi_bus_suspend+0x0/0x20 [scsi_mod] returned
0 after 6 usecs
[   74.700382] scsi host8: calling scsi_bus_suspend+0x0/0x20 [scsi_mod] @ 181,
parent: ata9
[   74.700404] scsi host8: scsi_bus_suspend+0x0/0x20 [scsi_mod] returned 0
after 6 usecs
[   74.700439]  ata9: calling ata_port_pm_suspend+0x0/0x40 [libata] @ 173,
parent: :39:00.0
[   74.700522]  ata9: ata_port_pm_suspend+0x0/0x40 [libata] returned 0 after 67
usecs
[   74.700544] ahci :39:00.0: calling pci_pm_suspend+0x0/0x120 @ 7, parent:
:00:08.2
[   74.700565] ahci :39:00.0: pci_pm_suspend+0x0/0x120 returned 0 after 14
usecs
[   74.700588] pcieport :00:08.2: calling pci_pm_suspend+0x0/0x120 @ 1666,
parent: pci:00
[   74.700620] pcieport :00:08.2: pci_pm_suspend+0x0/0x120 returned 0 after
25 usecs
[   74.712063] usb 1-10: usb_dev_suspend+0x0/0x10 [usbcore] returned 0 after
36874 usecs
[   74.712092] usb usb1: calling usb_dev_suspend+0x0/0x10 [usbcore] @ 174,
parent: :15:00.0
[   74.712156] usb usb1: usb_dev_suspend+0x0/0x10 [usbcore] returned 0 after 51
usecs
[   74.712173] xhci_hcd :15:00.0: calling pci_pm_suspend+0x0/0x120 @ 1669,
parent: :00:01.2
[   74.714213] xhci_hcd :15:00.0: pci_pm_suspend+0x0/0x120 returned 0 after
1985 usecs
[   74.714233] pcieport :00:01.2: calling pci_pm_suspend+0x0/0x120 @ 1671,
parent: pci:00
[   74.714248] pcieport :00:01.2: pci_pm_suspend+0x0/0x120 returned 0 after
11 usecs
[   75.670011] amdgpu :38:00.0: 7bb4b3e1 unpin not necessary
[   75.736060] amdgpu :38:00.0: pci_pm_suspend+0x0/0x120 returned 0 after
1017036 usecs

[1]: https://github.com/01org/pm-graph/

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 5/5] drm/sun4i: Substitute sun4i_backend_format_is_yuv() with format->is_yuv

[Ayan Halder]

On Wed, Jul 18, 2018 at 01:15:40PM +0300, Ville Syrj?l? wrote:

Hi Ville,
> On Tue, Jul 17, 2018 at 06:13:46PM +0100, Ayan Kumar Halder wrote:
> > drm_format_info table has a field 'is_yuv' to denote if the format
> > is yuv or not. The driver is expected to use this instead of
> > having a function for the same purpose.
> > 
> > Signed-off-by: Ayan Kumar halder 
> > ---
> >  drivers/gpu/drm/sun4i/sun4i_backend.c | 12 +++-
> >  1 file changed, 3 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c 
> > b/drivers/gpu/drm/sun4i/sun4i_backend.c
> > index de0a76d..d7950b5 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> > @@ -86,12 +86,6 @@ static inline bool 
> > sun4i_backend_format_is_packed_yuv422(uint32_t format)
> > }
> >  }
> >  
> > -static inline bool sun4i_backend_format_is_yuv(uint32_t format)
> > -{
> > -   return sun4i_backend_format_is_planar_yuv(format) ||
> > -   sun4i_backend_format_is_packed_yuv422(format);
> > -}
> 
> sun4i_backend_format_is_planar_yuv() is now unused so could be nuked.
> Also the one remaining use of sun4i_backend_format_is_packed_yuv422()
> could be replaced with 'is_yuv && num_planes == 1', so that one could
> be easily killed off ass well.

I will have this in a separate patch.
> > -
> >  static void sun4i_backend_apply_color_correction(struct sunxi_engine 
> > *engine)
> >  {
> > int i;
> > @@ -304,7 +298,7 @@ int sun4i_backend_update_layer_formats(struct 
> > sun4i_backend *backend,
> >SUN4I_BACKEND_ATTCTL_REG0_LAY_GLBALPHA_EN,
> >val);
> >  
> > -   if (sun4i_backend_format_is_yuv(fb->format->format))
> > +   if (fb->format->is_yuv)
> > return sun4i_backend_update_yuv_format(backend, layer, plane);
> >  
> > ret = sun4i_backend_drm_format_to_layer(fb->format->format, );
> > @@ -384,7 +378,7 @@ int sun4i_backend_update_layer_buffer(struct 
> > sun4i_backend *backend,
> >  */
> > paddr -= PHYS_OFFSET;
> >  
> > -   if (sun4i_backend_format_is_yuv(fb->format->format))
> > +   if (fb->format->is_yuv)
> > return sun4i_backend_update_yuv_buffer(backend, fb, paddr);
> >  
> > /* Write the 32 lower bits of the address (in bits) */
> > @@ -502,7 +496,7 @@ static int sun4i_backend_atomic_check(struct 
> > sunxi_engine *engine,
> > if (fb->format->has_alpha || (plane_state->alpha != 
> > DRM_BLEND_ALPHA_OPAQUE))
> > num_alpha_planes++;
> >  
> > -   if (sun4i_backend_format_is_yuv(fb->format->format)) {
> > +   if (fb->format->is_yuv) {
> > DRM_DEBUG_DRIVER("Plane FB format is YUV\n");
> > num_yuv_planes++;
> > }
> > -- 
> > 2.7.4
> > 
> > ___
> > Intel-gfx mailing list
> > intel-...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrj?l?
> Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 107266] Radeon Pro Duo (Polaris) - ring sdma0 timeout

[bugzilla-daemon]

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

--- Comment #5 from robert  ---
So I stripped this down, and the ring error pops up after I've applied a new
pp_table and start utilizing the GPU. 

My guess is this error has something to do with why this doesnt work in
drm-next-4.19-wip

[2.635258] amdgpu: [powerplay] Failed to retrieve minimum clocks.
[2.635259] amdgpu: [powerplay] Error in phm_get_clock_info 

That error is not present when I load 4.17.0-rc2-180424-fkxamd kernel.

I apply same pp_table file while running 4.17.0-rc2-180424-fkxamd, and it works
as expected.

So there is something funky in the powerplay of drm-next-4.19-wip

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 200517] Vega 8/Radeon 535 hybrid graphics - amdgpu crash on modesetting

[bugzilla-daemon]

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

--- Comment #12 from Alex Deucher (alexdeuc...@gmail.com) ---
If you are using X, you need to use xrandr to setup the dGPU for render
offload.  E.g., see:
https://wiki.archlinux.org/index.php/PRIME
or similar pages.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 107266] Radeon Pro Duo (Polaris) - ring sdma0 timeout

[bugzilla-daemon]

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

--- Comment #4 from robert  ---
No, I wasnt running ROCm userland.

I've been using amdgpu-pro-18.20-606296 for several weeks with the fkxamd
kernel as recommened by Felix.

When I remove all userland, I dont see the ring sdma0 timeout.  Without any
userland,  it initializes the driver, but with a couple warnings.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 107266] Radeon Pro Duo (Polaris) - ring sdma0 timeout

[bugzilla-daemon]

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

--- Comment #3 from robert  ---
Created attachment 140697
  --> https://bugs.freedesktop.org/attachment.cgi?id=140697=edit
dmesg.amdgpupro.txt

Polaris radeon pro duo, dmesg with amdgpu-pro 18.20-606296

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 107266] Radeon Pro Duo (Polaris) - ring sdma0 timeout

[bugzilla-daemon]

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

--- Comment #2 from robert  ---
Created attachment 140696
  --> https://bugs.freedesktop.org/attachment.cgi?id=140696=edit
dmesg.base.txt

Polaris radeon pro duo, dmesg with no userland.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 105684] Loading amdgpu hits general protection fault: 0000 [#1] SMP NOPTI

[bugzilla-daemon]

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

--- Comment #32 from Paul Menzel  ---
I am also seeing this with Linux 4.18-rc5+ on a MSI MS-7A37/B350M MORTAR.

```
19.940: [0.072004] ACPI BIOS Error (bug): Failure creating [\_SB.SMIC],
AE_ALREADY_EXISTS (20180531/dswload2-316)
19.939: [0.080004] ACPI Error: AE_ALREADY_EXISTS, During name
lookup/catalog (20180531/psobject-221)
19.939: [0.092000] ACPI Error: Ignore error and continue table load
(20180531/psobject-604)
19.939: [0.096021] ACPI BIOS Error (bug): Failure creating [\_SB.SMIB],
AE_ALREADY_EXISTS (20180531/dsfield-594)
20.044: [0.205797] AMD-Vi: Unable to write to IOMMU perf counter.
20.104: [1.077147] ata9.00: failed to set xfermode (err_mask=0x40)
54.879: [   35.052403] sp5100-tco sp5100-tco: Watchdog hardware is disabled
54.924: [   35.104108] Error: Driver 'pcspkr' is already registered,
aborting...
55.061: [   35.246972] kvm: disabled by bios
55.077: [   35.265941] kfd kfd: kgd2kfd_probe failed
55.344: [   35.537445] general protection fault:  [#1] SMP NOPTI
55.344: [   35.543209] CPU: 0 PID: 367 Comm: systemd-udevd Not tainted
4.18.0-rc5+ #1
55.345: [   35.550371] Hardware name: MSI MS-7A37/B350M MORTAR (MS-7A37), BIOS
1.G1 05/17/2018
55.345: [   35.558562] RIP: 0010:prefetch_freepointer+0x10/0x20
55.346: [   35.563881] Code: 89 d3 e8 c3 fe 4a 00 85 c0 0f 85 31 75 00 00 48 83
c4 08 5b 5d 41 5c 41 5d c3 0f 1f 44 00 00 48 85 f6 74 13 8b 47 20 48 01 c6 <48>
33 36 48 33 b7 38 01 00 00 0f 18 0e c3 66 90 0f 1f 44 00 00 48 
55.347: [   35.584215] RSP: 0018:9c3181f77560 EFLAGS: 00010202
55.347: [   35.589849] RAX:  RBX: 4b2c8be0a60f6ab9 RCX:
0ccc
55.348: [   35.597492] RDX: 0ccb RSI: 4b2c8be0a60f6ab9 RDI:
8d5b1e406e80
55.349: [   35.605166] RBP: 8d5b1e406e80 R08: 8d5b1e824f00 R09:
c0a45423
55.349: [   35.612808] R10: f1b60fff64c0 R11: 9c3181f77520 R12:
006080c0
55.350: [   35.620451] R13: 0230 R14: 8d5b1e406e80 R15:
8d5b0c304400
55.350: [   35.628116] FS:  7fb194e4b8c0() GS:8d5b1e80()
knlGS:
55.351: [   35.636771] CS:  0010 DS:  ES:  CR0: 80050033
55.351: [   35.642931] CR2: 55c0661d60d0 CR3: 00040b9ee000 CR4:
003406f0
55.352: [   35.650566] Call Trace:
55.352: [   35.653198]  kmem_cache_alloc_trace+0xb5/0x1c0
55.352: [   35.658077]  ? dal_ddc_service_create+0x38/0x110 [amdgpu]
55.353: [   35.663962]  dal_ddc_service_create+0x38/0x110 [amdgpu]
55.353: [   35.669656]  ? dal_gpio_create_irq+0x19/0x30 [amdgpu]
55.354: [   35.675155]  ? dal_gpio_service_create_irq+0x48/0x70 [amdgpu]
55.354: [   35.681413]  ? get_hpd_gpio+0x63/0x90 [amdgpu]
55.355: [   35.686281]  construct+0x201/0x640 [amdgpu]
55.355: [   35.690838]  link_create+0x33/0x50 [amdgpu]
55.356: [   35.695400]  dc_create+0x2d3/0x640 [amdgpu]
55.356: [   35.699980]  dm_hw_init+0xc8/0x130 [amdgpu]
55.356: [   35.704568]  amdgpu_device_init.cold.28+0x1043/0x11ee [amdgpu]
55.357: [   35.710923]  amdgpu_driver_load_kms+0x86/0x2c0 [amdgpu]
55.357: [   35.716535]  drm_dev_register+0x109/0x140 [drm]
55.358: [   35.721468]  amdgpu_pci_probe+0x13c/0x1c0 [amdgpu]
55.358: [   35.726612]  local_pci_probe+0x41/0x90
55.358: [   35.730628]  pci_device_probe+0x189/0x1a0
55.359: [   35.734952]  driver_probe_device+0x2b9/0x460
55.359: [   35.739544]  __driver_attach+0xdd/0x110
55.359: [   35.743633]  ? driver_probe_device+0x460/0x460
55.360: [   35.748383]  bus_for_each_dev+0x76/0xc0
55.360: [   35.752480]  ? klist_add_tail+0x3b/0x70
55.360: [   35.756574]  bus_add_driver+0x152/0x230
55.361: [   35.760692]  ? 0xc0c37000
55.361: [   35.764255]  driver_register+0x6b/0xb0
55.361: [   35.768265]  ? 0xc0c37000
55.361: [   35.771814]  do_one_initcall+0x46/0x1c3
55.362: [   35.775944]  ? _cond_resched+0x15/0x30
55.362: [   35.779940]  ? kmem_cache_alloc_trace+0x15c/0x1c0
55.362: [   35.784982]  ? do_init_module+0x22/0x210
55.362: [   35.789172]  do_init_module+0x5a/0x210
55.363: [   35.793227]  load_module+0x2124/0x2500
55.363: [   35.797250]  ? vfs_read+0x110/0x140
55.363: [   35.800985]  ? __do_sys_finit_module+0xa8/0x110
55.364: [   35.805837]  __do_sys_finit_module+0xa8/0x110
55.364: [   35.810516]  do_syscall_64+0x55/0xe0
55.364: [   35.814358]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
55.365: [   35.819768] RIP: 0033:0x7fb195ea8a79
55.365: [   35.823607] Code: 00 f3 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00
48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48>
3d 01 f0 ff ff 73 01 c3 48 8b 0d df 43 0c 00 f7 d8 64 89 01 48 
55.366: [   35.843819] RSP: 002b:7ffec10a9518 EFLAGS: 0246 ORIG_RAX:
0139
55.367: [   35.851960] RAX: ffda RBX: 5561d3488730 RCX:
7fb195ea8a79
55.367: [   35.859595] RDX:  RSI: 7fb195bb00ed RDI:
0013
55.368: [   35.867206] RBP: 7fb195bb00ed R08:  R09:

Re: [PATCH] backlight: pwm_bl: Fix uninitialized variable

[Daniel Thompson]

On Wed, Jul 18, 2018 at 02:08:53PM +0100, Lee Jones wrote:
> > > > > No, then we are back to the initial issue of num_steps
> > > > > potentially not
> > > > > being initialised. We really want both of_property_read_u32() to
> > > > > succeed AND num_steps to actually be set.
> > > > 
> > > > I also think num_steps should be pre-initialised.
> > 
> > Yes, I guess it definitely does not hurt.
> > 
> > > > Then it will only be set if of_property_read_u32() succeeds.
> > 
> > Yes, but we still need to check for both, the function not failing and
> > num_steps to actually be non zero.
> 
> Why?  You don't do anything differently if it fails.

Only if you initialize num_steps...

We should either initialize to zero and not worry about the return
code[1] or we check the return code and not worry about
initialization[2]. I don't think both are worthwhile.

Whilst initialization can fix this specific instance we generally avoid
overusing it since it messes up static analysis and, in this instance,
distance from declaration to use is >25 lines, hence current patchset.


Daniel.


[1] https://lkml.org/lkml/2018/7/16/399
[2] https://lkml.org/lkml/2018/7/16/1042

Or...

We check the return code and leave number

num_steps is uninitialized and stack allocated so it only has a valid
value if of_property_read_u32() succeeds.

We can (and I originally did) fix the bug by initializing num_steps to 0
but its quite some distance between declaration and use so I accepted
Marcel's counter proposal to check the return code instead.


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

Re: [PATCH] drm/panel: simple: Support simple VGA panels

[Liviu Dudau]

On Tue, Jul 17, 2018 at 08:02:46AM -0600, Rob Herring wrote:
> On Tue, Jul 17, 2018 at 1:47 AM Linus Walleij  
> wrote:
> >
> > On Tue, Jul 17, 2018 at 12:50 AM Rob Herring  wrote:
> > > On Mon, Jul 16, 2018 at 3:23 AM Linus Walleij  
> > > wrote:
> >
> > > > +Video Graphics Array
> > >
> > > VGA is more a controller interface than a panel...
> >
> > I don't know what else to call it though, other than formulating someting
> > bureaucratic like
> > "Video Graphics Array Display Resolutions"
> >
> > Or should I use the abbreviation:
> > "VGA Display Resolutions" rather?
> >
> > The Wikipedia article "display resolution"
> > https://en.wikipedia.org/wiki/Display_resolution
> > just calls these three resolutions "VGA", "SVGA " and "XGA".
> >
> > > > +static const struct drm_display_mode video_graphics_array_modes[] = {
> > > > +   {
> > > > +   /*
> > > > +* This is the most standardized "vanilla" VGA mode 
> > > > there is:
> > > > +* 640x480 @ 60 Hz
> > >
> > > Don't we have standard VESA timings already defined as well as timings
> > > that can be calculated based on resolution and refresh rate (called
> > > CVT IIRC). Why duplicate here?
> >
> > They are inside drivers/gpu/drm/drm_edid.c
> > all in static const arrays and enumerated by the index in the
> > DMT spec taken out of XFree86. (drm_dmt_modes[])
> >
> > I don't know. It is quite encapsulated into that EDID driver and
> > doesn't seem very reusable. To pick out a few from inside EDID
> > seems pretty daunting. I'd like to hear what the DRM folks think
> > about that.
> >
> > > Why don't you just define a 'vga-connector' node
> >
> > Because this is not a VGA connector, it is just the VGA resolution.
> > In other circumstances I do use it.
> 
> It's not a panel either. A connector is something with variable
> resolution. A panel is fixed resolution. Which do you want?
> 
> > I think you most often have to use that connector with the dumb
> > VGA DAC bridge though, as the VGA connector is analog and
> > what comes out of a display controller is digital. So it needs to
> > make some kind of sense here.
> >
> > In a way (as we discussed before) the whole panel-simple.c thing
> > is a bit bogus, as it is probably in most cases hiding a bridge or
> > a dumb DAC or at least a VGA connector or similar that should've
> > been properly modeled, but in this case I am more certain that it is
> > actually the right choice.
> >
> > I guess I could try to use the dumb VGA bridge and the VGA
> > connector, and it indeed falls back to VGA resolutions if no DDC
> > channel is available but if I go in and say there is a DAC bridge
> > and VGA connector I am essentially lying.
> >
> > > and IIRC, you can
> > > just force the standard modes from userspace with DRM.
> >
> > I guess you mean the kernel command line arguments, lest
> > there will be no boot console.
> 
> Yes, the kernel command line is another way. But if you aren't using
> fbcon, then userspace is the way.
> 
> > Apart from being a user experience horror story, that requires
> > a VGA connector bridge, as per above. (It's the EDID code that
> > does that command line parsing.) And that requires lying about
> > having a VGA connector bridge.
> 
> Because you have to set the resolution on the kernel command line? I'm
> open to having a default resolution for the connector in DT.
> 
> > But I can surely lie if everyone thinks that is the best idea :D
> >
> > > Maybe you need
> > > some flag to force a connection in the emulator and perhaps some fake
> > > EDID data to set a default mode.
> >
> > This device tree I'm creating is for ARM RTSM which is a proprietary
> > ARM tool.
> >
> > Sudeep at ARM says it does not emulate any DAC bridge such
> > as found in the Versatile Express machine family. It just expects
> > to have the right resolution parameters written into the registers
> > of the PL111, which in turn of course can only get it from a
> > panel or bridge driver of some sort.
> 
> So putting *anything* in DT is a lie...

A long time ago I've tried to sorted this problem by creating a virtual
DRM encoder/connector that read the OF data to provide the EDID info.
Linaro has adopted the patches for a while, but it has never made it
into mainline because there was not enough interest for this "fake"
encoder/connector.

If interested in this solution I can try to revive the patches and make
another attempt at upstreaming them

https://git.linaro.org/landing-teams/working/arm/kernel-release.git/commit/?h=latest-armlt=9a93df172b2987c5a8e0efcefb2abdfebd1a3d68
https://git.linaro.org/landing-teams/working/arm/kernel-release.git/commit/?h=latest-armlt=15283f7be4b1e586702551e85b4caf06531ac2fc

Best regards,
Liviu

> 
> > The way those emulators work (AFAICT) is that they simply monitor
> > register writes to the resolutions parameters to scale the emulator
> > display window and then they read out the RGB data into that
> > window, pixel by pixel, from the 

Re: [PATCH] backlight: pwm_bl: Fix uninitialized variable

[Lee Jones]

On Wed, 18 Jul 2018, Marcel Ziswiler wrote:

> On Wed, 2018-07-18 at 11:12 +0100, Daniel Thompson wrote:
> > On Wed, Jul 18, 2018 at 10:53:35AM +0100, Lee Jones wrote:
> > > On Wed, 18 Jul 2018, Marcel Ziswiler wrote:
> > > 
> > > > On Wed, 2018-07-18 at 09:09 +0100, Lee Jones wrote:
> > > > > On Mon, 16 Jul 2018, Daniel Thompson wrote:
> > > > > 
> > > > > > Currently, if the DT does not define num-interpolated-steps
> > > > > > then
> > > > > > num_steps is undefined and the interpolation code will deploy
> > > > > > randomly.
> > > > > > Fix this.
> > > > > > 
> > > > > > Fixes: 573fe6d1c25c ("backlight: pwm_bl: Linear interpolation
> > > > > > between
> > > > > > brightness-levels")
> > > > > > Reported-by: Marcel Ziswiler 
> > > > > > Signed-off-by: Daniel Thompson 
> > > > > > Signed-off-by: Marcel Ziswiler 
> > > > > 
> > > > > This line is confusing.  Did you guys author this patch
> > > > > together?
> > > > 
> > > > Yes, I reported it and we came to a conclusion together.
> > > 
> > > It sounds like you need to have all of the tags (except this one).
> > > :)
> > > 
> > >  Reported-by:  for reporting the issue
> > >  Suggested-by: for suggesting a resolution
> > >  Acked-by: for reviewing it
> > >  Tested-by:for testing it
> > > 
> > > Signed-off-by usually means you either wrote a significant amount
> > > of
> > > the diffstat or you were part of the submission path.
> > 
> > He did [I don't object to but wouldn't have used the extra brackets
> > you
> > brought up ;-) ].
> 
> Yes, I take all the blame for the extra brackets. Regardless of having
> a masters in CS or not I still prefer too many then too few of them (;-
> p).
> 
> > > > > My guess is that this line should be dropped and the RB and TB
> > > > > tags
> > > > > should remain?  If it was reviewed too, perhaps an AB too?
> > > > 
> > > > I'm OK either way and do not need any explicit authorship to be
> > > > expressed for me.
> > > 
> > > In this instance I suggest keeping Reported-by and Tested-by.
> > > 
> > > > > > Tested-by: Marcel Ziswiler 
> > > > > > ---
> > > > > >  drivers/video/backlight/pwm_bl.c | 17 -
> > > > > >  1 file changed, 8 insertions(+), 9 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/video/backlight/pwm_bl.c
> > > > > > b/drivers/video/backlight/pwm_bl.c
> > > > > > index 9ee4c1b735b2..e3c22b79fbcd 100644
> > > > > > --- a/drivers/video/backlight/pwm_bl.c
> > > > > > +++ b/drivers/video/backlight/pwm_bl.c
> > > > > > @@ -299,15 +299,14 @@ static int
> > > > > > pwm_backlight_parse_dt(struct
> > > > > > device *dev,
> > > > > >  * interpolation between each of the values
> > > > > > of
> > > > > > brightness levels
> > > > > >  * and creates a new pre-computed table.
> > > > > >  */
> > > > > > -   of_property_read_u32(node, "num-
> > > > > > interpolated-
> > > > > > steps",
> > > > > > -_steps);
> > > > > > -
> > > > > > -   /*
> > > > > > -* Make sure that there is at least two
> > > > > > entries in
> > > > > > the
> > > > > > -* brightness-levels table, otherwise we
> > > > > > can't
> > > > > > interpolate
> > > > > > -* between two points.
> > > > > > -*/
> > > > > > -   if (num_steps) {
> > > > > > +   if ((of_property_read_u32(node, "num-
> > > > > > interpolated-
> > > > > > steps",
> > > > > > + _steps) == 0)
> > > > > > &&
> > > > > > num_steps) {
> > > > > 
> > > > > This is pretty ugly, and isn't it suffering from over-
> > > > > bracketing?  My
> > > > > suggestion would be to break out the invocation of
> > > > > of_property_read_u32() from the if and test only the result.
> > > > > 
> > > > >   of_property_read_u32(node, "num-interpolated-
> > > > > steps", 
> > > > > _steps);
> > > > 
> > > > you mean:
> > > > 
> > > > ret = of_property_read_u32(node, "num-interpolated-
> > > > steps", _steps);
> > > > 
> > > > >   if (!ret && num_steps) {
> > > > > 
> > > > > I haven't checked the underling code, but is it even feasible
> > > > > for
> > > > > of_property_read_u32() to not succeed AND for num_steps to be
> > > > > set?
> > > > > 
> > > > > If not, the check for !ret if superfluous and you can drop it.
> > > > 
> > > > No, then we are back to the initial issue of num_steps
> > > > potentially not
> > > > being initialised. We really want both of_property_read_u32() to
> > > > succeed AND num_steps to actually be set.
> > > 
> > > I also think num_steps should be pre-initialised.
> 
> Yes, I guess it definitely does not hurt.
> 
> > > Then it will only be set if of_property_read_u32() succeeds.
> 
> Yes, but we still need to check for both, the function not failing and
> num_steps to actually be non zero.

Why?  You don't do anything differently if it fails.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source 

Re: [PATCH v3 00/18] arm64: allwinner: Add A64 DE2 HDMI support

[Maxime Ripard]

On Wed, Jul 18, 2018 at 04:24:40PM +0530, Jagan Teki wrote:
> Allwinner A64 has display engine pipeline like other Allwinner SOC's 
> A83T/H3/H5.
> 
> A64 behaviour similar to Allwinner A83T where
> Mixer0 => TCON0 => LVDS/RGB/MIPI-DSI
> Mixer1 => TCON1 => HDMI
> as per Display System Block Diagram from Allwinner_A64_User_Manual_V1.1.pdf
> 
> This is third patch-set followed with previous RFC[1], first and second
> series[2][3] and merely concentrated on HDMI pipeline through TCON1 and
> rest will add eventually.
> 
> This series fixed previous version comments
> - Rebasing on linux-next
> - sqash all pipeline components in one patch
> - Enable all pipeline components in board dts
> - about documenting fallback compatibles
> - adding new compatible for mixer1

You still haven't figured out the SRAM parts. We asked you to fix this
in the v1 and the v2 already, and we're not going to merge this
without those bits figured out properly.

> Log:
> [1.450984] Jagan: sun8i_mixer_probe
> [1.464981] sun4i-drm display-engine: bound 120.mixer (ops 
> sun8i_mixer_ops)
> [1.472572] sun4i-drm display-engine: bound 1c0d000.lcd-controller (ops 
> sun4i_tcon_ops)
> [1.480676] sun8i-dw-hdmi 1ee.hdmi: Linked as a consumer to 
> regulator.10
> [1.488738] sun8i-dw-hdmi 1ee.hdmi: Detected HDMI TX controller v1.32a 
> with HDCP (sun8i_dw_hdmi_phy)
> [1.498879] sun8i-dw-hdmi 1ee.hdmi: registered DesignWare HDMI I2C bus 
> driver
> [1.507372] sun4i-drm display-engine: bound 1ee.hdmi (ops 
> sun8i_dw_hdmi_ops)
> [1.514778] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> [1.521398] [drm] No driver support for vblank timestamp query.
> [1.684611] random: fast init done
> [2.011575] Console: switching to colour frame buffer device 180x56
> [2.049858] sun4i-drm display-engine: fb0: DRM emulated frame buffer device
> [2.057268] [drm] Initialized sun4i-drm 1.0.0 20150629 for display-engine 
> on minor 0
> 
> Note:
> Pine64 boards are unable to get edid by default like other A64 boards,
> but forcing 'video=HDMI-A-1:1920x1080@60D' kernel command line can
> create edid with display on penel

Then that needs to be figured out before it's enabled, instead of
having a hack that barely works.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 104790] When using OpenGL 4.x some applications crash

[bugzilla-daemon]

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

--- Comment #6 from lei.p...@gmail.com ---
It doesn't matter how I enabled it, results are the same.

However, I tested now, with Mesa 18.1.4 and it does not crash applications that
were crashing before, everything seems to work fine with any OpenGL CORE
profile from 4.1 to 4.5 (did not test earlier or latter versions). COMPAT
profile still has artifacts but that's to be expected I guess.

So, it seems that problem is resolved, have to test a bit more, if anything
goes wrong I will post.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 107273] [Intel GFX CI] amdgpu 0000:01:00.0: HDMI-A-1: EDID is invalid

[bugzilla-daemon]

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

Bug ID: 107273
   Summary: [Intel GFX CI] amdgpu :01:00.0: HDMI-A-1: EDID is
invalid
   Product: DRI
   Version: XOrg git
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/AMDgpu
  Assignee: dri-devel@lists.freedesktop.org
  Reporter: martin.pe...@free.fr

When suspending a KBL-g NUC, we got the following EDID read failure:

[  200.686086] amdgpu :01:00.0: HDMI-A-1: EDID is invalid:
[  200.686088]  [00] BAD  00 ff ff ff ff ff ff 00 05 e3 cd 0c 00 00 00 00
[  200.686089]  [00] BAD  03 1b 01 03 80 3e 22 78 ea 1e c5 ae 4f 34 b1 26
[  200.686090]  [00] BAD  01 50 54 2f cf 00 d1 cf b3 00 a9 c0 95 00 81 81
[  200.686091]  [00] BAD  81 00 81 c0 d1 00 02 3a 80 18 71 38 2d 40 58 2c
[  200.686092]  [00] BAD  35 00 e0 0e 11 00 00 1a e2 68 00 a0 a0 40 2e 60
[  200.686092]  [00] BAD  30 20 36 00 80 90 21 00 00 1a 56 5e 00 a0 a0 a0
[  200.686093]  [00] BAD  29 50 30 20 36 00 80 68 21 00 00 1a 00 00 00 fc
[  200.686094]  [00] BAD  00 32 38 45 38 35 30 0a 20 20 20 20 20 20 01 b2
[  200.686117] [drm:dc_link_detect [amdgpu]] *ERROR* No EDID read.

https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_4430/fi-kbl-8809g/igt@gem_exec_susp...@basic-s3.html

This seems to have happened only once, but let's see if we can reproduce this
issue.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 00/18] arm64: allwinner: Add A64 DE2 HDMI support

[Chen-Yu Tsai]

On Wed, Jul 18, 2018 at 6:54 PM, Jagan Teki  wrote:
> Allwinner A64 has display engine pipeline like other Allwinner SOC's 
> A83T/H3/H5.
>
> A64 behaviour similar to Allwinner A83T where
> Mixer0 => TCON0 => LVDS/RGB/MIPI-DSI
> Mixer1 => TCON1 => HDMI
> as per Display System Block Diagram from Allwinner_A64_User_Manual_V1.1.pdf
>
> This is third patch-set followed with previous RFC[1], first and second
> series[2][3] and merely concentrated on HDMI pipeline through TCON1 and
> rest will add eventually.
>
> This series fixed previous version comments
> - Rebasing on linux-next
> - sqash all pipeline components in one patch
> - Enable all pipeline components in board dts
> - about documenting fallback compatibles
> - adding new compatible for mixer1
>
> Log:
> [1.450984] Jagan: sun8i_mixer_probe
> [1.464981] sun4i-drm display-engine: bound 120.mixer (ops 
> sun8i_mixer_ops)
> [1.472572] sun4i-drm display-engine: bound 1c0d000.lcd-controller (ops 
> sun4i_tcon_ops)
> [1.480676] sun8i-dw-hdmi 1ee.hdmi: Linked as a consumer to 
> regulator.10
> [1.488738] sun8i-dw-hdmi 1ee.hdmi: Detected HDMI TX controller v1.32a 
> with HDCP (sun8i_dw_hdmi_phy)
> [1.498879] sun8i-dw-hdmi 1ee.hdmi: registered DesignWare HDMI I2C bus 
> driver
> [1.507372] sun4i-drm display-engine: bound 1ee.hdmi (ops 
> sun8i_dw_hdmi_ops)
> [1.514778] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> [1.521398] [drm] No driver support for vblank timestamp query.
> [1.684611] random: fast init done
> [2.011575] Console: switching to colour frame buffer device 180x56
> [2.049858] sun4i-drm display-engine: fb0: DRM emulated frame buffer device
> [2.057268] [drm] Initialized sun4i-drm 1.0.0 20150629 for display-engine 
> on minor 0
>
> Note:
> Pine64 boards are unable to get edid by default like other A64 boards,
> but forcing 'video=HDMI-A-1:1920x1080@60D' kernel command line can
> create edid with display on penel
>
> [3] https://lkml.org/lkml/2018/5/18/461
> [2] https://lkml.org/lkml/2018/4/30/288
> [1] https://lkml.org/lkml/2018/4/24/547
>
> Icenowy Zheng (1):
>   drm: sun4i: add support for HVCC regulator for DWC HDMI glue
>
> Jagan Teki (17):
>   clk: sunxi-ng: a64: Add minimal rate for video PLLs
>   drm/sun4i: Add support for A64 mixer1
>   dt-bindings: display: Add compatible for A64 DE2 tcon1 blocks
>   drm/sun4i: Add support for A64 display engine
>   dt-bindings: display: Add compatible for A64 HDMI
>   dt-bindings: clock: sun50i-a64-ccu: Add PLL_VIDEO[0-1] macros
>   arm64: dts: allwinner: a64: Add tcon1 HDMI pipeline
>   clk: sunxi-ng: Enable DE2_CCU for SUN8I and SUN50I
>   arm64: defconfig: Enable CONFIG_DRM_SUN4I
>   drm/sun4i: Enable DE2 Mixer for SUN8I and SUN50I
>   drm/sun4i: Enable DesignWare HDMI for SUN50I

>   arm64: dts: allwinner: a64: bananapi-m64: Enable HDMI output
>   arm64: dts: allwinner: a64: nanopi-a64: Enable HDMI output
>   arm64: dts: allwinner: a64: orangepi-win: Enable HDMI output
>   arm64: dts: allwinner: a64: a64-olinuxino: Enable HDMI output
>   arm64: dts: allwinner: a64: pine64: Enable HDMI output
>   arm64: dts: allwinner: a64: sopine: Enable HDMI output

Please squash all the patches enabling HDMI on some board into one,
and write an informative commit message. See commit 8b1447aed5f4
("ARM: dts: sun6i: Enable HDMI support on some A31/A31s devices")
for such an example.

ChenYu

>
>  .../bindings/display/sunxi/sun4i-drm.txt  |   4 +
>  .../dts/allwinner/sun50i-a64-bananapi-m64.dts |  34 ++
>  .../dts/allwinner/sun50i-a64-nanopi-a64.dts   |  34 ++
>  .../dts/allwinner/sun50i-a64-olinuxino.dts|  34 ++
>  .../dts/allwinner/sun50i-a64-orangepi-win.dts |  34 ++
>  .../boot/dts/allwinner/sun50i-a64-pine64.dts  |  34 ++
>  .../allwinner/sun50i-a64-sopine-baseboard.dts |  34 ++
>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 110 ++
>  arch/arm64/configs/defconfig  |   1 +
>  drivers/clk/sunxi-ng/Kconfig  |   2 +
>  drivers/clk/sunxi-ng/ccu-sun50i-a64.c |  46 
>  drivers/gpu/drm/sun4i/Kconfig |   3 +-
>  drivers/gpu/drm/sun4i/sun4i_drv.c |   1 +
>  drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c |  14 +++
>  drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h |   2 +
>  drivers/gpu/drm/sun4i/sun8i_mixer.c   |  12 ++
>  include/dt-bindings/clock/sun50i-a64-ccu.h|   2 +
>  17 files changed, 378 insertions(+), 23 deletions(-)
>
> --
> 2.17.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/amdgpu/pm: Remove VLA usage

[Zhu, Rex]

Patch has been applied to our internal amd-staging-drm-next tree.


Thanks.


Best Regards

Rex


From: Christian K?nig 
Sent: Tuesday, July 17, 2018 2:58:21 PM
To: Zhu, Rex; Kees Cook; Deucher, Alexander
Cc: Zhou, David(ChunMing); David Airlie; Kuehling, Felix; LKML; amd-gfx list; 
Huang, Ray; Maling list - DRI developers; Koenig, Christian
Subject: Re: [PATCH] drm/amdgpu/pm: Remove VLA usage

Who's tree should this go through?
To answer the question: When Rex is ok with that he pushes it to our internal 
amd-staging-drm-next tree.

Alex then pushes that tree to a public server and at some point sends a pull 
request for inclusion in drm-next.

Regards,
Christian.

Am 17.07.2018 um 08:23 schrieb Zhu, Rex:
Patch is:
Reviewed-by: Rex Zhumailto:re...@amd.com>>



Best Regards
Rex



From: keesc...@google.com 
 on behalf of Kees Cook 

Sent: Tuesday, July 17, 2018 11:59 AM
To: Deucher, Alexander
Cc: LKML; Koenig, Christian; Zhou, David(ChunMing); David Airlie; Zhu, Rex; 
Huang, Ray; Kuehling, Felix; amd-gfx list; Maling list - DRI developers
Subject: Re: [PATCH] drm/amdgpu/pm: Remove VLA usage

On Wed, Jun 20, 2018 at 11:26 AM, Kees Cook 
 wrote:
> In the quest to remove all stack VLA usage from the kernel[1], this
> uses the maximum sane buffer size and removes copy/paste code.
>
> [1] 
> https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qpxydaacu1rq...@mail.gmail.com
>
> Signed-off-by: Kees Cook 

Friendly ping! Who's tree should this go through?

Thanks!

-Kees

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 100 +++--
>  1 file changed, 42 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> index b455da487782..5eb98cde22ed 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> @@ -593,40 +593,59 @@ static ssize_t amdgpu_get_pp_dpm_sclk(struct device 
> *dev,
> return snprintf(buf, PAGE_SIZE, "\n");
>  }
>
> -static ssize_t amdgpu_set_pp_dpm_sclk(struct device *dev,
> -   struct device_attribute *attr,
> -   const char *buf,
> -   size_t count)
> +/*
> + * Worst case: 32 bits individually specified, in octal at 12 characters
> + * per line (+1 for \n).
> + */
> +#define AMDGPU_MASK_BUF_MAX(32 * 13)
> +
> +static ssize_t amdgpu_read_mask(const char *buf, size_t count, uint32_t 
> *mask)
>  {
> -   struct drm_device *ddev = dev_get_drvdata(dev);
> -   struct amdgpu_device *adev = ddev->dev_private;
> int ret;
> long level;
> -   uint32_t mask = 0;
> char *sub_str = NULL;
> char *tmp;
> -   char buf_cpy[count];
> +   char buf_cpy[AMDGPU_MASK_BUF_MAX + 1];
> const char delimiter[3] = {' ', '\n', '\0'};
> +   size_t bytes;
>
> -   memcpy(buf_cpy, buf, count+1);
> +   *mask = 0;
> +
> +   bytes = min(count, sizeof(buf_cpy) - 1);
> +   memcpy(buf_cpy, buf, bytes);
> +   buf_cpy[bytes] = '\0';
> tmp = buf_cpy;
> while (tmp[0]) {
> -   sub_str =  strsep(, delimiter);
> +   sub_str = strsep(, delimiter);
> if (strlen(sub_str)) {
> ret = kstrtol(sub_str, 0, );
> -
> -   if (ret) {
> -   count = -EINVAL;
> -   goto fail;
> -   }
> -   mask |= 1 << level;
> +   if (ret)
> +   return -EINVAL;
> +   *mask |= 1 << level;
> } else
> break;
> }
> +
> +   return 0;
> +}
> +
> +static ssize_t amdgpu_set_pp_dpm_sclk(struct device *dev,
> +   struct device_attribute *attr,
> +   const char *buf,
> +   size_t count)
> +{
> +   struct drm_device *ddev = dev_get_drvdata(dev);
> +   struct amdgpu_device *adev = ddev->dev_private;
> +   int ret;
> +   uint32_t mask = 0;
> +
> +   ret = amdgpu_read_mask(buf, count, );
> +   if (ret)
> +   return ret;
> +
> if (adev->powerplay.pp_funcs->force_clock_level)
> amdgpu_dpm_force_clock_level(adev, PP_SCLK, mask);
>
> -fail:
> return count;
>  }
>
> @@ -651,32 +670,15 @@ static ssize_t amdgpu_set_pp_dpm_mclk(struct device 
> *dev,
> struct drm_device *ddev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = ddev->dev_private;
> int ret;
> -   long level;
> uint32_t mask = 0;
> -   char *sub_str = NULL;
> -   char *tmp;
> -   char buf_cpy[count];
> -   const char delimiter[3] = {' ', '\n', '\0'};
>
> -   

Re: [PATCH 1/5] drm/fourcc: Add is_yuv field to drm_format_info to denote if the format is yuv

[Ville Syrjälä]

On Wed, Jul 18, 2018 at 10:12:02AM +0100, Brian Starkey wrote:
> Hi Ayan,
> 
> On Tue, Jul 17, 2018 at 06:13:42PM +0100, Ayan Kumar Halder wrote:
> >A lot of drivers duplicate the function to check if a format is yuv or not.
> >If we add a field (to denote whether the format is yuv or not) in the
> >drm_format_info table, all the drivers can use this field and it will
> >prevent duplication of similar logic.
> 
> This looks like a good idea to me.
> 
> I wonder if the two "has_alpha" and "is_yuv" bools should be bitfields
> to reduce the footprint (not sure what the general DRM attitude to
> bitfields is), but either way:

There are quite a few of these so tighter packing (of the other fields
as well perhaps) might be a decent idea.

For the series (except for the omapdrm patch which I'll leave for Tomi):
Reviewed-by: Ville Syrjälä 

> 
> Reviewed-by: Brian Starkey 
> 
> >
> >Signed-off-by: Ayan Kumar halder 
> >---
> > drivers/gpu/drm/drm_fourcc.c | 42 +-
> > include/drm/drm_fourcc.h |  2 ++
> > 2 files changed, 23 insertions(+), 21 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> >index 5ca6395..35c1e27 100644
> >--- a/drivers/gpu/drm/drm_fourcc.c
> >+++ b/drivers/gpu/drm/drm_fourcc.c
> >@@ -152,27 +152,27 @@ const struct drm_format_info *__drm_format_info(u32 
> >format)
> > { .format = DRM_FORMAT_XBGR_A8, .depth = 32, 
> > .num_planes = 2, .cpp = { 4, 1, 0 }, .hsub = 1, .vsub = 1, .has_alpha = 
> > true },
> > { .format = DRM_FORMAT_RGBX_A8, .depth = 32, 
> > .num_planes = 2, .cpp = { 4, 1, 0 }, .hsub = 1, .vsub = 1, .has_alpha = 
> > true },
> > { .format = DRM_FORMAT_BGRX_A8, .depth = 32, 
> > .num_planes = 2, .cpp = { 4, 1, 0 }, .hsub = 1, .vsub = 1, .has_alpha = 
> > true },
> >-{ .format = DRM_FORMAT_YUV410,  .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 4 },
> >-{ .format = DRM_FORMAT_YVU410,  .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 4 },
> >-{ .format = DRM_FORMAT_YUV411,  .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 1 },
> >-{ .format = DRM_FORMAT_YVU411,  .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 1 },
> >-{ .format = DRM_FORMAT_YUV420,  .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 2 },
> >-{ .format = DRM_FORMAT_YVU420,  .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 2 },
> >-{ .format = DRM_FORMAT_YUV422,  .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 1 },
> >-{ .format = DRM_FORMAT_YVU422,  .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 1 },
> >-{ .format = DRM_FORMAT_YUV444,  .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 1, .vsub = 1 },
> >-{ .format = DRM_FORMAT_YVU444,  .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 1, .vsub = 1 },
> >-{ .format = DRM_FORMAT_NV12,.depth = 0,  
> >.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2 },
> >-{ .format = DRM_FORMAT_NV21,.depth = 0,  
> >.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2 },
> >-{ .format = DRM_FORMAT_NV16,.depth = 0,  
> >.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 1 },
> >-{ .format = DRM_FORMAT_NV61,.depth = 0,  
> >.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 1 },
> >-{ .format = DRM_FORMAT_NV24,.depth = 0,  
> >.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1 },
> >-{ .format = DRM_FORMAT_NV42,.depth = 0,  
> >.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1 },
> >-{ .format = DRM_FORMAT_YUYV,.depth = 0,  
> >.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> >-{ .format = DRM_FORMAT_YVYU,.depth = 0,  
> >.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> >-{ .format = DRM_FORMAT_UYVY,.depth = 0,  
> >.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> >-{ .format = DRM_FORMAT_VYUY,.depth = 0,  
> >.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> >-{ .format = DRM_FORMAT_AYUV,.depth = 0,  
> >.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true 
> >},
> >+{ .format = DRM_FORMAT_YUV410,  .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 4, .is_yuv = true },
> >+{ .format = DRM_FORMAT_YVU410,  .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 4, .is_yuv = true },

Re: [Intel-gfx] [PATCH 4/5] drm/omapdrm: Substitute format_is_yuv() with format->is_yuv

[Ville Syrjälä]

On Tue, Jul 17, 2018 at 06:13:45PM +0100, Ayan Kumar Halder wrote:
> drm_format_info table has a field 'is_yuv' to denote if the format
> is yuv or not. The driver is expected to use this instead of
> having a function for the same purpose.
> 
> Signed-off-by: Ayan Kumar halder 
> ---
>  drivers/gpu/drm/omapdrm/dss/dispc.c | 26 ++
>  1 file changed, 10 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c 
> b/drivers/gpu/drm/omapdrm/dss/dispc.c
> index 84f274c..8d2d7a4 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dispc.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
> @@ -1140,18 +1140,6 @@ static void dispc_ovl_set_color_mode(struct 
> dispc_device *dispc,
>   REG_FLD_MOD(dispc, DISPC_OVL_ATTRIBUTES(plane), m, 4, 1);
>  }
>  
> -static bool format_is_yuv(u32 fourcc)
> -{
> - switch (fourcc) {
> - case DRM_FORMAT_YUYV:
> - case DRM_FORMAT_UYVY:
> - case DRM_FORMAT_NV12:
> - return true;
> - default:
> - return false;
> - }
> -}
> -
>  static void dispc_ovl_configure_burst_type(struct dispc_device *dispc,
>  enum omap_plane_id plane,
>  enum omap_dss_rotation_type rotation)
> @@ -1910,11 +1898,14 @@ static void dispc_ovl_set_scaling_uv(struct 
> dispc_device *dispc,
>   int scale_x = out_width != orig_width;
>   int scale_y = out_height != orig_height;
>   bool chroma_upscale = plane != OMAP_DSS_WB;
> + const struct drm_format_info *info;
> +
> + info = drm_format_info(fourcc);

Not sure Tomi wants drm usage (apart from the fourccs) inside the
dss code.

>  
>   if (!dispc_has_feature(dispc, FEAT_HANDLE_UV_SEPARATE))
>   return;
>  
> - if (!format_is_yuv(fourcc)) {
> + if (!info->is_yuv) {
>   /* reset chroma resampling for RGB formats  */
>   if (plane != OMAP_DSS_WB)
>   REG_FLD_MOD(dispc, DISPC_OVL_ATTRIBUTES2(plane),
> @@ -2632,6 +2623,9 @@ static int dispc_ovl_setup_common(struct dispc_device 
> *dispc,
>   bool ilace = !!(vm->flags & DISPLAY_FLAGS_INTERLACED);
>   unsigned long pclk = dispc_plane_pclk_rate(dispc, plane);
>   unsigned long lclk = dispc_plane_lclk_rate(dispc, plane);
> + const struct drm_format_info *info;
> +
> + info = drm_format_info(fourcc);
>  
>   /* when setting up WB, dispc_plane_pclk_rate() returns 0 */
>   if (plane == OMAP_DSS_WB)
> @@ -2640,7 +2634,7 @@ static int dispc_ovl_setup_common(struct dispc_device 
> *dispc,
>   if (paddr == 0 && rotation_type != OMAP_DSS_ROT_TILER)
>   return -EINVAL;
>  
> - if (format_is_yuv(fourcc) && (in_width & 1)) {
> + if (info->is_yuv && (in_width & 1)) {
>   DSSERR("input width %d is not even for YUV format\n", in_width);
>   return -EINVAL;
>   }
> @@ -2680,7 +2674,7 @@ static int dispc_ovl_setup_common(struct dispc_device 
> *dispc,
>   DSSDBG("predecimation %d x %x, new input size %d x %d\n",
>   x_predecim, y_predecim, in_width, in_height);
>  
> - if (format_is_yuv(fourcc) && (in_width & 1)) {
> + if (info->is_yuv && (in_width & 1)) {
>   DSSDBG("predecimated input width is not even for YUV format\n");
>   DSSDBG("adjusting input width %d -> %d\n",
>   in_width, in_width & ~1);
> @@ -2688,7 +2682,7 @@ static int dispc_ovl_setup_common(struct dispc_device 
> *dispc,
>   in_width &= ~1;
>   }
>  
> - if (format_is_yuv(fourcc))
> + if (info->is_yuv)
>   cconv = 1;
>  
>   if (ilace && !fieldmode) {
> -- 
> 2.7.4
> 
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 5/5] drm/sun4i: Substitute sun4i_backend_format_is_yuv() with format->is_yuv

[Ville Syrjälä]

On Tue, Jul 17, 2018 at 06:13:46PM +0100, Ayan Kumar Halder wrote:
> drm_format_info table has a field 'is_yuv' to denote if the format
> is yuv or not. The driver is expected to use this instead of
> having a function for the same purpose.
> 
> Signed-off-by: Ayan Kumar halder 
> ---
>  drivers/gpu/drm/sun4i/sun4i_backend.c | 12 +++-
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c 
> b/drivers/gpu/drm/sun4i/sun4i_backend.c
> index de0a76d..d7950b5 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> @@ -86,12 +86,6 @@ static inline bool 
> sun4i_backend_format_is_packed_yuv422(uint32_t format)
>   }
>  }
>  
> -static inline bool sun4i_backend_format_is_yuv(uint32_t format)
> -{
> - return sun4i_backend_format_is_planar_yuv(format) ||
> - sun4i_backend_format_is_packed_yuv422(format);
> -}

sun4i_backend_format_is_planar_yuv() is now unused so could be nuked.
Also the one remaining use of sun4i_backend_format_is_packed_yuv422()
could be replaced with 'is_yuv && num_planes == 1', so that one could
be easily killed off ass well.

> -
>  static void sun4i_backend_apply_color_correction(struct sunxi_engine *engine)
>  {
>   int i;
> @@ -304,7 +298,7 @@ int sun4i_backend_update_layer_formats(struct 
> sun4i_backend *backend,
>  SUN4I_BACKEND_ATTCTL_REG0_LAY_GLBALPHA_EN,
>  val);
>  
> - if (sun4i_backend_format_is_yuv(fb->format->format))
> + if (fb->format->is_yuv)
>   return sun4i_backend_update_yuv_format(backend, layer, plane);
>  
>   ret = sun4i_backend_drm_format_to_layer(fb->format->format, );
> @@ -384,7 +378,7 @@ int sun4i_backend_update_layer_buffer(struct 
> sun4i_backend *backend,
>*/
>   paddr -= PHYS_OFFSET;
>  
> - if (sun4i_backend_format_is_yuv(fb->format->format))
> + if (fb->format->is_yuv)
>   return sun4i_backend_update_yuv_buffer(backend, fb, paddr);
>  
>   /* Write the 32 lower bits of the address (in bits) */
> @@ -502,7 +496,7 @@ static int sun4i_backend_atomic_check(struct sunxi_engine 
> *engine,
>   if (fb->format->has_alpha || (plane_state->alpha != 
> DRM_BLEND_ALPHA_OPAQUE))
>   num_alpha_planes++;
>  
> - if (sun4i_backend_format_is_yuv(fb->format->format)) {
> + if (fb->format->is_yuv) {
>   DRM_DEBUG_DRIVER("Plane FB format is YUV\n");
>   num_yuv_planes++;
>   }
> -- 
> 2.7.4
> 
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] backlight: pwm_bl: Fix uninitialized variable

[Daniel Thompson]

On Wed, Jul 18, 2018 at 10:53:35AM +0100, Lee Jones wrote:
> On Wed, 18 Jul 2018, Marcel Ziswiler wrote:
> 
> > On Wed, 2018-07-18 at 09:09 +0100, Lee Jones wrote:
> > > On Mon, 16 Jul 2018, Daniel Thompson wrote:
> > > 
> > > > Currently, if the DT does not define num-interpolated-steps then
> > > > num_steps is undefined and the interpolation code will deploy
> > > > randomly.
> > > > Fix this.
> > > > 
> > > > Fixes: 573fe6d1c25c ("backlight: pwm_bl: Linear interpolation
> > > > between
> > > > brightness-levels")
> > > > Reported-by: Marcel Ziswiler 
> > > > Signed-off-by: Daniel Thompson 
> > > > Signed-off-by: Marcel Ziswiler 
> > > 
> > > This line is confusing.  Did you guys author this patch together?
> > 
> > Yes, I reported it and we came to a conclusion together.
> 
> It sounds like you need to have all of the tags (except this one). :)
> 
>  Reported-by:  for reporting the issue
>  Suggested-by: for suggesting a resolution
>  Acked-by: for reviewing it
>  Tested-by:for testing it
> 
> Signed-off-by usually means you either wrote a significant amount of
> the diffstat or you were part of the submission path.

He did [I don't object to but wouldn't have used the extra brackets you
brought up ;-) ].

> 
> > > My guess is that this line should be dropped and the RB and TB tags
> > > should remain?  If it was reviewed too, perhaps an AB too?
> > 
> > I'm OK either way and do not need any explicit authorship to be
> > expressed for me.
> 
> In this instance I suggest keeping Reported-by and Tested-by.
> 
> > > > Tested-by: Marcel Ziswiler 
> > > > ---
> > > >  drivers/video/backlight/pwm_bl.c | 17 -
> > > >  1 file changed, 8 insertions(+), 9 deletions(-)
> > > > 
> > > > diff --git a/drivers/video/backlight/pwm_bl.c
> > > > b/drivers/video/backlight/pwm_bl.c
> > > > index 9ee4c1b735b2..e3c22b79fbcd 100644
> > > > --- a/drivers/video/backlight/pwm_bl.c
> > > > +++ b/drivers/video/backlight/pwm_bl.c
> > > > @@ -299,15 +299,14 @@ static int pwm_backlight_parse_dt(struct
> > > > device *dev,
> > > >  * interpolation between each of the values of
> > > > brightness levels
> > > >  * and creates a new pre-computed table.
> > > >  */
> > > > -   of_property_read_u32(node, "num-interpolated-
> > > > steps",
> > > > -_steps);
> > > > -
> > > > -   /*
> > > > -* Make sure that there is at least two entries in
> > > > the
> > > > -* brightness-levels table, otherwise we can't
> > > > interpolate
> > > > -* between two points.
> > > > -*/
> > > > -   if (num_steps) {
> > > > +   if ((of_property_read_u32(node, "num-interpolated-
> > > > steps",
> > > > + _steps) == 0) &&
> > > > num_steps) {
> > > 
> > > This is pretty ugly, and isn't it suffering from over-bracketing?  My
> > > suggestion would be to break out the invocation of
> > > of_property_read_u32() from the if and test only the result.
> > > 
> > >   of_property_read_u32(node, "num-interpolated-steps", 
> > > _steps);
> > 
> > you mean:
> > 
> > ret = of_property_read_u32(node, "num-interpolated-
> > steps", _steps);
> > 
> > >   if (!ret && num_steps) {
> > > 
> > > I haven't checked the underling code, but is it even feasible for
> > > of_property_read_u32() to not succeed AND for num_steps to be set?
> > > 
> > > If not, the check for !ret if superfluous and you can drop it.
> > 
> > No, then we are back to the initial issue of num_steps potentially not
> > being initialised. We really want both of_property_read_u32() to
> > succeed AND num_steps to actually be set.
> 
> I also think num_steps should be pre-initialised.
> 
> Then it will only be set if of_property_read_u32() succeeds.
> 
> -- 
> Lee Jones [李琼斯]
> Linaro Services Technical Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 11/11] drm: rcar-du: Support interlaced video output through vsp1

[Kieran Bingham]

+ MM Team.

On 18/07/18 09:55, Laurent Pinchart wrote:
> Hi Kieran,
> 
> On Tuesday, 17 July 2018 23:32:56 EEST Kieran Bingham wrote:
>> On 17/07/18 14:51, Laurent Pinchart wrote:
>>> On Monday, 16 July 2018 20:20:30 EEST Kieran Bingham wrote:
 On 24/05/18 12:50, Laurent Pinchart wrote:
> On Thursday, 3 May 2018 16:36:22 EEST Kieran Bingham wrote:
>> Use the newly exposed VSP1 interface to enable interlaced frame support
>> through the VSP1 lif pipelines.
>
> s/lif/LIF/

 Fixed.

>> Signed-off-by: Kieran Bingham 
>> ---
>>
>>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 1 +
>>  drivers/gpu/drm/rcar-du/rcar_du_vsp.c  | 3 +++
>>  2 files changed, 4 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
>> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index
>> d71d709fe3d9..206532959ec9
>> 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
>> @@ -289,6 +289,7 @@ static void rcar_du_crtc_set_display_timing(struct
>> rcar_du_crtc *rcrtc)
>>
>>  /* Signal polarities */
>>  value = ((mode->flags & DRM_MODE_FLAG_PVSYNC) ? DSMR_VSL : 0)
>>  
>>| ((mode->flags & DRM_MODE_FLAG_PHSYNC) ? DSMR_HSL : 0)
>>
>> +  | ((mode->flags & DRM_MODE_FLAG_INTERLACE) ? DSMR_ODEV : 
>> 0)
>
> How will this affect Gen2 ?.

 The bit is documented identically for Gen2. Potentially / Probably it
 'might' reverse the fields... I'm not certain yet. I don't have access
 to a Gen2 platform to test.

 I'll see if this change can be dropped, but I think it is playing a role
 in ensuring that the field detection occurs in VSP1 through the
 VI6_STATUS_FLD_STD() field. (see vsp1_dlm_irq_frame_end())

> Could you document what this change does in the
> commit message ?

 This sets the position in the buffer of the ODDF. With this set, the ODD
 field is located in the second half (BOTTOM) of the same frame of the
 interlace display.

 Otherwise, it's in the first half (TOP)

 I faced some issues as to the ordering when testing, so I suspect this
 might actually be related to that. (re VI6_STATUS_FLD_STD in
 vsp1_dlm_irq_frame_end()).

 As you mention - this may have a negative effect on the Gen2
 implementation - so it needs considering with that.


 /me to investigate further.
>>>
>>> Thank you. I don't object to this change, but I'd like to know what its
>>> implications are on Gen2. It might even fix a bug :-) Let me know if you'd
>>> like me to run tests on a Lager board.
>>
>> I've done some testing with this (removing the DSMR change, and
>> inverting the VI6_STATUS_FLD_STD handling) and had some odd results.
>> Perhaps my testing needs refinement.
>>
>> So, yes please - I think I'd really like to know what the effects are on
>> a Lager platform.
>>
>> Would you (or anyone with a Gen2 and interest in vsp1/du) be able to
>> test my latest vsp1/du/interlaced branch/tag on your local Gen2 platform
>> please?
> 
> I can test it when I'll come back home on the 24th (or rather on the next day 
> as I'll land in the evening). Could you please ping me on the 25th ?
> 
>> I'm testing interlaced with:
>>
>> kmstest # sanity test.
>> kmstest -c 1 -r 1920x1080i --flip

 ## -c 1 may have to change to be the right CRTC number

> 
> My monitors don't support interlaced modes, so this won't be easy :-/

More than that - I suspect "won't be possible"?

Perhaps Niklas, Jacopo, or Uli could help ?

Do any of you have a Gen2 platform - and a monitor/TV capable of showing
interlaced display output?



> Also, what's the expected outcome of the above command in the working and non-
> working cases ?

Expected outcome ... a clean output of the standard kmstest pattern for
the sanity test.

For:
kmstest -c 1 -r 1920x1080i --flip


Good:
  A clean view of the usual 'flip' bar moving from left to right across
the screen.
 Must visually check the following for good result:
  - No 'tearing' on the vertical bar
  - A full height bar, showing the colours, red, green, and blue,
followed by 3 shades of grey; each separated by a white block.
  - cleanly drawn frame numbers



Bad symptoms
 - Half height frames
   (only RGB, or 3 Grey shades showing in the flip bar)

 - Tearing or image racing of any kind (check the frame numbers as they
overdraw on the moving bar. Any blurring of the numbers as the bar
travels past is a fail)


> 
>> Any (easy) other methods for testing interlaced pipelines are welcome.
>>
>> Is it possible to set the mode for kmscube? (--help doesn't look promising)
> 
> Not that I know of, but I think that shouldn't be too difficult to add.
> 
>> I have various test streams of interlaced media content in my media
>> library, but not an easy way of decoding and presenting these on the
>> screen on the 

Re: [PATCH] backlight: pwm_bl: Fix uninitialized variable

[Lee Jones]

On Wed, 18 Jul 2018, Marcel Ziswiler wrote:

> On Wed, 2018-07-18 at 09:09 +0100, Lee Jones wrote:
> > On Mon, 16 Jul 2018, Daniel Thompson wrote:
> > 
> > > Currently, if the DT does not define num-interpolated-steps then
> > > num_steps is undefined and the interpolation code will deploy
> > > randomly.
> > > Fix this.
> > > 
> > > Fixes: 573fe6d1c25c ("backlight: pwm_bl: Linear interpolation
> > > between
> > > brightness-levels")
> > > Reported-by: Marcel Ziswiler 
> > > Signed-off-by: Daniel Thompson 
> > > Signed-off-by: Marcel Ziswiler 
> > 
> > This line is confusing.  Did you guys author this patch together?
> 
> Yes, I reported it and we came to a conclusion together.

It sounds like you need to have all of the tags (except this one). :)

 Reported-by:  for reporting the issue
 Suggested-by: for suggesting a resolution
 Acked-by: for reviewing it
 Tested-by:for testing it

Signed-off-by usually means you either wrote a significant amount of
the diffstat or you were part of the submission path.

> > My guess is that this line should be dropped and the RB and TB tags
> > should remain?  If it was reviewed too, perhaps an AB too?
> 
> I'm OK either way and do not need any explicit authorship to be
> expressed for me.

In this instance I suggest keeping Reported-by and Tested-by.

> > > Tested-by: Marcel Ziswiler 
> > > ---
> > >  drivers/video/backlight/pwm_bl.c | 17 -
> > >  1 file changed, 8 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/video/backlight/pwm_bl.c
> > > b/drivers/video/backlight/pwm_bl.c
> > > index 9ee4c1b735b2..e3c22b79fbcd 100644
> > > --- a/drivers/video/backlight/pwm_bl.c
> > > +++ b/drivers/video/backlight/pwm_bl.c
> > > @@ -299,15 +299,14 @@ static int pwm_backlight_parse_dt(struct
> > > device *dev,
> > >* interpolation between each of the values of
> > > brightness levels
> > >* and creates a new pre-computed table.
> > >*/
> > > - of_property_read_u32(node, "num-interpolated-
> > > steps",
> > > -  _steps);
> > > -
> > > - /*
> > > -  * Make sure that there is at least two entries in
> > > the
> > > -  * brightness-levels table, otherwise we can't
> > > interpolate
> > > -  * between two points.
> > > -  */
> > > - if (num_steps) {
> > > + if ((of_property_read_u32(node, "num-interpolated-
> > > steps",
> > > +   _steps) == 0) &&
> > > num_steps) {
> > 
> > This is pretty ugly, and isn't it suffering from over-bracketing?  My
> > suggestion would be to break out the invocation of
> > of_property_read_u32() from the if and test only the result.
> > 
> > of_property_read_u32(node, "num-interpolated-steps", 
> > _steps);
> 
> you mean:
> 
>   ret = of_property_read_u32(node, "num-interpolated-
> steps", _steps);
> 
> > if (!ret && num_steps) {
> > 
> > I haven't checked the underling code, but is it even feasible for
> > of_property_read_u32() to not succeed AND for num_steps to be set?
> > 
> > If not, the check for !ret if superfluous and you can drop it.
> 
> No, then we are back to the initial issue of num_steps potentially not
> being initialised. We really want both of_property_read_u32() to
> succeed AND num_steps to actually be set.

I also think num_steps should be pre-initialised.

Then it will only be set if of_property_read_u32() succeeds.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/sun4i: sun4i: Introduce a quirk for lowest plane alpha support

[Paul Kocialkowski]

On Wed, 2018-07-18 at 11:29 +0200, Paul Kocialkowski wrote:
> Not all sunxi platforms with the first version of the Display Engine
> support an alpha component on the plane with the lowest z position
> (as in: lowest z-pos), that gets blended with the background color.
> 
> In particular, the A13 is known to have this limitation. However, it was
> recently discovered that the A20 and A33 are capable of having alpha on
> their lowest plane.
> 
> Thus, this introduces a specific quirk to indicate such support,
> per-platform. Since this was not tested on sun4i and sun6i platforms, a
> conservative approach is kept and this feature is not supported.

Woops, I forgot to include PATCH 1/2 from v1, which did not change.

Would it be preferable for me to send v2 again (and maybe call it v3)
for convenience?

Sorry about that,

Paul

> Signed-off-by: Paul Kocialkowski 
> ---
> 
> Changes since v1:
> * reordered backend declaration;
> * updated comment to reflect that not all platforms are affected;
> * used num_alpha_planes_max variable instead of define, since it is now
>   dynamic;
> * reordered check to have quirk first in associated conditional.
> 
>  drivers/gpu/drm/sun4i/sun4i_backend.c | 40 +--
>  drivers/gpu/drm/sun4i/sun4i_backend.h |  1 -
>  2 files changed, 26 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c 
> b/drivers/gpu/drm/sun4i/sun4i_backend.c
> index de0a76dfa1a2..eda9466b793b 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> @@ -34,6 +34,8 @@
>  struct sun4i_backend_quirks {
>   /* backend <-> TCON muxing selection done in backend */
>   bool needs_output_muxing;
> + /* alpha at the lowest z position is not always supported */
> + bool supports_lowest_plane_alpha;
>  };
>  
>  static const u32 sunxi_rgb2yuv_coef[12] = {
> @@ -463,12 +465,14 @@ static int sun4i_backend_atomic_check(struct 
> sunxi_engine *engine,
> struct drm_crtc_state *crtc_state)
>  {
>   struct drm_plane_state *plane_states[SUN4I_BACKEND_NUM_LAYERS] = { 0 };
> + struct sun4i_backend *backend = engine_to_sun4i_backend(engine);
>   struct drm_atomic_state *state = crtc_state->state;
>   struct drm_device *drm = state->dev;
>   struct drm_plane *plane;
>   unsigned int num_planes = 0;
>   unsigned int num_alpha_planes = 0;
>   unsigned int num_frontend_planes = 0;
> + unsigned int num_alpha_planes_max = 1;
>   unsigned int num_yuv_planes = 0;
>   unsigned int current_pipe = 0;
>   unsigned int i;
> @@ -532,33 +536,39 @@ static int sun4i_backend_atomic_check(struct 
> sunxi_engine *engine,
>* the layer with the highest priority.
>*
>* The second step is the actual alpha blending, that takes
> -  * the two pipes as input, and uses the eventual alpha
> +  * the two pipes as input, and uses the potential alpha
>* component to do the transparency between the two.
>*
> -  * This two steps scenario makes us unable to guarantee a
> +  * This two-step scenario makes us unable to guarantee a
>* robust alpha blending between the 4 layers in all
>* situations, since this means that we need to have one layer
>* with alpha at the lowest position of our two pipes.
>*
> -  * However, we cannot even do that, since the hardware has a
> -  * bug where the lowest plane of the lowest pipe (pipe 0,
> -  * priority 0), if it has any alpha, will discard the pixel
> -  * entirely and just display the pixels in the background
> -  * color (black by default).
> +  * However, we cannot even do that on every platform, since the
> +  * hardware has a bug where the lowest plane of the lowest pipe
> +  * (pipe 0, priority 0), if it has any alpha, will discard the
> +  * pixel data entirely and just display the pixels in the
> +  * background color (black by default).
>*
> -  * This means that we effectively have only three valid
> -  * configurations with alpha, all of them with the alpha being
> -  * on pipe1 with the lowest position, which can be 1, 2 or 3
> -  * depending on the number of planes and their zpos.
> +  * This means that on the affected platforms, we effectively
> +  * have only three valid configurations with alpha, all of them
> +  * with the alpha being on pipe1 with the lowest position, which
> +  * can be 1, 2 or 3 depending on the number of planes and their zpos.
>*/
> - if (num_alpha_planes > SUN4I_BACKEND_NUM_ALPHA_LAYERS) {
> +
> + /* For platforms that are not affected by the issue described above. */
> + if (backend->quirks->supports_lowest_plane_alpha)
> + num_alpha_planes_max++;
> +
> + if (num_alpha_planes > num_alpha_planes_max) {
>   DRM_DEBUG_DRIVER("Too many planes with alpha, rejecting...\n");
> 

Re: [PATCH v2 1/1] fbdev/core: Disable console-lock warnings when fb.lockless_register_fb is set

[Hans de Goede]

Hi,

On 18-07-18 11:30, Thomas Zimmermann wrote:

If the console is unlocked during registration, the console subsystem
generates significant amounts of warnings, which obfuscate actual
debugging messages. Setting ignore_console_lock_warning while debugging
console registration avoid the noise.

v2:
- restore ignore_console_lock_warning if lock_fb_info() fails

Signed-off-by: Thomas Zimmermann 


Patch looks good to me, thanks:

Reviewed-by: Hans de Goede 

For "[PATCH v2] console: Replace #if 1 with a bool to ignore"
Petr wrote:

"Acked-by: Petr Mladek 

I assume that it will go via fbdev tree with the other changes
unless I hear otherwise."

So that patch and this one can both be picked up by Bartlomiej for
merging through the fbdev tree when he is back.

Regards,

Hans






---
  drivers/video/fbdev/core/fbmem.c | 15 +++
  1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 9e2f9d3c760e..432c26eeabfb 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1627,6 +1627,7 @@ static int do_register_framebuffer(struct fb_info 
*fb_info)
int i, ret;
struct fb_event event;
struct fb_videomode mode;
+   bool saved_ignore_console_lock_warning = ignore_console_lock_warning;
  
  	if (fb_check_foreignness(fb_info))

return -ENOSYS;
@@ -1691,17 +1692,23 @@ static int do_register_framebuffer(struct fb_info 
*fb_info)
event.info = fb_info;
if (!lockless_register_fb)
console_lock();
+   else
+   ignore_console_lock_warning = true;
if (!lock_fb_info(fb_info)) {
-   if (!lockless_register_fb)
-   console_unlock();
-   return -ENODEV;
+   ret = -ENODEV;
+   goto unlock_console;
}
+   ret = 0;
  
  	fb_notifier_call_chain(FB_EVENT_FB_REGISTERED, );

unlock_fb_info(fb_info);
+unlock_console:
if (!lockless_register_fb)
console_unlock();
-   return 0;
+   else
+   ignore_console_lock_warning =
+   saved_ignore_console_lock_warning;
+   return ret;
  }
  
  static int do_unregister_framebuffer(struct fb_info *fb_info)



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

[PATCH v2] drm/sun4i: sun4i: Introduce a quirk for lowest plane alpha support

[Paul Kocialkowski]

Not all sunxi platforms with the first version of the Display Engine
support an alpha component on the plane with the lowest z position
(as in: lowest z-pos), that gets blended with the background color.

In particular, the A13 is known to have this limitation. However, it was
recently discovered that the A20 and A33 are capable of having alpha on
their lowest plane.

Thus, this introduces a specific quirk to indicate such support,
per-platform. Since this was not tested on sun4i and sun6i platforms, a
conservative approach is kept and this feature is not supported.

Signed-off-by: Paul Kocialkowski 
---

Changes since v1:
* reordered backend declaration;
* updated comment to reflect that not all platforms are affected;
* used num_alpha_planes_max variable instead of define, since it is now
  dynamic;
* reordered check to have quirk first in associated conditional.

 drivers/gpu/drm/sun4i/sun4i_backend.c | 40 +--
 drivers/gpu/drm/sun4i/sun4i_backend.h |  1 -
 2 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c 
b/drivers/gpu/drm/sun4i/sun4i_backend.c
index de0a76dfa1a2..eda9466b793b 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -34,6 +34,8 @@
 struct sun4i_backend_quirks {
/* backend <-> TCON muxing selection done in backend */
bool needs_output_muxing;
+   /* alpha at the lowest z position is not always supported */
+   bool supports_lowest_plane_alpha;
 };
 
 static const u32 sunxi_rgb2yuv_coef[12] = {
@@ -463,12 +465,14 @@ static int sun4i_backend_atomic_check(struct sunxi_engine 
*engine,
  struct drm_crtc_state *crtc_state)
 {
struct drm_plane_state *plane_states[SUN4I_BACKEND_NUM_LAYERS] = { 0 };
+   struct sun4i_backend *backend = engine_to_sun4i_backend(engine);
struct drm_atomic_state *state = crtc_state->state;
struct drm_device *drm = state->dev;
struct drm_plane *plane;
unsigned int num_planes = 0;
unsigned int num_alpha_planes = 0;
unsigned int num_frontend_planes = 0;
+   unsigned int num_alpha_planes_max = 1;
unsigned int num_yuv_planes = 0;
unsigned int current_pipe = 0;
unsigned int i;
@@ -532,33 +536,39 @@ static int sun4i_backend_atomic_check(struct sunxi_engine 
*engine,
 * the layer with the highest priority.
 *
 * The second step is the actual alpha blending, that takes
-* the two pipes as input, and uses the eventual alpha
+* the two pipes as input, and uses the potential alpha
 * component to do the transparency between the two.
 *
-* This two steps scenario makes us unable to guarantee a
+* This two-step scenario makes us unable to guarantee a
 * robust alpha blending between the 4 layers in all
 * situations, since this means that we need to have one layer
 * with alpha at the lowest position of our two pipes.
 *
-* However, we cannot even do that, since the hardware has a
-* bug where the lowest plane of the lowest pipe (pipe 0,
-* priority 0), if it has any alpha, will discard the pixel
-* entirely and just display the pixels in the background
-* color (black by default).
+* However, we cannot even do that on every platform, since the
+* hardware has a bug where the lowest plane of the lowest pipe
+* (pipe 0, priority 0), if it has any alpha, will discard the
+* pixel data entirely and just display the pixels in the
+* background color (black by default).
 *
-* This means that we effectively have only three valid
-* configurations with alpha, all of them with the alpha being
-* on pipe1 with the lowest position, which can be 1, 2 or 3
-* depending on the number of planes and their zpos.
+* This means that on the affected platforms, we effectively
+* have only three valid configurations with alpha, all of them
+* with the alpha being on pipe1 with the lowest position, which
+* can be 1, 2 or 3 depending on the number of planes and their zpos.
 */
-   if (num_alpha_planes > SUN4I_BACKEND_NUM_ALPHA_LAYERS) {
+
+   /* For platforms that are not affected by the issue described above. */
+   if (backend->quirks->supports_lowest_plane_alpha)
+   num_alpha_planes_max++;
+
+   if (num_alpha_planes > num_alpha_planes_max) {
DRM_DEBUG_DRIVER("Too many planes with alpha, rejecting...\n");
return -EINVAL;
}
 
/* We can't have an alpha plane at the lowest position */
-   if (plane_states[0]->fb->format->has_alpha ||
-   (plane_states[0]->alpha != DRM_BLEND_ALPHA_OPAQUE))
+   if (!backend->quirks->supports_lowest_plane_alpha &&
+   

[PATCH v2 1/1] fbdev/core: Disable console-lock warnings when fb.lockless_register_fb is set

[Thomas Zimmermann]

If the console is unlocked during registration, the console subsystem
generates significant amounts of warnings, which obfuscate actual
debugging messages. Setting ignore_console_lock_warning while debugging
console registration avoid the noise.

v2:
- restore ignore_console_lock_warning if lock_fb_info() fails

Signed-off-by: Thomas Zimmermann 
---
 drivers/video/fbdev/core/fbmem.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 9e2f9d3c760e..432c26eeabfb 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1627,6 +1627,7 @@ static int do_register_framebuffer(struct fb_info 
*fb_info)
int i, ret;
struct fb_event event;
struct fb_videomode mode;
+   bool saved_ignore_console_lock_warning = ignore_console_lock_warning;
 
if (fb_check_foreignness(fb_info))
return -ENOSYS;
@@ -1691,17 +1692,23 @@ static int do_register_framebuffer(struct fb_info 
*fb_info)
event.info = fb_info;
if (!lockless_register_fb)
console_lock();
+   else
+   ignore_console_lock_warning = true;
if (!lock_fb_info(fb_info)) {
-   if (!lockless_register_fb)
-   console_unlock();
-   return -ENODEV;
+   ret = -ENODEV;
+   goto unlock_console;
}
+   ret = 0;
 
fb_notifier_call_chain(FB_EVENT_FB_REGISTERED, );
unlock_fb_info(fb_info);
+unlock_console:
if (!lockless_register_fb)
console_unlock();
-   return 0;
+   else
+   ignore_console_lock_warning =
+   saved_ignore_console_lock_warning;
+   return ret;
 }
 
 static int do_unregister_framebuffer(struct fb_info *fb_info)
-- 
2.18.0

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

[PATCH v2 0/1] fbdev/core: Disable console-lock warnings when fb.lockless_register_fb is set

[Thomas Zimmermann]

Hi,

this is a follow-up patch for the recent addition of
'ignore_console_lock_warning' to the console module [1] and should probably
go through the same tree. When debugging fbdev console registration, the
console-lock warnings will now be disabled temporarily.

Best regards
Thomas

[1] https://marc.info/?l=dri-devel=153140585411176=2

v2:
- restore ignore_console_lock_warning if lock_fb_info() fails

Thomas Zimmermann (1):
  fbdev/core: Disable console-lock warnings when fb.lockless_register_fb
is set

 drivers/video/fbdev/core/fbmem.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

--
2.18.0

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

[PATCH V3] drm: handle error values properly

[Nicholas Mc Guire]

drm_legacy_ctxbitmap_next() returns idr_alloc() which can return
-ENOMEM, -EINVAL or -ENOSPC none of which are -1. since drm_context_t
is an unsigned int an intermediate variable is used to handle the
error cases, and then cast to drm_context_t after ensuring that the
value is >= 0. The explicit cast is to mark the type conversion as
intentional.

Signed-off-by: Nicholas Mc Guire 
Reported-by: kbuild test robot 
Reported-by: Sean Paul 
Fixes: d530b5f1ca0b ("drm: re-enable error handling")
Fixes: 62968144e673 ("drm: convert drm context code to use Linux idr")
---

V3: bug in patch - omission to remove old code properly - V3 fixes the
original problem as proposed in V2 and drops the excess line.
reported by Sean Paul  - thanks!

V2: The proposed fix in d530b5f1ca0b ("drm: re-enable error handling")
actually was ineffective as the negative return value check was
against a unsigned int and thus always false as reported by
kbuild test robot . The below patch removes that
warning and fixes the original problem of missed error handling.

drm_context_t is actually just used in a few placed so the type could be
changed but it is also exported via tools/include/uapi/drm/drm.h so
changing the typedef of drm_context_t could break applications and thus
this is not an option.

Patch was compile tested with: x86_64_defconfig

Patch is against 4.18-rc5 (localversion-next is next-20180718)

 drivers/gpu/drm/drm_context.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_context.c b/drivers/gpu/drm/drm_context.c
index f973d28..ad268c8 100644
--- a/drivers/gpu/drm/drm_context.c
+++ b/drivers/gpu/drm/drm_context.c
@@ -361,22 +361,25 @@ int drm_legacy_addctx(struct drm_device *dev, void *data,
 {
struct drm_ctx_list *ctx_entry;
struct drm_ctx *ctx = data;
+   int ret;
 
if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT) &&
!drm_core_check_feature(dev, DRIVER_LEGACY))
return -EINVAL;
 
-   ctx->handle = drm_legacy_ctxbitmap_next(dev);
-   if (ctx->handle == DRM_KERNEL_CONTEXT) {
+   ret = drm_legacy_ctxbitmap_next(dev);
+   if (ret == DRM_KERNEL_CONTEXT) {
/* Skip kernel's context and get a new one. */
-   ctx->handle = drm_legacy_ctxbitmap_next(dev);
+   ret = drm_legacy_ctxbitmap_next(dev);
}
-   DRM_DEBUG("%d\n", ctx->handle);
-   if (ctx->handle < 0) {
+   DRM_DEBUG("ctxbitmap is error code %d\n", ret);
+   if (ret < 0) {
DRM_DEBUG("Not enough free contexts.\n");
/* Should this return -EBUSY instead? */
return -ENOMEM;
}
+   /* valid context is >= 0 */
+   ctx->handle = (drm_context_t)ret;
 
ctx_entry = kmalloc(sizeof(*ctx_entry), GFP_KERNEL);
if (!ctx_entry) {
-- 
2.1.4

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

Re: [PATCH 1/1] fbdev/core: Disable console-lock warnings when fb.lockless_register_fb is set

[Thomas Zimmermann]

Am 18.07.2018 um 10:44 schrieb Hans de Goede:
> Hi,
> 
> On 18-07-18 10:36, Thomas Zimmermann wrote:
>> If the console is unlocked during registration, the console subsystem
>> generates significant amounts of warnings, which obfuscate actual
>> debugging messages. Setting ignore_console_lock_warning while debugging
>> console registration avoid the noise.
>>
>> Signed-off-by: Thomas Zimmermann 
> 
> Thank you for doing this, but there are multiple console_unlock exit
> paths in do_register_framebuffer(), you missed the one in:

Sorry for this half-baked patch and thanks for the review. I'll provide
an update.

Best regards
Thomas


> 
>     if (!lock_fb_info(fb_info)) {
>     if (!lockless_register_fb)
>     console_unlock();
>     return -ENODEV;
>     }
> 
> I would change this to:
> 
>     if (!lock_fb_info(fb_info)) {
>     ret = -ENODEV;
>     goto unlock_console;
>     }
> 
> ret = 0;
> 
> And put a "unlock_console:" label here:
> 
> unlock_console:
>    if (!lockless_register_fb)
>    console_unlock();
> else
>     ignore_console_lock_warning =
>     saved_ignore_console_lock_warning;
> 
> And change the final return to:
> 
> return ret;
> 
> Otherwise this looks good to me.
> 
> Regards,
> 
> Hans
> 
> 
> 
> 
> 
>> ---
>>   drivers/video/fbdev/core/fbmem.c | 6 ++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/video/fbdev/core/fbmem.c
>> b/drivers/video/fbdev/core/fbmem.c
>> index 9e2f9d3c760e..79b489ad603d 100644
>> --- a/drivers/video/fbdev/core/fbmem.c
>> +++ b/drivers/video/fbdev/core/fbmem.c
>> @@ -1627,6 +1627,7 @@ static int do_register_framebuffer(struct
>> fb_info *fb_info)
>>   int i, ret;
>>   struct fb_event event;
>>   struct fb_videomode mode;
>> +    bool saved_ignore_console_lock_warning =
>> ignore_console_lock_warning;
>>     if (fb_check_foreignness(fb_info))
>>   return -ENOSYS;
>> @@ -1691,6 +1692,8 @@ static int do_register_framebuffer(struct
>> fb_info *fb_info)
>>   event.info = fb_info;
>>   if (!lockless_register_fb)
>>   console_lock();
>> +    else
>> +    ignore_console_lock_warning = true;
>>   if (!lock_fb_info(fb_info)) {
>>   if (!lockless_register_fb)
>>   console_unlock();
>> @@ -1701,6 +1704,9 @@ static int do_register_framebuffer(struct
>> fb_info *fb_info)
>>   unlock_fb_info(fb_info);
>>   if (!lockless_register_fb)
>>   console_unlock();
>> +    else
>> +    ignore_console_lock_warning =
>> +    saved_ignore_console_lock_warning;
>>   return 0;
>>   }
>>  

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Linux GmbH, Maxfeldstr. 5, D-90409 Nürnberg
Tel: +49-911-74053-0; Fax: +49-911-7417755;  https://www.suse.com/
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard,
Graham Norton, HRB 21284 (AG Nürnberg)



signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/5] drm/fourcc: Add is_yuv field to drm_format_info to denote if the format is yuv

[Brian Starkey]

Hi Ayan,

On Tue, Jul 17, 2018 at 06:13:42PM +0100, Ayan Kumar Halder wrote:

A lot of drivers duplicate the function to check if a format is yuv or not.
If we add a field (to denote whether the format is yuv or not) in the
drm_format_info table, all the drivers can use this field and it will
prevent duplication of similar logic.


This looks like a good idea to me.

I wonder if the two "has_alpha" and "is_yuv" bools should be bitfields
to reduce the footprint (not sure what the general DRM attitude to
bitfields is), but either way:

Reviewed-by: Brian Starkey 



Signed-off-by: Ayan Kumar halder 
---
drivers/gpu/drm/drm_fourcc.c | 42 +-
include/drm/drm_fourcc.h |  2 ++
2 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 5ca6395..35c1e27 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -152,27 +152,27 @@ const struct drm_format_info *__drm_format_info(u32 
format)
{ .format = DRM_FORMAT_XBGR_A8, .depth = 32, 
.num_planes = 2, .cpp = { 4, 1, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
{ .format = DRM_FORMAT_RGBX_A8, .depth = 32, 
.num_planes = 2, .cpp = { 4, 1, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
{ .format = DRM_FORMAT_BGRX_A8, .depth = 32, 
.num_planes = 2, .cpp = { 4, 1, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
-   { .format = DRM_FORMAT_YUV410,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 4 },
-   { .format = DRM_FORMAT_YVU410,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 4 },
-   { .format = DRM_FORMAT_YUV411,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 1 },
-   { .format = DRM_FORMAT_YVU411,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 1 },
-   { .format = DRM_FORMAT_YUV420,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 2 },
-   { .format = DRM_FORMAT_YVU420,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 2 },
-   { .format = DRM_FORMAT_YUV422,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 1 },
-   { .format = DRM_FORMAT_YVU422,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 1 },
-   { .format = DRM_FORMAT_YUV444,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 1, .vsub = 1 },
-   { .format = DRM_FORMAT_YVU444,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 1, .vsub = 1 },
-   { .format = DRM_FORMAT_NV12,.depth = 0,  
.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2 },
-   { .format = DRM_FORMAT_NV21,.depth = 0,  
.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2 },
-   { .format = DRM_FORMAT_NV16,.depth = 0,  
.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 1 },
-   { .format = DRM_FORMAT_NV61,.depth = 0,  
.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 1 },
-   { .format = DRM_FORMAT_NV24,.depth = 0,  
.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1 },
-   { .format = DRM_FORMAT_NV42,.depth = 0,  
.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1 },
-   { .format = DRM_FORMAT_YUYV,.depth = 0,  
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
-   { .format = DRM_FORMAT_YVYU,.depth = 0,  
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
-   { .format = DRM_FORMAT_UYVY,.depth = 0,  
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
-   { .format = DRM_FORMAT_VYUY,.depth = 0,  
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
-   { .format = DRM_FORMAT_AYUV,.depth = 0,  
.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
+   { .format = DRM_FORMAT_YUV410,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 4, .is_yuv = true },
+   { .format = DRM_FORMAT_YVU410,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 4, .is_yuv = true },
+   { .format = DRM_FORMAT_YUV411,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 1, .is_yuv = true },
+   { .format = DRM_FORMAT_YVU411,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 1, .is_yuv = true },
+   { .format = DRM_FORMAT_YUV420,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 2, .is_yuv = true },
+   

Re: [PATCH v4 11/11] drm: rcar-du: Support interlaced video output through vsp1

[Laurent Pinchart]

Hi Kieran,

On Tuesday, 17 July 2018 23:32:56 EEST Kieran Bingham wrote:
> On 17/07/18 14:51, Laurent Pinchart wrote:
> > On Monday, 16 July 2018 20:20:30 EEST Kieran Bingham wrote:
> >> On 24/05/18 12:50, Laurent Pinchart wrote:
> >>> On Thursday, 3 May 2018 16:36:22 EEST Kieran Bingham wrote:
>  Use the newly exposed VSP1 interface to enable interlaced frame support
>  through the VSP1 lif pipelines.
> >>> 
> >>> s/lif/LIF/
> >> 
> >> Fixed.
> >> 
>  Signed-off-by: Kieran Bingham 
>  ---
>  
>   drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 1 +
>   drivers/gpu/drm/rcar-du/rcar_du_vsp.c  | 3 +++
>   2 files changed, 4 insertions(+)
>  
>  diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
>  b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index
>  d71d709fe3d9..206532959ec9
>  100644
>  --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
>  +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
>  @@ -289,6 +289,7 @@ static void rcar_du_crtc_set_display_timing(struct
>  rcar_du_crtc *rcrtc)
>  
>   /* Signal polarities */
>   value = ((mode->flags & DRM_MODE_FLAG_PVSYNC) ? DSMR_VSL : 0)
>   
> | ((mode->flags & DRM_MODE_FLAG_PHSYNC) ? DSMR_HSL : 0)
>  
>  +  | ((mode->flags & DRM_MODE_FLAG_INTERLACE) ? DSMR_ODEV : 
>  0)
> >>> 
> >>> How will this affect Gen2 ?.
> >> 
> >> The bit is documented identically for Gen2. Potentially / Probably it
> >> 'might' reverse the fields... I'm not certain yet. I don't have access
> >> to a Gen2 platform to test.
> >> 
> >> I'll see if this change can be dropped, but I think it is playing a role
> >> in ensuring that the field detection occurs in VSP1 through the
> >> VI6_STATUS_FLD_STD() field. (see vsp1_dlm_irq_frame_end())
> >> 
> >>> Could you document what this change does in the
> >>> commit message ?
> >> 
> >> This sets the position in the buffer of the ODDF. With this set, the ODD
> >> field is located in the second half (BOTTOM) of the same frame of the
> >> interlace display.
> >> 
> >> Otherwise, it's in the first half (TOP)
> >> 
> >> I faced some issues as to the ordering when testing, so I suspect this
> >> might actually be related to that. (re VI6_STATUS_FLD_STD in
> >> vsp1_dlm_irq_frame_end()).
> >> 
> >> As you mention - this may have a negative effect on the Gen2
> >> implementation - so it needs considering with that.
> >> 
> >> 
> >> /me to investigate further.
> > 
> > Thank you. I don't object to this change, but I'd like to know what its
> > implications are on Gen2. It might even fix a bug :-) Let me know if you'd
> > like me to run tests on a Lager board.
> 
> I've done some testing with this (removing the DSMR change, and
> inverting the VI6_STATUS_FLD_STD handling) and had some odd results.
> Perhaps my testing needs refinement.
> 
> So, yes please - I think I'd really like to know what the effects are on
> a Lager platform.
> 
> Would you (or anyone with a Gen2 and interest in vsp1/du) be able to
> test my latest vsp1/du/interlaced branch/tag on your local Gen2 platform
> please?

I can test it when I'll come back home on the 24th (or rather on the next day 
as I'll land in the evening). Could you please ping me on the 25th ?

> I'm testing interlaced with:
> 
> kmstest # sanity test.
> kmstest -c 1 -r 1920x1080i --flip

My monitors don't support interlaced modes, so this won't be easy :-/

Also, what's the expected outcome of the above command in the working and non-
working cases ?

> Any (easy) other methods for testing interlaced pipelines are welcome.
> 
> Is it possible to set the mode for kmscube? (--help doesn't look promising)

Not that I know of, but I think that shouldn't be too difficult to add.

> I have various test streams of interlaced media content in my media
> library, but not an easy way of decoding and presenting these on the
> screen on the Gen3.
> 
> I believe GStreamer now has a drm/kms sink ... Perhaps I should get that
> recompiled. (That would help me with other tasks too actually)
> 
> | DSMR_DIPM_DISP | DSMR_CSPM;
>   
>   rcar_du_crtc_write(rcrtc, DSMR, value);
>  
>  diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
>  b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index
>  af7822a66dee..c7b37232ee91
>  100644
>  --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
>  +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
>  @@ -186,6 +186,9 @@ static void rcar_du_vsp_plane_setup(struct
>  rcar_du_vsp_plane *plane)
>   };
>   unsigned int i;
>  
>  +cfg.interlaced = !!(plane->plane.state->crtc->mode.flags
>  +& DRM_MODE_FLAG_INTERLACE);
>  +
>   cfg.src.left = state->state.src.x1 >> 16;
>   cfg.src.top = state->state.src.y1 >> 16;
>   cfg.src.width = drm_rect_width(>state.src) >> 16;

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/1] fbdev/core: Disable console-lock warnings when fb.lockless_register_fb is set

[Hans de Goede]

Hi,

On 18-07-18 10:36, Thomas Zimmermann wrote:

If the console is unlocked during registration, the console subsystem
generates significant amounts of warnings, which obfuscate actual
debugging messages. Setting ignore_console_lock_warning while debugging
console registration avoid the noise.

Signed-off-by: Thomas Zimmermann 


Thank you for doing this, but there are multiple console_unlock exit
paths in do_register_framebuffer(), you missed the one in:

if (!lock_fb_info(fb_info)) {
if (!lockless_register_fb)
console_unlock();
return -ENODEV;
}

I would change this to:

if (!lock_fb_info(fb_info)) {
ret = -ENODEV;
goto unlock_console;
}

ret = 0;

And put a "unlock_console:" label here:

unlock_console:
if (!lockless_register_fb)
console_unlock();
else
ignore_console_lock_warning =
saved_ignore_console_lock_warning;

And change the final return to:

return ret;

Otherwise this looks good to me.

Regards,

Hans






---
  drivers/video/fbdev/core/fbmem.c | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 9e2f9d3c760e..79b489ad603d 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1627,6 +1627,7 @@ static int do_register_framebuffer(struct fb_info 
*fb_info)
int i, ret;
struct fb_event event;
struct fb_videomode mode;
+   bool saved_ignore_console_lock_warning = ignore_console_lock_warning;
  
  	if (fb_check_foreignness(fb_info))

return -ENOSYS;
@@ -1691,6 +1692,8 @@ static int do_register_framebuffer(struct fb_info 
*fb_info)
event.info = fb_info;
if (!lockless_register_fb)
console_lock();
+   else
+   ignore_console_lock_warning = true;
if (!lock_fb_info(fb_info)) {
if (!lockless_register_fb)
console_unlock();
@@ -1701,6 +1704,9 @@ static int do_register_framebuffer(struct fb_info 
*fb_info)
unlock_fb_info(fb_info);
if (!lockless_register_fb)
console_unlock();
+   else
+   ignore_console_lock_warning =
+   saved_ignore_console_lock_warning;
return 0;
  }
  


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

[PATCH 0/1] fbdev/core: Disable console-lock warnings when fb.lockless_register_fb is set

[Thomas Zimmermann]

Hi,

this is a follow-up patch for the recent addition of
'ignore_console_lock_warning' to the console module [1] and should probably
go through the same tree. When debugging fbdev console registration, the
console-lock warnings will now be disabled temporarily.

Best regards
Thomas

[1] https://marc.info/?l=dri-devel=153140585411176=2

Thomas Zimmermann (1):
  fbdev/core: Disable console-lock warnings when fb.lockless_register_fb
is set

 drivers/video/fbdev/core/fbmem.c | 6 ++
 1 file changed, 6 insertions(+)

--
2.18.0

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

[PATCH 1/1] fbdev/core: Disable console-lock warnings when fb.lockless_register_fb is set

[Thomas Zimmermann]

If the console is unlocked during registration, the console subsystem
generates significant amounts of warnings, which obfuscate actual
debugging messages. Setting ignore_console_lock_warning while debugging
console registration avoid the noise.

Signed-off-by: Thomas Zimmermann 
---
 drivers/video/fbdev/core/fbmem.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 9e2f9d3c760e..79b489ad603d 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1627,6 +1627,7 @@ static int do_register_framebuffer(struct fb_info 
*fb_info)
int i, ret;
struct fb_event event;
struct fb_videomode mode;
+   bool saved_ignore_console_lock_warning = ignore_console_lock_warning;
 
if (fb_check_foreignness(fb_info))
return -ENOSYS;
@@ -1691,6 +1692,8 @@ static int do_register_framebuffer(struct fb_info 
*fb_info)
event.info = fb_info;
if (!lockless_register_fb)
console_lock();
+   else
+   ignore_console_lock_warning = true;
if (!lock_fb_info(fb_info)) {
if (!lockless_register_fb)
console_unlock();
@@ -1701,6 +1704,9 @@ static int do_register_framebuffer(struct fb_info 
*fb_info)
unlock_fb_info(fb_info);
if (!lockless_register_fb)
console_unlock();
+   else
+   ignore_console_lock_warning =
+   saved_ignore_console_lock_warning;
return 0;
 }
 
-- 
2.18.0

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

Re: [Nouveau] [PATCH 1/5] drm/nouveau: Prevent RPM callback recursion in suspend/resume paths

[Lukas Wunner]

On Wed, Jul 18, 2018 at 10:25:05AM +0200, Lukas Wunner wrote:
> The GPU contains an i2c subdevice for each connector with DDC lines.
> I believe those are modelled as children of the GPU's PCI device as
> they're accessed via mmio of the PCI device.
> 
> The problem here is that when the GPU's PCI device runtime suspends,
> its i2c child device needs to be runtime active to suspend the MST
> topology.  Catch-22.
> 
> I don't know whether or not it's necessary to suspend the MST topology.
> I'm not an expert on DisplayPort MultiStream transport.
> 
> BTW Lyude, in patch 4 and 5 of this series, you're runtime resuming
> pad->i2c->subdev.device->dev.  Is this the PCI device or is it the i2c
> device?  I'm always confused by nouveau's structs.  In nvkm_i2c_bus_ctor()
> I can see that the device you're runtime resuming is the parent of the
> i2c_adapter:
> 
>   struct nvkm_device *device = pad->i2c->subdev.device;
>   [...]
>   bus->i2c.dev.parent = device->dev;
> 
> If the i2c_adapter is a child of the PCI device, it's sufficient
> to runtime resume the i2c_adapter, i.e. bus->i2c.dev, and this will
> implicitly runtime resume its parent.

Actually, having written all this I just remembered that we have this
in the documentation:

8. "No-Callback" Devices

Some "devices" are only logical sub-devices of their parent and cannot be
power-managed on their own. [...]

Subsystems can tell the PM core about these devices by calling
pm_runtime_no_callbacks().

So it might actually be sufficient to just call pm_runtime_no_callbacks()
for the i2c devices...

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

Re: [Nouveau] [PATCH 1/5] drm/nouveau: Prevent RPM callback recursion in suspend/resume paths

[Rafael J. Wysocki]

On Wed, Jul 18, 2018 at 10:25 AM, Lukas Wunner  wrote:
> On Wed, Jul 18, 2018 at 09:38:41AM +0200, Rafael J. Wysocki wrote:
>> On Tue, Jul 17, 2018 at 8:20 PM, Lukas Wunner  wrote:
>> > Okay, the PCI device is suspending and the nvkm_i2c_aux_acquire()
>> > wants it in resumed state, so is waiting forever for the device to
>> > runtime suspend in order to resume it again immediately afterwards.
>> >
>> > The deadlock in the stack trace you've posted could be resolved using
>> > the technique I used in d61a5c106351 by adding the following to
>> > include/linux/pm_runtime.h:
>> >
>> > static inline bool pm_runtime_status_suspending(struct device *dev)
>> > {
>> > return dev->power.runtime_status == RPM_SUSPENDING;
>> > }
>> >
>> > static inline bool is_pm_work(struct device *dev)
>> > {
>> > struct work_struct *work = current_work();
>> >
>> > return work && work->func == dev->power.work;
>> > }
>> >
>> > Then adding this to nvkm_i2c_aux_acquire():
>> >
>> > struct device *dev = pad->i2c->subdev.device->dev;
>> >
>> > if (!(is_pm_work(dev) && pm_runtime_status_suspending(dev))) {
>> > ret = pm_runtime_get_sync(dev);
>> > if (ret < 0 && ret != -EACCES)
>> > return ret;
>> > }
> [snip]
>>
>> For the record, I don't quite like this approach as it seems to be
>> working around a broken dependency graph.
>>
>> If you need to resume device A from within the runtime resume callback
>> of device B, then clearly B depends on A and there should be a link
>> between them.
>>
>> That said, I do realize that it may be the path of least resistance,
>> but then I wonder if we can do better than this.
>
> The GPU contains an i2c subdevice for each connector with DDC lines.
> I believe those are modelled as children of the GPU's PCI device as
> they're accessed via mmio of the PCI device.
>
> The problem here is that when the GPU's PCI device runtime suspends,
> its i2c child device needs to be runtime active to suspend the MST
> topology.  Catch-22.

I see.

This sounds like a case for the ignore_children flag, maybe in a
slightly modified form, that will allow the parent to be suspended
regardless of the state of the children.

I wonder what happens to the I2C subdevices when the PCI device goes
into D3.  They are not accessible through MMIO any more then, so how
can they be suspended then?  Or do they need to be suspended at all?

> I don't know whether or not it's necessary to suspend the MST topology.
> I'm not an expert on DisplayPort MultiStream transport.

Me neither. :-)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] backlight: pwm_bl: Fix uninitialized variable

[Daniel Thompson]

On Wed, Jul 18, 2018 at 09:09:13AM +0100, Lee Jones wrote:
> On Mon, 16 Jul 2018, Daniel Thompson wrote:
> 
> > Currently, if the DT does not define num-interpolated-steps then
> > num_steps is undefined and the interpolation code will deploy randomly.
> > Fix this.
> > 
> > Fixes: 573fe6d1c25c ("backlight: pwm_bl: Linear interpolation between
> > brightness-levels")
> > Reported-by: Marcel Ziswiler 
> > Signed-off-by: Daniel Thompson 
> > Signed-off-by: Marcel Ziswiler 
> 
> This line is confusing.  Did you guys author this patch together?

Yes (although the manipulations were fairly mechanical).

> 
> My guess is that this line should be dropped and the RB and TB tags
> should remain?  If it was reviewed too, perhaps an AB too?
> 
> > Tested-by: Marcel Ziswiler 
> > ---
> >  drivers/video/backlight/pwm_bl.c | 17 -
> >  1 file changed, 8 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/video/backlight/pwm_bl.c 
> > b/drivers/video/backlight/pwm_bl.c
> > index 9ee4c1b735b2..e3c22b79fbcd 100644
> > --- a/drivers/video/backlight/pwm_bl.c
> > +++ b/drivers/video/backlight/pwm_bl.c
> > @@ -299,15 +299,14 @@ static int pwm_backlight_parse_dt(struct device *dev,
> >  * interpolation between each of the values of brightness levels
> >  * and creates a new pre-computed table.
> >  */
> > -   of_property_read_u32(node, "num-interpolated-steps",
> > -_steps);
> > -
> > -   /*
> > -* Make sure that there is at least two entries in the
> > -* brightness-levels table, otherwise we can't interpolate
> > -* between two points.
> > -*/
> > -   if (num_steps) {
> > +   if ((of_property_read_u32(node, "num-interpolated-steps",
> > + _steps) == 0) && num_steps) {
> 
> This is pretty ugly, and isn't it suffering from over-bracketing?  My
> suggestion would be to break out the invocation of
> of_property_read_u32() from the if and test only the result.
> 
>   of_property_read_u32(node, "num-interpolated-steps", 
> _steps);
>   if (!ret && num_steps) {
> 
> I haven't checked the underling code, but is it even feasible for
> of_property_read_u32() to not succeed AND for num_steps to be set?
> 
> If not, the check for !ret if superfluous and you can drop it.
> 
> > +   /*
> > +* Make sure that there is at least two entries in the
> 
> s/is/are/
> 
> > +* brightness-levels table, otherwise we can't
> > +* interpolate
> 
> Why break the line here?
> 
> > +* between two points.
> > +*/
> > if (data->max_brightness < 2) {
> > dev_err(dev, "can't interpolate\n");
> > return -EINVAL;
> 
> -- 
> Lee Jones [李琼斯]
> Linaro Services Technical Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Nouveau] [PATCH 1/5] drm/nouveau: Prevent RPM callback recursion in suspend/resume paths

[Lukas Wunner]

On Wed, Jul 18, 2018 at 09:38:41AM +0200, Rafael J. Wysocki wrote:
> On Tue, Jul 17, 2018 at 8:20 PM, Lukas Wunner  wrote:
> > Okay, the PCI device is suspending and the nvkm_i2c_aux_acquire()
> > wants it in resumed state, so is waiting forever for the device to
> > runtime suspend in order to resume it again immediately afterwards.
> >
> > The deadlock in the stack trace you've posted could be resolved using
> > the technique I used in d61a5c106351 by adding the following to
> > include/linux/pm_runtime.h:
> >
> > static inline bool pm_runtime_status_suspending(struct device *dev)
> > {
> > return dev->power.runtime_status == RPM_SUSPENDING;
> > }
> >
> > static inline bool is_pm_work(struct device *dev)
> > {
> > struct work_struct *work = current_work();
> >
> > return work && work->func == dev->power.work;
> > }
> >
> > Then adding this to nvkm_i2c_aux_acquire():
> >
> > struct device *dev = pad->i2c->subdev.device->dev;
> >
> > if (!(is_pm_work(dev) && pm_runtime_status_suspending(dev))) {
> > ret = pm_runtime_get_sync(dev);
> > if (ret < 0 && ret != -EACCES)
> > return ret;
> > }
[snip]
> 
> For the record, I don't quite like this approach as it seems to be
> working around a broken dependency graph.
> 
> If you need to resume device A from within the runtime resume callback
> of device B, then clearly B depends on A and there should be a link
> between them.
> 
> That said, I do realize that it may be the path of least resistance,
> but then I wonder if we can do better than this.

The GPU contains an i2c subdevice for each connector with DDC lines.
I believe those are modelled as children of the GPU's PCI device as
they're accessed via mmio of the PCI device.

The problem here is that when the GPU's PCI device runtime suspends,
its i2c child device needs to be runtime active to suspend the MST
topology.  Catch-22.

I don't know whether or not it's necessary to suspend the MST topology.
I'm not an expert on DisplayPort MultiStream transport.

BTW Lyude, in patch 4 and 5 of this series, you're runtime resuming
pad->i2c->subdev.device->dev.  Is this the PCI device or is it the i2c
device?  I'm always confused by nouveau's structs.  In nvkm_i2c_bus_ctor()
I can see that the device you're runtime resuming is the parent of the
i2c_adapter:

struct nvkm_device *device = pad->i2c->subdev.device;
[...]
bus->i2c.dev.parent = device->dev;

If the i2c_adapter is a child of the PCI device, it's sufficient
to runtime resume the i2c_adapter, i.e. bus->i2c.dev, and this will
implicitly runtime resume its parent.

Thanks,

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

Re: [PATCH 2/2] drm/sun4i: sun4i: Introduce a quirk for lowest plane alpha support

[Paul Kocialkowski]

Hi,

On Tue, 2018-07-17 at 14:25 +0200, Maxime Ripard wrote:
> On Tue, Jul 17, 2018 at 10:52:30AM +0200, Paul Kocialkowski wrote:
> > Not all sunxi platforms with the first version of the Display Engine
> > support an alpha component on the plane with the lowest z position
> > (as in: lowest z-pos), that gets blended with the background color.
> > 
> > In particular, the A13 is known to have this limitation. However, it was
> > recently discovered that the A20 and A33 are capable of having alpha on
> > their lowest plane.
> > 
> > Thus, this introduces a specific quirk to indicate such support,
> > per-platform. Since this was not tested on sun4i and sun6i platforms, a
> > conservative approach is kept and this feature is not supported.
> > 
> > Signed-off-by: Paul Kocialkowski 
> > ---
> >  drivers/gpu/drm/sun4i/sun4i_backend.c | 10 --
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c 
> > b/drivers/gpu/drm/sun4i/sun4i_backend.c
> > index a3cc398d4d80..cdc4a8a91ea2 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> > @@ -35,6 +35,8 @@
> >  struct sun4i_backend_quirks {
> > /* backend <-> TCON muxing selection done in backend */
> > bool needs_output_muxing;
> > +   /* alpha at the lowest z position is not always supported */
> > +   bool supports_lowest_plane_alpha;
> >  };
> >  
> >  static void sun4i_backend_apply_color_correction(struct sunxi_engine 
> > *engine)
> > @@ -484,6 +486,7 @@ static void sun4i_backend_atomic_begin(struct 
> > sunxi_engine *engine,
> >  static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
> >   struct drm_crtc_state *crtc_state)
> >  {
> > +   struct sun4i_backend *backend = engine_to_sun4i_backend(engine);
> > struct drm_plane_state *plane_states[SUN4I_BACKEND_NUM_LAYERS] = { 0 };
> 
> Your new variable should be here.

Ok, will do in v2.

> > struct drm_atomic_state *state = crtc_state->state;
> > struct drm_device *drm = state->dev;
> > @@ -584,8 +587,9 @@ static int sun4i_backend_atomic_check(struct 
> > sunxi_engine *engine,
> > }
> >  
> > /* We can't have an alpha plane at the lowest position */
> > -   if (plane_states[0]->fb->format->has_alpha ||
> > -   (plane_states[0]->alpha != DRM_BLEND_ALPHA_OPAQUE))
> > +   if ((plane_states[0]->fb->format->has_alpha ||
> > +   (plane_states[0]->alpha != DRM_BLEND_ALPHA_OPAQUE)) &&
> > +   !backend->quirks->supports_lowest_plane_alpha)
> > return -EINVAL;
> 
> This only partially does the job. This only allows to have an alpha
> plane at the lowest position, but the fact that the alpha works at the
> lowest position also means you can have two alpha planes now, and you
> didn't change that check.

You're right, the number of available planes with alpha has to be
changed accordingly. Will do in v2.

> The pipe allocation algorithm would also need to be checked.

From my understanding, the limitation on the number of alpha planes only
takes effect after pipe allocation, so this change doesn't require
modifying the allocation algorithm.

Cheers and thanks for the review,

Paul

-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com


signature.asc
Description: This is a digitally signed message part
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] console: Replace #if 1 with a bool to ignore

[Thomas Zimmermann]

Am 12.07.2018 um 15:29 schrieb Steven Rostedt:
> 
> From: Steven Rostedt (VMware) 
> 
> There's been discussion on the fb list about the addition of
> WARN_CONSOLE_UNLOCKED() inside the fb code. The complaint is that when
> the fb module is loaded with lockless_register_fb the console lock is
> not taken for debugging reasons. With the addition of
> WARN_CONSOLE_UNLOCK() within the fb code, this causes the console to
> fill up with warnings when trying to debug the fb driver.
> 
> There's also a #if 1 that enables the warning which was added before
> git history, and we look down on constant #if's in the kernel nowadays
> anyway.
> 
> Remove the #if 1 and add a ignore_console_lock_warning boolean that can
> be set by drivers to ignore the warning in order to do debugging.
> 
> Link: http://lkml.kernel.org/r/717e6337-e7a6-7a92-1c1b-8929a2569...@suse.de
> Reviewed-by: Hans de Goede 
> Acked-by: Sergey Senozhatsky 
> Signed-off-by: Steven Rostedt (VMware) 
> ---
> Changes since V1:
>  - Added comment to describe ignore_console_unlock_waring variable
>  - Abide by 80 character limit
> 
> Index: linux-trace.git/include/linux/console.h
> ===
> --- linux-trace.git.orig/include/linux/console.h
> +++ linux-trace.git/include/linux/console.h
> @@ -200,11 +200,14 @@ void vcs_make_sysfs(int index);
>  void vcs_remove_sysfs(int index);
>  
>  /* Some debug stub to catch some of the obvious races in the VT code */
> -#if 1
> -#define WARN_CONSOLE_UNLOCKED()  WARN_ON(!is_console_locked() && 
> !oops_in_progress)
> -#else
> -#define WARN_CONSOLE_UNLOCKED()
> -#endif
> +#define WARN_CONSOLE_UNLOCKED()  
> \
> + WARN_ON(!ignore_console_lock_warning && \
> + !is_console_locked() && !oops_in_progress)
> +/*
> + * Set ignore_console_lock_warning to true if you need to quiet
> + * WARN_CONSOLE_UNLOCKED() for debugging purposes.
> + */
> +extern bool ignore_console_lock_warning;
>  
>  /* VESA Blanking Levels */
>  #define VESA_NO_BLANKING0
> Index: linux-trace.git/kernel/printk/printk.c
> ===
> --- linux-trace.git.orig/kernel/printk/printk.c
> +++ linux-trace.git/kernel/printk/printk.c
> @@ -66,6 +66,9 @@ int console_printk[4] = {
>   CONSOLE_LOGLEVEL_DEFAULT,   /* default_console_loglevel */
>  };
>  
> +bool ignore_console_lock_warning __read_mostly;
> +EXPORT_SYMBOL(ignore_console_lock_warning);
> +
>  /*
>   * Low level drivers may need that to know if they can schedule in
>   * their unblank() callback or not. So let's export it.
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

This resolves the problem for me.

Tested-by: Thomas Zimmermann 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Linux GmbH, Maxfeldstr. 5, D-90409 Nürnberg
Tel: +49-911-74053-0; Fax: +49-911-7417755;  https://www.suse.com/
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard,
Graham Norton, HRB 21284 (AG Nürnberg)



signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] backlight: pwm_bl: Fix uninitialized variable

[Lee Jones]

On Wed, 18 Jul 2018, Lee Jones wrote:

> On Mon, 16 Jul 2018, Daniel Thompson wrote:
> 
> > Currently, if the DT does not define num-interpolated-steps then
> > num_steps is undefined and the interpolation code will deploy randomly.
> > Fix this.
> > 
> > Fixes: 573fe6d1c25c ("backlight: pwm_bl: Linear interpolation between
> > brightness-levels")
> > Reported-by: Marcel Ziswiler 
> > Signed-off-by: Daniel Thompson 
> > Signed-off-by: Marcel Ziswiler 
> 
> This line is confusing.  Did you guys author this patch together?
> 
> My guess is that this line should be dropped and the RB and TB tags
> should remain?  If it was reviewed too, perhaps an AB too?
> 
> > Tested-by: Marcel Ziswiler 
> > ---
> >  drivers/video/backlight/pwm_bl.c | 17 -
> >  1 file changed, 8 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/video/backlight/pwm_bl.c 
> > b/drivers/video/backlight/pwm_bl.c
> > index 9ee4c1b735b2..e3c22b79fbcd 100644
> > --- a/drivers/video/backlight/pwm_bl.c
> > +++ b/drivers/video/backlight/pwm_bl.c
> > @@ -299,15 +299,14 @@ static int pwm_backlight_parse_dt(struct device *dev,
> >  * interpolation between each of the values of brightness levels
> >  * and creates a new pre-computed table.
> >  */
> > -   of_property_read_u32(node, "num-interpolated-steps",
> > -_steps);
> > -
> > -   /*
> > -* Make sure that there is at least two entries in the
> > -* brightness-levels table, otherwise we can't interpolate
> > -* between two points.
> > -*/
> > -   if (num_steps) {
> > +   if ((of_property_read_u32(node, "num-interpolated-steps",
> > + _steps) == 0) && num_steps) {
> 
> This is pretty ugly, and isn't it suffering from over-bracketing?  My
> suggestion would be to break out the invocation of
> of_property_read_u32() from the if and test only the result.
> 
>   of_property_read_u32(node, "num-interpolated-steps", 
> _steps);
>   if (!ret && num_steps) {

Whoops!  I was playing around with the 80-char limit and forgot to
revert.  The lines should read:

ret = of_property_read_u32(node, "num-interpolated-steps",
   _steps);
if (!ret && num_steps) {

> I haven't checked the underling code, but is it even feasible for
> of_property_read_u32() to not succeed AND for num_steps to be set?
> 
> If not, the check for !ret if superfluous and you can drop it.
> 
> > +   /*
> > +* Make sure that there is at least two entries in the
> 
> s/is/are/
> 
> > +* brightness-levels table, otherwise we can't
> > +* interpolate
> 
> Why break the line here?
> 
> > +* between two points.
> > +*/
> > if (data->max_brightness < 2) {
> > dev_err(dev, "can't interpolate\n");
> > return -EINVAL;
> 

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] backlight: pwm_bl: Fix uninitialized variable

[Lee Jones]

On Mon, 16 Jul 2018, Daniel Thompson wrote:

> Currently, if the DT does not define num-interpolated-steps then
> num_steps is undefined and the interpolation code will deploy randomly.
> Fix this.
> 
> Fixes: 573fe6d1c25c ("backlight: pwm_bl: Linear interpolation between
> brightness-levels")
> Reported-by: Marcel Ziswiler 
> Signed-off-by: Daniel Thompson 
> Signed-off-by: Marcel Ziswiler 

This line is confusing.  Did you guys author this patch together?

My guess is that this line should be dropped and the RB and TB tags
should remain?  If it was reviewed too, perhaps an AB too?

> Tested-by: Marcel Ziswiler 
> ---
>  drivers/video/backlight/pwm_bl.c | 17 -
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/video/backlight/pwm_bl.c 
> b/drivers/video/backlight/pwm_bl.c
> index 9ee4c1b735b2..e3c22b79fbcd 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
> @@ -299,15 +299,14 @@ static int pwm_backlight_parse_dt(struct device *dev,
>* interpolation between each of the values of brightness levels
>* and creates a new pre-computed table.
>*/
> - of_property_read_u32(node, "num-interpolated-steps",
> -  _steps);
> -
> - /*
> -  * Make sure that there is at least two entries in the
> -  * brightness-levels table, otherwise we can't interpolate
> -  * between two points.
> -  */
> - if (num_steps) {
> + if ((of_property_read_u32(node, "num-interpolated-steps",
> +   _steps) == 0) && num_steps) {

This is pretty ugly, and isn't it suffering from over-bracketing?  My
suggestion would be to break out the invocation of
of_property_read_u32() from the if and test only the result.

of_property_read_u32(node, "num-interpolated-steps", 
_steps);
if (!ret && num_steps) {

I haven't checked the underling code, but is it even feasible for
of_property_read_u32() to not succeed AND for num_steps to be set?

If not, the check for !ret if superfluous and you can drop it.

> + /*
> +  * Make sure that there is at least two entries in the

s/is/are/

> +  * brightness-levels table, otherwise we can't
> +  * interpolate

Why break the line here?

> +  * between two points.
> +  */
>   if (data->max_brightness < 2) {
>   dev_err(dev, "can't interpolate\n");
>   return -EINVAL;

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [resend PATCH v4 4/5] drm/mediatek: Add support for mmsys through a pdev

[Laurent Pinchart]

Hi Sean,

On Wednesday, 18 July 2018 05:57:35 EEST Sean Wang wrote:
> On Wed, 2018-07-18 at 00:03 +0200, matthias@kernel.org wrote:
> > From: Matthias Brugger 
> > 
> > The MMSYS subsystem includes clocks and drm components.
> > This patch adds an initailization path through a platform device
> > for the clock part, so that both drivers get probed from the same
> > device tree compatible.
> 
> Sorry for that I should have a response earlier for the series.
> 
> Some points I felt they're not exactly right and should be fixed up
> before we're moving on
> 
> Currently, drm driver have a wrong reference to the dt-binding,
> "mediatek,mt2701-mmsys" or "mediatek,mt8173-mmsys", they should be all
> for the subsystem exporting clock and reset line such common resource to
> its sub-devices. Every subsystem has a similar shape. I hope mmsys
> shouldn't be an exception.
> 
> DRM device needs to have its own dt-binding show how connections between
> DRM components being made and its node should be put under mmsys node.
> 
> In this way, it becomes easy to see how the topology of the subsystem is
> and grows, like a tree "device tree", instead of hiding the details in
> the implementation.
> 
> The similar example we already did for audsys on mt2701 and mt7623 as
> below
> 
>   audsys: clock-controller@1122 {
>   
compatible = "mediatek,mt7623-audsys",
>   
 "mediatek,mt2701-audsys",
>   
 "syscon";
>   
...
> 
>   
afe: audio-controller {
>   
compatible = "mediatek,mt7623-audio",
>   

 "mediatek,mt2701-audio";
> 
>   
...
>   
};
>   };

This looks very strange to me. I'm not familiar with the hardware 
architecture, but a clock controller that includes an audio controller seems 
like a very weird design. It's usually the other way around, you have an audio 
controller, and it also contains hardware that produces clocks, and possibly 
handles miscellaneous audio-related routing tasks. I would thus expect the 
reverse in the device tree:

afe: audio-controller@1122 {

compatible = "mediatek,mt7623-audio",

 "mediatek,mt2701-audio";

..


audsys: clock-controller {

compatible = "mediatek,mt7623-audsys",


 "mediatek,mt2701-audsys",


 "syscon";

...

};
};

And if audsys only exposes clocks, you don't even need a subnode to represent 
it, the audio controller itself can be a clock provider.

afe: audio-controller@1122 {

compatible = "mediatek,mt7623-audio",

 "mediatek,mt2701-audio";

#clock-cells = <1>;

..
};

> > Signed-off-by: Matthias Brugger 
> > ---
> > 
> >  drivers/gpu/drm/mediatek/mtk_drm_drv.c | 18 ++
> >  drivers/gpu/drm/mediatek/mtk_drm_drv.h |  2 ++
> >  2 files changed, 20 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > b/drivers/gpu/drm/mediatek/mtk_drm_drv.c index dd249cf5121e..c946aea722e5
> > 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > @@ -173,6 +173,7 @@ static const struct mtk_mmsys_driver_data
> > mt2701_mmsys_driver_data = {> 
> > .ext_path = mt2701_mtk_ddp_ext,
> > .ext_len = 
ARRAY_SIZE(mt2701_mtk_ddp_ext),
> > .shadow_register = true,
> > 
> > +   .clk_drv_name = "clk-mt2701-mm",
> > 
> >  };
> >  
> >  static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = {
> > 
> > @@ -180,6 +181,7 @@ static const struct mtk_mmsys_driver_data
> > mt8173_mmsys_driver_data = {> 
> > .main_len = 
ARRAY_SIZE(mt8173_mtk_ddp_main),
> > .ext_path = mt8173_mtk_ddp_ext,
> > .ext_len = 
ARRAY_SIZE(mt8173_mtk_ddp_ext),
> > 
> > +   .clk_drv_name = "clk-mt8173-mm",
> > 
> >  };
> >  
> >  static int mtk_drm_kms_init(struct drm_device *drm)
> > 
> > @@ -411,6 +413,19 @@ static int mtk_drm_probe(struct platform_device
> > *pdev)
> > 
> > if (IS_ERR(private->config_regs))
> > 
> > 
return PTR_ERR(private->config_regs);
> > 
> > +   if (private->data->clk_drv_name) {
> > +   
private->clk_dev = platform_device_register_data(dev,
> > +   


private->data->clk_drv_name, -1,
> > +   


NULL, 0);
> > +
> > +   
if (IS_ERR(private->clk_dev)) {
> > +   
pr_err("failed to register %s platform 
device\n",
> > +   


private->data->clk_drv_name);
> > +
> > +   
return PTR_ERR(private->clk_dev);
> > +   
}
> > +   }
> > +
> > 
> > /* Iterate over sibling DISP 
function 

Re: [Nouveau] [PATCH 1/5] drm/nouveau: Prevent RPM callback recursion in suspend/resume paths

[Lukas Wunner]

On Tue, Jul 17, 2018 at 02:34:47PM -0400, Lyude Paul wrote:
> On Tue, 2018-07-17 at 20:32 +0200, Lukas Wunner wrote:
> > On Tue, Jul 17, 2018 at 02:24:31PM -0400, Lyude Paul wrote:
> > > On Tue, 2018-07-17 at 20:20 +0200, Lukas Wunner wrote:
> > > > Okay, the PCI device is suspending and the nvkm_i2c_aux_acquire()
> > > > wants it in resumed state, so is waiting forever for the device to
> > > > runtime suspend in order to resume it again immediately afterwards.
> > > > 
> > > > The deadlock in the stack trace you've posted could be resolved using
> > > > the technique I used in d61a5c106351 by adding the following to
> > > > include/linux/pm_runtime.h:
> > > > 
> > > > static inline bool pm_runtime_status_suspending(struct device *dev)
> > > > {
> > > > return dev->power.runtime_status == RPM_SUSPENDING;
> > > > }
> > > > 
> > > > static inline bool is_pm_work(struct device *dev)
> > > > {
> > > > struct work_struct *work = current_work();
> > > > 
> > > > return work && work->func == dev->power.work;
> > > > }
> > > > 
> > > > Then adding this to nvkm_i2c_aux_acquire():
> > > > 
> > > > struct device *dev = pad->i2c->subdev.device->dev;
> > > > 
> > > > if (!(is_pm_work(dev) && pm_runtime_status_suspending(dev))) {
> > > > ret = pm_runtime_get_sync(dev);
> > > > if (ret < 0 && ret != -EACCES)
> > > > return ret;
> > > > }
> > > > 
> > > > But here's the catch:  This only works for an *async* runtime suspend.
> > > > It doesn't work for pm_runtime_put_sync(), pm_runtime_suspend() etc,
> > > > because then the runtime suspend is executed in the context of the 
> > > > caller,
> > > > not in the context of dev->power.work.
[snip]
> 
> Something I'm curious about. This isn't the first time I've hit a
> situation like this (see: the improper disable_depth fix I added into
> amdgpu I now need to go and fix), which makes me wonder: is there
> actually any reason Linux's runtime PM core doesn't just turn get/puts()
> in the context of s/r callbacks into no-ops by default? 

So the PM core could save a pointer to the "current" task_struct
in struct device before invoking the ->runtime_suspend or
->runtime_resume callback, and all subsequent rpm_resume() and
rpm_suspend() calls could then become no-ops if "current" is
equivalent to the saved pointer.  (This is also how you could
solve the deadlock you're dealing with for sync suspend.)

For a recursive resume during a resume or a recursive suspend
during a suspend, this might actually be fine.

For a recursive suspend during a resume or a recursive resume
during a suspend, things become murkier:  How should the PM core
know if the particular part of the device is still accessible
when hitting a recursive resume during a suspend?  Let's say a
clock is needed for i2c.  Then the recursive resume during a
suspend may only become a no-op before that clock has been
turned off.  That's something only the device driver itself
has knowledge about, because it implements the order in which
subdevices of the GPU are turned off.

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

Re: [Nouveau] [PATCH 1/5] drm/nouveau: Prevent RPM callback recursion in suspend/resume paths

[Rafael J. Wysocki]

On Tue, Jul 17, 2018 at 8:34 PM, Lyude Paul  wrote:
> On Tue, 2018-07-17 at 20:32 +0200, Lukas Wunner wrote:
>> On Tue, Jul 17, 2018 at 02:24:31PM -0400, Lyude Paul wrote:
>> > On Tue, 2018-07-17 at 20:20 +0200, Lukas Wunner wrote:
>> > > Okay, the PCI device is suspending and the nvkm_i2c_aux_acquire()
>> > > wants it in resumed state, so is waiting forever for the device to
>> > > runtime suspend in order to resume it again immediately afterwards.
>> > >
>> > > The deadlock in the stack trace you've posted could be resolved using
>> > > the technique I used in d61a5c106351 by adding the following to
>> > > include/linux/pm_runtime.h:
>> > >
>> > > static inline bool pm_runtime_status_suspending(struct device *dev)
>> > > {
>> > >   return dev->power.runtime_status == RPM_SUSPENDING;
>> > > }
>> > >
>> > > static inline bool is_pm_work(struct device *dev)
>> > > {
>> > >   struct work_struct *work = current_work();
>> > >
>> > >   return work && work->func == dev->power.work;
>> > > }
>> > >
>> > > Then adding this to nvkm_i2c_aux_acquire():
>> > >
>> > >   struct device *dev = pad->i2c->subdev.device->dev;
>> > >
>> > >   if (!(is_pm_work(dev) && pm_runtime_status_suspending(dev))) {
>> > >   ret = pm_runtime_get_sync(dev);
>> > >   if (ret < 0 && ret != -EACCES)
>> > >   return ret;
>> > >   }
>> > >
>> > > But here's the catch:  This only works for an *async* runtime suspend.
>> > > It doesn't work for pm_runtime_put_sync(), pm_runtime_suspend() etc,
>> > > because then the runtime suspend is executed in the context of the 
>> > > caller,
>> > > not in the context of dev->power.work.
>> > >
>> > > So it's not a full solution, but hopefully something that gets you
>> > > going.  I'm not really familiar with the code paths leading to
>> > > nvkm_i2c_aux_acquire() to come up with a full solution off the top
>> > > of my head I'm afraid.
>> >
>> > OK-I was considering doing something similar to that commit beforehand but 
>> > I
>> > wasn't sure if I was going to just be hacking around an actual issue. That
>> > doesn't seem to be the case. This is very helpful and hopefully I should be
>> > able
>> > to figure something out from this, thanks!
>>
>> In some cases, the function acquiring the runtime PM ref is only called
>> from a couple of places and then it would be feasible and appropriate
>> to add a bool parameter to the function telling it to acquire the ref
>> or not.  So the function is told using a parameter which context it's
>> running in:  In the runtime_suspend code path or some other code path.
>>
>> The technique to use current_work() is an alternative approach to figure
>> out the context if passing in an additional parameter is not feasible
>> for some reason.  That was the case with d61a5c106351.  That approach
>> only works for work items though.
>
> Something I'm curious about. This isn't the first time I've hit a situation 
> like
> this (see: the improper disable_depth fix I added into amdgpu I now need to go
> and fix), which makes me wonder: is there actually any reason Linux's runtime 
> PM
> core doesn't just turn get/puts() in the context of s/r callbacks into no-ops 
> by
> default?

Because it's hard to detect reliably enough and because hiding issues
is a bad idea in general.

As I've just said in the message to Lukas, the fact that you need to
resume another device from within your resume callback indicates that
you're hiding your dependency graph from the core.

Thanks,
Rafael
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Nouveau] [PATCH 1/5] drm/nouveau: Prevent RPM callback recursion in suspend/resume paths

[Rafael J. Wysocki]

On Tue, Jul 17, 2018 at 8:20 PM, Lukas Wunner  wrote:
> On Tue, Jul 17, 2018 at 12:53:11PM -0400, Lyude Paul wrote:
>> On Tue, 2018-07-17 at 09:16 +0200, Lukas Wunner wrote:
>> > On Mon, Jul 16, 2018 at 07:59:25PM -0400, Lyude Paul wrote:
>> > > In order to fix all of the spots that need to have runtime PM get/puts()
>> > > added, we need to ensure that it's possible for us to call
>> > > pm_runtime_get/put() in any context, regardless of how deep, since
>> > > almost all of the spots that are currently missing refs can potentially
>> > > get called in the runtime suspend/resume path. Otherwise, we'll try to
>> > > resume the GPU as we're trying to resume the GPU (and vice-versa) and
>> > > cause the kernel to deadlock.
>> > >
>> > > With this, it should be safe to call the pm runtime functions in any
>> > > context in nouveau with one condition: any point in the driver that
>> > > calls pm_runtime_get*() cannot hold any locks owned by nouveau that
>> > > would be acquired anywhere inside nouveau_pmops_runtime_resume().
>> > > This includes modesetting locks, i2c bus locks, etc.
>> >
>> > [snip]
>> > > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
>> > > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
>> > > @@ -835,6 +835,8 @@ nouveau_pmops_runtime_suspend(struct device *dev)
>> > >   return -EBUSY;
>> > >   }
>> > >
>> > > + dev->power.disable_depth++;
>> > > +
>> >
>> > Anyway, if I understand the commit message correctly, you're hitting a
>> > pm_runtime_get_sync() in a code path that itself is called during a
>> > pm_runtime_get_sync().  Could you include stack traces in the commit
>> > message?  My gut feeling is that this patch masks a deeper issue,
>> > e.g. if the runtime_resume code path does in fact directly poll outputs,
>> > that would seem wrong.  Runtime resume should merely make the card
>> > accessible, i.e. reinstate power if necessary, put into PCI_D0,
>> > restore registers, etc.  Output polling should be scheduled
>> > asynchronously.
>>
>> So: the reason that patch was added was mainly for the patches later in the
>> series that add guards around the i2c bus and aux bus, since both of those
>> require that the device be awake for it to work. Currently, the spot where it
>> would recurse is:
>
> Okay, the PCI device is suspending and the nvkm_i2c_aux_acquire()
> wants it in resumed state, so is waiting forever for the device to
> runtime suspend in order to resume it again immediately afterwards.
>
> The deadlock in the stack trace you've posted could be resolved using
> the technique I used in d61a5c106351 by adding the following to
> include/linux/pm_runtime.h:
>
> static inline bool pm_runtime_status_suspending(struct device *dev)
> {
> return dev->power.runtime_status == RPM_SUSPENDING;
> }
>
> static inline bool is_pm_work(struct device *dev)
> {
> struct work_struct *work = current_work();
>
> return work && work->func == dev->power.work;
> }
>
> Then adding this to nvkm_i2c_aux_acquire():
>
> struct device *dev = pad->i2c->subdev.device->dev;
>
> if (!(is_pm_work(dev) && pm_runtime_status_suspending(dev))) {
> ret = pm_runtime_get_sync(dev);
> if (ret < 0 && ret != -EACCES)
> return ret;
> }
>
> But here's the catch:  This only works for an *async* runtime suspend.
> It doesn't work for pm_runtime_put_sync(), pm_runtime_suspend() etc,
> because then the runtime suspend is executed in the context of the caller,
> not in the context of dev->power.work.
>
> So it's not a full solution, but hopefully something that gets you
> going.  I'm not really familiar with the code paths leading to
> nvkm_i2c_aux_acquire() to come up with a full solution off the top
> of my head I'm afraid.
>
> Note, it's not sufficient to just check pm_runtime_status_suspending(dev)
> because if the runtime_suspend is carried out concurrently by something
> else, this will return true but it's not guaranteed that the device is
> actually kept awake until the i2c communication has been fully performed.

For the record, I don't quite like this approach as it seems to be
working around a broken dependency graph.

If you need to resume device A from within the runtime resume callback
of device B, then clearly B depends on A and there should be a link
between them.

That said, I do realize that it may be the path of least resistance,
but then I wonder if we can do better than this.

Thanks,
Rafael
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 200531] amdgpu: *ERROR* REG_WAIT timeout when a display is put to sleep

[bugzilla-daemon]

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

--- Comment #3 from Alexander Mezin (mezin.alexan...@gmail.com) ---
Created attachment 277393
  --> https://bugzilla.kernel.org/attachment.cgi?id=277393=edit
dmesg with BUG: unable to handle kernel NULL pointer dereference

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 200531] amdgpu: *ERROR* REG_WAIT timeout when a display is put to sleep

[bugzilla-daemon]

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

--- Comment #2 from Alexander Mezin (mezin.alexan...@gmail.com) ---
Also got "BUG: unable to handle kernel NULL pointer dereference". Happened when
waking the display from suspend (the system itself wasn't suspended, because of
https://bugzilla.kernel.org/show_bug.cgi?id=199959).

Seems that GUI becomes unresponsive after this, not after "REG_WAIT timeout".

Should I report this as a different bug?

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [resend PATCH v4 5/5] drm: mediatek: Omit warning on probe defers

[Sean Wang]

On Wed, 2018-07-18 at 00:03 +0200, matthias@kernel.org wrote:
> From: Matthias Brugger 
> 
> It can happen that the clock drivers wasn't probed before the
> ddp driver gets invoked. The driver used to omit a warning
> that the driver failed to get the clocks. Omit this error on
> the defered probe path.
> 

so if we list drm device as a child device of mmsys we don't need the
patch, and that make more sense.

> Signed-off-by: Matthias Brugger 
> Acked-by: CK Hu 
> ---
>  drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c 
> b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> index bafc5c77c4fb..6b399348a2dc 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> @@ -374,7 +374,8 @@ static int mtk_ddp_probe(struct platform_device *pdev)
>  
>   ddp->clk = devm_clk_get(dev, NULL);
>   if (IS_ERR(ddp->clk)) {
> - dev_err(dev, "Failed to get clock\n");
> + if (PTR_ERR(ddp->clk) != -EPROBE_DEFER)
> + dev_err(dev, "Failed to get clock\n");
>   return PTR_ERR(ddp->clk);
>   }
>  


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

[PATCH] drm/etnaviv: try harder to dump core

[Russell King]

Try a bit harder to produce a devcoredump when things go wrong.  If we
fail to allocate the memory for a dump including the actual bo contents,
omit the bos and try again.  Capturing some information from the GPU is
better than having no information.

Signed-off-by: Russell King 
---
 drivers/gpu/drm/etnaviv/etnaviv_dump.c | 30 ++
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_dump.c 
b/drivers/gpu/drm/etnaviv/etnaviv_dump.c
index 48aef6cf6a42..c9987bc38acc 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_dump.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_dump.c
@@ -129,6 +129,7 @@ void etnaviv_core_dump(struct etnaviv_gpu *gpu)
unsigned int n_obj, n_bomap_pages;
size_t file_size, mmu_size;
__le64 *bomap, *bomap_start;
+   bool bos = true;
 
/* Only catch the first event, or when manually re-armed */
if (!etnaviv_dump_core)
@@ -137,6 +138,7 @@ void etnaviv_core_dump(struct etnaviv_gpu *gpu)
 
mmu_size = etnaviv_iommu_dump_size(gpu->mmu);
 
+again:
/* We always dump registers, mmu, ring and end marker */
n_obj = 4;
n_bomap_pages = 0;
@@ -159,9 +161,12 @@ void etnaviv_core_dump(struct etnaviv_gpu *gpu)
continue;
 
obj = vram->object;
-   file_size += obj->base.size;
n_bomap_pages += obj->base.size >> PAGE_SHIFT;
-   n_obj++;
+
+   if (bos) {
+   file_size += obj->base.size;
+   n_obj++;
+   }
}
 
/* If we have any buffer objects, add a bomap object */
@@ -177,6 +182,11 @@ void etnaviv_core_dump(struct etnaviv_gpu *gpu)
iter.start = __vmalloc(file_size, GFP_KERNEL | __GFP_NOWARN | 
__GFP_NORETRY,
   PAGE_KERNEL);
if (!iter.start) {
+   if (bos) {
+   dev_warn(gpu->dev, "devcoredump too big, trying without 
bos\n");
+   bos = false;
+   goto again;
+   }
dev_warn(gpu->dev, "failed to allocate devcoredump file\n");
return;
}
@@ -234,14 +244,18 @@ void etnaviv_core_dump(struct etnaviv_gpu *gpu)
*bomap++ = cpu_to_le64(page_to_phys(*pages++));
}
 
-   iter.hdr->iova = cpu_to_le64(vram->iova);
+   if (bos) {
+   iter.hdr->iova = cpu_to_le64(vram->iova);
 
-   vaddr = etnaviv_gem_vmap(>base);
-   if (vaddr)
-   memcpy(iter.data, vaddr, obj->base.size);
+   vaddr = etnaviv_gem_vmap(>base);
+   dev_crit(gpu->dev, "Copying object %p ops %pS vaddr %p 
to %p size %zu\n",
+   obj, obj->ops, vaddr, iter.data, 
obj->base.size);
+   if (vaddr)
+   memcpy(iter.data, vaddr, obj->base.size);
 
-   etnaviv_core_dump_header(, ETDUMP_BUF_BO, iter.data +
-obj->base.size);
+   etnaviv_core_dump_header(, ETDUMP_BUF_BO, 
iter.data +
+obj->base.size);
+   }
}
 
etnaviv_core_dump_header(, ETDUMP_BUF_END, iter.data);
-- 
2.7.4

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

[PATCH v5 09/11] media: vsp1: Provide support for extended command pools

[Kieran Bingham]

From: Kieran Bingham 

VSPD and VSP-DL devices can provide extended display lists supporting
extended command display list objects.

These extended commands require their own dma memory areas for a header
and body specific to the command type.

Implement a command pool to allocate all necessary memory in a single
DMA allocation to reduce pressure on the TLB, and provide convenient
re-usable command objects for the entities to utilise.

Signed-off-by: Kieran Bingham 

---
v2:
 - Fix spelling typo in commit message
 - constify, and staticify the instantiation of vsp1_extended_commands
 - s/autfld_cmds/autofld_cmds/
 - staticify cmd pool functions (Thanks kbuild-bot)

v5:
 - Rename vsp1_dl_ext_cmd_header -> vsp1_pre_ext_dl_body
 - fixup vsp1_cmd_pool structure documentation
 - Rename dlm->autofld_cmds dlm->cmdpool
 - Separate out the instatiation of vsp1_extended_commands
 - Move initialisation of lock, and lists in vsp1_dl_cmd_pool_create to
   immediately after allocation
 - simplify vsp1_dlm_get_autofld_cmd
 - Rename vsp1_dl_get_autofld_cmd() to vsp1_dl_get_pre_cmd() and moved
   to "Display List Extended Command Management" section of vsp1_dl

 drivers/media/platform/vsp1/vsp1_dl.c | 194 +++-
 drivers/media/platform/vsp1/vsp1_dl.h |   3 +-
 2 files changed, 197 insertions(+)

diff --git a/drivers/media/platform/vsp1/vsp1_dl.c 
b/drivers/media/platform/vsp1/vsp1_dl.c
index 2fffe977aa35..d5b3c24d160c 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -141,6 +141,30 @@ struct vsp1_dl_body_pool {
 };
 
 /**
+ * struct vsp1_cmd_pool - Display List commands pool
+ * @dma: DMA address of the entries
+ * @size: size of the full DMA memory pool in bytes
+ * @mem: CPU memory pointer for the pool
+ * @cmds: Array of command structures for the pool
+ * @free: Free pool entries
+ * @lock: Protects the free list
+ * @vsp1: the VSP1 device
+ */
+struct vsp1_dl_cmd_pool {
+   /* DMA allocation */
+   dma_addr_t dma;
+   size_t size;
+   void *mem;
+
+   struct vsp1_dl_ext_cmd *cmds;
+   struct list_head free;
+
+   spinlock_t lock;
+
+   struct vsp1_device *vsp1;
+};
+
+/**
  * struct vsp1_dl_list - Display list
  * @list: entry in the display list manager lists
  * @dlm: the display list manager
@@ -186,6 +210,7 @@ struct vsp1_dl_list {
  * @queued: list queued to the hardware (written to the DL registers)
  * @pending: list waiting to be queued to the hardware
  * @pool: body pool for the display list bodies
+ * @autofld_cmds: command pool to support auto-fld interlaced mode
  */
 struct vsp1_dl_manager {
unsigned int index;
@@ -199,6 +224,7 @@ struct vsp1_dl_manager {
struct vsp1_dl_list *pending;
 
struct vsp1_dl_body_pool *pool;
+   struct vsp1_dl_cmd_pool *cmdpool;
 };
 
 /* 
-
@@ -362,6 +388,157 @@ void vsp1_dl_body_write(struct vsp1_dl_body *dlb, u32 
reg, u32 data)
 }
 
 /* 
-
+ * Display List Extended Command Management
+ */
+
+enum vsp1_extcmd_type {
+   VSP1_EXTCMD_AUTODISP,
+   VSP1_EXTCMD_AUTOFLD,
+};
+
+struct vsp1_extended_command_info {
+   u16 opcode;
+   size_t body_size;
+};
+
+static const struct vsp1_extended_command_info vsp1_extended_commands[] = {
+   [VSP1_EXTCMD_AUTODISP] = { 0x02, 96 },
+   [VSP1_EXTCMD_AUTOFLD]  = { 0x03, 160 },
+};
+
+/**
+ * vsp1_dl_cmd_pool_create - Create a pool of commands from a single allocation
+ * @vsp1: The VSP1 device
+ * @type: The command pool type
+ * @num_cmds: The number of commands to allocate
+ *
+ * Allocate a pool of commands each with enough memory to contain the private
+ * data of each command. The allocation sizes are dependent upon the command
+ * type.
+ *
+ * Return a pointer to the pool on success or NULL if memory can't be 
allocated.
+ */
+static struct vsp1_dl_cmd_pool *
+vsp1_dl_cmd_pool_create(struct vsp1_device *vsp1, enum vsp1_extcmd_type type,
+   unsigned int num_cmds)
+{
+   struct vsp1_dl_cmd_pool *pool;
+   unsigned int i;
+   size_t cmd_size;
+
+   pool = kzalloc(sizeof(*pool), GFP_KERNEL);
+   if (!pool)
+   return NULL;
+
+   spin_lock_init(>lock);
+   INIT_LIST_HEAD(>free);
+
+   pool->cmds = kcalloc(num_cmds, sizeof(*pool->cmds), GFP_KERNEL);
+   if (!pool->cmds) {
+   kfree(pool);
+   return NULL;
+   }
+
+   cmd_size = sizeof(struct vsp1_pre_ext_dl_body) +
+  vsp1_extended_commands[type].body_size;
+   cmd_size = ALIGN(cmd_size, 16);
+
+   pool->size = cmd_size * num_cmds;
+   pool->mem = dma_alloc_wc(vsp1->bus_master, pool->size, >dma,
+GFP_KERNEL);
+   if (!pool->mem) {
+   kfree(pool->cmds);
+   kfree(pool);
+   return NULL;
+