[PATCH] drm/rcar-du: Replace drm_dev_unref with drm_dev_put

2018-07-17 Thread Thomas Zimmermann
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 
---
 drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c 
b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index 02aee6cb0e53..9696a3030319 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -407,7 +407,7 @@ static int rcar_du_remove(struct platform_device *pdev)
drm_kms_helper_poll_fini(ddev);
drm_mode_config_cleanup(ddev);
 
-   drm_dev_unref(ddev);
+   drm_dev_put(ddev);
 
return 0;
 }
-- 
2.18.0

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


[PATCH] drm/shmobile: Replace drm_dev_unref with drm_dev_put

2018-07-17 Thread Thomas Zimmermann
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 
---
 drivers/gpu/drm/shmobile/shmob_drm_drv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c 
b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
index 592572554eb0..8d1ff596c774 100644
--- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
+++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
@@ -198,7 +198,7 @@ static int shmob_drm_remove(struct platform_device *pdev)
drm_kms_helper_poll_fini(ddev);
drm_mode_config_cleanup(ddev);
drm_irq_uninstall(ddev);
-   drm_dev_unref(ddev);
+   drm_dev_put(ddev);
 
return 0;
 }
@@ -294,7 +294,7 @@ static int shmob_drm_probe(struct platform_device *pdev)
drm_kms_helper_poll_fini(ddev);
drm_mode_config_cleanup(ddev);
 err_free_drm_dev:
-   drm_dev_unref(ddev);
+   drm_dev_put(ddev);
 
return ret;
 }
-- 
2.18.0

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


[PATCH] drm/tve200: Replace drm_dev_unref with drm_dev_put

2018-07-17 Thread Thomas Zimmermann
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 
---
 drivers/gpu/drm/tve200/tve200_drv.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/tve200/tve200_drv.c 
b/drivers/gpu/drm/tve200/tve200_drv.c
index ac344ddb23bc..18e9c8e709f9 100644
--- a/drivers/gpu/drm/tve200/tve200_drv.c
+++ b/drivers/gpu/drm/tve200/tve200_drv.c
@@ -198,12 +198,12 @@ static int tve200_probe(struct platform_device *pdev)
if (IS_ERR(priv->pclk)) {
dev_err(dev, "unable to get PCLK\n");
ret = PTR_ERR(priv->pclk);
-   goto dev_unref;
+   goto dev_put;
}
ret = clk_prepare_enable(priv->pclk);
if (ret) {
dev_err(dev, "failed to enable PCLK\n");
-   goto dev_unref;
+   goto dev_put;
}
 
/* This clock is for the pixels (27MHz) */
@@ -249,8 +249,8 @@ static int tve200_probe(struct platform_device *pdev)
 
 clk_disable:
clk_disable_unprepare(priv->pclk);
-dev_unref:
-   drm_dev_unref(drm);
+dev_put:
+   drm_dev_put(drm);
return ret;
 }
 
@@ -265,7 +265,7 @@ static int tve200_remove(struct platform_device *pdev)
drm_panel_bridge_remove(priv->bridge);
drm_mode_config_cleanup(drm);
clk_disable_unprepare(priv->pclk);
-   drm_dev_unref(drm);
+   drm_dev_put(drm);
 
return 0;
 }
-- 
2.18.0

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


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

2018-07-17 Thread 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 
---
 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 };
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;
 
