[PATCH] gma500: Fix possible sleep-in-atomic bugs in gma_power_begin

2017-10-10 Thread Jia-Ju Bai
The driver may sleep under a spinlock, and the function call paths are:
gma_power_begin (acquire the spinlock) (drivers/gpu/drm/gma500/power.c)
   gma_resume_pci
 pci_set_power_state
   __pci_start_power_transition (drivers/pci/pci.c)
 msleep --> may sleep

gma_power_begin (acquire the spinlock) (drivers/gpu/drm/gma500/power.c)
   gma_resume_pci
 pci_enable_device
   pci_enable_device_flags (drivers/pci/pci.c)
 do_pci_enable_device
   pci_set_power_state
 __pci_start_power_transition
   msleep --> may sleep 

To fix them, the spinlock is released before gma_resume_pci, and it is acquired 
again after gma_resume_pci.

This bug is found by my static analysis tool and my code review.

Signed-off-by: Jia-Ju Bai 
---
 drivers/gpu/drm/gma500/power.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/gma500/power.c b/drivers/gpu/drm/gma500/power.c
index bea8578..c355d7e 100644
--- a/drivers/gpu/drm/gma500/power.c
+++ b/drivers/gpu/drm/gma500/power.c
@@ -264,7 +264,9 @@ bool gma_power_begin(struct drm_device *dev, bool force_on)
goto out_false;
 
