[RFC PATCH 1/4] memory: tegra124-emc: Add EMC driver

[Thierry Reding]

On Wed, Jun 18, 2014 at 07:23:47PM +0200, Tomeu Vizoso wrote:
> On 06/17/2014 06:15 PM, Stephen Warren wrote:
> >On 06/17/2014 06:16 AM, Tomeu Vizoso wrote:
> >>On 06/16/2014 10:02 PM, Stephen Warren wrote:
> >>>On 06/16/2014 07:35 AM, Tomeu Vizoso wrote:
> >>>>+#ifdef CONFIG_TEGRA124_EMC
> >>>>+int tegra124_emc_reserve_bandwidth(unsigned int consumer, unsigned
> >>>>long rate);
> >>>>+void tegra124_emc_set_floor(unsigned long freq);
> >>>>+void tegra124_emc_set_ceiling(unsigned long freq);
> >>>>+#else
> >>>>+int tegra124_emc_reserve_bandwidth(unsigned int consumer, unsigned
> >>>>long rate)
> >>>>+{ return -ENODEV; }
> >>>>+void tegra124_emc_set_floor(unsigned long freq)
> >>>>+{ return; }
> >>>>+void tegra124_emc_set_ceiling(unsigned long freq)
> >>>>+{ return; }
> >>>>+#endif
> >>>
> >>>I'll repeat what I said off-list so that we can have the whole
> >>>conversation on the list:
> >>>
> >>>That looks like a custom Tegra-specific API. I think it'd be much better
> >>>to integrate this into the common clock framework as a standard clock
> >>>constraints API. There are other use-cases for clock constraints besides
> >>>EMC scaling (e.g. some in audio on Tegra, and I'm sure many on other
> >>>SoCs too).
> >>
> >>Yes, I wrote a bit in the cover letter about our requirements and how
> >>they map to the CCF. Could you please comment on that?
> >
> >My comments remain the same. I believe this is something that belongs in
> >the clock driver, or at the least, some API that takes a struct clock as
> >its parameter, so that drivers can use the existing DT clock lookup
> >mechanism.
> 
> Ok, let me put this strawman here to see if I have gotten close to what you
> have in mind:
> 
> * add per-client accounting (Rabin's patches referenced before)
> 
> * add clk_set_floor, to be used by cpufreq, load stats, etc.
> 
> * add clk_set_ceiling, to be used by battery drivers, thermal, etc.
> 
> * an EMC driver would collect bandwidth and latency requests from consumers
> and call clk_set_floor on the EMC clock.
> 
> * the EMC driver would also register for rate change notifications in the
> EMC clock and would update the latency allowance registers at that point.

Latency allowance registers are part of the MC rather than the EMC. So I
think we have two options: a) have a unified driver for MC and EMC or b)
provide two parts of the API in two drivers.

Or perhaps c), create a generic framework that both MC and EMC can
register with (bandwidth for EMC, latency for MC).

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140619/4ac6b0a7/attachment.sig>

[RFC PATCH 1/4] memory: tegra124-emc: Add EMC driver

[Thierry Reding]

On Wed, Jun 18, 2014 at 11:46:49AM -0600, Stephen Warren wrote:
> On 06/18/2014 11:23 AM, Tomeu Vizoso wrote:
> > On 06/17/2014 06:15 PM, Stephen Warren wrote:
> >> On 06/17/2014 06:16 AM, Tomeu Vizoso wrote:
> >>> On 06/16/2014 10:02 PM, Stephen Warren wrote:
>  On 06/16/2014 07:35 AM, Tomeu Vizoso wrote:
> > +#ifdef CONFIG_TEGRA124_EMC
> > +int tegra124_emc_reserve_bandwidth(unsigned int consumer, unsigned
> > long rate);
> > +void tegra124_emc_set_floor(unsigned long freq);
> > +void tegra124_emc_set_ceiling(unsigned long freq);
> > +#else
> > +int tegra124_emc_reserve_bandwidth(unsigned int consumer, unsigned
> > long rate)
> > +{ return -ENODEV; }
> > +void tegra124_emc_set_floor(unsigned long freq)
> > +{ return; }
> > +void tegra124_emc_set_ceiling(unsigned long freq)
> > +{ return; }
> > +#endif
> 
>  I'll repeat what I said off-list so that we can have the whole
>  conversation on the list:
> 
>  That looks like a custom Tegra-specific API. I think it'd be much
>  better
>  to integrate this into the common clock framework as a standard clock
>  constraints API. There are other use-cases for clock constraints
>  besides
>  EMC scaling (e.g. some in audio on Tegra, and I'm sure many on other
>  SoCs too).
> >>>
> >>> Yes, I wrote a bit in the cover letter about our requirements and how
> >>> they map to the CCF. Could you please comment on that?
> >>
> >> My comments remain the same. I believe this is something that belongs in
> >> the clock driver, or at the least, some API that takes a struct clock as
> >> its parameter, so that drivers can use the existing DT clock lookup
> >> mechanism.
> > 
> > Ok, let me put this strawman here to see if I have gotten close to what
> > you have in mind:
> > 
> > * add per-client accounting (Rabin's patches referenced before)
> > 
> > * add clk_set_floor, to be used by cpufreq, load stats, etc.
> > 
> > * add clk_set_ceiling, to be used by battery drivers, thermal, etc.
> 
> Yes. I'd expect those to be maintained per-client, and so the clock core
> (or whatever higher level code implements clk_set_floor/ceiling)
> performs the logic that "blends" together all the different requests
> from different clients.
> 
> As an aside, for audio usage, I would expect clk_set_rate to be a
> per-client (rather than per HW clock) operation too, and to error out if
> one client says it wants to set pll_a to the rate needed for
> 44.1KHz-based audio and a different client wants the rate for
> 48KHz-based audio.

[RFC PATCH 1/4] memory: tegra124-emc: Add EMC driver

[Thierry Reding]

On Wed, Jun 18, 2014 at 04:09:06PM -0600, Stephen Warren wrote:
> On 06/18/2014 04:03 PM, Thierry Reding wrote:
> > On Wed, Jun 18, 2014 at 11:46:49AM -0600, Stephen Warren wrote:
> >> On 06/18/2014 11:23 AM, Tomeu Vizoso wrote:
> >>> On 06/17/2014 06:15 PM, Stephen Warren wrote:
> >>>> On 06/17/2014 06:16 AM, Tomeu Vizoso wrote:
> >>>>> On 06/16/2014 10:02 PM, Stephen Warren wrote:
> >>>>>> On 06/16/2014 07:35 AM, Tomeu Vizoso wrote:
> >>>>>>> +#ifdef CONFIG_TEGRA124_EMC
> >>>>>>> +int tegra124_emc_reserve_bandwidth(unsigned int consumer, unsigned
> >>>>>>> long rate);
> >>>>>>> +void tegra124_emc_set_floor(unsigned long freq);
> >>>>>>> +void tegra124_emc_set_ceiling(unsigned long freq);
> >>>>>>> +#else
> >>>>>>> +int tegra124_emc_reserve_bandwidth(unsigned int consumer, unsigned
> >>>>>>> long rate)
> >>>>>>> +{ return -ENODEV; }
> >>>>>>> +void tegra124_emc_set_floor(unsigned long freq)
> >>>>>>> +{ return; }
> >>>>>>> +void tegra124_emc_set_ceiling(unsigned long freq)
> >>>>>>> +{ return; }
> >>>>>>> +#endif
> >>>>>>
> >>>>>> I'll repeat what I said off-list so that we can have the whole
> >>>>>> conversation on the list:
> >>>>>>
> >>>>>> That looks like a custom Tegra-specific API. I think it'd be much
> >>>>>> better
> >>>>>> to integrate this into the common clock framework as a standard clock
> >>>>>> constraints API. There are other use-cases for clock constraints
> >>>>>> besides
> >>>>>> EMC scaling (e.g. some in audio on Tegra, and I'm sure many on other
> >>>>>> SoCs too).
> >>>>>
> >>>>> Yes, I wrote a bit in the cover letter about our requirements and how
> >>>>> they map to the CCF. Could you please comment on that?
> >>>>
> >>>> My comments remain the same. I believe this is something that belongs in
> >>>> the clock driver, or at the least, some API that takes a struct clock as
> >>>> its parameter, so that drivers can use the existing DT clock lookup
> >>>> mechanism.
> >>>
> >>> Ok, let me put this strawman here to see if I have gotten close to what
> >>> you have in mind:
> >>>
> >>> * add per-client accounting (Rabin's patches referenced before)
> >>>
> >>> * add clk_set_floor, to be used by cpufreq, load stats, etc.
> >>>
> >>> * add clk_set_ceiling, to be used by battery drivers, thermal, etc.
> >>
> >> Yes. I'd expect those to be maintained per-client, and so the clock core
> >> (or whatever higher level code implements clk_set_floor/ceiling)
> >> performs the logic that "blends" together all the different requests
> >> from different clients.
> >>
> >> As an aside, for audio usage, I would expect clk_set_rate to be a
> >> per-client (rather than per HW clock) operation too, and to error out if
> >> one client says it wants to set pll_a to the rate needed for
> >> 44.1KHz-based audio and a different client wants the rate for
> >> 48KHz-based audio.
> > 
> > From what I remember, Mike was fairly strongly opposing the idea of
> > virtual clocks, but what you're proposing here sounds like it would
> > assume the existence of virtual clocks. clk_set_rate() per client
> > doesn't work with the current API as I understand it.
> > 
> > Or perhaps what you're proposing isn't about the individual clocks at
> > all but rather about a mechanism to express constraints for a set of
> > clocks?
> 
> This doesn't have anything to do with virtual clocks. As you mention,
> it's just about constraints.
> 
> One user of clock "cpu" wants min rate 216MHz. Another wants max rate
> 1GHz. cpufreq will request some rate between the 2, or be capped to
> those limits. These set of imposed constraints would need to be stored
> per client of the clock, not per HW clock, since many clients could set
> different max rates (e.g. thermal throttle 1.5GHz due to temperature,
> CPU policy 1GHz due to the user selecting low CPU power, etc.)
> 
> Similarly for audio, of there are N clients of 1 clock/PLL, and they
> each want the PLL to run at a different rate, something needs to detect
> that and deny it.

I'm wondering how this should work with the current API. Could the clock
core be modified to return a per-client struct clk * that references the
hardware clock internally? Or do we need to add a new API?

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140619/c06e9ba5/attachment.sig>

[RFC PATCH 1/4] memory: tegra124-emc: Add EMC driver

[Thierry Reding]

On Wed, Jun 18, 2014 at 04:33:39PM -0600, Stephen Warren wrote:
> On 06/18/2014 04:19 PM, St?phane Marchesin wrote:
> > On Wed, Jun 18, 2014 at 3:00 PM, Thierry Reding
> >  wrote:
> >> On Wed, Jun 18, 2014 at 07:23:47PM +0200, Tomeu Vizoso wrote:
> >>> On 06/17/2014 06:15 PM, Stephen Warren wrote:
> >>>> On 06/17/2014 06:16 AM, Tomeu Vizoso wrote:
> >>>>> On 06/16/2014 10:02 PM, Stephen Warren wrote:
> >>>>>> On 06/16/2014 07:35 AM, Tomeu Vizoso wrote:
> >>>>>>> +#ifdef CONFIG_TEGRA124_EMC
> >>>>>>> +int tegra124_emc_reserve_bandwidth(unsigned int consumer, unsigned
> >>>>>>> long rate);
> >>>>>>> +void tegra124_emc_set_floor(unsigned long freq);
> >>>>>>> +void tegra124_emc_set_ceiling(unsigned long freq);
> >>>>>>> +#else
> >>>>>>> +int tegra124_emc_reserve_bandwidth(unsigned int consumer, unsigned
> >>>>>>> long rate)
> >>>>>>> +{ return -ENODEV; }
> >>>>>>> +void tegra124_emc_set_floor(unsigned long freq)
> >>>>>>> +{ return; }
> >>>>>>> +void tegra124_emc_set_ceiling(unsigned long freq)
> >>>>>>> +{ return; }
> >>>>>>> +#endif
> >>>>>>
> >>>>>> I'll repeat what I said off-list so that we can have the whole
> >>>>>> conversation on the list:
> >>>>>>
> >>>>>> That looks like a custom Tegra-specific API. I think it'd be much 
> >>>>>> better
> >>>>>> to integrate this into the common clock framework as a standard clock
> >>>>>> constraints API. There are other use-cases for clock constraints 
> >>>>>> besides
> >>>>>> EMC scaling (e.g. some in audio on Tegra, and I'm sure many on other
> >>>>>> SoCs too).
> >>>>>
> >>>>> Yes, I wrote a bit in the cover letter about our requirements and how
> >>>>> they map to the CCF. Could you please comment on that?
> >>>>
> >>>> My comments remain the same. I believe this is something that belongs in
> >>>> the clock driver, or at the least, some API that takes a struct clock as
> >>>> its parameter, so that drivers can use the existing DT clock lookup
> >>>> mechanism.
> >>>
> >>> Ok, let me put this strawman here to see if I have gotten close to what 
> >>> you
> >>> have in mind:
> >>>
> >>> * add per-client accounting (Rabin's patches referenced before)
> >>>
> >>> * add clk_set_floor, to be used by cpufreq, load stats, etc.
> >>>
> >>> * add clk_set_ceiling, to be used by battery drivers, thermal, etc.
> >>>
> >>> * an EMC driver would collect bandwidth and latency requests from 
> >>> consumers
> >>> and call clk_set_floor on the EMC clock.
> >>>
> >>> * the EMC driver would also register for rate change notifications in the
> >>> EMC clock and would update the latency allowance registers at that point.
> >>
> >> Latency allowance registers are part of the MC rather than the EMC. So I
> >> think we have two options: a) have a unified driver for MC and EMC or b)
> >> provide two parts of the API in two drivers.
> >>
> >> Or perhaps c), create a generic framework that both MC and EMC can
> >> register with (bandwidth for EMC, latency for MC).
> > 
> > Is there any motivation for keeping MC and EMC separate? In my mind,
> > the solution was always to handle those together.
> 
> Well, they are documented as being separate HW modules in the TRM.
> 
> I know there's an interlock in HW so that when the EMC clock is changed,
> the EMC registers can flip atomically to a new configuration.
> 
> I'm not aware of any similar HW interlock between MC and EMC registers.
> That would imply that very tight co-ordination shouldn't be required.
> 
> Do the MC latency allowance registers /really/ need to be *very tightly*
> managed whenever the EMC clock is changed, or is it just a matter of it
> being a good idea to update EMC clock and MC latency allowance registers
> at roughly the same time?

I guess depending on the timing you could get FIFO underflows if the
registers aren't updated promptly enough. Once the programming takes
effect things should be able to catch up again, but it's possible that
there could be glitches.

> Even if there's some co-ordination required,
> maybe it can be handled rather like cpufreq notifications; use clock
> pre-rate change notifications to set MC up in a way that'll work at both
> old/new EMC clocks, and then clock post-rate notifications to the final
> MC configuration?

Yes, I think something like that should work. As I understand it, the
latency allowance is dependent on the EMC frequency, so in case where
the EMC frequency is increased, adjusting in a post-rate notifier should
be fine. When the EMC frequency is decreased, then programming the
latency allowance registers in a pre-rate notifier should allow glitch-
free transition.

Note that this is all purely theoretical knowledge, so I have no idea if
it'll actually work like that in practice.

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140619/24068e19/attachment.sig>

[PATCH] drm/tegra: add MODULE_DEVICE_TABLEs

[Thierry Reding]

On Wed, Jun 18, 2014 at 04:21:18PM -0600, Stephen Warren wrote:
> On 06/18/2014 03:51 PM, Thierry Reding wrote:
> > On Wed, Jun 18, 2014 at 03:19:15PM -0600, Stephen Warren wrote:
> >> From: Stephen Warren 
> >>
> >> When tegra-drm.ko is built as a module, these MODULE_DEVICE_TABLEs allow
> >> the module to be auto-loaded since the module will match the devices
> >> instantiated from device tree.
> > 
> > I vaguely remember doing something like this a while back and getting a
> > bunch of link-time errors. But I assume that you've tested this, so I
> > must be remembering wrongly.
> 
> Were the problems due to:
> 
> a) Simply building the tegradrm driver as modules.
> 
> I vaguely recall some runtime issues with tegradrm as a module, but I'm
> not sure about build issues. I don't think this patch could make this
> any worse.
> 
> b) Building as modules works, but adding MODULE_DEVICE_TABLE broke that.

I think it was this variant. Although adding MODULE_DEVICE_TABLE also
broke building the driver as builtin module. I think the issue was that
the linker was complaining about some symbol being defined multiple
times. But admittedly this was a long time ago, so I'm not sure that my
memory is entirely accurate.

> This seems unlikely since *many* module in the kernel have a
> MODULE_DEVICE_TABLE...
> 
> Certainly, with this patch applied, building tegradrm as a module in
> next-20140611 works out just fine, and the code runs fine too. Building
> tegra_defconfig (which has tegradrm builtin) on Linus' master with this
> patch applied also works out fine.

Okay, sounds good then. I'll do some build testing to see if I can
reproduce the errors, otherwise this looks good to me.

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140619/f8e967b0/attachment-0001.sig>

[PATCH] drivers/i915: Fix unnoticed failure of init_ring_common()

[Konrad Zapalowicz]

This commit add check for return value of init_ring_common() in the
init_render_ring(). Now, when failure is detected the error code is
propagated to the caller layer instead of being ignored.

I believe that this fix will have a positive impact on the oops that
I hit recently and which starts when init_ring_common() fails:

[drm:init_ring_common] *ERROR* render ring initialization failed
 ctl 0001f001 head 000c tail  start 003eb000
BUG: unable to handle kernel NULL pointer dereference at 006c
IP: [] i915_gem_obj_to_ggtt+0x9/0x40 [i915]

Signed-off-by: Konrad Zapalowicz 
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 279488a..d205b0d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -604,6 +604,8 @@ static int init_render_ring(struct intel_engine_cs *ring)
struct drm_device *dev = ring->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
int ret = init_ring_common(ring);
+   if (ret)
+   return ret;

/* WaTimedSingleVertexDispatch:cl,bw,ctg,elk,ilk,snb */
if (INTEL_INFO(dev)->gen >= 4 && INTEL_INFO(dev)->gen < 7)
-- 
1.8.1.2

[git pull] drm fixes

[Dave Airlie]

Hi Linus,

this looks bigger than it is, as one of the nouveau firmware fixes 
"drm/gf100-/gr: report class data to host on fwmthd failure" regenerates a 
bunch of the firmware files after changing the assembly by a few lines,
without that, its more of a
" 36 files changed, 370 insertions(+), 129 deletions(-)"

It contains some vt.c fixes acked by Greg, for rare hard hangs on i915 
loading, that also fixes hangs on reload and spurious register write 
errors,
drm core: one fix for uninit memory,
nouveau: displayport rework caused a few regressions, Ben has been fixing
them as the appear, along with some other fixes
radeon: pageflipping regression fix, deep color fix, mode validation fixes
i915: fbc disable, vga console kick off, backlight fix, div 0 fix.

Dave.

The following changes since commit 7171511eaec5bf23fb06078f59784a3a0626b38f:

  Linux 3.16-rc1 (2014-06-15 17:45:28 -1000)

are available in the git repository at:

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

for you to fetch changes up to 884d6147ba19640a40fb45efe64360cdf92cac27:

  Merge tag 'drm-intel-fixes-2014-06-17' of 
git://anongit.freedesktop.org/drm-intel into drm-next (2014-06-19 10:54:35 
+1000)



Alex Deucher (2):
  drm/radeon: update mode_valid testing for DP
  drm/radeon: improve dvi_mode_valid

Ben Skeggs (8):
  drm/gk104/clk: only touch divider for mode we'll be using
  drm/gk104/ibus: increase various random timeouts
  drm/gf100-/gr: report class data to host on fwmthd failure
  drm/gk104/fb/ram: fixups from an earlier search+replace
  drm/nouveau/disp/dp: don't touch link config after success
  drm/nv50/disp: fix a potential oops in supervisor handling
  drm/nouveau/pwr: fix typo in fifo wrap handling
  drm/gf117/i2c: no aux channels on this chipset

Chris Wilson (3):
  drm/i915: Disable FBC by default also on Haswell and later
  drm/i95: Initialize active ring->pid to -1
  drm/i915: Reorder semaphore deadlock check

Daniel Vetter (5):
  vt: Fix replacement console check when unbinding
  vt: Fix up unregistration of vt drivers
  vt: Don't ignore unbind errors in vt_unbind
  drm/i915: Fixup global gtt cleanup
  drm/i915: Kick out vga console

Dave Airlie (5):
  Merge branch 'drm-nouveau-next' of 
git://anongit.freedesktop.org/git/nouveau/linux-2.6 into drm-next
  Merge branch 'drm-next-3.16' of git://people.freedesktop.org/~agd5f/linux 
into drm-next
  Merge branch 'drm-nouveau-next' of 
git://anongit.freedesktop.org/git/nouveau/linux-2.6 into drm-next
  Merge branch 'drm-fixes-3.16' of 
git://people.freedesktop.org/~agd5f/linux into drm-next
  Merge tag 'drm-intel-fixes-2014-06-17' of 
git://anongit.freedesktop.org/drm-intel into drm-next

Fredrik H?glund (1):
  drm/radeon: use pixel formats instead of depth/bpp

Imre Deak (1):
  drm/i915: fix possible refcount leak when resetting forcewake

Jani Nikula (2):
  drm/i915: set backlight duty cycle after backlight enable for gen4
  Merge remote-tracking branch 'drm-intel/topic/kicking-dogs-and-vgacon' 
into drm-intel-fixes

Maarten Lankhorst (1):
  drm/nouveau/disp: fix oops in destructor with headless cards

Mario Kleiner (3):
  drm/radeon: Bypass hw lut's for > 8 bpc framebuffer scanout.
  drm/nouveau/kms: reference vblank for crtc during pageflip.
  drm/radeon: Use dce5/6 hdmi deep color clock setup also on dce8+

Martin Peres (1):
  drm/nouveau/doc: update the thermal documentation

Michel D?nzer (2):
  Revert "drm/radeon: remove drm_vblank_get|put from pflip handling"
  drm/radeon: Fix radeon_irq_kms_pflip_irq_get/put() imbalance

Pierre Moreau (2):
  drm/nv50/gr: fix overlap while zeroing zcull regions
  drm/nv50/gr: remove an unneeded write while initialising PGRAPH

Rob Clark (1):
  drm: fix uninitialized acquire_ctx fields (v2)

Tom O'Rourke (1):
  drm/i915/bdw: remove erroneous chv specific workarounds from bdw code

Ville Syrj?l? (1):
  drm/i915: Avoid div-by-zero when pixel_multiplier is zero

 Documentation/thermal/nouveau_thermal  |   7 +-
 drivers/gpu/drm/drm_modeset_lock.c |   1 +
 drivers/gpu/drm/i915/i915_dma.c|  47 ++-
 drivers/gpu/drm/i915/i915_gem_gtt.c|   9 +-
 drivers/gpu/drm/i915/i915_gpu_error.c  |   3 +-
 drivers/gpu/drm/i915/i915_irq.c|  18 +-
 drivers/gpu/drm/i915/intel_panel.c |   5 +-
 drivers/gpu/drm/i915/intel_pm.c|   9 +-
 drivers/gpu/drm/i915/intel_ringbuffer.h|   2 +-
 drivers/gpu/drm/i915/intel_sdvo.c  |   4 +-
 drivers/gpu/drm/i915/intel_uncore.c|   3 +-
 drivers/gpu/drm/nouveau/Makefile   |   1 +
 drivers/gpu/drm/nouveau/core/engine/device/nvc0.c  |   2 +-
 drivers/gpu/drm/nouveau/core/engine/disp/base.c|   6 +-
 drivers/gpu

[PATCH 1/3] drm/radeon: stop poisoning the GART TLB

[Michel Dänzer]

On 15.06.2014 21:48, Christian K?nig wrote:
> Am 13.06.2014 23:31, schrieb Alex Deucher:
>> On Fri, Jun 13, 2014 at 11:45 AM, Christian K?nig
>>  wrote:
>>> Hi Marek,
>>>
>>> ah, yes! Piglit in combination with that patch can indeed crash the box.
>>>
>>> Going to investigate now that I can reproduce it.
>> I wonder if it's a clockgating issue with the MC or BIF?  You might
>> try adjusting the rdev->cg_flags (try setting it to 0) in
>> radeon_asic.c or disabling dpm.
> 
> Unfortunately that was just a false alarm.
> 
> I was just on a branch which didn't had the "stop poisoning the GART
> TLB" patch, after applying this patch I can again let piglit run for the
> whole night without a lockup.
> 
> No idea what goes wrong when Marek runs piglit, but 3.15.0+"stop
> poisoning the GART TLB"+"force_gtt" is rock solid here.

FWIW, 3.15 doesn't survive piglit on my Bonaire either, but 3.14 is
fine. 3.15 seems stable on Kaveri though, but I haven't tried the
force_gtt patch on that yet.

There have also been a number of bug reports about stability regressions
in 3.15 on various SI and CIK cards. It seems likely that at least some
of those are related to this issue as well.

If we can't figure out the problem soon, we probably need to revert the
'Use normal BOs for page tables' and dependent changes at least for 3.15.y?


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

[PATCH] drm/fb-helper: Return correct type to match drm_fb_helper_debug_enter() prototype

[Liu Ying]

The return type of drm_fb_helper_debug_enter() is int, so we should return '0'
instead of 'false'.

Signed-off-by: Liu Ying 
---
 drivers/gpu/drm/drm_fb_helper.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index d5d8cea..8daa4ad 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -200,7 +200,7 @@ int drm_fb_helper_debug_enter(struct fb_info *info)
int i;

if (list_empty(&kernel_fb_helper_list))
-   return false;
+   return 0;