for (i = 1; i < num_planes; i++) {
@@ -970,9 +974,11 @@ static const struct sun4i_backend_quirks 
sun6i_backend_quirks = {
 
 static const struct sun4i_backend_quirks sun7i_backend_quirks = {
.needs_output_muxing = true,
+   .supports_lowest_plane_alpha = true,
 };
 
 static const struct sun4i_backend_quirks sun8i_a33_backend_quirks = {
+   .supports_lowest_plane_alpha = true,
 };
 
 static const struct sun4i_backend_quirks sun9i_backend_quirks = {
-- 
2.17.1

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


[PATCH 1/2] drm/sun4i: sun4i: Register quirks with the backend structure

2018-07-17 Thread Paul Kocialkowski
In prevision for introducing a new quirk that will be used at atomic
plane check time, register the quirks structure with the backend
structure. This way, it can easily be grabbed where needed.

Signed-off-by: Paul Kocialkowski 
---
 drivers/gpu/drm/sun4i/sun4i_backend.c | 2 ++
 drivers/gpu/drm/sun4i/sun4i_backend.h | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c 
b/drivers/gpu/drm/sun4i/sun4i_backend.c
index 17dca4df00f2..a3cc398d4d80 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -909,6 +909,8 @@ static int sun4i_backend_bind(struct device *dev, struct 
device *master,
: SUN4I_BACKEND_MODCTL_OUT_LCD0));
}
 
+   backend->quirks = quirks;
+
return 0;
 
 err_disable_ram_clk:
diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.h 
b/drivers/gpu/drm/sun4i/sun4i_backend.h
index e33be2307c67..71de6147b483 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.h
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.h
@@ -187,6 +187,8 @@ struct sun4i_backend {
/* Protects against races in the frontend teardown */
spinlock_t  frontend_lock;
boolfrontend_teardown;
+
+   const struct sun4i_backend_quirks   *quirks;
 };
 
 static inline struct sun4i_backend *
-- 
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

2018-07-17 Thread Christian König

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'};
>
> -   memcpy(buf_cpy, buf, count+1);
> -   tmp = buf_cpy;
> -   while (tmp[0]) {
> -   sub_str =  strsep(, delimiter);
> -   if (strlen(sub_str)) {
> -   ret = kstrtol(sub_str, 0, );
> +   ret = amdgpu_read_mask(buf, count, );
> +   if (ret)
> +   return ret;
>
> -   if (ret) {
> -   count = -EINVAL;
> -   goto fail;
> -   }
> -   mask |= 1 << level;
> -   } 

Re: [Intel-gfx] [PATCH 3/6] drm/i915: Move out non-modeset calls from modeset init and cleanup

2018-07-17 Thread Chris Wilson
Quoting José Roberto de Souza (2018-07-16 23:38:38)
> @@ -1395,9 +1379,22 @@ int i915_driver_load(struct pci_dev *pdev, const 
> struct pci_device_id *ent)
> goto out_cleanup_hw;
> }
>  
> +   ret = intel_irq_install(dev_priv);
> +   if (ret)
> +   goto out_cleanup_hw;
> +
> +   /* i915_gem_init() call chain will call
> +* intel_display_power_put(i915, POWER_DOMAIN_GT_IRQ);
> +*/
> +   intel_power_domains_init_hw(dev_priv, false);
> +
> +   ret = i915_gem_init(dev_priv);
> +   if (ret)
> +   goto cleanup_irq;
> +
> ret = i915_load_modeset_init(_priv->drm);
> if (ret < 0)
> -   goto out_cleanup_hw;
> +   goto cleanup_gem;

Bzzt. Order is extremely important. e.g. modeset init needs to reserve
portions of the GTT still in use by the BIOS before we wipe it.
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [Nouveau] [PATCH v2 2/3] drm/nouveau: Fix runtime PM leak in nv50_disp_atomic_commit()

2018-07-17 Thread Lukas Wunner
On Thu, Jul 12, 2018 at 01:02:53PM -0400, Lyude Paul wrote:
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -1878,7 +1878,7 @@ nv50_disp_atomic_commit(struct drm_device *dev,
>   nv50_disp_atomic_commit_tail(state);
>  
>   drm_for_each_crtc(crtc, dev) {
> - if (crtc->state->enable) {
> + if (crtc->state->active) {
>   if (!drm->have_disp_power_ref) {
>   drm->have_disp_power_ref = true;
>   return 0;

Somewhat tangential comment on this older patch, since you
continue to dig around in the runtime PM area:

Whenever a crtc is activated or deactivated in nouveau, we iterate
over all crtcs and acquire a runtime PM if a crtc is active and
previously there was no active one, or we drop a ref if none is
active and previously there was an active one.

For a while now I've been thinking that it would be more straightforward
to acquire a ref whenever a crtc is activated and drop one when a crtc
is deactivated, i.e. hold one ref for every active crtc.  That way the
have_disp_power_ref variable as well as the iteration logic could be
removed, leading to a simplification.  Just a suggestion anyway.

Thanks,

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

2018-07-17 Thread Rafael J. Wysocki
On Tue, Jul 17, 2018 at 9:16 AM, Lukas Wunner  wrote:
> [cc += linux-pm]
>
> Hi Lyude,
>
> First of all, thanks a lot for looking into this.
>
> 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++;

This is effectively equivalent to __pm_runtime_disable(dev, false)
except for the locking (which is necessary).

>> +
>
> I'm not sure if that variable is actually private to the PM core.
> Grepping through the tree I only find a single occurrence where it's
> accessed outside the PM core and that's in amdgpu.  So this looks
> a little fishy TBH.  It may make sense to cc such patches to linux-pm
> to get Rafael & other folks involved with the PM core to comment.

You are right, power.disable_depth is internal to the PM core.
Accessing it (and updating it in particular) directly from drivers is
not a good idea.

> Also, the disable_depth variable only exists if the kernel was
> compiled with CONFIG_PM enabled, but I can't find a "depends on PM"
> or something like that in nouveau's Kconfig.  Actually, if PM is
> not selected, all the nouveau_pmops_*() functions should be #ifdef'ed
> away, but oddly there's no #ifdef CONFIG_PM anywhere in nouveau_drm.c.
>
> Anywayn, 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.

Right.

Thanks,
Rafael
___
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

2018-07-17 Thread Linus Walleij
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.

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.

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.

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.

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 indicated memory area.
It works for them.

In QEMU, we had the same problem. It didn't support proper reporting
of fake EDID on these platforms either, because of not properly
modeling the hardware it was emulating, instead relying on the register
reads as above. Since it is open source I could
simply fix it:
https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg04651.html
https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg04652.html
https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg04653.html

After this, the QEMU for Versatile Express can properly use the
bridge.

I do try to be gritty and thorough! :D

I don't really know what RTSM is and I can't run it myself. I just
have to support it in the refactoring to use DRM for the ARM
Versatile Express machines. I cannot change its source code.

> There's also some other cases of
> "transparent" bridges which don't have any driver.

Yeah I think they mostly use panel-simple.c in the DRM
case don't they? We come full circle.

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


Re: [PATCH v2] ARM: dts: imx6sl: Add vivante gpu nodes

2018-07-17 Thread Shawn Guo
On Fri, Jul 13, 2018 at 12:39:35PM +0300, Leonard Crestez wrote:
> The imx6sl soc has gpu_2d and gpu_vg, no 3d support:
> 
> etnaviv-gpu 220.gpu: model: GC320, revision: 5007
> etnaviv-gpu 2204000.gpu: model: GC355, revision: 1215
> 
> The IP blocks seem to be already supported.
> 
> Signed-off-by: Leonard Crestez 
> Reviewed-by: Lucas Stach 

Applied, thanks.
___
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

2018-07-17 Thread 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'};
>
> -   memcpy(buf_cpy, buf, count+1);
> -   tmp = buf_cpy;
> -   while (tmp[0]) {
> -   sub_str =  strsep(, delimiter);
> -   if (strlen(sub_str)) {
> -   ret = kstrtol(sub_str, 0, );
> +   ret = amdgpu_read_mask(buf, count, );
> +   if (ret)
> +   return ret;
>
> -   if (ret) {
> -   count = -EINVAL;
> -   goto fail;
> -   }
> -   mask |= 1 << level;
> -   } else
> -   break;
> -   }
> if (adev->powerplay.pp_funcs->force_clock_level)
> amdgpu_dpm_force_clock_level(adev, PP_MCLK, mask);
>
> -fail:
> return count;
>  }
>
> @@ -701,33 +703,15 @@ static ssize_t amdgpu_set_pp_dpm_pcie(struct device 
> *dev,
> struct drm_device *ddev = 

[Bug 105760] [4.17-rc1] RIP: smu7_populate_single_firmware_entry.isra.6+0x57/0xc0 [amdgpu] RSP: ffffa17901efb930

2018-07-17 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=105760

--- Comment #50 from Thomas Martitz  ---
Created attachment 140660
  --> https://bugs.freedesktop.org/attachment.cgi?id=140660=edit
dmesg with force_asic_init.diff + 0001-workaround-v2.patch

Doesn't seem to make a difference.


[  255.418659] [drm:gfx_v8_0_ring_test_ring] *ERROR* amdgpu: ring 0 test failed
(scratch(0xC040)=0xCAFEDEAD)
[  255.418670] [drm:amdgpu_device_ip_resume_phase2] *ERROR* resume of IP block
 failed -22
[  255.418675] [drm:amdgpu_device_resume] *ERROR* amdgpu_device_ip_resume
failed (-22).

-- 
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: [Nouveau] [PATCH 2/5] drm/nouveau: Grab RPM ref while probing outputs

2018-07-17 Thread Lukas Wunner
On Mon, Jul 16, 2018 at 07:59:26PM -0400, Lyude Paul wrote:
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -2012,10 +2012,18 @@ nv50_disp_atomic_state_alloc(struct drm_device *dev)
>   return >state;
>  }
>  
> +static void
> +nouveau_output_poll_changed(struct drm_device *dev)
> +{
> + pm_runtime_get_sync(dev->dev);
> + drm_fb_helper_hotplug_event(dev->fb_helper);
> + pm_runtime_put_autosuspend(dev->dev);
> +}
> +
>  static const struct drm_mode_config_funcs
>  nv50_disp_func = {
>   .fb_create = nouveau_user_framebuffer_create,
> - .output_poll_changed = drm_fb_helper_output_poll_changed,
> + .output_poll_changed = nouveau_output_poll_changed,

It might make sense to provide a generic DRM helper for this.
Same for patch 3 in this series.

Thanks,

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


Re: [Intel-gfx] [PATCH 1/6] drm: Let userspace check if driver supports modeset

2018-07-17 Thread Chris Wilson
Quoting José Roberto de Souza (2018-07-16 23:38:36)
> GPU accelerators usually don't have display block or the display
> driver part can be disable when building driver(for servers it save
> some resources) so it is important let userspace check this
> capability too.

We currently communicate that by having no modeset resources. What does
another cap bit accomplish that we don't already know?
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v14 2/2] dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings

2018-07-17 Thread Stephen Boyd
Quoting Sandeep Panda (2018-07-16 01:43:30)
> Document the bindings used for the sn65dsi86 DSI to eDP bridge.
> 
> Changes in v1:
>  - Rephrase the dt-binding descriptions to be more inline with existing
>bindings (Andrzej Hajda).
>  - Add missing dt-binding that are parsed by corresponding driver
>(Andrzej Hajda).
> 
> Changes in v2:
>  - Remove edp panel specific dt-binding entries. Only keep bridge
>specific entries (Sean Paul).
>  - Remove custom-modes dt entry since its usage is removed from driver also 
> (Sean Paul).
>  - Remove is-pluggable dt entry since this will not be needed anymore (Sean 
> Paul).
> 
> Changes in v3:
>  - Remove irq-gpio dt entry and instead populate is an interrupt
>property (Rob Herring).
> 
> Changes in v4:
>  - Add link to bridge chip datasheet (Stephen Boyd)
>  - Add vpll and vcc regulator supply bindings (Stephen Boyd)
>  - Add ref clk optional dt binding (Stephen Boyd)
>  - Add gpio-controller optional dt binding (Stephen Boyd)
> 
> Changes in v5:
>  - Use clock property to specify the input refclk (Stephen Boyd).
>  - Update gpio cell and pwm cell numbers (Stephen Boyd).
> 
> Changes in v6:
>  - Add property to mention the lane mapping scheme and polarity inversion
>(Stephen Boyd).
> 
> Changes in v7:
>  - Detail description of lane mapping scheme dt property (Andrzej
>Hajda/ Rob Herring).
>  - Removed HDP gpio binding, since the bridge uses IRQ signal to
>determine HPD, and IRQ property is already documented in binding.
> 
> Changes in v8:
>  - Removed unnecessary explanation of lane mapping and polarity dt
>property, since these are already explained in media/video-interface
>dt binidng (Rob Herring).
> 
> Changes in v9:
>  - Avoid putting re-definition of lane mapping and polarity dt binding
>(Rob Herring).
> 
> Changes in v10:
>  - Use interrupts-extended property instead of interrupts to specify
>interrupt line (Andrzej Hajda).
>  - Move data-lanes and lane-polarity property example to proper place 
> (Andrzej Hajda).
> 
> Changes in v11:
>  - Add a property for suspend gpio function of GPIO1 pin on bridge chip
>(Stephen Boyd).
> 
> Changes in v12:
>  - Remove binding for dedicated DDC line (Andrzej Hajda).
> 
> Signed-off-by: Sandeep Panda 
> ---

Reviewed-by: Stephen Boyd 

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


Re: [PATCH RESEND] drm/meson: Make DMT timings parameters and pixel clock generic

2018-07-17 Thread Jerome Brunet
On Mon, 2018-07-16 at 09:40 +0200, Neil Armstrong wrote:
> Remove the modes timings tables for DMT modes and calculate the HW
> paremeters from the modes timings.
> 
> Switch the DMT modes pixel clock calculation out of the static frequency
> list to a generic calculation from a range of possible PLL dividers.
> 
> This patch is an intermediate step towards usage of the Common Clock
> Framwework for PLL setup, by reworking the code to have common
> sel_pll() function called by the CEA (HDMI) freq setup and the generic
> DMT frequencies setup, we should be able to simply call clk_set_rate()
> on the PLL clock handle in a near future.
> 
> The CEA (HDMI) and CVBS modes needs very specific clock paths that CCF will
> never be able to determine by itself, so there is still some work to do for
> a full handoff to CCF handling the clocks.

Patch seems to be a good step forward making the display compatible with CCF
indeed. While full automatic handling through CCF might not possible, it would
be good if, someday,  we could handle the SoC quirks in CCF, removing the need
check is the SoC is gxbb, gxl or gxm while setting the clocks.

If the display driver needs a detailed control over the clock setup, maybe we
could solve the problem by exporting the intermediate clock elements in CCF
(such as muxes, ODs, etc...) and let the display driver claim them all ?

Anyway, the situation is improving so:
Acked-by: Jerome Brunet 

> 
> This setup permits setting non-CEA modes like :
> - 1600x900-60Hz
> - 1280x1024-75Hz
> - 1280x1024-60Hz
> - 1440x900-60Hz
> - 1366x768-60Hz
> - 1280x800-60Hz
> - 1152x864-75Hz
> - 1024x768-75Hz
> - 1024x768-70Hz
> - 1024x768-60Hz
> - 832x624-75Hz
> - 800x600-75Hz
> - 800x600-72Hz
> - 800x600-60Hz
> - 640x480-75Hz
> - 640x480-73Hz
> - 640x480-67Hz
> 
> Signed-off-by: Neil Armstrong 
> ---
>  drivers/gpu/drm/meson/meson_dw_hdmi.c |  22 +-
>  drivers/gpu/drm/meson/meson_vclk.c| 672 
> +++---
>  drivers/gpu/drm/meson/meson_vclk.h|   4 +
>  drivers/gpu/drm/meson/meson_venc.c| 377 +++
>  drivers/gpu/drm/meson/meson_venc.h|   3 +-
>  5 files changed, 358 insertions(+), 720 deletions(-)
> 
> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c 
> b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> index c9ad456..df7247c 100644
> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> @@ -329,6 +329,12 @@ static void dw_hdmi_set_vclk(struct meson_dw_hdmi 
> *dw_hdmi,
>  
>   vclk_freq = mode->clock;
>  
> + if (!vic) {
> + meson_vclk_setup(priv, MESON_VCLK_TARGET_DMT, vclk_freq,
> +  vclk_freq, vclk_freq, false);
> + return;
> + }
> +
>   if (mode->flags & DRM_MODE_FLAG_DBLCLK)
>   vclk_freq *= 2;
>  
> @@ -542,10 +548,12 @@ static enum drm_mode_status
>  dw_hdmi_mode_valid(struct drm_connector *connector,
>  const struct drm_display_mode *mode)
>  {
> + struct meson_drm *priv = connector->dev->dev_private;
>   unsigned int vclk_freq;
>   unsigned int venc_freq;
>   unsigned int hdmi_freq;
>   int vic = drm_match_cea_mode(mode);
> + enum drm_mode_status status;
>  
>   DRM_DEBUG_DRIVER("Modeline %d:\"%s\" %d %d %d %d %d %d %d %d %d %d 0x%x 
> 0x%x\n",
>   mode->base.id, mode->name, mode->vrefresh, mode->clock,
> @@ -556,8 +564,11 @@ dw_hdmi_mode_valid(struct drm_connector *connector,
>  
>   /* Check against non-VIC supported modes */
>   if (!vic) {
> - if (!meson_venc_hdmi_supported_mode(mode))
> - return MODE_BAD;
> + status = meson_venc_hdmi_supported_mode(mode);
> + if (status != MODE_OK)
> + return status;
> +
> + return meson_vclk_dmt_supported_freq(priv, mode->clock);
>   /* Check against supported VIC modes */
>   } else if (!meson_venc_hdmi_supported_vic(vic))
>   return MODE_BAD;
> @@ -583,16 +594,11 @@ dw_hdmi_mode_valid(struct drm_connector *connector,
>   dev_dbg(connector->dev->dev, "%s: vclk:%d venc=%d hdmi=%d\n", __func__,
>   vclk_freq, venc_freq, hdmi_freq);
>  
> - /* Finally filter by configurable vclk frequencies */
> + /* Finally filter by configurable vclk frequencies for VIC modes */
>   switch (vclk_freq) {
> - case 25175:
> - case 4:
>   case 54000:
> - case 65000:
>   case 74250:
> - case 108000:
>   case 148500:
> - case 162000:
>   case 297000:
>   case 594000:
>   return MODE_OK;
> diff --git a/drivers/gpu/drm/meson/meson_vclk.c 
> b/drivers/gpu/drm/meson/meson_vclk.c
> index f051122..0b17788 100644
> --- a/drivers/gpu/drm/meson/meson_vclk.c
> +++ b/drivers/gpu/drm/meson/meson_vclk.c
> @@ -320,32 +320,23 @@ static void meson_venci_cvbs_clock_config(struct 
> meson_drm *priv)
>   CTS_VDAC_EN, CTS_VDAC_EN);
>  }
>  
> -
> +enum {
> 

[PATCH] drm/i915/selftests: Remove redundant code

2018-07-17 Thread Gustavo A. R. Silva
err is assigned to -EIO, but this value is never actually
used and *err* is updated later on.

Remove such reduntant code.

Addresses-Coverity-ID: 1471816 ("Unused value")
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/gpu/drm/i915/selftests/intel_guc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/selftests/intel_guc.c 
b/drivers/gpu/drm/i915/selftests/intel_guc.c
index 407c98f..c10e75c 100644
--- a/drivers/gpu/drm/i915/selftests/intel_guc.c
+++ b/drivers/gpu/drm/i915/selftests/intel_guc.c
@@ -206,7 +206,6 @@ static int igt_guc_clients(void *args)
 
if (!available_dbs(guc, guc->execbuf_client->priority)) {
pr_err("doorbell not available when it should\n");
-   err = -EIO;
goto out_db;
}
 
-- 
2.7.4

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


[PATCH v14 0/2] Add suppport for sn65dsi86 bridge chip

2018-07-17 Thread Sandeep Panda
Changes in current patchset:
 - eDP panels report EDID via DP-AUX channel, so remove support for
   dedicated DDC line.

Sandeep Panda (2):
  drm/bridge: add support for sn65dsi86 bridge driver
  dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings

 .../bindings/display/bridge/ti,sn65dsi86.txt   |  87 +++
 drivers/gpu/drm/bridge/Kconfig |   9 +
 drivers/gpu/drm/bridge/Makefile|   1 +
 drivers/gpu/drm/bridge/ti-sn65dsi86.c  | 676 +
 4 files changed, 773 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
 create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c

-- 
The Qualcomm Innovation Center, Inc. is a member of the 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 v2 5/5] drm/vkms: Implement CRC debugfs API

2018-07-17 Thread Haneen Mohammed
On Mon, Jul 16, 2018 at 02:22:56PM -0400, Sean Paul wrote:
> On Sat, Jul 14, 2018 at 03:23:32PM +0300, Haneen Mohammed wrote:
> > Implement the set_crc_source() callback.
> > Compute CRC using crc32 on the visible part of the framebuffer.
> > Use ordered workqueue to compute and add CRC at the end of a vblank.
> > 
> > Use appropriate synchronization methods since the crc computation must
> > be atomic wrt the generated vblank event for a given atomic update,
> > 
> > Signed-off-by: Haneen Mohammed 
> 
> Hey Haneen,
> Thanks for revising this patch set. Things are looking good across the series,
> just a few more comments :-)
> 

Thank you so much for the review! 

> > ---
> > Changes in v2:
> > - Include this patch in this patchset.
> > 
> >  drivers/gpu/drm/vkms/Makefile |  2 +-
> >  drivers/gpu/drm/vkms/vkms_crtc.c  | 60 ++-
> >  drivers/gpu/drm/vkms/vkms_drv.c   |  1 +
> >  drivers/gpu/drm/vkms/vkms_drv.h   | 21 +++
> >  drivers/gpu/drm/vkms/vkms_plane.c | 10 ++
> >  5 files changed, 85 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile
> > index 986297da51bf..37966914f70b 100644
> > --- a/drivers/gpu/drm/vkms/Makefile
> > +++ b/drivers/gpu/drm/vkms/Makefile
> > @@ -1,3 +1,3 @@
> > -vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o vkms_gem.o
> > +vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o vkms_gem.o 
> > vkms_crc.o
> >  
> >  obj-$(CONFIG_DRM_VKMS) += vkms.o
> > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c 
> > b/drivers/gpu/drm/vkms/vkms_crtc.c
> > index 26babb85ca77..f3a674db33b8 100644
> > --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> > @@ -10,18 +10,36 @@
> >  #include 
> >  #include 
> >  
> > -static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer)
> > +static void _vblank_handle(struct vkms_output *output)
> >  {
> > -   struct vkms_output *output = container_of(timer, struct vkms_output,
> > - vblank_hrtimer);
> > struct drm_crtc *crtc = >crtc;
> > -   int ret_overrun;
> > +   struct vkms_crtc_state *state = to_vkms_crtc_state(crtc->state);
> > bool ret;
> >  
> > +   int crc_enabled = 0;
> > +
> > +   spin_lock(>lock);
> > +   crc_enabled = output->crc_enabled;
> 
> Aside from the implicit bool -> int cast, I don't think you need this local 
> var,
> just use output->crc_enabled directly below.
> 
> > ret = drm_crtc_handle_vblank(crtc);
> > if (!ret)
> > DRM_ERROR("vkms failure on handling vblank");
> 
> This would be more useful with the error code printed.
> 

I think this only returns false on failure. Also I've noticed most of the usage 
of
drm_crtc_handle_vblank don't check the return value, should I do the
same as well and drop ret and error message?

> >  
> > +   if (state && crc_enabled) {
> > +   state->n_frame = drm_crtc_accurate_vblank_count(crtc);
> > +   queue_work(output->crc_workq, >crc_work);
> > +   }
> > +
> > +   spin_unlock(>lock);
> > +}
> > +
> > +static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer)
> > +{
> > +   struct vkms_output *output = container_of(timer, struct vkms_output,
> > + vblank_hrtimer);
> > +   int ret_overrun;
> > +
> > +   _vblank_handle(output);
> > +
> > ret_overrun = hrtimer_forward_now(>vblank_hrtimer,
> >   output->period_ns);
> >  
> > @@ -97,6 +115,8 @@ vkms_atomic_crtc_duplicate_state(struct drm_crtc *crtc)
> >  
> > __drm_atomic_helper_crtc_duplicate_state(crtc, _state->base);
> >  
> > +   INIT_WORK(_state->crc_work, vkms_crc_work_handle);
> > +
> > return _state->base;
> >  }
> >  
> > @@ -104,11 +124,18 @@ static void vkms_atomic_crtc_destroy_state(struct 
> > drm_crtc *crtc,
> >struct drm_crtc_state *state)
> >  {
> > struct vkms_crtc_state *vkms_state;
> > +   struct vkms_output *vkms_out = drm_crtc_to_vkms_output(crtc);
> >  
> > vkms_state = to_vkms_crtc_state(state);
> >  
> > __drm_atomic_helper_crtc_destroy_state(state);
> > -   kfree(vkms_state);
> > +
> > +   if (vkms_state) {
> > +   flush_workqueue(vkms_out->crc_workq);
> 
> I'm a little worried about this bit. Since the workqueue is per-output, is it
> possible you'll be waiting for more frames to complete than you need to be?
> 

I see, maybe I should flush per work_struct instead?

> > +   drm_framebuffer_put(_state->data.fb);
> > +   memset(_state->data, 0, sizeof(struct vkms_crc_data));
> > +   kfree(vkms_state);
> > +   }
> >  }
> >  
> >  static const struct drm_crtc_funcs vkms_crtc_funcs = {
> > @@ -120,6 +147,7 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = {
> > .atomic_destroy_state   = vkms_atomic_crtc_destroy_state,
> > .enable_vblank  = vkms_enable_vblank,
> > 

[PATCH v2] drm/msm/display: negative x/y in cursor move

2018-07-17 Thread Carsten Behling
modesetting X11 driver may provide negative x/y cordinates in
mdp5_crtc_cursor_move call when rotation is enabled.

Cursor buffer can overlap down to its negative width/height.

ROI has to be recalculated for negative x/y indicating using the
lower/right corner of the cursor buffer and hotspot must be set
in MDP5_LM_CURSOR_XY_SRC_Y MDP5_LM_CURSOR_XY_SRC_X.

Signed-off-by: Carsten Behling 
---
Changes in v2:
- fixed format specifier in debug message

 drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 51 ++-
 1 file changed, 43 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c 
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
index 10271359789e..a7f4a6688fec 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
@@ -65,7 +65,7 @@ struct mdp5_crtc {
struct drm_gem_object *scanout_bo;
uint64_t iova;
uint32_t width, height;
-   uint32_t x, y;
+   int x, y;
} cursor;
 };
 #define to_mdp5_crtc(x) container_of(x, struct mdp5_crtc, base)
@@ -760,20 +760,31 @@ static void get_roi(struct drm_crtc *crtc, uint32_t 
*roi_w, uint32_t *roi_h)
 * Cursor Region Of Interest (ROI) is a plane read from cursor
 * buffer to render. The ROI region is determined by the visibility of
 * the cursor point. In the default Cursor image the cursor point will
-* be at the top left of the cursor image, unless it is specified
-* otherwise using hotspot feature.
+* be at the top left of the cursor image.
 *
+* Without rotation:
 * If the cursor point reaches the right (xres - x < cursor.width) or
 * bottom (yres - y < cursor.height) boundary of the screen, then ROI
 * width and ROI height need to be evaluated to crop the cursor image
 * accordingly.
 * (xres-x) will be new cursor width when x > (xres - cursor.width)
 * (yres-y) will be new cursor height when y > (yres - cursor.height)
+*
+* With rotation:
+* We get negative x and/or y coordinates.
+* (cursor.width - abs(x)) will be new cursor width when x < 0
+* (cursor.height - abs(y)) will be new cursor width when y < 0
 */
-   *roi_w = min(mdp5_crtc->cursor.width, xres -
+   if (mdp5_crtc->cursor.x >= 0)
+   *roi_w = min(mdp5_crtc->cursor.width, xres -
mdp5_crtc->cursor.x);
-   *roi_h = min(mdp5_crtc->cursor.height, yres -
+   else
+   *roi_w = mdp5_crtc->cursor.width - abs(mdp5_crtc->cursor.x);
+   if (mdp5_crtc->cursor.y >= 0)
+   *roi_h = min(mdp5_crtc->cursor.height, yres -
mdp5_crtc->cursor.y);
+   else
+   *roi_h = mdp5_crtc->cursor.height - abs(mdp5_crtc->cursor.y);
 }
 
 static void mdp5_crtc_restore_cursor(struct drm_crtc *crtc)
@@ -783,7 +794,7 @@ static void mdp5_crtc_restore_cursor(struct drm_crtc *crtc)
struct mdp5_kms *mdp5_kms = get_kms(crtc);
const enum mdp5_cursor_alpha cur_alpha = CURSOR_ALPHA_PER_PIXEL;
uint32_t blendcfg, stride;
-   uint32_t x, y, width, height;
+   uint32_t x, y, src_x, src_y, width, height;
uint32_t roi_w, roi_h;
int lm;
 
@@ -800,6 +811,26 @@ static void mdp5_crtc_restore_cursor(struct drm_crtc *crtc)
 
get_roi(crtc, _w, _h);
 
+   /* If cusror buffer overlaps due to rotation on the
+* upper or left screen border the pixel offset inside
+* the cursor buffer of the ROI is the positive overlap
+* distance.
+*/
+   if (mdp5_crtc->cursor.x < 0) {
+   src_x = abs(mdp5_crtc->cursor.x);
+   x = 0;
+   } else {
+   src_x = 0;
+   }
+   if (mdp5_crtc->cursor.y < 0) {
+   src_y = abs(mdp5_crtc->cursor.y);
+   y = 0;
+   } else {
+   src_y = 0;
+   }
+   DBG("%s: x=%u, y=%u roi_w=%u roi_h=%u src_x=%u src_y=%u",
+   crtc->name, x, y, roi_w, roi_h, src_x, src_y);
+
mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_STRIDE(lm), stride);
mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_FORMAT(lm),
MDP5_LM_CURSOR_FORMAT_FORMAT(CURSOR_FMT_ARGB));
@@ -812,6 +843,9 @@ static void mdp5_crtc_restore_cursor(struct drm_crtc *crtc)
mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_START_XY(lm),
MDP5_LM_CURSOR_START_XY_Y_START(y) |
MDP5_LM_CURSOR_START_XY_X_START(x));
+   mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_XY(lm),
+   MDP5_LM_CURSOR_XY_SRC_Y(src_y) |
+   MDP5_LM_CURSOR_XY_SRC_X(src_x));
mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_BASE_ADDR(lm),
mdp5_crtc->cursor.iova);
 
@@ -932,8 +966,9 @@ static int mdp5_crtc_cursor_move(struct drm_crtc *crtc, int 
x, int y)
if 

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

2018-07-17 Thread Kees Cook
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'};
>
> -   memcpy(buf_cpy, buf, count+1);
> -   tmp = buf_cpy;
> -   while (tmp[0]) {
> -   sub_str =  strsep(, delimiter);
> -   if (strlen(sub_str)) {
> -   ret = kstrtol(sub_str, 0, );
> +   ret = amdgpu_read_mask(buf, count, );
> +   if (ret)
> +   return ret;
>
> -   if (ret) {
> -   count = -EINVAL;
> -   goto fail;
> -   }
> -   mask |= 1 << level;
> -   } else
> -   break;
> -   }
> if (adev->powerplay.pp_funcs->force_clock_level)
> amdgpu_dpm_force_clock_level(adev, PP_MCLK, mask);
>
> -fail:
> return count;
>  }
>
> @@ -701,33 +703,15 @@ static ssize_t amdgpu_set_pp_dpm_pcie(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'};
> -
> -   memcpy(buf_cpy, buf, count+1);
> -   tmp = buf_cpy;
>
> -   while (tmp[0]) {
> -   sub_str =  strsep(, delimiter);

Re: [PATCHv3] lib/ratelimit: Lockless ratelimiting

2018-07-17 Thread Dmitry Safonov
I would be glad if someone helps/bothers to review the change :C

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)
> + pr_notice("random: %u get_random_xx
> warning(s) missed "
> +   "due to ratelimiting\n",
> unseeded_miss);
> + urandom_miss = atomic_xchg(_warning.missed,
> 0);
> + if 

Re: [PATCH] kernel.h: Add for_each_if()

2018-07-17 Thread NeilBrown
On Mon, Jul 16 2018, Andy Shevchenko wrote:

> On Fri, 2018-07-13 at 16:42 -0700, Randy Dunlap wrote:
>> On 07/13/2018 04:37 PM, NeilBrown wrote:
>
>> 
>> coding-style.rst says:
>> Also, use braces when a loop contains more than a single simple
>> statement:
>
> Independently on a) would we use some macro for condition, or b) fix
> macros against this kind of nested conditions, there is another
> weirdness we would like to avoid, i.e.
>
> for_each_foo() {
>  ...
> } else {
>  ...
> }
>
> It is written according to coding style, but too much weird.

Agreed.  Too weird.

>
> So, summarize this discussion I think we would
> - keep for_each_if() in DRM subsystem alone
> - fix macros which are using positive condition 'if (cond)' by replacing
> with 'if (!cond) {} else' form for sake of robustness.
>
> Do you agree on that?

I agree.

Thanks,
NeilBrown


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


[PATCH v14 2/2] dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings

2018-07-17 Thread Sandeep Panda
Document the bindings used for the sn65dsi86 DSI to eDP bridge.

Changes in v1:
 - Rephrase the dt-binding descriptions to be more inline with existing
   bindings (Andrzej Hajda).
 - Add missing dt-binding that are parsed by corresponding driver
   (Andrzej Hajda).

Changes in v2:
 - Remove edp panel specific dt-binding entries. Only keep bridge
   specific entries (Sean Paul).
 - Remove custom-modes dt entry since its usage is removed from driver also 
(Sean Paul).
 - Remove is-pluggable dt entry since this will not be needed anymore (Sean 
Paul).

Changes in v3:
 - Remove irq-gpio dt entry and instead populate is an interrupt
   property (Rob Herring).

Changes in v4:
 - Add link to bridge chip datasheet (Stephen Boyd)
 - Add vpll and vcc regulator supply bindings (Stephen Boyd)
 - Add ref clk optional dt binding (Stephen Boyd)
 - Add gpio-controller optional dt binding (Stephen Boyd)

Changes in v5:
 - Use clock property to specify the input refclk (Stephen Boyd).
 - Update gpio cell and pwm cell numbers (Stephen Boyd).

Changes in v6:
 - Add property to mention the lane mapping scheme and polarity inversion
   (Stephen Boyd).

Changes in v7:
 - Detail description of lane mapping scheme dt property (Andrzej
   Hajda/ Rob Herring).
 - Removed HDP gpio binding, since the bridge uses IRQ signal to
   determine HPD, and IRQ property is already documented in binding.

Changes in v8:
 - Removed unnecessary explanation of lane mapping and polarity dt
   property, since these are already explained in media/video-interface
   dt binidng (Rob Herring).

Changes in v9:
 - Avoid putting re-definition of lane mapping and polarity dt binding
   (Rob Herring).

Changes in v10:
 - Use interrupts-extended property instead of interrupts to specify
   interrupt line (Andrzej Hajda).
 - Move data-lanes and lane-polarity property example to proper place (Andrzej 
Hajda).

Changes in v11:
 - Add a property for suspend gpio function of GPIO1 pin on bridge chip
   (Stephen Boyd).

Changes in v12:
 - Remove binding for dedicated DDC line (Andrzej Hajda).

Signed-off-by: Sandeep Panda 
---
 .../bindings/display/bridge/ti,sn65dsi86.txt   | 87 ++
 1 file changed, 87 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt

diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt 
b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
new file mode 100644
index ..eac6588ae37a
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
@@ -0,0 +1,87 @@
+SN65DSI86 DSI to eDP bridge chip
+
+
+This is the binding for Texas Instruments SN65DSI86 bridge.
+http://www.ti.com/general/docs/lit/getliterature.tsp?genericPartNumber=sn65dsi86=pdf
+
+Required properties:
+- compatible: Must be "ti,sn65dsi86"
+- reg: i2c address of the chip, 0x2d as per datasheet
+- enable-gpios: OF device-tree gpio specification for bridge_en pin (active 
high)
+
+- vccio-supply: A 1.8V supply that powers up the digital IOs.
+- vpll-supply: A 1.8V supply that powers up the displayport PLL.
+- vcca-supply: A 1.2V supply that powers up the analog circuits.
+- vcc-supply: A 1.2V supply that powers up the digital core.
+
+Optional properties:
+- interrupts-extended: Specifier for the SN65DSI86 interrupt line.
+
+- gpio-controller: Marks the device has a GPIO controller.
+- #gpio-cells: Should be two. The first cell is the pin number and
+   the second cell is used to specify flags.
+   See ../../gpio/gpio.txt for more information.
+- #pwm-cells : Should be one. See ../../pwm/pwm.txt for description of
+   the cell formats.
+
+- clock-names: should be "refclk"
+- clocks: Specification for input reference clock. The reference
+ clock rate must be 12 MHz, 19.2 MHz, 26 MHz, 27 MHz or 38.4 MHz.
+
+- data-lanes: See ../../media/video-interface.txt
+- lane-polarities: See ../../media/video-interface.txt
+
+- suspend-gpios: OF device-tree specification for GPIO1 pin on bridge (active 
low)
+
+Required nodes:
+This device has two video ports. Their connections are modelled using the
+OF graph bindings specified in Documentation/devicetree/bindings/graph.txt.
+
+- Video port 0 for DSI input
+- Video port 1 for eDP output
+
+Example
+---
+
+edp-bridge@2d {
+   compatible = "ti,sn65dsi86";
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <0x2d>;
+
+   enable-gpios = < 33 GPIO_ACTIVE_HIGH>;
+   suspend-gpios = < 34 GPIO_ACTIVE_LOW>;
+
+   interrupts-extended = < 4 IRQ_TYPE_EDGE_FALLING>;
+
+   vccio-supply = <_l17>;
+   vcca-supply = <_l6>;
+   vpll-supply = <_l17>;
+   vcc-supply = <_l6>;
+
+   clock-names = "refclk";
+   clocks = <_refclk>;
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = 

[Bug 105760] [4.17-rc1] RIP: smu7_populate_single_firmware_entry.isra.6+0x57/0xc0 [amdgpu] RSP: ffffa17901efb930

2018-07-17 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=105760

--- Comment #51 from Thomas Martitz  ---
Btw, your suggestion to disable runtime pm (amdgpu.runpm=0) doesn't help as far
as system suspend/resume is concerned. I think runtime pm generally works,
because I see occasional debug outout from
smu7_populate_single_firmware_entry() function even before suspending (that
suggests to me that the GPU has been suspended regardless of system suspend)

-- 
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 105760] [4.17-rc1] RIP: smu7_populate_single_firmware_entry.isra.6+0x57/0xc0 [amdgpu] RSP: ffffa17901efb930

2018-07-17 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=105760

--- Comment #52 from Thomas Martitz  ---
(In reply to Thomas Martitz from comment #51)
> because I see occasional debug outout from
> smu7_populate_single_firmware_entry() function even before suspending

Forgot to add that the debug output before system suspend doesn't indicate
errors (toc->num_entries/toc->structure_version is 0/1 as expected)

-- 
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: [Nouveau] [PATCH 1/5] drm/nouveau: Prevent RPM callback recursion in suspend/resume paths

2018-07-17 Thread Lukas Wunner
[cc += linux-pm]

Hi Lyude,

First of all, thanks a lot for looking into this. 

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++;
> +

I'm not sure if that variable is actually private to the PM core.
Grepping through the tree I only find a single occurrence where it's
accessed outside the PM core and that's in amdgpu.  So this looks
a little fishy TBH.  It may make sense to cc such patches to linux-pm
to get Rafael & other folks involved with the PM core to comment.

Also, the disable_depth variable only exists if the kernel was
compiled with CONFIG_PM enabled, but I can't find a "depends on PM"
or something like that in nouveau's Kconfig.  Actually, if PM is
not selected, all the nouveau_pmops_*() functions should be #ifdef'ed
away, but oddly there's no #ifdef CONFIG_PM anywhere in nouveau_drm.c.

Anywayn, 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.

Thanks,

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


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

2018-07-17 Thread Sandeep Panda
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 +
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 676 ++
 3 files changed, 686 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 3b99d5a06c16..8153150acd36 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ 

Re: [PATCH] mm, oom: distinguish blockable mode for mmu notifiers

2018-07-17 Thread Leon Romanovsky
On Mon, Jul 16, 2018 at 04:12:49PM -0700, Andrew Morton wrote:
> On Mon, 16 Jul 2018 13:50:58 +0200 Michal Hocko  wrote:
>
> > From: Michal Hocko 
> >
> > There are several blockable mmu notifiers which might sleep in
> > mmu_notifier_invalidate_range_start and that is a problem for the
> > oom_reaper because it needs to guarantee a forward progress so it cannot
> > depend on any sleepable locks.
> >
> > Currently we simply back off and mark an oom victim with blockable mmu
> > notifiers as done after a short sleep. That can result in selecting a
> > new oom victim prematurely because the previous one still hasn't torn
> > its memory down yet.
> >
> > We can do much better though. Even if mmu notifiers use sleepable locks
> > there is no reason to automatically assume those locks are held.
> > Moreover majority of notifiers only care about a portion of the address
> > space and there is absolutely zero reason to fail when we are unmapping an
> > unrelated range. Many notifiers do really block and wait for HW which is
> > harder to handle and we have to bail out though.
> >
> > This patch handles the low hanging fruid. 
> > __mmu_notifier_invalidate_range_start
> > gets a blockable flag and callbacks are not allowed to sleep if the
> > flag is set to false. This is achieved by using trylock instead of the
> > sleepable lock for most callbacks and continue as long as we do not
> > block down the call chain.
>
> I assume device driver developers are wondering "what does this mean
> for me".  As I understand it, the only time they will see
> blockable==false is when their driver is being called in response to an
> out-of-memory condition, yes?  So it is a very rare thing.

I can't say for everyone, but at least for me (mlx5), it is not rare event.
I'm seeing OOM very often while I'm running my tests in low memory VMs.

Thanks

>
> Any suggestions regarding how the driver developers can test this code
> path?  I don't think we presently have a way to fake an oom-killing
> event?  Perhaps we should add such a thing, given the problems we're
> having with that feature.


signature.asc
Description: PGP signature
___
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

2018-07-17 Thread Michal Vokáč

On 16.7.2018 11:23, Linus Walleij wrote:

The need to support some straight-forward VGA panels that are
not adhering to any standard like DPI arise in the ARM RTSM VE
Real-Time Systems Model Virtual Executive. This emulator (which
is not QEMU) does not model any bridge or panel other than
displaying whatever the user defines that they have.

Currently a fake "DPI panel" is used for this with the old
FBDEV driver, but this is wrong as it is not conforming to any
MIPI DPI standards. However the resolution chosen there is
standard VGA, and we can cover this with the simple panel
by simply adding the most common VGA resolutions as a kind
of "simple panel".

In difference from other cases where "simple panel" might be
hiding something more complex (such as a panel driver or bridge)
this case is actually applicable, since we are running inside
a simulation with no real hardware on the output.

Cc: devicet...@vger.kernel.org
Cc: Sudeep Holla 
Cc: Lorenzo Pieralisi 
Cc: Liviu Dudau 
Cc: Robin Murphy 
Signed-off-by: Linus Walleij 
---
  .../display/panel/video-graphics-array.txt| 21 ++
  drivers/gpu/drm/panel/panel-simple.c  | 66 +++
  2 files changed, 87 insertions(+)
  create mode 100644 
Documentation/devicetree/bindings/display/panel/video-graphics-array.txt

diff --git 
a/Documentation/devicetree/bindings/display/panel/video-graphics-array.txt 
b/Documentation/devicetree/bindings/display/panel/video-graphics-array.txt
new file mode 100644
index ..193d24ebe052
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/video-graphics-array.txt
@@ -0,0 +1,21 @@
+Video Graphics Array
+
+This binding is for simple panels using the standardized VGA resolutions
+defined by de facto standards beginning with the IBM PS/2 in 1987.
+
+Required properties:
+- compatible: should be "video-graphics-array"
+
+This binding is compatible with the simple-panel binding, which is specified
+in simple-panel.txt in this directory.
+
+Example:
+
+panel {
+   compatible = "video-graphics-array";
+   port {
+   vga_panel_in: endpoint {
+   remote-endpoint = <>;
+   };
+   };
+};
diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index cbf1ab404ee7..db4db5a3f75e 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -2069,6 +2069,69 @@ static const struct panel_desc winstar_wf35ltiacd = {
.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
  };
  
+static const struct drm_display_mode video_graphics_array_modes[] = {

+   {
+   /*
+* This is the most standardized "vanilla" VGA mode there is:
+* 640x480 @ 60 Hz
+*/
+   .clock = 27175,
+   .hdisplay = 640,
+   .hsync_start = 640 + 16,
+   .hsync_end = 640 + 16 + 96,
+   .htotal = 640 + 16 + 96 + 48,
+   .vdisplay = 480,
+   .vsync_start = 480 + 10,
+   .vsync_end = 480 + 10 + 2,
+   .vtotal = 480 + 10 + 2 + 33,
+   .vrefresh = 60,
+   .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
+   },
+   {
+   /* SVGA 800x600 @60 Hz */
+   .clock = 4,
+   .hdisplay = 800,
+   .hsync_start = 800 + 40,
+   .hsync_end = 800 + 40 + 128,
+   .htotal = 800 + 40 + 128 + 88,
+   .vdisplay = 600,
+   .vsync_start = 600 + 1,
+   .vsync_end = 600 + 1 + 4,
+   .vtotal = 600 + 1 + 4 + 23,
+   .vrefresh = 60,
+   .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
+   },
+   {
+   /* XGA 1024x768 @60 Hz */
+   .clock = 65000,
+   .hdisplay = 1024,
+   .hsync_start = 1024 + 24,
+   .hsync_end = 1024 + 24 + 136,
+   .htotal = 1024 + 24 + 136 + 160,
+   .vdisplay = 768,
+   .vsync_start = 768 + 3,
+   .vsync_end = 768 + 3 + 6,
+   .vtotal = 768 + 3 + 6 + 29,
+   .vrefresh = 60,
+   .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
+   },
+};
+
+static const struct panel_desc video_graphics_array = {
+   .modes = video_graphics_array_modes,
+   .num_modes = ARRAY_SIZE(video_graphics_array_modes),
+   .bpc = 8,
+   /*
+* Some typical 4:3 aspect ratio VGA monitor surely has these dimensions
+* of 40x30 mm.

I am sure it has not :) s/mm/cm or add a zero.

Michal


+*/
+   .size = {
+   .width = 400,
+   .height = 300,
+   },
+   .bus_format = MEDIA_BUS_FMT_RGB888_1X24,
+};
+
  static const struct of_device_id platform_of_match[] = {
{
.compatible = "ampire,am-480272h3tmqw-t01h",
@@ -2286,6 +2349,9 @@ static const struct 

[PATCH] drm/msm/display: negative x/y in cursor move

2018-07-17 Thread Carsten Behling
modesetting X11 driver may provide negative x/y cordinates in
mdp5_crtc_cursor_move call when rotation is enabled.

Cursor buffer can overlap down to its negative width/height.

ROI has to be recalculated for negative x/y indicating using the
lower/right corner of the cursor buffer and hotspot must be set
in MDP5_LM_CURSOR_XY_SRC_Y MDP5_LM_CURSOR_XY_SRC_X.

Signed-off-by: Carsten Behling 
---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 51 ++-
 1 file changed, 43 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c 
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
index 10271359789e..43a86582876c 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
@@ -65,7 +65,7 @@ struct mdp5_crtc {
struct drm_gem_object *scanout_bo;
uint64_t iova;
uint32_t width, height;
-   uint32_t x, y;
+   int x, y;
} cursor;
 };
 #define to_mdp5_crtc(x) container_of(x, struct mdp5_crtc, base)
@@ -760,20 +760,31 @@ static void get_roi(struct drm_crtc *crtc, uint32_t 
*roi_w, uint32_t *roi_h)
 * Cursor Region Of Interest (ROI) is a plane read from cursor
 * buffer to render. The ROI region is determined by the visibility of
 * the cursor point. In the default Cursor image the cursor point will
-* be at the top left of the cursor image, unless it is specified
-* otherwise using hotspot feature.
+* be at the top left of the cursor image.
 *
+* Without rotation:
 * If the cursor point reaches the right (xres - x < cursor.width) or
 * bottom (yres - y < cursor.height) boundary of the screen, then ROI
 * width and ROI height need to be evaluated to crop the cursor image
 * accordingly.
 * (xres-x) will be new cursor width when x > (xres - cursor.width)
 * (yres-y) will be new cursor height when y > (yres - cursor.height)
+*
+* With rotation:
+* We get negative x and/or y coordinates.
+* (cursor.width - abs(x)) will be new cursor width when x < 0
+* (cursor.height - abs(y)) will be new cursor width when y < 0
 */
-   *roi_w = min(mdp5_crtc->cursor.width, xres -
+   if (mdp5_crtc->cursor.x >= 0)
+   *roi_w = min(mdp5_crtc->cursor.width, xres -
mdp5_crtc->cursor.x);
-   *roi_h = min(mdp5_crtc->cursor.height, yres -
+   else
+   *roi_w = mdp5_crtc->cursor.width - abs(mdp5_crtc->cursor.x);
+   if (mdp5_crtc->cursor.y >= 0)
+   *roi_h = min(mdp5_crtc->cursor.height, yres -
mdp5_crtc->cursor.y);
+   else
+   *roi_h = mdp5_crtc->cursor.height - abs(mdp5_crtc->cursor.y);
 }
 
 static void mdp5_crtc_restore_cursor(struct drm_crtc *crtc)
@@ -783,7 +794,7 @@ static void mdp5_crtc_restore_cursor(struct drm_crtc *crtc)
struct mdp5_kms *mdp5_kms = get_kms(crtc);
const enum mdp5_cursor_alpha cur_alpha = CURSOR_ALPHA_PER_PIXEL;
uint32_t blendcfg, stride;
-   uint32_t x, y, width, height;
+   uint32_t x, y, src_x, src_y, width, height;
uint32_t roi_w, roi_h;
int lm;
 
@@ -800,6 +811,26 @@ static void mdp5_crtc_restore_cursor(struct drm_crtc *crtc)
 
get_roi(crtc, _w, _h);
 
+   /* If cusror buffer overlaps due to rotation on the
+* upper or left screen border the pixel offset inside
+* the cursor buffer of the ROI is the positive overlap
+* distance.
+*/
+   if (mdp5_crtc->cursor.x < 0) {
+   src_x = abs(mdp5_crtc->cursor.x);
+   x = 0;
+   } else {
+   src_x = 0;
+   }
+   if (mdp5_crtc->cursor.y < 0) {
+   src_y = abs(mdp5_crtc->cursor.y);
+   y = 0;
+   } else {
+   src_y = 0;
+   }
+   DBG("%s: x=%d, y=%d roi_w=%d roi_h=%d src_x=%d src_y=%d",
+   x, y, roi_w, roi_h, src_x, src_y);
+
mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_STRIDE(lm), stride);
mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_FORMAT(lm),
MDP5_LM_CURSOR_FORMAT_FORMAT(CURSOR_FMT_ARGB));
@@ -812,6 +843,9 @@ static void mdp5_crtc_restore_cursor(struct drm_crtc *crtc)
mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_START_XY(lm),
MDP5_LM_CURSOR_START_XY_Y_START(y) |
MDP5_LM_CURSOR_START_XY_X_START(x));
+   mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_XY(lm),
+   MDP5_LM_CURSOR_XY_SRC_Y(src_y) |
+   MDP5_LM_CURSOR_XY_SRC_X(src_x));
mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_BASE_ADDR(lm),
mdp5_crtc->cursor.iova);
 