/* Ok power up needed */
+   spin_unlock_irqrestore(&power_ctrl_lock, flags);
ret = gma_resume_pci(dev->pdev);
+   spin_lock_irqsave(&power_ctrl_lock, flags);
if (ret == 0) {
psb_irq_preinstall(dev);
psb_irq_postinstall(dev);
-- 
1.7.9.5


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


[PATCH] pci: Fix a possible sleep-in-atomic bug in pci_set_power_state

2017-10-10 Thread Jia-Ju Bai
The drivers vt6655 and gma500 call pci_set_power_state under a spinlock, which 
may sleep.
The function call paths are:
gma_power_begin (acquire the spinlock) (drivers/gpu/drm/gma500/power.c)
  gma_resume_pci
pci_set_power_state
  __pci_start_power_transition (drivers/pci/pci.c)
msleep --> may sleep

gma_power_begin (acquire the spinlock) (drivers/gpu/drm/gma500/power.c)
  gma_resume_pci
pci_enable_device
  pci_enable_device_flags (drivers/pci/pci.c)
do_pci_enable_device
  pci_set_power_state
__pci_start_power_transition
  msleep --> may sleep

vt6655_suspend (acquire the spinlock) (drivers/staging/vt6655/device_main.c)
  pci_set_power_state
__pci_start_power_transition (drivers/pci/pci.c)
  msleep --> may sleep

To fix these bugs, msleep is replaced with mdelay in 
__pci_start_power_transition

These bugs are found by my static analysis tool and my code review.

Signed-off-by: Jia-Ju Bai 
---
 drivers/pci/pci.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 6078dfc..7b763a3 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -823,7 +823,7 @@ static void __pci_start_power_transition(struct pci_dev 
*dev, pci_power_t state)
 */
if (dev->runtime_d3cold) {
if (dev->d3cold_delay)
-   msleep(dev->d3cold_delay);
+   mdelay(dev->d3cold_delay);
/*
 * When powering on a bridge from D3cold, the
 * whole hierarchy may be powered on into
-- 
1.7.9.5


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


Re: Fwd: linux video documentation ?

2017-10-10 Thread Brad Walker
Thanks for the quick pointer!!

-brad w.

On Mon, Oct 9, 2017 at 3:58 AM, Daniel Vetter  wrote:

> On Fri, Oct 06, 2017 at 01:44:48PM -0500, Brad Walker wrote:
> > I noticed this email address is listed as the relevant area for the
> > Documentation/devicetree/bindings/video/ directory in the Linux kernel.
> I
> > have a question about this.
> >
> > I noticed the Documentation/devicetree/bindings/video/ is no longer in
> the
> > Linux kernel repo. It was in release 4.3.6 but got dropped in release
> > v4.4-rc1.
> >
> > Is there a reason for this? I ask because I am using the
> > "simple-framebuffer" and wanted to possibly make some improvements to the
> > documentation.
>
> video/ moved to display/, it's all still there.
> -Daniel
>
> >
> > Thanks for any insight you can provide.
> >
> > -brad w.
>
> > ___
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] pci: Fix a possible sleep-in-atomic bug in pci_set_power_state

2017-10-10 Thread Bjorn Helgaas
[+cc Rafael, linux-pm]

Hi Jia-Ju,

On Mon, Oct 09, 2017 at 04:16:20PM +0800, Jia-Ju Bai wrote:
> The drivers vt6655 and gma500 call pci_set_power_state under a spinlock, 
> which may sleep.
> The function call paths are:
> gma_power_begin (acquire the spinlock) (drivers/gpu/drm/gma500/power.c)
>   gma_resume_pci
> pci_set_power_state
>   __pci_start_power_transition (drivers/pci/pci.c)
> msleep --> may sleep
> 
> gma_power_begin (acquire the spinlock) (drivers/gpu/drm/gma500/power.c)
>   gma_resume_pci
> pci_enable_device
>   pci_enable_device_flags (drivers/pci/pci.c)
> do_pci_enable_device
>   pci_set_power_state
> __pci_start_power_transition
>   msleep --> may sleep
> 
> vt6655_suspend (acquire the spinlock) (drivers/staging/vt6655/device_main.c)
>   pci_set_power_state
> __pci_start_power_transition (drivers/pci/pci.c)
>   msleep --> may sleep
> 
> To fix these bugs, msleep is replaced with mdelay in 
> __pci_start_power_transition
> 
> These bugs are found by my static analysis tool and my code review.

We can either

  - change pci_set_power_state() so it can be called while holding a
spinlock (as this patch does), or

  - change the drivers so they don't hold the spinlock while calling 
pci_set_power_state().

I think the latter is better because d3cold_delay is typically 100ms,
and that's a long time to spin with interrupts disabled.

I assume it's easy to produce an actual failure here?  Why haven't we
seen bug reports about this?

Bjorn

> Signed-off-by: Jia-Ju Bai 
> ---
>  drivers/pci/pci.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 6078dfc..7b763a3 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -823,7 +823,7 @@ static void __pci_start_power_transition(struct pci_dev 
> *dev, pci_power_t state)
>*/
>   if (dev->runtime_d3cold) {
>   if (dev->d3cold_delay)
> - msleep(dev->d3cold_delay);
> + mdelay(dev->d3cold_delay);
>   /*
>* When powering on a bridge from D3cold, the
>* whole hierarchy may be powered on into
> -- 
> 1.7.9.5
> 
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4] drm/bridge/sii8620: add remote control support

2017-10-10 Thread Sean Young
Hi,

On Thu, Sep 21, 2017 at 12:46:04PM +0100, Sean Young wrote:
> On Mon, Sep 18, 2017 at 04:37:52PM +0200, Hans Verkuil wrote:
> > On 09/18/2017 04:15 PM, Maciej Purski wrote:
> > > Hi Hans,
> > > some time ago in reply to your email I described what messages does
> > > the MHL driver receive and at what time intervals.
> > > Regarding that information, do you think that a similar solution as
> > > in [1] is required? Would it be OK, if I just set REP_DELAY and REP_PERIOD
> > > to values, which I presented in my last email?
> > 
> > Based on the timings you measured I would say that there is a 99% chance 
> > that MHL
> > uses exactly the same "Press and Hold" mechanism as CEC. Since you already 
> > specify
> > RC_PROTO_BIT_CEC in the driver, it will set REP_DELAY and REP_PERIOD to the 
> > right
> > values automatically.
> > 
> > You will have to implement the same code as in cec-adap.c for the key press 
> > handling,
> > though. It's a bit tricky to get it right.
> > 
> > Since you will have to do the same thing as I do in CEC, I wonder if it 
> > would make
> > more sense to move this code to the RC core. Basically it ensures that 
> > repeat mode
> > doesn't turn on until you get two identical key presses 550ms or less 
> > apart. This
> > is independent of REP_DELAY which can be changed by userspace.
> > 
> > Sean, what do you think?
> 
> Yes, this makes sense. In fact, since IR protocols have different repeat
> times, I would like to make this protocol dependant anyway.
> 
> To be honest I find REP_DELAY of 500ms too long and REP_PERIOD of 125ms
> too short; IOW it takes too long for a key to start repeating, and once
> it starts repeating it is very hard to control. If I try to increase the
> volume with my remote it first hardly changes at all and then after 500ms
> the volume shoots up far too quickly, same thing when navigating menus.
> 
> CEC dictates a delay period of 550ms, which is not great for the user IMO.
> 
> Anyway I'll have a look at this over the weekend.

I have started on this, but I haven't gotten it in a state where I'm
happy to submit it. I hope to have it ready for v4.15; in the mean time,
there is no reason to block this patch on this.


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


[PATCH v4] drm/i915: Replace *_reference/unreference() or *_ref/unref with _get/put()

2017-10-10 Thread Harsha Sharma
Replace instances of drm_framebuffer_reference/unreference() with
*_get/put() suffixes and drm_dev_unref with *_put() suffix
because get/put is shorter and consistent with the
kernel use of *_get/put suffixes.
Done with following coccinelle semantic patch

@@
expression ex;
@@

(
-drm_framebuffer_unreference(ex);
+drm_framebuffer_put(ex);
|
-drm_dev_unref(ex);
+drm_dev_put(ex);
|
-drm_framebuffer_reference(ex);
+drm_framebuffer_get(ex);
)

Signed-off-by: Harsha Sharma 
---
Changes in v4:
 -change one instance of *_put to *_get
Changes in v3:
 -Removed changes in selftests
Changes in v2:
 -Added cocinelle patch in log message
 -cc to all driver-specific mailing lists

 drivers/gpu/drm/i915/i915_pci.c  |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 10 +-
 drivers/gpu/drm/i915/intel_fbdev.c   |  4 ++--
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 09d97e0990b7..2f106cca46b4 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -510,7 +510,7 @@ static void i915_pci_remove(struct pci_dev *pdev)
struct drm_device *dev = pci_get_drvdata(pdev);
 
i915_driver_unload(dev);
-   drm_dev_unref(dev);
+   drm_dev_put(dev);
 }
 
 static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id 
*ent)
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 64f7b51ed97c..cb62f7e5d59a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2856,7 +2856,7 @@ intel_find_initial_plane_obj(struct intel_crtc 
*intel_crtc,
 
if (intel_plane_ggtt_offset(state) == plane_config->base) {
fb = c->primary->fb;
-   drm_framebuffer_reference(fb);
+   drm_framebuffer_get(fb);
goto valid_fb;
}
}
@@ -2887,7 +2887,7 @@ intel_find_initial_plane_obj(struct intel_crtc 
*intel_crtc,
  intel_crtc->pipe, PTR_ERR(intel_state->vma));
 
intel_state->vma = NULL;
-   drm_framebuffer_unreference(fb);
+   drm_framebuffer_put(fb);
return;
}
 
@@ -2908,7 +2908,7 @@ intel_find_initial_plane_obj(struct intel_crtc 
*intel_crtc,
if (i915_gem_object_is_tiled(obj))
dev_priv->preserve_bios_swizzle = true;
 
-   drm_framebuffer_reference(fb);
+   drm_framebuffer_get(fb);
primary->fb = primary->state->fb = fb;
primary->crtc = primary->state->crtc = &intel_crtc->base;
 
@@ -9847,7 +9847,7 @@ mode_fits_in_fbdev(struct drm_device *dev,
if (obj->base.size < mode->vdisplay * fb->pitches[0])
return NULL;
 
-   drm_framebuffer_reference(fb);
+   drm_framebuffer_get(fb);
return fb;
 #else
return NULL;
@@ -10028,7 +10028,7 @@ int intel_get_load_detect_pipe(struct drm_connector 
*connector,
if (ret)
goto fail;
 
-   drm_framebuffer_unreference(fb);
+   drm_framebuffer_put(fb);
 
ret = drm_atomic_set_mode_for_crtc(&crtc_state->base, mode);
if (ret)
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c 
b/drivers/gpu/drm/i915/intel_fbdev.c
index 262e75c00dd2..e34334a1fbf9 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -189,7 +189,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
  " releasing it\n",
  intel_fb->base.width, intel_fb->base.height,
  sizes->fb_width, sizes->fb_height);
-   drm_framebuffer_unreference(&intel_fb->base);
+   drm_framebuffer_put(&intel_fb->base);
intel_fb = ifbdev->fb = NULL;
}
if (!intel_fb || WARN_ON(!intel_fb->obj)) {
@@ -624,7 +624,7 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
ifbdev->preferred_bpp = fb->base.format->cpp[0] * 8;
ifbdev->fb = fb;
 
-   drm_framebuffer_reference(&ifbdev->fb->base);
+   drm_framebuffer_get(&ifbdev->fb->base);
 
/* Final pass to check if any active pipes don't have fbs */
for_each_crtc(dev, crtc) {
-- 
2.11.0

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


Re: [PATCH] drm/rockchip: Rely on the default best_encoder() behavior

2017-10-10 Thread Haneen Mohammed
On Wed, Sep 27, 2017 at 04:06:21PM -0400, Sean Paul wrote:
> On Wed, Sep 27, 2017 at 12:23:17PM -0600, Haneen Mohammed wrote:
> > Since the output has 1:1 relationship between connectors and encoders,
> > and the driver is relying on the atomic helpers, remove the custom
> > best_encoder() and let the core call drm_atomic_helper_best_encoder().
> > 
> > Signed-off-by: Haneen Mohammed 
> 
> Thanks for the patch, I've applied it to drm-misc-next.
> 
> I noticed a few instances of .best_encoder = drm_atomic_helper_best_encoder
> hanging around. Any interest in removing those as well?
> 
> Sean
> 
> 

I have found three instances, the one in bridge/synopsys/sw-hdmi.c I
checked and the log msg of commit c2a441fe8f7 mentioned that an explicit
assginment to the best_encoder is needed because the bridge is compatible
with both atomic and non-atomic devices.

I suspect this is the same case for vmwgfx/vmwgfx_/ldu/scrn/stdu.c from
checking commit d947d1b71.

Lastly, there is an instance in tinydrm/core/tinydrm-pipe.c but I am not
sure how to verify if the driver supports atomic and non-atomic devices
as well, hence justify deleting it. 

- Haneen

> > ---
> >  drivers/gpu/drm/rockchip/cdn-dp-core.c | 9 -
> >  1 file changed, 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c 
> > b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> > index a57da05..275844d 100644
> > --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
> > +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> > @@ -287,14 +287,6 @@ static int cdn_dp_connector_get_modes(struct 
> > drm_connector *connector)
> > return ret;
> >  }
> >  
> > -static struct drm_encoder *
> > -cdn_dp_connector_best_encoder(struct drm_connector *connector)
> > -{
> > -   struct cdn_dp_device *dp = connector_to_dp(connector);
> > -
> > -   return &dp->encoder;
> > -}
> > -
> >  static int cdn_dp_connector_mode_valid(struct drm_connector *connector,
> >struct drm_display_mode *mode)
> >  {
> > @@ -346,7 +338,6 @@ static int cdn_dp_connector_mode_valid(struct 
> > drm_connector *connector,
> >  
> >  static struct drm_connector_helper_funcs cdn_dp_connector_helper_funcs = {
> > .get_modes = cdn_dp_connector_get_modes,
> > -   .best_encoder = cdn_dp_connector_best_encoder,
> > .mode_valid = cdn_dp_connector_mode_valid,
> >  };
> >  
> > -- 
> > 2.7.4
> > 
> 
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] pci: Fix a possible sleep-in-atomic bug in pci_set_power_state

2017-10-10 Thread Rafael J. Wysocki
On Mon, Oct 9, 2017 at 7:18 PM, Bjorn Helgaas  wrote:
> On Mon, Oct 09, 2017 at 12:15:17PM -0500, Bjorn Helgaas wrote:
>> [+cc Rafael, linux-pm]
>>
>> Hi Jia-Ju,
>>
>> On Mon, Oct 09, 2017 at 04:16:20PM +0800, Jia-Ju Bai wrote:
>> > The drivers vt6655 and gma500 call pci_set_power_state under a spinlock, 
>> > which may sleep.
>> > The function call paths are:
>> > gma_power_begin (acquire the spinlock) (drivers/gpu/drm/gma500/power.c)
>> >   gma_resume_pci
>> > pci_set_power_state
>> >   __pci_start_power_transition (drivers/pci/pci.c)
>> > msleep --> may sleep
>> >
>> > gma_power_begin (acquire the spinlock) (drivers/gpu/drm/gma500/power.c)
>> >   gma_resume_pci
>> > pci_enable_device
>> >   pci_enable_device_flags (drivers/pci/pci.c)
>> > do_pci_enable_device
>> >   pci_set_power_state
>> > __pci_start_power_transition
>> >   msleep --> may sleep
>> >
>> > vt6655_suspend (acquire the spinlock) 
>> > (drivers/staging/vt6655/device_main.c)
>> >   pci_set_power_state
>> > __pci_start_power_transition (drivers/pci/pci.c)
>> >   msleep --> may sleep
>> >
>> > To fix these bugs, msleep is replaced with mdelay in 
>> > __pci_start_power_transition
>> >
>> > These bugs are found by my static analysis tool and my code review.
>>
>> We can either
>>
>>   - change pci_set_power_state() so it can be called while holding a
>> spinlock (as this patch does), or
>>
>>   - change the drivers so they don't hold the spinlock while calling
>> pci_set_power_state().
>>
>> I think the latter is better because d3cold_delay is typically 100ms,
>> and that's a long time to spin with interrupts disabled.
>>
>> I assume it's easy to produce an actual failure here?  Why haven't we
>> seen bug reports about this?
>
> Sigh, could have saved myself some time if I'd read the whole thread
> before responding :)  Sorry for repeating what Greg already said!

Well, calling pci_set_power_state() with a spinlock held is a bug,
plain and simple, among other things because it may involve running
AML.  Messing up with the single delay in it simply doesn't help.

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


Re: [PATCH] pci: Fix a possible sleep-in-atomic bug in pci_set_power_state

2017-10-10 Thread Jia-Ju Bai

Oh, sorry, I will send the patches for each driver.


Thanks,
Jia-Ju Bai

On 2017/10/9 16:17, Greg KH wrote:

On Mon, Oct 09, 2017 at 04:16:20PM +0800, Jia-Ju Bai wrote:

The drivers vt6655 and gma500 call pci_set_power_state under a spinlock, which 
may sleep.
The function call paths are:
gma_power_begin (acquire the spinlock) (drivers/gpu/drm/gma500/power.c)
   gma_resume_pci
 pci_set_power_state
   __pci_start_power_transition (drivers/pci/pci.c)
 msleep --> may sleep

gma_power_begin (acquire the spinlock) (drivers/gpu/drm/gma500/power.c)
   gma_resume_pci
 pci_enable_device
   pci_enable_device_flags (drivers/pci/pci.c)
 do_pci_enable_device
   pci_set_power_state
 __pci_start_power_transition
   msleep --> may sleep

vt6655_suspend (acquire the spinlock) (drivers/staging/vt6655/device_main.c)
   pci_set_power_state
 __pci_start_power_transition (drivers/pci/pci.c)
   msleep --> may sleep

To fix these bugs, msleep is replaced with mdelay in 
__pci_start_power_transition

These bugs are found by my static analysis tool and my code review.

Wait, no, why not fix the callers to not have a spinlock.  Those are the
only users of these calls that are doing so incorrectly, don't change
the PCI core for the fault of 2 broken drivers.

thanks,

greg k-h



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


Re: [PATCH] pci: Fix a possible sleep-in-atomic bug in pci_set_power_state

2017-10-10 Thread Bjorn Helgaas
On Mon, Oct 09, 2017 at 12:15:17PM -0500, Bjorn Helgaas wrote:
> [+cc Rafael, linux-pm]
> 
> Hi Jia-Ju,
> 
> On Mon, Oct 09, 2017 at 04:16:20PM +0800, Jia-Ju Bai wrote:
> > The drivers vt6655 and gma500 call pci_set_power_state under a spinlock, 
> > which may sleep.
> > The function call paths are:
> > gma_power_begin (acquire the spinlock) (drivers/gpu/drm/gma500/power.c)
> >   gma_resume_pci
> > pci_set_power_state
> >   __pci_start_power_transition (drivers/pci/pci.c)
> > msleep --> may sleep
> > 
> > gma_power_begin (acquire the spinlock) (drivers/gpu/drm/gma500/power.c)
> >   gma_resume_pci
> > pci_enable_device
> >   pci_enable_device_flags (drivers/pci/pci.c)
> > do_pci_enable_device
> >   pci_set_power_state
> > __pci_start_power_transition
> >   msleep --> may sleep
> > 
> > vt6655_suspend (acquire the spinlock) (drivers/staging/vt6655/device_main.c)
> >   pci_set_power_state
> > __pci_start_power_transition (drivers/pci/pci.c)
> >   msleep --> may sleep
> > 
> > To fix these bugs, msleep is replaced with mdelay in 
> > __pci_start_power_transition
> > 
> > These bugs are found by my static analysis tool and my code review.
> 
> We can either
> 
>   - change pci_set_power_state() so it can be called while holding a
> spinlock (as this patch does), or
> 
>   - change the drivers so they don't hold the spinlock while calling 
> pci_set_power_state().
> 
> I think the latter is better because d3cold_delay is typically 100ms,
> and that's a long time to spin with interrupts disabled.
> 
> I assume it's easy to produce an actual failure here?  Why haven't we
> seen bug reports about this?

Sigh, could have saved myself some time if I'd read the whole thread
before responding :)  Sorry for repeating what Greg already said!

> > Signed-off-by: Jia-Ju Bai 
> > ---
> >  drivers/pci/pci.c |2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 6078dfc..7b763a3 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -823,7 +823,7 @@ static void __pci_start_power_transition(struct pci_dev 
> > *dev, pci_power_t state)
> >  */
> > if (dev->runtime_d3cold) {
> > if (dev->d3cold_delay)
> > -   msleep(dev->d3cold_delay);
> > +   mdelay(dev->d3cold_delay);
> > /*
> >  * When powering on a bridge from D3cold, the
> >  * whole hierarchy may be powered on into
> > -- 
> > 1.7.9.5
> > 
> > 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 6/6] drm: Add four ioctls for managing drm mode object leases [v4]

2017-10-10 Thread Dave Airlie
> +*/
> +   ret = idr_alloc(&leases, &drm_lease_idr_object , object_id, 
> object_id + 1, GFP_KERNEL);

In current kernels, the idr_alloc warns if start < 0, so if object_id
is a negative signed integer, which your
igt tests actually make it.

So we get an ugly debug splat from the idr code, you should probably
make sure object_id is
something valid in signed space first, to avoid the splat.

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


Re: [dim PATCH] dim: allow a space separated list of URLs for each repo in drm_tip_repos

2017-10-10 Thread Andrzej Hajda
On 09.10.2017 12:30, Jani Nikula wrote:
> On Tue, 03 Oct 2017, Jani Nikula  wrote:
>> I merged this last week with Daniel's IRC ack. We'll need to give people
>> a little bit of time before updating nightly.conf. Sorry for the
>> inconvenience in the mean time.
> Andrzej, all the bits and pieces for this have been pushed, so https://
> should just work for all repos *except* Dave's drm tree. I don't know
> why, but [1] doesn't advertize https for it.
>
> BR,
> Jani.
>
>
> [1] https://cgit.freedesktop.org/~airlied/linux/
>
Sorry for late response, I have tested the code and it works, thanks.


Regards

Andrzej

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


[Bug 103175] R9285 Unreal tournament perf regression with agd5f 4.15-wip kernels possibly CPU related

2017-10-10 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=103175

--- Comment #3 from Michel Dänzer  ---
According to the HUD, the GPU load is actually higher in the bad cases. Have
you checked what clocks the GPU runs at in each case?

-- 
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 103175] R9285 Unreal tournament perf regression with agd5f 4.15-wip kernels possibly CPU related

2017-10-10 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=103175

--- Comment #4 from Michel Dänzer  ---
https://cgit.freedesktop.org/~agd5f/linux/commit/?h=amd-staging-drm-next&id=e5f6a57e350a7921e4edc30874679bdff11b13f4
might help.

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


[PATCH 2/4] drm/ttm: add transparent huge page support for cached allocations v2

2017-10-10 Thread Christian König
From: Christian König 

Try to allocate huge pages when it makes sense.

v2: avoid compound pages for now

Signed-off-by: Christian König 
Acked-by: Felix Kuehling 
Acked-by: Alex Deucher 
---
 drivers/gpu/drm/ttm/ttm_page_alloc.c | 50 ++--
 1 file changed, 42 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c 
b/drivers/gpu/drm/ttm/ttm_page_alloc.c
index 6c852e8..1bc6053 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
@@ -685,12 +685,24 @@ static void ttm_put_pages(struct page **pages, unsigned 
npages, int flags,
 
if (pool == NULL) {
/* No pool for this memory type so free the pages */
-   for (i = 0; i < npages; i++) {
-   if (pages[i]) {
-   if (page_count(pages[i]) != 1)
-   pr_err("Erroneous page count. Leaking 
pages.\n");
-   __free_page(pages[i]);
-   pages[i] = NULL;
+   i = 0;
+   while (i < npages) {
+   unsigned order;
+
+   if (!pages[i]) {
+   ++i;
+   continue;
+   }
+
+   if (page_count(pages[i]) != 1)
+   pr_err("Erroneous page count. Leaking 
pages.\n");
+   order = compound_order(pages[i]);
+   __free_pages(pages[i], order);
+
+   order = 1 << order;
+   while (order) {
+   pages[i++] = NULL;
+   --order;
}
}
return;
@@ -740,12 +752,33 @@ static int ttm_get_pages(struct page **pages, unsigned 
npages, int flags,
 
/* No pool for cached pages */
if (pool == NULL) {
+   unsigned i, j;
+
if (flags & TTM_PAGE_FLAG_DMA32)
gfp_flags |= GFP_DMA32;
else
gfp_flags |= GFP_HIGHUSER;
 
-   for (r = 0; r < npages; ++r) {
+   i = 0;
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+   while (npages >= HPAGE_PMD_NR) {
+   gfp_t huge_flags = gfp_flags;
+
+   huge_flags |= GFP_TRANSHUGE;
+   huge_flags &= ~__GFP_MOVABLE;
+   huge_flags &= ~__GFP_COMP;
+   p = alloc_pages(huge_flags, HPAGE_PMD_ORDER);
+   if (!p)
+   break;
+
+   for (j = 0; j < HPAGE_PMD_NR; ++j)
+   pages[i++] = p++;
+
+   npages -= HPAGE_PMD_NR;
+   }
+#endif
+
+   while (npages) {
p = alloc_page(gfp_flags);
if (!p) {
 
@@ -753,7 +786,8 @@ static int ttm_get_pages(struct page **pages, unsigned 
npages, int flags,
return -ENOMEM;
}
 
-   pages[r] = p;
+   pages[i++] = p;
+   --npages;
}
return 0;
}
-- 
2.7.4

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


[PATCH 1/4] drm/ttm: don't use compound pages for now

2017-10-10 Thread Christian König
From: Christian König 

We need to figure out first how to correctly map them into the CPU page tables.

bug: https://bugs.freedesktop.org/show_bug.cgi?id=103138
Signed-off-by: Christian König 
---
 drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c 
b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
index e5ef10d..96ad129 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
@@ -913,6 +913,7 @@ static gfp_t ttm_dma_pool_gfp_flags(struct ttm_dma_tt 
*ttm_dma, bool huge)
if (huge) {
gfp_flags |= GFP_TRANSHUGE;
gfp_flags &= ~__GFP_MOVABLE;
+   gfp_flags &= ~__GFP_COMP;
}
 
return gfp_flags;
-- 
2.7.4

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


[PATCH 3/4] drm/ttm: move more logic into ttm_page_pool_get_pages

2017-10-10 Thread Christian König
From: Christian König 

Make it easier to add huge page pool.

Signed-off-by: Christian König 
Acked-by: Alex Deucher 
---
 drivers/gpu/drm/ttm/ttm_page_alloc.c | 98 +++-
 1 file changed, 52 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c 
b/drivers/gpu/drm/ttm/ttm_page_alloc.c
index 1bc6053..3974732 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
@@ -627,19 +627,20 @@ static void ttm_page_pool_fill_locked(struct 
ttm_page_pool *pool,
 }
 
 /**
- * Cut 'count' number of pages from the pool and put them on the return list.
+ * Allocate pages from the pool and put them on the return list.
  *
- * @return count of pages still required to fulfill the request.
+ * @return zero for success or negative error code.
  */
-static unsigned ttm_page_pool_get_pages(struct ttm_page_pool *pool,
-   struct list_head *pages,
-   int ttm_flags,
-   enum ttm_caching_state cstate,
-   unsigned count)
+static int ttm_page_pool_get_pages(struct ttm_page_pool *pool,
+  struct list_head *pages,
+  int ttm_flags,
+  enum ttm_caching_state cstate,
+  unsigned count)
 {
unsigned long irq_flags;
struct list_head *p;
unsigned i;
+   int r = 0;
 
spin_lock_irqsave(&pool->lock, irq_flags);
ttm_page_pool_fill_locked(pool, ttm_flags, cstate, count, &irq_flags);
@@ -672,7 +673,35 @@ static unsigned ttm_page_pool_get_pages(struct 
ttm_page_pool *pool,
count = 0;
 out:
spin_unlock_irqrestore(&pool->lock, irq_flags);
-   return count;
+
+   /* clear the pages coming from the pool if requested */
+   if (ttm_flags & TTM_PAGE_FLAG_ZERO_ALLOC) {
+   struct page *page;
+
+   list_for_each_entry(page, pages, lru) {
+   if (PageHighMem(page))
+   clear_highpage(page);
+   else
+   clear_page(page_address(page));
+   }
+   }
+
+   /* If pool didn't have enough pages allocate new one. */
+   if (count) {
+   gfp_t gfp_flags = pool->gfp_flags;
+
+   /* set zero flag for page allocation if required */
+   if (ttm_flags & TTM_PAGE_FLAG_ZERO_ALLOC)
+   gfp_flags |= __GFP_ZERO;
+
+   /* ttm_alloc_new_pages doesn't reference pool so we can run
+* multiple requests in parallel.
+**/
+   r = ttm_alloc_new_pages(pages, gfp_flags, ttm_flags, cstate,
+   count);
+   }
+
+   return r;
 }
 
 /* Put all pages in pages list to correct pool to wait for reuse */
@@ -742,18 +771,18 @@ static int ttm_get_pages(struct page **pages, unsigned 
npages, int flags,
struct ttm_page_pool *pool = ttm_get_pool(flags, cstate);
struct list_head plist;
struct page *p = NULL;
-   gfp_t gfp_flags = GFP_USER;
unsigned count;
int r;
 
-   /* set zero flag for page allocation if required */
-   if (flags & TTM_PAGE_FLAG_ZERO_ALLOC)
-   gfp_flags |= __GFP_ZERO;
-
/* No pool for cached pages */
if (pool == NULL) {
+   gfp_t gfp_flags = GFP_USER;
unsigned i, j;
 
+   /* set zero flag for page allocation if required */
+   if (flags & TTM_PAGE_FLAG_ZERO_ALLOC)
+   gfp_flags |= __GFP_ZERO;
+
if (flags & TTM_PAGE_FLAG_DMA32)
gfp_flags |= GFP_DMA32;
else
@@ -792,44 +821,21 @@ static int ttm_get_pages(struct page **pages, unsigned 
npages, int flags,
return 0;
}
 
-   /* combine zero flag to pool flags */
-   gfp_flags |= pool->gfp_flags;
-
/* First we take pages from the pool */
INIT_LIST_HEAD(&plist);
-   npages = ttm_page_pool_get_pages(pool, &plist, flags, cstate, npages);
+   r = ttm_page_pool_get_pages(pool, &plist, flags, cstate, npages);
+
count = 0;
-   list_for_each_entry(p, &plist, lru) {
+   list_for_each_entry(p, &plist, lru)
pages[count++] = p;
-   }
-
-   /* clear the pages coming from the pool if requested */
-   if (flags & TTM_PAGE_FLAG_ZERO_ALLOC) {
-   list_for_each_entry(p, &plist, lru) {
-   if (PageHighMem(p))
-   clear_highpage(p);
-   else
-   clear_page(page_address(p));
-   }
-   }
 
-   /* If pool didn't have enough pages allocate new one. */
-   if (npages > 0) {
-   /* ttm_alloc_ne

[PATCH 4/4] drm/ttm: add transparent huge page support for wc or uc allocations v2

2017-10-10 Thread Christian König
From: Christian König 

Add a new huge page pool and try to allocate from it when it makes sense.

v2: avoid compound pages for now

Signed-off-by: Christian König 
Acked-by: Alex Deucher 
---
 drivers/gpu/drm/ttm/ttm_page_alloc.c | 136 ---
 1 file changed, 109 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c 
b/drivers/gpu/drm/ttm/ttm_page_alloc.c
index 3974732..b6f16e7ff 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
@@ -95,7 +95,7 @@ struct ttm_pool_opts {
unsignedsmall;
 };
 
-#define NUM_POOLS 4
+#define NUM_POOLS 6
 
 /**
  * struct ttm_pool_manager - Holds memory pools for fst allocation
@@ -122,6 +122,8 @@ struct ttm_pool_manager {
struct ttm_page_pooluc_pool;
struct ttm_page_poolwc_pool_dma32;
struct ttm_page_pooluc_pool_dma32;
+   struct ttm_page_poolwc_pool_huge;
+   struct ttm_page_pooluc_pool_huge;
} ;
};
 };
@@ -256,8 +258,8 @@ static int set_pages_array_uc(struct page **pages, int 
addrinarray)
 
 /**
  * Select the right pool or requested caching state and ttm flags. */
-static struct ttm_page_pool *ttm_get_pool(int flags,
-   enum ttm_caching_state cstate)
+static struct ttm_page_pool *ttm_get_pool(int flags, bool huge,
+ enum ttm_caching_state cstate)
 {
int pool_index;
 
@@ -269,9 +271,15 @@ static struct ttm_page_pool *ttm_get_pool(int flags,
else
pool_index = 0x1;
 
-   if (flags & TTM_PAGE_FLAG_DMA32)
+   if (flags & TTM_PAGE_FLAG_DMA32) {
+   if (huge)
+   return NULL;
pool_index |= 0x2;
 
+   } else if (huge) {
+   pool_index |= 0x4;
+   }
+
return &_manager->pools[pool_index];
 }
 
@@ -494,12 +502,14 @@ static void ttm_handle_caching_state_failure(struct 
list_head *pages,
  * pages returned in pages array.
  */
 static int ttm_alloc_new_pages(struct list_head *pages, gfp_t gfp_flags,
-   int ttm_flags, enum ttm_caching_state cstate, unsigned count)
+  int ttm_flags, enum ttm_caching_state cstate,
+  unsigned count, unsigned order)
 {
struct page **caching_array;
struct page *p;
int r = 0;
-   unsigned i, cpages;
+   unsigned i, j, cpages;
+   unsigned npages = 1 << order;
unsigned max_cpages = min(count,
(unsigned)(PAGE_SIZE/sizeof(struct page *)));
 
@@ -512,7 +522,7 @@ static int ttm_alloc_new_pages(struct list_head *pages, 
gfp_t gfp_flags,
}
 
for (i = 0, cpages = 0; i < count; ++i) {
-   p = alloc_page(gfp_flags);
+   p = alloc_pages(gfp_flags, order);
 
if (!p) {
pr_err("Unable to get page %u\n", i);
@@ -531,14 +541,18 @@ static int ttm_alloc_new_pages(struct list_head *pages, 
gfp_t gfp_flags,
goto out;
}
 
+   list_add(&p->lru, pages);
+
 #ifdef CONFIG_HIGHMEM
/* gfp flags of highmem page should never be dma32 so we
 * we should be fine in such case
 */
-   if (!PageHighMem(p))
+   if (PageHighMem(p))
+   continue;
+
 #endif
-   {
-   caching_array[cpages++] = p;
+   for (j = 0; j < npages; ++j) {
+   caching_array[cpages++] = p++;
if (cpages == max_cpages) {
 
r = ttm_set_pages_caching(caching_array,
@@ -552,8 +566,6 @@ static int ttm_alloc_new_pages(struct list_head *pages, 
gfp_t gfp_flags,
cpages = 0;
}
}
-
-   list_add(&p->lru, pages);
}
 
if (cpages) {
@@ -573,9 +585,9 @@ static int ttm_alloc_new_pages(struct list_head *pages, 
gfp_t gfp_flags,
  * Fill the given pool if there aren't enough pages and the requested number of
  * pages is small.
  */
-static void ttm_page_pool_fill_locked(struct ttm_page_pool *pool,
-   int ttm_flags, enum ttm_caching_state cstate, unsigned count,
-   unsigned long *irq_flags)
+static void ttm_page_pool_fill_locked(struct ttm_page_pool *pool, int 
ttm_flags,
+ enum ttm_caching_state cstate,
+ unsigned count, unsigned long *irq_flags)
 {
struct page *p;
int r;
@@ -605,7 +617,7 @@ static void ttm_page_pool_fill_locked(struct ttm_page_pool 
*pool,
 
INIT_LIST_HEAD(&new_pages);
r = ttm_alloc_new_pages(&new_pages, pool->gfp_flags, ttm_flags,
-   cstate, alloc_s

Re: [PATCH 3/3] drm: Add CRTC_GET_SEQUENCE and CRTC_QUEUE_SEQUENCE ioctls [v2]

2017-10-10 Thread Daniel Vetter
On Mon, Oct 09, 2017 at 10:18:30AM -0700, Keith Packard wrote:
> Daniel Vetter  writes:
> 
> > I just figured that -modesetting would be the simplest domenstration
> > vehicle, since the vulkan patches don't look ready yet. I need fully
> > reviewed&tested userspace before we can land any kernel stuff. Doing
> > the quick modesetting conversion would unblock.
> 
> I've provided a patch for the modesetting driver and the preliminary
> bits are merged, leaving only a fairly straightforward addition of the
> new ioctls to that code. I'm not sure how to make more progress there at
> this point; that code would need testing, and it requires a hand-patched
> kernel to test.
> 
> I also posted IGT tests for the new functionality, again, getting those
> reviewed and tested depends on someone willing to build a patched
> kernel. Dave Airlie has started trying to get that done.
> 
> >> (regarding FIRST_PIXEL_OUT):
> >>
> >> If the timestamp is the only important thing, it sounds like the kernel
> >> already satisfies that, which is cool.
> >
> > Would be good to confirm that. If it's not, we have a problem.
> 
> Michel Dänzer says that the timestamp provided is computed to be
> first_pixel_out for all hardware. Given that, I suggest we specify that
> in the documentation and remove this bit from the API.
> 
> Seem reasonable?

Ack on all. It looks like Dave is taking care of merging too, in that
case ack on all these patches from my side.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 1/2] drm/msm/dsi: Use correct pm_runtime_put variant during host_init

2017-10-10 Thread Daniel Vetter
On Tue, Oct 10, 2017 at 08:58:03AM +0530, Archit Taneja wrote:
> 
> 
> On 10/06/2017 07:32 PM, Daniel Vetter wrote:
> > On Fri, Oct 06, 2017 at 04:27:06PM +0530, Archit Taneja wrote:
> > > The DSI runtime PM suspend/resume callbacks check whether
> > > msm_host->cfg_hnd is non-NULL before trying to enable the bus clocks.
> > > This is done to accommodate early calls to these functions that may
> > > happen before the bus clocks are even initialized.
> > > 
> > > Calling pm_runtime_put_autosuspend() in dsi_host_init() can result in
> > > racy behaviour since msm_host->cfg_hnd is set very soon after. If the
> > > suspend callback happens too late, we end up trying to disable clocks
> > > that were never enabled, resulting in a bunch of WARN_ON splats.
> > 
> > Sounds like the correct fix here is to block autosuspend until after
> > everything is set up, including bus clocks. This patch just makes the race
> > harder to hit in practice ...
> 
> Thanks for the review. pm_runtime_put_sync() ensures that the suspend handler
> is called in the same context as that of the caller, right? msm_host->cfg_hnd
> is set to a non-NULL value only when we return from dsi_get_config(). The race
> would never happen in this case.
> 
> This call is a one time thing during DSI probe, we do a pm_runtime_get_sync()
> just so that we can read the block revision number. Once we have the revision
> number, we look at an internal table which maintains IP version specific
> resources, like what bus clocks to get, etc. Having 
> pm_runtime_put_autosuspend()
> here didn't help much anyway.

Maybe this stuff is different on arm than on pci, but on x86 you have to
explicitly enable autosuspend. Before that, the device stays on.

What I mean with properly fixing this, is to delay enabling of autosuspend
until you're fully set up. Which then allows you to drop the check for
clocks and other stuff.

For i915, what we do is hold an artificial runtime pm reference that we
grab first thing in ->probe and drop only once everything is fully set up
(including asynchronous workers that load firmware). That way we make sure
that our runtime pm code never sees a partially initiliazed driver.

Doing something similar sounds best too, i.e. instead of dropping the
runtime pm reference here, only drop it once dsi_get_config has been
called. Pronto, no race, no need for special functions.
-Daniel

> 
> Archit
> 
> 
> > -Daniel
> > 
> > > 
> > > Use pm_runtime_put_sync() so that the suspend callback is called
> > > immediately.
> > > 
> > > Reported-by: Nicolas Dechesne 
> > > Signed-off-by: Archit Taneja 
> > > ---
> > >   drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
> > > b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > > index dbb31a014419..deaf869374ea 100644
> > > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> > > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > > @@ -248,7 +248,7 @@ static const struct msm_dsi_cfg_handler 
> > > *dsi_get_config(
> > >   clk_disable_unprepare(ahb_clk);
> > >   disable_gdsc:
> > >   regulator_disable(gdsc_reg);
> > > - pm_runtime_put_autosuspend(dev);
> > > + pm_runtime_put_sync(dev);
> > >   put_clk:
> > >   clk_put(ahb_clk);
> > >   put_gdsc:
> > > -- 
> > > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> > > hosted by The Linux Foundation
> > > 
> > > ___
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> 
> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 103175] R9285 Unreal tournament perf regression with agd5f 4.15-wip kernels possibly CPU related

2017-10-10 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=103175

--- Comment #5 from Andy Furniss  ---
I had that fix and a couple of others on the first 4.15-wip I tried. They are
in now AFAICT. I see Alex updated again since yesterday so I'll try that one
later.

I did notice but forgot to highlight the GPU difference.
Given all the powerplay changes in 4.15-wip I do wonder if all is well but I
would have expected Unigine/Xonotic to show it if something was wrong there.

One thing different about UT is it asks for more vram than I have, maybe that
makes it stand out from other tests.

After posting this I did wonder whether the CPU difference is just because the
fps is low and not the cause.

4.15-wip doesn't work for me going away from head (will throw  some vmfaults
and then lock soon after startx) so bisecting is not an easy option.

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


Re: [PATCH] drm/rockchip: Rely on the default best_encoder() behavior

2017-10-10 Thread Daniel Vetter
On Mon, Oct 09, 2017 at 05:18:57PM -0600, Haneen Mohammed wrote:
> On Wed, Sep 27, 2017 at 04:06:21PM -0400, Sean Paul wrote:
> > On Wed, Sep 27, 2017 at 12:23:17PM -0600, Haneen Mohammed wrote:
> > > Since the output has 1:1 relationship between connectors and encoders,
> > > and the driver is relying on the atomic helpers, remove the custom
> > > best_encoder() and let the core call drm_atomic_helper_best_encoder().
> > > 
> > > Signed-off-by: Haneen Mohammed 
> > 
> > Thanks for the patch, I've applied it to drm-misc-next.
> > 
> > I noticed a few instances of .best_encoder = drm_atomic_helper_best_encoder
> > hanging around. Any interest in removing those as well?
> > 
> > Sean
> > 
> > 
> 
> I have found three instances, the one in bridge/synopsys/sw-hdmi.c I
> checked and the log msg of commit c2a441fe8f7 mentioned that an explicit
> assginment to the best_encoder is needed because the bridge is compatible
> with both atomic and non-atomic devices.

Yes, this is blocked on armada not yet being converted to atomic.

Might be good to add a comment to assignment in the code.

> I suspect this is the same case for vmwgfx/vmwgfx_/ldu/scrn/stdu.c from
> checking commit d947d1b71.

vmgfx is fully atomic.

> Lastly, there is an instance in tinydrm/core/tinydrm-pipe.c but I am not
> sure how to verify if the driver supports atomic and non-atomic devices
> as well, hence justify deleting it. 

Same for tinydrm.

You can check this by looking for drm_crtc_helper_set_config() - if a
driver is using that then it's a legacy one, if not, then it's atomic.
-Daniel

> 
> - Haneen
> 
> > > ---
> > >  drivers/gpu/drm/rockchip/cdn-dp-core.c | 9 -
> > >  1 file changed, 9 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c 
> > > b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> > > index a57da05..275844d 100644
> > > --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
> > > +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> > > @@ -287,14 +287,6 @@ static int cdn_dp_connector_get_modes(struct 
> > > drm_connector *connector)
> > >   return ret;
> > >  }
> > >  
> > > -static struct drm_encoder *
> > > -cdn_dp_connector_best_encoder(struct drm_connector *connector)
> > > -{
> > > - struct cdn_dp_device *dp = connector_to_dp(connector);
> > > -
> > > - return &dp->encoder;
> > > -}
> > > -
> > >  static int cdn_dp_connector_mode_valid(struct drm_connector *connector,
> > >  struct drm_display_mode *mode)
> > >  {
> > > @@ -346,7 +338,6 @@ static int cdn_dp_connector_mode_valid(struct 
> > > drm_connector *connector,
> > >  
> > >  static struct drm_connector_helper_funcs cdn_dp_connector_helper_funcs = 
> > > {
> > >   .get_modes = cdn_dp_connector_get_modes,
> > > - .best_encoder = cdn_dp_connector_best_encoder,
> > >   .mode_valid = cdn_dp_connector_mode_valid,
> > >  };
> > >  
> > > -- 
> > > 2.7.4
> > > 
> > 
> > -- 
> > Sean Paul, Software Engineer, Google / Chromium OS

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v5 2/2] staging: ion: create one device entry per heap

2017-10-10 Thread Mark Brown
On Mon, Oct 09, 2017 at 05:10:37PM -0700, Laura Abbott wrote:
> On 10/09/2017 03:08 PM, Mark Brown wrote:
> > On Mon, Oct 09, 2017 at 02:25:47PM -0700, Laura Abbott wrote:

> >> Anyway, to move this forward I think we need to see a proof of concept
> >> of using selinux to protect access to specific heaps.

> > Aren't Unix permissions enough with separate files or am I
> > misunderstanding what you're looking to see a proof of concept for?

> The goal is to be able to restrict heap access to certain services
> and selinux groups on Android so straight unix permissions aren't
> sufficient.

Oh, there's Android users for this?  The users I was aware of were
non-Android.  Though even so I'd have thought that given that SELinux is
a superset of Unix file permissions it ought to be sufficient to be able
to use them.  I'd been thinking people were suggesting SELinux as a
replacement for file permissions, using the single file and the greater
capabilities of SELinux.


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


Re: [PATCH v2 4/9] drm/exynos: gsc: Convert driver to IPP v2 core API

2017-10-10 Thread Hoegeun Kwon

Hi Marek,

On 09/29/2017 04:32 PM, Marek Szyprowski wrote:

This patch adapts Exynos DRM rotator driver to new IPP v2 core API.
The side effect of this conversion is a switch to driver component API
to register properly in the Exynos DRM core.

Signed-off-by: Marek Szyprowski 
Tested-by: Hoegeun Kwon 
---
  drivers/gpu/drm/exynos/Kconfig  |   3 +-
  drivers/gpu/drm/exynos/exynos_drm_drv.c |   1 +
  drivers/gpu/drm/exynos/exynos_drm_gsc.c | 853 
  drivers/gpu/drm/exynos/exynos_drm_gsc.h |  24 -
  4 files changed, 198 insertions(+), 683 deletions(-)
  delete mode 100644 drivers/gpu/drm/exynos/exynos_drm_gsc.h



...


+static int gsc_bind(struct device *dev, struct device *master, void *data)
+{
+   struct gsc_context *ctx = dev_get_drvdata(dev);
+   struct drm_device *drm_dev = data;
+   struct exynos_drm_ipp *ipp = &ctx->ipp;
+
+   ctx->drm_dev = drm_dev;
+   drm_iommu_attach_device(drm_dev, dev);
+
+   exynos_drm_ipp_register(drm_dev, ipp, &ipp_funcs,
+  DRM_EXYNOS_IPP_CAP_CROP | DRM_EXYNOS_IPP_CAP_ROTATE |
+  DRM_EXYNOS_IPP_CAP_SCALE | 
DRM_EXYNOS_IPP_CAP_CONVERT,
+  gsc_formats, "gsc");
+
+   dev_info(dev, "The exynos gscaler is probed successfully\n");
+
+   return 0;
  }
  
+static void gsc_unbind(struct device *dev, struct device *master,

+   void *data)
+{
+   struct gsc_context *ctx = dev_get_drvdata(dev);


I think there is missing ipp_unregister() in unbind() of GSC and FIMC.

struct drm_device *drm_dev = data;
struct exynos_drm_ipp *ipp = &ctx->ipp;

exynos_drm_ipp_unregister(drm_dev, ipp);

Best regards,
Hoegeun


+
+   drm_iommu_detach_device(ctx->drm_dev, ctx->dev);
+}
+
+static const struct component_ops gsc_component_ops = {
+   .bind   = gsc_bind,
+   .unbind = gsc_unbind,
+};
+
+
  static int gsc_probe(struct platform_device *pdev)
  {
struct device *dev = &pdev->dev;
struct gsc_context *ctx;
struct resource *res;
-   struct exynos_drm_ippdrv *ippdrv;
int ret;
  
  	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);

if (!ctx)
return -ENOMEM;
  
-	if (dev->of_node) {

-   ctx->sysreg = syscon_regmap_lookup_by_phandle(dev->of_node,
-   "samsung,sysreg");
-   if (IS_ERR(ctx->sysreg)) {
-   dev_warn(dev, "failed to get system register.\n");
-   ctx->sysreg = NULL;
-   }
-   }
+   ctx->dev = dev;
  
  	/* clock control */

ctx->gsc_clk = devm_clk_get(dev, "gscl");
@@ -1699,8 +1260,8 @@ static int gsc_probe(struct platform_device *pdev)
}
  
  	ctx->irq = res->start;

-   ret = devm_request_threaded_irq(dev, ctx->irq, NULL, gsc_irq_handler,
-   IRQF_ONESHOT, "drm_gsc", ctx);
+   ret = devm_request_irq(dev, ctx->irq, gsc_irq_handler, 0,
+  dev_name(dev), ctx);
if (ret < 0) {
dev_err(dev, "failed to request irq.\n");
return ret;
@@ -1709,38 +1270,19 @@ static int gsc_probe(struct platform_device *pdev)
/* context initailization */
ctx->id = pdev->id;
  
-	ippdrv = &ctx->ippdrv;

-   ippdrv->dev = dev;
-   ippdrv->ops[EXYNOS_DRM_OPS_SRC] = &gsc_src_ops;
-   ippdrv->ops[EXYNOS_DRM_OPS_DST] = &gsc_dst_ops;
-   ippdrv->check_property = gsc_ippdrv_check_property;
-   ippdrv->reset = gsc_ippdrv_reset;
-   ippdrv->start = gsc_ippdrv_start;
-   ippdrv->stop = gsc_ippdrv_stop;
-   ret = gsc_init_prop_list(ippdrv);
-   if (ret < 0) {
-   dev_err(dev, "failed to init property list.\n");
-   return ret;
-   }
-
-   DRM_DEBUG_KMS("id[%d]ippdrv[%pK]\n", ctx->id, ippdrv);
-
-   mutex_init(&ctx->lock);
platform_set_drvdata(pdev, ctx);
  
  	pm_runtime_enable(dev);
  
-	ret = exynos_drm_ippdrv_register(ippdrv);

-   if (ret < 0) {
-   dev_err(dev, "failed to register drm gsc device.\n");
-   goto err_ippdrv_register;
-   }
+   ret = component_add(dev, &gsc_component_ops);
+   if (ret)
+   goto err_pm_dis;
  
  	dev_info(dev, "drm gsc registered successfully.\n");
  
  	return 0;
  
-err_ippdrv_register:

+err_pm_dis:
pm_runtime_disable(dev);
return ret;
  }
@@ -1748,11 +1290,6 @@ static int gsc_probe(struct platform_device *pdev)
  static int gsc_remove(struct platform_device *pdev)
  {
struct device *dev = &pdev->dev;
-   struct gsc_context *ctx = get_gsc_context(dev);
-   struct exynos_drm_ippdrv *ippdrv = &ctx->ippdrv;
-
-   exynos_drm_ippdrv_unregister(ippdrv);
-   mutex_destroy(&ctx->lock);
  
  	pm_runtime_set_suspended(dev);

pm_runtime_disable(dev);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.h 
b/drivers/gpu/drm/exynos/exynos_drm_g

[Bug 102926] Tonga agd5f drm-next-4.15-wip powerplay display artifacts.

2017-10-10 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=102926

Andy Furniss  changed:

   What|Removed |Added

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

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


Re: [PATCH v2 0/5] Simplify panel bridge cleanup

2017-10-10 Thread Benjamin Gaignard
2017-10-10 5:36 GMT+02:00 Archit Taneja :
>
>
> On 10/02/2017 03:02 PM, Benjamin Gaignard wrote:
>>
>> The goal of this series is to simplify driver code when they need to clean
>> up
>> a previously allocated panel bridge.
>> Few drivers have "is_panel_bridge" flag to be able to distinguish a
>> drm_panel_bridge from "simple" drm_bridge.
>> To remove this flag I propose to
>> - let drm_panel_bridge_remove() check if the bridge provided in parameter
>> is
>>really a drm_panel_bridge.
>> - add drm_of_panel_bridge_remove() to remove a bridge given DT port and
>>endpoint
>> Finally that allow to remove drm_bridge structure and "is_panel_bridge"
>> flag
>> from stm driver internal structure.
>>
>> version 2:
>> - does the same for vc4 and dw-mipi-dsi
>
> For the series:
>
> Reviewed-by: Archit Taneja 
>
> Feel free to queue to drm-misc-next.

Pushed thanks

>
> Thanks,
> Archit
>
>>
>> Benjamin Gaignard (5):
>>drm/bridge: make drm_panel_bridge_remove more robust
>>drm/drm_of: add drm_of_panel_bridge_remove function
>>drm/stm: ltdc: remove bridge from driver internal structure
>>drm/vc4: remove bridge from driver internal structure
>>drm/bridge/synopsys: dsi :remove is_panel_bridge
>>
>>   drivers/gpu/drm/bridge/panel.c| 10 +++-
>>   drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c |  5 +---
>>   drivers/gpu/drm/drm_of.c  | 33
>> +++
>>   drivers/gpu/drm/stm/ltdc.c| 16 -
>>   drivers/gpu/drm/stm/ltdc.h|  2 --
>>   drivers/gpu/drm/vc4/vc4_dpi.c | 17 +-
>>   include/drm/drm_of.h  |  8 +++
>>   7 files changed, 62 insertions(+), 29 deletions(-)
>>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project



-- 
Benjamin Gaignard

Graphic Study Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH libdrm] Add const qualifier to arguments of drmModeAddFB2()

2017-10-10 Thread Tobias Jakobi
Both drmModeAddFB2() and drmModeAddFB2WithModifiers() have some
arguments that are just pointers to uint32_t in disguise. These
are not modified (just copied) in the function, so we can add a
const qualifier here.

Signed-off-by: Tobias Jakobi 
---
 xf86drmMode.c | 10 +-
 xf86drmMode.h | 11 ++-
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/xf86drmMode.c b/xf86drmMode.c
index d3bc20ea..2b3887b3 100644
--- a/xf86drmMode.c
+++ b/xf86drmMode.c
@@ -271,9 +271,9 @@ int drmModeAddFB(int fd, uint32_t width, uint32_t height, 
uint8_t depth,
 }
 
 int drmModeAddFB2WithModifiers(int fd, uint32_t width, uint32_t height,
-   uint32_t pixel_format, uint32_t bo_handles[4],
-   uint32_t pitches[4], uint32_t offsets[4],
-   uint64_t modifier[4], uint32_t *buf_id, 
uint32_t flags)
+   uint32_t pixel_format, const uint32_t 
bo_handles[4],
+   const uint32_t pitches[4], const uint32_t 
offsets[4],
+   const uint64_t modifier[4], uint32_t *buf_id, 
uint32_t flags)
 {
struct drm_mode_fb_cmd2 f;
int ret;
@@ -297,8 +297,8 @@ int drmModeAddFB2WithModifiers(int fd, uint32_t width, 
uint32_t height,
 }
 
 int drmModeAddFB2(int fd, uint32_t width, uint32_t height,
-  uint32_t pixel_format, uint32_t bo_handles[4],
-  uint32_t pitches[4], uint32_t offsets[4],
+  uint32_t pixel_format, const uint32_t bo_handles[4],
+  const uint32_t pitches[4], const uint32_t offsets[4],
   uint32_t *buf_id, uint32_t flags)
 {
return drmModeAddFB2WithModifiers(fd, width, height,
diff --git a/xf86drmMode.h b/xf86drmMode.h
index 5b390d9f..6dbe3353 100644
--- a/xf86drmMode.h
+++ b/xf86drmMode.h
@@ -369,15 +369,16 @@ extern int drmModeAddFB(int fd, uint32_t width, uint32_t 
height, uint8_t depth,
uint32_t *buf_id);
 /* ...with a specific pixel format */
 extern int drmModeAddFB2(int fd, uint32_t width, uint32_t height,
-uint32_t pixel_format, uint32_t bo_handles[4],
-uint32_t pitches[4], uint32_t offsets[4],
+uint32_t pixel_format, const uint32_t bo_handles[4],
+const uint32_t pitches[4], const uint32_t offsets[4],
 uint32_t *buf_id, uint32_t flags);
 
 /* ...with format modifiers */
 int drmModeAddFB2WithModifiers(int fd, uint32_t width, uint32_t height,
-  uint32_t pixel_format, uint32_t bo_handles[4],
-  uint32_t pitches[4], uint32_t offsets[4],
-  uint64_t modifier[4], uint32_t *buf_id, uint32_t 
flags);
+  uint32_t pixel_format, const uint32_t 
bo_handles[4],
+  const uint32_t pitches[4], const uint32_t 
offsets[4],
+  const uint64_t modifier[4], uint32_t *buf_id,
+  uint32_t flags);
 
 /**
  * Destroies the given framebuffer.
-- 
2.13.5

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


[Bug 103142] R600g+sb: optimizer apparently stuck in an endless loop

2017-10-10 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=103142

--- Comment #3 from Gert Wollny  ---
Created attachment 134771
  --> https://bugs.freedesktop.org/attachment.cgi?id=134771&action=edit
Patch to fix shader in UE4 (added for completeness)

-- 
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 103142] R600g+sb: optimizer apparently stuck in an endless loop

2017-10-10 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=103142

--- Comment #4 from Gert Wollny  ---
The bug can be triggered on BARTS (6850 HD) by using the Unreal Engine 4.17.1
or 4.18.0-pre3 with this patch applied [1] to fix a shader that otherwise would
use too many registers.

(Note that in mesa-debug mode UE4Editor will likely crash at one point because
of 
#102387 [2]) 

[1] Attached "Patch to fix shader in UE4"
[2] https://bugs.freedesktop.org/show_bug.cgi?id=102387

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


Re: [PATCH libdrm] Add const qualifier to arguments of drmModeAddFB2()

2017-10-10 Thread Eric Engestrom
On Tuesday, 2017-10-10 10:12:52 +, Tobias Jakobi wrote:
> Both drmModeAddFB2() and drmModeAddFB2WithModifiers() have some
> arguments that are just pointers to uint32_t in disguise. These
> are not modified (just copied) in the function, so we can add a
> const qualifier here.
> 
> Signed-off-by: Tobias Jakobi 

Reviewed-by: Eric Engestrom 

> ---
>  xf86drmMode.c | 10 +-
>  xf86drmMode.h | 11 ++-
>  2 files changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/xf86drmMode.c b/xf86drmMode.c
> index d3bc20ea..2b3887b3 100644
> --- a/xf86drmMode.c
> +++ b/xf86drmMode.c
> @@ -271,9 +271,9 @@ int drmModeAddFB(int fd, uint32_t width, uint32_t height, 
> uint8_t depth,
>  }
>  
>  int drmModeAddFB2WithModifiers(int fd, uint32_t width, uint32_t height,
> -   uint32_t pixel_format, uint32_t bo_handles[4],
> -   uint32_t pitches[4], uint32_t offsets[4],
> -   uint64_t modifier[4], uint32_t *buf_id, 
> uint32_t flags)
> +   uint32_t pixel_format, const uint32_t 
> bo_handles[4],
> +   const uint32_t pitches[4], const uint32_t 
> offsets[4],
> +   const uint64_t modifier[4], uint32_t *buf_id, 
> uint32_t flags)
>  {
>   struct drm_mode_fb_cmd2 f;
>   int ret;
> @@ -297,8 +297,8 @@ int drmModeAddFB2WithModifiers(int fd, uint32_t width, 
> uint32_t height,
>  }
>  
>  int drmModeAddFB2(int fd, uint32_t width, uint32_t height,
> -  uint32_t pixel_format, uint32_t bo_handles[4],
> -  uint32_t pitches[4], uint32_t offsets[4],
> +  uint32_t pixel_format, const uint32_t bo_handles[4],
> +  const uint32_t pitches[4], const uint32_t offsets[4],
>uint32_t *buf_id, uint32_t flags)
>  {
>   return drmModeAddFB2WithModifiers(fd, width, height,
> diff --git a/xf86drmMode.h b/xf86drmMode.h
> index 5b390d9f..6dbe3353 100644
> --- a/xf86drmMode.h
> +++ b/xf86drmMode.h
> @@ -369,15 +369,16 @@ extern int drmModeAddFB(int fd, uint32_t width, 
> uint32_t height, uint8_t depth,
>   uint32_t *buf_id);
>  /* ...with a specific pixel format */
>  extern int drmModeAddFB2(int fd, uint32_t width, uint32_t height,
> -  uint32_t pixel_format, uint32_t bo_handles[4],
> -  uint32_t pitches[4], uint32_t offsets[4],
> +  uint32_t pixel_format, const uint32_t bo_handles[4],
> +  const uint32_t pitches[4], const uint32_t offsets[4],
>uint32_t *buf_id, uint32_t flags);
>  
>  /* ...with format modifiers */
>  int drmModeAddFB2WithModifiers(int fd, uint32_t width, uint32_t height,
> -uint32_t pixel_format, uint32_t bo_handles[4],
> -uint32_t pitches[4], uint32_t offsets[4],
> -uint64_t modifier[4], uint32_t *buf_id, uint32_t 
> flags);
> +uint32_t pixel_format, const uint32_t 
> bo_handles[4],
> +const uint32_t pitches[4], const uint32_t 
> offsets[4],
> +const uint64_t modifier[4], uint32_t *buf_id,
> +uint32_t flags);
>  
>  /**
>   * Destroies the given framebuffer.
> -- 
> 2.13.5
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


drm/imx: lock scanout transfers for consecutive bursts

2017-10-10 Thread Patrick Brünn
Hi Philipp,

since commit 790cb4c7c9545953d22d3d425e49b36a711bae5b my display on CX9020 (a 
i.MX53 based machine) stopped working:

 # dmesg | grep -i drm
[0.141829] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[0.141838] [drm] No driver support for vblank timestamp query.
[0.142016] imx-drm display-subsystem: bound imx-ipuv3-crtc.2 (ops 
ipu_crtc_ops)
[0.142114] imx-drm display-subsystem: bound imx-ipuv3-crtc.3 (ops 
ipu_crtc_ops)
[0.142219] imx-drm display-subsystem: bound display-0 (ops imx_pd_ops)
[0.167491] drm: X: num_bursts: 8
[0.242633] imx-drm display-subsystem: fb0:  frame buffer device
[0.243172] [drm] Initialized imx-drm 1.0.0 20120507 for display-subsystem 
on minor 0
[   11.366172] [drm:drm_atomic_helper_commit_cleanup_done] *ERROR* 
[CRTC:28:crtc-0] flip_done timed out
[   21.606164] [drm:drm_atomic_helper_commit_cleanup_done] *ERROR* 
[CRTC:28:crtc-0] flip_done timed out

When I remove the new call to ipu_idmac_lock_enable(ipu_plane->ipu_ch, 
num_bursts) (attached patch),
"[drm:drm_atomic_helper_commit_cleanup_done] *ERROR* [CRTC:28:crtc-0] flip_done 
timed out"
messages disappear and I get my display back:
# dmesg | grep -i drm
[0.142292] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[0.142302] [drm] No driver support for vblank timestamp query.
[0.142484] imx-drm display-subsystem: bound imx-ipuv3-crtc.2 (ops 
ipu_crtc_ops)
[0.142587] imx-drm display-subsystem: bound imx-ipuv3-crtc.3 (ops 
ipu_crtc_ops)
[0.142693] imx-drm display-subsystem: bound display-0 (ops imx_pd_ops)
[0.167917] drm: X: num_bursts: 8
[0.243023] imx-drm display-subsystem: fb0:  frame buffer device
[0.243566] [drm] Initialized imx-drm 1.0.0 20120507 for display-subsystem 
on minor 0

Is your patch supposed to work on i.MX53, too? Did I miss some configuration 
[1], to make it work correctly?
If you need any additional info, just let me know.

Best Regards,
Patrick

[1] 
http://elixir.free-electrons.com/linux/v4.14-rc4/source/arch/arm/boot/dts/imx53-cx9020.dts#L29

---
 drivers/gpu/drm/imx/ipuv3-plane.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c 
b/drivers/gpu/drm/imx/ipuv3-plane.c
index cf98596c7ce1..ca9513a09cfc 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -610,7 +610,7 @@ static void ipu_plane_atomic_update(struct drm_plane *plane,
info = drm_format_info(fb->format->format);
ipu_calculate_bursts(width, info->cpp[0], fb->pitches[0],
 &burstsize, &num_bursts);
-
+printk("drm: X: num_bursts: %u\n", num_bursts);
ipu_cpmem_zero(ipu_plane->ipu_ch);
ipu_cpmem_set_resolution(ipu_plane->ipu_ch, width, height);
ipu_cpmem_set_fmt(ipu_plane->ipu_ch, fb->format->format);
@@ -685,7 +685,7 @@ static void ipu_plane_atomic_update(struct drm_plane *plane,
}
ipu_cpmem_set_buffer(ipu_plane->ipu_ch, 0, eba);
ipu_cpmem_set_buffer(ipu_plane->ipu_ch, 1, eba);
-   ipu_idmac_lock_enable(ipu_plane->ipu_ch, num_bursts);
+//XXX  ipu_idmac_lock_enable(ipu_plane->ipu_ch, num_bursts);
ipu_plane_enable(ipu_plane);
 }

--
Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff
Registered office: Verl, Germany | Register court: Guetersloh HRA 7075



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


[Bug 102581] 3D texture mipmapping broken

2017-10-10 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=102581

--- Comment #4 from Nicolai Hähnle  ---
Maybe an SI-specific bug? Looks like the bottom part of the screenshot
comparison for me on both Carrizo and Tonga (VI) with Git master.

-- 
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 102809] Rust shadows(?) flash random colours

2017-10-10 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=102809

--- Comment #10 from Nicolai Hähnle  ---
I can confirm similar weird color flashes as shown in the video when playing
the trace on RX Vega 64. So oddly enough, this seems Vega-specific, as I didn't
observe it on Raven. I'm going to take a closer look.

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


Re: [PATCH 1/2] drm/msm/dsi: Use correct pm_runtime_put variant during host_init

2017-10-10 Thread Rob Clark
On Tue, Oct 10, 2017 at 4:59 AM, Daniel Vetter  wrote:
> On Tue, Oct 10, 2017 at 08:58:03AM +0530, Archit Taneja wrote:
>>
>>
>> On 10/06/2017 07:32 PM, Daniel Vetter wrote:
>> > On Fri, Oct 06, 2017 at 04:27:06PM +0530, Archit Taneja wrote:
>> > > The DSI runtime PM suspend/resume callbacks check whether
>> > > msm_host->cfg_hnd is non-NULL before trying to enable the bus clocks.
>> > > This is done to accommodate early calls to these functions that may
>> > > happen before the bus clocks are even initialized.
>> > >
>> > > Calling pm_runtime_put_autosuspend() in dsi_host_init() can result in
>> > > racy behaviour since msm_host->cfg_hnd is set very soon after. If the
>> > > suspend callback happens too late, we end up trying to disable clocks
>> > > that were never enabled, resulting in a bunch of WARN_ON splats.
>> >
>> > Sounds like the correct fix here is to block autosuspend until after
>> > everything is set up, including bus clocks. This patch just makes the race
>> > harder to hit in practice ...
>>
>> Thanks for the review. pm_runtime_put_sync() ensures that the suspend handler
>> is called in the same context as that of the caller, right? msm_host->cfg_hnd
>> is set to a non-NULL value only when we return from dsi_get_config(). The 
>> race
>> would never happen in this case.
>>
>> This call is a one time thing during DSI probe, we do a pm_runtime_get_sync()
>> just so that we can read the block revision number. Once we have the revision
>> number, we look at an internal table which maintains IP version specific
>> resources, like what bus clocks to get, etc. Having 
>> pm_runtime_put_autosuspend()
>> here didn't help much anyway.
>
> Maybe this stuff is different on arm than on pci, but on x86 you have to
> explicitly enable autosuspend. Before that, the device stays on.

On arm, we cannot assume that clks are enabled ever unless we've
enabled them.  And unclocked register access is insta-reboot.
Although we probably shouldn't be using _autosuspend() on the display
side of things.  It makes sense for the gpu, where "booting up" the
gpu is a heavier process and we might want to keep things alive for a
bit longer incase another submit comes in.

BR,
-R

> What I mean with properly fixing this, is to delay enabling of autosuspend
> until you're fully set up. Which then allows you to drop the check for
> clocks and other stuff.
>
> For i915, what we do is hold an artificial runtime pm reference that we
> grab first thing in ->probe and drop only once everything is fully set up
> (including asynchronous workers that load firmware). That way we make sure
> that our runtime pm code never sees a partially initiliazed driver.
>
> Doing something similar sounds best too, i.e. instead of dropping the
> runtime pm reference here, only drop it once dsi_get_config has been
> called. Pronto, no race, no need for special functions.
> -Daniel
>
>>
>> Archit
>>
>>
>> > -Daniel
>> >
>> > >
>> > > Use pm_runtime_put_sync() so that the suspend callback is called
>> > > immediately.
>> > >
>> > > Reported-by: Nicolas Dechesne 
>> > > Signed-off-by: Archit Taneja 
>> > > ---
>> > >   drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +-
>> > >   1 file changed, 1 insertion(+), 1 deletion(-)
>> > >
>> > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
>> > > b/drivers/gpu/drm/msm/dsi/dsi_host.c
>> > > index dbb31a014419..deaf869374ea 100644
>> > > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
>> > > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
>> > > @@ -248,7 +248,7 @@ static const struct msm_dsi_cfg_handler 
>> > > *dsi_get_config(
>> > >   clk_disable_unprepare(ahb_clk);
>> > >   disable_gdsc:
>> > >   regulator_disable(gdsc_reg);
>> > > - pm_runtime_put_autosuspend(dev);
>> > > + pm_runtime_put_sync(dev);
>> > >   put_clk:
>> > >   clk_put(ahb_clk);
>> > >   put_gdsc:
>> > > --
>> > > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
>> > > Forum,
>> > > hosted by The Linux Foundation
>> > >
>> > > ___
>> > > dri-devel mailing list
>> > > dri-devel@lists.freedesktop.org
>> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>> >
>>
>> --
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>> a Linux Foundation Collaborative Project
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 103192] adytm404

2017-10-10 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=103192

Bug ID: 103192
   Summary: adytm404
   Product: DRI
   Version: XOrg git
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/amdkfd
  Assignee: dri-devel@lists.freedesktop.org
  Reporter: ogyaadya...@gmail.com

Created attachment 134772
  --> https://bugs.freedesktop.org/attachment.cgi?id=134772&action=edit
apaaja

asdadasdasdadasdadsd

-- 
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 102800] DRI_PRIME regression- radeon: Failed to allocate virtual address for buffer

2017-10-10 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=102800

Alex Deucher  changed:

   What|Removed |Added

 Attachment #134265|text/x-log  |text/plain
  mime type||

-- 
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 102800] DRI_PRIME regression- radeon: Failed to allocate virtual address for buffer

2017-10-10 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=102800

--- Comment #8 from Alex Deucher  ---
Created attachment 134779
  --> https://bugs.freedesktop.org/attachment.cgi?id=134779&action=edit
workaround to test

Does this patch fix the issue?

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


Re: drm/imx: lock scanout transfers for consecutive bursts

2017-10-10 Thread Philipp Zabel
Hi Patrick,

On Tue, 2017-10-10 at 10:24 +, Patrick Brünn wrote:
> Hi Philipp,
> 
> since commit 790cb4c7c9545953d22d3d425e49b36a711bae5b my display on CX9020 (a 
> i.MX53 based machine) stopped working:
> 
>  # dmesg | grep -i drm
> [0.141829] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> [0.141838] [drm] No driver support for vblank timestamp query.
> [0.142016] imx-drm display-subsystem: bound imx-ipuv3-crtc.2 (ops 
> ipu_crtc_ops)
> [0.142114] imx-drm display-subsystem: bound imx-ipuv3-crtc.3 (ops 
> ipu_crtc_ops)
> [0.142219] imx-drm display-subsystem: bound display-0 (ops imx_pd_ops)
> [0.167491] drm: X: num_bursts: 8
> [0.242633] imx-drm display-subsystem: fb0:  frame buffer device
> [0.243172] [drm] Initialized imx-drm 1.0.0 20120507 for display-subsystem 
> on minor 0
> [   11.366172] [drm:drm_atomic_helper_commit_cleanup_done] *ERROR* 
> [CRTC:28:crtc-0] flip_done timed out
> [   21.606164] [drm:drm_atomic_helper_commit_cleanup_done] *ERROR* 
> [CRTC:28:crtc-0] flip_done timed out
> 
> When I remove the new call to ipu_idmac_lock_enable(ipu_plane->ipu_ch, 
> num_bursts) (attached patch),
> "[drm:drm_atomic_helper_commit_cleanup_done] *ERROR* [CRTC:28:crtc-0] 
> flip_done timed out"
> messages disappear and I get my display back:
> # dmesg | grep -i drm
> [0.142292] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> [0.142302] [drm] No driver support for vblank timestamp query.
> [0.142484] imx-drm display-subsystem: bound imx-ipuv3-crtc.2 (ops 
> ipu_crtc_ops)
> [0.142587] imx-drm display-subsystem: bound imx-ipuv3-crtc.3 (ops 
> ipu_crtc_ops)
> [0.142693] imx-drm display-subsystem: bound display-0 (ops imx_pd_ops)
> [0.167917] drm: X: num_bursts: 8
> [0.243023] imx-drm display-subsystem: fb0:  frame buffer device
> [0.243566] [drm] Initialized imx-drm 1.0.0 20120507 for display-subsystem 
> on minor 0
> 
> Is your patch supposed to work on i.MX53, too? Did I miss some configuration 
> [1], to make it work correctly?
> If you need any additional info, just let me know.

Thank you for the report. I can confirm that on i.MX53 QSB, VGA output
breaks with the arbitration locking enabled: I see a picture, the timing
signals are stable, but it looks like bursts are getting lost. The image
data is not horizontally synchronised.

According to the reference manual, the IPU_IDMAC_LOCK_EN_1 register on
i.MX53 / IPUv3M should work the same as on i.MX6Q / IPUv3H. Searching
around a bit, I found patches that do enable 8-burst locking for IPUv3M
in some ancient Freescale BSP, but I don't know what prerequisites have
to be met for this to work properly.

I'll send a patch to disable this on all but i.MX6 / IPUv3H for now.

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


[PATCH] gpu: ipu-v3: Allow channel burst locking on i.MX6 only

2017-10-10 Thread Philipp Zabel
The IDMAC_LOCK_EN registers on i.MX51 have a different layout, and on
i.MX53 enabling the lock feature causes bursts to get lost. Restrict
enabling the burst lock feature to i.MX6.

Reported-by: Patrick Brünn 
Fixes: 790cb4c7c954 ("drm/imx: lock scanout transfers for consecutive bursts")
Signed-off-by: Philipp Zabel 
---
 drivers/gpu/ipu-v3/ipu-common.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/ipu-v3/ipu-common.c b/drivers/gpu/ipu-v3/ipu-common.c
index 6a573d21d3cc2..658fa2d3e40c2 100644
--- a/drivers/gpu/ipu-v3/ipu-common.c
+++ b/drivers/gpu/ipu-v3/ipu-common.c
@@ -405,6 +405,14 @@ int ipu_idmac_lock_enable(struct ipuv3_channel *channel, 
int num_bursts)
return -EINVAL;
}
 
+   /*
+* IPUv3EX / i.MX51 has a different register layout, and on IPUv3M /
+* i.MX53 channel arbitration locking doesn't seem to work properly.
+* Allow enabling the lock feature on IPUv3H / i.MX6 only.
+*/
+   if (bursts && ipu->ipu_type != IPUV3H)
+   return -EINVAL;
+
for (i = 0; i < ARRAY_SIZE(idmac_lock_en_info); i++) {
if (channel->num == idmac_lock_en_info[i].chnum)
break;
-- 
2.11.0

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


[Bug 101483] A10-8780P (Carrizo) + R7 M260/M265 does not boot without any "workaround" parameter.

2017-10-10 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=101483

--- Comment #17 from FFAB  ---
Upstream kernel test - kernel 4.14-rc4


boot-time +/- 40sec (message sound),
black screen
no net connection - neither cable nor wlan.
n o remote-login possible
-> no dmesg

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


[PATCH] drm/vblank: Fix flip event vblank count

2017-10-10 Thread Ville Syrjala
From: Ville Syrjälä 

On machines where the vblank interrupt fires some time after the start
of vblank (or we just manage to race with the vblank interrupt handler)
we will currently stuff a stale vblank counter value into the flip event,
and thus we'll prematurely complete the flip.

Switch over to drm_crtc_accurate_vblank_count() to make sure we have an
up to date counter value, crucially also remember to add the +1 so that
the delayed vblank interrupt won't complete the flip prematurely.

Cc: sta...@vger.kernel.org
Cc: Daniel Vetter 
Suggested-by: Daniel Vetter 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/drm_vblank.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 70f2b9593edc..f14e6c92e332 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -869,7 +869,7 @@ void drm_crtc_arm_vblank_event(struct drm_crtc *crtc,
assert_spin_locked(&dev->event_lock);
 
e->pipe = pipe;
-   e->event.sequence = drm_vblank_count(dev, pipe);
+   e->event.sequence = drm_crtc_accurate_vblank_count(crtc) + 1;
e->event.crtc_id = crtc->base.id;
list_add_tail(&e->base.link, &dev->vblank_event_list);
 }
-- 
2.13.6

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


[PATCH] drm/i915: remove redundant check on has_aliasing_ppgtt

2017-10-10 Thread Colin King
From: Colin Ian King 

There is a previous check to on has_aliasing_ppgtt that returns
0 if it is false, so it is impossible for has_aliasing_ppgtt to
be false on the final return of function intel_sanitize_enable_ppgtt,
so final return in the function always will return 1.  Hence the
redundant ternary operator can be replaced with a return 1.

Detected by CoverityScan, CID#1357136 ("Logically dead code")

Signed-off-by: Colin Ian King 
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 4c82ceb8d318..e1a318ea4327 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -188,7 +188,7 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private 
*dev_priv,
return 2;
}
 
-   return has_aliasing_ppgtt ? 1 : 0;
+   return 1;
 }
 
 static int ppgtt_bind_vma(struct i915_vma *vma,
-- 
2.14.1

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


Re: [PATCH] drm: tilcdc: Calculate the vrefresh if it is not set by userspace

2017-10-10 Thread Jyri Sarha

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

On 10/10/17 15:09, Kevin Hao wrote:
> The check for vrefresh has been removed by commit 11abbc9f39e0
> ("drm/tilcdc: Set framebuffer DMA address to HW only if CRTC is
> enabled"). But the userspace is really possible to request a display
> mode with vrefresh uninitialized, then it would cause the following
> calltrace:
>   [] (unwind_backtrace) from [] (show_stack+0x20/0x24)
>   [] (show_stack) from [] (dump_stack+0x20/0x28)
>   [] (dump_stack) from [] (__div0+0x20/0x28)
>   [] (__div0) from [] (Ldiv0+0x8/0x14)
>   [] (Ldiv0) from [] (tilcdc_crtc_update_fb+0xa8/0x1d4)
>   [] (tilcdc_crtc_update_fb) from [] 
> (tilcdc_plane_atomic_update+0x54/0x5c)
>   [] (tilcdc_plane_atomic_update) from [] 
> (drm_atomic_helper_commit_planes+0x1b4/0x270)
>   [] (drm_atomic_helper_commit_planes) from [] 
> (tilcdc_commit+0x58/0x84)
>   [] (tilcdc_commit) from [] (drm_atomic_commit+0x54/0x68)
>   [] (drm_atomic_commit) from [] 
> (drm_atomic_helper_set_config+0x68/0x9c)
>   [] (drm_atomic_helper_set_config) from [] 
> (__drm_mode_set_config_internal+0x60/0xe0)
>   [] (__drm_mode_set_config_internal) from [] 
> (drm_mode_setcrtc+0x3a8/0x4c0)
>   [] (drm_mode_setcrtc) from [] 
> (drm_ioctl_kernel+0x78/0xb0)
>   [] (drm_ioctl_kernel) from [] (drm_ioctl+0x1fc/0x328)
>   [] (drm_ioctl) from [] (vfs_ioctl+0x28/0x44)
>   [] (vfs_ioctl) from [] (do_vfs_ioctl+0x890/0x8ec)
>   [] (do_vfs_ioctl) from [] (SyS_ioctl+0x60/0x7c)
>   [] (SyS_ioctl) from [] (ret_fast_syscall+0x0/0x48)
> 
> So calculate the vrefresh in mode_fixup() to make sure there always
> have a valid vrefresh.
> 

The earlier check was there for the frame buffer updates when the
display is not yet enabled and the hwmode's vrefresh is not valid yet.
Now, with the lock protected enabled state variable check, this should
never happen.

Do you have a reliable way to reproduce the problem you are seeing?
Could try adding a test for the vrefresh validity in the
tilcdc_crtc_mode_valid() to see if that helps?

Adding the extra test in the mode validity check probably produces a
failed mode set attempt, which I guess is Ok if the mode has zero
vertical refresh time. But if this does not help, then the issue you are
seeing may be an indication of a synchronization problem somewhere.

Best regards,
Jyri

> Fixes: 11abbc9f39e0 ("drm/tilcdc: Set framebuffer DMA address to HW only if 
> CRTC is enabled")
> Cc:  # v4.11+
> Signed-off-by: Kevin Hao 
> ---
>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c 
> b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> index d2589f310437..16b020a9044c 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> @@ -690,6 +690,9 @@ static bool tilcdc_crtc_mode_fixup(struct drm_crtc *crtc,
>   adjusted_mode->flags &= ~DRM_MODE_FLAG_PHSYNC;
>   }
>  
> + if (!adjusted_mode->vrefresh)
> + adjusted_mode->vrefresh = drm_mode_vrefresh(adjusted_mode);
> +
>   return true;
>  }
>  
> 


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


Re: [PATCH] drm/i915: remove redundant check on has_aliasing_ppgtt

2017-10-10 Thread Joonas Lahtinen
On Tue, 2017-10-10 at 14:47 +0100, Colin King wrote:
> From: Colin Ian King 
> 
> There is a previous check to on has_aliasing_ppgtt that returns
> 0 if it is false, so it is impossible for has_aliasing_ppgtt to
> be false on the final return of function intel_sanitize_enable_ppgtt,
> so final return in the function always will return 1.  Hence the
> redundant ternary operator can be replaced with a return 1.
> 
> Detected by CoverityScan, CID#1357136 ("Logically dead code")
> 
> Signed-off-by: Colin Ian King 

Thanks, I took it a few steps further and removed the variable
altogether. I Cc'd you on the patch.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/i915: remove redundant check on has_aliasing_ppgtt

2017-10-10 Thread Colin Ian King
On 10/10/17 15:35, Joonas Lahtinen wrote:
> On Tue, 2017-10-10 at 14:47 +0100, Colin King wrote:
>> From: Colin Ian King 
>>
>> There is a previous check to on has_aliasing_ppgtt that returns
>> 0 if it is false, so it is impossible for has_aliasing_ppgtt to
>> be false on the final return of function intel_sanitize_enable_ppgtt,
>> so final return in the function always will return 1.  Hence the
>> redundant ternary operator can be replaced with a return 1.
>>
>> Detected by CoverityScan, CID#1357136 ("Logically dead code")
>>
>> Signed-off-by: Colin Ian King 
> 
> Thanks, I took it a few steps further and removed the variable
> altogether. I Cc'd you on the patch.

Yep, it's an improvement on my fix. Thanks for sorting that out.

Colin

> 
> Regards, Joonas
> 

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


Re: [PATCH 6/6] drm: Add four ioctls for managing drm mode object leases [v4]

2017-10-10 Thread Keith Packard
Dave Airlie  writes:

>> +*/
>> +   ret = idr_alloc(&leases, &drm_lease_idr_object , object_id, 
>> object_id + 1, GFP_KERNEL);
>
> In current kernels, the idr_alloc warns if start < 0, so if object_id
> is a negative signed integer, which your
> igt tests actually make it.
>
> So we get an ugly debug splat from the idr code, you should probably
> make sure object_id is
> something valid in signed space first, to avoid the splat.

Yeah, makes sense. I also found the vboxvideo driver in staging which
needed an API tweak.

I've pushed that as drm-lease-v5 and will plan on dumping that to the
list when you're done with -v4.

There are a couple of conflicts and another update needed when merging
leases and the new vblank ioctls together; once we've got one of the two
ready to go, I can rebase the other on top of that and fix things up.

-- 
-keith


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


Re: [PATCH 1/3] drm/dp: WARN about invalid/unknown link rates and bw codes

2017-10-10 Thread Manasi Navare
On Mon, Oct 09, 2017 at 12:29:57PM +0300, Jani Nikula wrote:
> Falling back to the lowest value is likely the only thing we can do, but
> doing it silently seems like a bad thing to do. Catch it early and make
> loud noises.
> 
> Cc: Alex Deucher 
> Cc: Thierry Reding 
> Cc: Rob Clark 
> Cc: Sean Paul 
> Cc: Manasi Navare 
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Jani Nikula 

Reviewed-by: Manasi Navare 

> ---
>  drivers/gpu/drm/drm_dp_helper.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 08af8d6b844b..dca21b5a03ec 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -137,8 +137,10 @@ EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay);
>  u8 drm_dp_link_rate_to_bw_code(int link_rate)
>  {
>   switch (link_rate) {
> - case 162000:
>   default:
> + WARN(1, "unknown DP link rate %d, using %x\n", link_rate,
> +  DP_LINK_BW_1_62);
> + case 162000:
>   return DP_LINK_BW_1_62;
>   case 27:
>   return DP_LINK_BW_2_7;
> @@ -151,8 +153,9 @@ EXPORT_SYMBOL(drm_dp_link_rate_to_bw_code);
>  int drm_dp_bw_code_to_link_rate(u8 link_bw)
>  {
>   switch (link_bw) {
> - case DP_LINK_BW_1_62:
>   default:
> + WARN(1, "unknown DP link bw code %x, using 162000\n", link_bw);
> + case DP_LINK_BW_1_62:
>   return 162000;
>   case DP_LINK_BW_2_7:
>   return 27;
> -- 
> 2.11.0
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 09/12] of: overlay: avoid race condition between applying multiple overlays

2017-10-10 Thread Rob Herring
On Wed, Oct 04, 2017 at 08:29:59PM -0700, Frank Rowand wrote:
> On 10/04/17 08:19, Rob Herring wrote:
> > On Mon, Oct 2, 2017 at 10:53 PM,   wrote:
> >> From: Frank Rowand 
> >>
> >> The process of applying an overlay consists of:
> >>   - unflatten an overlay FDT (flattened device tree) into an
> >> EDT (expanded device tree)
> >>   - fixup the phandle values in the overlay EDT to fit in a
> >> range above the phandle values in the live device tree
> >>   - create the overlay changeset to reflect the contents of
> >> the overlay EDT
> >>   - apply the overlay changeset, to modify the live device tree,
> >> potentially changing the maximum phandle value in the live
> >> device tree
> >>
> >> There is currently no protection against two overlay applies
> >> concurrently determining what range of phandle values are in use
> >> in the live device tree, and subsequently changing that range.
> >> Add a mutex to prevent multiple overlay applies from occurring
> >> simultaneously.
> >>
> >> Ignoring 2 checkpatch warnings: Prefer using '"%s...", __func__'
> >> so that the WARN() string will be more easily grepped.
> >>
> >> Signed-off-by: Frank Rowand 
> >> ---
> >>  drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c |  7 +++
> >>  drivers/of/overlay.c | 22 ++
> >>  drivers/of/unittest.c| 21 +
> >>  include/linux/of.h   | 19 +++
> >>  4 files changed, 69 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c 
> >> b/drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c
> >> index 7a7be0515bfd..c99f7924b1c6 100644
> >> --- a/drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c
> >> +++ b/drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c
> >> @@ -221,6 +221,11 @@ static void __init tilcdc_convert_slave_node(void)
> >> goto out;
> >> }
> >>
> >> +   /*
> >> +* protect from of_resolve_phandles() through of_overlay_apply()
> >> +*/
> >> +   of_overlay_mutex_lock();
> >> +
> > 
> > We can't be relying on callers to get the locking right...
> 
> Agreed.
> 
> 
> > 
> >> overlay = tilcdc_get_overlay(&kft);
> >> if (!overlay)
> >> goto out;
> >> @@ -256,6 +261,8 @@ static void __init tilcdc_convert_slave_node(void)
> >> pr_info("%s: ti,tilcdc,slave node successfully 
> >> converted\n",
> >> __func__);
> >>  out:
> >> +   of_overlay_mutex_unlock();
> >> +
> >> kfree_table_free(&kft);
> >> of_node_put(i2c);
> >> of_node_put(slave);
> >> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> >> index a0d3222febdc..4ed372af6ce7 100644
> >> --- a/drivers/of/overlay.c
> >> +++ b/drivers/of/overlay.c
> >> @@ -71,6 +71,28 @@ static int build_changeset_next_level(struct 
> >> overlay_changeset *ovcs,
> >> const struct device_node *overlay_node,
> >> bool is_symbols_node);
> >>
> >> +/*
> >> + * of_resolve_phandles() finds the largest phandle in the live tree.
> >> + * of_overlay_apply() may add a larger phandle to the live tree.
> >> + * Do not allow race between two overlays being applied simultaneously:
> >> + *mutex_lock(&of_overlay_phandle_mutex)
> >> + *of_resolve_phandles()
> >> + *of_overlay_apply()
> >> + *mutex_unlock(&of_overlay_phandle_mutex)
> > 
> > Why do these need to be separate functions? I think I mentioned it
> > before, but essentially overlay_data_add() should be part of the
> > overlay API. We may need to allow for callers to do each step, but
> > generally I think the interface should just be "apply this fdt blob".
> 
> Yes, that is where I want to end up.

So, is that not doable now? To put it another way, why does 
of_resolve_phandles need to be a separate call? Seems like an internal 
detail of how you apply an overlay to me.

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


[Bug 97942] [IGT][BYT/BXT] gem_mmap_gtt subtest basic-wc fails due to Test assertion failure

2017-10-10 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=97942

--- Comment #8 from Elizabeth  ---
(In reply to Humberto Israel Perez Rodriguez from comment #6)
> The following test cases failure on BXT with latest configuration
> 
> igt@gem_mmap@swap-bo
> igt@gem_mmap_gtt@coherency
> igt@gem_mmap_gtt@swap-copy
> igt@gem_mmap_gtt@swap-copy-odd
> igt@gem_mmap_gtt@swap-copy-xy

Please note that:
Test igt@gem_mmap_gtt@basic-wc is being follow on bug 102995 also.
Test igt@gem_mmap_gtt@coherency is being follow on bug 100587.
Test igt@gem_mmap@swap-bo is being follow on bug 101701.

A different bug should be used for swap-copy* tests:

igt@gem_mmap_gtt@swap-copy
igt@gem_mmap_gtt@swap-copy-odd
igt@gem_mmap_gtt@swap-copy-xy

Also there is no track of these (swap-copy*) on CI.

Thank you.

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


Re: [PATCH 09/12] of: overlay: avoid race condition between applying multiple overlays

2017-10-10 Thread Rob Herring
On Tue, Oct 10, 2017 at 2:39 PM, Frank Rowand  wrote:
> On 10/10/17 11:40, Rob Herring wrote:
>> On Wed, Oct 04, 2017 at 08:29:59PM -0700, Frank Rowand wrote:
>>> On 10/04/17 08:19, Rob Herring wrote:
 On Mon, Oct 2, 2017 at 10:53 PM,   wrote:
> From: Frank Rowand 
>
> The process of applying an overlay consists of:
>   - unflatten an overlay FDT (flattened device tree) into an
> EDT (expanded device tree)
>   - fixup the phandle values in the overlay EDT to fit in a
> range above the phandle values in the live device tree
>   - create the overlay changeset to reflect the contents of
> the overlay EDT
>   - apply the overlay changeset, to modify the live device tree,
> potentially changing the maximum phandle value in the live
> device tree
>
> There is currently no protection against two overlay applies
> concurrently determining what range of phandle values are in use
> in the live device tree, and subsequently changing that range.
> Add a mutex to prevent multiple overlay applies from occurring
> simultaneously.
>
> Ignoring 2 checkpatch warnings: Prefer using '"%s...", __func__'
> so that the WARN() string will be more easily grepped.
>
> Signed-off-by: Frank Rowand 
> ---
>  drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c |  7 +++
>  drivers/of/overlay.c | 22 ++
>  drivers/of/unittest.c| 21 +
>  include/linux/of.h   | 19 +++
>  4 files changed, 69 insertions(+)
>
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c 
> b/drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c
> index 7a7be0515bfd..c99f7924b1c6 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c
> @@ -221,6 +221,11 @@ static void __init tilcdc_convert_slave_node(void)
> goto out;
> }
>
> +   /*
> +* protect from of_resolve_phandles() through of_overlay_apply()
> +*/
> +   of_overlay_mutex_lock();
> +

 We can't be relying on callers to get the locking right...
>>>
>>> Agreed.
>>>
>>>

> overlay = tilcdc_get_overlay(&kft);
> if (!overlay)
> goto out;
> @@ -256,6 +261,8 @@ static void __init tilcdc_convert_slave_node(void)
> pr_info("%s: ti,tilcdc,slave node successfully 
> converted\n",
> __func__);
>  out:
> +   of_overlay_mutex_unlock();
> +
> kfree_table_free(&kft);
> of_node_put(i2c);
> of_node_put(slave);
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index a0d3222febdc..4ed372af6ce7 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -71,6 +71,28 @@ static int build_changeset_next_level(struct 
> overlay_changeset *ovcs,
> const struct device_node *overlay_node,
> bool is_symbols_node);
>
> +/*
> + * of_resolve_phandles() finds the largest phandle in the live tree.
> + * of_overlay_apply() may add a larger phandle to the live tree.
> + * Do not allow race between two overlays being applied simultaneously:
> + *mutex_lock(&of_overlay_phandle_mutex)
> + *of_resolve_phandles()
> + *of_overlay_apply()
> + *mutex_unlock(&of_overlay_phandle_mutex)

 Why do these need to be separate functions? I think I mentioned it
 before, but essentially overlay_data_add() should be part of the
 overlay API. We may need to allow for callers to do each step, but
 generally I think the interface should just be "apply this fdt blob".
>>>
>>> Yes, that is where I want to end up.
>>
>> So, is that not doable now? To put it another way, why does
>> of_resolve_phandles need to be a separate call? Seems like an internal
>> detail of how you apply an overlay to me.
>>
>> Rob
>
> Yes, of_resolve_phandles() should become an internal call made from
> the "apply this fdt blob" function.

I mean just moving of_resolve_phandles into of_overlay_apply. Not the
unflattening too.

> The biggest obstacle is drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c
> using overlays in a convoluted manner.  The second obstacle will
> probably be the older overlay tests in drivers/of/unittest.c.  I
> need to look at how to convert them to using actual overlays.
>
> There are other fixes and improvements to the overlay code that
> need to occur, but it is like pulling on a loose thread in a
> sweater - it just goes on and on.  I'd like to get this set of
> patches in, with whatever changes are absolutely essential,
> then continue on with more patch sets.  This code will be
> much easier for me to modify in the future if this patch set
>

[Bug 103138] [regression, vega] BUG: Bad page state in process gnome-shell pfn:77cc33

2017-10-10 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=103138

--- Comment #5 from Vedran Miletić  ---
I can confirm both that the issue is still present in
2c7cb03ed2bb119f146ad0b9d9ab0a9ebb04b5a1 (current tip of amd-staging-drm-next)
and that the patch fixes it.

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


Re: [PATCH 4/4] drm/ttm: add transparent huge page support for wc or uc allocations v2

2017-10-10 Thread Alex Deucher
On Tue, Oct 10, 2017 at 4:53 AM, Christian König
 wrote:
> From: Christian König 
>
> Add a new huge page pool and try to allocate from it when it makes sense.
>
> v2: avoid compound pages for now
>
> Signed-off-by: Christian König 
> Acked-by: Alex Deucher 

Series is:
Acked-by: Alex Deucher 

> ---
>  drivers/gpu/drm/ttm/ttm_page_alloc.c | 136 
> ---
>  1 file changed, 109 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c 
> b/drivers/gpu/drm/ttm/ttm_page_alloc.c
> index 3974732..b6f16e7ff 100644
> --- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
> +++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
> @@ -95,7 +95,7 @@ struct ttm_pool_opts {
> unsignedsmall;
>  };
>
> -#define NUM_POOLS 4
> +#define NUM_POOLS 6
>
>  /**
>   * struct ttm_pool_manager - Holds memory pools for fst allocation
> @@ -122,6 +122,8 @@ struct ttm_pool_manager {
> struct ttm_page_pooluc_pool;
> struct ttm_page_poolwc_pool_dma32;
> struct ttm_page_pooluc_pool_dma32;
> +   struct ttm_page_poolwc_pool_huge;
> +   struct ttm_page_pooluc_pool_huge;
> } ;
> };
>  };
> @@ -256,8 +258,8 @@ static int set_pages_array_uc(struct page **pages, int 
> addrinarray)
>
>  /**
>   * Select the right pool or requested caching state and ttm flags. */
> -static struct ttm_page_pool *ttm_get_pool(int flags,
> -   enum ttm_caching_state cstate)
> +static struct ttm_page_pool *ttm_get_pool(int flags, bool huge,
> + enum ttm_caching_state cstate)
>  {
> int pool_index;
>
> @@ -269,9 +271,15 @@ static struct ttm_page_pool *ttm_get_pool(int flags,
> else
> pool_index = 0x1;
>
> -   if (flags & TTM_PAGE_FLAG_DMA32)
> +   if (flags & TTM_PAGE_FLAG_DMA32) {
> +   if (huge)
> +   return NULL;
> pool_index |= 0x2;
>
> +   } else if (huge) {
> +   pool_index |= 0x4;
> +   }
> +
> return &_manager->pools[pool_index];
>  }
>
> @@ -494,12 +502,14 @@ static void ttm_handle_caching_state_failure(struct 
> list_head *pages,
>   * pages returned in pages array.
>   */
>  static int ttm_alloc_new_pages(struct list_head *pages, gfp_t gfp_flags,
> -   int ttm_flags, enum ttm_caching_state cstate, unsigned count)
> +  int ttm_flags, enum ttm_caching_state cstate,
> +  unsigned count, unsigned order)
>  {
> struct page **caching_array;
> struct page *p;
> int r = 0;
> -   unsigned i, cpages;
> +   unsigned i, j, cpages;
> +   unsigned npages = 1 << order;
> unsigned max_cpages = min(count,
> (unsigned)(PAGE_SIZE/sizeof(struct page *)));
>
> @@ -512,7 +522,7 @@ static int ttm_alloc_new_pages(struct list_head *pages, 
> gfp_t gfp_flags,
> }
>
> for (i = 0, cpages = 0; i < count; ++i) {
> -   p = alloc_page(gfp_flags);
> +   p = alloc_pages(gfp_flags, order);
>
> if (!p) {
> pr_err("Unable to get page %u\n", i);
> @@ -531,14 +541,18 @@ static int ttm_alloc_new_pages(struct list_head *pages, 
> gfp_t gfp_flags,
> goto out;
> }
>
> +   list_add(&p->lru, pages);
> +
>  #ifdef CONFIG_HIGHMEM
> /* gfp flags of highmem page should never be dma32 so we
>  * we should be fine in such case
>  */
> -   if (!PageHighMem(p))
> +   if (PageHighMem(p))
> +   continue;
> +
>  #endif
> -   {
> -   caching_array[cpages++] = p;
> +   for (j = 0; j < npages; ++j) {
> +   caching_array[cpages++] = p++;
> if (cpages == max_cpages) {
>
> r = ttm_set_pages_caching(caching_array,
> @@ -552,8 +566,6 @@ static int ttm_alloc_new_pages(struct list_head *pages, 
> gfp_t gfp_flags,
> cpages = 0;
> }
> }
> -
> -   list_add(&p->lru, pages);
> }
>
> if (cpages) {
> @@ -573,9 +585,9 @@ static int ttm_alloc_new_pages(struct list_head *pages, 
> gfp_t gfp_flags,
>   * Fill the given pool if there aren't enough pages and the requested number 
> of
>   * pages is small.
>   */
> -static void ttm_page_pool_fill_locked(struct ttm_page_pool *pool,
> -   int ttm_flags, enum ttm_caching_state cstate, unsigned count,
> -   unsigned long *irq_flags)
> +static void ttm_page_pool_fill_locked(struct ttm_page_pool *pool, int 
> ttm_flags,
> + enum ttm_caching_state cstate,
> + 

[Bug 102955] HyperZ related rendering issue in ARK: Survival Evolved

2017-10-10 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=102955

Elliot Thomas  changed:

   What|Removed |Added

 Resolution|FIXED   |---
 Status|RESOLVED|REOPENED

--- Comment #6 from Elliot Thomas  ---
The DB metadata flush workaround is broken as of mesa git
69ccb9dae7616038dd54d3833da9e7c68e28067d, "radeonsi: use new VS blit shaders
(VS inputs in SGPRs)".

The R600_DEBUG=nohyperz workaround still works.

-- 
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 102800] DRI_PRIME regression- radeon: Failed to allocate virtual address for buffer

2017-10-10 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=102800

--- Comment #9 from higu...@gmx.net ---
Yes, applying the patch, i can use the radeon card without the runpm=0. Also,
turning OFF and ON the dedicated GPU now works fine

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


[PATCH 3/3] drm: Add CRTC_GET_SEQUENCE and CRTC_QUEUE_SEQUENCE ioctls [v3]

2017-10-10 Thread Keith Packard
These provide crtc-id based functions instead of pipe-number, while
also offering higher resolution time (ns) and wider frame count (64)
as required by the Vulkan API.

v2:

 * Check for DRIVER_MODESET in new crtc-based vblank ioctls

Failing to check this will oops the driver.

 * Ensure vblank interupt is running in crtc_get_sequence ioctl

The sequence and timing values are not correct while the
interrupt is off, so make sure it's running before asking for
them.

 * Short-circuit get_sequence if the counter is enabled and accurate

Steal the idea from the code in wait_vblank to avoid the
expense of drm_vblank_get/put

 * Return active state of crtc in crtc_get_sequence ioctl

Might be useful for applications that aren't in charge of
modesetting?

 * Use drm_crtc_vblank_get/put in new crtc-based vblank sequence ioctls

Daniel Vetter prefers these over the old drm_vblank_put/get
APIs.

 * Return s64 ns instead of u64 in new sequence event

Suggested-by: Daniel Vetter 
Suggested-by: Ville Syrjälä 

v3:

 * Removed FIRST_PIXEL_OUT_FLAG
 * Document that the timestamp in the query and event are
   that of the first pixel leaving the display engine for
   the display (using the same wording as the Vulkan spec).

Suggested-by: Michel Dänzer 
Acked-by: Dave Airlie 

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/drm_internal.h |   6 ++
 drivers/gpu/drm/drm_ioctl.c|   2 +
 drivers/gpu/drm/drm_vblank.c   | 168 +
 include/drm/drm_vblank.h   |   1 +
 include/uapi/drm/drm.h |  36 +
 5 files changed, 213 insertions(+)

diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index fbc3f308fa19..ba3e12eeb63d 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -71,6 +71,12 @@ int drm_legacy_modeset_ctl_ioctl(struct drm_device *dev, 
void *data,
 int drm_legacy_irq_control(struct drm_device *dev, void *data,
   struct drm_file *file_priv);
 
+int drm_crtc_get_sequence_ioctl(struct drm_device *dev, void *data,
+   struct drm_file *filp);
+
+int drm_crtc_queue_sequence_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *filp);
+
 /* drm_auth.c */
 int drm_getmagic(struct drm_device *dev, void *data,
 struct drm_file *file_priv);
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index a9ae6dd2d593..25559aa4c65b 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -663,6 +663,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
  DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl,
  DRM_UNLOCKED|DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl, 
DRM_UNLOCKED),
+   DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE, 
drm_crtc_queue_sequence_ioctl, DRM_UNLOCKED),
 };
 
 #define DRM_CORE_IOCTL_COUNT   ARRAY_SIZE( drm_ioctls )
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 7d2674e0362a..45ac2efd5078 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -831,6 +831,11 @@ static void send_vblank_event(struct drm_device *dev,
e->event.vbl.tv_sec = tv.tv_sec;
e->event.vbl.tv_usec = tv.tv_usec;
break;
+   case DRM_EVENT_CRTC_SEQUENCE:
+   if (seq)
+   e->event.seq.sequence = seq;
+   e->event.seq.time_ns = ktime_to_ns(now);
+   break;
}
trace_drm_vblank_event_delivered(e->base.file_priv, e->pipe, seq);
drm_send_event_locked(dev, &e->base);
@@ -1675,3 +1680,166 @@ bool drm_crtc_handle_vblank(struct drm_crtc *crtc)
return drm_handle_vblank(crtc->dev, drm_crtc_index(crtc));
 }
 EXPORT_SYMBOL(drm_crtc_handle_vblank);
+
+/*
+ * Get crtc VBLANK count.
+ *
+ * \param dev DRM device
+ * \param data user arguement, pointing to a drm_crtc_get_sequence structure.
+ * \param file_priv drm file private for the user's open file descriptor
+ */
+
+int drm_crtc_get_sequence_ioctl(struct drm_device *dev, void *data,
+   struct drm_file *file_priv)
+{
+   struct drm_crtc *crtc;
+   struct drm_vblank_crtc *vblank;
+   int pipe;
+   struct drm_crtc_get_sequence *get_seq = data;
+   ktime_t now;
+   bool vblank_enabled;
+   int ret;
+
+   if (!drm_core_check_feature(dev, DRIVER_MODESET))
+   return -EINVAL;
+
+   if (!dev->irq_enabled)
+   return -EINVAL;
+
+   crtc = drm_crtc_find(dev, get_seq->crtc_id);
+   if (!crtc)
+   return -ENOENT;
+
+   pipe = drm_crtc_index(crtc);
+
+   vblank = &dev->vblank[pipe];
+   vblank_enabled = dev->vblank_disable_imme

[PATCH 0/3] drm: add new vblank ioctls [v3]

2017-10-10 Thread Keith Packard
This version removes the FIRST_PIXEL_OUT flag as the existing code
already conforms to that requirement. It has also been rebased to
drm-next.

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


[PATCH 1/3] drm: Widen vblank UAPI to 64 bits. Change vblank time to ktime_t [v2]

2017-10-10 Thread Keith Packard
This modifies the datatypes used by the vblank code to provide both 64
bits of vblank count and switch to using ktime_t for timestamps to
increase resolution from microseconds to nanoseconds.

The driver interfaces have been left using 32 bits of vblank count;
all of the code necessary to widen that value for the user API was
already included to handle devices returning fewer than 32-bits.

This will provide the necessary datatypes for the Vulkan API.

v2:

 * Re-write wait_vblank ioctl to ABSOLUTE sequence

When an application uses the WAIT_VBLANK ioctl with RELATIVE
or NEXTONMISS bits set, the target vblank interval is updated
within the kernel. We need to write that target back to the
ioctl buffer and update the flags bits so that if the wait is
interrupted by a signal, when it is re-started, it will target
precisely the same vblank count as before.

 * Leave driver API with 32-bit vblank count

Suggested-by:  Michel Dänzer 
Suggested-by: Daniel Vetter 
Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/drm_vblank.c | 214 +++
 include/drm/drm_drv.h|   2 +-
 include/drm/drm_vblank.h |  16 ++--
 3 files changed, 147 insertions(+), 85 deletions(-)

diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 70f2b9593edc..8c3f2fdd821a 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -78,7 +78,7 @@
 
 static bool
 drm_get_last_vbltimestamp(struct drm_device *dev, unsigned int pipe,
- struct timeval *tvblank, bool in_vblank_irq);
+ ktime_t *tvblank, bool in_vblank_irq);
 
 static unsigned int drm_timestamp_precision = 20;  /* Default to 20 usecs. */
 
@@ -99,7 +99,7 @@ MODULE_PARM_DESC(timestamp_monotonic, "Use monotonic 
timestamps");
 
 static void store_vblank(struct drm_device *dev, unsigned int pipe,
 u32 vblank_count_inc,
-struct timeval *t_vblank, u32 last)
+ktime_t t_vblank, u32 last)
 {
struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
 
@@ -108,7 +108,7 @@ static void store_vblank(struct drm_device *dev, unsigned 
int pipe,
vblank->last = last;
 
write_seqlock(&vblank->seqlock);
-   vblank->time = *t_vblank;
+   vblank->time = t_vblank;
vblank->count += vblank_count_inc;
write_sequnlock(&vblank->seqlock);
 }
@@ -151,7 +151,7 @@ static void drm_reset_vblank_timestamp(struct drm_device 
*dev, unsigned int pipe
 {
u32 cur_vblank;
bool rc;
-   struct timeval t_vblank;
+   ktime_t t_vblank;
int count = DRM_TIMESTAMP_MAXRETRIES;
 
spin_lock(&dev->vblank_time_lock);
@@ -171,13 +171,13 @@ static void drm_reset_vblank_timestamp(struct drm_device 
*dev, unsigned int pipe
 * interrupt and assign 0 for now, to mark the vblanktimestamp as 
invalid.
 */
if (!rc)
-   t_vblank = (struct timeval) {0, 0};
+   t_vblank = 0;
 
/*
 * +1 to make sure user will never see the same
 * vblank counter value before and after a modeset
 */
-   store_vblank(dev, pipe, 1, &t_vblank, cur_vblank);
+   store_vblank(dev, pipe, 1, t_vblank, cur_vblank);
 
spin_unlock(&dev->vblank_time_lock);
 }
@@ -200,7 +200,7 @@ static void drm_update_vblank_count(struct drm_device *dev, 
unsigned int pipe,
struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
u32 cur_vblank, diff;
bool rc;
-   struct timeval t_vblank;
+   ktime_t t_vblank;
int count = DRM_TIMESTAMP_MAXRETRIES;
int framedur_ns = vblank->framedur_ns;
 
@@ -225,11 +225,9 @@ static void drm_update_vblank_count(struct drm_device 
*dev, unsigned int pipe,
/* trust the hw counter when it's around */
diff = (cur_vblank - vblank->last) & dev->max_vblank_count;
} else if (rc && framedur_ns) {
-   const struct timeval *t_old;
-   u64 diff_ns;
+   ktime_t diff_ns;
 
-   t_old = &vblank->time;
-   diff_ns = timeval_to_ns(&t_vblank) - timeval_to_ns(t_old);
+   diff_ns = t_vblank - vblank->time;
 
/*
 * Figure out how many vblanks we've missed based
@@ -263,7 +261,7 @@ static void drm_update_vblank_count(struct drm_device *dev, 
unsigned int pipe,
}
 
DRM_DEBUG_VBL("updating vblank count on crtc %u:"
- " current=%u, diff=%u, hw=%u hw_last=%u\n",
+ " current=%llu, diff=%u, hw=%u hw_last=%u\n",
  pipe, vblank->count, diff, cur_vblank, vblank->last);
 
if (diff == 0) {
@@ -278,9 +276,9 @@ static void drm_update_vblank_count(struct drm_device *dev, 
unsigned int pipe,
 * for now, to mark the vblanktimestamp as invalid.
 */
if (!rc && !in_vblank_irq)
-

[PATCH 2/3] drm: Reorganize drm_pending_event to support future event types [v2]

2017-10-10 Thread Keith Packard
Place drm_event_vblank in a new union that includes that and a bare
drm_event structure. This will allow new members of that union to be
added in the future without changing code related to the existing vbl
event type.

Assignments to the crtc_id field are now done when the event is
allocated, rather than when delievered. This way, delivery doesn't
need to have the crtc ID available.

v2:
 * Remove 'dev' argument from create_vblank_event

It wasn't being used anyways, and if we need it in the future,
we can always get it from crtc->dev.

 * Check for MODESETTING before looking for crtc in queue_vblank_event

UMS drivers will oops if we try to get a crtc, so make sure
we're modesetting before we try to find a crtc_id to fill into
the event.

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/drm_atomic.c |  7 ---
 drivers/gpu/drm/drm_plane.c  |  2 +-
 drivers/gpu/drm/drm_vblank.c | 30 ++
 drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c |  4 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |  4 ++--
 include/drm/drm_vblank.h |  8 +++-
 6 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 366c56fe5f58..08ff1b023f7b 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1815,7 +1815,7 @@ int drm_atomic_debugfs_init(struct drm_minor *minor)
  */
 
 static struct drm_pending_vblank_event *create_vblank_event(
-   struct drm_device *dev, uint64_t user_data)
+   struct drm_crtc *crtc, uint64_t user_data)
 {
struct drm_pending_vblank_event *e = NULL;
 
@@ -1825,7 +1825,8 @@ static struct drm_pending_vblank_event 
*create_vblank_event(
 
e->event.base.type = DRM_EVENT_FLIP_COMPLETE;
e->event.base.length = sizeof(e->event);
-   e->event.user_data = user_data;
+   e->event.vbl.crtc_id = crtc->base.id;
+   e->event.vbl.user_data = user_data;
 
return e;
 }
@@ -2079,7 +2080,7 @@ static int prepare_crtc_signaling(struct drm_device *dev,
if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT || fence_ptr) {
struct drm_pending_vblank_event *e;
 
-   e = create_vblank_event(dev, arg->user_data);
+   e = create_vblank_event(crtc, arg->user_data);
if (!e)
return -ENOMEM;
 
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 72cba9805edc..192071209baa 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -1039,7 +1039,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
}
e->event.base.type = DRM_EVENT_FLIP_COMPLETE;
e->event.base.length = sizeof(e->event);
-   e->event.user_data = page_flip->user_data;
+   e->event.vbl.user_data = page_flip->user_data;
ret = drm_event_reserve_init(dev, file_priv, &e->base, 
&e->event.base);
if (ret) {
kfree(e);
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 8c3f2fdd821a..7d2674e0362a 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -823,14 +823,16 @@ static void send_vblank_event(struct drm_device *dev,
 {
struct timeval tv;
 
-   tv = ktime_to_timeval(now);
-   e->event.sequence = seq;
-   e->event.tv_sec = tv.tv_sec;
-   e->event.tv_usec = tv.tv_usec;
-
-   trace_drm_vblank_event_delivered(e->base.file_priv, e->pipe,
-e->event.sequence);
-
+   switch (e->event.base.type) {
+   case DRM_EVENT_VBLANK:
+   case DRM_EVENT_FLIP_COMPLETE:
+   tv = ktime_to_timeval(now);
+   e->event.vbl.sequence = seq;
+   e->event.vbl.tv_sec = tv.tv_sec;
+   e->event.vbl.tv_usec = tv.tv_usec;
+   break;
+   }
+   trace_drm_vblank_event_delivered(e->base.file_priv, e->pipe, seq);
drm_send_event_locked(dev, &e->base);
 }
 
@@ -882,7 +884,6 @@ void drm_crtc_arm_vblank_event(struct drm_crtc *crtc,
 
e->pipe = pipe;
e->sequence = drm_vblank_count(dev, pipe);
-   e->event.crtc_id = crtc->base.id;
list_add_tail(&e->base.link, &dev->vblank_event_list);
 }
 EXPORT_SYMBOL(drm_crtc_arm_vblank_event);
@@ -913,7 +914,6 @@ void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
now = get_drm_timestamp();
}
e->pipe = pipe;
-   e->event.crtc_id = crtc->base.id;
send_vblank_event(dev, e, seq, now);
 }
 EXPORT_SYMBOL(drm_crtc_send_vblank_event);
@@ -1347,8 +1347,14 @@ static int drm_queue_vblank_event(struct drm_device 
*dev, unsigned int pipe,
 
e->pipe = pipe;
e->event.base.type = DRM_EVENT_VBLANK;
-   e->event.base.length = sizeof(e->event);
-   e->event.use

[PATCH 4/6] drm: Add drm_object lease infrastructure [v4]

2017-10-10 Thread Keith Packard
This provides new data structures to hold "lease" information about
drm mode setting objects, and provides for creating new drm_masters
which have access to a subset of the available drm resources.

An 'owner' is a drm_master which is not leasing the objects from
another drm_master, and hence 'owns' them.

A 'lessee' is a drm_master which is leasing objects from some other
drm_master. Each lessee holds the set of objects which it is leasing
from the lessor.

A 'lessor' is a drm_master which is leasing objects to another
drm_master. This is the same as the owner in the current code.

The set of objects any drm_master 'controls' is limited to the set of
objects it leases (for lessees) or all objects (for owners).

Objects not controlled by a drm_master cannot be modified through the
various state manipulating ioctls, and any state reported back to user
space will be edited to make them appear idle and/or unusable. For
instance, connectors always report 'disconnected', while encoders
report no possible crtcs or clones.

The full list of lessees leasing objects from an owner (either
directly, or indirectly through another lessee), can be searched from
an idr in the drm_master of the owner.

Changes for v2 as suggested by Daniel Vetter :

* Sub-leasing has been disabled.

* BUG_ON for lock checking replaced with lockdep_assert_held

* 'change' ioctl has been removed.

* Leased objects can always be controlled by the lessor; the
  'mask_lease' flag has been removed

* Checking for leased status has been simplified, replacing
  the drm_lease_check function with drm_lease_held.

Changes in v3, some suggested by Dave Airlie 

* Add revocation. This allows leases to be effectively revoked by
  removing all of the objects they have access to. The lease itself
  hangs around as it's hanging off a file.

* Free the leases IDR when the master is destroyed

* _drm_lease_held should look at lessees, not lessor

* Allow non-master files to check for lease status

Changes in v4, suggested by Dave Airlie 

* Formatting and whitespace changes

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/Makefile  |   2 +-
 drivers/gpu/drm/drm_auth.c|  29 +++-
 drivers/gpu/drm/drm_lease.c   | 379 ++
 include/drm/drm_auth.h|  20 +++
 include/drm/drm_lease.h   |  36 
 include/drm/drm_mode_object.h |   1 +
 6 files changed, 465 insertions(+), 2 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_lease.c
 create mode 100644 include/drm/drm_lease.h

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index a3fdc5a68dff..81ff79336623 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -17,7 +17,7 @@ drm-y   :=drm_auth.o drm_bufs.o drm_cache.o \
drm_encoder.o drm_mode_object.o drm_property.o \
drm_plane.o drm_color_mgmt.o drm_print.o \
drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \
-   drm_syncobj.o
+   drm_syncobj.o drm_lease.o
 
 drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
 drm-$(CONFIG_DRM_VM) += drm_vm.o
diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 7ff697389d74..541177c71d51 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -31,6 +31,7 @@
 #include 
 #include "drm_internal.h"
 #include "drm_legacy.h"
+#include 
 
 /**
  * DOC: master and authentication
@@ -93,7 +94,7 @@ int drm_authmagic(struct drm_device *dev, void *data,
return file ? 0 : -EINVAL;
 }
 
-static struct drm_master *drm_master_create(struct drm_device *dev)
+struct drm_master *drm_master_create(struct drm_device *dev)
 {
struct drm_master *master;
 
@@ -107,6 +108,14 @@ static struct drm_master *drm_master_create(struct 
drm_device *dev)
idr_init(&master->magic_map);
master->dev = dev;
 
+   /* initialize the tree of output resource lessees */
+   master->lessor = NULL;
+   master->lessee_id = 0;
+   INIT_LIST_HEAD(&master->lessees);
+   INIT_LIST_HEAD(&master->lessee_list);
+   idr_init(&master->leases);
+   idr_init(&master->lessee_idr);
+
return master;
 }
 
@@ -189,6 +198,12 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data,
goto out_unlock;
}
 
+   if (file_priv->master->lessor != NULL) {
+   DRM_DEBUG_LEASE("Attempt to set lessee %d as master\n", 
file_priv->master->lessee_id);
+   ret = -EINVAL;
+   goto out_unlock;
+   }
+
ret = drm_set_master(dev, file_priv, false);
 out_unlock:
mutex_unlock(&dev->master_mutex);
@@ -270,6 +285,13 @@ void drm_master_release(struct drm_file *file_priv)
if (dev->master == file_priv->master)
drm_drop_master(dev, file_priv);
 out:
+   if (file_priv->is_master) {
+   /* Revoke any leases held by this or lessees, but only if
+* this is the "real" master
+*/
+  

[PATCH 6/6] drm: Add four ioctls for managing drm mode object leases [v5]

2017-10-10 Thread Keith Packard
drm_mode_create_lease

Creates a lease for a list of drm mode objects, returning an
fd for the new drm_master and a 64-bit identifier for the lessee

drm_mode_list_lesees

List the identifiers of the lessees for a master file

drm_mode_get_lease

List the leased objects for a master file

drm_mode_revoke_lease

Erase the set of objects managed by a lease.

This should suffice to at least create and query leases.

Changes for v2 as suggested by Daniel Vetter :

 * query ioctls only query the master associated with
   the provided file.

 * 'mask_lease' value has been removed

 * change ioctl has been removed.

Changes for v3 suggested in part by Dave Airlie 

 * Add revoke ioctl.

Changes for v4 suggested by Dave Airlie 

 * Expand on the comment about the magic use of &drm_lease_idr_object
 * Pad lease ioctl structures to align on 64-bit boundaries

Changes for v5 suggested by Dave Airlie 

 * Check for non-negative object_id in create_lease to avoid debug
   output from the kernel.

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/drm_ioctl.c  |   4 +
 drivers/gpu/drm/drm_lease.c  | 282 +++
 drivers/gpu/drm/drm_vblank.c |   4 +-
 include/drm/drm_lease.h  |  12 ++
 include/uapi/drm/drm.h   |   5 +
 include/uapi/drm/drm_mode.h  |  66 ++
 6 files changed, 371 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 78a0d6996e12..c37dc4478d83 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -665,6 +665,10 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
  DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl, 
DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE, 
drm_crtc_queue_sequence_ioctl, DRM_UNLOCKED),
+   DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl, 
DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+   DRM_IOCTL_DEF(DRM_IOCTL_MODE_LIST_LESSEES, drm_mode_list_lessees_ioctl, 
DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+   DRM_IOCTL_DEF(DRM_IOCTL_MODE_GET_LEASE, drm_mode_get_lease_ioctl, 
DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+   DRM_IOCTL_DEF(DRM_IOCTL_MODE_REVOKE_LEASE, drm_mode_revoke_lease_ioctl, 
DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 };
 
 #define DRM_CORE_IOCTL_COUNT   ARRAY_SIZE( drm_ioctls )
diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
index 2ac404264d75..69b54cb557ce 100644
--- a/drivers/gpu/drm/drm_lease.c
+++ b/drivers/gpu/drm/drm_lease.c
@@ -23,6 +23,8 @@
 #define drm_for_each_lessee(lessee, lessor) \
list_for_each_entry((lessee), &(lessor)->lessees, lessee_list)
 
+static uint64_t drm_lease_idr_object;
+
 /**
  * drm_lease_owner - return ancestor owner drm_master
  * @master: drm_master somewhere within tree of lessees and lessors
@@ -377,3 +379,283 @@ void _drm_lease_revoke(struct drm_master *top)
}
}
 }
+
+/**
+ * drm_mode_create_lease_ioctl - create a new lease
+ * @dev: the drm device
+ * @data: pointer to struct drm_mode_create_lease
+ * @file_priv: the file being manipulated
+ *
+ * The master associated with the specified file will have a lease
+ * created containing the objects specified in the ioctl structure.
+ * A file descriptor will be allocated for that and returned to the
+ * application.
+ */
+int drm_mode_create_lease_ioctl(struct drm_device *dev,
+   void *data, struct drm_file *lessor_priv)
+{
+   struct drm_mode_create_lease *cl = data;
+   size_t object_count;
+   size_t o;
+   int ret = 0;
+   struct idr leases;
+   struct drm_master *lessor = lessor_priv->master;
+   struct drm_master *lessee = NULL;
+   struct file *lessee_file = NULL;
+   struct file *lessor_file = lessor_priv->filp;
+   struct drm_file *lessee_priv;
+   int fd = -1;
+
+   /* Do not allow sub-leases */
+   if (lessor->lessor)
+   return -EINVAL;
+
+   object_count = cl->object_count;
+   idr_init(&leases);
+
+   /* Allocate a file descriptor for the lease */
+   fd = get_unused_fd_flags(cl->flags & (O_CLOEXEC | O_NONBLOCK));
+
+   DRM_DEBUG_LEASE("Creating new lease\n");
+
+   /* Lookup the mode objects and add their IDs to the lease request */
+   for (o = 0; o < object_count; o++) {
+   __u32 object_id;
+
+   if (copy_from_user(&object_id,
+  u64_to_user_ptr(cl->object_ids) + o * sizeof 
(__u32),
+  sizeof (__u32))) {
+   ret = -EFAULT;
+   goto out_leases;
+   }
+   DRM_DEBUG_LEASE("Adding object %d to lease\n", object_id);
+
+   if ((int) object_id < 0) {
+   ret = -EINVAL;
+   goto out_leases;
+ 

[PATCH 3/6] drm: Add new LEASE debug level

2017-10-10 Thread Keith Packard
Separate out lease debugging from the core.

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/drm_drv.c | 3 ++-
 include/drm/drmP.h| 4 
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index c0292e5d7281..a934fd5e7e55 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -57,7 +57,8 @@ MODULE_PARM_DESC(debug, "Enable debug output, where each bit 
enables a debug cat
 "\t\tBit 2 (0x04) will enable KMS messages (modesetting code)\n"
 "\t\tBit 3 (0x08) will enable PRIME messages (prime code)\n"
 "\t\tBit 4 (0x10) will enable ATOMIC messages (atomic code)\n"
-"\t\tBit 5 (0x20) will enable VBL messages (vblank code)");
+"\t\tBit 5 (0x20) will enable VBL messages (vblank code)\n"
+"\t\tBit 7 (0x80) will enable LEASE messages (leasing code)");
 module_param_named(debug, drm_debug, int, 0600);
 
 static DEFINE_SPINLOCK(drm_minor_lock);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 7277783a4ff0..59be1232d005 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -136,6 +136,7 @@ struct pci_controller;
 #define DRM_UT_ATOMIC  0x10
 #define DRM_UT_VBL 0x20
 #define DRM_UT_STATE   0x40
+#define DRM_UT_LEASE   0x80
 
 /***/
 /** \name DRM template customization defaults */
@@ -250,6 +251,9 @@ struct pci_controller;
 #define DRM_DEBUG_VBL(fmt, ...)\
drm_printk(KERN_DEBUG, DRM_UT_VBL, fmt, ##__VA_ARGS__)
 
+#define DRM_DEBUG_LEASE(fmt, ...)  \
+   drm_printk(KERN_DEBUG, DRM_UT_LEASE, fmt, ##__VA_ARGS__)
+
 #define _DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, level, fmt, args...)\
 ({ \
static DEFINE_RATELIMIT_STATE(_rs,  \
-- 
2.13.3

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


[PATCH 2/6] drm: Allow render nodes to query display objects

2017-10-10 Thread Keith Packard
This allows an application to discover what display resources are
available before requesting a lease from the X server.

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/drm_ioctl.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 25559aa4c65b..78a0d6996e12 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -613,27 +613,27 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
DRM_IOCTL_DEF(DRM_IOCTL_GEM_FLINK, drm_gem_flink_ioctl, 
DRM_AUTH|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_GEM_OPEN, drm_gem_open_ioctl, 
DRM_AUTH|DRM_UNLOCKED),
 
-   DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETRESOURCES, drm_mode_getresources, 
DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+   DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETRESOURCES, drm_mode_getresources, 
DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 
DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, 
drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, 
drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
 
-   DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, 
DRM_CONTROL_ALLOW|DRM_UNLOCKED),
-   DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, 
DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+   DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, 
DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+   DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, 
DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETCRTC, drm_mode_setcrtc, 
DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
-   DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANE, drm_mode_getplane, 
DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+   DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANE, drm_mode_getplane, 
DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETPLANE, drm_mode_setplane, 
DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_CURSOR, drm_mode_cursor_ioctl, 
DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETGAMMA, drm_mode_gamma_get_ioctl, 
DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETGAMMA, drm_mode_gamma_set_ioctl, 
DRM_MASTER|DRM_UNLOCKED),
-   DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETENCODER, drm_mode_getencoder, 
DRM_CONTROL_ALLOW|DRM_UNLOCKED),
-   DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCONNECTOR, drm_mode_getconnector, 
DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+   DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETENCODER, drm_mode_getencoder, 
DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+   DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCONNECTOR, drm_mode_getconnector, 
DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_ATTACHMODE, drm_noop, 
DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_DETACHMODE, drm_noop, 
DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
-   DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPROPERTY, drm_mode_getproperty_ioctl, 
DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+   DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPROPERTY, drm_mode_getproperty_ioctl, 
DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETPROPERTY, 
drm_mode_connector_property_set_ioctl, 
DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
-   DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPROPBLOB, drm_mode_getblob_ioctl, 
DRM_CONTROL_ALLOW|DRM_UNLOCKED),
-   DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETFB, drm_mode_getfb, 
DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+   DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPROPBLOB, drm_mode_getblob_ioctl, 
DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+   DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETFB, drm_mode_getfb, 
DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_ADDFB, drm_mode_addfb, 
DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_ADDFB2, drm_mode_addfb2, 
DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_RMFB, drm_mode_rmfb, 
DRM_CONTROL_ALLOW|DRM_UNLOCKED),
@@ -642,7 +642,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_DUMB, drm_mode_create_dumb_ioctl, 
DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_MAP_DUMB, drm_mode_mmap_dumb_ioctl, 
DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_DESTROY_DUMB, drm_mode_destroy_dumb_ioctl, 
DRM_CONTROL_ALLOW|DRM_UNLOCKED),
-   DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_GETPROPERTIES, 
drm_mode_obj_get_properties_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+   DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_GETPROPERTIES, 
drm_mode_obj_get_properties_ioctl, 
DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_SETPROPERTY, 
drm_mode_obj_set_property_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_CURSOR2, drm_mode_cur

[PATCH 5/6] drm: Check mode object lease status in all master ioctl paths [v2]

2017-10-10 Thread Keith Packard
Attempts to modify un-leased objects are rejected with an error.
Information returned about unleased objects is modified to make them
appear unusable and/or disconnected.

Changes for v2 as suggested by Daniel Vetter :

With the change in the __drm_mode_object_find API to pass the
file_priv along, we can now centralize most of the lease-based access
checks in that function.

A few places skip that API and require in-line checks.

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/drm_auth.c|  2 +-
 drivers/gpu/drm/drm_connector.c   |  5 +++--
 drivers/gpu/drm/drm_encoder.c |  8 +---
 drivers/gpu/drm/drm_mode_config.c | 32 +++-
 drivers/gpu/drm/drm_mode_object.c | 22 ++
 drivers/gpu/drm/drm_plane.c   |  8 +---
 drivers/gpu/drm/drm_vblank.c  | 18 --
 7 files changed, 71 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 541177c71d51..bab26b477738 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -310,7 +310,7 @@ void drm_master_release(struct drm_file *file_priv)
  */
 bool drm_is_current_master(struct drm_file *fpriv)
 {
-   return fpriv->is_master && fpriv->master == fpriv->minor->dev->master;
+   return fpriv->is_master && drm_lease_owner(fpriv->master) == 
fpriv->minor->dev->master;
 }
 EXPORT_SYMBOL(drm_is_current_master);
 
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 704fc8934616..0712c14cb57c 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1315,7 +1315,8 @@ int drm_mode_getconnector(struct drm_device *dev, void 
*data,
return -ENOENT;
 
for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++)
-   if (connector->encoder_ids[i] != 0)
+   if (connector->encoder_ids[i] != 0 &&
+   drm_lease_held(file_priv, connector->encoder_ids[i]))
encoders_count++;
 
if ((out_resp->count_encoders >= encoders_count) && encoders_count) {
@@ -1382,7 +1383,7 @@ int drm_mode_getconnector(struct drm_device *dev, void 
*data,
 
drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
encoder = drm_connector_get_encoder(connector);
-   if (encoder)
+   if (encoder && drm_lease_held(file_priv, encoder->base.id))
out_resp->encoder_id = encoder->base.id;
else
out_resp->encoder_id = 0;
diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
index 43f644844b83..6ad6416f2ede 100644
--- a/drivers/gpu/drm/drm_encoder.c
+++ b/drivers/gpu/drm/drm_encoder.c
@@ -226,7 +226,7 @@ int drm_mode_getencoder(struct drm_device *dev, void *data,
 
drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
crtc = drm_encoder_get_crtc(encoder);
-   if (crtc)
+   if (crtc && drm_lease_held(file_priv, crtc->base.id))
enc_resp->crtc_id = crtc->base.id;
else
enc_resp->crtc_id = 0;
@@ -234,8 +234,10 @@ int drm_mode_getencoder(struct drm_device *dev, void *data,
 
enc_resp->encoder_type = encoder->encoder_type;
enc_resp->encoder_id = encoder->base.id;
-   enc_resp->possible_crtcs = encoder->possible_crtcs;
-   enc_resp->possible_clones = encoder->possible_clones;
+   enc_resp->possible_crtcs = drm_lease_filter_crtcs(file_priv,
+ 
encoder->possible_crtcs);
+   enc_resp->possible_clones = drm_lease_filter_encoders(file_priv,
+ 
encoder->possible_clones);
 
return 0;
 }
diff --git a/drivers/gpu/drm/drm_mode_config.c 
b/drivers/gpu/drm/drm_mode_config.c
index 74f6ff5df656..fad6646b38d3 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -122,20 +122,24 @@ int drm_mode_getresources(struct drm_device *dev, void 
*data,
count = 0;
crtc_id = u64_to_user_ptr(card_res->crtc_id_ptr);
drm_for_each_crtc(crtc, dev) {
-   if (count < card_res->count_crtcs &&
-   put_user(crtc->base.id, crtc_id + count))
-   return -EFAULT;
-   count++;
+   if (drm_lease_held(file_priv, crtc->base.id)) {
+   if (count < card_res->count_crtcs &&
+   put_user(crtc->base.id, crtc_id + count))
+   return -EFAULT;
+   count++;
+   }
}
card_res->count_crtcs = count;
 
count = 0;
encoder_id = u64_to_user_ptr(card_res->encoder_id_ptr);
drm_for_each_encoder(encoder, dev) {
-   if (count < card_res->count_encoders &&
-   put_user(encoder->base.id, encoder_id + count))
-   return -EFAULT;
-   count++;
+   if (dr

[PATCH 0/6] drm: Add drm mode object leases

2017-10-10 Thread Keith Packard
New since last time:

 * Fix vboxvideo driver in staging
 * Fixed some formatting and whitespace errors (Dave Airlie)
 * Explain the odd use of the idr interface for leases (Dave Airlie)
 * Disallow negative object_id in lease creation (Dave Airlie)

It's also been rebased on top of the drm-sequence-64 sequence just
sent out so that merging one and then the other will be simplified.

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


[PATCH 1/6] drm: Pass struct drm_file * to __drm_mode_object_find [v2]

2017-10-10 Thread Keith Packard
This will allow __drm_mode_object_file to be extended to perform
access control checks based on the file in use.

v2: Also fix up vboxvideo driver in staging

Suggested-by: Daniel Vetter 
Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c   | 16 
 drivers/gpu/drm/amd/amdgpu/dce_virtual.c |  4 ++--
 drivers/gpu/drm/ast/ast_mode.c   |  2 +-
 drivers/gpu/drm/bochs/bochs_kms.c|  2 +-
 drivers/gpu/drm/cirrus/cirrus_mode.c |  2 +-
 drivers/gpu/drm/drm_atomic.c |  8 
 drivers/gpu/drm/drm_atomic_helper.c  |  2 +-
 drivers/gpu/drm/drm_color_mgmt.c |  4 ++--
 drivers/gpu/drm/drm_connector.c  |  2 +-
 drivers/gpu/drm/drm_crtc.c   |  8 
 drivers/gpu/drm/drm_crtc_internal.h  |  1 +
 drivers/gpu/drm/drm_encoder.c|  2 +-
 drivers/gpu/drm/drm_framebuffer.c|  9 +
 drivers/gpu/drm/drm_mode_object.c| 10 ++
 drivers/gpu/drm/drm_plane.c  | 14 +++---
 drivers/gpu/drm/drm_probe_helper.c   |  2 +-
 drivers/gpu/drm/drm_property.c   |  6 +++---
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c |  2 +-
 drivers/gpu/drm/i915/intel_display.c |  2 +-
 drivers/gpu/drm/i915/intel_overlay.c |  2 +-
 drivers/gpu/drm/i915/intel_sprite.c  |  2 +-
 drivers/gpu/drm/mgag200/mgag200_mode.c   |  2 +-
 drivers/gpu/drm/nouveau/nouveau_connector.c  |  4 ++--
 drivers/gpu/drm/radeon/r100.c|  2 +-
 drivers/gpu/drm/radeon/r600_cs.c |  2 +-
 drivers/gpu/drm/radeon/radeon_connectors.c   | 16 
 drivers/gpu/drm/udl/udl_connector.c  |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c|  4 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  |  2 +-
 drivers/staging/vboxvideo/vbox_mode.c|  2 +-
 include/drm/drm_connector.h  |  3 ++-
 include/drm/drm_crtc.h   |  5 +++--
 include/drm/drm_encoder.h|  3 ++-
 include/drm/drm_framebuffer.h|  1 +
 include/drm/drm_mode_object.h|  2 ++
 include/drm/drm_plane.h  |  3 ++-
 include/drm/drm_property.h   |  3 ++-
 37 files changed, 85 insertions(+), 73 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
index f51b41f094ef..df9cbc78e168 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
@@ -231,7 +231,7 @@ amdgpu_connector_update_scratch_regs(struct drm_connector 
*connector,
if (connector->encoder_ids[i] == 0)
break;
 
-   encoder = drm_encoder_find(connector->dev,
+   encoder = drm_encoder_find(connector->dev, NULL,
connector->encoder_ids[i]);
if (!encoder)
continue;
@@ -256,7 +256,7 @@ amdgpu_connector_find_encoder(struct drm_connector 
*connector,
for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
if (connector->encoder_ids[i] == 0)
break;
-   encoder = drm_encoder_find(connector->dev,
+   encoder = drm_encoder_find(connector->dev, NULL,
connector->encoder_ids[i]);
if (!encoder)
continue;
@@ -372,7 +372,7 @@ amdgpu_connector_best_single_encoder(struct drm_connector 
*connector)
 
/* pick the encoder ids */
if (enc_id)
-   return drm_encoder_find(connector->dev, enc_id);
+   return drm_encoder_find(connector->dev, NULL, enc_id);
return NULL;
 }
 
@@ -1077,7 +1077,7 @@ amdgpu_connector_dvi_detect(struct drm_connector 
*connector, bool force)
if (connector->encoder_ids[i] == 0)
break;
 
-   encoder = drm_encoder_find(connector->dev, 
connector->encoder_ids[i]);
+   encoder = drm_encoder_find(connector->dev, NULL, 
connector->encoder_ids[i]);
if (!encoder)
continue;
 
@@ -1134,7 +1134,7 @@ amdgpu_connector_dvi_encoder(struct drm_connector 
*connector)
if (connector->encoder_ids[i] == 0)
break;
 
-   encoder = drm_encoder_find(connector->dev, 
connector->encoder_ids[i]);
+   encoder = drm_encoder_find(connector->dev, NULL, 
connector->encoder_ids[i]);
if (!encoder)
continue;
 
@@ -1153,7 +1153,7 @@ amdgpu_connector_dvi_encoder(struct drm_connector 
*connector)
/* then check use digitial

[PATCH i-g-t 0/2] Add new vblank and lease tests [v2]

2017-10-10 Thread Keith Packard
Changes since last version:

 * Add local definitions of new ioctls to avoid dependency on proposed
   libdrm bits.

 * Remove FIRST_PIXEL_OUT as that has been removed from the proposed
   kernel interface

 * Add tests for get_lease and list_lessees

 * Fix commit message on the lease test patch

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


[PATCH i-g-t 1/2] tests/kms_sequence: Add tests for new CRTC get/queue sequence ioctls [v2]

2017-10-10 Thread Keith Packard
These ioctls replace drmWaitVBlank and add ns time resolution and
64-bit sequence numbers to comply with the Vulkan API specifications.

The tests were derived from the existing kms_vblank tests with the
'wait' variant elided as the new API doesn't provide a mechanism for
blocking in the kernel.

v2: from Dave Airlie 

 * Add local definitions of new ioctls to avoid requiring latest
   libdrm.

 * Remove FIRST_PIXEL_OUT as that has been removed from the proposed
   kernel patches.

Signed-off-by: Keith Packard 
---
 lib/igt_kms.c  |   2 +-
 lib/igt_kms.h  |   1 +
 tests/Makefile.sources |   1 +
 tests/kms_sequence.c   | 317 +
 tests/meson.build  |   1 +
 5 files changed, 321 insertions(+), 1 deletion(-)
 create mode 100644 tests/kms_sequence.c

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 379bd0c3..44285baa 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1983,7 +1983,7 @@ report_dup:
}
 }
 
-static igt_pipe_t *igt_output_get_driving_pipe(igt_output_t *output)
+igt_pipe_t *igt_output_get_driving_pipe(igt_output_t *output)
 {
igt_display_t *display = output->display;
enum pipe pipe;
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 8dc118c9..92b66676 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -397,6 +397,7 @@ igt_plane_t *igt_output_get_plane(igt_output_t *output, int 
plane_idx);
 igt_plane_t *igt_output_get_plane_type(igt_output_t *output, int plane_type);
 igt_output_t *igt_output_from_connector(igt_display_t *display,
 drmModeConnector *connector);
+igt_pipe_t *igt_output_get_driving_pipe(igt_output_t *output);
 igt_plane_t *igt_pipe_get_plane_type(igt_pipe_t *pipe, int plane_type);
 bool igt_pipe_get_property(igt_pipe_t *pipe, const char *name,
   uint32_t *prop_id, uint64_t *value,
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index bb6652e2..9dc1d250 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -215,6 +215,7 @@ TESTS_progs = \
kms_tv_load_detect \
kms_universal_plane \
kms_vblank \
+   kms_sequence \
meta_test \
perf \
pm_backlight \
diff --git a/tests/kms_sequence.c b/tests/kms_sequence.c
new file mode 100644
index ..783dda25
--- /dev/null
+++ b/tests/kms_sequence.c
@@ -0,0 +1,317 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ * Copyright © 2017 Keith Packard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+/** @file kms_sequence.c
+ *
+ * This is a test of drmCrtcGetSequence and drmCrtcQueueSequence
+ */
+
+#include "igt.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include "intel_bufmgr.h"
+
+IGT_TEST_DESCRIPTION("Test CrtcGetSequence and CrtcQueueSequence.");
+
+typedef struct {
+   igt_display_t display;
+   struct igt_fb primary_fb;
+   igt_output_t *output;
+   uint32_t crtc_id;
+   enum pipe pipe;
+   unsigned int flags;
+#define IDLE 1
+#define BUSY 2
+#define FORKED 4
+} data_t;
+
+struct local_drm_crtc_get_sequence {
+   __u32 crtc_id;
+   __u32 active;
+   __u64 sequence;
+   __u64 sequence_ns;
+};
+
+struct local_drm_crtc_queue_sequence {
+   __u32 crtc_id;
+   __u32 flags;
+   __u64 sequence;
+   __u64 user_data;
+};
+
+#define LOCAL_DRM_IOCTL_CRTC_GET_SEQUENCE DRM_IOWR(0x3b, struct 
local_drm_crtc_get_sequence)
+#define LOCAL_DRM_IOCTL_CRTC_QUEUE_SEQUENCE   DRM_IOWR(0x3c, struct 
local_drm_crtc_queue_sequence)
+
+#define LOCAL_DRM_CRTC_SEQUENCE_RELATIVE  0x0001  /* 
sequence is relative to current */
+#define LOCAL_DRM_CRTC_SEQUENCE_NEXT_ON_MISS  0x0002  /* Use 
next sequence if we've missed */
+
+struct local_drm_event_crtc_sequence {
+struct drm_eventbase;
+__u64 

[PATCH i-g-t 2/2] tests/kms_lease: add tests for lease ioctls [v2]

2017-10-10 Thread Keith Packard
Validate that the leasing API creates leases that allow access to a
subset of the available resources and that lease revocation works.

v2: from Dave Airlie 

 * Update ioctl numbers to latest proposed values.
 * Fix commit message
 * Add tests for get_lease and list_lessees

Signed-off-by: Keith Packard 
---
 tests/Makefile.sources |   1 +
 tests/kms_lease.c  | 597 +
 tests/meson.build  |   1 +
 3 files changed, 599 insertions(+)
 create mode 100644 tests/kms_lease.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 9dc1d250..1ca6e104 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -193,6 +193,7 @@ TESTS_progs = \
kms_frontbuffer_tracking \
kms_hdmi_inject \
kms_invalid_dotclock \
+   kms_lease \
kms_legacy_colorkey \
kms_mmap_write_crc \
kms_mmio_vs_cs_flip \
diff --git a/tests/kms_lease.c b/tests/kms_lease.c
new file mode 100644
index ..49226163
--- /dev/null
+++ b/tests/kms_lease.c
@@ -0,0 +1,597 @@
+/*
+ * Copyright © 2017 Keith Packard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+/** @file kms_lease.c
+ *
+ * This is a test of DRM leases
+ */
+
+
+#include "igt.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+IGT_TEST_DESCRIPTION("Test of CreateLease.");
+
+struct local_drm_mode_create_lease {
+/** Pointer to array of object ids (__u32) */
+__u64 object_ids;
+/** Number of object ids */
+__u32 object_count;
+/** flags for new FD (O_CLOEXEC, etc) */
+__u32 flags;
+
+/** Return: unique identifier for lessee. */
+__u32 lessee_id;
+/** Return: file descriptor to new drm_master file */
+__u32 fd;
+};
+
+struct local_drm_mode_list_lessees {
+/** Number of lessees.
+ * On input, provides length of the array.
+ * On output, provides total number. No
+ * more than the input number will be written
+ * back, so two calls can be used to get
+ * the size and then the data.
+ */
+__u32 count_lessees;
+__u32 pad;
+
+/** Pointer to lessees.
+ * pointer to __u64 array of lessee ids
+ */
+__u64 lessees_ptr;
+};
+
+struct local_drm_mode_get_lease {
+/** Number of leased objects.
+ * On input, provides length of the array.
+ * On output, provides total number. No
+ * more than the input number will be written
+ * back, so two calls can be used to get
+ * the size and then the data.
+ */
+__u32 count_objects;
+__u32 pad;
+
+/** Pointer to objects.
+ * pointer to __u32 array of object ids
+ */
+__u64 objects_ptr;
+};
+
+/**
+ * Revoke lease
+ */
+struct local_drm_mode_revoke_lease {
+/** Unique ID of lessee
+ */
+__u32 lessee_id;
+};
+
+
+#define LOCAL_DRM_IOCTL_MODE_CREATE_LEASE DRM_IOWR(0xC6, struct 
local_drm_mode_create_lease)
+#define LOCAL_DRM_IOCTL_MODE_LIST_LESSEES DRM_IOWR(0xC7, struct 
local_drm_mode_list_lessees)
+#define LOCAL_DRM_IOCTL_MODE_GET_LEASEDRM_IOWR(0xC8, struct 
local_drm_mode_get_lease)
+#define LOCAL_DRM_IOCTL_MODE_REVOKE_LEASE DRM_IOWR(0xC9, struct 
local_drm_mode_revoke_lease)
+
+typedef struct {
+   int fd;
+   uint32_t lessee_id;
+   igt_display_t display;
+   struct igt_fb primary_fb;
+   igt_output_t *output;
+   drmModeModeInfo *mode;
+} lease_t;
+
+typedef struct {
+   lease_t master;
+   enum pipe pipe;
+   uint32_t crtc_id;
+   uint32_t connector_id;
+} data_t;
+
+static uint32_t pipe_to_crtc_id(igt_display_t *display, enum pipe pipe)
+{
+   return display->pipes[pipe].crtc_id;
+}
+
+static enum pipe crtc_id

Re: [PATCH v2 2/9] drm/exynos: ipp: Add IPP v2 framework

2017-10-10 Thread Hoegeun Kwon

Hi Marek,

Currently, IPP v2 and gsc do not seem to have any code to
check for hardware limits. How can I check the hardware limits
of device(gsc, rotator, ...) at IPP v2?

Best regards,
Hoegeun

On 09/29/2017 04:32 PM, Marek Szyprowski wrote:

This patch adds Exynos IPP v2 subsystem and userspace API.

New userspace API is focused ONLY on memory-to-memory image processing.
The two remainging IPP operation modes (framebuffer writeback and
local-path output with image processing) can be implemented using
standard DRM features: writeback connectors and additional DRM planes with
scaling features.

V2 IPP userspace API is not compatible with old IPP ioctls. This is a
significant change, but the old IPP subsystem in mainline Linux kernel was
partially disfunctional anyway and not used in any open-source project.

V2 IPP userspace API is based on stateless approach, which much better fits
to memory-to-memory image processing model. It also provides support for
all image formats, which are both already defined in DRM API and supported
by the existing IPP hardware modules.

The API consists of the following ioctls:
- DRM_IOCTL_EXYNOS_IPP_GET_RESOURCES: to enumerate all available image
   processing modules,
- DRM_IOCTL_EXYNOS_IPP_GET_CAPS: to query capabilities and supported image
   formats of given IPP module,
- DRM_IOCTL_EXYNOS_IPP_GET_LIMITS: to query hardware limitiations for
   selected image format of given IPP module,
- DRM_IOCTL_EXYNOS_IPP_COMMIT: to perform operation described by the
   provided structures (source and destination buffers, operation rectangle,
   transformation, etc).

The proposed userspace API is extensible. In the future more advanced image
processing operations can be defined to support for example blending.

Userspace API is fully functional also on DRM render nodes, so it is not
limited to the root/privileged client.

Internal driver API also has been completely rewritten. New IPP core
performs all possible input validation, checks and object life-time
control. The drivers can focus only on writing configuration to hardware
registers. Stateless nature of DRM_IOCTL_EXYNOS_IPP_COMMIT ioctl simplifies
the driver API. Minimal driver needs to provide a single callback for
starting processing and an array with supported image formats.

Signed-off-by: Marek Szyprowski 
Tested-by: Hoegeun Kwon 
---
  drivers/gpu/drm/exynos/Kconfig  |   3 +
  drivers/gpu/drm/exynos/Makefile |   1 +
  drivers/gpu/drm/exynos/exynos_drm_drv.c |   9 +
  drivers/gpu/drm/exynos/exynos_drm_ipp.c | 894 
  drivers/gpu/drm/exynos/exynos_drm_ipp.h | 191 +++
  include/uapi/drm/exynos_drm.h   | 238 +
  6 files changed, 1336 insertions(+)
  create mode 100644 drivers/gpu/drm/exynos/exynos_drm_ipp.c
  create mode 100644 drivers/gpu/drm/exynos/exynos_drm_ipp.h

diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig
index 88cff0e039b6..2e097a81df12 100644
--- a/drivers/gpu/drm/exynos/Kconfig
+++ b/drivers/gpu/drm/exynos/Kconfig
@@ -94,6 +94,9 @@ config DRM_EXYNOS_G2D
help
  Choose this option if you want to use Exynos G2D for DRM.
  
+config DRM_EXYNOS_IPP

+   bool
+
  config DRM_EXYNOS_FIMC
bool "FIMC"
depends on BROKEN && MFD_SYSCON
diff --git a/drivers/gpu/drm/exynos/Makefile b/drivers/gpu/drm/exynos/Makefile
index 09bb002c9555..f663490e949d 100644
--- a/drivers/gpu/drm/exynos/Makefile
+++ b/drivers/gpu/drm/exynos/Makefile
@@ -17,6 +17,7 @@ exynosdrm-$(CONFIG_DRM_EXYNOS_MIXER)  += exynos_mixer.o
  exynosdrm-$(CONFIG_DRM_EXYNOS_HDMI)   += exynos_hdmi.o
  exynosdrm-$(CONFIG_DRM_EXYNOS_VIDI)   += exynos_drm_vidi.o
  exynosdrm-$(CONFIG_DRM_EXYNOS_G2D)+= exynos_drm_g2d.o
+exynosdrm-$(CONFIG_DRM_EXYNOS_IPP) += exynos_drm_ipp.o
  exynosdrm-$(CONFIG_DRM_EXYNOS_FIMC)   += exynos_drm_fimc.o
  exynosdrm-$(CONFIG_DRM_EXYNOS_ROTATOR)+= exynos_drm_rotator.o
  exynosdrm-$(CONFIG_DRM_EXYNOS_GSC)+= exynos_drm_gsc.o
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c 
b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 2fc5d3c01390..de4cce258a5b 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -26,6 +26,7 @@
  #include "exynos_drm_fb.h"
  #include "exynos_drm_gem.h"
  #include "exynos_drm_plane.h"
+#include "exynos_drm_ipp.h"
  #include "exynos_drm_vidi.h"
  #include "exynos_drm_g2d.h"
  #include "exynos_drm_iommu.h"
@@ -114,6 +115,14 @@ static void exynos_drm_lastclose(struct drm_device *dev)
DRM_AUTH | DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(EXYNOS_G2D_EXEC, exynos_g2d_exec_ioctl,
DRM_AUTH | DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF_DRV(EXYNOS_IPP_GET_RESOURCES, 
exynos_drm_ipp_get_res_ioctl,
+   DRM_AUTH | DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF_DRV(EXYNOS_IPP_GET_CAPS, exynos_drm_ipp_get_caps_ioctl,
+   DRM_AUTH | DRM_RENDER_ALLOW),
+   DRM_IOCT

RE: [PATCH] gpu: ipu-v3: Allow channel burst locking on i.MX6 only

2017-10-10 Thread Patrick Brünn
>From: Philipp Zabel [mailto:p.za...@pengutronix.de]
>Sent: Dienstag, 10. Oktober 2017 15:28
>
>The IDMAC_LOCK_EN registers on i.MX51 have a different layout, and on
>i.MX53 enabling the lock feature causes bursts to get lost. Restrict
>enabling the burst lock feature to i.MX6.
>
>Reported-by: Patrick Brünn 
>Fixes: 790cb4c7c954 ("drm/imx: lock scanout transfers for consecutive bursts")
>Signed-off-by: Philipp Zabel 
>---
> drivers/gpu/ipu-v3/ipu-common.c | 8 
> 1 file changed, 8 insertions(+)
>
>diff --git a/drivers/gpu/ipu-v3/ipu-common.c b/drivers/gpu/ipu-v3/ipu-
>common.c
>index 6a573d21d3cc2..658fa2d3e40c2 100644
>--- a/drivers/gpu/ipu-v3/ipu-common.c
>+++ b/drivers/gpu/ipu-v3/ipu-common.c
>@@ -405,6 +405,14 @@ int ipu_idmac_lock_enable(struct ipuv3_channel
>*channel, int num_bursts)
>   return -EINVAL;
>   }
>
>+  /*
>+   * IPUv3EX / i.MX51 has a different register layout, and on IPUv3M /
>+   * i.MX53 channel arbitration locking doesn't seem to work properly.
>+   * Allow enabling the lock feature on IPUv3H / i.MX6 only.
>+   */
>+  if (bursts && ipu->ipu_type != IPUV3H)
>+  return -EINVAL;
>+
>   for (i = 0; i < ARRAY_SIZE(idmac_lock_en_info); i++) {
>   if (channel->num == idmac_lock_en_info[i].chnum)
>   break;
>--
>2.11.0
I tested this patch on CX9020 (i.MX53) and can confirm the reported issue gets 
fixed by it.
Tested-by: Patrick Brünn 

Thanks for this super-fast fix, Philipp.

Regards,
Patrick
Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff
Registered office: Verl, Germany | Register court: Guetersloh HRA 7075


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