list_for_each_entry(helper, &kernel_fb_helper_list, kernel_fb_list) {
for (i = 0; i < helper->crtc_count; i++) {
-- 
1.7.9.5

[PATCH] drm/radeon: fix race condition in radeon_crtc_page_flip

[Michel Dänzer]

This patch only applies to 3.15, right?


On 19.06.2014 02:11, Christian K?nig wrote:
> From: Christian K?nig 
> 
> radeon_crtc_handle_flip can be called concurrently and if
> we set the unpin_work to early try to flip an unpinned BO or
> worse.

Spelling: 'too early'

Maybe something like:

radeon_crtc_handle_flip can be called concurrently, and if
we set the unpin_work too early, it may try to flip an unpinned BO or
worse.


> Signed-off-by: Christian K?nig 
> Cc: stable at vger.kernel.org
> ---
>  drivers/gpu/drm/radeon/radeon_display.c | 31 ---
>  1 file changed, 16 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/radeon_display.c 
> b/drivers/gpu/drm/radeon/radeon_display.c
> index 356b733..cf22741 100644
> --- a/drivers/gpu/drm/radeon/radeon_display.c
> +++ b/drivers/gpu/drm/radeon/radeon_display.c
> @@ -393,17 +393,6 @@ static int radeon_crtc_page_flip(struct drm_crtc *crtc,
>  
>   INIT_WORK(&work->work, radeon_unpin_work_func);
>  
> - /* We borrow the event spin lock for protecting unpin_work */
> - spin_lock_irqsave(&dev->event_lock, flags);
> - if (radeon_crtc->unpin_work) {
> - DRM_DEBUG_DRIVER("flip queue: crtc already busy\n");
> - r = -EBUSY;
> - goto unlock_free;
> - }
> - radeon_crtc->unpin_work = work;
> - radeon_crtc->deferred_flip_completion = 0;
> - spin_unlock_irqrestore(&dev->event_lock, flags);
> -
>   /* pin the new buffer */
>   DRM_DEBUG_DRIVER("flip-ioctl() cur_fbo = %p, cur_bbo = %p\n",
>work->old_rbo, rbo);
> @@ -461,10 +450,6 @@ static int radeon_crtc_page_flip(struct drm_crtc *crtc,
>   base &= ~7;
>   }
>  
> - spin_lock_irqsave(&dev->event_lock, flags);
> - work->new_crtc_base = base;
> - spin_unlock_irqrestore(&dev->event_lock, flags);
> -
>   /* update crtc fb */
>   crtc->primary->fb = fb;
>  
> @@ -477,6 +462,22 @@ static int radeon_crtc_page_flip(struct drm_crtc *crtc,
>   /* set the proper interrupt */
>   radeon_pre_page_flip(rdev, radeon_crtc->crtc_id);
>  
> + /* We borrow the event spin lock for protecting unpin_work */
> + spin_lock_irqsave(&dev->event_lock, flags);
> + if (radeon_crtc->unpin_work) {
> + spin_unlock_irqrestore(&dev->event_lock, flags);
> + radeon_post_page_flip(rdev, radeon_crtc->crtc_id);
> + drm_vblank_put(dev, radeon_crtc->crtc_id);
> +
> + DRM_DEBUG_DRIVER("flip queue: crtc already busy\n");
> + r = -EBUSY;
> + goto pflip_cleanup1;
> + }
> + radeon_crtc->unpin_work = work;
> + radeon_crtc->deferred_flip_completion = 0;
> + work->new_crtc_base = base;
> + spin_unlock_irqrestore(&dev->event_lock, flags);
> +

This introduces a path where crtc->primary->fb is updated, but then we
return -EBUSY.


It also introduces a warning:

drivers/gpu/drm/radeon/radeon_display.c: In function ?radeon_crtc_page_flip?:
drivers/gpu/drm/radeon/radeon_display.c:496:1: warning: label ?unlock_free? 
defined but not used [-Wunused-label]
 unlock_free:
 ^


Apart from that, looks good.


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

[PATCH] drm/exynos: defer hdmi probe when fail to get regulators

[Inki Dae]

On 2014? 06? 18? 22:42, Rahul Sharma wrote:
> HDMI probe proceeds with dummy regulators when the regulators
> are not provided in DT node or regulator provider has not get
> probed or failed to register the regulators.
> 
> This patch modify hdmi driver to defer the probe in case the
> regulators are not available.

No, already available. See the below comments.

> 
> Signed-off-by: Rahul Sharma 
> ---
> Based on exynos-drm-fixes branch in Inki dae's tree.
>  drivers/gpu/drm/exynos/exynos_hdmi.c |   69 
> --
>  1 file changed, 50 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c 
> b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index aa259b0..3f24c49 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -83,7 +83,7 @@ struct hdmi_resources {
>   struct clk  *sclk_pixel;
>   struct clk  *sclk_hdmiphy;
>   struct clk  *mout_hdmi;
> - struct regulator_bulk_data  *regul_bulk;
> + struct regulator**regulators;
>   int regul_count;
>  };
>  
> @@ -2022,6 +2022,36 @@ static void hdmi_commit(struct exynos_drm_display 
> *display)
>   hdmi_conf_apply(hdata);
>  }
>  
> +int hdmi_regulator_enable(struct hdmi_context *hdata)
> +{
> + struct hdmi_resources *res = &hdata->res;
> + int i, ret;
> +
> + for (i = 0; i < res->regul_count; ++i) {
> + ret = regulator_enable(res->regulators[i]);
> + if (ret < 0) {
> + DRM_ERROR("fail to enable regulators.\n");
> + return ret;
> + }
> + }
> + return 0;
> +}
> +
> +int hdmi_regulator_disable(struct hdmi_context *hdata)
> +{
> + struct hdmi_resources *res = &hdata->res;
> + int i, ret;
> +
> + for (i = 0; i < res->regul_count; ++i) {
> + ret = regulator_disable(res->regulators[i]);
> + if (ret < 0) {
> + DRM_ERROR("fail to disable regulators.\n");
> + return ret;
> + }
> + }
> + return 0;
> +}
> +
>  static void hdmi_poweron(struct exynos_drm_display *display)
>  {
>   struct hdmi_context *hdata = display->ctx;
> @@ -2039,8 +2069,8 @@ static void hdmi_poweron(struct exynos_drm_display 
> *display)
>  
>   pm_runtime_get_sync(hdata->dev);
>  
> - if (regulator_bulk_enable(res->regul_count, res->regul_bulk))
> - DRM_DEBUG_KMS("failed to enable regulator bulk\n");
> + if (hdmi_regulator_enable(hdata))
> + DRM_DEBUG_KMS("failed to enable regulators\n");
>  
>   /* set pmu hdmiphy control bit to enable hdmiphy */
>   regmap_update_bits(hdata->pmureg, PMU_HDMI_PHY_CONTROL,
> @@ -2077,7 +2107,8 @@ static void hdmi_poweroff(struct exynos_drm_display 
> *display)
>   regmap_update_bits(hdata->pmureg, PMU_HDMI_PHY_CONTROL,
>   PMU_HDMI_PHY_ENABLE_BIT, 0);
>  
> - regulator_bulk_disable(res->regul_count, res->regul_bulk);
> + if (hdmi_regulator_disable(hdata))
> + DRM_DEBUG_KMS("failed to disable regulators\n");
>  
>   pm_runtime_put_sync(hdata->dev);
>  
> @@ -2211,24 +2242,24 @@ static int hdmi_resources_init(struct hdmi_context 
> *hdata)
>  
>   clk_set_parent(res->mout_hdmi, res->sclk_pixel);
>  
> - res->regul_bulk = devm_kzalloc(dev, ARRAY_SIZE(supply) *
> - sizeof(res->regul_bulk[0]), GFP_KERNEL);
> - if (!res->regul_bulk) {
> - ret = -ENOMEM;
> - goto fail;
> - }
> + res->regul_count = ARRAY_SIZE(supply);
> +
> + res->regulators = devm_kzalloc(dev, res->regul_count *
> + sizeof(res->regulators[0]), GFP_KERNEL);
> + if (!res->regulators)
> + return -ENOMEM;
> +
>   for (i = 0; i < ARRAY_SIZE(supply); ++i) {
> - res->regul_bulk[i].supply = supply[i];
> - res->regul_bulk[i].consumer = NULL;
> - }
> - ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(supply), res->regul_bulk);
> - if (ret) {
> - DRM_ERROR("failed to get regulators\n");
> - return ret;
> + res->regulators[i] =
> + devm_regulator_get_optional(dev, supply[i]);
> + if (IS_ERR(res->regulators[i])) {
> + DRM_ERROR("fail to get regulator: %s.\n",
> + supply[i]);
> + return -EPROBE_DEFER;

devm_regulator_get_optional function would return -EPROBE_DEFER if
needed. So it's not good to force returning -EPROBE_DEFER in case of all
errors.

Thanks,
Inki Dae

> + }
>   }
> - res->regul_count = ARRAY_SIZE(supply);
>  
> - return ret;
> + return 0;
>  fail:
>   DRM_ERROR("HDMI resource init - failed\n");
>   return ret;
> 

[REPOST PATCH 1/8] fence: dma-buf cross-device synchronization (v17)

[Sumit Semwal]

Hi Greg,

On 19 June 2014 06:55, Rob Clark  wrote:
> On Wed, Jun 18, 2014 at 9:16 PM, Greg KH  
> wrote:
>> On Wed, Jun 18, 2014 at 12:36:54PM +0200, Maarten Lankhorst wrote:
>>> A fence can be attached to a buffer which is being filled or consumed
>>> by hw, to allow userspace to pass the buffer without waiting to another
>>> device.  For example, userspace can call page_flip ioctl to display the
>>> next frame of graphics after kicking the GPU but while the GPU is still
>>> rendering.  The display device sharing the buffer with the GPU would
>>> attach a callback to get notified when the GPU's rendering-complete IRQ
>>> fires, to update the scan-out address of the display, without having to
>>> wake up userspace.
>>>
>>> A driver must allocate a fence context for each execution ring that can
>>> run in parallel. The function for this takes an argument with how many
>>> contexts to allocate:
>>>   + fence_context_alloc()
>>>
>>> A fence is transient, one-shot deal.  It is allocated and attached
>>> to one or more dma-buf's.  When the one that attached it is done, with
>>> the pending operation, it can signal the fence:
>>>   + fence_signal()
>>>
>>> To have a rough approximation whether a fence is fired, call:
>>>   + fence_is_signaled()
>>>
>>> The dma-buf-mgr handles tracking, and waiting on, the fences associated
>>> with a dma-buf.
>>>
>>> The one pending on the fence can add an async callback:
>>>   + fence_add_callback()
>>>
>>> The callback can optionally be cancelled with:
>>>   + fence_remove_callback()
>>>
>>> To wait synchronously, optionally with a timeout:
>>>   + fence_wait()
>>>   + fence_wait_timeout()
>>>
>>> When emitting a fence, call:
>>>   + trace_fence_emit()
>>>
>>> To annotate that a fence is blocking on another fence, call:
>>>   + trace_fence_annotate_wait_on(fence, on_fence)
>>>
>>> A default software-only implementation is provided, which can be used
>>> by drivers attaching a fence to a buffer when they have no other means
>>> for hw sync.  But a memory backed fence is also envisioned, because it
>>> is common that GPU's can write to, or poll on some memory location for
>>> synchronization.  For example:
>>>
>>>   fence = custom_get_fence(...);
>>>   if ((seqno_fence = to_seqno_fence(fence)) != NULL) {
>>> dma_buf *fence_buf = seqno_fence->sync_buf;
>>> get_dma_buf(fence_buf);
>>>
>>> ... tell the hw the memory location to wait ...
>>> custom_wait_on(fence_buf, seqno_fence->seqno_ofs, fence->seqno);
>>>   } else {
>>> /* fall-back to sw sync * /
>>> fence_add_callback(fence, my_cb);
>>>   }
>>>
>>> On SoC platforms, if some other hw mechanism is provided for synchronizing
>>> between IP blocks, it could be supported as an alternate implementation
>>> with it's own fence ops in a similar way.
>>>
>>> enable_signaling callback is used to provide sw signaling in case a cpu
>>> waiter is requested or no compatible hardware signaling could be used.
>>>
>>> The intention is to provide a userspace interface (presumably via eventfd)
>>> later, to be used in conjunction with dma-buf's mmap support for sw access
>>> to buffers (or for userspace apps that would prefer to do their own
>>> synchronization).
>>>
>>> v1: Original
>>> v2: After discussion w/ danvet and mlankhorst on #dri-devel, we decided
>>> that dma-fence didn't need to care about the sw->hw signaling path
>>> (it can be handled same as sw->sw case), and therefore the fence->ops
>>> can be simplified and more handled in the core.  So remove the signal,
>>> add_callback, cancel_callback, and wait ops, and replace with a simple
>>> enable_signaling() op which can be used to inform a fence supporting
>>> hw->hw signaling that one or more devices which do not support hw
>>> signaling are waiting (and therefore it should enable an irq or do
>>> whatever is necessary in order that the CPU is notified when the
>>> fence is passed).
>>> v3: Fix locking fail in attach_fence() and get_fence()
>>> v4: Remove tie-in w/ dma-buf..  after discussion w/ danvet and mlankorst
>>> we decided that we need to be able to attach one fence to N dma-buf's,
>>> so using the list_head in dma-fence struct would be problematic.
>>> v5: [ Maarten Lankhorst ] Updated for dma-bikeshed-fence and 
>>> dma-buf-manager.
>>> v6: [ Maarten Lankhorst ] I removed dma_fence_cancel_callback and some 
>>> comments
>>> about checking if fence fired or not. This is broken by design.
>>> waitqueue_active during destruction is now fatal, since the signaller
>>> should be holding a reference in enable_signalling until it signalled
>>> the fence. Pass the original dma_fence_cb along, and call __remove_wait
>>> in the dma_fence_callback handler, so that no cleanup needs to be
>>> performed.
>>> v7: [ Maarten Lankhorst ] Set cb->func and only enable sw signaling if
>>> fence wasn't signaled yet, for example for hardware fences that may
>>> choose to signal blindly.
>>> v8: [ 

[PATCH] drm/exynos: defer hdmi probe when fail to get regulators

[Rahul Sharma]

Thanks Inki,

On 19 June 2014 09:34, Inki Dae  wrote:
> On 2014? 06? 18? 22:42, Rahul Sharma wrote:
>> HDMI probe proceeds with dummy regulators when the regulators
>> are not provided in DT node or regulator provider has not get
>> probed or failed to register the regulators.
>>
>> This patch modify hdmi driver to defer the probe in case the
>> regulators are not available.
>
> No, already available. See the below comments.
>
>>
>> Signed-off-by: Rahul Sharma 
>> ---
>> Based on exynos-drm-fixes branch in Inki dae's tree.
>>  drivers/gpu/drm/exynos/exynos_hdmi.c |   69 
>> --
>>  1 file changed, 50 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c 
>> b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> index aa259b0..3f24c49 100644
>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> @@ -83,7 +83,7 @@ struct hdmi_resources {
>>   struct clk  *sclk_pixel;
>>   struct clk  *sclk_hdmiphy;
>>   struct clk  *mout_hdmi;
>> - struct regulator_bulk_data  *regul_bulk;
>> + struct regulator**regulators;
>>   int regul_count;
>>  };
>>
>> @@ -2022,6 +2022,36 @@ static void hdmi_commit(struct exynos_drm_display 
>> *display)
>>   hdmi_conf_apply(hdata);
>>  }
>>
>> +int hdmi_regulator_enable(struct hdmi_context *hdata)
>> +{
>> + struct hdmi_resources *res = &hdata->res;
>> + int i, ret;
>> +
>> + for (i = 0; i < res->regul_count; ++i) {
>> + ret = regulator_enable(res->regulators[i]);
>> + if (ret < 0) {
>> + DRM_ERROR("fail to enable regulators.\n");
>> + return ret;
>> + }
>> + }
>> + return 0;
>> +}
>> +
>> +int hdmi_regulator_disable(struct hdmi_context *hdata)
>> +{
>> + struct hdmi_resources *res = &hdata->res;
>> + int i, ret;
>> +
>> + for (i = 0; i < res->regul_count; ++i) {
>> + ret = regulator_disable(res->regulators[i]);
>> + if (ret < 0) {
>> + DRM_ERROR("fail to disable regulators.\n");
>> + return ret;
>> + }
>> + }
>> + return 0;
>> +}
>> +
>>  static void hdmi_poweron(struct exynos_drm_display *display)
>>  {
>>   struct hdmi_context *hdata = display->ctx;
>> @@ -2039,8 +2069,8 @@ static void hdmi_poweron(struct exynos_drm_display 
>> *display)
>>
>>   pm_runtime_get_sync(hdata->dev);
>>
>> - if (regulator_bulk_enable(res->regul_count, res->regul_bulk))
>> - DRM_DEBUG_KMS("failed to enable regulator bulk\n");
>> + if (hdmi_regulator_enable(hdata))
>> + DRM_DEBUG_KMS("failed to enable regulators\n");
>>
>>   /* set pmu hdmiphy control bit to enable hdmiphy */
>>   regmap_update_bits(hdata->pmureg, PMU_HDMI_PHY_CONTROL,
>> @@ -2077,7 +2107,8 @@ static void hdmi_poweroff(struct exynos_drm_display 
>> *display)
>>   regmap_update_bits(hdata->pmureg, PMU_HDMI_PHY_CONTROL,
>>   PMU_HDMI_PHY_ENABLE_BIT, 0);
>>
>> - regulator_bulk_disable(res->regul_count, res->regul_bulk);
>> + if (hdmi_regulator_disable(hdata))
>> + DRM_DEBUG_KMS("failed to disable regulators\n");
>>
>>   pm_runtime_put_sync(hdata->dev);
>>
>> @@ -2211,24 +2242,24 @@ static int hdmi_resources_init(struct hdmi_context 
>> *hdata)
>>
>>   clk_set_parent(res->mout_hdmi, res->sclk_pixel);
>>
>> - res->regul_bulk = devm_kzalloc(dev, ARRAY_SIZE(supply) *
>> - sizeof(res->regul_bulk[0]), GFP_KERNEL);
>> - if (!res->regul_bulk) {
>> - ret = -ENOMEM;
>> - goto fail;
>> - }
>> + res->regul_count = ARRAY_SIZE(supply);
>> +
>> + res->regulators = devm_kzalloc(dev, res->regul_count *
>> + sizeof(res->regulators[0]), GFP_KERNEL);
>> + if (!res->regulators)
>> + return -ENOMEM;
>> +
>>   for (i = 0; i < ARRAY_SIZE(supply); ++i) {
>> - res->regul_bulk[i].supply = supply[i];
>> - res->regul_bulk[i].consumer = NULL;
>> - }
>> - ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(supply), 
>> res->regul_bulk);
>> - if (ret) {
>> - DRM_ERROR("failed to get regulators\n");
>> - return ret;
>> + res->regulators[i] =
>> + devm_regulator_get_optional(dev, supply[i]);
>> + if (IS_ERR(res->regulators[i])) {
>> + DRM_ERROR("fail to get regulator: %s.\n",
>> + supply[i]);
>> + return -EPROBE_DEFER;
>
> devm_regulator_get_optional function would return -EPROBE_DEFER if
> needed. So it's not good to force returning -EPROBE_DEFER in case of all
> errors.
>

Yea you are right. Its returning  -EPROBE_DEFER. I will change this in next
version.

Regards,
Rahul Sharma.

> Thanks,
> Inki Dae
>
>> + 

[REPOST PATCH 1/8] fence: dma-buf cross-device synchronization (v17)

[Sumit Semwal]

On 19 June 2014 10:24, Greg KH  wrote:
> On Thu, Jun 19, 2014 at 09:57:35AM +0530, Sumit Semwal wrote:
>> Hi Greg,
>>

>> >>
>> >> Who is going to sign up to maintain this code?  (hint, it's not me...)
>> >
>> > that would be Sumit (dma-buf tree)..
>> >
>> > probably we should move fence/reservation/dma-buf into drivers/dma-buf
>> > (or something approximately like that)
>> Yes, that would be me - it might be better to create a new directory
>> as suggested above (drivers/dma-buf).
>
> That's fine with me, there is going to be more than just one file in
> there, right?  :)
>
> greg k-h
Certainly atleast 3 :)

~sumit

[Bug 78321] New: Xorg Freeze with radeon.dpm=1

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

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

Bug ID: 78321
   Summary: Xorg Freeze with radeon.dpm=1
   Product: Drivers
   Version: 2.5
Kernel Version: 3.15.1-1-ARCH
  Hardware: x86-64
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: normal
  Priority: P1
 Component: Video(DRI - non Intel)
  Assignee: drivers_video-dri at kernel-bugs.osdl.org
  Reporter: perry3d at gmail.com
Regression: No

Created attachment 140381
  --> https://bugzilla.kernel.org/attachment.cgi?id=140381&action=edit
journalctl -k output

Hi,

i am using a "[AMD/ATI] Barts XT [Radeon HD 6870]" card on an Arch Installation
with Kernel 3.15.1. 
If i append "radeon.dpm=1" to the kernel parameters i can boot fine into KDM.
But at the login process (KDE) Xorg freezes. I can still move the mouse pointer
and the SysRQ Keys are also working.
Sometimes, after i press SysRQ + K i can manage to get back to KDM and start
another login attempt. And in some cases i can login completely and work in a
stable environment.
I attached the output of "journalctl -k" after getting back to KDM with
SysRQ+K.

Right now i am building linux-git from AUR.

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

[PATCH] drivers/i915: Fix unnoticed failure of init_ring_common()

[Daniel Vetter]

On Thu, Jun 19, 2014 at 12:38 AM, Konrad Zapalowicz
 wrote:
> This commit add check for return value of init_ring_common() in the
> init_render_ring(). Now, when failure is detected the error code is
> propagated to the caller layer instead of being ignored.
>
> I believe that this fix will have a positive impact on the oops that
> I hit recently and which starts when init_ring_common() fails:
>
> [drm:init_ring_common] *ERROR* render ring initialization failed
>  ctl 0001f001 head 000c tail  start 003eb000
> BUG: unable to handle kernel NULL pointer dereference at 006c
> IP: [] i915_gem_obj_to_ggtt+0x9/0x40 [i915]
>
> Signed-off-by: Konrad Zapalowicz 

Do you have the full Oops somewhere?

> ---
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 279488a..d205b0d 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -604,6 +604,8 @@ static int init_render_ring(struct intel_engine_cs *ring)
> struct drm_device *dev = ring->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> int ret = init_ring_common(ring);
> +   if (ret)
> +   return ret;

Yeah, on gen5+ this looks needed.
-Daniel

>
> /* WaTimedSingleVertexDispatch:cl,bw,ctg,elk,ilk,snb */
> if (INTEL_INFO(dev)->gen >= 4 && INTEL_INFO(dev)->gen < 7)
> --
> 1.8.1.2
>



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

[Bug 78321] Xorg Freeze with radeon.dpm=1

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

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

--- Comment #1 from perry3d at gmail.com ---
Kernel build from git doesn't help.

The freeze seems to be related to the hdmi audio output. If i blacklist the
snd_hda_intel and the snd_hda_codec_hdmi modules the system is running fine (at
least for my first try).

Unfortunately my onboard sound card is also blacklisted with these modules.

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

[REPOST PATCH 4/8] android: convert sync to fence api, v5

[Daniel Vetter]

On Wed, Jun 18, 2014 at 06:15:56PM -0700, Greg KH wrote:
> On Wed, Jun 18, 2014 at 12:37:11PM +0200, Maarten Lankhorst wrote:
> > Just to show it's easy.
> > 
> > Android syncpoints can be mapped to a timeline. This removes the need
> > to maintain a separate api for synchronization. I've left the android
> > trace events in place, but the core fence events should already be
> > sufficient for debugging.
> > 
> > v2:
> > - Call fence_remove_callback in sync_fence_free if not all fences have 
> > fired.
> > v3:
> > - Merge Colin Cross' bugfixes, and the android fence merge optimization.
> > v4:
> > - Merge with the upstream fixes.
> > v5:
> > - Fix small style issues pointed out by Thomas Hellstrom.
> > 
> > Signed-off-by: Maarten Lankhorst 
> > Acked-by: John Stultz 
> > ---
> >  drivers/staging/android/Kconfig  |1 
> >  drivers/staging/android/Makefile |2 
> >  drivers/staging/android/sw_sync.c|6 
> >  drivers/staging/android/sync.c   |  913 
> > +++---
> >  drivers/staging/android/sync.h   |   79 ++-
> >  drivers/staging/android/sync_debug.c |  247 +
> >  drivers/staging/android/trace/sync.h |   12 
> >  7 files changed, 609 insertions(+), 651 deletions(-)
> >  create mode 100644 drivers/staging/android/sync_debug.c
> 
> With these changes, can we pull the android sync logic out of
> drivers/staging/ now?

Afaik the google guys never really looked at this and acked it. So I'm not
sure whether they'll follow along. The other issue I have as the
maintainer of gfx driver is that I don't want to implement support for two
different sync object primitives (once for dma-buf and once for android
syncpts), and my impression thus far has been that even with this we're
not there.

I'm trying to get our own android guys to upstream their i915 syncpts
support, but thus far I haven't managed to convince them to throw people's
time at this.

It looks like a step into the right direction, but until I have the proof
in the form of i915 patches that I won't have to support 2 gfx fencing
frameworks I'm opposed to de-staging android syncpts. Ofc someone else
could do that too, but besides i915 I don't see a full-fledged (modeset
side only kinda doesn't count) upstream gfx driver shipping on android.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[Bug 78321] Xorg Freeze with radeon.dpm=1

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

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

--- Comment #2 from perry3d at gmail.com ---
Instead of blacklisting the kernel modules, adding radeon.audio=0 to the kernel
parameters also helps.

For me this is a workaround as i don't need the hdmi audio output.

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

[Bug 79980] Random radeonsi crashes

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

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

--- Comment #19 from agapito  ---
(In reply to comment #17)
> (In reply to comment #15)
> > This bug is caused by TAHITI_mc2.bin firmware. The old firmware works good.
> 
> Did you test a new kernel with the old firmware or an old kernel without the
> new firmware patch?  It could be some other change if you did the latter.

3.14 or 3.15 + New firmware = Crashes

3.14 or 3.15 + Old firmware = No problems!

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

[PATCH 1/3] drm: add register and unregister functions for connectors

[Daniel Vetter]

On Tue, Jun 10, 2014 at 06:21:35PM +0200, David Herrmann wrote:
> Hi
> 
> On Thu, May 29, 2014 at 5:57 PM, Thomas Wood  wrote:
> > Introduce generic functions to register and unregister connectors. This
> > provides a common place to add and remove associated user space
> > interfaces.
> >
> > Signed-off-by: Thomas Wood 
> > ---
> >  Documentation/DocBook/drm.tmpl|  6 +++---
> >  drivers/gpu/drm/armada/armada_output.c|  4 ++--
> >  drivers/gpu/drm/ast/ast_mode.c|  4 ++--
> >  drivers/gpu/drm/bridge/ptn3460.c  |  2 +-
> >  drivers/gpu/drm/drm_crtc.c| 30 
> > ++-
> >  drivers/gpu/drm/drm_sysfs.c   |  2 --
> >  drivers/gpu/drm/exynos/exynos_dp_core.c   |  2 +-
> >  drivers/gpu/drm/exynos/exynos_drm_connector.c |  6 +++---
> >  drivers/gpu/drm/exynos/exynos_drm_dpi.c   |  4 ++--
> >  drivers/gpu/drm/exynos/exynos_drm_dsi.c   |  2 +-
> >  drivers/gpu/drm/exynos/exynos_drm_vidi.c  |  2 +-
> >  drivers/gpu/drm/exynos/exynos_hdmi.c  |  2 +-
> >  drivers/gpu/drm/gma500/cdv_intel_crt.c|  4 ++--
> >  drivers/gpu/drm/gma500/cdv_intel_dp.c |  4 ++--
> >  drivers/gpu/drm/gma500/cdv_intel_hdmi.c   |  4 ++--
> >  drivers/gpu/drm/gma500/cdv_intel_lvds.c   |  4 ++--
> >  drivers/gpu/drm/gma500/mdfld_dsi_output.c |  4 ++--
> >  drivers/gpu/drm/gma500/oaktrail_hdmi.c|  2 +-
> >  drivers/gpu/drm/gma500/oaktrail_lvds.c|  2 +-
> >  drivers/gpu/drm/gma500/psb_intel_lvds.c   |  4 ++--
> >  drivers/gpu/drm/gma500/psb_intel_sdvo.c   |  4 ++--
> >  drivers/gpu/drm/i915/intel_crt.c  |  2 +-
> >  drivers/gpu/drm/i915/intel_display.c  |  2 +-
> >  drivers/gpu/drm/i915/intel_dp.c   |  4 ++--
> >  drivers/gpu/drm/i915/intel_dsi.c  |  2 +-
> >  drivers/gpu/drm/i915/intel_dvo.c  |  2 +-
> >  drivers/gpu/drm/i915/intel_hdmi.c |  2 +-
> >  drivers/gpu/drm/i915/intel_lvds.c |  2 +-
> >  drivers/gpu/drm/i915/intel_sdvo.c | 10 -
> >  drivers/gpu/drm/i915/intel_tv.c   |  2 +-
> >  drivers/gpu/drm/mgag200/mgag200_mode.c|  2 +-
> >  drivers/gpu/drm/msm/hdmi/hdmi_connector.c |  4 ++--
> >  drivers/gpu/drm/nouveau/nouveau_connector.c   |  4 ++--
> >  drivers/gpu/drm/omapdrm/omap_connector.c  |  4 ++--
> >  drivers/gpu/drm/qxl/qxl_display.c |  4 ++--
> >  drivers/gpu/drm/radeon/radeon_connectors.c|  6 +++---
> >  drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c |  4 ++--
> >  drivers/gpu/drm/rcar-du/rcar_du_vgacon.c  |  4 ++--
> >  drivers/gpu/drm/shmobile/shmob_drm_crtc.c |  6 +++---
> >  drivers/gpu/drm/tegra/output.c|  4 ++--
> >  drivers/gpu/drm/tilcdc/tilcdc_panel.c |  2 +-
> >  drivers/gpu/drm/tilcdc/tilcdc_slave.c |  2 +-
> >  drivers/gpu/drm/tilcdc/tilcdc_tfp410.c|  2 +-
> >  drivers/gpu/drm/udl/udl_connector.c   |  4 ++--
> >  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c   |  2 +-
> >  drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c   |  2 +-
> >  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c  |  2 +-
> >  drivers/staging/imx-drm/imx-drm-core.c|  6 +++---
> 
> You even caught imx.. and you removed the EXPORT_SYMBOL. So looks all
> good to me.
> I like that refactoring and I don't think we need an ACK from all
> driver authors. This is:
> 
> Reviewed-by: David Herrmann 
> 
> Maybe Daniel or Dave can pick this up?

I've pulled in all three patches into my topic/core-stuff branch and will
send a pull request to Dave once drm-next is open.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] drm/fb-helper: Return correct type to match drm_fb_helper_debug_enter() prototype

[Daniel Vetter]

On Thu, Jun 19, 2014 at 10:50:31AM +0800, Liu Ying wrote:
> The return type of drm_fb_helper_debug_enter() is int, so we should return '0'
> instead of 'false'.
> 
> Signed-off-by: Liu Ying 
> ---
>  drivers/gpu/drm/drm_fb_helper.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index d5d8cea..8daa4ad 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -200,7 +200,7 @@ int drm_fb_helper_debug_enter(struct fb_info *info)
>   int i;
>  
>   if (list_empty(&kernel_fb_helper_list))
> - return false;
> + return 0;

Actually we can remove the entire if check since list_for_each_entry is a
no-op on an empty list, and then we'll fall right through to the return 0;
at the end of the function. Care to respin your patch?
-Daniel

>  
>   list_for_each_entry(helper, &kernel_fb_helper_list, kernel_fb_list) {
>   for (i = 0; i < helper->crtc_count; i++) {
> -- 
> 1.7.9.5
> 
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[PATCH] drm/fb-helper: Redundant info->fix.type_aux setting in drm_fb_helper_fill_fix()

[Liu Ying]

The variable info->fix.type_aux is set to zero twice in the function
drm_fb_helper_fill_fix().  This patch removes one redundant.

Signed-off-by: Liu Ying 
---
 drivers/gpu/drm/drm_fb_helper.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 8daa4ad..142c39c 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1056,7 +1056,6 @@ void drm_fb_helper_fill_fix(struct fb_info *info, 
uint32_t pitch,
info->fix.ypanstep = 1; /* doing it in hw */
info->fix.ywrapstep = 0;
info->fix.accel = FB_ACCEL_NONE;
-   info->fix.type_aux = 0;

info->fix.line_length = pitch;
return;
-- 
1.7.9.5

[PATCH] drm/fb-helper: Redundant info->fix.type_aux setting in drm_fb_helper_fill_fix()

[Daniel Vetter]

On Thu, Jun 19, 2014 at 03:14:37PM +0800, Liu Ying wrote:
> The variable info->fix.type_aux is set to zero twice in the function
> drm_fb_helper_fill_fix().  This patch removes one redundant.
> 
> Signed-off-by: Liu Ying 

Merged into my topic/core-stuff branch, will show up in 3.17.
-Daniel

> ---
>  drivers/gpu/drm/drm_fb_helper.c |1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 8daa4ad..142c39c 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1056,7 +1056,6 @@ void drm_fb_helper_fill_fix(struct fb_info *info, 
> uint32_t pitch,
>   info->fix.ypanstep = 1; /* doing it in hw */
>   info->fix.ywrapstep = 0;
>   info->fix.accel = FB_ACCEL_NONE;
> - info->fix.type_aux = 0;
>  
>   info->fix.line_length = pitch;
>   return;
> -- 
> 1.7.9.5
> 
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[PATCH] drm/fb-helper: Return correct type to match drm_fb_helper_debug_enter() prototype

[Liu Ying]

On 06/19/2014 03:01 PM, Daniel Vetter wrote:
> On Thu, Jun 19, 2014 at 10:50:31AM +0800, Liu Ying wrote:
>> The return type of drm_fb_helper_debug_enter() is int, so we should return 
>> '0'
>> instead of 'false'.
>>
>> Signed-off-by: Liu Ying 
>> ---
>>   drivers/gpu/drm/drm_fb_helper.c |2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c 
>> b/drivers/gpu/drm/drm_fb_helper.c
>> index d5d8cea..8daa4ad 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -200,7 +200,7 @@ int drm_fb_helper_debug_enter(struct fb_info *info)
>>  int i;
>>
>>  if (list_empty(&kernel_fb_helper_list))
>> -return false;
>> +return 0;
>
> Actually we can remove the entire if check since list_for_each_entry is a
> no-op on an empty list, and then we'll fall right through to the return 0;
> at the end of the function. Care to respin your patch?
> -Daniel

Ok, I will respin my patch.  Thanks.

Liu Ying

>
>>
>>  list_for_each_entry(helper, &kernel_fb_helper_list, kernel_fb_list) {
>>  for (i = 0; i < helper->crtc_count; i++) {
>> --
>> 1.7.9.5
>>
>> ___
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>

[Bug 78221] 3.16 RC1: AMD R9 270 GPU locks up on some heavy 2D activity - GPU VM fault occurs. (possibly DMA copying issue strikes back?)

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

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

--- Comment #3 from t3st3r at mail.ru ---
Not looks like this. Re-checked again, using very same version of drivers in
process. I'm unable to trigged bug with 3.15 mainline kernel, no matter what.
But it happens easily with older kernels like 3.14 or early 3.15RCs, and would
also happen with 3.16-RC1. This makes me to think it comes to DMA issues. What
makes me even more suspicious about it is that early 3.15RC were crashing as
well, but in release version its gone. Looks like last-minute changes related
to DMA have fixed this instability. But now it re-appeared again :(.

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

[PATCH] drm/fb-helper: Remove unnecessary list empty check in drm_fb_helper_debug_enter()

[Liu Ying]

The following list empty check is unnecessary because we would still do nothing
real and return 'val' if my_list is empty.

if (list_empty(&my_list))
return val;

list_for_each_entry(pos, &my_list, member) {
...
}

return val;

This patch removes the unnecessary check in drm_fb_helper_debug_enter().

Signed-off-by: Liu Ying 
---
 drivers/gpu/drm/drm_fb_helper.c |3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index d5d8cea..eb77a2f 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -199,9 +199,6 @@ int drm_fb_helper_debug_enter(struct fb_info *info)
struct drm_crtc_helper_funcs *funcs;
int i;

-   if (list_empty(&kernel_fb_helper_list))
-   return false;
-
list_for_each_entry(helper, &kernel_fb_helper_list, kernel_fb_list) {
for (i = 0; i < helper->crtc_count; i++) {
struct drm_mode_set *mode_set =
-- 
1.7.9.5

[PATCH] drm/fb-helper: Remove unnecessary list empty check in drm_fb_helper_debug_enter()

[Daniel Vetter]

On Thu, Jun 19, 2014 at 04:01:44PM +0800, Liu Ying wrote:
> The following list empty check is unnecessary because we would still do 
> nothing
> real and return 'val' if my_list is empty.
> 
> if (list_empty(&my_list))
>   return val;
> 
> list_for_each_entry(pos, &my_list, member) {
>   ...
> }
> 
> return val;
> 
> This patch removes the unnecessary check in drm_fb_helper_debug_enter().
> 
> Signed-off-by: Liu Ying 

Merged into my core-stuff branch, will send a pull to Dave for 3.17.
-Daniel

> ---
>  drivers/gpu/drm/drm_fb_helper.c |3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index d5d8cea..eb77a2f 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -199,9 +199,6 @@ int drm_fb_helper_debug_enter(struct fb_info *info)
>   struct drm_crtc_helper_funcs *funcs;
>   int i;
>  
> - if (list_empty(&kernel_fb_helper_list))
> - return false;
> -
>   list_for_each_entry(helper, &kernel_fb_helper_list, kernel_fb_list) {
>   for (i = 0; i < helper->crtc_count; i++) {
>   struct drm_mode_set *mode_set =
> -- 
> 1.7.9.5
> 

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

[PATCH v3 RESEND] drm/i2c: tda998x: Fix lack of required reg in DT documentation

[Jean-Francois Moine]

The I2C address (reg) is required for the TDA998x driver to be loaded
and initialized.

Signed-off-by: Jean-Francois Moine 
---
- v3
- change subject to drm/i2c
- v2
- don't force the I2C address to be 0x70
This patch applies to linux-next.
---
 Documentation/devicetree/bindings/drm/i2c/tda998x.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt 
b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
index d7df01c..fc7effa 100644
--- a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
+++ b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
@@ -3,6 +3,8 @@ Device-Tree bindings for the NXP TDA998x HDMI transmitter
 Required properties;
   - compatible: must be "nxp,tda998x"

+  - reg: I2C address
+
 Optional properties:
   - interrupts: interrupt number and trigger type
default: polling
-- 
1.9.1

[PATCH] drm/radeon: fix race condition in radeon_crtc_page_flip v2

[Christian König]

From: Christian K?nig 

radeon_crtc_handle_flip can be called concurrently, and if
we set the unpin_work too early, it may try to flip an unpinned BO or
worse.

v2: fix compiler warning, update commit message,
set crtc->primary->fb only when everything went well

Signed-off-by: Christian K?nig 
Cc: stable at vger.kernel.org
---
 drivers/gpu/drm/radeon/radeon_display.c | 39 ++---
 1 file changed, 17 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_display.c 
b/drivers/gpu/drm/radeon/radeon_display.c
index 356b733..8aaa7ac 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -393,17 +393,6 @@ static int radeon_crtc_page_flip(struct drm_crtc *crtc,

INIT_WORK(&work->work, radeon_unpin_work_func);

-   /* We borrow the event spin lock for protecting unpin_work */
-   spin_lock_irqsave(&dev->event_lock, flags);
-   if (radeon_crtc->unpin_work) {
-   DRM_DEBUG_DRIVER("flip queue: crtc already busy\n");
-   r = -EBUSY;
-   goto unlock_free;
-   }
-   radeon_crtc->unpin_work = work;
-   radeon_crtc->deferred_flip_completion = 0;
-   spin_unlock_irqrestore(&dev->event_lock, flags);
-
/* pin the new buffer */
DRM_DEBUG_DRIVER("flip-ioctl() cur_fbo = %p, cur_bbo = %p\n",
 work->old_rbo, rbo);
@@ -461,13 +450,6 @@ static int radeon_crtc_page_flip(struct drm_crtc *crtc,
base &= ~7;
}

-   spin_lock_irqsave(&dev->event_lock, flags);
-   work->new_crtc_base = base;
-   spin_unlock_irqrestore(&dev->event_lock, flags);
-
-   /* update crtc fb */
-   crtc->primary->fb = fb;
-
r = drm_vblank_get(dev, radeon_crtc->crtc_id);
if (r) {
DRM_ERROR("failed to get vblank before flip\n");
@@ -477,6 +459,23 @@ static int radeon_crtc_page_flip(struct drm_crtc *crtc,
/* set the proper interrupt */
radeon_pre_page_flip(rdev, radeon_crtc->crtc_id);

+   /* We borrow the event spin lock for protecting unpin_work */
+   spin_lock_irqsave(&dev->event_lock, flags);
+   if (radeon_crtc->unpin_work) {
+   spin_unlock_irqrestore(&dev->event_lock, flags);
+   radeon_post_page_flip(rdev, radeon_crtc->crtc_id);
+   drm_vblank_put(dev, radeon_crtc->crtc_id);
+
+   DRM_DEBUG_DRIVER("flip queue: crtc already busy\n");
+   r = -EBUSY;
+   goto pflip_cleanup1;
+   }
+   radeon_crtc->unpin_work = work;
+   radeon_crtc->deferred_flip_completion = 0;
+   work->new_crtc_base = base;
+   crtc->primary->fb = fb;
+   spin_unlock_irqrestore(&dev->event_lock, flags);
+
return 0;

 pflip_cleanup1:
@@ -490,10 +489,6 @@ pflip_cleanup1:
radeon_bo_unreserve(rbo);

 pflip_cleanup:
-   spin_lock_irqsave(&dev->event_lock, flags);
-   radeon_crtc->unpin_work = NULL;
-unlock_free:
-   spin_unlock_irqrestore(&dev->event_lock, flags);
drm_gem_object_unreference_unlocked(old_radeon_fb->obj);
radeon_fence_unref(&work->fence);
kfree(work);
-- 
1.9.1

[PATCH] drm/radeon: fix race condition in radeon_crtc_page_flip

[Christian König]

Am 19.06.2014 05:49, schrieb Michel D?nzer:
> This patch only applies to 3.15, right?

Yes correct. For 3.16 we have the reworked flip which I think is still a 
good idea to keep for now.

I've addresses your comments and send out a v2 to the list CCing you, 
please review.

Thanks,
Christian.

>
>
> On 19.06.2014 02:11, Christian K?nig wrote:
>> From: Christian K?nig 
>>
>> radeon_crtc_handle_flip can be called concurrently and if
>> we set the unpin_work to early try to flip an unpinned BO or
>> worse.
> Spelling: 'too early'
>
> Maybe something like:
>
> radeon_crtc_handle_flip can be called concurrently, and if
> we set the unpin_work too early, it may try to flip an unpinned BO or
> worse.
>
>
>> Signed-off-by: Christian K?nig 
>> Cc: stable at vger.kernel.org
>> ---
>>   drivers/gpu/drm/radeon/radeon_display.c | 31 
>> ---
>>   1 file changed, 16 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon_display.c 
>> b/drivers/gpu/drm/radeon/radeon_display.c
>> index 356b733..cf22741 100644
>> --- a/drivers/gpu/drm/radeon/radeon_display.c
>> +++ b/drivers/gpu/drm/radeon/radeon_display.c
>> @@ -393,17 +393,6 @@ static int radeon_crtc_page_flip(struct drm_crtc *crtc,
>>   
>>  INIT_WORK(&work->work, radeon_unpin_work_func);
>>   
>> -/* We borrow the event spin lock for protecting unpin_work */
>> -spin_lock_irqsave(&dev->event_lock, flags);
>> -if (radeon_crtc->unpin_work) {
>> -DRM_DEBUG_DRIVER("flip queue: crtc already busy\n");
>> -r = -EBUSY;
>> -goto unlock_free;
>> -}
>> -radeon_crtc->unpin_work = work;
>> -radeon_crtc->deferred_flip_completion = 0;
>> -spin_unlock_irqrestore(&dev->event_lock, flags);
>> -
>>  /* pin the new buffer */
>>  DRM_DEBUG_DRIVER("flip-ioctl() cur_fbo = %p, cur_bbo = %p\n",
>>   work->old_rbo, rbo);
>> @@ -461,10 +450,6 @@ static int radeon_crtc_page_flip(struct drm_crtc *crtc,
>>  base &= ~7;
>>  }
>>   
>> -spin_lock_irqsave(&dev->event_lock, flags);
>> -work->new_crtc_base = base;
>> -spin_unlock_irqrestore(&dev->event_lock, flags);
>> -
>>  /* update crtc fb */
>>  crtc->primary->fb = fb;
>>   
>> @@ -477,6 +462,22 @@ static int radeon_crtc_page_flip(struct drm_crtc *crtc,
>>  /* set the proper interrupt */
>>  radeon_pre_page_flip(rdev, radeon_crtc->crtc_id);
>>   
>> +/* We borrow the event spin lock for protecting unpin_work */
>> +spin_lock_irqsave(&dev->event_lock, flags);
>> +if (radeon_crtc->unpin_work) {
>> +spin_unlock_irqrestore(&dev->event_lock, flags);
>> +radeon_post_page_flip(rdev, radeon_crtc->crtc_id);
>> +drm_vblank_put(dev, radeon_crtc->crtc_id);
>> +
>> +DRM_DEBUG_DRIVER("flip queue: crtc already busy\n");
>> +r = -EBUSY;
>> +goto pflip_cleanup1;
>> +}
>> +radeon_crtc->unpin_work = work;
>> +radeon_crtc->deferred_flip_completion = 0;
>> +work->new_crtc_base = base;
>> +spin_unlock_irqrestore(&dev->event_lock, flags);
>> +
> This introduces a path where crtc->primary->fb is updated, but then we
> return -EBUSY.
>
>
> It also introduces a warning:
>
> drivers/gpu/drm/radeon/radeon_display.c: In function ?radeon_crtc_page_flip?:
> drivers/gpu/drm/radeon/radeon_display.c:496:1: warning: label ?unlock_free? 
> defined but not used [-Wunused-label]
>   unlock_free:
>   ^
>
>
> Apart from that, looks good.
>
>

[PATCH 1/3] drm/radeon: stop poisoning the GART TLB

[Christian König]

Am 19.06.2014 03:48, schrieb Michel D?nzer:
> On 15.06.2014 21:48, Christian K?nig wrote:
>> Am 13.06.2014 23:31, schrieb Alex Deucher:
>>> On Fri, Jun 13, 2014 at 11:45 AM, Christian K?nig
>>>  wrote:
>>>> Hi Marek,
>>>>
>>>> ah, yes! Piglit in combination with that patch can indeed crash the box.
>>>>
>>>> Going to investigate now that I can reproduce it.
>>> I wonder if it's a clockgating issue with the MC or BIF?  You might
>>> try adjusting the rdev->cg_flags (try setting it to 0) in
>>> radeon_asic.c or disabling dpm.
>> Unfortunately that was just a false alarm.
>>
>> I was just on a branch which didn't had the "stop poisoning the GART
>> TLB" patch, after applying this patch I can again let piglit run for the
>> whole night without a lockup.
>>
>> No idea what goes wrong when Marek runs piglit, but 3.15.0+"stop
>> poisoning the GART TLB"+"force_gtt" is rock solid here.
> FWIW, 3.15 doesn't survive piglit on my Bonaire either, but 3.14 is
> fine. 3.15 seems stable on Kaveri though, but I haven't tried the
> force_gtt patch on that yet.

Yeah, I think it's just me who has a stable system with 3.15 and that 
annoys me quite a bit.

No idea what's the difference. What versions of LLVM/Mesa/Piglit are you 
using for the test?

>
> There have also been a number of bug reports about stability regressions
> in 3.15 on various SI and CIK cards. It seems likely that at least some
> of those are related to this issue as well.
>
> If we can't figure out the problem soon, we probably need to revert the
> 'Use normal BOs for page tables' and dependent changes at least for 3.15.y?

I thought about this for the whole 3.15 release cycle, but decided 
against it. But what we could do is applying the attached trivial patch, 
it pins down the page tables and so pretty much reverts to the old behavior.

I think even when we revert to the old code we have a couple of unsolved 
problems with the VM support or in the driver in general where we should 
try to understand the underlying reason for it instead of applying more 
workarounds.

Going to try harder crashing my 3.15 system,
Christian.
-- next part --
A non-text attachment was scrubbed...
Name: 0001-drm-radeon-pin-down-page-tables.patch
Type: text/x-diff
Size: 1008 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140619/b0b7a8df/attachment.patch>

[PATCH 1/3] drm/radeon: stop poisoning the GART TLB

[Marek Olšák]

Hi Michel,

3.15 doesn't contain Christian's fix yet, so it should be always
broken for everybody. The fix is currently only in 3.16.

Alternatively, you can cherry-pick the fix to 3.15, but it doesn't
apply cleanly.

There is a workaround in 3.15 which disables sDMA and uses CP DMA for
copying buffers. It seems to help Christian's machine, but not mine.

When I said the kernel driver was broken, I meant that
it was broken *with* the fix applied regardless of which engine was
used for the copying.

Marek

On Thu, Jun 19, 2014 at 3:48 AM, Michel D?nzer  wrote:
> On 15.06.2014 21:48, Christian K?nig wrote:
>> Am 13.06.2014 23:31, schrieb Alex Deucher:
>>> On Fri, Jun 13, 2014 at 11:45 AM, Christian K?nig
>>>  wrote:
 Hi Marek,

 ah, yes! Piglit in combination with that patch can indeed crash the box.

 Going to investigate now that I can reproduce it.
>>> I wonder if it's a clockgating issue with the MC or BIF?  You might
>>> try adjusting the rdev->cg_flags (try setting it to 0) in
>>> radeon_asic.c or disabling dpm.
>>
>> Unfortunately that was just a false alarm.
>>
>> I was just on a branch which didn't had the "stop poisoning the GART
>> TLB" patch, after applying this patch I can again let piglit run for the
>> whole night without a lockup.
>>
>> No idea what goes wrong when Marek runs piglit, but 3.15.0+"stop
>> poisoning the GART TLB"+"force_gtt" is rock solid here.
>
> FWIW, 3.15 doesn't survive piglit on my Bonaire either, but 3.14 is
> fine. 3.15 seems stable on Kaveri though, but I haven't tried the
> force_gtt patch on that yet.
>
> There have also been a number of bug reports about stability regressions
> in 3.15 on various SI and CIK cards. It seems likely that at least some
> of those are related to this issue as well.
>
> If we can't figure out the problem soon, we probably need to revert the
> 'Use normal BOs for page tables' and dependent changes at least for 3.15.y?
>
>
> --
> Earthling Michel D?nzer|  http://www.amd.com
> Libre software enthusiast  |Mesa and X developer
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/3] drm/radeon: stop poisoning the GART TLB

[Christian König]

Hi Marek,

> There is a workaround in 3.15 which disables sDMA and uses CP DMA for
> copying buffers. It seems to help Christian's machine, but not mine.
With stressing the box with piglit I was able to bring my machine down 
with the CP DMA as well, only cherry-picking the "stop poisoning the 
GART TLB" really fixed that issue.

But I'm pretty sure that even with "stop poisoning the GART TLB" 
back-ported we still have at least one stability issue I can't reproduce.

Christian.

Am 19.06.2014 12:20, schrieb Marek Ol??k:
> Hi Michel,
>
> 3.15 doesn't contain Christian's fix yet, so it should be always
> broken for everybody. The fix is currently only in 3.16.
>
> Alternatively, you can cherry-pick the fix to 3.15, but it doesn't
> apply cleanly.
>
> There is a workaround in 3.15 which disables sDMA and uses CP DMA for
> copying buffers. It seems to help Christian's machine, but not mine.
>
> When I said the kernel driver was broken, I meant that
> it was broken *with* the fix applied regardless of which engine was
> used for the copying.
>
> Marek
>
> On Thu, Jun 19, 2014 at 3:48 AM, Michel D?nzer  wrote:
>> On 15.06.2014 21:48, Christian K?nig wrote:
>>> Am 13.06.2014 23:31, schrieb Alex Deucher:
 On Fri, Jun 13, 2014 at 11:45 AM, Christian K?nig
  wrote:
> Hi Marek,
>
> ah, yes! Piglit in combination with that patch can indeed crash the box.
>
> Going to investigate now that I can reproduce it.
 I wonder if it's a clockgating issue with the MC or BIF?  You might
 try adjusting the rdev->cg_flags (try setting it to 0) in
 radeon_asic.c or disabling dpm.
>>> Unfortunately that was just a false alarm.
>>>
>>> I was just on a branch which didn't had the "stop poisoning the GART
>>> TLB" patch, after applying this patch I can again let piglit run for the
>>> whole night without a lockup.
>>>
>>> No idea what goes wrong when Marek runs piglit, but 3.15.0+"stop
>>> poisoning the GART TLB"+"force_gtt" is rock solid here.
>> FWIW, 3.15 doesn't survive piglit on my Bonaire either, but 3.14 is
>> fine. 3.15 seems stable on Kaveri though, but I haven't tried the
>> force_gtt patch on that yet.
>>
>> There have also been a number of bug reports about stability regressions
>> in 3.15 on various SI and CIK cards. It seems likely that at least some
>> of those are related to this issue as well.
>>
>> If we can't figure out the problem soon, we probably need to revert the
>> 'Use normal BOs for page tables' and dependent changes at least for 3.15.y?
>>
>>
>> --
>> Earthling Michel D?nzer|  http://www.amd.com
>> Libre software enthusiast  |Mesa and X developer
>> ___
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH libdrm 1/2] intel: Sync the command parser version parameter from kernel

[Damien Lespiau]

Cc: Bradley Volkin 
Signed-off-by: Damien Lespiau 
---
 include/drm/i915_drm.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index 2f4eb8c..980dbf8 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -337,6 +337,7 @@ typedef struct drm_i915_irq_wait {
 #define I915_PARAM_HAS_EXEC_NO_RELOC25
 #define I915_PARAM_HAS_EXEC_HANDLE_LUT   26
 #define I915_PARAM_HAS_WT   27
+#define I915_PARAM_CMD_PARSER_VERSION   28

 typedef struct drm_i915_getparam {
int param;
-- 
1.8.3.1

[PATCH libdrm 2/2] intel: Sync typo fix from the kernel sources.

[Damien Lespiau]

Signed-off-by: Damien Lespiau 
---
 include/drm/i915_drm.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index 980dbf8..8eb0cb3 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -722,7 +722,7 @@ struct drm_i915_gem_execbuffer2 {
  */
 #define I915_EXEC_IS_PINNED(1<<10)

-/** Provide a hint to the kernel that the command stream and auxilliary
+/** Provide a hint to the kernel that the command stream and auxiliary
  * state buffers already holds the correct presumed addresses and so the
  * relocation process may be skipped if no buffers need to be moved in
  * preparation for the execbuffer.
-- 
1.8.3.1

[Bug 75061] System boot failure very often

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

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

--- Comment #1 from Yuking  ---
Created attachment 140391
  --> https://bugzilla.kernel.org/attachment.cgi?id=140391&action=edit
X crashed caused by kernel crash.

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

[REPOST PATCH 4/8] android: convert sync to fence api, v5

[Thierry Reding]

On Thu, Jun 19, 2014 at 08:37:27AM +0200, Daniel Vetter wrote:
> On Wed, Jun 18, 2014 at 06:15:56PM -0700, Greg KH wrote:
> > On Wed, Jun 18, 2014 at 12:37:11PM +0200, Maarten Lankhorst wrote:
> > > Just to show it's easy.
> > > 
> > > Android syncpoints can be mapped to a timeline. This removes the need
> > > to maintain a separate api for synchronization. I've left the android
> > > trace events in place, but the core fence events should already be
> > > sufficient for debugging.
> > > 
> > > v2:
> > > - Call fence_remove_callback in sync_fence_free if not all fences have 
> > > fired.
> > > v3:
> > > - Merge Colin Cross' bugfixes, and the android fence merge optimization.
> > > v4:
> > > - Merge with the upstream fixes.
> > > v5:
> > > - Fix small style issues pointed out by Thomas Hellstrom.
> > > 
> > > Signed-off-by: Maarten Lankhorst 
> > > Acked-by: John Stultz 
> > > ---
> > >  drivers/staging/android/Kconfig  |1 
> > >  drivers/staging/android/Makefile |2 
> > >  drivers/staging/android/sw_sync.c|6 
> > >  drivers/staging/android/sync.c   |  913 
> > > +++---
> > >  drivers/staging/android/sync.h   |   79 ++-
> > >  drivers/staging/android/sync_debug.c |  247 +
> > >  drivers/staging/android/trace/sync.h |   12 
> > >  7 files changed, 609 insertions(+), 651 deletions(-)
> > >  create mode 100644 drivers/staging/android/sync_debug.c
> > 
> > With these changes, can we pull the android sync logic out of
> > drivers/staging/ now?
> 
> Afaik the google guys never really looked at this and acked it. So I'm not
> sure whether they'll follow along. The other issue I have as the
> maintainer of gfx driver is that I don't want to implement support for two
> different sync object primitives (once for dma-buf and once for android
> syncpts), and my impression thus far has been that even with this we're
> not there.
> 
> I'm trying to get our own android guys to upstream their i915 syncpts
> support, but thus far I haven't managed to convince them to throw people's
> time at this.

This has been discussed a fair bit internally recently and some of our
GPU experts have raised concerns that this may result in seriously
degraded performance in our proprietary graphics stack. Now I don't care
very much for the proprietary graphics stack, but by extension I would
assume that the same restrictions are relevant for any open-source
driver as well.

I'm still trying to fully understand all the implications and at the
same time get some of the people who raised concerns to join in this
discussion. As I understand it the concern is mostly about explicit vs.
implicit synchronization and having this mechanism in the kernel will
implicitly synchronize all accesses to these buffers even in cases where
it's not needed (read vs. write locks, etc.). In one particular instance
it was even mentioned that this kind of implicit synchronization can
lead to deadlocks in some use-cases (this was mentioned for Android
compositing, but I suspect that the same may happen for Wayland or X
compositors).

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140619/651c1fff/attachment.sig>

[REPOST PATCH 4/8] android: convert sync to fence api, v5

[Daniel Vetter]

On Thu, Jun 19, 2014 at 1:48 PM, Thierry Reding
 wrote:
>> > With these changes, can we pull the android sync logic out of
>> > drivers/staging/ now?
>>
>> Afaik the google guys never really looked at this and acked it. So I'm not
>> sure whether they'll follow along. The other issue I have as the
>> maintainer of gfx driver is that I don't want to implement support for two
>> different sync object primitives (once for dma-buf and once for android
>> syncpts), and my impression thus far has been that even with this we're
>> not there.
>>
>> I'm trying to get our own android guys to upstream their i915 syncpts
>> support, but thus far I haven't managed to convince them to throw people's
>> time at this.
>
> This has been discussed a fair bit internally recently and some of our
> GPU experts have raised concerns that this may result in seriously
> degraded performance in our proprietary graphics stack. Now I don't care
> very much for the proprietary graphics stack, but by extension I would
> assume that the same restrictions are relevant for any open-source
> driver as well.
>
> I'm still trying to fully understand all the implications and at the
> same time get some of the people who raised concerns to join in this
> discussion. As I understand it the concern is mostly about explicit vs.
> implicit synchronization and having this mechanism in the kernel will
> implicitly synchronize all accesses to these buffers even in cases where
> it's not needed (read vs. write locks, etc.). In one particular instance
> it was even mentioned that this kind of implicit synchronization can
> lead to deadlocks in some use-cases (this was mentioned for Android
> compositing, but I suspect that the same may happen for Wayland or X
> compositors).

Well the implicit fences here actually can't deadlock. That's the
entire point behind using ww mutexes. I've also heard tons of
complaints about implicit enforced syncing (especially from opencl
people), but in the end drivers and always expose unsynchronized
access for specific cases. We do that in i915 for upload buffers and
other fun stuff. This is about shared stuff across different drivers
and different processes.

I also expect that i915 will loose implicit syncing in a few upcoming
hw modes because explicit syncing is a more natural fit there.

All that isn't about the discussion at hand imo since no matter what
i915 needs to have on internal representation for a bit of gpu work,
and afaics right now we don't have that. With this patch android
syncpts use Maarten's fences internally, but I can't freely exchange
one for the other. So in i915 I still expect to get stuck with both of
them, which is one too many.

The other issue (and I haven't dug into details that much) I have with
syncpts are some of the interface choices. Apparently you can commit a
fence after creation (or at least the hw composer interface works like
that) which means userspace can construct deadlocks with android
syncpts if I'm not super careful in my driver. I haven't seen any
generic code to do that, so I presume everyone just blindly trusts
surface-flinger to not do that. Speaks of the average quality of an
android gfx driver if the kernel is less trusted than the compositor
in userspace ...

There's a few other things like exposing timestamps (which are tricky
to do right, our driver is littered with wrong attempts) and other
details.

Finally I've never seen anyone from google or any android product guy
push a real driver enabling for syncpts to upstream, and google itself
has a bit a history of constantly exchanging their gfx framework for
the next best thing. So I really doubt this is worthwhile to pursue in
upstream with our essentially eternal api guarantees. At least until
we see serious uptake from vendors and gfx driver guys. Unfortunately
the Intel android folks are no exception here and haven't pushed
anything like this in my direction yet at all. Despite multiple pokes
from my side.

So from my side I think we should move ahead with Maarten's work and
figure the android side out once there's real interest.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[Bug 80233] DirectX wine crush

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

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

Chris Wilson  changed:

   What|Removed |Added

   Assignee|chris at chris-wilson.co.uk|dri-devel at 
lists.freedesktop
   ||.org
 QA Contact|intel-gfx-bugs at lists.freede |
   |sktop.org   |
Product|DRI |Mesa
  Component|DRM/Intel   |Drivers/DRI/i830

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

[Bug 78321] Xorg Freeze with radeon.dpm=1

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

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

Alex Deucher  changed:

   What|Removed |Added

 CC||alexdeucher at gmail.com

--- Comment #3 from Alex Deucher  ---
Duplicate of bug 68571.

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

[Bug 80235] New: Artifacts in kernel 3.16-rc1 and HD 7750

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

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

  Priority: medium
Bug ID: 80235
  Assignee: dri-devel at lists.freedesktop.org
   Summary: Artifacts in kernel 3.16-rc1 and HD 7750
  Severity: major
Classification: Unclassified
OS: Linux (All)
  Reporter: rgfernandes at gmail.com
  Hardware: x86-64 (AMD64)
Status: NEW
   Version: XOrg CVS
 Component: DRM/Radeon
   Product: DRI

Created attachment 101360
  --> https://bugs.freedesktop.org/attachment.cgi?id=101360&action=edit
Image showing the artifact

I have updated the kernel to 3.16-rc1 and got some screen artifacts.
I have bisected the problem and found that the commit
1aab5514ca9604e0263f658a067da0189c86a35b (drm/radeon: rework page flip handling
v3) is causing these artifacts.
I am attaching some images to show these artifacts.
There is also a artifact in steam logo inside Portal 2 (the background texture
seems to disapear) that does not happen in commit just before
(1a0e79184132c5dc0e03a4047eacecc52c24deae).
I got 2 freezes too. The computer does not respond anymore, I can't switch to
console and after some seconds the computer reboots.
But I not sure if this problem is in this commit
(1aab5514ca9604e0263f658a067da0189c86a35b).

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

[Bug 80235] Artifacts in kernel 3.16-rc1 and HD 7750

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

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

--- Comment #1 from Raul Fernandes  ---
Created attachment 101361
  --> https://bugs.freedesktop.org/attachment.cgi?id=101361&action=edit
Image showing another artifact

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

[Bug 80235] Artifacts in kernel 3.16-rc1 and HD 7750

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

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

--- Comment #2 from Raul Fernandes  ---
Created attachment 101362
  --> https://bugs.freedesktop.org/attachment.cgi?id=101362&action=edit
Image showing artifact

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

[Bug 80235] Artifacts in kernel 3.16-rc1 and HD 7750

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

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

--- Comment #3 from Raul Fernandes  ---
Created attachment 101363
  --> https://bugs.freedesktop.org/attachment.cgi?id=101363&action=edit
Artifact in steam logo

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

[Bug 80235] Artifacts in kernel 3.16-rc1 and HD 7750

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

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

--- Comment #4 from Raul Fernandes  ---
Created attachment 101364
  --> https://bugs.freedesktop.org/attachment.cgi?id=101364&action=edit
Image with no artifact in steam logo to serve as baseline

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

[Bug 80235] Artifacts in kernel 3.16-rc1 and HD 7750

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

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

Raul Fernandes  changed:

   What|Removed |Added

 Attachment #101363|text/plain  |image/png
  mime type||

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

[Bug 80235] Artifacts in kernel 3.16-rc1 and HD 7750

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

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

Raul Fernandes  changed:

   What|Removed |Added

 Attachment #101360|text/plain  |image/png
  mime type||

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

[PATCH/RESEND 0/9] drm: tilcdc driver fixes

[Darren Etheridge]

Guido,

Thanks and sorry I missed this the first time around.  I'll give it a 
try on a few of my AM335x based boards when I have access to them again 
on Monday.

Darren

On 06/17/2014 09:17 AM, Guido Mart?nez wrote:
> The tilcdc driver could be compiled as a module, but was severely broken
> and could not be used as such. This patchset attempts to fix the issues
> preventing a proper load/unload of the module.
>
> Issues included dangling sysfs nodes, dangling devices, memory leaks and
> a double kfree.
>
> It now seems to be working ok. We have tested this by loading and
> unloading the driver repeteadly, with both panel and slave connectors
> and found no flaws.
>
> There is still one warning left on tilcdc_crtc_destroy, caused by
> destroying the connector while still in an ON status. We don't know why
> this happens or why it's an issue, so we did not fix it.
>
> The first 7 patches are bug fixes which and should probably be applied
> in the stable trees. The last two are clean-ups.
>
>
> Resending this since I got no replies.
>
>
> Guido Mart?nez (9):
>drm/i2c: tda998x: move drm_i2c_encoder_destroy call
>drm/tilcdc: panel: fix dangling sysfs connector node
>drm/tilcdc: slave: fix dangling sysfs connector node
>drm/tilcdc: tfp410: fix dangling sysfs connector node
>drm/tilcdc: panel: fix leak when unloading the module
>drm/tilcdc: fix release order on exit
>drm/tilcdc: fix double kfree
>drm/tilcdc: remove submodule destroy calls
>drm/tilcdc: replace late_initcall with module_init
>
>   drivers/gpu/drm/i2c/tda998x_drv.c  |  2 +-
>   drivers/gpu/drm/tilcdc/Module.symvers  |  0
>   drivers/gpu/drm/tilcdc/tilcdc_drv.c| 15 +
>   drivers/gpu/drm/tilcdc/tilcdc_drv.h|  1 -
>   drivers/gpu/drm/tilcdc/tilcdc_panel.c  | 39 
> +-
>   drivers/gpu/drm/tilcdc/tilcdc_slave.c  | 27 +--
>   drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 35 +++---
>   7 files changed, 59 insertions(+), 60 deletions(-)
>   create mode 100644 drivers/gpu/drm/tilcdc/Module.symvers
>

[Bug 78221] 3.16 RC1: AMD R9 270 GPU locks up on some heavy 2D activity - GPU VM fault occurs. (possibly DMA copying issue strikes back?)

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

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

--- Comment #4 from Alex Deucher  ---
Can you bisect and see what fixed it in 3.15 or what broke it again in 3.16?

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

[REPOST PATCH 1/8] fence: dma-buf cross-device synchronization (v17)

[Rob Clark]

On Wed, Jun 18, 2014 at 9:13 PM, Greg KH  wrote:
> On Wed, Jun 18, 2014 at 12:36:54PM +0200, Maarten Lankhorst wrote:
>> +#define CREATE_TRACE_POINTS
>> +#include 
>> +
>> +EXPORT_TRACEPOINT_SYMBOL(fence_annotate_wait_on);
>> +EXPORT_TRACEPOINT_SYMBOL(fence_emit);
>
> Are you really willing to live with these as tracepoints for forever?
> What is the use of them in debugging?  Was it just for debugging the
> fence code, or for something else?
>
>> +/**
>> + * fence_context_alloc - allocate an array of fence contexts
>> + * @num: [in]amount of contexts to allocate
>> + *
>> + * This function will return the first index of the number of fences 
>> allocated.
>> + * The fence context is used for setting fence->context to a unique number.
>> + */
>> +unsigned fence_context_alloc(unsigned num)
>> +{
>> + BUG_ON(!num);
>> + return atomic_add_return(num, &fence_context_counter) - num;
>> +}
>> +EXPORT_SYMBOL(fence_context_alloc);
>
> EXPORT_SYMBOL_GPL()?  Same goes for all of the exports in here.
> Traditionally all of the driver core exports have been with this
> marking, any objection to making that change here as well?

tbh, I prefer EXPORT_SYMBOL()..  well, I'd prefer even more if there
wasn't even a need for EXPORT_SYMBOL_GPL(), but sadly it is a fact of
life.  We already went through this debate once with dma-buf.  We
aren't going to change $evil_vendor's mind about non-gpl modules.  The
only result will be a more flugly convoluted solution (ie. use syncpt
EXPORT_SYMBOL() on top of fence EXPORT_SYMBOL_GPL()) just as a
workaround, with the result that no-one benefits.

>> +int __fence_signal(struct fence *fence)
>> +{
>> + struct fence_cb *cur, *tmp;
>> + int ret = 0;
>> +
>> + if (WARN_ON(!fence))
>> + return -EINVAL;
>> +
>> + if (!ktime_to_ns(fence->timestamp)) {
>> + fence->timestamp = ktime_get();
>> + smp_mb__before_atomic();
>> + }
>> +
>> + if (test_and_set_bit(FENCE_FLAG_SIGNALED_BIT, &fence->flags)) {
>> + ret = -EINVAL;
>> +
>> + /*
>> +  * we might have raced with the unlocked fence_signal,
>> +  * still run through all callbacks
>> +  */
>> + } else
>> + trace_fence_signaled(fence);
>> +
>> + list_for_each_entry_safe(cur, tmp, &fence->cb_list, node) {
>> + list_del_init(&cur->node);
>> + cur->func(fence, cur);
>> + }
>> + return ret;
>> +}
>> +EXPORT_SYMBOL(__fence_signal);
>
> Don't export a function with __ in front of it, you are saying that an
> internal function is somehow "valid" for everyone else to call?  Why?
> You aren't even documenting the function, so who knows how to use it?

fwiw, the __ versions appear to mainly be concessions for android
syncpt.  That is the only user outside of fence.c, and it should stay
that way.

>> +/**
>> + * fence_signal - signal completion of a fence
>> + * @fence: the fence to signal
>> + *
>> + * Signal completion for software callbacks on a fence, this will unblock
>> + * fence_wait() calls and run all the callbacks added with
>> + * fence_add_callback(). Can be called multiple times, but since a fence
>> + * can only go from unsignaled to signaled state, it will only be effective
>> + * the first time.
>> + */
>> +int fence_signal(struct fence *fence)
>> +{
>> + unsigned long flags;
>> +
>> + if (!fence)
>> + return -EINVAL;
>> +
>> + if (!ktime_to_ns(fence->timestamp)) {
>> + fence->timestamp = ktime_get();
>> + smp_mb__before_atomic();
>> + }
>> +
>> + if (test_and_set_bit(FENCE_FLAG_SIGNALED_BIT, &fence->flags))
>> + return -EINVAL;
>> +
>> + trace_fence_signaled(fence);
>> +
>> + if (test_bit(FENCE_FLAG_ENABLE_SIGNAL_BIT, &fence->flags)) {
>> + struct fence_cb *cur, *tmp;
>> +
>> + spin_lock_irqsave(fence->lock, flags);
>> + list_for_each_entry_safe(cur, tmp, &fence->cb_list, node) {
>> + list_del_init(&cur->node);
>> + cur->func(fence, cur);
>> + }
>> + spin_unlock_irqrestore(fence->lock, flags);
>> + }
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(fence_signal);
>
> So, why should I use this over __fence_signal?  What is the difference?
> Am I missing some documentation somewhere?
>
>> +void release_fence(struct kref *kref)
>> +{
>> + struct fence *fence =
>> + container_of(kref, struct fence, refcount);
>> +
>> + trace_fence_destroy(fence);
>> +
>> + BUG_ON(!list_empty(&fence->cb_list));
>> +
>> + if (fence->ops->release)
>> + fence->ops->release(fence);
>> + else
>> + kfree(fence);
>> +}
>> +EXPORT_SYMBOL(release_fence);
>
> fence_release() makes it more unified with the other functions in this
> file, right?
>
>> +/**
>> + * fence_default_wait - default sleep until the fence gets signaled
>> + * or until timeout elapses

[Bug 68571] GPU lockup on AMD Radeon HD6850 with DPM=1

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

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

--- Comment #41 from perry3d at gmail.com ---
Hello kilobug,

Alex marked my bug (https://bugzilla.kernel.org/show_bug.cgi?id=78321) as a
duplicate. And my solution is to disable the hdmi audio output. 
You can do this by appending radeon.audio=0 to the kernel parameters or just
blacklist the snd_hda_intel and the snd_hda_codec_hdmi modules.

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

[PATCH] drivers/i915: Fix unnoticed failure of init_ring_common()

[Daniel Vetter]

Hm, that's a bit unexpected. You are on a gen4 device, which means
we'll return the right error in the same function after a few more
register writes. But those are harmless.

On gen5+ we do more (call the pipe_control setup code), which could
potentially clobber the error code. So your patch looks correct, but
it definitely won't fix anything for your machine. You can respin the
patch with an improved commit message if you want.

The actual bug we seem to have is blowing up on the ggtt_unpin in
context_fini. Which is doubly-impossible: Gen4 doesn't have hw
contexts, so should have dctx->obj == NULL. And ring init failures
fail earlier so shouldn't even hit the context_fini code below the
cleanup_gem: label in driver_load. Seriously confused here.
-Daniel


On Thu, Jun 19, 2014 at 2:45 PM, Konrad Zapalowicz
 wrote:
> On 06/19, Daniel Vetter wrote:
>> On Thu, Jun 19, 2014 at 12:38 AM, Konrad Zapalowicz
>>  wrote:
>> > This commit add check for return value of init_ring_common() in the
>> > init_render_ring(). Now, when failure is detected the error code is
>> > propagated to the caller layer instead of being ignored.
>> >
>> > I believe that this fix will have a positive impact on the oops that
>> > I hit recently and which starts when init_ring_common() fails:
>> >
>> > [drm:init_ring_common] *ERROR* render ring initialization failed
>> >  ctl 0001f001 head 000c tail  start 003eb000
>> > BUG: unable to handle kernel NULL pointer dereference at 006c
>> > IP: [] i915_gem_obj_to_ggtt+0x9/0x40 [i915]
>> >
>> > Signed-off-by: Konrad Zapalowicz 
>>
>> Do you have the full Oops somewhere?
>
> Here you go, the Oops plus some usefull data:
>   1. Oops
>   2. lspci -vv
>   3. uname -a
>   4. Oops analysis
>
> 1. The Oops:
>
> Jun 17 21:06:11 t400 kernel: [   12.136049] [drm:init_ring_common] *ERROR* 
> render ring initialization failed ctl 0001f001 head 000c tail  
> start 003eb000
> Jun 17 21:06:11 t400 kernel: [   12.136081] BUG: unable to handle kernel NULL 
> pointer dereference at 006c
> Jun 17 21:06:11 t400 kernel: [   12.136086] IP: [] 
> i915_gem_obj_to_ggtt+0x9/0x40 [i915]
> Jun 17 21:06:11 t400 kernel: [   12.136118] *pdpt = 33158001 *pde = 
> 
> Jun 17 21:06:11 t400 kernel: [   12.136123] Oops:  [#1] SMP
> Jun 17 21:06:11 t400 kernel: [   12.136127] Modules linked in: mac80211(E) 
> i915(E+) snd_hda_codec_conexant(E) snd_hda_codec_generic(E) snd_hda_intel(E) 
> snd_hda_controller(E) intel_gtt(E) snd_hda_codec(E) iwlwifi(E) 
> i2c_algo_bit(E) snd_hwdep(E) uvcvideo(E) thinkpad_acpi(E) drm_kms_helper(E) 
> snd_pcm(E) pcmcia(E) nvram(E) videobuf2_vmalloc(E) videobuf2_memops(E) 
> videobuf2_core(E) drm(E) psmouse(E) videodev(E) snd_seq_midi(E) 
> snd_seq_midi_event(E) snd_rawmidi(E) snd_seq(E) cfg80211(E) yenta_socket(E) 
> snd_timer(E) pcmcia_rsrc(E) serio_raw(E) snd_seq_device(E) pcmcia_core(E) 
> snd(E) pl2303(E) lpc_ich(E) ppdev(E) usb_storage(E) soundcore(E) usbserial(E) 
> wmi(E) video(E) tpm_tis(E) parport_pc(E) lp(E) parport(E) firewire_ohci(E) 
> firewire_core(E) crc_itu_t(E) ahci(E) libahci(E) e1000e(E) ptp(E) pps_core(E)
> Jun 17 21:06:11 t400 kernel: [   12.136187] CPU: 1 PID: 570 Comm: modprobe 
> Tainted: GE 3.15.0 #6
> Jun 17 21:06:11 t400 kernel: [   12.136191] Hardware name: LENOVO 
> 6475FA4/6475FA4, BIOS 7UET79WW (3.09 ) 10/13/2009
> Jun 17 21:06:11 t400 kernel: [   12.136195] task: f3141b60 ti: f316a000 
> task.ti: f316a000
> Jun 17 21:06:11 t400 kernel: [   12.136199] EIP: 0060:[] EFLAGS: 
> 00010282 CPU: 1
> Jun 17 21:06:11 t400 kernel: [   12.136223] EIP is at 
> i915_gem_obj_to_ggtt+0x9/0x40 [i915]
> Jun 17 21:06:11 t400 kernel: [   12.136227] EAX:  EBX: 0004 ECX: 
> f2e6c000 EDX: fffb
> Jun 17 21:06:11 t400 kernel: [   12.136230] ESI: f2e6d174 EDI:  EBP: 
> f316bb50 ESP: f316bb4c
> Jun 17 21:06:11 t400 kernel: [   12.136234]  DS: 007b ES: 007b FS: 00d8 GS: 
> 0033 SS: 0068
> Jun 17 21:06:11 t400 kernel: [   12.136239] CR0: 8005003b CR2: 006c CR3: 
> 33157000 CR4: 000407f0
> Jun 17 21:06:11 t400 kernel: [   12.136243] Stack:
> Jun 17 21:06:11 t400 kernel: [   12.136245]  0004 f316bb68 f8ca16c5 
> f316bb80 0004 f2e6d174 f2e794c0 f316bb80
> Jun 17 21:06:11 t400 kernel: [   12.136254]  f8c9473e f2e6c000 f2e6c000 
> f32e5800 fffb f316bba0 f8c9ea49 
> Jun 17 21:06:11 t400 kernel: [   12.136263]   f32e5838 f32e5800 
>  f2e6c000 f316bca4 f8cf63a4 f8cf3c70
> Jun 17 21:06:11 t400 kernel: [   12.136271] Call Trace:
> Jun 17 21:06:11 t400 kernel: [   12.136297]  [] 
> i915_gem_object_ggtt_unpin+0x15/0x90 [i915]
> Jun 17 21:06:11 t400 kernel: [   12.136323]  [] 
> i915_gem_context_fini+0x7e/0x130 [i915]
> Jun 17 21:06:11 t400 kernel: [   12.136349]  [] 
> i915_gem_init+0x69/0x1a0 [i915]
> Jun 17 21:06:11 t400 kernel: [   12.136381]  [] 
> i915_driver_load+0xa54/0xe50 [i915]
> Jun 17 21:06:11 t400 kernel: [   12.136411]  [] ? 
> i915_dma_init

[PATCH] drivers/i915: Fix unnoticed failure of init_ring_common()

[Daniel Vetter]

On Thu, Jun 19, 2014 at 4:35 PM, Daniel Vetter  
wrote:
> The actual bug we seem to have is blowing up on the ggtt_unpin in
> context_fini. Which is doubly-impossible: Gen4 doesn't have hw
> contexts, so should have dctx->obj == NULL. And ring init failures
> fail earlier so shouldn't even hit the context_fini code below the
> cleanup_gem: label in driver_load. Seriously confused here.

Also please retest with latest upstream, we've made the ring init
failure non-letal for the driver. That should help you, too.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[REPOST PATCH 4/8] android: convert sync to fence api, v5

[Maarten Lankhorst]

op 19-06-14 17:22, Colin Cross schreef:
> On Wed, Jun 18, 2014 at 11:37 PM, Daniel Vetter  wrote:
>> On Wed, Jun 18, 2014 at 06:15:56PM -0700, Greg KH wrote:
>>> On Wed, Jun 18, 2014 at 12:37:11PM +0200, Maarten Lankhorst wrote:
 Just to show it's easy.

 Android syncpoints can be mapped to a timeline. This removes the need
 to maintain a separate api for synchronization. I've left the android
 trace events in place, but the core fence events should already be
 sufficient for debugging.

 v2:
 - Call fence_remove_callback in sync_fence_free if not all fences have 
 fired.
 v3:
 - Merge Colin Cross' bugfixes, and the android fence merge optimization.
 v4:
 - Merge with the upstream fixes.
 v5:
 - Fix small style issues pointed out by Thomas Hellstrom.

 Signed-off-by: Maarten Lankhorst 
 Acked-by: John Stultz 
 ---
  drivers/staging/android/Kconfig  |1
  drivers/staging/android/Makefile |2
  drivers/staging/android/sw_sync.c|6
  drivers/staging/android/sync.c   |  913 
 +++---
  drivers/staging/android/sync.h   |   79 ++-
  drivers/staging/android/sync_debug.c |  247 +
  drivers/staging/android/trace/sync.h |   12
  7 files changed, 609 insertions(+), 651 deletions(-)
  create mode 100644 drivers/staging/android/sync_debug.c
>>> With these changes, can we pull the android sync logic out of
>>> drivers/staging/ now?
>> Afaik the google guys never really looked at this and acked it. So I'm not
>> sure whether they'll follow along. The other issue I have as the
>> maintainer of gfx driver is that I don't want to implement support for two
>> different sync object primitives (once for dma-buf and once for android
>> syncpts), and my impression thus far has been that even with this we're
>> not there.
> We have tested these patches to use dma fences to back the android
> sync driver and not found any major issues.  However, my understanding
> is that dma fences are designed for implicit sync, and explicit sync
> through the android sync driver is bolted on the side to share code.
> Android is not moving away from explicit sync, but we do wrap all of
> our userspace sync accesses through libsync
> (https://android.googlesource.com/platform/system/core/+/master/libsync/sync.c,
> ignore the sw_sync parts), so if the kernel supported a slightly
> different userspace explicit sync interface we could adapt to it
> fairly easily.  All we require is that individual kernel drivers need
> to be able to accept work alongisde an fd to wait on, and to return an
> fd that will signal when the work is done, and that userspace has some
> way to merge two of those fds, wait on an fd, and get some debugging
> info from an fd.  However, this patch set doesn't do that, it has no
> way to export a dma fence as an fd except through the android sync
> driver, so it is not yet ready to fully replace android sync.
>
Dma fences can be exported as android fences, just didn't see a need for it 
yet. :-)
To wait on all implicit fences attached to a dma-buf one could simply poll the 
dma-buf directly,
or use something like a android userspace fence.

sync_fence_create takes a sync_pt as function argument, but I kept that to keep 
source code
compatibility, not because it uses any sync_pt functions. Here's a patch to 
create a userspace
fd for dma-fence instead of a android fence, applied on top of "android: 
convert sync to fence api".

diff --git a/drivers/staging/android/sw_sync.c 
b/drivers/staging/android/sw_sync.c
index a76db3ff87cb..afc3c63b0438 100644
--- a/drivers/staging/android/sw_sync.c
+++ b/drivers/staging/android/sw_sync.c
@@ -184,7 +184,7 @@ static long sw_sync_ioctl_create_fence(struct 
sw_sync_timeline *obj,
}

data.name[sizeof(data.name) - 1] = '\0';
-   fence = sync_fence_create(data.name, pt);
+   fence = sync_fence_create(data.name, &pt->base);
if (fence == NULL) {
sync_pt_free(pt);
err = -ENOMEM;
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
index 70b09b5001ba..c89a6f954e41 100644
--- a/drivers/staging/android/sync.c
+++ b/drivers/staging/android/sync.c
@@ -188,7 +188,7 @@ static void fence_check_cb_func(struct fence *f, struct 
fence_cb *cb)
 }

 /* TODO: implement a create which takes more that one sync_pt */
-struct sync_fence *sync_fence_create(const char *name, struct sync_pt *pt)
+struct sync_fence *sync_fence_create(const char *name, struct fence *pt)
 {
struct sync_fence *fence;

@@ -199,10 +199,10 @@ struct sync_fence *sync_fence_create(const char *name, 
struct sync_pt *pt)
fence->num_fences = 1;
atomic_set(&fence->status, 1);

-   fence_get(&pt->base);
-   fence->cbs[0].sync_pt = &pt->base;
+   fence_get(pt);
+   fence->cbs[0].sync_pt = pt;
fence->cbs[0].fence = fence;
-   if (fenc

[PATCH/RESEND 0/9] drm: tilcdc driver fixes

[Guido Martínez]

Hi Darren,

On Thu, Jun 19, 2014 at 08:41:43AM -0500, Darren Etheridge wrote:
> Guido,
> 
> Thanks and sorry I missed this the first time around.  I'll give it a try on
> a few of my AM335x based boards when I have access to them again on Monday.
Thanks! And no worries :)

> 
> Darren
> 
-- 
Guido Mart?nez, VanguardiaSur
www.vanguardiasur.com.ar

[REPOST PATCH 4/8] android: convert sync to fence api, v5

[Daniel Vetter]

On Thu, Jun 19, 2014 at 08:35:29AM -0700, Colin Cross wrote:
> On Thu, Jun 19, 2014 at 5:28 AM, Daniel Vetter  wrote:
> > On Thu, Jun 19, 2014 at 1:48 PM, Thierry Reding
> >  wrote:
> >>> > With these changes, can we pull the android sync logic out of
> >>> > drivers/staging/ now?
> >>>
> >>> Afaik the google guys never really looked at this and acked it. So I'm not
> >>> sure whether they'll follow along. The other issue I have as the
> >>> maintainer of gfx driver is that I don't want to implement support for two
> >>> different sync object primitives (once for dma-buf and once for android
> >>> syncpts), and my impression thus far has been that even with this we're
> >>> not there.
> >>>
> >>> I'm trying to get our own android guys to upstream their i915 syncpts
> >>> support, but thus far I haven't managed to convince them to throw people's
> >>> time at this.
> >>
> >> This has been discussed a fair bit internally recently and some of our
> >> GPU experts have raised concerns that this may result in seriously
> >> degraded performance in our proprietary graphics stack. Now I don't care
> >> very much for the proprietary graphics stack, but by extension I would
> >> assume that the same restrictions are relevant for any open-source
> >> driver as well.
> >>
> >> I'm still trying to fully understand all the implications and at the
> >> same time get some of the people who raised concerns to join in this
> >> discussion. As I understand it the concern is mostly about explicit vs.
> >> implicit synchronization and having this mechanism in the kernel will
> >> implicitly synchronize all accesses to these buffers even in cases where
> >> it's not needed (read vs. write locks, etc.). In one particular instance
> >> it was even mentioned that this kind of implicit synchronization can
> >> lead to deadlocks in some use-cases (this was mentioned for Android
> >> compositing, but I suspect that the same may happen for Wayland or X
> >> compositors).
> >
> > Well the implicit fences here actually can't deadlock. That's the
> > entire point behind using ww mutexes. I've also heard tons of
> > complaints about implicit enforced syncing (especially from opencl
> > people), but in the end drivers and always expose unsynchronized
> > access for specific cases. We do that in i915 for upload buffers and
> > other fun stuff. This is about shared stuff across different drivers
> > and different processes.
> >
> > I also expect that i915 will loose implicit syncing in a few upcoming
> > hw modes because explicit syncing is a more natural fit there.
> >
> > All that isn't about the discussion at hand imo since no matter what
> > i915 needs to have on internal representation for a bit of gpu work,
> > and afaics right now we don't have that. With this patch android
> > syncpts use Maarten's fences internally, but I can't freely exchange
> > one for the other. So in i915 I still expect to get stuck with both of
> > them, which is one too many.
> >
> > The other issue (and I haven't dug into details that much) I have with
> > syncpts are some of the interface choices. Apparently you can commit a
> > fence after creation (or at least the hw composer interface works like
> > that) which means userspace can construct deadlocks with android
> > syncpts if I'm not super careful in my driver. I haven't seen any
> > generic code to do that, so I presume everyone just blindly trusts
> > surface-flinger to not do that. Speaks of the average quality of an
> > android gfx driver if the kernel is less trusted than the compositor
> > in userspace ...
> 
> Android sync is designed not to allow userspace to deadlock the
> kernel, a sync_pt should only get created by the kernel when it has
> received a chunk of work that it expects to complete in the near
> future.  The CONFIG_SW_SYNC_USER driver violates that by allowing
> userspace to create and signal arbitrary sync points, but that is
> intended only for testing sync.

Ok, that makes sense. As long as we sufficiently taint the kernel and hide
the sw_sync framework we should be good. I was confused by the hw composer
interface spec which seemed to suggest that the fences for a screen update
completion should be attached before surfaceflinger commits the state. But
I never looked at an implemention so guess that impression is wrong.

> > There's a few other things like exposing timestamps (which are tricky
> > to do right, our driver is littered with wrong attempts) and other
> > details.
> 
> Timestamps are necessary for vsync synchronization to reduce the frame 
> latency.

I'm not against timestamps (we have them for drm vblank events too after
all), just would like for them to be optional. And we need to give
userspace very clear indication which hw clock the timestamp was based on
(or whether we're using the clock_monotonic system clock) to make sure
debug and profiling tools can properly align things. Because hw clocks
always get out of sync. Last time I've looked at the syncpt ioctls

[REPOST PATCH 1/8] fence: dma-buf cross-device synchronization (v17)

[Greg KH]

On Thu, Jun 19, 2014 at 10:00:18AM -0400, Rob Clark wrote:
> On Wed, Jun 18, 2014 at 9:13 PM, Greg KH  
> wrote:
> > On Wed, Jun 18, 2014 at 12:36:54PM +0200, Maarten Lankhorst wrote:
> >> +#define CREATE_TRACE_POINTS
> >> +#include 
> >> +
> >> +EXPORT_TRACEPOINT_SYMBOL(fence_annotate_wait_on);
> >> +EXPORT_TRACEPOINT_SYMBOL(fence_emit);
> >
> > Are you really willing to live with these as tracepoints for forever?
> > What is the use of them in debugging?  Was it just for debugging the
> > fence code, or for something else?
> >
> >> +/**
> >> + * fence_context_alloc - allocate an array of fence contexts
> >> + * @num: [in]amount of contexts to allocate
> >> + *
> >> + * This function will return the first index of the number of fences 
> >> allocated.
> >> + * The fence context is used for setting fence->context to a unique 
> >> number.
> >> + */
> >> +unsigned fence_context_alloc(unsigned num)
> >> +{
> >> + BUG_ON(!num);
> >> + return atomic_add_return(num, &fence_context_counter) - num;
> >> +}
> >> +EXPORT_SYMBOL(fence_context_alloc);
> >
> > EXPORT_SYMBOL_GPL()?  Same goes for all of the exports in here.
> > Traditionally all of the driver core exports have been with this
> > marking, any objection to making that change here as well?
> 
> tbh, I prefer EXPORT_SYMBOL()..  well, I'd prefer even more if there
> wasn't even a need for EXPORT_SYMBOL_GPL(), but sadly it is a fact of
> life.  We already went through this debate once with dma-buf.  We
> aren't going to change $evil_vendor's mind about non-gpl modules.  The
> only result will be a more flugly convoluted solution (ie. use syncpt
> EXPORT_SYMBOL() on top of fence EXPORT_SYMBOL_GPL()) just as a
> workaround, with the result that no-one benefits.

It has been proven that using _GPL() exports have caused companies to
release their code "properly" over the years, so as these really are
Linux-only apis, please change them to be marked this way, it helps
everyone out in the end.

> >> +int __fence_signal(struct fence *fence)
> >> +{
> >> + struct fence_cb *cur, *tmp;
> >> + int ret = 0;
> >> +
> >> + if (WARN_ON(!fence))
> >> + return -EINVAL;
> >> +
> >> + if (!ktime_to_ns(fence->timestamp)) {
> >> + fence->timestamp = ktime_get();
> >> + smp_mb__before_atomic();
> >> + }
> >> +
> >> + if (test_and_set_bit(FENCE_FLAG_SIGNALED_BIT, &fence->flags)) {
> >> + ret = -EINVAL;
> >> +
> >> + /*
> >> +  * we might have raced with the unlocked fence_signal,
> >> +  * still run through all callbacks
> >> +  */
> >> + } else
> >> + trace_fence_signaled(fence);
> >> +
> >> + list_for_each_entry_safe(cur, tmp, &fence->cb_list, node) {
> >> + list_del_init(&cur->node);
> >> + cur->func(fence, cur);
> >> + }
> >> + return ret;
> >> +}
> >> +EXPORT_SYMBOL(__fence_signal);
> >
> > Don't export a function with __ in front of it, you are saying that an
> > internal function is somehow "valid" for everyone else to call?  Why?
> > You aren't even documenting the function, so who knows how to use it?
> 
> fwiw, the __ versions appear to mainly be concessions for android
> syncpt.  That is the only user outside of fence.c, and it should stay
> that way.

How are you going to "ensure" this?  And where did you document it?
Please fix this up, it's a horrid way to create a new api.

If the android code needs to be fixed to fit into this model, then fix
it.

> >> +void
> >> +__fence_init(struct fence *fence, const struct fence_ops *ops,
> >> +  spinlock_t *lock, unsigned context, unsigned seqno)
> >> +{
> >> + BUG_ON(!lock);
> >> + BUG_ON(!ops || !ops->wait || !ops->enable_signaling ||
> >> +!ops->get_driver_name || !ops->get_timeline_name);
> >> +
> >> + kref_init(&fence->refcount);
> >> + fence->ops = ops;
> >> + INIT_LIST_HEAD(&fence->cb_list);
> >> + fence->lock = lock;
> >> + fence->context = context;
> >> + fence->seqno = seqno;
> >> + fence->flags = 0UL;
> >> +
> >> + trace_fence_init(fence);
> >> +}
> >> +EXPORT_SYMBOL(__fence_init);
> >
> > Again with the __ exported function...
> >
> > I don't even see a fence_init() function anywhere, why the __ ?
> >
> 
> think of it as a 'protected' constructor.. only the derived fence
> subclass should call.

Where do you say this?  Again, not a good reason, fix up the api please.

> >> + kref_get(&fence->refcount);
> >> +}
> >
> > Why is this inline?
> 
> performance can be critical.. especially if the driver is using this
> fence mechanism for internal buffers as well as shared buffers (which
> is what I'd like to do to avoid having to deal with two different
> fencing mechanisms for shared vs non-shared buffers), since you could
> easily have 100's or perhaps 1000's of buffers involved in a submit.

"can be".  Did you actually measure it?  Please do so.

> The fence stuff does t

AGP GART clarifications, please!

[Émeric MASCHINO]

DRI gurus,

If I'm not mistaken, the current Linux graphics stack is as follows
(excluding Wayland protocol and LLVM or GLAMOR-based approaches):

X11/OpenGL app -> libX/Mesa -> DDX driver/Mesa DRI module -> kernel
DRM -> hardware

What's unclear to me is, in the case of an AGP graphics adapter, where
does the AGP GART takes place in this stack (if applicable)?

Say I have an AGP ATI R300-based graphics adapter. In the above stack,
DDX driver is x86-video-ati, Mesa DRI module is r300 (Classic or
Gallium3D) and kernel DRM is radeon. (Am I still right?)

Obviously, this AGP graphics adapter nevertheless works flawlessly
without AGP GART compiled in kernel or as module. This is at least
true for the open source stack, I've tested it. Is my AGP graphics
adapter thus running in what's known as PCI/PCIe mode? I've read all
the AGP scatter/gather, texturing and fast writes things, but I can't
see any difference performance-wise between having AGP GART compiled
in kernel or as a module and no AGP GART. Is it because my usage
doesn't stress the graphics subsystem enough or is it because PCI/PCIe
mode is so amazing that AGP GART doesn't provide any performance
enhancements? AGP GART however provides me nice stability issues ;-)

When compiled in kernel or as a module, is AGP GART only used for 3D
hardware acceleration by the r300 Mesa DRI module (or is it by the
radeon DRM? Or both?) or also by the xf86-video-ati DDX driver for
XAA/EXA acceleration? And what about video acceleration?

What happens when the AGP GART isn't compiled in kernel or as a
module? Is it simply a matter of skipping a participant (the AGP GART)
in the graphics stack or are there different code paths in the DDX
driver, Mesa DRI module and/or kernel DRM depending upon the
availability of AGP GART or not?

Is the code path the same in the following situations:
- no AGP GART at all;
- AGP GART compiled in kernel or as a module but "options radeon
agpmode=-1" set in /etc/modprobe.d/radeon-kms.conf.

Is setting a different AGP mode (1x, 2x, 4x, 8x) in
/etc/modprobe.d/radeon-kms.conf only a hardware thing or are there
different code paths taken in the various components of the graphics
stack depending on the current AGP mode?

What happens if you compile AGP GART in kernel or as a module with a
PCI/PCIe graphics adapter? Is it simply ignored? How? Out of Linux
control at the hardware level or are there simply no code path taking
advantages of the AGP GART in a PCI/PCIe graphics stack?

Finally is this assertion of the "radeon-KMS with AGP gfxcards"
section of the radeonBuildHowTo [1] still true?

"AGP gfxcards have a lot of problems so if you have one it is good
idea to test PCI/PCIE mode using radeon.agpmode=-1."

Thanks,

 ?meric


[1] http://www.x.org/wiki/radeonBuildHowTo/

[PATCH libdrm 1/2] intel: Sync the command parser version parameter from kernel

[Damien Lespiau]

On Thu, Jun 19, 2014 at 09:28:08AM -0700, Volkin, Bradley D wrote:
> On Thu, Jun 19, 2014 at 11:31:53AM +0100, Damien Lespiau wrote:
> > Cc: Bradley Volkin 
> > Signed-off-by: Damien Lespiau 
> 
> Thanks for taking care of this Damien.
> 
> Reviewed-by: Brad Volkin 

Thanks for the review, pushed both patches.

-- 
Damien

[REPOST PATCH 1/8] fence: dma-buf cross-device synchronization (v17)

[Rob Clark]

On Thu, Jun 19, 2014 at 1:00 PM, Greg KH  wrote:
> On Thu, Jun 19, 2014 at 10:00:18AM -0400, Rob Clark wrote:
>> On Wed, Jun 18, 2014 at 9:13 PM, Greg KH  
>> wrote:
>> > On Wed, Jun 18, 2014 at 12:36:54PM +0200, Maarten Lankhorst wrote:
>> >> +#define CREATE_TRACE_POINTS
>> >> +#include 
>> >> +
>> >> +EXPORT_TRACEPOINT_SYMBOL(fence_annotate_wait_on);
>> >> +EXPORT_TRACEPOINT_SYMBOL(fence_emit);
>> >
>> > Are you really willing to live with these as tracepoints for forever?
>> > What is the use of them in debugging?  Was it just for debugging the
>> > fence code, or for something else?
>> >
>> >> +/**
>> >> + * fence_context_alloc - allocate an array of fence contexts
>> >> + * @num: [in]amount of contexts to allocate
>> >> + *
>> >> + * This function will return the first index of the number of fences 
>> >> allocated.
>> >> + * The fence context is used for setting fence->context to a unique 
>> >> number.
>> >> + */
>> >> +unsigned fence_context_alloc(unsigned num)
>> >> +{
>> >> + BUG_ON(!num);
>> >> + return atomic_add_return(num, &fence_context_counter) - num;
>> >> +}
>> >> +EXPORT_SYMBOL(fence_context_alloc);
>> >
>> > EXPORT_SYMBOL_GPL()?  Same goes for all of the exports in here.
>> > Traditionally all of the driver core exports have been with this
>> > marking, any objection to making that change here as well?
>>
>> tbh, I prefer EXPORT_SYMBOL()..  well, I'd prefer even more if there
>> wasn't even a need for EXPORT_SYMBOL_GPL(), but sadly it is a fact of
>> life.  We already went through this debate once with dma-buf.  We
>> aren't going to change $evil_vendor's mind about non-gpl modules.  The
>> only result will be a more flugly convoluted solution (ie. use syncpt
>> EXPORT_SYMBOL() on top of fence EXPORT_SYMBOL_GPL()) just as a
>> workaround, with the result that no-one benefits.
>
> It has been proven that using _GPL() exports have caused companies to
> release their code "properly" over the years, so as these really are
> Linux-only apis, please change them to be marked this way, it helps
> everyone out in the end.

Well, maybe that is the true in some cases.  But it certainly didn't
work out that way for dma-buf.  And I think the end result is worse.

I don't really like coming down on the side of EXPORT_SYMBOL() instead
of EXPORT_SYMBOL_GPL(), but if we do use EXPORT_SYMBOL_GPL() then the
result will only be creative workarounds using the _GPL symbols
indirectly by whatever is available via EXPORT_SYMBOL().  I don't
really see how that will be better.

>> >> +int __fence_signal(struct fence *fence)
>> >> +{
>> >> + struct fence_cb *cur, *tmp;
>> >> + int ret = 0;
>> >> +
>> >> + if (WARN_ON(!fence))
>> >> + return -EINVAL;
>> >> +
>> >> + if (!ktime_to_ns(fence->timestamp)) {
>> >> + fence->timestamp = ktime_get();
>> >> + smp_mb__before_atomic();
>> >> + }
>> >> +
>> >> + if (test_and_set_bit(FENCE_FLAG_SIGNALED_BIT, &fence->flags)) {
>> >> + ret = -EINVAL;
>> >> +
>> >> + /*
>> >> +  * we might have raced with the unlocked fence_signal,
>> >> +  * still run through all callbacks
>> >> +  */
>> >> + } else
>> >> + trace_fence_signaled(fence);
>> >> +
>> >> + list_for_each_entry_safe(cur, tmp, &fence->cb_list, node) {
>> >> + list_del_init(&cur->node);
>> >> + cur->func(fence, cur);
>> >> + }
>> >> + return ret;
>> >> +}
>> >> +EXPORT_SYMBOL(__fence_signal);
>> >
>> > Don't export a function with __ in front of it, you are saying that an
>> > internal function is somehow "valid" for everyone else to call?  Why?
>> > You aren't even documenting the function, so who knows how to use it?
>>
>> fwiw, the __ versions appear to mainly be concessions for android
>> syncpt.  That is the only user outside of fence.c, and it should stay
>> that way.
>
> How are you going to "ensure" this?  And where did you document it?
> Please fix this up, it's a horrid way to create a new api.
>
> If the android code needs to be fixed to fit into this model, then fix
> it.

heh, and in fact I was wrong about this.. the __ versions are actually
for when the lock is already held.  Maarten needs to rename (ie
_locked suffix) and add some API docs for this.

>> >> +void
>> >> +__fence_init(struct fence *fence, const struct fence_ops *ops,
>> >> +  spinlock_t *lock, unsigned context, unsigned seqno)
>> >> +{
>> >> + BUG_ON(!lock);
>> >> + BUG_ON(!ops || !ops->wait || !ops->enable_signaling ||
>> >> +!ops->get_driver_name || !ops->get_timeline_name);
>> >> +
>> >> + kref_init(&fence->refcount);
>> >> + fence->ops = ops;
>> >> + INIT_LIST_HEAD(&fence->cb_list);
>> >> + fence->lock = lock;
>> >> + fence->context = context;
>> >> + fence->seqno = seqno;
>> >> + fence->flags = 0UL;
>> >> +
>> >> + trace_fence_init(fence);
>> >> +}
>> >> +EXPORT_SYMBOL(__fence_init);
>> >
>> > 

[REPOST PATCH 1/8] fence: dma-buf cross-device synchronization (v17)

[Greg KH]

On Thu, Jun 19, 2014 at 01:45:30PM -0400, Rob Clark wrote:
> On Thu, Jun 19, 2014 at 1:00 PM, Greg KH  
> wrote:
> > On Thu, Jun 19, 2014 at 10:00:18AM -0400, Rob Clark wrote:
> >> On Wed, Jun 18, 2014 at 9:13 PM, Greg KH  
> >> wrote:
> >> > On Wed, Jun 18, 2014 at 12:36:54PM +0200, Maarten Lankhorst wrote:
> >> >> +#define CREATE_TRACE_POINTS
> >> >> +#include 
> >> >> +
> >> >> +EXPORT_TRACEPOINT_SYMBOL(fence_annotate_wait_on);
> >> >> +EXPORT_TRACEPOINT_SYMBOL(fence_emit);
> >> >
> >> > Are you really willing to live with these as tracepoints for forever?
> >> > What is the use of them in debugging?  Was it just for debugging the
> >> > fence code, or for something else?
> >> >
> >> >> +/**
> >> >> + * fence_context_alloc - allocate an array of fence contexts
> >> >> + * @num: [in]amount of contexts to allocate
> >> >> + *
> >> >> + * This function will return the first index of the number of fences 
> >> >> allocated.
> >> >> + * The fence context is used for setting fence->context to a unique 
> >> >> number.
> >> >> + */
> >> >> +unsigned fence_context_alloc(unsigned num)
> >> >> +{
> >> >> + BUG_ON(!num);
> >> >> + return atomic_add_return(num, &fence_context_counter) - num;
> >> >> +}
> >> >> +EXPORT_SYMBOL(fence_context_alloc);
> >> >
> >> > EXPORT_SYMBOL_GPL()?  Same goes for all of the exports in here.
> >> > Traditionally all of the driver core exports have been with this
> >> > marking, any objection to making that change here as well?
> >>
> >> tbh, I prefer EXPORT_SYMBOL()..  well, I'd prefer even more if there
> >> wasn't even a need for EXPORT_SYMBOL_GPL(), but sadly it is a fact of
> >> life.  We already went through this debate once with dma-buf.  We
> >> aren't going to change $evil_vendor's mind about non-gpl modules.  The
> >> only result will be a more flugly convoluted solution (ie. use syncpt
> >> EXPORT_SYMBOL() on top of fence EXPORT_SYMBOL_GPL()) just as a
> >> workaround, with the result that no-one benefits.
> >
> > It has been proven that using _GPL() exports have caused companies to
> > release their code "properly" over the years, so as these really are
> > Linux-only apis, please change them to be marked this way, it helps
> > everyone out in the end.
> 
> Well, maybe that is the true in some cases.  But it certainly didn't
> work out that way for dma-buf.  And I think the end result is worse.
> 
> I don't really like coming down on the side of EXPORT_SYMBOL() instead
> of EXPORT_SYMBOL_GPL(), but if we do use EXPORT_SYMBOL_GPL() then the
> result will only be creative workarounds using the _GPL symbols
> indirectly by whatever is available via EXPORT_SYMBOL().  I don't
> really see how that will be better.

You are saying that you _know_ companies will violate our license, so
you should just "give up"?  And how do you know people aren't working on
preventing those "indirect" usages as well?  :)

Sorry, I'm not going to give up here, again, it has proven to work in
the past in changing the ways of _very_ large companies, why stop now?

thanks,

greg k-h

[Bug 80141] Fails to page flip multiple time, queue overflows waiting for one to finish that never does crashing entire system.

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

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

--- Comment #4 from Aaron B  ---
Created attachment 101374
  --> https://bugs.freedesktop.org/attachment.cgi?id=101374&action=edit
Crash information from card unresponsive crash. This one recovered from it,
though.

Just had a crash happen on 3.15 generic, I'm going to go back to 3.13 now and
see if I can trigger it on there from doing the same things.

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

[REPOST PATCH 1/8] fence: dma-buf cross-device synchronization (v17)

[James Bottomley]

On Thu, 2014-06-19 at 11:19 -0700, Greg KH wrote:
> On Thu, Jun 19, 2014 at 01:45:30PM -0400, Rob Clark wrote:
> > On Thu, Jun 19, 2014 at 1:00 PM, Greg KH  
> > wrote:
> > > On Thu, Jun 19, 2014 at 10:00:18AM -0400, Rob Clark wrote:
> > >> On Wed, Jun 18, 2014 at 9:13 PM, Greg KH  
> > >> wrote:
> > >> > On Wed, Jun 18, 2014 at 12:36:54PM +0200, Maarten Lankhorst wrote:
> > >> >> +#define CREATE_TRACE_POINTS
> > >> >> +#include 
> > >> >> +
> > >> >> +EXPORT_TRACEPOINT_SYMBOL(fence_annotate_wait_on);
> > >> >> +EXPORT_TRACEPOINT_SYMBOL(fence_emit);
> > >> >
> > >> > Are you really willing to live with these as tracepoints for forever?
> > >> > What is the use of them in debugging?  Was it just for debugging the
> > >> > fence code, or for something else?
> > >> >
> > >> >> +/**
> > >> >> + * fence_context_alloc - allocate an array of fence contexts
> > >> >> + * @num: [in]amount of contexts to allocate
> > >> >> + *
> > >> >> + * This function will return the first index of the number of fences 
> > >> >> allocated.
> > >> >> + * The fence context is used for setting fence->context to a unique 
> > >> >> number.
> > >> >> + */
> > >> >> +unsigned fence_context_alloc(unsigned num)
> > >> >> +{
> > >> >> + BUG_ON(!num);
> > >> >> + return atomic_add_return(num, &fence_context_counter) - num;
> > >> >> +}
> > >> >> +EXPORT_SYMBOL(fence_context_alloc);
> > >> >
> > >> > EXPORT_SYMBOL_GPL()?  Same goes for all of the exports in here.
> > >> > Traditionally all of the driver core exports have been with this
> > >> > marking, any objection to making that change here as well?
> > >>
> > >> tbh, I prefer EXPORT_SYMBOL()..  well, I'd prefer even more if there
> > >> wasn't even a need for EXPORT_SYMBOL_GPL(), but sadly it is a fact of
> > >> life.  We already went through this debate once with dma-buf.  We
> > >> aren't going to change $evil_vendor's mind about non-gpl modules.  The
> > >> only result will be a more flugly convoluted solution (ie. use syncpt
> > >> EXPORT_SYMBOL() on top of fence EXPORT_SYMBOL_GPL()) just as a
> > >> workaround, with the result that no-one benefits.
> > >
> > > It has been proven that using _GPL() exports have caused companies to
> > > release their code "properly" over the years, so as these really are
> > > Linux-only apis, please change them to be marked this way, it helps
> > > everyone out in the end.
> > 
> > Well, maybe that is the true in some cases.  But it certainly didn't
> > work out that way for dma-buf.  And I think the end result is worse.
> > 
> > I don't really like coming down on the side of EXPORT_SYMBOL() instead
> > of EXPORT_SYMBOL_GPL(), but if we do use EXPORT_SYMBOL_GPL() then the
> > result will only be creative workarounds using the _GPL symbols
> > indirectly by whatever is available via EXPORT_SYMBOL().  I don't
> > really see how that will be better.
> 
> You are saying that you _know_ companies will violate our license, so
> you should just "give up"?  And how do you know people aren't working on
> preventing those "indirect" usages as well?  :)
> 
> Sorry, I'm not going to give up here, again, it has proven to work in
> the past in changing the ways of _very_ large companies, why stop now?

When you try to train a dog, you have to be consistent about it.  We're
fantastically inconsistent in symbol exports.

For instance, the mutex primitives are all EXPORT_SYMBOL(), so we're
telling proprietary modules they can use them.  However, when the kernel
is built with CONFIG_DEBUG_MUTEX, they all become
EXPORT_SYMBOL_GPL() ... what type of crazy message is that supposed to
send?  It's OK to use mutexes but it's potentially a GPL violation to
debug them?

We either need to decide that we have a defined and consistent part of
our API that's GPL only or make the bold statement that we don't have
any part of our API that's usable by non-GPL modules.  Right at the
minute we do neither and it confuses people no end about what is and
isn't allowed.

James

[REPOST PATCH 1/8] fence: dma-buf cross-device synchronization (v17)

[Rob Clark]

On Thu, Jun 19, 2014 at 2:19 PM, Greg KH  wrote:
> On Thu, Jun 19, 2014 at 01:45:30PM -0400, Rob Clark wrote:
>> On Thu, Jun 19, 2014 at 1:00 PM, Greg KH  
>> wrote:
>> > On Thu, Jun 19, 2014 at 10:00:18AM -0400, Rob Clark wrote:
>> >> On Wed, Jun 18, 2014 at 9:13 PM, Greg KH  
>> >> wrote:
>> >> > On Wed, Jun 18, 2014 at 12:36:54PM +0200, Maarten Lankhorst wrote:
>> >> >> +#define CREATE_TRACE_POINTS
>> >> >> +#include 
>> >> >> +
>> >> >> +EXPORT_TRACEPOINT_SYMBOL(fence_annotate_wait_on);
>> >> >> +EXPORT_TRACEPOINT_SYMBOL(fence_emit);
>> >> >
>> >> > Are you really willing to live with these as tracepoints for forever?
>> >> > What is the use of them in debugging?  Was it just for debugging the
>> >> > fence code, or for something else?
>> >> >
>> >> >> +/**
>> >> >> + * fence_context_alloc - allocate an array of fence contexts
>> >> >> + * @num: [in]amount of contexts to allocate
>> >> >> + *
>> >> >> + * This function will return the first index of the number of fences 
>> >> >> allocated.
>> >> >> + * The fence context is used for setting fence->context to a unique 
>> >> >> number.
>> >> >> + */
>> >> >> +unsigned fence_context_alloc(unsigned num)
>> >> >> +{
>> >> >> + BUG_ON(!num);
>> >> >> + return atomic_add_return(num, &fence_context_counter) - num;
>> >> >> +}
>> >> >> +EXPORT_SYMBOL(fence_context_alloc);
>> >> >
>> >> > EXPORT_SYMBOL_GPL()?  Same goes for all of the exports in here.
>> >> > Traditionally all of the driver core exports have been with this
>> >> > marking, any objection to making that change here as well?
>> >>
>> >> tbh, I prefer EXPORT_SYMBOL()..  well, I'd prefer even more if there
>> >> wasn't even a need for EXPORT_SYMBOL_GPL(), but sadly it is a fact of
>> >> life.  We already went through this debate once with dma-buf.  We
>> >> aren't going to change $evil_vendor's mind about non-gpl modules.  The
>> >> only result will be a more flugly convoluted solution (ie. use syncpt
>> >> EXPORT_SYMBOL() on top of fence EXPORT_SYMBOL_GPL()) just as a
>> >> workaround, with the result that no-one benefits.
>> >
>> > It has been proven that using _GPL() exports have caused companies to
>> > release their code "properly" over the years, so as these really are
>> > Linux-only apis, please change them to be marked this way, it helps
>> > everyone out in the end.
>>
>> Well, maybe that is the true in some cases.  But it certainly didn't
>> work out that way for dma-buf.  And I think the end result is worse.
>>
>> I don't really like coming down on the side of EXPORT_SYMBOL() instead
>> of EXPORT_SYMBOL_GPL(), but if we do use EXPORT_SYMBOL_GPL() then the
>> result will only be creative workarounds using the _GPL symbols
>> indirectly by whatever is available via EXPORT_SYMBOL().  I don't
>> really see how that will be better.
>
> You are saying that you _know_ companies will violate our license, so
> you should just "give up"?  And how do you know people aren't working on
> preventing those "indirect" usages as well?  :)

Well, all I know is what happened with dmabuf.  This seems like the
exact same scenario (same vendor, same driver, same use-case).

Not really sure how we could completely prevent indirect usage, given
that drm core and many of the drivers are dual MIT/GPL.   (But ofc,
IANAL.)

> Sorry, I'm not going to give up here, again, it has proven to work in
> the past in changing the ways of _very_ large companies, why stop now?

In the general case, I would agree.  But in this specific case, I am
not very optimistic.

That said, it isn't really my loss if it is _GPL()..  I don't have to
use or support that particular driver.  But given that we have some
history from the same debate with dma-buf, I think it is pretty easy
to infer the result from making fence EXPORT_SYMBOL_GPL().

BR,
-R

> thanks,
>
> greg k-h

[PATCH v2] drivers/i915: Fix unnoticed failure of init_ring_common()

[Daniel Vetter]

On Thu, Jun 19, 2014 at 07:07:15PM +0200, Konrad Zapalowicz wrote:
> This commit add check for return value of init_ring_common() in the
> init_render_ring(). Now, when failure is detected the error code is
> propagated to the caller instead of being ignored.
> 
> Signed-off-by: Konrad Zapalowicz 

Queued for -next, thanks for the patch.
-Daniel

> ---
>  
>  v2:
>- remove from commit message references to the Oops.
> 
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 279488a..d205b0d 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -604,6 +604,8 @@ static int init_render_ring(struct intel_engine_cs *ring)
>   struct drm_device *dev = ring->dev;
>   struct drm_i915_private *dev_priv = dev->dev_private;
>   int ret = init_ring_common(ring);
> + if (ret)
> + return ret;
>  
>   /* WaTimedSingleVertexDispatch:cl,bw,ctg,elk,ilk,snb */
>   if (INTEL_INFO(dev)->gen >= 4 && INTEL_INFO(dev)->gen < 7)
> -- 
> 1.8.1.2
> 

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

[REPOST PATCH 1/8] fence: dma-buf cross-device synchronization (v17)

[Daniel Vetter]

On Thu, Jun 19, 2014 at 7:00 PM, Greg KH  wrote:
>> >> + BUG_ON(f1->context != f2->context);
>> >
>> > Nice, you just crashed the kernel, making it impossible to debug or
>> > recover :(
>>
>> agreed, that should probably be 'if (WARN_ON(...)) return NULL;'
>>
>> (but at least I wouldn't expect to hit that under console_lock so you
>> should at least see the last N lines of the backtrace on the screen
>> ;-))
>
> Lots of devices don't have console screens :)

Aside: This is a pet peeve of mine and recently I've switched to
rejecting all patch that have a BUG_ON, period. Except when you can
prove that the kernel will die in the next few lines and there's
nothing you can do about it a WARN_ON is always better - I've wasted
_way_ too much time debugging hard hangs because such a "benign"
BUG_ON ended up eating my irq handler or a spinlock required by such.
Or some other nonsense that makes debugging a royal pita, especially
if your remote debugger consists of a frustrated users staring at a
hung machine.



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

[REPOST PATCH 1/8] fence: dma-buf cross-device synchronization (v17)

[Daniel Vetter]

On Thu, Jun 19, 2014 at 8:19 PM, Greg KH  wrote:
>> >> > EXPORT_SYMBOL_GPL()?  Same goes for all of the exports in here.
>> >> > Traditionally all of the driver core exports have been with this
>> >> > marking, any objection to making that change here as well?
>> >>
>> >> tbh, I prefer EXPORT_SYMBOL()..  well, I'd prefer even more if there
>> >> wasn't even a need for EXPORT_SYMBOL_GPL(), but sadly it is a fact of
>> >> life.  We already went through this debate once with dma-buf.  We
>> >> aren't going to change $evil_vendor's mind about non-gpl modules.  The
>> >> only result will be a more flugly convoluted solution (ie. use syncpt
>> >> EXPORT_SYMBOL() on top of fence EXPORT_SYMBOL_GPL()) just as a
>> >> workaround, with the result that no-one benefits.
>> >
>> > It has been proven that using _GPL() exports have caused companies to
>> > release their code "properly" over the years, so as these really are
>> > Linux-only apis, please change them to be marked this way, it helps
>> > everyone out in the end.
>>
>> Well, maybe that is the true in some cases.  But it certainly didn't
>> work out that way for dma-buf.  And I think the end result is worse.
>>
>> I don't really like coming down on the side of EXPORT_SYMBOL() instead
>> of EXPORT_SYMBOL_GPL(), but if we do use EXPORT_SYMBOL_GPL() then the
>> result will only be creative workarounds using the _GPL symbols
>> indirectly by whatever is available via EXPORT_SYMBOL().  I don't
>> really see how that will be better.
>
> You are saying that you _know_ companies will violate our license, so
> you should just "give up"?  And how do you know people aren't working on
> preventing those "indirect" usages as well?  :)
>
> Sorry, I'm not going to give up here, again, it has proven to work in
> the past in changing the ways of _very_ large companies, why stop now?

Dave should chime in here since currently dma-buf is _GPL and the
drm_prime.c wrapper for it is not (and he merged that one, contributed
from said $vendor). And since we're gfx people everything we do is MIT
licensed (that's where X is from after all), so _GPL for for drm stuff
really doesn't make a lot of sense for us. ianal and all that applies.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[REPOST PATCH 1/8] fence: dma-buf cross-device synchronization (v17)

[Greg KH]

On Thu, Jun 19, 2014 at 09:15:36PM +0200, Daniel Vetter wrote:
> On Thu, Jun 19, 2014 at 7:00 PM, Greg KH  
> wrote:
> >> >> + BUG_ON(f1->context != f2->context);
> >> >
> >> > Nice, you just crashed the kernel, making it impossible to debug or
> >> > recover :(
> >>
> >> agreed, that should probably be 'if (WARN_ON(...)) return NULL;'
> >>
> >> (but at least I wouldn't expect to hit that under console_lock so you
> >> should at least see the last N lines of the backtrace on the screen
> >> ;-))
> >
> > Lots of devices don't have console screens :)
> 
> Aside: This is a pet peeve of mine and recently I've switched to
> rejecting all patch that have a BUG_ON, period.

Please do, I have been for a few years now as well for the same reasons
you cite.

greg k-h

[PATCH] [RFC] drm/i915/dp: add support for load detect Apple DP->VGA dongles

[Daniel Vetter]

On Fri, Jun 06, 2014 at 12:57:46PM +0300, Jani Nikula wrote:
> On Fri, 06 Jun 2014, Dave Airlie  wrote:
> > From: Dave Airlie 
> >
> > The DP1.2 spec has some bits for forced load sensing on VGA dongles,
> > however the apple dongle I have doesn't do DP 1.2 yet, so I dug into
> > its vendor specific area and found a register that seems to correspond
> > to load detected on the outputs.
> >
> > The main reason I need this was at LCA this year the slide capture system
> > failed to provide EDID, but I realised OSX worked. Really need to add 
> > support
> > to nouveau, but hey i915 is a start.
> >
> > The dongle also appears to use short IRQs to denote a plug/unplug event,
> > though something seems to be failing in reading back the dpcd values from 
> > it.
> > (also this is based on my MST work just because.)
> 
> Good timing for making use of the OUI! See
> 
> http://mid.gmane.org/1401920981-3137-1-git-send-email-clinton.a.taylor at 
> intel.com
> 
> > Signed-off-by: Dave Airlie 

Just a high-level bikeshed: Now that we have this nice dp helper library
shouldn't we add a dp_detect function there which fills out a bunch of
sink state in the core dp struct and also implements tricks like this?
Maybe as the plain dp version of the relevant dp mst handler?

Definitely not much point in copy&pasting this for radeon, noveau, tegra
and all the others.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] drm/msm: Implement msm drm fb_mmap callback function

[Rob Clark]

On Thu, Jun 19, 2014 at 5:19 PM, Stephen Boyd  wrote:
> On 06/18/14 13:55, Hai Li wrote:
>> diff --git a/drivers/gpu/drm/msm/msm_fbdev.c 
>> b/drivers/gpu/drm/msm/msm_fbdev.c
>> index 4f4e7b4..2522f51 100644
>> --- a/drivers/gpu/drm/msm/msm_fbdev.c
>> +++ b/drivers/gpu/drm/msm/msm_fbdev.c
>> @@ -19,6 +19,11 @@
>>
>>  #include "drm_crtc.h"
>>  #include "drm_fb_helper.h"
>> +#include "msm_gem.h"
>> +
>> +extern int msm_gem_mmap_obj(struct drm_gem_object *obj,
>> + struct vm_area_struct *vma);
>
> This can't go into some header file?

it probably should go in msm_gem.h.. although this would be the one
special case where it is needed outside of msm_gem.c so I don't mind
too much

>> +static int msm_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma);
>>
>>  /*
>>   * fbdev funcs, to implement legacy fbdev interface on top of drm driver
>> @@ -43,6 +48,7 @@ static struct fb_ops msm_fb_ops = {
>>   .fb_fillrect = sys_fillrect,
>>   .fb_copyarea = sys_copyarea,
>>   .fb_imageblit = sys_imageblit,
>> + .fb_mmap = msm_fbdev_mmap,
>>
>>   .fb_check_var = drm_fb_helper_check_var,
>>   .fb_set_par = drm_fb_helper_set_par,
>> @@ -51,6 +57,31 @@ static struct fb_ops msm_fb_ops = {
>>   .fb_setcmap = drm_fb_helper_setcmap,
>>  };
>>
>> +static int msm_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma)
>> +{
>> + struct drm_fb_helper *helper = (struct drm_fb_helper *)info->par;
>> + struct msm_fbdev *fbdev = to_msm_fbdev(helper);
>> + struct drm_gem_object *drm_obj = fbdev->bo;
>> + struct drm_device *dev = helper->dev;
>> + int ret = 0;
>
> unnecessary initialization to 0.
>
>> +
>> + if (drm_device_is_unplugged(dev))
>> + return -ENODEV;
>> +
>> + mutex_lock(&dev->struct_mutex);
>> +
>> + ret = drm_gem_mmap_obj(drm_obj, drm_obj->size, vma);
>> +
>> + mutex_unlock(&dev->struct_mutex);
>> +
>> + if (ret) {
>> + pr_err("%s:drm_gem_mmap_obj fail\n", __func__);
>> + return ret;
>> + }
>> +
>> + return msm_gem_mmap_obj(drm_obj, vma);
>> +}
>> +
>>  static int msm_fbdev_create(struct drm_fb_helper *helper,
>>   struct drm_fb_helper_surface_size *sizes)
>>  {
>> @@ -108,7 +139,11 @@ static int msm_fbdev_create(struct drm_fb_helper 
>> *helper,
>>   mutex_lock(&dev->struct_mutex);
>>
>>   /* TODO implement our own fb_mmap so we don't need this: */
>
> Is this comment still relevant?

partially.. although it changes to the "can we make sure we can safely
pin on panic when fbcon is restored"..

probably a low priority thing, because we are not really tight on
device virtual space.

BR,
-R


>> - msm_gem_get_iova_locked(fbdev->bo, 0, &paddr);
>> + ret = msm_gem_get_iova_locked(fbdev->bo, 0, &paddr);
>> + if (ret) {
>> + dev_err(dev->dev, "failed to get buffer obj iova: %d\n", ret);
>> + goto fail;
>> + }
>>
>>   fbi = framebuffer_alloc(0, dev->dev);
>>   if (!fbi) {
>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted by The Linux Foundation
>

[PATCH] drm/tegra: sor - Add register debugging support

[Stéphane Marchesin]

This adds a debugfs entry to print the register state. This can be
fairly useful when debugging eDP link issues.

Signed-off-by: St?phane Marchesin 
---
 drivers/gpu/drm/tegra/sor.c | 163 +++-
 1 file changed, 162 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index 27c979b..fe92098 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -37,6 +37,8 @@ struct tegra_sor {
struct mutex lock;
bool enabled;

+   struct drm_minor *minor;
+   struct drm_info_list *debugfs_files;
struct dentry *debugfs;
 };

@@ -1238,6 +1240,139 @@ static const struct file_operations tegra_sor_crc_fops 
= {
.release = tegra_sor_crc_release,
 };

+static int tegra_sor_show_regs(struct seq_file *s, void *data)
+{
+   struct drm_info_node *node = s->private;
+   struct tegra_sor *sor = node->info_ent->data;
+
+#define DUMP_REG(name) \
+   seq_printf(s, "%-40s %#05x %08lx\n", #name, name,   \
+  tegra_sor_readl(sor, name))
+
+   DUMP_REG(SOR_CTXSW);
+   DUMP_REG(SOR_SUPER_STATE_0);
+   DUMP_REG(SOR_SUPER_STATE_1);
+   DUMP_REG(SOR_STATE_0);
+   DUMP_REG(SOR_STATE_1);
+   DUMP_REG(SOR_HEAD_STATE_0(0));
+   DUMP_REG(SOR_HEAD_STATE_0(1));
+   DUMP_REG(SOR_HEAD_STATE_1(0));
+   DUMP_REG(SOR_HEAD_STATE_1(1));
+   DUMP_REG(SOR_HEAD_STATE_2(0));
+   DUMP_REG(SOR_HEAD_STATE_2(1));
+   DUMP_REG(SOR_HEAD_STATE_3(0));
+   DUMP_REG(SOR_HEAD_STATE_3(1));
+   DUMP_REG(SOR_HEAD_STATE_4(0));
+   DUMP_REG(SOR_HEAD_STATE_4(1));
+   DUMP_REG(SOR_HEAD_STATE_5(0));
+   DUMP_REG(SOR_HEAD_STATE_5(1));
+   DUMP_REG(SOR_CRC_CNTRL);
+   DUMP_REG(SOR_DP_DEBUG_MVID);
+   DUMP_REG(SOR_CLK_CNTRL);
+   DUMP_REG(SOR_CAP);
+   DUMP_REG(SOR_PWR);
+   DUMP_REG(SOR_TEST);
+   DUMP_REG(SOR_PLL_0);
+   DUMP_REG(SOR_PLL_1);
+   DUMP_REG(SOR_PLL_2);
+   DUMP_REG(SOR_PLL_3);
+   DUMP_REG(SOR_CSTM);
+   DUMP_REG(SOR_LVDS);
+   DUMP_REG(SOR_CRC_A);
+   DUMP_REG(SOR_CRC_B);
+   DUMP_REG(SOR_BLANK);
+   DUMP_REG(SOR_SEQ_CTL);
+   DUMP_REG(SOR_LANE_SEQ_CTL);
+   DUMP_REG(SOR_SEQ_INST(0x0));
+   DUMP_REG(SOR_SEQ_INST(0x1));
+   DUMP_REG(SOR_SEQ_INST(0x2));
+   DUMP_REG(SOR_SEQ_INST(0x3));
+   DUMP_REG(SOR_SEQ_INST(0x4));
+   DUMP_REG(SOR_SEQ_INST(0x5));
+   DUMP_REG(SOR_SEQ_INST(0x6));
+   DUMP_REG(SOR_SEQ_INST(0x7));
+   DUMP_REG(SOR_SEQ_INST(0x8));
+   DUMP_REG(SOR_SEQ_INST(0x9));
+   DUMP_REG(SOR_SEQ_INST(0xa));
+   DUMP_REG(SOR_SEQ_INST(0xb));
+   DUMP_REG(SOR_SEQ_INST(0xc));
+   DUMP_REG(SOR_SEQ_INST(0xd));
+   DUMP_REG(SOR_SEQ_INST(0xe));
+   DUMP_REG(SOR_SEQ_INST(0xf));
+   DUMP_REG(SOR_PWM_DIV);
+   DUMP_REG(SOR_PWM_CTL);
+   DUMP_REG(SOR_VCRC_A_0);
+   DUMP_REG(SOR_VCRC_A_1);
+   DUMP_REG(SOR_VCRC_B_0);
+   DUMP_REG(SOR_VCRC_B_1);
+   DUMP_REG(SOR_CCRC_A_0);
+   DUMP_REG(SOR_CCRC_A_1);
+   DUMP_REG(SOR_CCRC_B_0);
+   DUMP_REG(SOR_CCRC_B_1);
+   DUMP_REG(SOR_EDATA_A_0);
+   DUMP_REG(SOR_EDATA_A_1);
+   DUMP_REG(SOR_EDATA_B_0);
+   DUMP_REG(SOR_EDATA_B_1);
+   DUMP_REG(SOR_COUNT_A_0);
+   DUMP_REG(SOR_COUNT_A_1);
+   DUMP_REG(SOR_COUNT_B_0);
+   DUMP_REG(SOR_COUNT_B_1);
+   DUMP_REG(SOR_DEBUG_A_0);
+   DUMP_REG(SOR_DEBUG_A_1);
+   DUMP_REG(SOR_DEBUG_B_0);
+   DUMP_REG(SOR_DEBUG_B_1);
+   DUMP_REG(SOR_TRIG);
+   DUMP_REG(SOR_MSCHECK);
+   DUMP_REG(SOR_XBAR_CTRL);
+   DUMP_REG(SOR_XBAR_POL);
+   DUMP_REG(SOR_DP_LINKCTL_0);
+   DUMP_REG(SOR_DP_LINKCTL_1);
+   DUMP_REG(SOR_LANE_DRIVE_CURRENT_0);
+   DUMP_REG(SOR_LANE_DRIVE_CURRENT_1);
+   DUMP_REG(SOR_LANE4_DRIVE_CURRENT_0);
+   DUMP_REG(SOR_LANE4_DRIVE_CURRENT_1);
+   DUMP_REG(SOR_LANE_PREEMPHASIS_0);
+   DUMP_REG(SOR_LANE_PREEMPHASIS_1);
+   DUMP_REG(SOR_LANE4_PREEMPHASIS_0);
+   DUMP_REG(SOR_LANE4_PREEMPHASIS_1);
+   DUMP_REG(SOR_LANE_POST_CURSOR_0);
+   DUMP_REG(SOR_LANE_POST_CURSOR_1);
+   DUMP_REG(SOR_DP_CONFIG_0);
+   DUMP_REG(SOR_DP_CONFIG_1);
+   DUMP_REG(SOR_DP_MN_0);
+   DUMP_REG(SOR_DP_MN_1);
+   DUMP_REG(SOR_DP_PADCTL_0);
+   DUMP_REG(SOR_DP_PADCTL_1);
+   DUMP_REG(SOR_DP_DEBUG_0);
+   DUMP_REG(SOR_DP_DEBUG_1);
+   DUMP_REG(SOR_DP_SPARE_0);
+   DUMP_REG(SOR_DP_SPARE_1);
+   DUMP_REG(SOR_DP_AUDIO_CTRL);
+   DUMP_REG(SOR_DP_AUDIO_HBLANK_SYMBOLS);
+   DUMP_REG(SOR_DP_AUDIO_VBLANK_SYMBOLS);
+   DUMP_REG(SOR_DP_GENERIC_INFOFRAME_HEADER);
+   DUMP_REG(SOR_DP_GENERIC_INFOFRAME_SUBPACK_0);
+   DUMP_REG(SOR_DP_GENERIC_INFOFRAME_SUBPACK_1);
+   DUMP_REG(SOR_DP_GENERIC_INFOFRAME_SUBPACK_2);
+   DUMP_REG(SOR_DP_GENERIC_INFOFRAME_SUBPACK_3);
+   DUMP_REG(SOR_DP_GENERIC_INFOFRAME_SUBP

[REPOST PATCH 1/8] fence: dma-buf cross-device synchronization (v17)

[James Bottomley]

On Thu, 2014-06-19 at 15:39 -0700, H. Peter Anvin wrote:
> On 06/19/2014 01:01 PM, Greg KH wrote:
> > On Thu, Jun 19, 2014 at 09:15:36PM +0200, Daniel Vetter wrote:
> >> On Thu, Jun 19, 2014 at 7:00 PM, Greg KH  
> >> wrote:
> >> + BUG_ON(f1->context != f2->context);
> >
> > Nice, you just crashed the kernel, making it impossible to debug or
> > recover :(
> 
>  agreed, that should probably be 'if (WARN_ON(...)) return NULL;'
> 
>  (but at least I wouldn't expect to hit that under console_lock so you
>  should at least see the last N lines of the backtrace on the screen
>  ;-))
> >>>
> >>> Lots of devices don't have console screens :)
> >>
> >> Aside: This is a pet peeve of mine and recently I've switched to
> >> rejecting all patch that have a BUG_ON, period.
> > 
> > Please do, I have been for a few years now as well for the same reasons
> > you cite.
> > 
> 
> I'm actually concerned about this trend.  Downgrading things to WARN_ON
> can allow a security bug in the kernel to continue to exist, for
> example, or make the error message disappear.

Me too.  We use BUG_ON in the I/O subsystem where we're forced to
violate a guarantee.  When the choice is corrupt something or panic the
system, I prefer the latter every time.

> I am wondering if the right thing here isn't to have a user (command
> line?) settable policy as to how to proceed on an assert violation,
> instead of hardcoding it at compile time.

I'd say it depends on the consequence of the assertion violation.  We
have assertions that are largely theoretical, ones that govern process
internal state (so killing the process mostly sanitizes the system) and
a few that imply data loss or data corruption.

James

[REPOST PATCH 1/8] fence: dma-buf cross-device synchronization (v17)

[Rob Clark]

On Thu, Jun 19, 2014 at 5:50 PM, Dave Airlie  wrote:
> On 20 June 2014 04:19, Greg KH  wrote:
>> On Thu, Jun 19, 2014 at 01:45:30PM -0400, Rob Clark wrote:
>>> On Thu, Jun 19, 2014 at 1:00 PM, Greg KH  
>>> wrote:
>>> > On Thu, Jun 19, 2014 at 10:00:18AM -0400, Rob Clark wrote:
>>> >> On Wed, Jun 18, 2014 at 9:13 PM, Greg KH  
>>> >> wrote:
>>> >> > On Wed, Jun 18, 2014 at 12:36:54PM +0200, Maarten Lankhorst wrote:
>>> >> >> +#define CREATE_TRACE_POINTS
>>> >> >> +#include 
>>> >> >> +
>>> >> >> +EXPORT_TRACEPOINT_SYMBOL(fence_annotate_wait_on);
>>> >> >> +EXPORT_TRACEPOINT_SYMBOL(fence_emit);
>>> >> >
>>> >> > Are you really willing to live with these as tracepoints for forever?
>>> >> > What is the use of them in debugging?  Was it just for debugging the
>>> >> > fence code, or for something else?
>>> >> >
>>> >> >> +/**
>>> >> >> + * fence_context_alloc - allocate an array of fence contexts
>>> >> >> + * @num: [in]amount of contexts to allocate
>>> >> >> + *
>>> >> >> + * This function will return the first index of the number of fences 
>>> >> >> allocated.
>>> >> >> + * The fence context is used for setting fence->context to a unique 
>>> >> >> number.
>>> >> >> + */
>>> >> >> +unsigned fence_context_alloc(unsigned num)
>>> >> >> +{
>>> >> >> + BUG_ON(!num);
>>> >> >> + return atomic_add_return(num, &fence_context_counter) - num;
>>> >> >> +}
>>> >> >> +EXPORT_SYMBOL(fence_context_alloc);
>>> >> >
>>> >> > EXPORT_SYMBOL_GPL()?  Same goes for all of the exports in here.
>>> >> > Traditionally all of the driver core exports have been with this
>>> >> > marking, any objection to making that change here as well?
>>> >>
>>> >> tbh, I prefer EXPORT_SYMBOL()..  well, I'd prefer even more if there
>>> >> wasn't even a need for EXPORT_SYMBOL_GPL(), but sadly it is a fact of
>>> >> life.  We already went through this debate once with dma-buf.  We
>>> >> aren't going to change $evil_vendor's mind about non-gpl modules.  The
>>> >> only result will be a more flugly convoluted solution (ie. use syncpt
>>> >> EXPORT_SYMBOL() on top of fence EXPORT_SYMBOL_GPL()) just as a
>>> >> workaround, with the result that no-one benefits.
>>> >
>>> > It has been proven that using _GPL() exports have caused companies to
>>> > release their code "properly" over the years, so as these really are
>>> > Linux-only apis, please change them to be marked this way, it helps
>>> > everyone out in the end.
>>>
>>> Well, maybe that is the true in some cases.  But it certainly didn't
>>> work out that way for dma-buf.  And I think the end result is worse.
>>>
>>> I don't really like coming down on the side of EXPORT_SYMBOL() instead
>>> of EXPORT_SYMBOL_GPL(), but if we do use EXPORT_SYMBOL_GPL() then the
>>> result will only be creative workarounds using the _GPL symbols
>>> indirectly by whatever is available via EXPORT_SYMBOL().  I don't
>>> really see how that will be better.
>>
>> You are saying that you _know_ companies will violate our license, so
>> you should just "give up"?  And how do you know people aren't working on
>> preventing those "indirect" usages as well?  :)
>>
>> Sorry, I'm not going to give up here, again, it has proven to work in
>> the past in changing the ways of _very_ large companies, why stop now?
>
> I've found large companies shipping lots of hw putting pressure on
> other large/small companies seems to be only way this has ever
> happened, we'd like to cover that up and say its some great GPL
> enforcement thing.
>
> To be honest, author's choice is how I'd treat this.
>
> Personally I think _GPL is broken by design, and that Linus's initial
> point for them has been so diluted by random lobby groups asking for
> every symbol to be _GPL that they are becoming effectively pointless
> now. I also dislike the fact that the lobby groups don't just bring
> violators to court. I'm also sure someone like the LF could have a
> nice income stream if Linus gave them permission to enforce his
> copyrights.
>
> But anyways, has someone checked that iOS or Windows don't have a
> fence interface? so we know that this is a Linux only interface and
> any works using it are derived? Say the nvidia driver isn't a derived
> work now, will using this interface magically translate it into a
> derived work, so we can go sue them? I don't think so.

I've no ideas about what the APIs are in windows, but windows has had
multi-gpu support for a *long* time, which implies some mechanism like
dmabuf and fence.. this isn't exactly an area where we are
trailblazing here.

BR,
-R


> But its up to Maarten and Rob, and if they say no _GPL then I don't
> think we should be overriding authors intents.
>
> Dave.

[REPOST PATCH 1/8] fence: dma-buf cross-device synchronization (v17)

[H. Peter Anvin]

On 06/19/2014 01:01 PM, Greg KH wrote:
> On Thu, Jun 19, 2014 at 09:15:36PM +0200, Daniel Vetter wrote:
>> On Thu, Jun 19, 2014 at 7:00 PM, Greg KH  
>> wrote:
>> + BUG_ON(f1->context != f2->context);
>
> Nice, you just crashed the kernel, making it impossible to debug or
> recover :(

 agreed, that should probably be 'if (WARN_ON(...)) return NULL;'

 (but at least I wouldn't expect to hit that under console_lock so you
 should at least see the last N lines of the backtrace on the screen
 ;-))
>>>
>>> Lots of devices don't have console screens :)
>>
>> Aside: This is a pet peeve of mine and recently I've switched to
>> rejecting all patch that have a BUG_ON, period.
> 
> Please do, I have been for a few years now as well for the same reasons
> you cite.
> 

I'm actually concerned about this trend.  Downgrading things to WARN_ON
can allow a security bug in the kernel to continue to exist, for
example, or make the error message disappear.

I am wondering if the right thing here isn't to have a user (command
line?) settable policy as to how to proceed on an assert violation,
instead of hardcoding it at compile time.

-hpa

[REPOST PATCH 1/8] fence: dma-buf cross-device synchronization (v17)

[Greg KH]

On Thu, Jun 19, 2014 at 03:39:47PM -0700, H. Peter Anvin wrote:
> On 06/19/2014 01:01 PM, Greg KH wrote:
> > On Thu, Jun 19, 2014 at 09:15:36PM +0200, Daniel Vetter wrote:
> >> On Thu, Jun 19, 2014 at 7:00 PM, Greg KH  
> >> wrote:
> >> + BUG_ON(f1->context != f2->context);
> >
> > Nice, you just crashed the kernel, making it impossible to debug or
> > recover :(
> 
>  agreed, that should probably be 'if (WARN_ON(...)) return NULL;'
> 
>  (but at least I wouldn't expect to hit that under console_lock so you
>  should at least see the last N lines of the backtrace on the screen
>  ;-))
> >>>
> >>> Lots of devices don't have console screens :)
> >>
> >> Aside: This is a pet peeve of mine and recently I've switched to
> >> rejecting all patch that have a BUG_ON, period.
> > 
> > Please do, I have been for a few years now as well for the same reasons
> > you cite.
> > 
> 
> I'm actually concerned about this trend.  Downgrading things to WARN_ON
> can allow a security bug in the kernel to continue to exist, for
> example, or make the error message disappear.

A BUG_ON makes any error message disappear pretty quickly :)

I'm talking about foolish "ASSERT-like" BUG_ON that driver authors like
to add to their code when writing it to catch things they are messing
up.  After the code is working, they should be removed, like this one.

Don't enforce an api requirement with a kernel crash, warn and return an
error which the caller should always be checking anyway.

thanks,

greg k-h

[Intel-gfx] [PATCH 10/9] drm: Add dev->vblank_disable_immediate flag

[Matt Roper]

On Mon, May 26, 2014 at 05:26:47PM +0300, ville.syrjala at linux.intel.com 
wrote:
> From: Ville Syrj?l? 
> 
> Add a flag to drm_device which will cause the vblank code to bypass the
> disable timer and always disable the vblank interrupt immediately when
> the last reference is dropped.
> 
> Signed-off-by: Ville Syrj?l? 
> ---
>  drivers/gpu/drm/drm_irq.c |  6 +++---
>  include/drm/drmP.h| 10 ++
>  2 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 20a4855..b008803 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -994,11 +994,11 @@ void drm_vblank_put(struct drm_device *dev, int crtc)
>  
>   /* Last user schedules interrupt disable */
>   if (atomic_dec_and_test(&vblank->refcount)) {
> - if (drm_vblank_offdelay > 0)
> + if (dev->vblank_disable_immediate || drm_vblank_offdelay == 0)
> + vblank_disable_fn((unsigned long)vblank);
> + else if (drm_vblank_offdelay > 0)
>   mod_timer(&vblank->disable_timer,
> jiffies + ((drm_vblank_offdelay * HZ)/1000));
> - else if (drm_vblank_offdelay == 0)
> - vblank_disable_fn((unsigned long)vblank);
>   }
>  }
>  EXPORT_SYMBOL(drm_vblank_put);

Would it be better if we just squashed this device flag logic back into
patch 7, but kept the drm_vblank_offdelay == 0 meaning of "never
disable?"  I can see there being people who might already use this when
debugging new and potentially flaky hardware platforms who would be
surprised by the change in behavior.  So basically something like:

if (drm_vblank_offdelay == 0)
return
else if (dev->vblank_disable_immediate)
vblank_disable_fn((unsigned long)vblank);
else
mod_timer(...);


I'd also suggest adding a comment in drm_stub.c to indicate that
drm_vblank_offdelay gets ignored for drivers that set
vblank_disable_immediate.

Other than that, patches 1-8, 10, and 11 are
Reviewed-by: Matt Roper 

I'll finish up reviewing #9 and 12-14 tomorrow when I have some more
time.



Matt


> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 979a498..0944544 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -1117,6 +1117,16 @@ struct drm_device {
>*/
>   bool vblank_disable_allowed;
>  
> + /*
> +  * If true, vblank interrupt will be disabled immediately when the
> +  * refcount drops to zero, as opposed to via the vblank disable
> +  * timer.
> +  * This can be set to true it the hardware has a working vblank
> +  * counter and the driver uses drm_vblank_on() and drm_vblank_off()
> +  * appropriately.
> +  */
> + bool vblank_disable_immediate;
> +
>   /* array of size num_crtcs */
>   struct drm_vblank_crtc *vblank;
>  
> -- 
> 1.8.5.5
> 
> ___
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795

[PATCH 1/3] drm/tegra: sor - Add register debugging support

[Stéphane Marchesin]

This adds a debugfs entry to print the register state. This can be
pretty useful when debugging eDP link issues.

Signed-off-by: St?phane Marchesin 
---
 drivers/gpu/drm/tegra/sor.c | 159 +++-
 1 file changed, 158 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index 27c979b..925f955 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -37,6 +37,7 @@ struct tegra_sor {
struct mutex lock;
bool enabled;

+   struct drm_info_list *debugfs_files;
struct dentry *debugfs;
 };

@@ -1238,11 +1239,145 @@ static const struct file_operations tegra_sor_crc_fops 
= {
.release = tegra_sor_crc_release,
 };

+static int tegra_sor_show_regs(struct seq_file *s, void *data)
+{
+   struct drm_info_node *node = s->private;
+   struct tegra_sor *sor = node->info_ent->data;
+
+#define DUMP_REG(name) \
+   seq_printf(s, "%-40s %#05x %08lx\n", #name, name,   \
+  tegra_sor_readl(sor, name))
+
+   DUMP_REG(SOR_CTXSW);
+   DUMP_REG(SOR_SUPER_STATE_0);
+   DUMP_REG(SOR_SUPER_STATE_1);
+   DUMP_REG(SOR_STATE_0);
+   DUMP_REG(SOR_STATE_1);
+   DUMP_REG(SOR_HEAD_STATE_0(0));
+   DUMP_REG(SOR_HEAD_STATE_0(1));
+   DUMP_REG(SOR_HEAD_STATE_1(0));
+   DUMP_REG(SOR_HEAD_STATE_1(1));
+   DUMP_REG(SOR_HEAD_STATE_2(0));
+   DUMP_REG(SOR_HEAD_STATE_2(1));
+   DUMP_REG(SOR_HEAD_STATE_3(0));
+   DUMP_REG(SOR_HEAD_STATE_3(1));
+   DUMP_REG(SOR_HEAD_STATE_4(0));
+   DUMP_REG(SOR_HEAD_STATE_4(1));
+   DUMP_REG(SOR_HEAD_STATE_5(0));
+   DUMP_REG(SOR_HEAD_STATE_5(1));
+   DUMP_REG(SOR_CRC_CNTRL);
+   DUMP_REG(SOR_DP_DEBUG_MVID);
+   DUMP_REG(SOR_CLK_CNTRL);
+   DUMP_REG(SOR_CAP);
+   DUMP_REG(SOR_PWR);
+   DUMP_REG(SOR_TEST);
+   DUMP_REG(SOR_PLL_0);
+   DUMP_REG(SOR_PLL_1);
+   DUMP_REG(SOR_PLL_2);
+   DUMP_REG(SOR_PLL_3);
+   DUMP_REG(SOR_CSTM);
+   DUMP_REG(SOR_LVDS);
+   DUMP_REG(SOR_CRC_A);
+   DUMP_REG(SOR_CRC_B);
+   DUMP_REG(SOR_BLANK);
+   DUMP_REG(SOR_SEQ_CTL);
+   DUMP_REG(SOR_LANE_SEQ_CTL);
+   DUMP_REG(SOR_SEQ_INST(0x0));
+   DUMP_REG(SOR_SEQ_INST(0x1));
+   DUMP_REG(SOR_SEQ_INST(0x2));
+   DUMP_REG(SOR_SEQ_INST(0x3));
+   DUMP_REG(SOR_SEQ_INST(0x4));
+   DUMP_REG(SOR_SEQ_INST(0x5));
+   DUMP_REG(SOR_SEQ_INST(0x6));
+   DUMP_REG(SOR_SEQ_INST(0x7));
+   DUMP_REG(SOR_SEQ_INST(0x8));
+   DUMP_REG(SOR_SEQ_INST(0x9));
+   DUMP_REG(SOR_SEQ_INST(0xa));
+   DUMP_REG(SOR_SEQ_INST(0xb));
+   DUMP_REG(SOR_SEQ_INST(0xc));
+   DUMP_REG(SOR_SEQ_INST(0xd));
+   DUMP_REG(SOR_SEQ_INST(0xe));
+   DUMP_REG(SOR_SEQ_INST(0xf));
+   DUMP_REG(SOR_PWM_DIV);
+   DUMP_REG(SOR_PWM_CTL);
+   DUMP_REG(SOR_VCRC_A_0);
+   DUMP_REG(SOR_VCRC_A_1);
+   DUMP_REG(SOR_VCRC_B_0);
+   DUMP_REG(SOR_VCRC_B_1);
+   DUMP_REG(SOR_CCRC_A_0);
+   DUMP_REG(SOR_CCRC_A_1);
+   DUMP_REG(SOR_CCRC_B_0);
+   DUMP_REG(SOR_CCRC_B_1);
+   DUMP_REG(SOR_EDATA_A_0);
+   DUMP_REG(SOR_EDATA_A_1);
+   DUMP_REG(SOR_EDATA_B_0);
+   DUMP_REG(SOR_EDATA_B_1);
+   DUMP_REG(SOR_COUNT_A_0);
+   DUMP_REG(SOR_COUNT_A_1);
+   DUMP_REG(SOR_COUNT_B_0);
+   DUMP_REG(SOR_COUNT_B_1);
+   DUMP_REG(SOR_DEBUG_A_0);
+   DUMP_REG(SOR_DEBUG_A_1);
+   DUMP_REG(SOR_DEBUG_B_0);
+   DUMP_REG(SOR_DEBUG_B_1);
+   DUMP_REG(SOR_TRIG);
+   DUMP_REG(SOR_MSCHECK);
+   DUMP_REG(SOR_XBAR_CTRL);
+   DUMP_REG(SOR_XBAR_POL);
+   DUMP_REG(SOR_DP_LINKCTL_0);
+   DUMP_REG(SOR_DP_LINKCTL_1);
+   DUMP_REG(SOR_LANE_DRIVE_CURRENT_0);
+   DUMP_REG(SOR_LANE_DRIVE_CURRENT_1);
+   DUMP_REG(SOR_LANE4_DRIVE_CURRENT_0);
+   DUMP_REG(SOR_LANE4_DRIVE_CURRENT_1);
+   DUMP_REG(SOR_LANE_PREEMPHASIS_0);
+   DUMP_REG(SOR_LANE_PREEMPHASIS_1);
+   DUMP_REG(SOR_LANE4_PREEMPHASIS_0);
+   DUMP_REG(SOR_LANE4_PREEMPHASIS_1);
+   DUMP_REG(SOR_LANE_POST_CURSOR_0);
+   DUMP_REG(SOR_LANE_POST_CURSOR_1);
+   DUMP_REG(SOR_DP_CONFIG_0);
+   DUMP_REG(SOR_DP_CONFIG_1);
+   DUMP_REG(SOR_DP_MN_0);
+   DUMP_REG(SOR_DP_MN_1);
+   DUMP_REG(SOR_DP_PADCTL_0);
+   DUMP_REG(SOR_DP_PADCTL_1);
+   DUMP_REG(SOR_DP_DEBUG_0);
+   DUMP_REG(SOR_DP_DEBUG_1);
+   DUMP_REG(SOR_DP_SPARE_0);
+   DUMP_REG(SOR_DP_SPARE_1);
+   DUMP_REG(SOR_DP_AUDIO_CTRL);
+   DUMP_REG(SOR_DP_AUDIO_HBLANK_SYMBOLS);
+   DUMP_REG(SOR_DP_AUDIO_VBLANK_SYMBOLS);
+   DUMP_REG(SOR_DP_GENERIC_INFOFRAME_HEADER);
+   DUMP_REG(SOR_DP_GENERIC_INFOFRAME_SUBPACK_0);
+   DUMP_REG(SOR_DP_GENERIC_INFOFRAME_SUBPACK_1);
+   DUMP_REG(SOR_DP_GENERIC_INFOFRAME_SUBPACK_2);
+   DUMP_REG(SOR_DP_GENERIC_INFOFRAME_SUBPACK_3);
+   DUMP_REG(SOR_DP_GENERIC_INFOFRAME_SUBPACK_4);
+   DUMP_REG(SOR_DP_

[PATCH 2/3] drm/panel: panel-simple - Add support for bpc

[Stéphane Marchesin]

bpc is provided by the EDID normally, but if we're using drm_panel,
we need to store it somewhere. So we add a drm_panel entry for it.

Signed-off-by: St?phane Marchesin 
---
 drivers/gpu/drm/panel/panel-simple.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index a251361..1f5fa46 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -37,6 +37,8 @@ struct panel_desc {
const struct drm_display_mode *modes;
unsigned int num_modes;

+   unsigned int bpc;
+
struct {
unsigned int width;
unsigned int height;
@@ -87,6 +89,7 @@ static int panel_simple_get_fixed_modes(struct panel_simple 
*panel)
num++;
}

+   connector->display_info.bpc = panel->desc->bpc;
connector->display_info.width_mm = panel->desc->size.width;
connector->display_info.height_mm = panel->desc->size.height;

@@ -285,6 +288,7 @@ static const struct drm_display_mode auo_b101aw03_mode = {
 static const struct panel_desc auo_b101aw03 = {
.modes = &auo_b101aw03_mode,
.num_modes = 1,
+   .bpc = 6,
.size = {
.width = 223,
.height = 125,
@@ -307,6 +311,7 @@ static const struct drm_display_mode auo_b133xtn01_mode = {
 static const struct panel_desc auo_b133xtn01 = {
.modes = &auo_b133xtn01_mode,
.num_modes = 1,
+   .bpc = 6,
.size = {
.width = 293,
.height = 165,
@@ -329,6 +334,7 @@ static const struct drm_display_mode 
chunghwa_claa101wa01a_mode = {
 static const struct panel_desc chunghwa_claa101wa01a = {
.modes = &chunghwa_claa101wa01a_mode,
.num_modes = 1,
+   .bpc = 6,
.size = {
.width = 220,
.height = 120,
@@ -351,6 +357,7 @@ static const struct drm_display_mode 
chunghwa_claa101wb01_mode = {
 static const struct panel_desc chunghwa_claa101wb01 = {
.modes = &chunghwa_claa101wb01_mode,
.num_modes = 1,
+   .bpc = 6,
.size = {
.width = 223,
.height = 125,
@@ -419,6 +426,7 @@ static const struct drm_display_mode lg_lp129qe_mode = {
 static const struct panel_desc lg_lp129qe = {
.modes = &lg_lp129qe_mode,
.num_modes = 1,
+   .bpc = 8,
.size = {
.width = 272,
.height = 181,
@@ -441,6 +449,7 @@ static const struct drm_display_mode 
samsung_ltn101nt05_mode = {
 static const struct panel_desc samsung_ltn101nt05 = {
.modes = &samsung_ltn101nt05_mode,
.num_modes = 1,
+   .bpc = 6,
.size = {
.width = 1024,
.height = 600,
-- 
2.0.0.526.g5318336

[PATCH 3/3] drm/tegra: sor - Use bpc value from drm_panel

[Stéphane Marchesin]

This change uses the bpc value coming from drm_panel to remove one
more hardcoded value.

Signed-off-by: St?phane Marchesin 
---
 drivers/gpu/drm/tegra/sor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index 925f955..f02d9b6 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -526,7 +526,7 @@ static int tegra_output_sor_enable(struct tegra_output 
*output)
dev_err(sor->dev, "failed to set safe parent clock: %d\n", err);

memset(&config, 0, sizeof(config));
-   config.bits_per_pixel = 24; /* XXX: don't hardcode? */
+   config.bits_per_pixel = output->connector.display_info.bpc * 3;

err = tegra_sor_calc_config(sor, mode, &config, &link);
if (err < 0)
-- 
2.0.0.526.g5318336

[PATCH v2 7/7] ARM: at91/dt: enable the LCD panel on sama5d3xek boards

[Bo Shen]

Hi Boris,

On 06/10/2014 12:04 AM, Boris BREZILLON wrote:
> diff --git a/arch/arm/boot/dts/sama5d33ek.dts 
> b/arch/arm/boot/dts/sama5d33ek.dts
> index cbd6a3f..f2ab41d 100644
> --- a/arch/arm/boot/dts/sama5d33ek.dts
> +++ b/arch/arm/boot/dts/sama5d33ek.dts
> @@ -36,9 +36,33 @@
>   macb0: ethernet at f0028000 {
>   status = "okay";
>   };
> +
> + hlcdc: hlcdc at f003 {
> + status = "okay";
> +
> + hlcdc-display-controller {
> + atmel,panel = <&panel 3 0>;

One question here, in the driver code, it will configuration the frame 
buffer mode depends on this parameter.
So, my question is if the framebuffer bits per pixel is different with 
output bits per pixel, how to setting it?

For example, frame buffer use 16 bits/pixel, while output 24 bits/pixel.

> + };
> + };
>   };
>   };
>

Best Regards,
Bo Shen

[PATCH v2 4/7] ARM: at91/dt: split sama5d3 lcd pin definitions to match RGB mode configs

[Boris BREZILLON]

On 19/06/2014 09:07, Bo Shen wrote:
> Hi Boris,
>
> On 06/10/2014 12:04 AM, Boris BREZILLON wrote:
>> The HLCDC (High LCD Controller) IP supports 4 different output mode
>> (RGB444, RGB565, RGB666 and RGB888) and the pin muxing depends on the
>> chosen RGB mode.
>>
>> Split the pin definition to be able to set the pin config according
>> to the
>> selected mode.
>>
>> Signed-off-by: Boris BREZILLON 
>> ---
>>   arch/arm/boot/dts/sama5d3_lcd.dtsi | 127
>> -
>>   1 file changed, 96 insertions(+), 31 deletions(-)
>
> On sama5d3xek board, it only works in 24bits output mode. And it
> depends on the hardware design. So, I think only keep only one pinctrl
> configuration.

As you pointed out (during our discussion on IRC) sama5d3 SoCs support
several pin muxing for LCDDAT pins.
I'll take care to define all these pin mux here and select the
appropriate ones in sama5d3xdm.dtsi (patch 6) instead of
sama5d3_lcd.dtsi (patch 5).


Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[REPOST PATCH 4/8] android: convert sync to fence api, v5

[Colin Cross]

On Thu, Jun 19, 2014 at 5:28 AM, Daniel Vetter  wrote:
> On Thu, Jun 19, 2014 at 1:48 PM, Thierry Reding
>  wrote:
>>> > With these changes, can we pull the android sync logic out of
>>> > drivers/staging/ now?
>>>
>>> Afaik the google guys never really looked at this and acked it. So I'm not
>>> sure whether they'll follow along. The other issue I have as the
>>> maintainer of gfx driver is that I don't want to implement support for two
>>> different sync object primitives (once for dma-buf and once for android
>>> syncpts), and my impression thus far has been that even with this we're
>>> not there.
>>>
>>> I'm trying to get our own android guys to upstream their i915 syncpts
>>> support, but thus far I haven't managed to convince them to throw people's
>>> time at this.
>>
>> This has been discussed a fair bit internally recently and some of our
>> GPU experts have raised concerns that this may result in seriously
>> degraded performance in our proprietary graphics stack. Now I don't care
>> very much for the proprietary graphics stack, but by extension I would
>> assume that the same restrictions are relevant for any open-source
>> driver as well.
>>
>> I'm still trying to fully understand all the implications and at the
>> same time get some of the people who raised concerns to join in this
>> discussion. As I understand it the concern is mostly about explicit vs.
>> implicit synchronization and having this mechanism in the kernel will
>> implicitly synchronize all accesses to these buffers even in cases where
>> it's not needed (read vs. write locks, etc.). In one particular instance
>> it was even mentioned that this kind of implicit synchronization can
>> lead to deadlocks in some use-cases (this was mentioned for Android
>> compositing, but I suspect that the same may happen for Wayland or X
>> compositors).
>
> Well the implicit fences here actually can't deadlock. That's the
> entire point behind using ww mutexes. I've also heard tons of
> complaints about implicit enforced syncing (especially from opencl
> people), but in the end drivers and always expose unsynchronized
> access for specific cases. We do that in i915 for upload buffers and
> other fun stuff. This is about shared stuff across different drivers
> and different processes.
>
> I also expect that i915 will loose implicit syncing in a few upcoming
> hw modes because explicit syncing is a more natural fit there.
>
> All that isn't about the discussion at hand imo since no matter what
> i915 needs to have on internal representation for a bit of gpu work,
> and afaics right now we don't have that. With this patch android
> syncpts use Maarten's fences internally, but I can't freely exchange
> one for the other. So in i915 I still expect to get stuck with both of
> them, which is one too many.
>
> The other issue (and I haven't dug into details that much) I have with
> syncpts are some of the interface choices. Apparently you can commit a
> fence after creation (or at least the hw composer interface works like
> that) which means userspace can construct deadlocks with android
> syncpts if I'm not super careful in my driver. I haven't seen any
> generic code to do that, so I presume everyone just blindly trusts
> surface-flinger to not do that. Speaks of the average quality of an
> android gfx driver if the kernel is less trusted than the compositor
> in userspace ...

Android sync is designed not to allow userspace to deadlock the
kernel, a sync_pt should only get created by the kernel when it has
received a chunk of work that it expects to complete in the near
future.  The CONFIG_SW_SYNC_USER driver violates that by allowing
userspace to create and signal arbitrary sync points, but that is
intended only for testing sync.

> There's a few other things like exposing timestamps (which are tricky
> to do right, our driver is littered with wrong attempts) and other
> details.

Timestamps are necessary for vsync synchronization to reduce the frame latency.

> Finally I've never seen anyone from google or any android product guy
> push a real driver enabling for syncpts to upstream, and google itself
> has a bit a history of constantly exchanging their gfx framework for
> the next best thing. So I really doubt this is worthwhile to pursue in
> upstream with our essentially eternal api guarantees. At least until
> we see serious uptake from vendors and gfx driver guys. Unfortunately
> the Intel android folks are no exception here and haven't pushed
> anything like this in my direction yet at all. Despite multiple pokes
> from my side.

As far as I know, every SoC vendor that supports android is using sync
now, but none of them have succeeded in pushing their drivers upstream
for a variety of other reasons (interfaces only used by closed source
userspaces, KMS/DRM vs ADF, ION, etc.).

> So from my side I think we should move ahead with Maarten's work and
> figure the android side out once there's real interest.

As long as that doesn't involve r

[PATCH 0/3] Remove devm_request_and_ioremap()

[Wolfram Sang]

Pretty much a year ago, Tushar cleaned up a lot of deprecated uses of
devm_request_and_ioremap, yet some remains are still left. Remove the last two
users, and let the function rest in peace. I'd suggest that this series is
picked up as a whole to have that case finally closed. Greg? Are you interested
in picking it up?

   Wolfram


Tushar Behera (2):
  DRM: Armada: Use devm_ioremap_resource
  lib: devres: Remove deprecated devm_request_and_ioremap

Wolfram Sang (1):
  bus: brcmstb_gisb: Use devm_ioremap_resource

 Documentation/driver-model/devres.txt |  1 -
 drivers/bus/brcmstb_gisb.c|  6 +++---
 drivers/gpu/drm/armada/armada_crtc.c  |  8 +++-
 include/linux/device.h|  2 --
 lib/devres.c  | 28 
 5 files changed, 6 insertions(+), 39 deletions(-)

-- 
2.0.0

[PATCH 1/3] DRM: Armada: Use devm_ioremap_resource

[Wolfram Sang]

From: Tushar Behera 

While at it, propagate the error code.

Signed-off-by: Tushar Behera 
Signed-off-by: Wolfram Sang 
---
 drivers/gpu/drm/armada/armada_crtc.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/armada/armada_crtc.c 
b/drivers/gpu/drm/armada/armada_crtc.c
index 81c34f949dfc..3aedf9e993e6 100644
--- a/drivers/gpu/drm/armada/armada_crtc.c
+++ b/drivers/gpu/drm/armada/armada_crtc.c
@@ -1039,11 +1039,9 @@ int armada_drm_crtc_create(struct drm_device *dev, 
unsigned num,
if (ret)
return ret;

-   base = devm_request_and_ioremap(dev->dev, res);
-   if (!base) {
-   DRM_ERROR("failed to ioremap register\n");
-   return -ENOMEM;
-   }
+   base = devm_ioremap_resource(dev->dev, res);
+   if (IS_ERR(base))
+   return PTR_ERR(base);

dcrtc = kzalloc(sizeof(*dcrtc), GFP_KERNEL);
if (!dcrtc) {
-- 
2.0.0

[PATCH 0/3] Remove devm_request_and_ioremap()

[Wolfram Sang]

On Thu, Jun 19, 2014 at 08:48:58PM +0200, Wolfram Sang wrote:
> Pretty much a year ago, Tushar cleaned up a lot of deprecated uses of
> devm_request_and_ioremap, yet some remains are still left. Remove the last two
> users, and let the function rest in peace. I'd suggest that this series is
> picked up as a whole to have that case finally closed. Greg? Are you 
> interested
> in picking it up?

Note to self: Add Greg to CC list when asking him...

-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140619/d0af9782/attachment-0001.sig>

[PATCH v2 4/7] ARM: at91/dt: split sama5d3 lcd pin definitions to match RGB mode configs

[Boris BREZILLON]

Hello Bo,

On 19/06/2014 09:07, Bo Shen wrote:
> Hi Boris,
>
> On 06/10/2014 12:04 AM, Boris BREZILLON wrote:
>> The HLCDC (High LCD Controller) IP supports 4 different output mode
>> (RGB444, RGB565, RGB666 and RGB888) and the pin muxing depends on the
>> chosen RGB mode.
>>
>> Split the pin definition to be able to set the pin config according
>> to the
>> selected mode.
>>
>> Signed-off-by: Boris BREZILLON 
>> ---
>>   arch/arm/boot/dts/sama5d3_lcd.dtsi | 127
>> -
>>   1 file changed, 96 insertions(+), 31 deletions(-)
>
> On sama5d3xek board, it only works in 24bits output mode. And it
> depends on the hardware design. So, I think only keep only one pinctrl
> configuration.

I'm not describing a specific board design but rather SoC capabilities
(this dtsi is SoC related not board related), and the sama5d3 SoC
supports 4 different RGB output modes through the RGB connector.

If you take a look at patch 7, you'll see that I chose mode 3 (which is
RGB888), and given this mode the HLCDC driver (atmel_hlcdc_panel.c) will
request the appropriate pin state:

atmel,panel = <&panel 3 0>;


Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[RFC][PATCH] gpu:drm:i915:intel_detect_pch: back to check devfn instead of check class type

[Tiejun Chen]

Originally the reason to probe ISA bridge instead of Dev31:Fun0
is to make graphics device passthrough work easy for VMM, that
only need to expose ISA bridge to let driver know the real
hardware underneath. This is a requirement from virtualization
team. Especially in that virtualized environments, XEN, there
is irrelevant ISA bridge in the system with that legacy qemu
version specific to xen, qemu-xen-traditional. So to work
reliably, we should scan through all the ISA bridge devices
and check for the first match, instead of only checking the
first one.

But actually, qemu-xen-traditional, is always enumerated with
Dev31:Fun0, 00:1f.0 as follows:

hw/pt-graphics.c:

intel_pch_init()
|
+ pci_isa_bridge_init(bus, PCI_DEVFN(0x1f, 0), ...);

so this mean that isa bridge is still represented with Dev31:Func0
like the native OS. Furthermore, currently we're pushing VGA
passthrough support into qemu upstream, and with some discussion,
we wouldn't set the bridge class type and just expose this devfn.

So we just go back to check devfn to make life normal.

Signed-off-by: Tiejun Chen 
---
 drivers/gpu/drm/i915/i915_drv.c | 19 +++
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 651e65e..cb2526e 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -417,18 +417,8 @@ void intel_detect_pch(struct drm_device *dev)
return;
}

-   /*
-* The reason to probe ISA bridge instead of Dev31:Fun0 is to
-* make graphics device passthrough work easy for VMM, that only
-* need to expose ISA bridge to let driver know the real hardware
-* underneath. This is a requirement from virtualization team.
-*
-* In some virtualized environments (e.g. XEN), there is irrelevant
-* ISA bridge in the system. To work reliably, we should scan trhough
-* all the ISA bridge devices and check for the first match, instead
-* of only checking the first one.
-*/
-   while ((pch = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, pch))) {
+   pch = pci_get_bus_and_slot(0, PCI_DEVFN(0x1f, 0));
+   if (pch) {
if (pch->vendor == PCI_VENDOR_ID_INTEL) {
unsigned short id = pch->device & 
INTEL_PCH_DEVICE_ID_MASK;
dev_priv->pch_id = id;
@@ -462,10 +452,7 @@ void intel_detect_pch(struct drm_device *dev)
DRM_DEBUG_KMS("Found LynxPoint LP PCH\n");
WARN_ON(!IS_HASWELL(dev));
WARN_ON(!IS_ULT(dev));
-   } else
-   continue;
-
-   break;
+   }
}
}
if (!pch)
-- 
1.9.1

[PATCH] drivers/i915: Fix unnoticed failure of init_ring_common()

[Konrad Zapalowicz]

On 06/19, Daniel Vetter wrote:
> On Thu, Jun 19, 2014 at 12:38 AM, Konrad Zapalowicz
>  wrote:
> > This commit add check for return value of init_ring_common() in the
> > init_render_ring(). Now, when failure is detected the error code is
> > propagated to the caller layer instead of being ignored.
> >
> > I believe that this fix will have a positive impact on the oops that
> > I hit recently and which starts when init_ring_common() fails:
> >
> > [drm:init_ring_common] *ERROR* render ring initialization failed
> >  ctl 0001f001 head 000c tail  start 003eb000
> > BUG: unable to handle kernel NULL pointer dereference at 006c
> > IP: [] i915_gem_obj_to_ggtt+0x9/0x40 [i915]
> >
> > Signed-off-by: Konrad Zapalowicz 
> 
> Do you have the full Oops somewhere?

Here you go, the Oops plus some usefull data:
  1. Oops
  2. lspci -vv
  3. uname -a
  4. Oops analysis

1. The Oops:

Jun 17 21:06:11 t400 kernel: [   12.136049] [drm:init_ring_common] *ERROR* 
render ring initialization failed ctl 0001f001 head 000c tail  
start 003eb000
Jun 17 21:06:11 t400 kernel: [   12.136081] BUG: unable to handle kernel NULL 
pointer dereference at 006c
Jun 17 21:06:11 t400 kernel: [   12.136086] IP: [] 
i915_gem_obj_to_ggtt+0x9/0x40 [i915]
Jun 17 21:06:11 t400 kernel: [   12.136118] *pdpt = 33158001 *pde = 
 
Jun 17 21:06:11 t400 kernel: [   12.136123] Oops:  [#1] SMP 
Jun 17 21:06:11 t400 kernel: [   12.136127] Modules linked in: mac80211(E) 
i915(E+) snd_hda_codec_conexant(E) snd_hda_codec_generic(E) snd_hda_intel(E) 
snd_hda_controller(E) intel_gtt(E) snd_hda_codec(E) iwlwifi(E) i2c_algo_bit(E) 
snd_hwdep(E) uvcvideo(E) thinkpad_acpi(E) drm_kms_helper(E) snd_pcm(E) 
pcmcia(E) nvram(E) videobuf2_vmalloc(E) videobuf2_memops(E) videobuf2_core(E) 
drm(E) psmouse(E) videodev(E) snd_seq_midi(E) snd_seq_midi_event(E) 
snd_rawmidi(E) snd_seq(E) cfg80211(E) yenta_socket(E) snd_timer(E) 
pcmcia_rsrc(E) serio_raw(E) snd_seq_device(E) pcmcia_core(E) snd(E) pl2303(E) 
lpc_ich(E) ppdev(E) usb_storage(E) soundcore(E) usbserial(E) wmi(E) video(E) 
tpm_tis(E) parport_pc(E) lp(E) parport(E) firewire_ohci(E) firewire_core(E) 
crc_itu_t(E) ahci(E) libahci(E) e1000e(E) ptp(E) pps_core(E)
Jun 17 21:06:11 t400 kernel: [   12.136187] CPU: 1 PID: 570 Comm: modprobe 
Tainted: GE 3.15.0 #6
Jun 17 21:06:11 t400 kernel: [   12.136191] Hardware name: LENOVO 
6475FA4/6475FA4, BIOS 7UET79WW (3.09 ) 10/13/2009
Jun 17 21:06:11 t400 kernel: [   12.136195] task: f3141b60 ti: f316a000 
task.ti: f316a000
Jun 17 21:06:11 t400 kernel: [   12.136199] EIP: 0060:[] EFLAGS: 
00010282 CPU: 1
Jun 17 21:06:11 t400 kernel: [   12.136223] EIP is at 
i915_gem_obj_to_ggtt+0x9/0x40 [i915]
Jun 17 21:06:11 t400 kernel: [   12.136227] EAX:  EBX: 0004 ECX: 
f2e6c000 EDX: fffb
Jun 17 21:06:11 t400 kernel: [   12.136230] ESI: f2e6d174 EDI:  EBP: 
f316bb50 ESP: f316bb4c
Jun 17 21:06:11 t400 kernel: [   12.136234]  DS: 007b ES: 007b FS: 00d8 GS: 
0033 SS: 0068
Jun 17 21:06:11 t400 kernel: [   12.136239] CR0: 8005003b CR2: 006c CR3: 
33157000 CR4: 000407f0
Jun 17 21:06:11 t400 kernel: [   12.136243] Stack:
Jun 17 21:06:11 t400 kernel: [   12.136245]  0004 f316bb68 f8ca16c5 
f316bb80 0004 f2e6d174 f2e794c0 f316bb80
Jun 17 21:06:11 t400 kernel: [   12.136254]  f8c9473e f2e6c000 f2e6c000 
f32e5800 fffb f316bba0 f8c9ea49 
Jun 17 21:06:11 t400 kernel: [   12.136263]   f32e5838 f32e5800 
 f2e6c000 f316bca4 f8cf63a4 f8cf3c70
Jun 17 21:06:11 t400 kernel: [   12.136271] Call Trace:
Jun 17 21:06:11 t400 kernel: [   12.136297]  [] 
i915_gem_object_ggtt_unpin+0x15/0x90 [i915]
Jun 17 21:06:11 t400 kernel: [   12.136323]  [] 
i915_gem_context_fini+0x7e/0x130 [i915]
Jun 17 21:06:11 t400 kernel: [   12.136349]  [] 
i915_gem_init+0x69/0x1a0 [i915]
Jun 17 21:06:11 t400 kernel: [   12.136381]  [] 
i915_driver_load+0xa54/0xe50 [i915]
Jun 17 21:06:11 t400 kernel: [   12.136411]  [] ? 
i915_dma_init+0x2e0/0x2e0 [i915]
Jun 17 21:06:11 t400 kernel: [   12.136419]  [] ? 
kobject_uevent_env+0xfa/0x510
Jun 17 21:06:11 t400 kernel: [   12.136424]  [] ? 
kobject_uevent_env+0xfa/0x510
Jun 17 21:06:11 t400 kernel: [   12.136429]  [] ? 
add_uevent_var+0xd0/0xd0
Jun 17 21:06:11 t400 kernel: [   12.136435]  [] ? get_device+0x14/0x30
Jun 17 21:06:11 t400 kernel: [   12.136440]  [] ? 
klist_class_dev_get+0x12/0x20
Jun 17 21:06:11 t400 kernel: [   12.136447]  [] ? 
klist_node_init+0x35/0x50
Jun 17 21:06:11 t400 kernel: [   12.136452]  [] ? 
klist_add_tail+0x20/0x50
Jun 17 21:06:11 t400 kernel: [   12.136457]  [] ? put_device+0x14/0x20
Jun 17 21:06:11 t400 kernel: [   12.136462]  [] ? 
device_add+0x167/0x530
Jun 17 21:06:11 t400 kernel: [   12.136468]  [] ? 
kobject_set_name_vargs+0x42/0x60
Jun 17 21:06:11 t400 kernel: [   12.136485]  [] 
drm_dev_register+0x9e/0xf0 [drm]
Jun 17 21:06:11 t400 kernel: [   12.136499]  [] 
drm_get_pci_dev+0x6f/0x1e0 [drm]
Jun 17 21:06:11 

[RFC] drm/exynos: abort commit when framebuffer is removed from plane

[Rahul Sharma]

This situation arises when userspace remove the frambuffer object
and call setmode ioctl.

drm_mode_rmfb --> drm_plane_force_disable --> plane->crtc = NULL;
and
drm_mode_setcrtc --> exynos_plane_commit --> passes plane->crtc to
exynos_drm_crtc_plane_commit which is NULL.

This crashes the system.

Signed-off-by: Rahul Sharma 
---
This works fine but I am not confident on the correctness of the
solution.

 drivers/gpu/drm/exynos/exynos_drm_crtc.c |6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c 
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 95c9435..da4efe4 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -165,6 +165,12 @@ static int exynos_drm_crtc_mode_set_commit(struct drm_crtc 
*crtc, int x, int y,
return -EPERM;
}

+   /* when framebuffer is removed, commit should not proceed. */
+   if(!plane->fb){
+   DRM_ERROR("framebuffer has been removed from plane.\n");
+   return -EFAULT;
+   }
+
crtc_w = crtc->primary->fb->width - x;
crtc_h = crtc->primary->fb->height - y;

-- 
1.7.9.5

[REPOST PATCH 4/8] android: convert sync to fence api, v5

[Colin Cross]

On Wed, Jun 18, 2014 at 11:37 PM, Daniel Vetter  wrote:
> On Wed, Jun 18, 2014 at 06:15:56PM -0700, Greg KH wrote:
>> On Wed, Jun 18, 2014 at 12:37:11PM +0200, Maarten Lankhorst wrote:
>> > Just to show it's easy.
>> >
>> > Android syncpoints can be mapped to a timeline. This removes the need
>> > to maintain a separate api for synchronization. I've left the android
>> > trace events in place, but the core fence events should already be
>> > sufficient for debugging.
>> >
>> > v2:
>> > - Call fence_remove_callback in sync_fence_free if not all fences have 
>> > fired.
>> > v3:
>> > - Merge Colin Cross' bugfixes, and the android fence merge optimization.
>> > v4:
>> > - Merge with the upstream fixes.
>> > v5:
>> > - Fix small style issues pointed out by Thomas Hellstrom.
>> >
>> > Signed-off-by: Maarten Lankhorst 
>> > Acked-by: John Stultz 
>> > ---
>> >  drivers/staging/android/Kconfig  |1
>> >  drivers/staging/android/Makefile |2
>> >  drivers/staging/android/sw_sync.c|6
>> >  drivers/staging/android/sync.c   |  913 
>> > +++---
>> >  drivers/staging/android/sync.h   |   79 ++-
>> >  drivers/staging/android/sync_debug.c |  247 +
>> >  drivers/staging/android/trace/sync.h |   12
>> >  7 files changed, 609 insertions(+), 651 deletions(-)
>> >  create mode 100644 drivers/staging/android/sync_debug.c
>>
>> With these changes, can we pull the android sync logic out of
>> drivers/staging/ now?
>
> Afaik the google guys never really looked at this and acked it. So I'm not
> sure whether they'll follow along. The other issue I have as the
> maintainer of gfx driver is that I don't want to implement support for two
> different sync object primitives (once for dma-buf and once for android
> syncpts), and my impression thus far has been that even with this we're
> not there.

We have tested these patches to use dma fences to back the android
sync driver and not found any major issues.  However, my understanding
is that dma fences are designed for implicit sync, and explicit sync
through the android sync driver is bolted on the side to share code.
Android is not moving away from explicit sync, but we do wrap all of
our userspace sync accesses through libsync
(https://android.googlesource.com/platform/system/core/+/master/libsync/sync.c,
ignore the sw_sync parts), so if the kernel supported a slightly
different userspace explicit sync interface we could adapt to it
fairly easily.  All we require is that individual kernel drivers need
to be able to accept work alongisde an fd to wait on, and to return an
fd that will signal when the work is done, and that userspace has some
way to merge two of those fds, wait on an fd, and get some debugging
info from an fd.  However, this patch set doesn't do that, it has no
way to export a dma fence as an fd except through the android sync
driver, so it is not yet ready to fully replace android sync.

> I'm trying to get our own android guys to upstream their i915 syncpts
> support, but thus far I haven't managed to convince them to throw people's
> time at this.
>
> It looks like a step into the right direction, but until I have the proof
> in the form of i915 patches that I won't have to support 2 gfx fencing
> frameworks I'm opposed to de-staging android syncpts. Ofc someone else
> could do that too, but besides i915 I don't see a full-fledged (modeset
> side only kinda doesn't count) upstream gfx driver shipping on android.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH libdrm 1/2] intel: Sync the command parser version parameter from kernel

[Volkin, Bradley D]

On Thu, Jun 19, 2014 at 11:31:53AM +0100, Damien Lespiau wrote:
> Cc: Bradley Volkin 
> Signed-off-by: Damien Lespiau 

Thanks for taking care of this Damien.

Reviewed-by: Brad Volkin 

> ---
>  include/drm/i915_drm.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
> index 2f4eb8c..980dbf8 100644
> --- a/include/drm/i915_drm.h
> +++ b/include/drm/i915_drm.h
> @@ -337,6 +337,7 @@ typedef struct drm_i915_irq_wait {
>  #define I915_PARAM_HAS_EXEC_NO_RELOC  25
>  #define I915_PARAM_HAS_EXEC_HANDLE_LUT   26
>  #define I915_PARAM_HAS_WT 27
> +#define I915_PARAM_CMD_PARSER_VERSION 28
>  
>  typedef struct drm_i915_getparam {
>   int param;
> -- 
> 1.8.3.1
> 

[PATCH v2] drivers/i915: Fix unnoticed failure of init_ring_common()

[Konrad Zapalowicz]

This commit add check for return value of init_ring_common() in the
init_render_ring(). Now, when failure is detected the error code is
propagated to the caller instead of being ignored.

Signed-off-by: Konrad Zapalowicz 
---

 v2:
   - remove from commit message references to the Oops.

 drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 279488a..d205b0d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -604,6 +604,8 @@ static int init_render_ring(struct intel_engine_cs *ring)
struct drm_device *dev = ring->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
int ret = init_ring_common(ring);
+   if (ret)
+   return ret;

/* WaTimedSingleVertexDispatch:cl,bw,ctg,elk,ilk,snb */
if (INTEL_INFO(dev)->gen >= 4 && INTEL_INFO(dev)->gen < 7)
-- 
1.8.1.2

[PATCH] drm/msm: Implement msm drm fb_mmap callback function

[Stephen Boyd]

On 06/18/14 13:55, Hai Li wrote:
> diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c
> index 4f4e7b4..2522f51 100644
> --- a/drivers/gpu/drm/msm/msm_fbdev.c
> +++ b/drivers/gpu/drm/msm/msm_fbdev.c
> @@ -19,6 +19,11 @@
>  
>  #include "drm_crtc.h"
>  #include "drm_fb_helper.h"
> +#include "msm_gem.h"
> +
> +extern int msm_gem_mmap_obj(struct drm_gem_object *obj,
> + struct vm_area_struct *vma);

This can't go into some header file?

> +static int msm_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma);
>  
>  /*
>   * fbdev funcs, to implement legacy fbdev interface on top of drm driver
> @@ -43,6 +48,7 @@ static struct fb_ops msm_fb_ops = {
>   .fb_fillrect = sys_fillrect,
>   .fb_copyarea = sys_copyarea,
>   .fb_imageblit = sys_imageblit,
> + .fb_mmap = msm_fbdev_mmap,
>  
>   .fb_check_var = drm_fb_helper_check_var,
>   .fb_set_par = drm_fb_helper_set_par,
> @@ -51,6 +57,31 @@ static struct fb_ops msm_fb_ops = {
>   .fb_setcmap = drm_fb_helper_setcmap,
>  };
>  
> +static int msm_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma)
> +{
> + struct drm_fb_helper *helper = (struct drm_fb_helper *)info->par;
> + struct msm_fbdev *fbdev = to_msm_fbdev(helper);
> + struct drm_gem_object *drm_obj = fbdev->bo;
> + struct drm_device *dev = helper->dev;
> + int ret = 0;

unnecessary initialization to 0.

> +
> + if (drm_device_is_unplugged(dev))
> + return -ENODEV;
> +
> + mutex_lock(&dev->struct_mutex);
> +
> + ret = drm_gem_mmap_obj(drm_obj, drm_obj->size, vma);
> +
> + mutex_unlock(&dev->struct_mutex);
> +
> + if (ret) {
> + pr_err("%s:drm_gem_mmap_obj fail\n", __func__);
> + return ret;
> + }
> +
> + return msm_gem_mmap_obj(drm_obj, vma);
> +}
> +
>  static int msm_fbdev_create(struct drm_fb_helper *helper,
>   struct drm_fb_helper_surface_size *sizes)
>  {
> @@ -108,7 +139,11 @@ static int msm_fbdev_create(struct drm_fb_helper *helper,
>   mutex_lock(&dev->struct_mutex);
>  
>   /* TODO implement our own fb_mmap so we don't need this: */

Is this comment still relevant?

> - msm_gem_get_iova_locked(fbdev->bo, 0, &paddr);
> + ret = msm_gem_get_iova_locked(fbdev->bo, 0, &paddr);
> + if (ret) {
> + dev_err(dev->dev, "failed to get buffer obj iova: %d\n", ret);
> + goto fail;
> + }
>  
>   fbi = framebuffer_alloc(0, dev->dev);
>   if (!fbi) {


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCH v2 4/7] ARM: at91/dt: split sama5d3 lcd pin definitions to match RGB mode configs

[Bo Shen]

Hi Boris,

On 06/10/2014 12:04 AM, Boris BREZILLON wrote:
> The HLCDC (High LCD Controller) IP supports 4 different output mode
> (RGB444, RGB565, RGB666 and RGB888) and the pin muxing depends on the
> chosen RGB mode.
>
> Split the pin definition to be able to set the pin config according to the
> selected mode.
>
> Signed-off-by: Boris BREZILLON 
> ---
>   arch/arm/boot/dts/sama5d3_lcd.dtsi | 127 
> -
>   1 file changed, 96 insertions(+), 31 deletions(-)

On sama5d3xek board, it only works in 24bits output mode. And it depends 
on the hardware design. So, I think only keep only one pinctrl 
configuration.

Best Regards,
Bo Shen

> diff --git a/arch/arm/boot/dts/sama5d3_lcd.dtsi 
> b/arch/arm/boot/dts/sama5d3_lcd.dtsi
> index 85d3027..2186b89 100644
> --- a/arch/arm/boot/dts/sama5d3_lcd.dtsi
> +++ b/arch/arm/boot/dts/sama5d3_lcd.dtsi
> @@ -15,38 +15,103 @@
>   apb {
>   pinctrl at f200 {
>   lcd {
> - pinctrl_lcd: lcd-0 {
> + pinctrl_lcd_pwm: lcd-pwm-0 {
> + atmel,pins =  AT91_PERIPH_A AT91_PINCTRL_NONE>;/* LCDPWM */
> + };
> +
> + pinctrl_lcd_base: lcd-base-0 {
> + atmel,pins =
> +  AT91_PERIPH_A AT91_PINCTRL_NONE   /* LCDVSYNC */
> +  AT91_PIOA 27 
> AT91_PERIPH_A AT91_PINCTRL_NONE   /* LCDHSYNC */
> +  AT91_PIOA 25 
> AT91_PERIPH_A AT91_PINCTRL_NONE   /* LCDDISP */
> +  AT91_PIOA 29 
> AT91_PERIPH_A AT91_PINCTRL_NONE   /* LCDDEN */
> +  AT91_PIOA 28 
> AT91_PERIPH_A AT91_PINCTRL_NONE>; /* LCDPCK */
> + };
> +
> + pinctrl_lcd_rgb444: lcd-rgb-0 {
> + atmel,pins =
> +  AT91_PERIPH_A AT91_PINCTRL_NONE/* LCDD0 pin */
> +  AT91_PIOA 1 
> AT91_PERIPH_A AT91_PINCTRL_NONE/* LCDD1 pin */
> +  AT91_PIOA 2 
> AT91_PERIPH_A AT91_PINCTRL_NONE/* LCDD2 pin */
> +  AT91_PIOA 3 
> AT91_PERIPH_A AT91_PINCTRL_NONE/* LCDD3 pin */
> +  AT91_PIOA 4 
> AT91_PERIPH_A AT91_PINCTRL_NONE/* LCDD4 pin */
> +  AT91_PIOA 5 
> AT91_PERIPH_A AT91_PINCTRL_NONE/* LCDD5 pin */
> +  AT91_PIOA 6 
> AT91_PERIPH_A AT91_PINCTRL_NONE/* LCDD6 pin */
> +  AT91_PIOA 7 
> AT91_PERIPH_A AT91_PINCTRL_NONE/* LCDD7 pin */
> +  AT91_PIOA 8 
> AT91_PERIPH_A AT91_PINCTRL_NONE/* LCDD8 pin */
> +  AT91_PIOA 9 
> AT91_PERIPH_A AT91_PINCTRL_NONE/* LCDD9 pin */
> +  AT91_PIOA 10 
> AT91_PERIPH_A AT91_PINCTRL_NONE   /* LCDD10 pin */
> +  AT91_PIOA 11 
> AT91_PERIPH_A AT91_PINCTRL_NONE>; /* LCDD11 pin */
> + };
> +
> + pinctrl_lcd_rgb565: lcd-rgb-1 {
> + atmel,pins =
> +  AT91_PERIPH_A AT91_PINCTRL_NONE/* LCDD0 pin */
> +  AT91_PIOA 1 
> AT91_PERIPH_A AT91_PINCTRL_NONE/* LCDD1 pin */
> +  AT91_PIOA 2 
> AT91_PERIPH_A AT91_PINCTRL_NONE/* LCDD2 pin */
> +  AT91_PIOA 3 
> AT91_PERIPH_A AT91_PINCTRL_NONE/* LCDD3 pin */
> +  AT91_PIOA 4 
> AT91_PERIPH_A AT91_PINCTRL_NONE/* LCDD4 pin */
> +  AT91_PIOA 5 
> AT91_PERIPH_A AT91_PINCTRL_NONE/* LCDD5 pin */
> +  AT91_PIOA 6 
> AT91_PERIPH_A AT91_PINCTRL_NONE/* LCDD6 pin */
> +  AT91_PIOA 7 
> AT91_PERIPH_A AT91_PINCTRL_NONE/* LCDD7 pin */
> +  AT91_PIOA 8 
> AT91_PERIPH_A AT91_PINCTRL_NONE/* LCDD8 pin */
> +  AT91_

[PATCH v2 7/7] ARM: at91/dt: enable the LCD panel on sama5d3xek boards

[Boris BREZILLON]

On 19/06/2014 09:12, Bo Shen wrote:
> Hi Boris,
>
> On 06/10/2014 12:04 AM, Boris BREZILLON wrote:
>> diff --git a/arch/arm/boot/dts/sama5d33ek.dts
>> b/arch/arm/boot/dts/sama5d33ek.dts
>> index cbd6a3f..f2ab41d 100644
>> --- a/arch/arm/boot/dts/sama5d33ek.dts
>> +++ b/arch/arm/boot/dts/sama5d33ek.dts
>> @@ -36,9 +36,33 @@
>>   macb0: ethernet at f0028000 {
>>   status = "okay";
>>   };
>> +
>> +hlcdc: hlcdc at f003 {
>> +status = "okay";
>> +
>> +hlcdc-display-controller {
>> +atmel,panel = <&panel 3 0>;
>
> One question here, in the driver code, it will configuration the frame
> buffer mode depends on this parameter.
> So, my question is if the framebuffer bits per pixel is different with
> output bits per pixel, how to setting it?
>
> For example, frame buffer use 16 bits/pixel, while output 24 bits/pixel.

Actually the HLCDC is responsible for converting input format (either
RGB or YUV) to output format (one of the four supported RGB formats).

AFAICT, the HLCDC always converts the input format in RGB888 and then
only use the relevant bits (i.e. if the output is RGB565, it will only
takes MSB for each color).

The REP field (available in all layer, e.g. LCDC_BASECFG4 for the base
layer) is here to tell how the HLCDC should expand to 24 bits format.

All this means that we don't have to bother about input to output format
conversion.

>
>> +};
>> +};
>>   };
>>   };
>>
>
> Best Regards,
> Bo Shen

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[REPOST PATCH 1/8] fence: dma-buf cross-device synchronization (v17)

[Eric Boxer]

Eric Boxer liked your message with Boxer. On June 19, 2014 at 12:45:30 PM CDT, 
Rob Clark  wrote: 
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140619/bac88f05/attachment.html>

[PATCH] drivers/i915: Fix unnoticed failure of init_ring_common()

[Konrad Zapalowicz]

On 06/19, Daniel Vetter wrote:
> On Thu, Jun 19, 2014 at 4:35 PM, Daniel Vetter  
> wrote:
> > The actual bug we seem to have is blowing up on the ggtt_unpin in
> > context_fini. Which is doubly-impossible: Gen4 doesn't have hw
> > contexts, so should have dctx->obj == NULL. And ring init failures
> > fail earlier so shouldn't even hit the context_fini code below the
> > cleanup_gem: label in driver_load. Seriously confused here.
> 
> Also please retest with latest upstream, we've made the ring init
> failure non-letal for the driver. That should help you, too.
> -Daniel

Thanks for comments and I will resubmit the patch, still it is better
to have it.

/Konrad

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

linux-next: Tree for Jun 19 (drm/i915)

[Randy Dunlap]

On 06/18/14 23:16, Stephen Rothwell wrote:
> Hi all,
> 
> The powerpc allyesconfig is again broken more than usual.
> 
> Changes since 20140618:
> 

on i386:

CONFIG_ACPI is not enabled.

  CC  drivers/gpu/drm/i915/i915_drv.o
../drivers/gpu/drm/i915/i915_drv.c: In function 'i915_drm_freeze':
../drivers/gpu/drm/i915/i915_drv.c:547:2: error: implicit declaration of 
function 'acpi_target_system_state' [-Werror=implicit-function-declaration]
../drivers/gpu/drm/i915/i915_drv.c:547:36: error: 'ACPI_STATE_S3' undeclared 
(first use in this function)
../drivers/gpu/drm/i915/i915_drv.c:547:36: note: each undeclared identifier is 
reported only once for each function it appears in
  CC  net/dccp/qpolicy.o
cc1: some warnings being treated as errors
make[5]: *** [drivers/gpu/drm/i915/i915_drv.o] Error 1



-- 
~Randy

[PATCH 0/3] Remove devm_request_and_ioremap()

['Greg Kroah-Hartman']

On Fri, Jun 20, 2014 at 11:36:03AM +0900, Jingoo Han wrote:
> On Friday, June 20, 2014 3:49 AM, Wolfram Sang wrote:
> > 
> > Pretty much a year ago, Tushar cleaned up a lot of deprecated uses of
> > devm_request_and_ioremap, yet some remains are still left. Remove the last 
> > two
> > users, and let the function rest in peace. I'd suggest that this series is
> > picked up as a whole to have that case finally closed. Greg? Are you 
> > interested
> > in picking it up?
> 
> (+cc Greg Kroah-Hartman)
> 
> I already sent the same patch as one single patch to Greg Kroah-Hartman. [1]
> Also, it was accepted by Greg Kroah-Hartman. [2] Thank you.
> 
> [1] https://lkml.org/lkml/2014/6/11/26
> [2] https://lkml.org/lkml/2014/6/11/649

Yeah, I'll go apply that right now while I'm remembering it :)

thanks,

greg k-h