@@ -932,8 +966,9 @@ static int mdp5_crtc_cursor_move(struct drm_crtc *crtc, int 
x, int y)
if (unlikely(!crtc->state->enable))
return 0;
 
-   

Re: [PATCH] drm/i2c: tda9950: Remove VLA usage

2018-07-17 Thread Kees Cook
On Wed, Jun 20, 2018 at 11:27 AM, Kees Cook  wrote:
> In the quest to remove all stack VLA usage from the kernel[1], this
> sets the buffer to maximum size and adds a sanity check.
>
> [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/i2c/tda9950.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i2c/tda9950.c b/drivers/gpu/drm/i2c/tda9950.c
> index 3f7396caad48..28314433c351 100644
> --- a/drivers/gpu/drm/i2c/tda9950.c
> +++ b/drivers/gpu/drm/i2c/tda9950.c
> @@ -76,9 +76,12 @@ struct tda9950_priv {
>  static int tda9950_write_range(struct i2c_client *client, u8 addr, u8 *p, 
> int cnt)
>  {
> struct i2c_msg msg;
> -   u8 buf[cnt + 1];
> +   u8 buf[CEC_MAX_MSG_SIZE + 3];
> int ret;
>
> +   if (WARN_ON(cnt > sizeof(buf)))
> +   return -EINVAL;
> +
> buf[0] = addr;
> memcpy(buf + 1, p, cnt);
>
> --
> 2.17.1
>
>
> --
> Kees Cook
> Pixel Security



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


Re: [PATCH] kernel.h: Add for_each_if()

2018-07-17 Thread Randy Dunlap
On 07/16/2018 01:11 AM, Andy Shevchenko wrote:
> On Fri, 2018-07-13 at 16:42 -0700, Randy Dunlap wrote:
>> On 07/13/2018 04:37 PM, NeilBrown wrote:
> 
>>
>> coding-style.rst says:
>> Also, use braces when a loop contains more than a single simple
>> statement:
> 
> Independently on a) would we use some macro for condition, or b) fix
> macros against this kind of nested conditions, there is another
> weirdness we would like to avoid, i.e.
> 
> for_each_foo() {
>  ...
> } else {
>  ...
> }
> 
> It is written according to coding style, but too much weird.

Yeah, that's odd.  Looks like else matches the for loop. (!)

> So, summarize this discussion I think we would
> - keep for_each_if() in DRM subsystem alone
> - fix macros which are using positive condition 'if (cond)' by replacing
> with 'if (!cond) {} else' form for sake of robustness.
> 
> Do you agree on that?

Sure, both of those sound good to me.

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


[PATCH 2/2] ARM: dts: opos6uldev: use OF graph to describe the display

2018-07-17 Thread Sébastien Szymanski
To make use of the new eLCDIF DRM driver OF graph description is
required. Describe the display using OF graph nodes.

Signed-off-by: Sébastien Szymanski 
---
 arch/arm/boot/dts/imx6ul-opos6uldev.dts | 37 ++---
 1 file changed, 16 insertions(+), 21 deletions(-)

diff --git a/arch/arm/boot/dts/imx6ul-opos6uldev.dts 
b/arch/arm/boot/dts/imx6ul-opos6uldev.dts
index 0e59ee57fd55..8ecdb9ad2b2e 100644
--- a/arch/arm/boot/dts/imx6ul-opos6uldev.dts
+++ b/arch/arm/boot/dts/imx6ul-opos6uldev.dts
@@ -56,7 +56,7 @@
stdout-path = 
};
 
-   backlight {
+   backlight: backlight {
compatible = "pwm-backlight";
pwms = < 0 191000>;
brightness-levels = <0 4 8 16 32 64 128 255>;
@@ -97,6 +97,18 @@
gpios = < 1 GPIO_ACTIVE_HIGH>;
};
 
+   panel: panel {
+   compatible = "armadeus,st0700-adapt";
+   power-supply = <_3v3>;
+   backlight = <>;
+
+   port {
+   panel_in: endpoint {
+   remote-endpoint = <_out>;
+   };
+   };
+   };
+
reg_5v: regulator-5v {
compatible = "regulator-fixed";
regulator-name = "5V";
@@ -182,28 +194,11 @@
  {
pinctrl-names = "default";
pinctrl-0 = <_lcdif>;
-   display = <>;
-   lcd-supply = <_3v3>;
status = "okay";
 
-   display0: display0 {
-   bits-per-pixel = <32>;
-   bus-width = <18>;
-
-   display-timings {
-   timing0: timing0 {
-   clock-frequency = <3333>;
-   hactive = <800>;
-   vactive = <480>;
-   hback-porch = <96>;
-   hfront-porch = <96>;
-   vback-porch = <20>;
-   vfront-porch = <21>;
-   hsync-len = <64>;
-   vsync-len = <4>;
-   de-active = <1>;
-   pixelclk-active = <0>;
-   };
+   port {
+   lcdif_out: endpoint {
+   remote-endpoint = <_in>;
};
};
 };
-- 
2.16.4

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


Re: [PATCH] kernel.h: Add for_each_if()

2018-07-17 Thread Andy Shevchenko
On Fri, 2018-07-13 at 16:42 -0700, Randy Dunlap wrote:
> On 07/13/2018 04:37 PM, NeilBrown wrote:

> 
> coding-style.rst says:
> Also, use braces when a loop contains more than a single simple
> statement:

Independently on a) would we use some macro for condition, or b) fix
macros against this kind of nested conditions, there is another
weirdness we would like to avoid, i.e.

for_each_foo() {
 ...
} else {
 ...
}

It is written according to coding style, but too much weird.

So, summarize this discussion I think we would
- keep for_each_if() in DRM subsystem alone
- fix macros which are using positive condition 'if (cond)' by replacing
with 'if (!cond) {} else' form for sake of robustness.

Do you agree on that?

-- 
Andy Shevchenko 
Intel Finland Oy
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


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

2018-07-17 Thread Souptick Joarder
On Mon, Jul 16, 2018 at 1:16 PM, 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 
> ---
>  drivers/gpu/drm/vgem/vgem_drv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
> index 0e5620f76ee0..ec6af8b920da 100644
> --- a/drivers/gpu/drm/vgem/vgem_drv.c
> +++ b/drivers/gpu/drm/vgem/vgem_drv.c
> @@ -504,7 +504,7 @@ static int __init vgem_init(void)
>  static void __exit vgem_exit(void)
>  {
> drm_dev_unregister(_device->drm);
> -   drm_dev_unref(_device->drm);
> +   drm_dev_put(_device->drm);
>  }
>
>  module_init(vgem_init);
> --
> 2.18.0
>

There are other gpu/drm drivers where drm_dev_unref is used.
Do we need to replace drm_dev_unref with drm_dev_put  in
other places as well ?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] drm/sun4i: sun8i: Avoid clearing blending order at each atomic commit

2018-07-17 Thread Paul Kocialkowski
Blending order is set based on the z position of each DRM plane. The
blending order register is currently cleared at each atomic DRM commit,
with the intent that each committed plane will set the appropriate
bits (based on its z-pos) when enabling the plane.

However, it sometimes happens that a particular plane is left unchanged
by an atomic commit and thus will not be configured again. In that
scenario, blending order is cleared and only the bits relevant for the
planes affected by the commit are set. This leaves the planes that did
not change without their blending order set in the register, leading
to that plane not being displayed.

Instead of clearing the blending order register at every atomic commit,
this change moves the register's initial clear at bind time and only
clears the bits for a specific plane when disabling it or changing its
zpos.

This way, planes that are left untouched by a DRM atomic commit are
no longer disabled.

Signed-off-by: Paul Kocialkowski 
---
 drivers/gpu/drm/sun4i/sun8i_mixer.c| 11 +++
 drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 24 
 drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 24 
 3 files changed, 43 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c 
b/drivers/gpu/drm/sun4i/sun8i_mixer.c
index 9bd069de3124..12cb7183ce51 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
@@ -260,13 +260,6 @@ const struct de2_fmt_info *sun8i_mixer_format_info(u32 
format)
return NULL;
 }
 
-static void sun8i_mixer_atomic_begin(struct sunxi_engine *engine,
-struct drm_crtc_state *old_state)
-{
-   regmap_update_bits(engine->regs, SUN8I_MIXER_BLEND_PIPE_CTL,
-  SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, 0);
-}
-
 static void sun8i_mixer_commit(struct sunxi_engine *engine)
 {
DRM_DEBUG_DRIVER("Committing changes\n");
@@ -318,7 +311,6 @@ static struct drm_plane **sun8i_layers_init(struct 
drm_device *drm,
 }
 
 static const struct sunxi_engine_ops sun8i_engine_ops = {
-   .atomic_begin   = sun8i_mixer_atomic_begin,
.commit = sun8i_mixer_commit,
.layers_init= sun8i_layers_init,
 };
@@ -445,6 +437,9 @@ static int sun8i_mixer_bind(struct device *dev, struct 
device *master,
regmap_write(mixer->engine.regs, SUN8I_MIXER_BLEND_MODE(i),
 SUN8I_MIXER_BLEND_MODE_DEF);
 
+   regmap_update_bits(mixer->engine.regs, SUN8I_MIXER_BLEND_PIPE_CTL,
+  SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, 0);
+
return 0;
 
 err_disable_bus_clk:
diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c 
b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
index 1a12cb02f2d2..f98fd9a9dd78 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
@@ -27,7 +27,8 @@
 #include "sun8i_ui_scaler.h"
 
 static void sun8i_ui_layer_enable(struct sun8i_mixer *mixer, int channel,
- int overlay, bool enable, unsigned int zpos)
+ int overlay, bool enable, unsigned int zpos,
+ unsigned int old_zpos)
 {
u32 val;
 
@@ -43,6 +44,18 @@ static void sun8i_ui_layer_enable(struct sun8i_mixer *mixer, 
int channel,
   SUN8I_MIXER_CHAN_UI_LAYER_ATTR(channel, overlay),
   SUN8I_MIXER_CHAN_UI_LAYER_ATTR_EN, val);
 
+   if (!enable || zpos != old_zpos) {
+   regmap_update_bits(mixer->engine.regs,
+  SUN8I_MIXER_BLEND_PIPE_CTL,
+  SUN8I_MIXER_BLEND_PIPE_CTL_EN(old_zpos),
+  0);
+
+   regmap_update_bits(mixer->engine.regs,
+  SUN8I_MIXER_BLEND_ROUTE,
+  SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(old_zpos),
+  0);
+   }
+
if (enable) {
val = SUN8I_MIXER_BLEND_PIPE_CTL_EN(zpos);
 
@@ -242,9 +255,11 @@ static void sun8i_ui_layer_atomic_disable(struct drm_plane 
*plane,
  struct drm_plane_state *old_state)
 {
struct sun8i_ui_layer *layer = plane_to_sun8i_ui_layer(plane);
+   unsigned int old_zpos = old_state->normalized_zpos;
struct sun8i_mixer *mixer = layer->mixer;
 
-   sun8i_ui_layer_enable(mixer, layer->channel, layer->overlay, false, 0);
+   sun8i_ui_layer_enable(mixer, layer->channel, layer->overlay, false, 0,
+ old_zpos);
 }
 
 static void sun8i_ui_layer_atomic_update(struct drm_plane *plane,
@@ -252,11 +267,12 @@ static void sun8i_ui_layer_atomic_update(struct drm_plane 
*plane,
 {
struct sun8i_ui_layer *layer = plane_to_sun8i_ui_layer(plane);
unsigned int zpos = plane->state->normalized_zpos;
+   unsigned int old_zpos = 

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

2018-07-17 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=107261

Bug ID: 107261
   Summary: [drm:generic_reg_wait [amdgpu]] *ERROR* REG_WAIT
timeout 1us * 10 tries - optc1_lock line:628
   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 140664
  --> https://bugs.freedesktop.org/attachment.cgi?id=140664=edit
Linux 4.18-rc5+ messages

On a MSI B350M MORTAR with Linux 4.18-rc5+ and Debian Sid/unstable with GNOME I
get the error below.

```
$ git log --oneline -1
30b06abfb92b (HEAD -> master, origin/master, origin/HEAD) Merge tag
'pinctrl-v4.18-3' of
git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl
$ git describe --dirty
v4.18-rc5-36-g30b06abfb92b
$ sudo dmesg
[…]
[  171.571430] [drm:generic_reg_wait [amdgpu]] *ERROR* REG_WAIT timeout 1us *
10 tries - optc1_lock line:628
[  171.571485] WARNING: CPU: 2 PID: 917 at
drivers/gpu/drm/amd/amdgpu/../display/dc/dc_helper.c:254
generic_reg_wait+0xe7/0x160 [amdgpu]
[  171.571486] Modules linked in: fuse nls_ascii amdkfd nls_cp437 vfat fat
wmi_bmof ppdev amdgpu edac_mce_amd ccp rng_core chash snd_hda_codec_realtek
efi_pstore gpu_sched kvm ttm irqbypass snd_hda_codec_generic snd_hda_codec_hdmi
crct10dif_pclmul sp5100_tco crc32_pclmul pcspkr snd_hda_intel i2c_piix4
ghash_clmulni_intel drm_kms_helper snd_hda_codec snd_hda_core efivars sg
snd_hwdep k10temp r8169 snd_pcm drm mii snd_timer snd i2c_algo_bit soundcore
parport_pc wmi parport video button acpi_cpufreq efivarfs ip_tables x_tables
autofs4 ext4 crc16 mbcache jbd2 fscrypto dm_crypt dm_mod raid10 raid456
libcrc32c crc32c_generic async_raid6_recov async_memcpy async_pq async_xor xor
async_tx raid6_pq raid1 raid0 multipath linear md_mod sd_mod evdev hid_generic
usbhid hid crc32c_intel ahci libahci xhci_pci aesni_intel
[  171.571521]  aes_x86_64 xhci_hcd crypto_simd cryptd glue_helper libata
usbcore scsi_mod gpio_amdpt gpio_generic
[  171.571527] CPU: 2 PID: 917 Comm: gnome-shell Not tainted 4.18.0-rc5+ #1
[  171.571528] Hardware name: MSI MS-7A37/B350M MORTAR (MS-7A37), BIOS 1.F0
04/27/2018
[  171.571569] RIP: 0010:generic_reg_wait+0xe7/0x160 [amdgpu]
[  171.571569] Code: 44 24 58 8b 54 24 48 89 de 44 89 4c 24 08 48 8b 4c 24 50
48 c7 c7 10 2e b5 c0 e8 f4 e1 ac ff 83 7d 20 01 44 8b 4c 24 08 74 02 <0f> 0b 48
83 c4 10 44 89 c8 5b 5d 41 5c 41 5d 41 5e 41 5f c3 41 0f 
[  171.571585] RSP: 0018:b2f003d1f920 EFLAGS: 00010297
[  171.571587] RAX:  RBX: 0001 RCX:

[  171.571587] RDX:  RSI: 9d185ec96738 RDI:
9d185ec96738
[  171.571588] RBP: 9d1843a49800 R08: 0392 R09:
0001
[  171.571588] R10:  R11: b4fcaf4d R12:
000b
[  171.571589] R13: 504d R14: 0100 R15:
0001
[  171.571590] FS:  7f7488a34ac0() GS:9d185ec8()
knlGS:
[  171.571591] CS:  0010 DS:  ES:  CR0: 80050033
[  171.571592] CR2: 01b1baab4488 CR3: 0003dc648000 CR4:
003406e0
[  171.571592] Call Trace:
[  171.571640]  optc1_lock+0xa0/0xb0 [amdgpu]
[  171.571684]  dcn10_pipe_control_lock.part.28+0x4a/0x70 [amdgpu]
[  171.571728]  dcn10_apply_ctx_for_surface+0xee/0x1210 [amdgpu]
[  171.571731]  ? _cond_resched+0x15/0x30
[  171.571774]  ? hubbub1_verify_allow_pstate_change_high+0xdd/0x180 [amdgpu]
[  171.571817]  ? dcn10_verify_allow_pstate_change_high+0x1d/0x240 [amdgpu]
[  171.571859]  ? dcn10_set_bandwidth+0x275/0x2d0 [amdgpu]
[  171.571900]  dc_commit_state+0x253/0x550 [amdgpu]
[  171.571940]  ? set_freesync_on_streams.part.6+0x4d/0x250 [amdgpu]
[  171.571979]  ? mod_freesync_set_user_enable+0x11f/0x150 [amdgpu]
[  171.572023]  amdgpu_dm_atomic_commit_tail+0x37c/0xd70 [amdgpu]
[  171.572058]  ? amdgpu_bo_pin_restricted+0xd6/0x300 [amdgpu]
[  171.572060]  ? _cond_resched+0x15/0x30
[  171.572061]  ? wait_for_completion_timeout+0x3b/0x1a0
[  171.572063]  ? refcount_inc+0x5/0x30
[  171.572070]  commit_tail+0x3d/0x70 [drm_kms_helper]
[  171.572075]  drm_atomic_helper_commit+0xb4/0x120 [drm_kms_helper]
[  171.572087]  drm_atomic_connector_commit_dpms+0xdb/0x100 [drm]
[  171.572097]  drm_mode_obj_set_property_ioctl+0x174/0x270 [drm]
[  171.572107]  ? drm_mode_obj_find_prop_id+0x40/0x40 [drm]
[  171.572116]  drm_ioctl_kernel+0xa1/0xf0 [drm]
[  171.572126]  drm_ioctl+0x1fc/0x390 [drm]
[  171.572135]  ? drm_mode_obj_find_prop_id+0x40/0x40 [drm]
[  171.572138]  ? eventfd_read+0xed/0x2a0
[  171.572171]  amdgpu_drm_ioctl+0x49/0x80 [amdgpu]
[  171.572174]  do_vfs_ioctl+0xa4/0x620
[  171.572176]  ksys_ioctl+0x60/0x90
[  171.572178]  ? ksys_read+0x9c/0xb0
[  171.572179]  __x64_sys_ioctl+0x16/0x20
[  171.572182]  

Re: [Nouveau] [PATCH 4/5] drm/nouveau: Grab RPM ref when i2c bus is in use

2018-07-17 Thread Ben Skeggs
On Tue, 17 Jul 2018 at 20:18, Karol Herbst  wrote:
>
> mhh, we shouldn't call to Linux APIs from within of nvkm. Rather gaurd
> the Linux glue code to the i2c stuff instead, but this is all done
> from inside of nvkm. I think we should move it out into
> drm/nouveau/nouveau_i2c.c and do the handling there.
Huh?  No, this is completely fine.

>
> On Tue, Jul 17, 2018 at 1:59 AM, Lyude Paul  wrote:
> > The i2c bus can be both accessed by DRM itself, along with any of it's
> > devnodes (/sys/class/i2c). So, we need to make sure that all codepaths
> > using the i2c bus keep the GPU resumed.
> >
> > Signed-off-by: Lyude Paul 
> > Cc: Karol Herbst 
> > Cc: sta...@vger.kernel.org
> > ---
> >  drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c | 12 +++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c 
> > b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c
> > index 807a2b67bd64..1de48c990b80 100644
> > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c
> > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c
> > @@ -119,18 +119,28 @@ nvkm_i2c_bus_release(struct nvkm_i2c_bus *bus)
> > BUS_TRACE(bus, "release");
> > nvkm_i2c_pad_release(pad);
> > mutex_unlock(>mutex);
> > +   pm_runtime_put_autosuspend(pad->i2c->subdev.device->dev);
> >  }
> >
> >  int
> >  nvkm_i2c_bus_acquire(struct nvkm_i2c_bus *bus)
> >  {
> > struct nvkm_i2c_pad *pad = bus->pad;
> > +   struct device *dev = pad->i2c->subdev.device->dev;
> > int ret;
> > +
> > BUS_TRACE(bus, "acquire");
> > +
> > +   ret = pm_runtime_get_sync(dev);
> > +   if (ret < 0 && ret != -EACCES)
> > +   return ret;
> > +
> > mutex_lock(>mutex);
> > ret = nvkm_i2c_pad_acquire(pad, NVKM_I2C_PAD_I2C);
> > -   if (ret)
> > +   if (ret) {
> > mutex_unlock(>mutex);
> > +   pm_runtime_put_autosuspend(dev);
> > +   }
> > return ret;
> >  }
> >
> > --
> > 2.17.1
> >
> > ___
> > Nouveau mailing list
> > nouv...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/nouveau
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


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

2018-07-17 Thread Maxime Ripard
On Tue, Jul 17, 2018 at 10:48:14AM +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 

Applied, thanks!
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


Re: [Nouveau] [PATCH v2 2/3] drm/nouveau: Fix runtime PM leak in nv50_disp_atomic_commit()

2018-07-17 Thread Ville Syrjälä
On Tue, Jul 17, 2018 at 09:33:52AM +0200, Lukas Wunner wrote:
> On Thu, Jul 12, 2018 at 01:02:53PM -0400, Lyude Paul wrote:
> > --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> > +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> > @@ -1878,7 +1878,7 @@ nv50_disp_atomic_commit(struct drm_device *dev,
> > nv50_disp_atomic_commit_tail(state);
> >  
> > drm_for_each_crtc(crtc, dev) {
> > -   if (crtc->state->enable) {
> > +   if (crtc->state->active) {
> > if (!drm->have_disp_power_ref) {
> > drm->have_disp_power_ref = true;
> > return 0;
> 
> Somewhat tangential comment on this older patch, since you
> continue to dig around in the runtime PM area:
> 
> Whenever a crtc is activated or deactivated in nouveau, we iterate
> over all crtcs and acquire a runtime PM if a crtc is active and
> previously there was no active one, or we drop a ref if none is
> active and previously there was an active one.
> 
> For a while now I've been thinking that it would be more straightforward
> to acquire a ref whenever a crtc is activated and drop one when a crtc
> is deactivated, i.e. hold one ref for every active crtc.  That way the
> have_disp_power_ref variable as well as the iteration logic could be
> removed, leading to a simplification.  Just a suggestion anyway.

The current code looks somewhat busted anyway. First problem is that
it's accessing crtc->state without the appropriate locks held (unless
something always pulls in all crtcs to every commit?). Second issue
is that the rpm_put() is called without waiting for nonblocking commits
to have finished so it looks like you can currentlly remove the power
before the hardware has been properly shut down.

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


Re: [PATCH v4 08/11] media: vsp1: Add support for extended display list headers

2018-07-17 Thread Laurent Pinchart
Hi Kieran,

On Monday, 16 July 2018 20:14:55 EEST Kieran Bingham wrote:
> On 24/05/18 12:44, Laurent Pinchart wrote:
> > On Thursday, 3 May 2018 16:36:19 EEST Kieran Bingham wrote:
> >> Extended display list headers allow pre and post command lists to be
> >> executed by the VSP pipeline. This provides the base support for
> >> features such as AUTO_FLD (for interlaced support) and AUTO_DISP (for
> >> supporting continuous camera preview pipelines.
> >> 
> >> Signed-off-by: Kieran Bingham 
> >> 
> >> ---
> >> 
> >> v2:
> >>  - remove __packed attributes
> >> 
> >> ---
> >> 
> >>  drivers/media/platform/vsp1/vsp1.h  |  1 +-
> >>  drivers/media/platform/vsp1/vsp1_dl.c   | 83 +-
> >>  drivers/media/platform/vsp1/vsp1_dl.h   | 29 -
> >>  drivers/media/platform/vsp1/vsp1_drv.c  |  7 +-
> >>  drivers/media/platform/vsp1/vsp1_regs.h |  5 +-
> >>  5 files changed, 116 insertions(+), 9 deletions(-)

[snip]

> >> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c
> >> b/drivers/media/platform/vsp1/vsp1_dl.c index 56514cd51c51..b64d32535edc
> >> 100644
> >> --- a/drivers/media/platform/vsp1/vsp1_dl.c
> >> +++ b/drivers/media/platform/vsp1/vsp1_dl.c

[snip]

> >> +struct vsp1_dl_ext_header {
> >> +  u32 reserved0;  /* alignment padding */
> >> +
> >> +  u16 pre_ext_cmd_qty;
> > 
> > Should this be called pre_ext_dl_num_cmd to match the datasheet ?
> 
> Yes, renamed.
> 
> >> +  u16 flags;
> > 
> > Aren't the flags supposed to come before the pre_ext_dl_num_cmd field ?
> 
> These are out-of-order to account for the fact that they are 16bit values.

Ah OK. It makes sense, but is a bit confusing when reading the datasheet.

> I felt that keeping them described in the struct was cleaner than shifts
> and masks - but clearly this stands out, due to the byte-ordering.
> 
> Would you prefer I re-write this as 32 bit accesses (or even 64bit),
> with shifts? Or is a comment sufficient here ?

If it doesn't make the code too ugly, using larger accesses would be better I 
think. Otherwise a comment would do I suppose.

> If we rewrite to be 32 bit accesses, would you be happy with the
> following naming:
> 
>   u32 reserved0;
>   u32 pre_ext_dl_num_cmd; /* Also stores command flags. */
>   u32 pre_ext_dl_plist;
>   u32 post_ext_dl_num_cmd;
>   u32 post_ext_dl_plist;
> 
> (Technically the flags are for the whole header, not the just the
> pre_ext, which is why I wanted it separated)
> 
> 
> Actually - now I write that - the 'number of commands' is sort of a flag
> or a parameter? so maybe the following is just as appropriate?:
> 
>   u32 reserved0;

Maybe "u32 zero;" or "u32 padding;" ? The datasheet states this is padding for 
alignment purpose.

>   u32 pre_ext_dl_flags;
>   u32 pre_ext_dl_plist;
>   u32 post_ext_dl_flags;
>   u32 post_ext_dl_plist;
> 
> Or they could be named 'options', or parameters?
> 
> Any comments before I hack that in?
> 
> The annoying part is that the 'flags' aren't part of the pre_ext cmds,
> they declare whether the pre or post cmd should be executed (or both I
> presume, we are yet to see post-cmd usage)

I agree with you, having a separate flag field would be nicer, as the flags 
are shared. I'll chose the easy option of letting you decide what you like 
best :-) All the above options are equally good to me, provided you add a 
comment explaining why the flag comes after the num_cmd field if you decide to 
keep it as a separate field.

> >> +  u32 pre_ext_cmd_plist;
> > 
> > And pre_ext_dl_plist ?
> > 
> >> +
> >> +  u32 post_ext_cmd_qty;
> >> +  u32 post_ext_cmd_plist;
> > 
> > Similar comments for these variables.
> 
> Renamed.
> 
> >> +};
> >> +
> >> +struct vsp1_dl_header_extended {
> >> +  struct vsp1_dl_header header;
> >> +  struct vsp1_dl_ext_header ext;
> >> +};
> >> +
> >>  struct vsp1_dl_entry {
> >>u32 addr;
> >>u32 data;
> >>  };
> >> 
> >> +struct vsp1_dl_ext_cmd_header {
> > 
> > Isn't this referred to in the datasheet as a body entry, not a header ?
> > How about naming it vsp1_dl_ext_cmd_entry ? Or just vsp1_dl_ext_cmd (in
> > which case the other structure that goes by the same name would need to be
> > renamed) ?
> 
> I think I was getting too creative. The reality is this part is really a
> 'header' describing the data in the body, but yes - it should be named
> to match a "Pre Extended Display List Body"
> 
>   s/vsp1_dl_ext_cmd_header/vsp1_pre_ext_dl_body/

Sounds good to me.

> This will then leave "struct vsp1_dl_ext_cmd" which represents the
> object instance within the VSP1 driver only.
> 
> >> +  u32 cmd;
> 
> This should really have been opcode then too :)

Good point.

> >> +  u32 flags;
> >> +  u32 data;
> >> +  u32 reserved;
> > 
> > The datasheet documents this as two 64-bit fields, shouldn't we handle the
> > structure the same way ?
> 
> I was trying to separate out the fields for clarity.
> 
> In this instance (unlike the 16bit handling above), the byte ordering of
> a 64 

[PATCH] drm/rockchip: Replace drm_dev_unref with drm_dev_put

2018-07-17 Thread Thomas Zimmermann
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 
---
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index f814d37b1db2..9c846be8fc64 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -184,7 +184,7 @@ static int rockchip_drm_bind(struct device *dev)
 err_free:
drm_dev->dev_private = NULL;
dev_set_drvdata(dev, NULL);
-   drm_dev_unref(drm_dev);
+   drm_dev_put(drm_dev);
return ret;
 }
 
@@ -204,7 +204,7 @@ static void rockchip_drm_unbind(struct device *dev)
 
drm_dev->dev_private = NULL;
dev_set_drvdata(dev, NULL);
-   drm_dev_unref(drm_dev);
+   drm_dev_put(drm_dev);
 }
 
 static const struct file_operations rockchip_drm_driver_fops = {
-- 
2.18.0

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


[PATCH] drm/zte: Replace drm_dev_unref with drm_dev_put

2018-07-17 Thread Thomas Zimmermann
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 
---
 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


Re: [PATCH v2] drm/sun4i: sun8i: Avoid clearing blending order at each atomic commit

2018-07-17 Thread Paul Kocialkowski
On Tue, 2018-07-17 at 14:25 +0200, Paul Kocialkowski wrote:
> Blending order is set based on the z position of each DRM plane. The
> blending order register is currently cleared at each atomic DRM commit,
> with the intent that each committed plane will set the appropriate
> bits (based on its z-pos) when enabling the plane.
> 
> However, it sometimes happens that a particular plane is left unchanged
> by an atomic commit and thus will not be configured again. In that
> scenario, blending order is cleared and only the bits relevant for the
> planes affected by the commit are set. This leaves the planes that did
> not change without their blending order set in the register, leading
> to that plane not being displayed.
> 
> Instead of clearing the blending order register at every atomic commit,
> this change moves the register's initial clear at bind time and only
> clears the bits for a specific plane when disabling it or changing its
> zpos.
> 
> This way, planes that are left untouched by a DRM atomic commit are
> no longer disabled.

This patch was rebased to apply on top of DRM misc. V1 had been based on
the first revision of the DE2 z-pos support patch, while a subsequent
revision of the patch made it to the kernel tree.

Cheers!

Paul

> Signed-off-by: Paul Kocialkowski 
> ---
>  drivers/gpu/drm/sun4i/sun8i_mixer.c| 15 +++
>  drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 24 
>  drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 24 
>  3 files changed, 43 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c 
> b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> index 8e81c24d736e..12cb7183ce51 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> @@ -260,17 +260,6 @@ const struct de2_fmt_info *sun8i_mixer_format_info(u32 
> format)
>   return NULL;
>  }
>  
> -static void sun8i_mixer_atomic_begin(struct sunxi_engine *engine,
> -  struct drm_crtc_state *old_state)
> -{
> - /*
> -  * Disable all pipes at the beginning. They will be enabled
> -  * again if needed in plane update callback.
> -  */
> - regmap_update_bits(engine->regs, SUN8I_MIXER_BLEND_PIPE_CTL,
> -SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, 0);
> -}
> -
>  static void sun8i_mixer_commit(struct sunxi_engine *engine)
>  {
>   DRM_DEBUG_DRIVER("Committing changes\n");
> @@ -322,7 +311,6 @@ static struct drm_plane **sun8i_layers_init(struct 
> drm_device *drm,
>  }
>  
>  static const struct sunxi_engine_ops sun8i_engine_ops = {
> - .atomic_begin   = sun8i_mixer_atomic_begin,
>   .commit = sun8i_mixer_commit,
>   .layers_init= sun8i_layers_init,
>  };
> @@ -449,6 +437,9 @@ static int sun8i_mixer_bind(struct device *dev, struct 
> device *master,
>   regmap_write(mixer->engine.regs, SUN8I_MIXER_BLEND_MODE(i),
>SUN8I_MIXER_BLEND_MODE_DEF);
>  
> + regmap_update_bits(mixer->engine.regs, SUN8I_MIXER_BLEND_PIPE_CTL,
> +SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, 0);
> +
>   return 0;
>  
>  err_disable_bus_clk:
> diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c 
> b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> index 518e1921f47e..28c15c6ef1ef 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> @@ -27,7 +27,8 @@
>  #include "sun8i_ui_scaler.h"
>  
>  static void sun8i_ui_layer_enable(struct sun8i_mixer *mixer, int channel,
> -   int overlay, bool enable, unsigned int zpos)
> +   int overlay, bool enable, unsigned int zpos,
> +   unsigned int old_zpos)
>  {
>   u32 val;
>  
> @@ -43,6 +44,18 @@ static void sun8i_ui_layer_enable(struct sun8i_mixer 
> *mixer, int channel,
>  SUN8I_MIXER_CHAN_UI_LAYER_ATTR(channel, overlay),
>  SUN8I_MIXER_CHAN_UI_LAYER_ATTR_EN, val);
>  
> + if (!enable || zpos != old_zpos) {
> + regmap_update_bits(mixer->engine.regs,
> +SUN8I_MIXER_BLEND_PIPE_CTL,
> +SUN8I_MIXER_BLEND_PIPE_CTL_EN(old_zpos),
> +0);
> +
> + regmap_update_bits(mixer->engine.regs,
> +SUN8I_MIXER_BLEND_ROUTE,
> +SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(old_zpos),
> +0);
> + }
> +
>   if (enable) {
>   val = SUN8I_MIXER_BLEND_PIPE_CTL_EN(zpos);
>  
> @@ -242,9 +255,11 @@ static void sun8i_ui_layer_atomic_disable(struct 
> drm_plane *plane,
> struct drm_plane_state *old_state)
>  {
>   struct sun8i_ui_layer *layer = plane_to_sun8i_ui_layer(plane);
> + unsigned int old_zpos = old_state->normalized_zpos;
>   struct sun8i_mixer 

[PATCH v2 2/3] ALSA: hda/i915: Associate audio component with devres

2018-07-17 Thread Takashi Iwai
The HD-audio i915 binding code contains a single pointer, hdac_acomp,
for allowing the access to audio component from the master bind/unbind
callbacks.  This was needed because the callbacks pass only the device
pointer and we can't guarantee the object type assigned to the drvdata
(which is free for each controller driver implementation).
And this implementation will be a problem if we support multiple
components for different DRM drivers, not only i915.

As a solution, allocate the audio component object via devres and
associate it with the given device, so that the component callbacks
can refer to it via devres_find().

The removal of the object is still done half-manually via
devres_destroy() to make the code consistent (although it may work
without the explicit call).

Also, the snd_hda_i915_register_notifier() had the reference to
hdac_acomp as well.  In this patch, the corresponding code is removed
by passing hdac_bus object to the function, too.

Signed-off-by: Takashi Iwai 
---
 include/sound/hda_i915.h |  6 --
 sound/hda/hdac_i915.c| 34 +-
 sound/pci/hda/patch_hdmi.c   |  5 +++--
 sound/soc/codecs/hdac_hdmi.c |  2 +-
 4 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/include/sound/hda_i915.h b/include/sound/hda_i915.h
index f69ea84e7b65..648263791559 100644
--- a/include/sound/hda_i915.h
+++ b/include/sound/hda_i915.h
@@ -17,7 +17,8 @@ int snd_hdac_acomp_get_eld(struct hdac_device *codec, 
hda_nid_t nid, int dev_id,
   bool *audio_enabled, char *buffer, int max_bytes);
 int snd_hdac_i915_init(struct hdac_bus *bus);
 int snd_hdac_i915_exit(struct hdac_bus *bus);
-int snd_hdac_i915_register_notifier(const struct drm_audio_component_audio_ops 
*);
+int snd_hdac_i915_register_notifier(struct hdac_bus *bus,
+   const struct drm_audio_component_audio_ops 
*ops);
 #else
 static inline int snd_hdac_set_codec_wakeup(struct hdac_bus *bus, bool enable)
 {
@@ -49,7 +50,8 @@ static inline int snd_hdac_i915_exit(struct hdac_bus *bus)
 {
return 0;
 }
-static inline int snd_hdac_i915_register_notifier(const struct 
drm_audio_component_audio_ops *ops)
+static inline int snd_hdac_i915_register_notifier(struct hdac_bus *bus,
+ const struct 
drm_audio_component_audio_ops *ops)
 {
return -ENODEV;
 }
diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c
index 1a88c1aaf9bb..861b77bbc7df 100644
--- a/sound/hda/hdac_i915.c
+++ b/sound/hda/hdac_i915.c
@@ -22,7 +22,14 @@
 #include 
 #include 
 
-static struct drm_audio_component *hdac_acomp;
+static void hdac_acomp_release(struct device *dev, void *res)
+{
+}
+
+static struct drm_audio_component *hdac_get_acomp(struct device *dev)
+{
+   return devres_find(dev, hdac_acomp_release, NULL, NULL);
+}
 
 /**
  * snd_hdac_set_codec_wakeup - Enable / disable HDMI/DP codec wakeup
@@ -262,7 +269,7 @@ EXPORT_SYMBOL_GPL(snd_hdac_acomp_get_eld);
 
 static int hdac_component_master_bind(struct device *dev)
 {
-   struct drm_audio_component *acomp = hdac_acomp;
+   struct drm_audio_component *acomp = hdac_get_acomp(dev);
int ret;
 
ret = component_bind_all(dev, acomp);
@@ -294,7 +301,7 @@ static int hdac_component_master_bind(struct device *dev)
 
 static void hdac_component_master_unbind(struct device *dev)
 {
-   struct drm_audio_component *acomp = hdac_acomp;
+   struct drm_audio_component *acomp = hdac_get_acomp(dev);
 
module_put(acomp->ops->owner);
component_unbind_all(dev, acomp);
@@ -314,6 +321,7 @@ static int hdac_component_master_match(struct device *dev, 
void *data)
 
 /**
  * snd_hdac_i915_register_notifier - Register i915 audio component ops
+ * @bus: HDA core bus
  * @aops: i915 audio component ops
  *
  * This function is supposed to be used only by a HD-audio controller
@@ -323,12 +331,13 @@ static int hdac_component_master_match(struct device 
*dev, void *data)
  *
  * Returns zero for success or a negative error code.
  */
-int snd_hdac_i915_register_notifier(const struct drm_audio_component_audio_ops 
*aops)
+int snd_hdac_i915_register_notifier(struct hdac_bus *bus,
+   const struct drm_audio_component_audio_ops 
*aops)
 {
-   if (!hdac_acomp)
+   if (!bus->audio_component)
return -ENODEV;
 
-   hdac_acomp->audio_ops = aops;
+   bus->audio_component->audio_ops = aops;
return 0;
 }
 EXPORT_SYMBOL_GPL(snd_hdac_i915_register_notifier);
@@ -365,18 +374,19 @@ int snd_hdac_i915_init(struct hdac_bus *bus)
struct drm_audio_component *acomp;
int ret;
 
-   if (WARN_ON(hdac_acomp))
+   if (WARN_ON(hdac_get_acomp(dev)))
return -EBUSY;
 
if (!i915_gfx_present())
return -ENODEV;
 
-   i915_acomp = kzalloc(sizeof(*i915_acomp), GFP_KERNEL);
+   i915_acomp = devres_alloc(hdac_acomp_release, 

[PATCH v2 0/3] Make the audio component binding more generic

2018-07-17 Thread Takashi Iwai
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.


Thanks!

Takashi

===

v1->v2:
* Change to SPDX for the new drm_audio_component.h
* Fix remaining i915 word in drm_audio_component.h comments
* Fix NULL dereference in master_bind / _unbind ops

===

Takashi Iwai (3):
  drm/i915: Split audio component to a generic type
  ALSA: hda/i915: Associate audio component with devres
  ALSA: hda: Make audio component support more generic

 drivers/gpu/drm/i915/Kconfig   |   1 +
 drivers/gpu/drm/i915/intel_audio.c |  22 +-
 include/drm/drm_audio_component.h  | 118 ++
 include/drm/i915_component.h   |  85 +---
 include/sound/hda_component.h  |  61 ++
 include/sound/hda_i915.h   |  37 +---
 include/sound/hdaudio.h|   6 +-
 sound/hda/Kconfig  |   7 +-
 sound/hda/Makefile |   1 +
 sound/hda/hdac_component.c | 335 +
 sound/hda/hdac_i915.c  | 335 ++---
 sound/pci/hda/patch_hdmi.c |  57 +++--
 sound/soc/codecs/hdac_hdmi.c   |  10 +-
 13 files changed, 607 insertions(+), 468 deletions(-)
 create mode 100644 include/drm/drm_audio_component.h
 create mode 100644 include/sound/hda_component.h
 create mode 100644 sound/hda/hdac_component.c

-- 
2.18.0

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


[PATCH v2 3/3] ALSA: hda: Make audio component support more generic

2018-07-17 Thread Takashi Iwai
This is the final step for more generic support of DRM audio
component.  The generic audio component code is now moved to its own
file, and the symbols are renamed from snd_hac_i915_* to
snd_hdac_acomp_*, respectively.  The generic code is enabled via the
new kconfig, CONFIG_SND_HDA_COMPONENT, while CONFIG_SND_HDA_I915 is
kept as the super-class.

Along with the split, three new callbacks are added to audio_ops:
pin2port is for providing the conversion between the pin number and
the widget id, and master_bind/master_unbin are called at binding /
unbinding the master component, respectively.  All these are optional,
but used in i915 implementation and also other later implementations.

A note about the new snd_hdac_acomp_init() function: there is a slight
difference between this and the old snd_hdac_i915_init().  The latter
(still) synchronizes with the master component binding, i.e. it
assures that the relevant DRM component gets bound when it returns, or
gives a negative error.  Meanwhile the new function doesn't
synchronize but just leaves as is.  It's the responsibility by the
caller's side to synchronize, or the caller may accept the
asynchronous binding on the fly.

v1->v2: Fix missing NULL check in master_bind/unbind

Signed-off-by: Takashi Iwai 
---
 drivers/gpu/drm/i915/Kconfig  |   1 +
 include/drm/drm_audio_component.h |  23 ++
 include/sound/hda_component.h |  61 ++
 include/sound/hda_i915.h  |  39 +---
 sound/hda/Kconfig |   7 +-
 sound/hda/Makefile|   1 +
 sound/hda/hdac_component.c| 335 +
 sound/hda/hdac_i915.c | 343 ++
 sound/pci/hda/patch_hdmi.c|  50 +++--
 sound/soc/codecs/hdac_hdmi.c  |   8 +-
 10 files changed, 486 insertions(+), 382 deletions(-)
 create mode 100644 include/sound/hda_component.h
 create mode 100644 sound/hda/hdac_component.c

diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index dfd95889f4b7..5c607f2c707b 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -23,6 +23,7 @@ config DRM_I915
select SYNC_FILE
select IOSF_MBI
select CRC32
+   select SND_HDA_I915 if SND_HDA_CORE
help
  Choose this option if you have a system that has "Intel Graphics
  Media Accelerator" or "HD Graphics" integrated graphics,
diff --git a/include/drm/drm_audio_component.h 
b/include/drm/drm_audio_component.h
index e85689f212c2..4923b00328c1 100644
--- a/include/drm/drm_audio_component.h
+++ b/include/drm/drm_audio_component.h
@@ -4,6 +4,8 @@
 #ifndef _DRM_AUDIO_COMPONENT_H_
 #define _DRM_AUDIO_COMPONENT_H_
 
+struct drm_audio_component;
+
 /**
  * struct drm_audio_component_ops - Ops implemented by DRM driver, called by 
hda driver
  */
@@ -72,6 +74,27 @@ struct drm_audio_component_audio_ops {
 * mode).
 */
void (*pin_eld_notify)(void *audio_ptr, int port, int pipe);
+   /**
+* @pin2port: Check and convert from pin node to port number
+*
+* Called by HDA driver to check and convert from the pin widget node
+* number to a port number in the graphics side.
+*/
+   int (*pin2port)(void *audio_ptr, int pin);
+   /**
+* @master_bind: (Optional) component master bind callback
+*
+* Called at binding master component, for HDA codec-specific
+* handling of dynamic binding.
+*/
+   int (*master_bind)(struct device *dev, struct drm_audio_component *);
+   /**
+* @master_unbind: (Optional) component master unbind callback
+*
+* Called at unbinding master component, for HDA codec-specific
+* handling of dynamic unbinding.
+*/
+   void (*master_unbind)(struct device *dev, struct drm_audio_component *);
 };
 
 /**
diff --git a/include/sound/hda_component.h b/include/sound/hda_component.h
new file mode 100644
index ..78626cde7081
--- /dev/null
+++ b/include/sound/hda_component.h
@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: GPL-2.0
+// HD-Audio helpers to sync with DRM driver
+
+#ifndef __SOUND_HDA_COMPONENT_H
+#define __SOUND_HDA_COMPONENT_H
+
+#include 
+
+#ifdef CONFIG_SND_HDA_COMPONENT
+int snd_hdac_set_codec_wakeup(struct hdac_bus *bus, bool enable);
+int snd_hdac_display_power(struct hdac_bus *bus, bool enable);
+int snd_hdac_sync_audio_rate(struct hdac_device *codec, hda_nid_t nid,
+int dev_id, int rate);
+int snd_hdac_acomp_get_eld(struct hdac_device *codec, hda_nid_t nid, int 
dev_id,
+  bool *audio_enabled, char *buffer, int max_bytes);
+int snd_hdac_acomp_init(struct hdac_bus *bus,
+   const struct drm_audio_component_audio_ops *aops,
+   int (*match_master)(struct device *, void *),
+   size_t extra_size);
+int snd_hdac_acomp_exit(struct hdac_bus *bus);
+int 

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

2018-07-17 Thread Philipp Zabel
On Tue, 2018-07-17 at 10:33 +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 

Thank you, applied to imx-drm/next.

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


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

2018-07-17 Thread Philipp Zabel
On Tue, 2018-07-17 at 10:35 +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 
> ---
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c 
> b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index a2ca90fc403c..5d024a154e1a 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -341,7 +341,7 @@ static int mtk_drm_bind(struct device *dev)
>  err_deinit:
>   mtk_drm_kms_deinit(drm);
>  err_free:
> - drm_dev_unref(drm);
> + drm_dev_put(drm);
>   return ret;
>  }
>  
> @@ -350,7 +350,7 @@ static void mtk_drm_unbind(struct device *dev)
>   struct mtk_drm_private *private = dev_get_drvdata(dev);
>  
>   drm_dev_unregister(private->drm);
> - drm_dev_unref(private->drm);
> + drm_dev_put(private->drm);
>   private->drm = NULL;
>  }
>  
> @@ -504,7 +504,7 @@ static int mtk_drm_remove(struct platform_device *pdev)
>  
>   drm_dev_unregister(drm);
>   mtk_drm_kms_deinit(drm);
> - drm_dev_unref(drm);
> + drm_dev_put(drm);
>  
>   component_master_del(>dev, _drm_ops);
>   pm_runtime_disable(>dev);

Reviewed-by: Philipp Zabel 

regards
Philipp
___
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

2018-07-17 Thread Karol Herbst
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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [Nouveau] [PATCH] drm/nouveau: Don't forget to label dp_aux devices

2018-07-17 Thread Karol Herbst
Reviewed-by: Karol Herbst 

2018-07-12 19:13 GMT+02:00 Lyude Paul :
> This makes debugging with DP tracing a lot harder to interpret, so name
> each i2c based off the name of the encoder that it's for
>
> Signed-off-by: Lyude Paul 
> ---
>  drivers/gpu/drm/nouveau/dispnv04/disp.c |  2 +-
>  drivers/gpu/drm/nouveau/dispnv50/disp.c |  2 +-
>  drivers/gpu/drm/nouveau/nouveau_connector.c | 12 ++--
>  drivers/gpu/drm/nouveau/nouveau_connector.h |  3 ++-
>  4 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/disp.c 
> b/drivers/gpu/drm/nouveau/dispnv04/disp.c
> index 501d2d290e9c..45ff1872d894 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/disp.c
> @@ -64,7 +64,7 @@ nv04_display_create(struct drm_device *dev)
> for (i = 0; i < dcb->entries; i++) {
> struct dcb_output *dcbent = >entry[i];
>
> -   connector = nouveau_connector_create(dev, dcbent->connector);
> +   connector = nouveau_connector_create(dev, dcbent);
> if (IS_ERR(connector))
> continue;
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
> b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> index 9382e99a0bc7..4f8d51590bbb 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -2198,7 +2198,7 @@ nv50_display_create(struct drm_device *dev)
>
> /* create encoder/connector objects based on VBIOS DCB table */
> for (i = 0, dcbe = >entry[0]; i < dcb->entries; i++, dcbe++) {
> -   connector = nouveau_connector_create(dev, dcbe->connector);
> +   connector = nouveau_connector_create(dev, dcbe);
> if (IS_ERR(connector))
> continue;
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c 
> b/drivers/gpu/drm/nouveau/nouveau_connector.c
> index 7b557c354307..0c5cc600c973 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -408,8 +408,10 @@ nouveau_connector_destroy(struct drm_connector 
> *connector)
> kfree(nv_connector->edid);
> drm_connector_unregister(connector);
> drm_connector_cleanup(connector);
> -   if (nv_connector->aux.transfer)
> +   if (nv_connector->aux.transfer) {
> drm_dp_aux_unregister(_connector->aux);
> +   kfree(nv_connector->aux.name);
> +   }
> kfree(connector);
>  }
>
> @@ -1201,13 +1203,16 @@ drm_conntype_from_dcb(enum dcb_connector_type dcb)
>  }
>
>  struct drm_connector *
> -nouveau_connector_create(struct drm_device *dev, int index)
> +nouveau_connector_create(struct drm_device *dev,
> +const struct dcb_output *dcbe)
>  {
> const struct drm_connector_funcs *funcs = _connector_funcs;
> struct nouveau_drm *drm = nouveau_drm(dev);
> struct nouveau_display *disp = nouveau_display(dev);
> struct nouveau_connector *nv_connector = NULL;
> struct drm_connector *connector;
> +   char aux_name[48] = {0};
> +   int index = dcbe->connector;
> int type, ret = 0;
> bool dummy;
>
> @@ -1306,6 +1311,9 @@ nouveau_connector_create(struct drm_device *dev, int 
> index)
> case DRM_MODE_CONNECTOR_eDP:
> nv_connector->aux.dev = dev->dev;
> nv_connector->aux.transfer = nouveau_connector_aux_xfer;
> +   snprintf(aux_name, sizeof(aux_name), "sor-%04x-%04x",
> +dcbe->hasht, dcbe->hashm);
> +   nv_connector->aux.name = kstrdup(aux_name, GFP_KERNEL);
> ret = drm_dp_aux_register(_connector->aux);
> if (ret) {
> NV_ERROR(drm, "failed to register aux channel\n");
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.h 
> b/drivers/gpu/drm/nouveau/nouveau_connector.h
> index a4d1a059bd3d..2c5cb51c7c33 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.h
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.h
> @@ -35,6 +35,7 @@
>  #include "nouveau_crtc.h"
>
>  struct nvkm_i2c_port;
> +struct dcb_output;
>
>  struct nouveau_connector {
> struct drm_connector base;
> @@ -76,7 +77,7 @@ nouveau_crtc_connector_get(struct nouveau_crtc *nv_crtc)
>  }
>
>  struct drm_connector *
> -nouveau_connector_create(struct drm_device *, int index);
> +nouveau_connector_create(struct drm_device *, const struct dcb_output *);
>
>  extern int nouveau_tv_disable;
>  extern int nouveau_ignorelid;
> --
> 2.17.1
>
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [V3] vga_switcheroo: set audio client id according to bound GPU id

2018-07-17 Thread Takashi Iwai
On Tue, 17 Jul 2018 10:56:37 +0200,
jimqu wrote:
> 
> 
> 
> On 2018年07月17日 16:52, Takashi Iwai wrote:
> > On Tue, 17 Jul 2018 10:38:58 +0200,
> > Lukas Wunner wrote:
> >> On Tue, Jul 17, 2018 at 04:20:50PM +0800, Jim Qu wrote:
> >>> On modern laptop, there are more and more platforms
> >>> have two GPUs, and each of them maybe have audio codec
> >>> for HDMP/DP output. For some dGPU which is no output,
> >>> audio codec usually is disabled.
> >>>
> >>> In currect HDA audio driver, it will set all codec as
> >>> VGA_SWITCHEROO_DIS, the audio which is binded to UMA
> >>> will be suspended if user use debugfs to contorl power
> >>>
> >>> In HDA driver side, it is difficult to know which GPU
> >>> the audio has binded to. So set the bound gpu pci dev
> >>> to vga_switcheroo.
> >>>
> >>> if the audio client is not the third registration, audio
> >>> id will set in vga_switcheroo enable function. if the
> >>> audio client is the last registration when vga_switcheroo
> >>> _ready() get true, we should get audio client id from bound
> >>> GPU directly.
> >>>
> >>> Signed-off-by: Jim Qu 
> >> Reviewed-by: Lukas Wunner 
> >>
> >> @Takashi, any preference which tree to merge this through?
> >> sound or drm-misc, either way would seem fine to me.  I think
> >> there's going to be one final drm-misc pull sent to Dave this
> >> week, after that it's 4.20.
> > Since it's basically an audio problem and I'd love to merge it for
> > 4.19, I'd prefer taking through sound git tree, unless anyone has
> > objection.
> 
> Thanks to Takashi and Lukas great help. Please kindly help merge the
> patch into suitable branch.

I pushed the fix to topic/vga_switcheroo branch of sound git tree now
so that 0day bot can check it.  I'll wait for a while and merge it
later to for-next branch if nothing happens.

The brach is (and will be) based on fresh 4.18-rc5 so that other trees
may merge it cleanly.


thanks,

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


[PATCH v2 1/3] drm/i915: Split audio component to a generic type

2018-07-17 Thread Takashi Iwai
For allowing other drivers to use the DRM audio component, rename the
i915_audio_component_* with drm_audio_component_*, and split the
generic part into drm_audio_component.h.  The i915 specific stuff
remains in struct i915_audio_component, which contains
drm_audio_component as the base.

The license of drm_audio_component.h is kept to MIT as same as the the
original i915_component.h.

This is a preliminary change for further development, and no
functional changes by this patch itself, merely code-split and
renames.

v1->v2: Use SPDX for drm_audio_component.h, fix remaining i915
argument in drm_audio_component.h

Signed-off-by: Takashi Iwai 
---
 drivers/gpu/drm/i915/intel_audio.c | 22 +++
 include/drm/drm_audio_component.h  | 95 ++
 include/drm/i915_component.h   | 85 ++
 include/sound/hda_i915.h   |  6 +-
 include/sound/hdaudio.h|  6 +-
 sound/hda/hdac_i915.c  | 40 +++--
 sound/pci/hda/patch_hdmi.c |  8 +--
 sound/soc/codecs/hdac_hdmi.c   |  2 +-
 8 files changed, 144 insertions(+), 120 deletions(-)
 create mode 100644 include/drm/drm_audio_component.h

diff --git a/drivers/gpu/drm/i915/intel_audio.c 
b/drivers/gpu/drm/i915/intel_audio.c
index 3ea566f99450..7dd5605d94ae 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -639,11 +639,12 @@ void intel_audio_codec_enable(struct intel_encoder 
*encoder,
dev_priv->av_enc_map[pipe] = encoder;
mutex_unlock(_priv->av_mutex);
 
-   if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify) {
+   if (acomp && acomp->base.audio_ops &&
+   acomp->base.audio_ops->pin_eld_notify) {
/* audio drivers expect pipe = -1 to indicate Non-MST cases */
if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST))
pipe = -1;
-   acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
+   
acomp->base.audio_ops->pin_eld_notify(acomp->base.audio_ops->audio_ptr,
 (int) port, (int) pipe);
}
 
@@ -681,11 +682,12 @@ void intel_audio_codec_disable(struct intel_encoder 
*encoder,
dev_priv->av_enc_map[pipe] = NULL;
mutex_unlock(_priv->av_mutex);
 
-   if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify) {
+   if (acomp && acomp->base.audio_ops &&
+   acomp->base.audio_ops->pin_eld_notify) {
/* audio drivers expect pipe = -1 to indicate Non-MST cases */
if (!intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DP_MST))
pipe = -1;
-   acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
+   
acomp->base.audio_ops->pin_eld_notify(acomp->base.audio_ops->audio_ptr,
 (int) port, (int) pipe);
}
 
@@ -880,7 +882,7 @@ static int i915_audio_component_get_eld(struct device 
*kdev, int port,
return ret;
 }
 
-static const struct i915_audio_component_ops i915_audio_component_ops = {
+static const struct drm_audio_component_ops i915_audio_component_ops = {
.owner  = THIS_MODULE,
.get_power  = i915_audio_component_get_power,
.put_power  = i915_audio_component_put_power,
@@ -897,12 +899,12 @@ static int i915_audio_component_bind(struct device 
*i915_kdev,
struct drm_i915_private *dev_priv = kdev_to_i915(i915_kdev);
int i;
 
-   if (WARN_ON(acomp->ops || acomp->dev))
+   if (WARN_ON(acomp->base.ops || acomp->base.dev))
return -EEXIST;
 
drm_modeset_lock_all(_priv->drm);
-   acomp->ops = _audio_component_ops;
-   acomp->dev = i915_kdev;
+   acomp->base.ops = _audio_component_ops;
+   acomp->base.dev = i915_kdev;
BUILD_BUG_ON(MAX_PORTS != I915_MAX_PORTS);
for (i = 0; i < ARRAY_SIZE(acomp->aud_sample_rate); i++)
acomp->aud_sample_rate[i] = 0;
@@ -919,8 +921,8 @@ static void i915_audio_component_unbind(struct device 
*i915_kdev,
struct drm_i915_private *dev_priv = kdev_to_i915(i915_kdev);
 
drm_modeset_lock_all(_priv->drm);
-   acomp->ops = NULL;
-   acomp->dev = NULL;
+   acomp->base.ops = NULL;
+   acomp->base.dev = NULL;
dev_priv->audio_component = NULL;
drm_modeset_unlock_all(_priv->drm);
 }
diff --git a/include/drm/drm_audio_component.h 
b/include/drm/drm_audio_component.h
new file mode 100644
index ..e85689f212c2
--- /dev/null
+++ b/include/drm/drm_audio_component.h
@@ -0,0 +1,95 @@
+// SPDX-License-Identifier: MIT
+// Copyright © 2014 Intel Corporation
+
+#ifndef _DRM_AUDIO_COMPONENT_H_
+#define _DRM_AUDIO_COMPONENT_H_
+
+/**
+ * struct drm_audio_component_ops - Ops implemented by DRM driver, called by 
hda driver
+ */
+struct drm_audio_component_ops {
+   /**
+* @owner: 

[PATCH] drm/amdgpu: fix spelling mistake "successed" -> "succeeded"

2018-07-17 Thread Colin King
From: Colin Ian King 

Trivial fix to spelling mistake in dev_err error message.

Signed-off-by: Colin Ian King 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 9883fa9bb41b..e9feb3c58389 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2004,7 +2004,7 @@ static int amdgpu_device_ip_reinit_early_sriov(struct 
amdgpu_device *adev)
continue;
 
r = block->version->funcs->hw_init(adev);
-   DRM_INFO("RE-INIT: %s %s\n", 
block->version->funcs->name, r?"failed":"successed");
+   DRM_INFO("RE-INIT: %s %s\n", 
block->version->funcs->name, r?"failed":"succeeded");
if (r)
return r;
}
@@ -2039,7 +2039,7 @@ static int amdgpu_device_ip_reinit_late_sriov(struct 
amdgpu_device *adev)
continue;
 
r = block->version->funcs->hw_init(adev);
-   DRM_INFO("RE-INIT: %s %s\n", 
block->version->funcs->name, r?"failed":"successed");
+   DRM_INFO("RE-INIT: %s %s\n", 
block->version->funcs->name, r?"failed":"succeeded");
if (r)
return r;
}
@@ -3092,7 +3092,7 @@ static int amdgpu_device_handle_vram_lost(struct 
amdgpu_device *adev)
  * @adev: amdgpu device pointer
  *
  * attempt to do soft-reset or full-reset and reinitialize Asic
- * return 0 means successed otherwise failed
+ * return 0 means succeeded otherwise failed
  */
 static int amdgpu_device_reset(struct amdgpu_device *adev)
 {
@@ -3170,7 +3170,7 @@ static int amdgpu_device_reset(struct amdgpu_device *adev)
  * @from_hypervisor: request from hypervisor
  *
  * do VF FLR and reinitialize Asic
- * return 0 means successed otherwise failed
+ * return 0 means succeeded otherwise failed
  */
 static int amdgpu_device_reset_sriov(struct amdgpu_device *adev,
 bool from_hypervisor)
@@ -3295,7 +3295,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
dev_info(adev->dev, "GPU reset(%d) failed\n", 
atomic_read(>gpu_reset_counter));
amdgpu_vf_error_put(adev, AMDGIM_ERROR_VF_GPU_RESET_FAIL, 0, r);
} else {
-   dev_info(adev->dev, "GPU reset(%d) 
successed!\n",atomic_read(>gpu_reset_counter));
+   dev_info(adev->dev, "GPU reset(%d) 
succeeded!\n",atomic_read(>gpu_reset_counter));
}
 
amdgpu_vf_error_trans_all(adev);
-- 
2.17.1

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


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

2018-07-17 Thread Linus Walleij
On Tue, Jul 17, 2018 at 10:53 AM 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 

Reviewed-by: Linus Walleij 

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


[Bug 21682] White screen with compiz/AIGLX on X.org server 1.5.2 when running on 16bpp (intel 945GM)

2018-07-17 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=21682

Stefan Dirsch  changed:

   What|Removed |Added

 Resolution|--- |WONTFIX
 Status|NEEDINFO|RESOLVED

--- Comment #25 from Stefan Dirsch  ---
compiz is no longer been shipped by any still supported Linux distribution I
guess. Also 945GM were still 32 bit machines. Probably no longer been used by
anyone. Let's better close this one as WONTFIX.

-- 
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: [Nouveau] [PATCH 2/5] drm/nouveau: Grab RPM ref while probing outputs

2018-07-17 Thread Karol Herbst
Reviewed-by: Karol Herbst 

On Tue, Jul 17, 2018 at 9:21 AM, Lukas Wunner  wrote:
> On Mon, Jul 16, 2018 at 07:59:26PM -0400, Lyude Paul wrote:
>> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
>> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
>> @@ -2012,10 +2012,18 @@ nv50_disp_atomic_state_alloc(struct drm_device *dev)
>>   return >state;
>>  }
>>
>> +static void
>> +nouveau_output_poll_changed(struct drm_device *dev)
>> +{
>> + pm_runtime_get_sync(dev->dev);
>> + drm_fb_helper_hotplug_event(dev->fb_helper);
>> + pm_runtime_put_autosuspend(dev->dev);
>> +}
>> +
>>  static const struct drm_mode_config_funcs
>>  nv50_disp_func = {
>>   .fb_create = nouveau_user_framebuffer_create,
>> - .output_poll_changed = drm_fb_helper_output_poll_changed,
>> + .output_poll_changed = nouveau_output_poll_changed,
>
> It might make sense to provide a generic DRM helper for this.
> Same for patch 3 in this series.
>
> Thanks,
>
> Lukas
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [Nouveau] [PATCH 3/5] drm/nouveau: Add missing RPM get/put() when probing connectors

2018-07-17 Thread Karol Herbst
Reviewed-by: Karol Herbst 

On Tue, Jul 17, 2018 at 1:59 AM, Lyude Paul  wrote:
> 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.
>
> Signed-off-by: Lyude Paul 
> Cc: Karol Herbst 
> Cc: sta...@vger.kernel.org
> ---
>  drivers/gpu/drm/nouveau/dispnv50/disp.c |  2 +-
>  drivers/gpu/drm/nouveau/nouveau_connector.c | 21 +++--
>  drivers/gpu/drm/nouveau/nouveau_connector.h |  3 +++
>  3 files changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
> b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> index ea2a886854fe..0f283ca75189 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 = nouveau_connector_probe_single_connector_modes,
> .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..feb142fb7a8a 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -770,6 +770,23 @@ nouveau_connector_force(struct drm_connector *connector)
> nouveau_connector_set_encoder(connector, nv_encoder);
>  }
>
> +int
> +nouveau_connector_probe_single_connector_modes(struct drm_connector 
> *connector,
> +  uint32_t maxX, uint32_t maxY)
> +{
> +   struct device *dev = connector->dev->dev;
> +   int ret;
> +
> +   ret = pm_runtime_get_sync(dev);
> +   if (WARN_ON(ret < 0 && ret != -EACCES))
> +   return 0;
> +
> +   ret = drm_helper_probe_single_connector_modes(connector, maxX, maxY);
> +
> +   pm_runtime_put_autosuspend(dev);
> +   return ret;
> +}
> +
>  static int
>  nouveau_connector_set_property(struct drm_connector *connector,
>struct drm_property *property, uint64_t value)
> @@ -1088,7 +1105,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 = nouveau_connector_probe_single_connector_modes,
> .set_property = nouveau_connector_set_property,
> .destroy = nouveau_connector_destroy,
> .atomic_duplicate_state = nouveau_conn_atomic_duplicate_state,
> @@ -1103,7 +1120,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 = nouveau_connector_probe_single_connector_modes,
> .set_property = nouveau_connector_set_property,
> .destroy = nouveau_connector_destroy,
> .atomic_duplicate_state = nouveau_conn_atomic_duplicate_state,
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.h 
> b/drivers/gpu/drm/nouveau/nouveau_connector.h
> index 2d9d35a146a4..e9ffc6eebda2 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.h
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.h
> @@ -106,6 +106,9 @@ nouveau_crtc_connector_get(struct nouveau_crtc *nv_crtc)
>
>  struct drm_connector *
>  nouveau_connector_create(struct drm_device *, const struct dcb_output *);
> +int
> +nouveau_connector_probe_single_connector_modes(struct drm_connector *,
> +  uint32_t, uint32_t);
>
>  extern int nouveau_tv_disable;
>  extern int nouveau_ignorelid;
> --
> 2.17.1
>
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [Nouveau] [PATCH 4/5] drm/nouveau: Grab RPM ref when i2c bus is in use

2018-07-17 Thread Karol Herbst
mhh, we shouldn't call to Linux APIs from within of nvkm. Rather gaurd
the Linux glue code to the i2c stuff instead, but this is all done
from inside of nvkm. I think we should move it out into
drm/nouveau/nouveau_i2c.c and do the handling there.

On Tue, Jul 17, 2018 at 1:59 AM, Lyude Paul  wrote:
> The i2c bus can be both accessed by DRM itself, along with any of it's
> devnodes (/sys/class/i2c). So, we need to make sure that all codepaths
> using the i2c bus keep the GPU resumed.
>
> Signed-off-by: Lyude Paul 
> Cc: Karol Herbst 
> Cc: sta...@vger.kernel.org
> ---
>  drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c | 12 +++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c 
> b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c
> index 807a2b67bd64..1de48c990b80 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c
> @@ -119,18 +119,28 @@ nvkm_i2c_bus_release(struct nvkm_i2c_bus *bus)
> BUS_TRACE(bus, "release");
> nvkm_i2c_pad_release(pad);
> mutex_unlock(>mutex);
> +   pm_runtime_put_autosuspend(pad->i2c->subdev.device->dev);
>  }
>
>  int
>  nvkm_i2c_bus_acquire(struct nvkm_i2c_bus *bus)
>  {
> struct nvkm_i2c_pad *pad = bus->pad;
> +   struct device *dev = pad->i2c->subdev.device->dev;
> int ret;
> +
> BUS_TRACE(bus, "acquire");
> +
> +   ret = pm_runtime_get_sync(dev);
> +   if (ret < 0 && ret != -EACCES)
> +   return ret;
> +
> mutex_lock(>mutex);
> ret = nvkm_i2c_pad_acquire(pad, NVKM_I2C_PAD_I2C);
> -   if (ret)
> +   if (ret) {
> mutex_unlock(>mutex);
> +   pm_runtime_put_autosuspend(dev);
> +   }
> return ret;
>  }
>
> --
> 2.17.1
>
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


<    1   2