[Bug 88925] GPU lockup in XCOM with R270X

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

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

Alex Deucher  changed:

   What|Removed |Added

  Component|DRM/Radeon  |Drivers/Gallium/radeonsi
Version|DRI git |git
Product|DRI |Mesa
 QA Contact||dri-devel at lists.freedesktop
   ||.org

-- 
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/20150203/afc697bb/attachment.html>

[Linaro-mm-sig] [RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms

[Daniel Vetter]

On Tue, Feb 3, 2015 at 10:42 PM, Arnd Bergmann  wrote:
>> Again assuming I'm not confused can't we just solve this by pushing the
>> dma api abstraction down one layer for just the gpu, and let it use its
>> private iommmu directly? Steps for binding a buffer would be:
>> 1. dma_map_sg
>> 2. Noodle the dma_addr_t out of the sg table and feed those into a 2nd
>> level mapping set up through the iommu api for the gpu-private mmu.
>
> If you want to do that, you run into the problem of telling the driver
> core about it. We associate the device with an iommu in the device
> tree, describing there how it is wired up.
>
> The driver core creates a platform_device for this and checks if it
> an iommu mapping is required or wanted for the device, which is then
> set up. When the device driver wants to create its own iommu mapping,
> this conflicts with the one that is already there. We can't just
> skip the iommu setup for all devices because it may be needed sometimes,
> and I don't really want to see hacks where the driver core knows which
> devices are GPUs and skips the mapping for them, which would be a
> layering violation.

I don't think you get a choice but to make gpus a special case.
There's a bunch of cases why the iommu private to the gpu is special:
- If there's gpu-private iommu at all you have a nice security
problem, and you must scan your cmd stream to make sure no gpu access
goes to arbitrary system memory. We kinda consider isolation between
clients optional, but isolation to everything else is mandatory. And
scanning the cmd stream in software has such big implications on the
design of your driver that you essentially need 2 different drivers.
Even if the IP block otherwise matches.
- If your iommu supports multiple address space then the gpu must
know. We've already covered this case.

So trying to wedge the dma api between the gpu and its private iommu
is imo the layering violation here. Imo the dma api only should
control an iommu for the gpu if:
- the iommu is shared (so can't be used for isolation and you need the
full blwon cmd scanner)
- it's a 2nd level iommu (e.g. what we have on i915) and there is
another private iommu.

Note that with private I only mean no other device can use it, I don't
mean whether it's on the same IP block or not (we even have an iommu
abstraction in i915 because the pagetable walkers are pretty much
separate from everything else and evolve mostly independently).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[Linaro-mm-sig] [RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms

[Arnd Bergmann]

On Tuesday 03 February 2015 12:35:34 Rob Clark wrote:
> 
> I can't think of cases outside of GPU's..  if it were more common I'd
> be in favor of teaching dma api about multiple contexts, but right now
> I think that would just amount to forcing a lot of churn on everyone
> else for the benefit of GPU's.

We have a couple of users of the iommu API at the moment outside of
GPUs:

* drivers/media/platform/omap3isp/isp.c
* drivers/remoteproc/remoteproc_core.c
* drivers/infiniband/hw/usnic/usnic_uiom.c
* drivers/vfio/

I assume we will see a few more over time. The vfio case is the most
important one here, since that is what the iommu API was designed for.

Arnd

[Linaro-mm-sig] [RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms

[Arnd Bergmann]

On Tuesday 03 February 2015 21:04:35 Daniel Vetter wrote:

> - On many soc people love to reuse iommus with the same or similar
>   interface all over the place. The solution thus far adopted on arm
>   platforms is to write an iommu driver for those and then implement the
>   dma-api on top of this iommu.
> 
>   But if we unconditionally do this then we rob the gpu driver's ability
>   to control its private iommu like it wants to, because a lot of the
>   functionality is lost behind the dma api abstraction.

I believe in case of rockchips, msm, exynos and tegra, the same
iommu is used directly by the DRM driver while it is used 
implicitly by the dma-mapping API. We have run into some problems
with this in 3.19, but they should be solvable.

> Again assuming I'm not confused can't we just solve this by pushing the
> dma api abstraction down one layer for just the gpu, and let it use its
> private iommmu directly? Steps for binding a buffer would be:
> 1. dma_map_sg
> 2. Noodle the dma_addr_t out of the sg table and feed those into a 2nd
> level mapping set up through the iommu api for the gpu-private mmu.

If you want to do that, you run into the problem of telling the driver
core about it. We associate the device with an iommu in the device
tree, describing there how it is wired up.

The driver core creates a platform_device for this and checks if it
an iommu mapping is required or wanted for the device, which is then
set up. When the device driver wants to create its own iommu mapping,
this conflicts with the one that is already there. We can't just
skip the iommu setup for all devices because it may be needed sometimes,
and I don't really want to see hacks where the driver core knows which
devices are GPUs and skips the mapping for them, which would be a
layering violation.

> Again, this is what i915 and all the ttm based drivers already do, except
> that we don't use the generic iommu interfaces but have our own (i915 has
> its interface in i915_gem_gtt.c, ttm just calls them tt for translation
> tables ...).

Right, if you have a private iommu, there is no problem. The tricky part
is using a single driver for the system-level translation and the gpu
private mappings when there is only one type of iommu in the system.

Arnd

[Bug 88925] GPU lockup in XCOM with R270X

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

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

--- Comment #4 from dex+fdobugzilla at dragonslave.de ---
(In reply to dex+fdobugzilla from comment #3)
> (In reply to Michel Dänzer from comment #2)
> > Sounds like bug 80419.
> 
> But the game worked fine until about start of the year. I think I
> may still have the packaged versions of llvm and mesa from then and perhaps
> I can confirm this.

With:
* Mesa 10.5.0-devel (git-6171131)
* LLVM 225080
* Kernel: 3.19.0-rc7-13532-g1b7a4a0 (same)

No lockups

So this is either a LLVM or Mesa Issue.

I will now compile latest Mesa with LLVM 225080 and see if that works.

Given that this is hardly a DRI/Radeon issue.. where should I move this bug?

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

[regression in linux-next] i915: broken graphics on laptop

[Andrey Skvortsov]

Hi,

tested next-20150202. System boots, but graphic output is broken (empty black 
screen).
Booted five times the same kernel, always got the same result. The system works 
with 3.19-rc7.

This is the first warning in the log: 

 WARNING: CPU: 0 PID: 855 at drivers/gpu/drm/i915/intel_uncore.c:169 
intel_uncore_forcewake_reset+0x188/0x24d [i915]()
 WARN_ON(dev_priv->uncore.fw_domains == 0)
 Modules linked in: i915(E+) lpc_ich(E) mfd_core(E) snd_hda_controller(E) 
snd_hda_codec(E) snd_hwdep(E) snd_pcm(E) snd_timer(E) drm_kms_helper(E) drm(E) 
battery(E) button(E) video(E) ac(E) snd(E) soundcore(E) i2c_algo_bit(E) 
i2c_core(E) acpi_cpufreq(E) processor(E) fuse(E) parport_pc(E) ppdev(E) lp(E) 
parport(E) autofs4(E) ext4(E) crc16(E) jbd2(E) mbcache(E) sd_mod(E) 
ata_generic(E) ahci(E) libahci(E) ata_piix(E) libata(E) scsi_mod(E) 
sdhci_pci(E) firewire_ohci(E) sdhci(E) b44(E) firewire_core(E) crc_itu_t(E) 
mii(E) ssb(E) mmc_core(E) libphy(E) ehci_pci(E) thermal(E) thermal_sys(E) 
uhci_hcd(E) ehci_hcd(E) usbcore(E) usb_common(E)
 CPU: 0 PID: 855 Comm: systemd-udevd Tainted: GE   
3.19.0-rc6-next-20150202-150201- #4
 Hardware name: Dell Inc. Vostro 1500 /0NX907, BIOS A06 
04/21/2008
   0009 813e790a 8800da083958
  8104178e a0601100 a0589bc4 8800da083988
  8800da6c00c8 8800da6c00c8  0246
 Call Trace:
  [] ? dump_stack+0x4a/0x74
  [] ? warn_slowpath_common+0x9d/0xb5
  [] ? intel_uncore_forcewake_reset+0x188/0x24d [i915]
  [] ? warn_slowpath_fmt+0x4a/0x4f
  [] ? intel_uncore_forcewake_reset+0x188/0x24d [i915]
  [] ? intel_uncore_init+0x1e4/0x4a8 [i915]
  [] ? i915_driver_load+0x58f/0xeda [i915]
  [] ? kobject_uevent_env+0x581/0x5d8
  [] ? kfree+0xa4/0x127
  [] ? kobject_uevent_env+0x581/0x5d8
  [] ? devtmpfs_create_node+0x102/0x117
  [] ? preempt_count_sub+0xab/0xca
  [] ? preempt_count_sub+0xab/0xca
  [] ? drm_dev_register+0x79/0xec [drm]
  [] ? drm_get_pci_dev+0xfc/0x1b7 [drm]
  [] ? pci_device_probe+0x74/0xd1
  [] ? driver_probe_device+0x2ff/0x2ff
  [] ? driver_probe_device+0x11c/0x2ff
  [] ? driver_probe_device+0x2ff/0x2ff
  [] ? __driver_attach+0x58/0x78
  [] ? bus_for_each_dev+0x53/0x84
  [] ? bus_add_driver+0x113/0x1f8
  [] ? driver_register+0x87/0xba
  [] ? 0xa0627000
  [] ? do_one_initcall+0xf7/0x18e
  [] ? kmem_cache_alloc_trace+0xd6/0xe8
  [] ? load_module+0x1c81/0x202e
  [] ? load_module+0x1cf9/0x202e
  [] ? mod_kobject_put+0x48/0x48
  [] ? copy_module_from_fd+0x8c/0xf5
  [] ? SyS_finit_module+0x82/0x9a
  [] ? system_call_fastpath+0x12/0x17


Other warnings are below:

 [   19.253096] WARNING: CPU: 1 PID: 746 at 
drivers/gpu/drm/i915/i915_gem.c:4525 i915_gem_free_object+0x134/0x272 [i915]()
 [   19.253098] WARN_ON(obj->frontbuffer_bits)
 [   19.253126] Modules linked in: snd_hda_intel(E+) i2c_i801(E+) i915(E+) 
lpc_ich(E) mfd_core(E) snd_hda_controller(E) snd_hda_codec(E) snd_hwdep(E) 
snd_pcm(E) snd_timer(E) drm_kms_helper(E) drm(E) battery(E) button(E) video(E) 
ac(E) snd(E) soundcore(E) i2c_algo_bit(E) i2c_core(E) acpi_cpufreq(E) 
processor(E) fuse(E) parport_pc(E) ppdev(E) lp(E) parport(E) autofs4(E) ext4(E) 
crc16(E) jbd2(E) mbcache(E) sd_mod(E) ata_generic(E) ahci(E) libahci(E) 
ata_piix(E) libata(E) scsi_mod(E) sdhci_pci(E) firewire_ohci(E) sdhci(E) b44(E) 
firewire_core(E) crc_itu_t(E) mii(E) ssb(E) mmc_core(E) libphy(E) ehci_pci(E) 
thermal(E) thermal_sys(E) uhci_hcd(E) ehci_hcd(E) usbcore(E) usb_common(E)
 [   19.253129] CPU: 1 PID: 746 Comm: kworker/u4:5 Tainted: GW   E   
3.19.0-rc6-next-20150202-150201- #4
 [   19.253130] Hardware name: Dell Inc. Vostro 1500 
/0NX907, BIOS A06 04/21/2008
 [   19.253135] Workqueue: events_unbound async_run_entry_fn
 [   19.253137]   0009 813e790a 
880037a9b9e8
 [   19.253139]  8104178e 8800da6c a056e22b 
880196dda800
 [   19.253140]  880196e54000 8800da6c 880196e54040 
880196e54040
 [   19.253141] Call Trace:
 [   19.253144]  [] ? dump_stack+0x4a/0x74
 [   19.253147]  [] ? warn_slowpath_common+0x9d/0xb5
 [   19.253173]  [] ? i915_gem_free_object+0x134/0x272 [i915]
 [   19.253176]  [] ? warn_slowpath_fmt+0x4a/0x4f
 [   19.253202]  [] ? i915_vma_unbind+0x18f/0x1cb [i915]
 [   19.253228]  [] ? i915_gem_free_object+0x134/0x272 [i915]
 [   19.253246]  [] ? drm_gem_object_release+0x3b/0x3b [drm]
 [   19.253277]  [] ? kref_sub.constprop.59+0x2f/0x38 [i915]
 [   19.253308]  [] ? 
intel_user_framebuffer_destroy+0x62/0x75 [i915]
 [   19.253321]  [] ? 
drm_framebuffer_unregister_private+0x37/0x37 [drm]
 [   19.25]  [] ? kref_sub.constprop.33+0x2f/0x38 [drm]
 [   19.253346]  [] ? drm_mode_set_config_internal+0xa6/0xd7 
[drm]
 [   19.253355]  [] ? restore_fbdev_mode+0xad/0xc8 
[drm_kms_helper]
 [   19.253361]  [] ? 
drm_fb_helper_restore_fbdev_mode_unlocked+0x24/0x5a [drm_kms_helper]
 [   19.253367]  [] ? 

[PATCH v2 08/12] drm: bridge/dw_hdmi: add audio config interfaces

[Yang Kuankuan]

On 02/02/2015 08:09 AM, Russell King - ARM Linux wrote:
> On Mon, Feb 02, 2015 at 07:32:05AM -0500, Yang Kuankuan wrote:
>> On 02/02/2015 06:53 AM, Russell King - ARM Linux wrote:
>>> On Mon, Feb 02, 2015 at 12:02:50PM +0800, Daniel Kurtz wrote:
 Hi ykk,

 On Sat, Jan 31, 2015 at 10:34 PM, Yang Kuankuan  
 wrote:
> On 01/31/2015 06:48 AM, Russell King - ARM Linux wrote:
>>> +void hdmi_audio_clk_enable(struct dw_hdmi *hdmi)
>>> +{
>>> +   if (hdmi->audio_enable)
>>> +   return;
>>> +
>>> +   mutex_lock(>audio_mutex);
>>> +   hdmi->audio_enable = true;
>>> +   hdmi_modb(hdmi, 0, HDMI_MC_CLKDIS_AUDCLK_DISABLE,
>>> HDMI_MC_CLKDIS);
>>> +   mutex_unlock(>audio_mutex);
>> This is racy.  The test needs to be within the mutex-protected region.
> This function will be called by other driver (dw-hdmi-audio), both modify
> the variable "hdmi->audio_enable", so i add the mutex-protected.
 >From your comment it isn't clear whether you understand what Russell 
 >meant.
 He is say you should do the following:

 {
 mutex_lock(>audio_mutex);

 if (hdmi->audio_enable) {
mutex_unlock(>audio_mutex);
return;
 }
 hdmi->audio_enable = true;
 hdmi_modb(hdmi, 0, HDMI_MC_CLKDIS_AUDCLK_DISABLE, HDMI_MC_CLKDIS);

 mutex_unlock(>audio_mutex);
 }
>>> Yes, however, I prefer this kind of layout:
>>>
>>> mutex_lock(>audio_mutex);
>>> if (!hdmi->audio_enable) {
>>> hdmi->audio_enable = true;
>>> hdmi_modb(hdmi, 0, HDMI_MC_CLKDIS_AUDCLK_DISABLE,
>>>   HDMI_MC_CLKDIS);
>>> }
>>>
>>> mutex_unlock(>audio_mutex);
>>>
>>> but that's a matter of personal opinion.  The important thing is that the
>>> testing and setting of the flag are both within the protected region.
>>>
>>> However, there are other bugs here: what if the audio driver is calling
>>> the sample rate setting function at the same time that a mode switch is
>>> occuring.  We actually need a mutex to protect more than just the
>>> audio_enable flag.
>> Okay, got it.
>>
>> Thanks.  : )
> I've been moving my code closer to what you have posted.  I've split up
> your patches into smaller steps so each change can be evaluated on iMX6.
> So far:
>
> drm: bridge/dw_hdmi: combine hdmi_set_clock_regenerator_n() and 
> hdmi_regenerate_cts()
>
>This patch _just_ combines the two functions without any other changes.
>
> drm: bridge/dw_hdmi: protect n/cts setting with a mutex
>
>This patch _just_ adds a mutex to protect the calls to
>hdmi_set_clk_regenerator(), since we will need to call that from both
>the DRM and audio drivers.
>
> drm: bridge/dw_hdmi: adjust n/cts setting order
>
>This patch changes the order to:
>- write CTS3 CTS_manual = 0
>- write CTS3 N_shift = 0
>- write CTS3 CTS value
>- write CTS2 CTS value
>- write CTS1 CTS value
>- write N3 N value
>- write N2 N value
>- write N1 N value
>which is broadly what you're doing, but without the initial N3 write
>and without CTS_manual=1.  I've asked Freescale whether they can
>clarify what effect adding those would have on their SoCs.
>
> Note: the mutex in my second patch needs to be a spinlock - as it seems
> my new workaround for iMX6 ERR005174 needs to call the CTS/N setting
> functions in an irqs-off region (from the ALSA ->trigger callback.)
> That will need further rework of the CTS/N code to reduce the size of
> the spinlock protected region.
>
> Incidentally, your Synopsis IP indicates that it is the same revision
> as the iMX6's IP revision which suffers from this ERR005174 errata.
> I think you need to check whether this errata applies on your SoC too.
> Seach for "iMX6 ERR005174" in google.
Thank you very much.  : )
I will take this order in next v3.
 By the way, it doesn't matter that the function is called from another 
 driver.
 What matters is that this function can be called concurrently on
 multiple different threads of execution to change the hdmi audio
 enable state.
 >From DRM land, it is called with DRM lock held when enabling/disabling
 hdmi audio (mode_set / DPMS).
 It is also called from audio land, when enabling/disabling audio in
 response to some audio events (userspace ioctls?).  I'm not sure
 exactly how the audio side works, or what locks are involved, but this
 mutex synchronizes calls from these two worlds to ensure that
 "hdmi->audio_enable" field always matches the current (intended)
 status of the hdmi audio clock.  This would be useful, for example, if
 you needed to temporarily disable all HDMI clocks during a mode set,
 and then restore the audio clock to its pre-mode_set state:
>>> There's some rather quirky comments in the driver right now which make
>>> me uneasy about 

[Bug 88951] Resume from sleep fails to restore keyboard and touchpad controls

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

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

Alex Deucher  changed:

   What|Removed |Added

  Component|DRM/Radeon  |Core
Version|DRI git |unspecified
   Assignee|dri-devel at lists.freedesktop |peter.hutterer at who-t.net
   |.org|
Product|DRI |libevdev

--- Comment #4 from Alex Deucher  ---
The radeon driver doesn't have anything to do with the keyboard or touchpad so
I doubt it's related.  I guess this is probably related to the kernel input
drivers or maybe some special platform handling for your system.

-- 
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/20150203/7ba076b8/attachment.html>

[Bug 88951] Resume from sleep fails to restore keyboard and touchpad controls

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

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

--- Comment #3 from Adrian Gabor  ---
Created attachment 113137
  --> https://bugs.freedesktop.org/attachment.cgi?id=113137=edit
xorg.log

-- 
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/20150203/19f48195/attachment.html>

[Bug 88951] Resume from sleep fails to restore keyboard and touchpad controls

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

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

--- Comment #2 from Adrian Gabor  ---
Created attachment 113136
  --> https://bugs.freedesktop.org/attachment.cgi?id=113136=edit
lspci output

-- 
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/20150203/455de842/attachment.html>

[Bug 88951] Resume from sleep fails to restore keyboard and touchpad controls

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

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

--- Comment #1 from Adrian Gabor  ---
Created attachment 113135
  --> https://bugs.freedesktop.org/attachment.cgi?id=113135=edit
dmesg output

-- 
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/20150203/56f1de82/attachment.html>

[Bug 88951] Resume from sleep fails to restore keyboard and touchpad controls

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

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

Bug ID: 88951
   Summary: Resume from sleep fails to restore keyboard and
touchpad controls
   Product: DRI
   Version: DRI git
  Hardware: x86-64 (AMD64)
OS: Linux (All)
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/Radeon
  Assignee: dri-devel at lists.freedesktop.org
  Reporter: adrianovidiugabor at gmail.com

Created attachment 113134
  --> https://bugs.freedesktop.org/attachment.cgi?id=113134=edit
journalctl output

When I wake up my machine from sleep it sometimes happens to lose keyboard and
touchpad controls(no input). It happens frequently with lightdm (about 1 in 3
wake-ups) and not that often with gdm (1 in 10+ wake-ups). This only happens to
the integrated input components, external usb mouse works fine.

The bug is distro agnostic (successfully reproduced it in Ubuntu 10.04 and
latest Arch) while using various versions of kernel (3.13 up to 3.18.5), mesa
(10.3 - mesa-git) and xserver (3.15-3.16).

I attached here the relevant output produced by journalctl (crash.log)

I posted this in the radeon section but I'm not sure at all if it's a radeon
issue. I'm apologise if this turns out to be unrelated to radeon.

-- 
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/20150203/8545bc40/attachment-0001.html>

[Linaro-mm-sig] [RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms

[Daniel Vetter]

On Tue, Feb 03, 2015 at 12:35:34PM -0500, Rob Clark wrote:
> On Tue, Feb 3, 2015 at 11:58 AM, Russell King - ARM Linux
>  wrote:
> >
> > Okay, but switching contexts is not something which the DMA API has
> > any knowledge of (so it can't know which context to associate with
> > which mapping.)  While it knows which device, it has no knowledge
> > (nor is there any way for it to gain knowledge) about contexts.
> >
> > My personal view is that extending the DMA API in this way feels quite
> > dirty - it's a violation of the DMA API design, which is to (a) demark
> > the buffer ownership between CPU and DMA agent, and (b) to translate
> > buffer locations into a cookie which device drivers can use to instruct
> > their device to access that memory.  To see why, consider... that you
> > map a buffer to a device in context A, and then you switch to context B,
> > which means the dma_addr_t given previously is no longer valid.  You
> > then try to unmap it... which is normally done using the (now no longer
> > valid) dma_addr_t.
> >
> > It seems to me that to support this at DMA API level, we would need to
> > completely revamp the DMA API, which IMHO isn't going to be nice.  (It
> > would mean that we end up with three APIs - the original PCI DMA API,
> > the existing DMA API, and some new DMA API.)
> >
> > Do we have any views on how common this feature is?
> >
> 
> I can't think of cases outside of GPU's..  if it were more common I'd
> be in favor of teaching dma api about multiple contexts, but right now
> I think that would just amount to forcing a lot of churn on everyone
> else for the benefit of GPU's.
> 
> IMHO it makes more sense for GPU drivers to bypass the dma api if they
> need to.  Plus, sooner or later, someone will discover that with some
> trick or optimization they can get moar fps, but the extra layer of
> abstraction will just be getting in the way.

See my other reply, but all existing full-blown drivers don't bypass the
dma api. Instead it's just a two-level scheme:
1. First level is dma api. Might or might not contain a system iommu.
2. 2nd level is the gpu-private iommu which is also used for per context
address spaces. Thus far all drivers just rolled their own drivers for
this (it's kinda fused to the chips on x86 hw anyway), but it looks like
using the iommu api gives us a somewhat suitable abstraction for code
sharing.

Imo you need both, otherwise we start leaking stuff like cpu cache
flushing all over the place. Looking at i915 (where the dma api assumes
that everything is coherent, which is kinda not the case) that won't be
pretty. And there's still the issue that you might nest a system iommu
and a 2nd level iommu for per-context pagetables (this is real and what's
going on right now on intel hw).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[Bug 89731] System doesn't boot on muxed IntelHD + HD5650

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

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

--- Comment #3 from Alex Deucher  ---
see also:
https://bugs.freedesktop.org/show_bug.cgi?id=88927

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

[Linaro-mm-sig] [RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms

[Daniel Vetter]

On Tue, Feb 03, 2015 at 05:36:59PM +0100, Arnd Bergmann wrote:
> On Tuesday 03 February 2015 11:22:01 Rob Clark wrote:
> > On Tue, Feb 3, 2015 at 11:12 AM, Arnd Bergmann  wrote:
> > > I agree for the case you are describing here. From what I understood
> > > from Rob was that he is looking at something more like:
> > >
> > > Fig 3
> > > CPU--L1cache--L2cache--Memory--IOMMU-device
> > >
> > > where the IOMMU controls one or more contexts per device, and is
> > > shared across GPU and non-GPU devices. Here, we need to use the
> > > dmap-mapping interface to set up the IO page table for any device
> > > that is unable to address all of system RAM, and we can use it
> > > for purposes like isolation of the devices. There are also cases
> > > where using the IOMMU is not optional.
> > 
> > 
> > Actually, just to clarify, the IOMMU instance is specific to the GPU..
> > not shared with other devices.  Otherwise managing multiple contexts
> > would go quite badly..
> > 
> > But other devices have their own instance of the same IOMMU.. so same
> > driver could be used.
> 
> I think from the driver perspective, I'd view those two cases as
> identical. Not sure if Russell agrees with that.

Imo whether the iommu is private to the device and required for gpu
functionality like context switching or shared across a bunch of devices
is fairly important. Assuming I understand this discussion correctly we
have two different things pulling in opposite directions:

- From a gpu functionality perspective we want to give the gpu driver full
  control over the device-private iommu, pushing it out of the control of
  the dma api. dma_map_sg would just map to whatever bus addresses that
  iommu would need to use for generating access cycles.

  This is the design used by every gpu driver we have in upstream thus far
  (where you always have some on-gpu iommu/pagetable walker thing), on top
  of whatever system iommu that might be there or not (which is then
  managed by the dma apis).

- On many soc people love to reuse iommus with the same or similar
  interface all over the place. The solution thus far adopted on arm
  platforms is to write an iommu driver for those and then implement the
  dma-api on top of this iommu.

  But if we unconditionally do this then we rob the gpu driver's ability
  to control its private iommu like it wants to, because a lot of the
  functionality is lost behind the dma api abstraction.

Again assuming I'm not confused can't we just solve this by pushing the
dma api abstraction down one layer for just the gpu, and let it use its
private iommmu directly? Steps for binding a buffer would be:
1. dma_map_sg
2. Noodle the dma_addr_t out of the sg table and feed those into a 2nd
level mapping set up through the iommu api for the gpu-private mmu.

Again, this is what i915 and all the ttm based drivers already do, except
that we don't use the generic iommu interfaces but have our own (i915 has
its interface in i915_gem_gtt.c, ttm just calls them tt for translation
tables ...).

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

[Bug 88947] "Unknown" display appears with no external display connected

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

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

Chris Wilson  changed:

   What|Removed |Added

  Component|General |Driver/nouveau
Version|DRI git |unspecified
   Assignee|dri-devel at lists.freedesktop |nouveau at 
lists.freedesktop.o
   |.org|rg
Product|DRI |xorg
 QA Contact||xorg-team at lists.x.org

-- 
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/20150203/98cb50dd/attachment.html>

[Intel-gfx] [regression in linux-next] i915: broken graphics on laptop

[Chris Wilson]

On Tue, Feb 03, 2015 at 10:15:47PM +0300, Andrey Skvortsov wrote:
> Hi,
> 
> tested next-20150202. System boots, but graphic output is broken (empty black 
> screen).
> Booted five times the same kernel, always got the same result. The system 
> works with 3.19-rc7.

Those two warnings are more or less symptoms of the black screen (well
the first is just overzealous). More important would be the drm.debug=6
dmesg from boot along with the gdm.log (or equivalent) aned Xorg.0.log
as my guess is that X (or the display server) is crashing.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH] drm: Kconfig: Let all DRM_GEM_CMA_HELPER related macros depend on HAVE_DMA_ATTRS

[Chen Gang S]

On 02/03/2015 06:45 PM, Laurent Pinchart wrote:
> Hi Chen,
> 
> Thank you for the patch.
> 
> On Sunday 01 February 2015 22:08:33 Chen Gang S wrote:
>> DRM_GEM_CMA_HELPER is depend on HAVE_DMA_ATTRS, or it will break the
>> building. The related error (with allmodconfig under xtensa):
>>
>> CC [M]  drivers/gpu/drm/drm_gem_cma_helper.o
>>   drivers/gpu/drm/drm_gem_cma_helper.c: In function 'drm_gem_cma_create':
>>   drivers/gpu/drm/drm_gem_cma_helper.c:110:19: error: implicit declaration
>> of function 'dma_alloc_writecombine'
>> [-Werror=implicit-function-declaration] cma_obj->vaddr =
>> dma_alloc_writecombine(drm->dev, size,
>>  ^
>>   drivers/gpu/drm/drm_gem_cma_helper.c:110:17: warning: assignment makes
>> pointer from integer without a cast [-Wint-conversion] cma_obj->vaddr =
>> dma_alloc_writecombine(drm->dev, size,
>>^
>>   drivers/gpu/drm/drm_gem_cma_helper.c: In function
>> 'drm_gem_cma_free_object': drivers/gpu/drm/drm_gem_cma_helper.c:193:3:
>> error: implicit declaration of function 'dma_free_writecombine'
>> [-Werror=implicit-function-declaration]
>> dma_free_writecombine(gem_obj->dev->dev, cma_obj->base.size,
>>  ^
>>   drivers/gpu/drm/drm_gem_cma_helper.c: In function 'drm_gem_cma_mmap_obj':
>>   drivers/gpu/drm/drm_gem_cma_helper.c:330:8: error: implicit declaration of
>> function 'dma_mmap_writecombine' [-Werror=implicit-function-declaration]
>> ret = dma_mmap_writecombine(cma_obj->base.dev->dev, vma,
>>   ^
>>
>> Signed-off-by: Chen Gang 
>> ---
>>  drivers/gpu/drm/Kconfig | 3 ++-
>>  drivers/gpu/drm/atmel-hlcdc/Kconfig | 2 +-
>>  drivers/gpu/drm/imx/Kconfig | 2 +-
>>  drivers/gpu/drm/rcar-du/Kconfig | 2 +-
>>  drivers/gpu/drm/shmobile/Kconfig| 2 +-
>>  drivers/gpu/drm/sti/Kconfig | 2 +-
>>  drivers/gpu/drm/tilcdc/Kconfig  | 2 +-
>>  7 files changed, 8 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>> index 308c104..151a050 100644
>> --- a/drivers/gpu/drm/Kconfig
>> +++ b/drivers/gpu/drm/Kconfig
>> @@ -62,12 +62,13 @@ config DRM_TTM
>>
>>  config DRM_GEM_CMA_HELPER
>>  bool
>> -depends on DRM
>> +depends on DRM && HAVE_DMA_ATTRS
>>  help
>>Choose this if you need the GEM CMA helper functions
>>
>>  config DRM_KMS_CMA_HELPER
>>  bool
>> +depends on DRM && HAVE_DMA_ATTRS
>>  select DRM_GEM_CMA_HELPER
>>  select DRM_KMS_FB_HELPER
>>  select FB_SYS_FILLRECT
>> diff --git a/drivers/gpu/drm/atmel-hlcdc/Kconfig
>> b/drivers/gpu/drm/atmel-hlcdc/Kconfig index 1a08562..219fc87 100644
>> --- a/drivers/gpu/drm/atmel-hlcdc/Kconfig
>> +++ b/drivers/gpu/drm/atmel-hlcdc/Kconfig
>> @@ -1,6 +1,6 @@
>>  config DRM_ATMEL_HLCDC
>>  tristate "DRM Support for ATMEL HLCDC Display Controller"
>> -depends on DRM && OF && COMMON_CLK && MFD_ATMEL_HLCDC
>> +depends on DRM && OF && COMMON_CLK && MFD_ATMEL_HLCDC && HAVE_DMA_ATTRS
> 
> I don't like having to add the explicit dependency to all users of 
> DRM_GEM_CMA_HELPER and DRM_KMS_CMA_HELPER, but there's no way around this 
> given the select vs. depends issues in Kconfig.
> 
> A better solution in the longer term would be to implement HAVE_DMA_ATTRS 
> support for xtensa (and all the other architectures that miss it), but this 
> patch looks fine to me as an interim solution.
> 
> Acked-by: Laurent Pinchart 
> 

OK, thanks. At present, 15 archs implement HAVE_DMA_ATTRS. I guess, it
is not quite easy to let the left 15 archs support HAVE_DMA_ATTRS, so we
have to accept the current interim solution.


Thanks.
-- 
Open, share, and attitude like air, water, and life which God blessed.

[PATCH v2 10/10] drm/imx: imx-ldb: allow to determine bus format from the connected panel

[Philipp Zabel]

This patch makes the fsl,data-width and fsl,data-mapping device tree
properties optional if a panel is connected to the ldb output port
via the of_graph bindings. The data mapping is determined from the
display_info.bus_format field provided by the panel.

Signed-off-by: Philipp Zabel 
---
 drivers/gpu/drm/imx/imx-ldb.c | 116 +++---
 1 file changed, 63 insertions(+), 53 deletions(-)

diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
index 544282b..abacc8f 100644
--- a/drivers/gpu/drm/imx/imx-ldb.c
+++ b/drivers/gpu/drm/imx/imx-ldb.c
@@ -63,6 +63,7 @@ struct imx_ldb_channel {
int edid_len;
struct drm_display_mode mode;
int mode_valid;
+   int bus_format;
 };

 struct bus_mux {
@@ -96,7 +97,11 @@ static int imx_ldb_connector_get_modes(struct drm_connector 
*connector)

if (imx_ldb_ch->panel && imx_ldb_ch->panel->funcs &&
imx_ldb_ch->panel->funcs->get_modes) {
+   struct drm_display_info *di = >display_info;
+
num_modes = 
imx_ldb_ch->panel->funcs->get_modes(imx_ldb_ch->panel);
+   if (!imx_ldb_ch->bus_format && di->num_bus_formats)
+   imx_ldb_ch->bus_format = di->bus_formats[0];
if (num_modes > 0)
return num_modes;
}
@@ -173,21 +178,33 @@ static void imx_ldb_encoder_prepare(struct drm_encoder 
*encoder)
 {
struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
struct imx_ldb *ldb = imx_ldb_ch->ldb;
+   int dual = ldb->ldb_ctrl & LDB_SPLIT_MODE_EN;
u32 bus_format;

-   switch (imx_ldb_ch->chno) {
-   case 0:
-   bus_format = (ldb->ldb_ctrl & LDB_DATA_WIDTH_CH0_24) ?
-   MEDIA_BUS_FMT_RGB888_1X24 : MEDIA_BUS_FMT_RGB666_1X18;
+   switch (imx_ldb_ch->bus_format) {
+   default:
+   dev_warn(ldb->dev,
+"could not determine data mapping, default to 18-bit 
\"spwg\"\n");
+   /* fallthrough */
+   case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
+   bus_format = MEDIA_BUS_FMT_RGB666_1X18;
break;
-   case 1:
-   bus_format = (ldb->ldb_ctrl & LDB_DATA_WIDTH_CH1_24) ?
-   MEDIA_BUS_FMT_RGB888_1X24 : MEDIA_BUS_FMT_RGB666_1X18;
+   case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
+   bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+   if (imx_ldb_ch->chno == 0 || dual)
+   ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH0_24;
+   if (imx_ldb_ch->chno == 1 || dual)
+   ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH1_24;
break;
-   default:
-   dev_err(ldb->dev, "unable to config di%d panel format\n",
-   imx_ldb_ch->chno);
+   case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA:
bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+   if (imx_ldb_ch->chno == 0 || dual)
+   ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH0_24 |
+LDB_BIT_MAP_CH0_JEIDA;
+   if (imx_ldb_ch->chno == 1 || dual)
+   ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH1_24 |
+LDB_BIT_MAP_CH1_JEIDA;
+   break;
}

imx_drm_set_bus_format(encoder, bus_format);
@@ -426,25 +443,39 @@ enum {
LVDS_BIT_MAP_JEIDA
 };

-static const char * const imx_ldb_bit_mappings[] = {
-   [LVDS_BIT_MAP_SPWG]  = "spwg",
-   [LVDS_BIT_MAP_JEIDA] = "jeida",
+struct imx_ldb_bit_mapping {
+   u32 bus_format;
+   u32 datawidth;
+   const char * const mapping;
 };

-static const int of_get_data_mapping(struct device_node *np)
+static const struct imx_ldb_bit_mapping imx_ldb_bit_mappings[] = {
+   { MEDIA_BUS_FMT_RGB666_1X7X3_SPWG,  18, "spwg" },
+   { MEDIA_BUS_FMT_RGB888_1X7X4_SPWG,  24, "spwg" },
+   { MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA, 24, "jeida" },
+};
+
+static u32 of_get_bus_format(struct device *dev, struct device_node *np)
 {
const char *bm;
+   u32 datawidth = 0;
int ret, i;

ret = of_property_read_string(np, "fsl,data-mapping", );
if (ret < 0)
return ret;

-   for (i = 0; i < ARRAY_SIZE(imx_ldb_bit_mappings); i++)
-   if (!strcasecmp(bm, imx_ldb_bit_mappings[i]))
-   return i;
+   of_property_read_u32(np, "fsl,data-width", );
+
+   for (i = 0; i < ARRAY_SIZE(imx_ldb_bit_mappings); i++) {
+   if (!strcasecmp(bm, imx_ldb_bit_mappings[i].mapping) &&
+   datawidth == imx_ldb_bit_mappings[i].datawidth)
+   return imx_ldb_bit_mappings[i].bus_format;
+   }
+
+   dev_err(dev, "invalid data mapping: %d-bit \"%s\"\n", datawidth, bm);

-   return -EINVAL;
+   return -ENOENT;
 }

 static struct bus_mux imx6q_lvds_mux[2] = {
@@ -481,8 +512,6 @@ static int 

[PATCH v2 09/10] drm/imx: imx-ldb: reset display clock input when disabling LVDS

[Philipp Zabel]

The LDB driver changes the attached display interface's input clock mux
to the LDB_DI clock reference. Change it back again when disabling the
LVDS display. Changing back to the PLL5 for example allows to configure
the same DI for HDMI output later.

Signed-off-by: Philipp Zabel 
---
 drivers/gpu/drm/imx/imx-ldb.c | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
index 4286399..544282b 100644
--- a/drivers/gpu/drm/imx/imx-ldb.c
+++ b/drivers/gpu/drm/imx/imx-ldb.c
@@ -77,6 +77,7 @@ struct imx_ldb {
struct imx_ldb_channel channel[2];
struct clk *clk[2]; /* our own clock */
struct clk *clk_sel[4]; /* parent of display clock */
+   struct clk *clk_parent[4]; /* original parent of clk_sel */
struct clk *clk_pll[2]; /* upstream clock we can adjust */
u32 ldb_ctrl;
const struct bus_mux *lvds_mux;
@@ -287,6 +288,7 @@ static void imx_ldb_encoder_disable(struct drm_encoder 
*encoder)
 {
struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
struct imx_ldb *ldb = imx_ldb_ch->ldb;
+   int mux, ret;

/*
 * imx_ldb_encoder_disable is called by
@@ -314,6 +316,28 @@ static void imx_ldb_encoder_disable(struct drm_encoder 
*encoder)
clk_disable_unprepare(ldb->clk[1]);
}

+   if (ldb->lvds_mux) {
+   const struct bus_mux *lvds_mux = NULL;
+
+   if (imx_ldb_ch == >channel[0])
+   lvds_mux = >lvds_mux[0];
+   else if (imx_ldb_ch == >channel[1])
+   lvds_mux = >lvds_mux[1];
+
+   regmap_read(ldb->regmap, lvds_mux->reg, );
+   mux &= lvds_mux->mask;
+   mux >>= lvds_mux->shift;
+   } else {
+   mux = (imx_ldb_ch == >channel[0]) ? 0 : 1;
+   }
+
+   /* set display clock mux back to original input clock */
+   ret = clk_set_parent(ldb->clk_sel[mux], ldb->clk_parent[mux]);
+   if (ret)
+   dev_err(ldb->dev,
+   "unable to set di%d parent clock to original parent\n",
+   mux);
+
drm_panel_unprepare(imx_ldb_ch->panel);
 }

@@ -499,6 +523,8 @@ static int imx_ldb_bind(struct device *dev, struct device 
*master, void *data)
imx_ldb->clk_sel[i] = NULL;
break;
}
+
+   imx_ldb->clk_parent[i] = clk_get_parent(imx_ldb->clk_sel[i]);
}
if (i == 0)
return ret;
-- 
2.1.4

[PATCH v2 08/10] drm/imx: imx-ldb: add drm_panel support

[Philipp Zabel]

This patch allows to optionally attach the lvds-channel to a panel
supported by a drm_panel driver using of-graph bindings, instead of
supplying the modes via display-timings in the device tree.

This depends on of_graph_get_port_by_id and uses the OF graph to
link the optional DRM panel to the LDB lvds-channel. The output
port number is 1 on devices without the 4-port input multiplexer
(i.MX5) and 4 on devices with the mux (i.MX6).

Before:

ldb {
...

lvds-channel at 0 {
...

display-timings {
native-timing = <>;
timing1: etm0700g0dh6 {
hactive = <800>;
vactive = <480>;
clock-frequency = <3326>;
hsync-len = <128>;
hback-porch = <88>;
hfront-porch = <40>;
vsync-len = <2>;
vback-porch = <33>;
vfront-porch = <10>;
hsync-active = <0>;
vsync-active = <0>;
...
};
};
...
};
};

After:
ldb {
...

lvds-channel at 0 {
...

port at 4 {
reg = <4>;

lvds_out: endpoint {
remote_endpoint = <_in>;
};
};
};
};

panel {
compatible = "edt,etm0700g0dh6", "simple-panel";
...

port {
panel_in: endpoint {
remote-endpoint = <_out>;
};
};
};

[Fixed build error due to missing select on DRM_PANEL --rmk]
Signed-off-by: Russell King 
Signed-off-by: Philipp Zabel 
---
Changes since v1:
 - Fixed output port to be port at 2 on i.MX53.
---
 Documentation/devicetree/bindings/drm/imx/ldb.txt | 51 ---
 drivers/gpu/drm/imx/Kconfig   |  1 +
 drivers/gpu/drm/imx/imx-ldb.c | 48 -
 3 files changed, 82 insertions(+), 18 deletions(-)

diff --git a/Documentation/devicetree/bindings/drm/imx/ldb.txt 
b/Documentation/devicetree/bindings/drm/imx/ldb.txt
index 443bcb6..ee96dc6 100644
--- a/Documentation/devicetree/bindings/drm/imx/ldb.txt
+++ b/Documentation/devicetree/bindings/drm/imx/ldb.txt
@@ -44,23 +44,29 @@ Optional properties:
 LVDS Channel
 

-Each LVDS Channel has to contain a display-timings node that describes the
-video timings for the connected LVDS display. For detailed information, also
-have a look at Documentation/devicetree/bindings/video/display-timing.txt.
+Each LVDS Channel must either be linked to a panel device tree node via an
+of-graph endpoint link on its output port, or it has to contain a node called
+display-timings that describes the video timings for the connected LVDS panel
+manally.

 Required properties:
  - reg : should be <0> or <1>
+ - port: Port nodes with endpoint definitions as defined in
+   Documentation/devicetree/bindings/media/video-interfaces.txt.
+   On i.MX5, the internal two-input-multiplexer is used. Due to limitations
+   in the clock tree, only one input port (port@[0,1]) can be used for each
+   channel (lvds-channel@[0,1], respectively). Here the output port is port at 
2.
+   On i.MX6, there should be four input ports (port@[0-3]) that correspond
+   to the four LVDS multiplexer inputs. Here the output port is port at 4.
+   The output port is optional if display-timings are used instead.
+
+Optional properties (required if display-timings are used):
+ - display-timings : A node that describes the display timings as defined in
+   Documentation/devicetree/bindings/video/display-timing.txt.
  - fsl,data-mapping : should be "spwg" or "jeida"
   This describes how the color bits are laid out in the
   serialized LVDS signal.
  - fsl,data-width : should be <18> or <24>
- - port: A port node with endpoint definitions as defined in
-   Documentation/devicetree/bindings/media/video-interfaces.txt.
-   On i.MX5, the internal two-input-multiplexer is used.
-   Due to hardware limitations, only one port (port@[0,1])
-   can be used for each channel (lvds-channel@[0,1], respectively)
-   On i.MX6, there should be four ports (port@[0-3]) that correspond
-   to the four LVDS multiplexer inputs.

 

[PATCH v2 07/10] drm/imx: consolidate bus format variable names

[Philipp Zabel]

This patch consolidates the different interface_pix_fmt, pixel_fmt, pix_fmt,
and pixfmt variables to a common name "bus_format" wherever they describe the
pixel format on the bus between display controller and encoder hardware.
At the same time, it renames imx_drm_panel_format to imx_drm_set_bus_format.

Signed-off-by: Philipp Zabel 
---
 drivers/gpu/drm/imx/dw_hdmi-imx.c  |  2 +-
 drivers/gpu/drm/imx/imx-drm-core.c | 14 +++---
 drivers/gpu/drm/imx/imx-drm.h  | 10 +-
 drivers/gpu/drm/imx/imx-ldb.c  | 10 +-
 drivers/gpu/drm/imx/imx-tve.c  |  6 +++---
 drivers/gpu/drm/imx/ipuv3-crtc.c   | 13 +
 drivers/gpu/drm/imx/parallel-display.c | 13 ++---
 7 files changed, 32 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/imx/dw_hdmi-imx.c 
b/drivers/gpu/drm/imx/dw_hdmi-imx.c
index 61ef987..1834ac8 100644
--- a/drivers/gpu/drm/imx/dw_hdmi-imx.c
+++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c
@@ -123,7 +123,7 @@ static void dw_hdmi_imx_encoder_commit(struct drm_encoder 
*encoder)

 static void dw_hdmi_imx_encoder_prepare(struct drm_encoder *encoder)
 {
-   imx_drm_panel_format(encoder, MEDIA_BUS_FMT_RGB888_1X24);
+   imx_drm_set_bus_format(encoder, MEDIA_BUS_FMT_RGB888_1X24);
 }

 static struct drm_encoder_helper_funcs dw_hdmi_imx_encoder_helper_funcs = {
diff --git a/drivers/gpu/drm/imx/imx-drm-core.c 
b/drivers/gpu/drm/imx/imx-drm-core.c
index a002f53..c6f2c472 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -103,8 +103,8 @@ static struct imx_drm_crtc *imx_drm_find_crtc(struct 
drm_crtc *crtc)
return NULL;
 }

-int imx_drm_panel_format_pins(struct drm_encoder *encoder,
-   u32 interface_pix_fmt, int hsync_pin, int vsync_pin)
+int imx_drm_set_bus_format_pins(struct drm_encoder *encoder, u32 bus_format,
+   int hsync_pin, int vsync_pin)
 {
struct imx_drm_crtc_helper_funcs *helper;
struct imx_drm_crtc *imx_crtc;
@@ -116,16 +116,16 @@ int imx_drm_panel_format_pins(struct drm_encoder *encoder,
helper = _crtc->imx_drm_helper_funcs;
if (helper->set_interface_pix_fmt)
return helper->set_interface_pix_fmt(encoder->crtc,
-   interface_pix_fmt, hsync_pin, vsync_pin);
+   bus_format, hsync_pin, vsync_pin);
return 0;
 }
-EXPORT_SYMBOL_GPL(imx_drm_panel_format_pins);
+EXPORT_SYMBOL_GPL(imx_drm_set_bus_format_pins);

-int imx_drm_panel_format(struct drm_encoder *encoder, u32 interface_pix_fmt)
+int imx_drm_set_bus_format(struct drm_encoder *encoder, u32 bus_format)
 {
-   return imx_drm_panel_format_pins(encoder, interface_pix_fmt, 2, 3);
+   return imx_drm_set_bus_format_pins(encoder, bus_format, 2, 3);
 }
-EXPORT_SYMBOL_GPL(imx_drm_panel_format);
+EXPORT_SYMBOL_GPL(imx_drm_set_bus_format);

 int imx_drm_crtc_vblank_get(struct imx_drm_crtc *imx_drm_crtc)
 {
diff --git a/drivers/gpu/drm/imx/imx-drm.h b/drivers/gpu/drm/imx/imx-drm.h
index 3c559cc..28e776d 100644
--- a/drivers/gpu/drm/imx/imx-drm.h
+++ b/drivers/gpu/drm/imx/imx-drm.h
@@ -18,7 +18,7 @@ struct imx_drm_crtc_helper_funcs {
int (*enable_vblank)(struct drm_crtc *crtc);
void (*disable_vblank)(struct drm_crtc *crtc);
int (*set_interface_pix_fmt)(struct drm_crtc *crtc,
-   u32 pix_fmt, int hsync_pin, int vsync_pin);
+   u32 bus_format, int hsync_pin, int vsync_pin);
const struct drm_crtc_helper_funcs *crtc_helper_funcs;
const struct drm_crtc_funcs *crtc_funcs;
 };
@@ -40,10 +40,10 @@ void imx_drm_mode_config_init(struct drm_device *drm);

 struct drm_gem_cma_object *imx_drm_fb_get_obj(struct drm_framebuffer *fb);

-int imx_drm_panel_format_pins(struct drm_encoder *encoder,
-   u32 interface_pix_fmt, int hsync_pin, int vsync_pin);
-int imx_drm_panel_format(struct drm_encoder *encoder,
-   u32 interface_pix_fmt);
+int imx_drm_set_bus_format_pins(struct drm_encoder *encoder,
+   u32 bus_format, int hsync_pin, int vsync_pin);
+int imx_drm_set_bus_format(struct drm_encoder *encoder,
+   u32 bus_format);

 int imx_drm_encoder_get_mux_id(struct device_node *node,
struct drm_encoder *encoder);
diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
index f9ec17a..cd062b1 100644
--- a/drivers/gpu/drm/imx/imx-ldb.c
+++ b/drivers/gpu/drm/imx/imx-ldb.c
@@ -163,24 +163,24 @@ static void imx_ldb_encoder_prepare(struct drm_encoder 
*encoder)
 {
struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
struct imx_ldb *ldb = imx_ldb_ch->ldb;
-   u32 pixel_fmt;
+   u32 bus_format;

switch (imx_ldb_ch->chno) {
case 0:
-   pixel_fmt = (ldb->ldb_ctrl & LDB_DATA_WIDTH_CH0_24) ?
+   bus_format = (ldb->ldb_ctrl & LDB_DATA_WIDTH_CH0_24) ?

[PATCH v2 06/10] drm/imx: switch to use media bus formats

[Philipp Zabel]

imx-drm internally misused the V4L2_PIX_FMT constants, which are supposed to
describe the pixel format of frame buffers in memory, to describe the pixel
format on the bus between the display controller and the encoder hardware
instead. Now that MEDIA_BUS_FMT constants are available to drm drivers, use
those instead.

Signed-off-by: Philipp Zabel 
---
 drivers/gpu/drm/imx/dw_hdmi-imx.c  |  2 +-
 drivers/gpu/drm/imx/imx-ldb.c  |  6 +++---
 drivers/gpu/drm/imx/imx-tve.c  |  4 ++--
 drivers/gpu/drm/imx/parallel-display.c |  8 
 drivers/gpu/ipu-v3/ipu-dc.c| 16 
 5 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/imx/dw_hdmi-imx.c 
b/drivers/gpu/drm/imx/dw_hdmi-imx.c
index 87fe8ed..61ef987 100644
--- a/drivers/gpu/drm/imx/dw_hdmi-imx.c
+++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c
@@ -123,7 +123,7 @@ static void dw_hdmi_imx_encoder_commit(struct drm_encoder 
*encoder)

 static void dw_hdmi_imx_encoder_prepare(struct drm_encoder *encoder)
 {
-   imx_drm_panel_format(encoder, V4L2_PIX_FMT_RGB24);
+   imx_drm_panel_format(encoder, MEDIA_BUS_FMT_RGB888_1X24);
 }

 static struct drm_encoder_helper_funcs dw_hdmi_imx_encoder_helper_funcs = {
diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
index 2d6dc94..f9ec17a 100644
--- a/drivers/gpu/drm/imx/imx-ldb.c
+++ b/drivers/gpu/drm/imx/imx-ldb.c
@@ -168,16 +168,16 @@ static void imx_ldb_encoder_prepare(struct drm_encoder 
*encoder)
switch (imx_ldb_ch->chno) {
case 0:
pixel_fmt = (ldb->ldb_ctrl & LDB_DATA_WIDTH_CH0_24) ?
-   V4L2_PIX_FMT_RGB24 : V4L2_PIX_FMT_BGR666;
+   MEDIA_BUS_FMT_RGB888_1X24 : MEDIA_BUS_FMT_RGB666_1X18;
break;
case 1:
pixel_fmt = (ldb->ldb_ctrl & LDB_DATA_WIDTH_CH1_24) ?
-   V4L2_PIX_FMT_RGB24 : V4L2_PIX_FMT_BGR666;
+   MEDIA_BUS_FMT_RGB888_1X24 : MEDIA_BUS_FMT_RGB666_1X18;
break;
default:
dev_err(ldb->dev, "unable to config di%d panel format\n",
imx_ldb_ch->chno);
-   pixel_fmt = V4L2_PIX_FMT_RGB24;
+   pixel_fmt = MEDIA_BUS_FMT_RGB888_1X24;
}

imx_drm_panel_format(encoder, pixel_fmt);
diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c
index b63601d..7b0f28b 100644
--- a/drivers/gpu/drm/imx/imx-tve.c
+++ b/drivers/gpu/drm/imx/imx-tve.c
@@ -297,11 +297,11 @@ static void imx_tve_encoder_prepare(struct drm_encoder 
*encoder)

switch (tve->mode) {
case TVE_MODE_VGA:
-   imx_drm_panel_format_pins(encoder, IPU_PIX_FMT_GBR24,
+   imx_drm_panel_format_pins(encoder, MEDIA_BUS_FMT_YUV8_1X24,
tve->hsync_pin, tve->vsync_pin);
break;
case TVE_MODE_TVOUT:
-   imx_drm_panel_format(encoder, V4L2_PIX_FMT_YUV444);
+   imx_drm_panel_format(encoder, MEDIA_BUS_FMT_YUV8_1X24);
break;
}
 }
diff --git a/drivers/gpu/drm/imx/parallel-display.c 
b/drivers/gpu/drm/imx/parallel-display.c
index 5e83e00..40a9194 100644
--- a/drivers/gpu/drm/imx/parallel-display.c
+++ b/drivers/gpu/drm/imx/parallel-display.c
@@ -225,14 +225,14 @@ static int imx_pd_bind(struct device *dev, struct device 
*master, void *data)
ret = of_property_read_string(np, "interface-pix-fmt", );
if (!ret) {
if (!strcmp(fmt, "rgb24"))
-   imxpd->interface_pix_fmt = V4L2_PIX_FMT_RGB24;
+   imxpd->interface_pix_fmt = MEDIA_BUS_FMT_RGB888_1X24;
else if (!strcmp(fmt, "rgb565"))
-   imxpd->interface_pix_fmt = V4L2_PIX_FMT_RGB565;
+   imxpd->interface_pix_fmt = MEDIA_BUS_FMT_RGB565_1X16;
else if (!strcmp(fmt, "bgr666"))
-   imxpd->interface_pix_fmt = V4L2_PIX_FMT_BGR666;
+   imxpd->interface_pix_fmt = MEDIA_BUS_FMT_RGB666_1X18;
else if (!strcmp(fmt, "lvds666"))
imxpd->interface_pix_fmt =
-   v4l2_fourcc('L', 'V', 'D', '6');
+   MEDIA_BUS_FMT_RGB666_1X24_CPADHI;
}

panel_node = of_parse_phandle(np, "fsl,panel", 0);
diff --git a/drivers/gpu/ipu-v3/ipu-dc.c b/drivers/gpu/ipu-v3/ipu-dc.c
index 323203d..da5a4b9 100644
--- a/drivers/gpu/ipu-v3/ipu-dc.c
+++ b/drivers/gpu/ipu-v3/ipu-dc.c
@@ -147,20 +147,20 @@ static void dc_write_tmpl(struct ipu_dc *dc, int word, 
u32 opcode, u32 operand,
writel(reg2, priv->dc_tmpl_reg + word * 8 + 4);
 }

-static int ipu_pixfmt_to_map(u32 fmt)
+static int ipu_bus_format_to_map(u32 fmt)
 {
switch (fmt) {
-   case V4L2_PIX_FMT_RGB24:
+   case MEDIA_BUS_FMT_RGB888_1X24:
return IPU_DC_MAP_RGB24;
-   case 

[PATCH v2 05/10] Add RGB666_1X24_CPADHI media bus format

[Philipp Zabel]

Commit 9e74d2926a28 ("staging: imx-drm: add LVDS666 support for parallel
display") describes a 24-bit bus format where three 6-bit components each
take the lower part of 8 bits with the two high bits zero padded. Add a
component-wise padded media bus format RGB666_1X24_CPADHI to support this
connection.

Cc: Emil Renner Berthing 
Signed-off-by: Philipp Zabel 
---
 Documentation/DocBook/media/v4l/subdev-formats.xml | 30 ++
 include/uapi/linux/media-bus-format.h  |  3 ++-
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/Documentation/DocBook/media/v4l/subdev-formats.xml 
b/Documentation/DocBook/media/v4l/subdev-formats.xml
index 8d1f624..c02af7a 100644
--- a/Documentation/DocBook/media/v4l/subdev-formats.xml
+++ b/Documentation/DocBook/media/v4l/subdev-formats.xml
@@ -480,6 +480,36 @@ see .
  b1
  b0

+   
+ MEDIA_BUS_FMT_RGB666_1X24_CPADHI
+ 0x1015
+ 
+ 
+ 0
+ 0
+ r5
+ r4
+ r3
+ r2
+ r1
+ r0
+ 0
+ 0
+ g5
+ g4
+ g3
+ g2
+ g1
+ g0
+ 0
+ 0
+ b5
+ b4
+ b3
+ b2
+ b1
+ b0
+   

  MEDIA_BUS_FMT_BGR888_1X24
  0x1013
diff --git a/include/uapi/linux/media-bus-format.h 
b/include/uapi/linux/media-bus-format.h
index 8dbf16c..83ea46f 100644
--- a/include/uapi/linux/media-bus-format.h
+++ b/include/uapi/linux/media-bus-format.h
@@ -33,7 +33,7 @@

 #define MEDIA_BUS_FMT_FIXED0x0001

-/* RGB - next is   0x1015 */
+/* RGB - next is   0x1016 */
 #define MEDIA_BUS_FMT_RGB444_1X12  0x100e
 #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE  0x1001
 #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE  0x1002
@@ -45,6 +45,7 @@
 #define MEDIA_BUS_FMT_RGB565_2X8_BE0x1007
 #define MEDIA_BUS_FMT_RGB565_2X8_LE0x1008
 #define MEDIA_BUS_FMT_RGB666_1X18  0x1009
+#define MEDIA_BUS_FMT_RGB666_1X24_CPADHI   0x1015
 #define MEDIA_BUS_FMT_RGB666_1X7X3_SPWG0x1010
 #define MEDIA_BUS_FMT_BGR888_1X24  0x1013
 #define MEDIA_BUS_FMT_GBR888_1X24  0x1014
-- 
2.1.4

[PATCH v2 04/10] Add YUV8_1X24 media bus format

[Philipp Zabel]

This patch adds the media bus format for a 24-bit bus format with three
8-bit YUV components.

Signed-off-by: Philipp Zabel 
Acked-by: Laurent Pinchart 
---
 Documentation/DocBook/media/v4l/subdev-formats.xml | 37 ++
 include/uapi/linux/media-bus-format.h  |  3 +-
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/Documentation/DocBook/media/v4l/subdev-formats.xml 
b/Documentation/DocBook/media/v4l/subdev-formats.xml
index 805cbe1..8d1f624 100644
--- a/Documentation/DocBook/media/v4l/subdev-formats.xml
+++ b/Documentation/DocBook/media/v4l/subdev-formats.xml
@@ -3015,6 +3015,43 @@ see .
  u1
  u0

+   
+ MEDIA_BUS_FMT_YUV8_1X24
+ 0x2024
+ 
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ y7
+ y6
+ y5
+ y4
+ y3
+ y2
+ y1
+ y0
+ u7
+ u6
+ u5
+ u4
+ u3
+ u2
+ u1
+ u0
+ v7
+ v6
+ v5
+ v4
+ v3
+ v2
+ v1
+ v0
+   

  MEDIA_BUS_FMT_YUV10_1X30
  0x2016
diff --git a/include/uapi/linux/media-bus-format.h 
b/include/uapi/linux/media-bus-format.h
index 6f6942e..8dbf16c 100644
--- a/include/uapi/linux/media-bus-format.h
+++ b/include/uapi/linux/media-bus-format.h
@@ -55,7 +55,7 @@
 #define MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA   0x1012
 #define MEDIA_BUS_FMT_ARGB_1X320x100d

-/* YUV (including grey) - next is  0x2024 */
+/* YUV (including grey) - next is  0x2025 */
 #define MEDIA_BUS_FMT_Y8_1X8   0x2001
 #define MEDIA_BUS_FMT_UV8_1X8  0x2015
 #define MEDIA_BUS_FMT_UYVY8_1_5X8  0x2002
@@ -81,6 +81,7 @@
 #define MEDIA_BUS_FMT_VYUY10_1X20  0x201b
 #define MEDIA_BUS_FMT_YUYV10_1X20  0x200d
 #define MEDIA_BUS_FMT_YVYU10_1X20  0x200e
+#define MEDIA_BUS_FMT_YUV8_1X240x2024
 #define MEDIA_BUS_FMT_YUV10_1X30   0x2016
 #define MEDIA_BUS_FMT_AYUV8_1X32   0x2017
 #define MEDIA_BUS_FMT_UYVY12_2X12  0x201c
-- 
2.1.4

[PATCH v2 03/10] Add BGR888_1X24 and GBR888_1X24 media bus formats

[Philipp Zabel]

This patch adds two more 24-bit RGB formats. BGR888 is more or less common,
GBR888 is used on the internal connection between the IPU display interface
and the TVE (VGA DAC) on i.MX53 SoCs.

Signed-off-by: Philipp Zabel 
Acked-by: Laurent Pinchart 
---
 Documentation/DocBook/media/v4l/subdev-formats.xml | 60 ++
 include/uapi/linux/media-bus-format.h  |  4 +-
 2 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/Documentation/DocBook/media/v4l/subdev-formats.xml 
b/Documentation/DocBook/media/v4l/subdev-formats.xml
index 18449b3..805cbe1 100644
--- a/Documentation/DocBook/media/v4l/subdev-formats.xml
+++ b/Documentation/DocBook/media/v4l/subdev-formats.xml
@@ -480,6 +480,66 @@ see .
  b1
  b0

+   
+ MEDIA_BUS_FMT_BGR888_1X24
+ 0x1013
+ 
+ 
+ b7
+ b6
+ b5
+ b4
+ b3
+ b2
+ b1
+ b0
+ g7
+ g6
+ g5
+ g4
+ g3
+ g2
+ g1
+ g0
+ r7
+ r6
+ r5
+ r4
+ r3
+ r2
+ r1
+ r0
+   
+   
+ MEDIA_BUS_FMT_GBR888_1X24
+ 0x1014
+ 
+ 
+ g7
+ g6
+ g5
+ g4
+ g3
+ g2
+ g1
+ g0
+ b7
+ b6
+ b5
+ b4
+ b3
+ b2
+ b1
+ b0
+ r7
+ r6
+ r5
+ r4
+ r3
+ r2
+ r1
+ r0
+   

  MEDIA_BUS_FMT_RGB888_1X24
  0x100a
diff --git a/include/uapi/linux/media-bus-format.h 
b/include/uapi/linux/media-bus-format.h
index 3fb9cbb..6f6942e 100644
--- a/include/uapi/linux/media-bus-format.h
+++ b/include/uapi/linux/media-bus-format.h
@@ -33,7 +33,7 @@

 #define MEDIA_BUS_FMT_FIXED0x0001

-/* RGB - next is   0x1013 */
+/* RGB - next is   0x1015 */
 #define MEDIA_BUS_FMT_RGB444_1X12  0x100e
 #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE  0x1001
 #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE  0x1002
@@ -46,6 +46,8 @@
 #define MEDIA_BUS_FMT_RGB565_2X8_LE0x1008
 #define MEDIA_BUS_FMT_RGB666_1X18  0x1009
 #define MEDIA_BUS_FMT_RGB666_1X7X3_SPWG0x1010
+#define MEDIA_BUS_FMT_BGR888_1X24  0x1013
+#define MEDIA_BUS_FMT_GBR888_1X24  0x1014
 #define MEDIA_BUS_FMT_RGB888_1X24  0x100a
 #define MEDIA_BUS_FMT_RGB888_2X12_BE   0x100b
 #define MEDIA_BUS_FMT_RGB888_2X12_LE   0x100c
-- 
2.1.4

[PATCH v2 02/10] Add LVDS RGB media bus formats

[Philipp Zabel]

This patch adds three new RGB media bus formats that describe
18-bit or 24-bit samples transferred over an LVDS bus with three
or four differential data pairs, serialized into 7 time slots,
using standard SPWG/PSWG/VESA or JEIDA data ordering.

Signed-off-by: Philipp Zabel 
Acked-by: Sakari Ailus 
---
 Documentation/DocBook/media/v4l/subdev-formats.xml | 255 +
 include/uapi/linux/media-bus-format.h  |   5 +-
 2 files changed, 259 insertions(+), 1 deletion(-)

diff --git a/Documentation/DocBook/media/v4l/subdev-formats.xml 
b/Documentation/DocBook/media/v4l/subdev-formats.xml
index 29fe601..18449b3 100644
--- a/Documentation/DocBook/media/v4l/subdev-formats.xml
+++ b/Documentation/DocBook/media/v4l/subdev-formats.xml
@@ -622,6 +622,261 @@ see .
  

   
+
+  On LVDS buses, usually each sample is transferred serialized in
+  seven time slots per pixel clock, on three (18-bit) or four (24-bit)
+  differential data pairs at the same time. The remaining bits are used for
+  control signals as defined by SPWG/PSWG/VESA or JEIDA standards.
+  The 24-bit RGB format serialized in seven time slots on four lanes using
+  JEIDA defined bit mapping will be named
+  MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA, for example.
+  
+
+  
+   LVDS RGB formats
+   
+ 
+ 
+ 
+ 
+ 
+ 
+ 
+ 
+ 
+ 
+   
+ Identifier
+ Code
+ 
+ 
+ Data organization
+   
+   
+ 
+ 
+ Timeslot
+ Lane
+ 3
+ 2
+ 1
+ 0
+   
+ 
+ 
+   
+ MEDIA_BUS_FMT_RGB666_1X7X3_SPWG
+ 0x1010
+ 0
+ 
+ -
+ d
+ b1
+ g0
+   
+   
+ 
+ 
+ 1
+ 
+ -
+ d
+ b0
+ r5
+   
+   
+ 
+ 
+ 2
+ 
+ -
+ d
+ g5
+ r4
+   
+   
+ 
+ 
+ 3
+ 
+ -
+ b5
+ g4
+ r3
+   
+   
+ 
+ 
+ 4
+ 
+ -
+ b4
+ g3
+ r2
+   
+   
+ 
+ 
+ 5
+ 
+ -
+ b3
+ g2
+ r1
+   
+   
+ 
+ 
+ 6
+ 
+ -
+ b2
+ g1
+ r0
+   
+   
+ MEDIA_BUS_FMT_RGB888_1X7X4_SPWG
+ 0x1011
+ 0
+ 
+ d
+ d
+ b1
+ g0
+   
+   
+ 
+ 
+ 1
+ 
+ b7
+ d
+ b0
+ r5
+   
+   
+ 
+ 
+ 2
+ 
+ b6
+ d
+ g5
+ r4
+   
+   
+ 
+ 
+ 3
+ 
+ g7
+ b5
+ g4
+ r3
+   
+   
+ 
+ 
+ 4
+ 
+ g6
+ b4
+ g3
+ r2
+   
+   
+ 
+ 
+ 5
+ 
+ r7
+ b3
+ g2
+ r1
+   
+   
+ 
+ 
+ 6
+ 
+ r6
+ b2
+ g1
+ r0
+   
+   
+ MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA
+ 0x1012
+ 0
+ 
+ d
+ d
+ b3
+ g2
+   
+   
+ 
+ 
+ 1
+ 
+ b1
+ d
+ b2
+ r7
+   
+   
+ 
+ 
+ 2
+ 
+ b0
+ d
+ g7
+ r6
+   
+   
+ 
+ 
+ 3
+ 
+ g1
+ b7
+ g6
+ r5
+   
+   
+ 
+ 
+ 4
+ 
+ g0
+ b6
+ g5
+ r4
+   
+   
+ 
+ 
+ 5
+ 
+ r1
+ b5
+ g4
+ r3
+   
+   
+ 
+ 

[PATCH v2 01/10] Add RGB444_1X12 and RGB565_1X16 media bus formats

[Philipp Zabel]

From: Boris Brezillion 

Add RGB444_1X12 and RGB565_1X16 format definitions and update the
documentation.

Signed-off-by: Boris Brezillon 
Acked-by: Mauro Carvalho Chehab 
Acked-by: Sakari Ailus 
Acked-by: Laurent Pinchart 
Signed-off-by: Philipp Zabel 
---
Changes since v1:
 - Fixed constant values in the documentation to correspond to the actual
   values.
 - Added Laurent's ack.
---
 Documentation/DocBook/media/v4l/subdev-formats.xml | 40 ++
 include/uapi/linux/media-bus-format.h  |  4 ++-
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/Documentation/DocBook/media/v4l/subdev-formats.xml 
b/Documentation/DocBook/media/v4l/subdev-formats.xml
index c5ea868..29fe601 100644
--- a/Documentation/DocBook/media/v4l/subdev-formats.xml
+++ b/Documentation/DocBook/media/v4l/subdev-formats.xml
@@ -192,6 +192,24 @@ see .

  
  
+   
+ MEDIA_BUS_FMT_RGB444_1X12
+ 0x100e
+ 
+ 
+ r3
+ r2
+ r1
+ r0
+ g3
+ g2
+ g1
+ g0
+ b3
+ b2
+ b1
+ b0
+   

  MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE
  0x1001
@@ -304,6 +322,28 @@ see .
  g4
  g3

+   
+ MEDIA_BUS_FMT_RGB565_1X16
+ 0x100f
+ 
+ 
+ r4
+ r3
+ r2
+ r1
+ r0
+ g5
+ g4
+ g3
+ g2
+ g1
+ g0
+ b4
+ b3
+ b2
+ b1
+ b0
+   

  MEDIA_BUS_FMT_BGR565_2X8_BE
  0x1005
diff --git a/include/uapi/linux/media-bus-format.h 
b/include/uapi/linux/media-bus-format.h
index 23b4090..37091c6 100644
--- a/include/uapi/linux/media-bus-format.h
+++ b/include/uapi/linux/media-bus-format.h
@@ -33,11 +33,13 @@

 #define MEDIA_BUS_FMT_FIXED0x0001

-/* RGB - next is   0x100e */
+/* RGB - next is   0x1010 */
+#define MEDIA_BUS_FMT_RGB444_1X12  0x100e
 #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE  0x1001
 #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE  0x1002
 #define MEDIA_BUS_FMT_RGB555_2X8_PADHI_BE  0x1003
 #define MEDIA_BUS_FMT_RGB555_2X8_PADHI_LE  0x1004
+#define MEDIA_BUS_FMT_RGB565_1X16  0x100f
 #define MEDIA_BUS_FMT_BGR565_2X8_BE0x1005
 #define MEDIA_BUS_FMT_BGR565_2X8_LE0x1006
 #define MEDIA_BUS_FMT_RGB565_2X8_BE0x1007
-- 
2.1.4

[PATCH v2 00/10] Use media bus formats in imx-drm and add drm panel support

[Philipp Zabel]

Sorry about the noise, this time sent to the correct list:

Currently the imx-drm driver misuses the V4L2_PIX_FMT constants to describe the
pixel format on the parallel bus between display controllers and encoders. Now
that MEDIA_BUS_FMT is available, use that instead.

I'd like to get the V4L2 maintainers' acks for the four necessary media
bus format patches to be merged through the drm tree.

The two drm/imx core patches replace V4L2_PIX_FMT with MEDIA_BUS_FMT where
appropriate and consolidate the variable names for clarification.

The three LDB patches depend on this of-graph helper series:
http://comments.gmane.org/gmane.linux.kernel/1871630
They allow to optionally use LVDS panels with drm_panel drivers, connected to
the LDB encoder in the device tree via of-graph endpoint links.

Changes since v1:
 - Fixed BGR888_1X24 and GBR888_1X24 constant values in the documentation
   and added Laurent's ack.
 - Fixed ldb drm_panel patch to use port at 2 as output on i.MX53
 - Use correct mailing list

regards
Philipp

Boris Brezillion (1):
  Add RGB444_1X12 and RGB565_1X16 media bus formats

Philipp Zabel (9):
  Add LVDS RGB media bus formats
  Add BGR888_1X24 and GBR888_1X24 media bus formats
  Add YUV8_1X24 media bus format
  Add RGB666_1X24_CPADHI media bus format
  drm/imx: switch to use media bus formats
  drm/imx: consolidate bus format variable names
  drm/imx: imx-ldb: add drm_panel support
  drm/imx: imx-ldb: reset display clock input when disabling LVDS
  drm/imx: imx-ldb: allow to determine bus format from the connected
panel

 Documentation/DocBook/media/v4l/subdev-formats.xml | 422 +
 Documentation/devicetree/bindings/drm/imx/ldb.txt  |  51 ++-
 drivers/gpu/drm/imx/Kconfig|   1 +
 drivers/gpu/drm/imx/dw_hdmi-imx.c  |   2 +-
 drivers/gpu/drm/imx/imx-drm-core.c |  14 +-
 drivers/gpu/drm/imx/imx-drm.h  |  10 +-
 drivers/gpu/drm/imx/imx-ldb.c  | 196 +++---
 drivers/gpu/drm/imx/imx-tve.c  |   6 +-
 drivers/gpu/drm/imx/ipuv3-crtc.c   |  13 +-
 drivers/gpu/drm/imx/parallel-display.c |  13 +-
 drivers/gpu/ipu-v3/ipu-dc.c|  16 +-
 include/uapi/linux/media-bus-format.h  |  13 +-
 12 files changed, 642 insertions(+), 115 deletions(-)

-- 
2.1.4

[PATCH 1/3] drm/panel: Add display_timing support

[Philipp Zabel]

Hi Thierry,

Am Dienstag, den 03.02.2015, 14:30 +0100 schrieb Thierry Reding:
> On Thu, Dec 11, 2014 at 06:32:44PM +0100, Philipp Zabel wrote:
> > Many panel data sheets additionally to typical values list allowed ranges 
> > for
> > timings such as hsync/vsync lengths, porches, and the pixel clock rate. 
> > These
> > can be stored in a struct display_timing, to be used by an encoder 
> > mode_fixup
> > callback to clamp user provided timing values or to validate workarounds for
> > clock source limitations.
> > 
> > This patch adds a new drm_panel_funcs callback that returns the panels'
> > available display_timing entries.
> > 
> > Signed-off-by: Philipp Zabel 
> > ---
> >  include/drm/drm_panel.h | 5 +
> >  1 file changed, 5 insertions(+)
> 
> Sorry for taking so long to get back on this. I generally like the idea,
> though a couple of things are unclear to me.
> 
> In struct display_timing we have:
> 
>   struct timing_entry hactive;
>   ...
>   struct timing_entry vactive;
> 
> I wonder if that really makes sense. Are there really datasheets that
> provide a valid range for the number of horizontal and vertical active
> pixels?

I often see datasheets that give minimum, typical and maximum values
also for the horizontal and vertical active pixels, but those are
usually the same value. I don't know if there are any panels that really
have documented ranges here. 

[...]
> > @@ -68,6 +71,8 @@ struct drm_panel_funcs {
> > int (*prepare)(struct drm_panel *panel);
> > int (*enable)(struct drm_panel *panel);
> > int (*get_modes)(struct drm_panel *panel);
> > +   int (*get_timings)(struct drm_panel *panel, unsigned int num_timings,
> > +  struct display_timing *timings);
> 
> Also are there even panels that support more than one set of timings?
> I've never heard of panels that are actually able to do more than one
> mode, so I'm wondering if num_timings isn't overdoing it a little here.
> I guess it doesn't hurt all that much, though.

I have not seen any yet that reasonably should be driven by the
simple-panel driver. I was thinking about the Pixel Qi panel, but it
turns out those just have a single timing and use the subpixels in B/W
reflective mode.

regards
Philipp

[PATCH] drm/cirrus: Limit modes depending on bpp option

[Takashi Iwai]

The commit [8975626ea35a: drm/cirrus: allow 32bpp framebuffers for
cirrus drm] broke X modesetting driver because cirrus driver still
provides the full list of modes up to 1280x1024 while the 32bpp can
support only up to 800x600.

We might be able to filter out the invalid modes in mode_valid
callback, but unfortunately the bpp in question can't be referred
there for now (let me know if there is a better way to retrieve the
bpp for the probed fb).

So, instead, this patch adds the bpp module option to specify the
maximal bpp explicitly and limits the resolutions in get_modes
depending on its value.

The default value is set to 24 so that the existing stuff keeps
working.  If you need a new 32bpp feature, specify cirrus.bpp=32
option explicitly.

Fixes: 8975626ea35a ('drm/cirrus: allow 32bpp framebuffers for cirrus drm')
Signed-off-by: Takashi Iwai 
---

Dave, this is a regression on 3.19-rc.  It'd be great if you can take it
(or fix in a different form) before 3.19-final.


thanks,

Takashi


 drivers/gpu/drm/cirrus/cirrus_drv.c  | 3 +++
 drivers/gpu/drm/cirrus/cirrus_drv.h  | 3 +++
 drivers/gpu/drm/cirrus/cirrus_main.c | 2 ++
 drivers/gpu/drm/cirrus/cirrus_mode.c | 9 +++--
 4 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.c 
b/drivers/gpu/drm/cirrus/cirrus_drv.c
index c2a1cba1e984..b9140032962d 100644
--- a/drivers/gpu/drm/cirrus/cirrus_drv.c
+++ b/drivers/gpu/drm/cirrus/cirrus_drv.c
@@ -16,9 +16,12 @@
 #include "cirrus_drv.h"

 int cirrus_modeset = -1;
+int cirrus_bpp = 24;

 MODULE_PARM_DESC(modeset, "Disable/Enable modesetting");
 module_param_named(modeset, cirrus_modeset, int, 0400);
+MODULE_PARM_DESC(bpp, "Max bits-per-pixel (default:24)");
+module_param_named(bpp, cirrus_bpp, int, 0400);

 /*
  * This is the generic driver code. This binds the driver to the drm core,
diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.h 
b/drivers/gpu/drm/cirrus/cirrus_drv.h
index 693a4565c4ff..705061537a27 100644
--- a/drivers/gpu/drm/cirrus/cirrus_drv.h
+++ b/drivers/gpu/drm/cirrus/cirrus_drv.h
@@ -262,4 +262,7 @@ static inline void cirrus_bo_unreserve(struct cirrus_bo *bo)

 int cirrus_bo_push_sysram(struct cirrus_bo *bo);
 int cirrus_bo_pin(struct cirrus_bo *bo, u32 pl_flag, u64 *gpu_addr);
+
+extern int cirrus_bpp;
+
 #endif /* __CIRRUS_DRV_H__ */
diff --git a/drivers/gpu/drm/cirrus/cirrus_main.c 
b/drivers/gpu/drm/cirrus/cirrus_main.c
index 4c2d68e9102d..e4b976658087 100644
--- a/drivers/gpu/drm/cirrus/cirrus_main.c
+++ b/drivers/gpu/drm/cirrus/cirrus_main.c
@@ -320,6 +320,8 @@ bool cirrus_check_framebuffer(struct cirrus_device *cdev, 
int width, int height,
const int max_pitch = 0x1FF << 3; /* (4096 - 1) & ~111b bytes */
const int max_size = cdev->mc.vram_size;

+   if (bpp > cirrus_bpp)
+   return false;
if (bpp > 32)
return false;

diff --git a/drivers/gpu/drm/cirrus/cirrus_mode.c 
b/drivers/gpu/drm/cirrus/cirrus_mode.c
index 99d4a74ffeaf..61385f2298bf 100644
--- a/drivers/gpu/drm/cirrus/cirrus_mode.c
+++ b/drivers/gpu/drm/cirrus/cirrus_mode.c
@@ -501,8 +501,13 @@ static int cirrus_vga_get_modes(struct drm_connector 
*connector)
int count;

/* Just add a static list of modes */
-   count = drm_add_modes_noedid(connector, 1280, 1024);
-   drm_set_preferred_mode(connector, 1024, 768);
+   if (cirrus_bpp <= 24) {
+   count = drm_add_modes_noedid(connector, 1280, 1024);
+   drm_set_preferred_mode(connector, 1024, 768);
+   } else {
+   count = drm_add_modes_noedid(connector, 800, 600);
+   drm_set_preferred_mode(connector, 800, 600);
+   }
return count;
 }

-- 
2.2.2

[Linaro-mm-sig] [RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms

[Arnd Bergmann]

On Tuesday 03 February 2015 11:22:01 Rob Clark wrote:
> On Tue, Feb 3, 2015 at 11:12 AM, Arnd Bergmann  wrote:
> > I agree for the case you are describing here. From what I understood
> > from Rob was that he is looking at something more like:
> >
> > Fig 3
> > CPU--L1cache--L2cache--Memory--IOMMU-device
> >
> > where the IOMMU controls one or more contexts per device, and is
> > shared across GPU and non-GPU devices. Here, we need to use the
> > dmap-mapping interface to set up the IO page table for any device
> > that is unable to address all of system RAM, and we can use it
> > for purposes like isolation of the devices. There are also cases
> > where using the IOMMU is not optional.
> 
> 
> Actually, just to clarify, the IOMMU instance is specific to the GPU..
> not shared with other devices.  Otherwise managing multiple contexts
> would go quite badly..
> 
> But other devices have their own instance of the same IOMMU.. so same
> driver could be used.

I think from the driver perspective, I'd view those two cases as
identical. Not sure if Russell agrees with that.

Arnd

[PATCH 14/14] drm/exynos: remove exynos_disable_plane()

[Gustavo Padovan]

From: Gustavo Padovan 

exynos_disable_plane() is used only once now thus remove it and call
exynos_plane_dpms() directly.

Signed-off-by: Gustavo Padovan 
---
 drivers/gpu/drm/exynos/exynos_drm_plane.c | 9 +
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c 
b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index c0c53d6..34dcf17b 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -191,18 +191,11 @@ exynos_update_plane(struct drm_plane *plane, struct 
drm_crtc *crtc,
exynos_crtc->ops->win_commit(exynos_crtc, exynos_plane->zpos);
 }

-static int exynos_disable_plane(struct drm_plane *plane)
-{
-   exynos_plane_dpms(plane, DRM_MODE_DPMS_OFF);
-
-   return 0;
-}
-
 static void exynos_plane_destroy(struct drm_plane *plane)
 {
struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);

-   exynos_disable_plane(plane);
+   exynos_plane_dpms(plane, DRM_MODE_DPMS_OFF);
drm_plane_cleanup(plane);
kfree(exynos_plane);
 }
-- 
1.9.3

[PATCH 13/14] drm/exynos: use correct pipe number on vblank event

[Gustavo Padovan]

From: Gustavo Padovan 

Use the pipe var received instead of using always -1 as pipe value.

Signed-off-by: Gustavo Padovan 
---
 drivers/gpu/drm/exynos/exynos_drm_crtc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c 
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 2edc73c..0f2b789 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -382,7 +382,7 @@ void exynos_drm_crtc_finish_pageflip(struct drm_device 
*dev, int pipe)
spin_lock_irqsave(>event_lock, flags);
if (exynos_crtc->event) {

-   drm_send_vblank_event(dev, -1, exynos_crtc->event);
+   drm_send_vblank_event(dev, pipe, exynos_crtc->event);
drm_vblank_put(dev, pipe);
wake_up(_crtc->pending_flip_queue);

-- 
1.9.3

[PATCH 12/14] drm/exynos: make exynos_plane_mode_set() static

[Gustavo Padovan]

From: Gustavo Padovan 

It is not used outside of the plane scope anymore.

Signed-off-by: Gustavo Padovan 
---
 drivers/gpu/drm/exynos/exynos_drm_plane.c | 11 ++-
 drivers/gpu/drm/exynos/exynos_drm_plane.h |  5 -
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c 
b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index 0980c84..c0c53d6 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -91,11 +91,12 @@ int exynos_check_plane(struct drm_plane *plane, struct 
drm_framebuffer *fb)
return 0;
 }

-void exynos_plane_mode_set(struct drm_plane *plane, struct drm_crtc *crtc,
- struct drm_framebuffer *fb, int crtc_x, int crtc_y,
- unsigned int crtc_w, unsigned int crtc_h,
- uint32_t src_x, uint32_t src_y,
- uint32_t src_w, uint32_t src_h)
+static void exynos_plane_mode_set(struct drm_plane *plane, struct drm_crtc 
*crtc,
+ struct drm_framebuffer *fb,
+ int crtc_x, int crtc_y,
+ unsigned int crtc_w, unsigned int crtc_h,
+ uint32_t src_x, uint32_t src_y,
+ uint32_t src_w, uint32_t src_h)
 {
struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
unsigned int actual_w;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.h 
b/drivers/gpu/drm/exynos/exynos_drm_plane.h
index 8361606..8cdca0d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.h
@@ -10,11 +10,6 @@
  */

 int exynos_check_plane(struct drm_plane *plane, struct drm_framebuffer *fb);
-void exynos_plane_mode_set(struct drm_plane *plane, struct drm_crtc *crtc,
-  struct drm_framebuffer *fb, int crtc_x, int crtc_y,
-  unsigned int crtc_w, unsigned int crtc_h,
-  uint32_t src_x, uint32_t src_y,
-  uint32_t src_w, uint32_t src_h);
 void exynos_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
struct drm_framebuffer *fb, int crtc_x, int crtc_y,
unsigned int crtc_w, unsigned int crtc_h,
-- 
1.9.3

[PATCH 11/14] drm/exynos: atomic phase 2: keep track of framebuffer pointer

[Gustavo Padovan]

From: Gustavo Padovan 

Use drm_atomic_set_fb_for_plane() in the legacy page_flip path to keep
track of the framebuffer pointer and reference.

Signed-off-by: Gustavo Padovan 
---
 drivers/gpu/drm/exynos/exynos_drm_crtc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c 
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 660ad64..2edc73c 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -211,6 +211,9 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc,
crtc_w, crtc_h, crtc->x, crtc->y,
crtc_w, crtc_h);

+   if (crtc->primary->state)
+   drm_atomic_set_fb_for_plane(crtc->primary->state, fb);
+
return 0;

 out:
-- 
1.9.3

[PATCH 10/14] drm/exynos: atomic phase 2: wire up state reset(), duplicate() and destroy()

[Gustavo Padovan]

From: Gustavo Padovan 

Set CRTC, planes and connectors to use the default implementations from
the atomic helper library. The helpers will work to keep track of state
for each DRM object.

Signed-off-by: Gustavo Padovan 
---
 drivers/gpu/drm/bridge/ptn3460.c  | 4 
 drivers/gpu/drm/exynos/exynos_dp_core.c   | 4 
 drivers/gpu/drm/exynos/exynos_drm_connector.c | 4 
 drivers/gpu/drm/exynos/exynos_drm_crtc.c  | 5 +
 drivers/gpu/drm/exynos/exynos_drm_dpi.c   | 4 
 drivers/gpu/drm/exynos/exynos_drm_drv.c   | 2 ++
 drivers/gpu/drm/exynos/exynos_drm_dsi.c   | 4 
 drivers/gpu/drm/exynos/exynos_drm_plane.c | 4 
 drivers/gpu/drm/exynos/exynos_drm_vidi.c  | 4 
 drivers/gpu/drm/exynos/exynos_hdmi.c  | 4 
 10 files changed, 39 insertions(+)

diff --git a/drivers/gpu/drm/bridge/ptn3460.c b/drivers/gpu/drm/bridge/ptn3460.c
index 826833e..30da10c 100644
--- a/drivers/gpu/drm/bridge/ptn3460.c
+++ b/drivers/gpu/drm/bridge/ptn3460.c
@@ -27,6 +27,7 @@

 #include "drm_crtc.h"
 #include "drm_crtc_helper.h"
+#include "drm_atomic_helper.h"
 #include "drm_edid.h"
 #include "drmP.h"

@@ -263,6 +264,9 @@ static struct drm_connector_funcs ptn3460_connector_funcs = 
{
.fill_modes = drm_helper_probe_single_connector_modes,
.detect = ptn3460_detect,
.destroy = ptn3460_connector_destroy,
+   .reset = drm_atomic_helper_connector_reset,
+   .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+   .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };

 int ptn3460_bridge_attach(struct drm_bridge *bridge)
diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c 
b/drivers/gpu/drm/exynos/exynos_dp_core.c
index bf17a60..6704d5c 100644
--- a/drivers/gpu/drm/exynos/exynos_dp_core.c
+++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 

@@ -952,6 +953,9 @@ static struct drm_connector_funcs exynos_dp_connector_funcs 
= {
.fill_modes = drm_helper_probe_single_connector_modes,
.detect = exynos_dp_detect,
.destroy = exynos_dp_connector_destroy,
+   .reset = drm_atomic_helper_connector_reset,
+   .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+   .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };

 static int exynos_dp_get_modes(struct drm_connector *connector)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_connector.c 
b/drivers/gpu/drm/exynos/exynos_drm_connector.c
index ba9b3d5..980b085 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_connector.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_connector.c
@@ -13,6 +13,7 @@

 #include 
 #include 
+#include 

 #include 
 #include "exynos_drm_drv.h"
@@ -182,6 +183,9 @@ static struct drm_connector_funcs exynos_connector_funcs = {
.fill_modes = exynos_drm_connector_fill_modes,
.detect = exynos_drm_connector_detect,
.destroy= exynos_drm_connector_destroy,
+   .reset = drm_atomic_helper_connector_reset,
+   .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+   .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };

 struct drm_connector *exynos_drm_connector_create(struct drm_device *dev,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c 
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 17b64f8..660ad64 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -14,6 +14,8 @@

 #include 
 #include 
+#include 
+#include 

 #include "exynos_drm_crtc.h"
 #include "exynos_drm_drv.h"
@@ -265,6 +267,9 @@ static struct drm_crtc_funcs exynos_crtc_funcs = {
.page_flip  = exynos_drm_crtc_page_flip,
.destroy= exynos_drm_crtc_destroy,
.set_property   = exynos_drm_crtc_set_property,
+   .reset = drm_atomic_helper_crtc_reset,
+   .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
+   .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
 };

 static const struct drm_prop_enum_list mode_names[] = {
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dpi.c 
b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
index 37678cf..ced5c23 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dpi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 

 #include 

@@ -63,6 +64,9 @@ static struct drm_connector_funcs exynos_dpi_connector_funcs 
= {
.detect = exynos_dpi_detect,
.fill_modes = drm_helper_probe_single_connector_modes,
.destroy = exynos_dpi_connector_destroy,
+   .reset = drm_atomic_helper_connector_reset,
+   .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+   .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };

 static int exynos_dpi_get_modes(struct drm_connector *connector)

[PATCH 09/14] drm/exynos: atomic phase 1: add .mode_set_nofb() callback

[Gustavo Padovan]

From: Gustavo Padovan 

The new atomic infrastructure needs the .mode_set_nofb() callback to
update CRTC timings before setting any plane.

Signed-off-by: Gustavo Padovan 

Conflicts:
drivers/gpu/drm/exynos/exynos_drm_crtc.c
---
 drivers/gpu/drm/exynos/exynos_drm_crtc.c | 55 +++-
 1 file changed, 11 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c 
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index be36cca..17b64f8 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -81,53 +81,19 @@ exynos_drm_crtc_mode_fixup(struct drm_crtc *crtc,
return true;
 }

-static int
-exynos_drm_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode,
- struct drm_display_mode *adjusted_mode, int x, int y,
- struct drm_framebuffer *old_fb)
-{
-   struct drm_framebuffer *fb = crtc->primary->fb;
-   unsigned int crtc_w;
-   unsigned int crtc_h;
-   int ret;
-
-   ret = exynos_check_plane(crtc->primary, fb);
-   if (ret < 0)
-   return ret;
-
-   crtc_w = fb->width - x;
-   crtc_h = fb->height - y;
-   exynos_plane_mode_set(crtc->primary, crtc, fb, 0, 0,
- crtc_w, crtc_h, x, y, crtc_w, crtc_h);
-
-   return 0;
-}
-
-static int exynos_drm_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
- struct drm_framebuffer *old_fb)
+static void
+exynos_drm_crtc_mode_set_nofb(struct drm_crtc *crtc)
 {
struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
-   struct drm_framebuffer *fb = crtc->primary->fb;
-   unsigned int crtc_w;
-   unsigned int crtc_h;
-   int ret;
+   struct drm_display_mode *adjusted_mode;

-   /* when framebuffer changing is requested, crtc's dpms should be on */
-   if (exynos_crtc->dpms > DRM_MODE_DPMS_ON) {
-   DRM_ERROR("failed framebuffer changing request.\n");
-   return -EPERM;
-   }
-
-   ret = exynos_check_plane(crtc->primary, fb);
-   if (ret)
-   return ret;
+   if (WARN_ON(!crtc->state))
+   return;

-   crtc_w = fb->width - x;
-   crtc_h = fb->height - y;
-   exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
-   crtc_w, crtc_h, x, y, crtc_w, crtc_h);
+   adjusted_mode = >state->adjusted_mode;

-   return 0;
+   if (exynos_crtc->ops->commit)
+   exynos_crtc->ops->commit(exynos_crtc);
 }

 static void exynos_drm_crtc_disable(struct drm_crtc *crtc)
@@ -184,8 +150,9 @@ static struct drm_crtc_helper_funcs 
exynos_crtc_helper_funcs = {
.prepare= exynos_drm_crtc_prepare,
.commit = exynos_drm_crtc_commit,
.mode_fixup = exynos_drm_crtc_mode_fixup,
-   .mode_set   = exynos_drm_crtc_mode_set,
-   .mode_set_base  = exynos_drm_crtc_mode_set_base,
+   .mode_set   = drm_helper_crtc_mode_set,
+   .mode_set_nofb  = exynos_drm_crtc_mode_set_nofb,
+   .mode_set_base  = drm_helper_crtc_mode_set_base,
.disable= exynos_drm_crtc_disable,
.atomic_begin   = exynos_crtc_atomic_begin,
.atomic_flush   = exynos_crtc_atomic_flush,
-- 
1.9.3

[PATCH 08/14] drm/exynos: atomic phase 1: add atomic_begin()/atomic_flush()

[Gustavo Padovan]

From: Gustavo Padovan 

Add CRTC callbacks .atomic_begin() .atomic_flush(). On exynos they
unprotect the windows before the commit and protects it after based on
a plane mask tha store which plane will be updated.

For that we create two new exynos_crtc callbacks: .win_protect() and
.win_unprotect(). The only driver that implement those now is FIMD.

Signed-off-by: Gustavo Padovan 
---
 drivers/gpu/drm/exynos/exynos_drm_crtc.c  | 34 ++
 drivers/gpu/drm/exynos/exynos_drm_drv.h   |  4 +++
 drivers/gpu/drm/exynos/exynos_drm_fimd.c  | 57 ++-
 drivers/gpu/drm/exynos/exynos_drm_plane.c |  4 +++
 4 files changed, 82 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c 
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 09d4780..be36cca 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -147,6 +147,38 @@ static void exynos_drm_crtc_disable(struct drm_crtc *crtc)
}
 }

+static void exynos_crtc_atomic_begin(struct drm_crtc *crtc)
+{
+   struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
+   struct drm_plane *plane;
+   int index = 0;
+
+   list_for_each_entry(plane, >dev->mode_config.plane_list, head) {
+   if (exynos_crtc->ops->win_protect &&
+   exynos_crtc->plane_mask & (0x01 << index))
+   exynos_crtc->ops->win_protect(exynos_crtc, index);
+
+   index++;
+   }
+}
+
+static void exynos_crtc_atomic_flush(struct drm_crtc *crtc)
+{
+   struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
+   struct drm_plane *plane;
+   int index = 0;
+
+   list_for_each_entry(plane, >dev->mode_config.plane_list, head) {
+   if (exynos_crtc->ops->win_unprotect &&
+   exynos_crtc->plane_mask & (0x01 << index))
+   exynos_crtc->ops->win_unprotect(exynos_crtc, index);
+
+   index++;
+   }
+
+   exynos_crtc->plane_mask = 0;
+}
+
 static struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
.dpms   = exynos_drm_crtc_dpms,
.prepare= exynos_drm_crtc_prepare,
@@ -155,6 +187,8 @@ static struct drm_crtc_helper_funcs 
exynos_crtc_helper_funcs = {
.mode_set   = exynos_drm_crtc_mode_set,
.mode_set_base  = exynos_drm_crtc_mode_set_base,
.disable= exynos_drm_crtc_disable,
+   .atomic_begin   = exynos_crtc_atomic_begin,
+   .atomic_flush   = exynos_crtc_atomic_flush,
 };

 static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h 
b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index cad54e7..43efd9f 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -194,6 +194,8 @@ struct exynos_drm_crtc_ops {
void (*win_enable)(struct exynos_drm_crtc *crtc, int zpos);
void (*win_disable)(struct exynos_drm_crtc *crtc, int zpos);
void (*te_handler)(struct exynos_drm_crtc *crtc);
+   void (*win_protect)(struct exynos_drm_crtc *crtc, int zpos);
+   void (*win_unprotect)(struct exynos_drm_crtc *crtc, int zpos);
 };

 enum exynos_crtc_mode {
@@ -214,6 +216,7 @@ enum exynos_crtc_mode {
  * we can refer to the crtc to current hardware interrupt occurred through
  * this pipe value.
  * @dpms: store the crtc dpms value
+ * @plane_mask: store planes to be updated on atomic modesetting
  * @mode: store the crtc mode value
  * @event: vblank event that is currently queued for flip
  * @ops: pointer to callbacks for exynos drm specific functionality
@@ -224,6 +227,7 @@ struct exynos_drm_crtc {
enum exynos_drm_output_type type;
unsigned intpipe;
unsigned intdpms;
+   unsigned intplane_mask;
enum exynos_crtc_mode   mode;
wait_queue_head_t   pending_flip_queue;
struct drm_pending_vblank_event *event;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c 
b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index ebb4cdc..f498d86 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -585,6 +585,16 @@ static void fimd_shadow_protect_win(struct fimd_context 
*ctx,
 {
u32 reg, bits, val;

+   /*
+* SHADOWCON/PRTCON register is used for enabling timing.
+*
+* for example, once only width value of a register is set,
+* if the dma is started then fimd hardware could malfunction so
+* with protect window setting, the register fields with prefix '_F'
+* wouldn't be updated at vsync also but updated once unprotect window
+* is set.
+*/
+
if (ctx->driver_data->has_shadowcon) {
reg = SHADOWCON;
bits = SHADOWCON_WINx_PROTECT(win);
@@ -627,20 

[PATCH 07/14] drm/exynos: atomic phase 1: use drm_plane_helper_disable()

[Gustavo Padovan]

From: Gustavo Padovan 

The atomic helper to disable planes also uses the optional
.atomic_disable() helper. The unique operation it does is calling
.win_disable()

exynos_drm_fb_get_buf_cnt() needs a fb check too to avoid a null pointer.

Signed-off-by: Gustavo Padovan 
---
 drivers/gpu/drm/exynos/exynos_drm_fb.c|  2 +-
 drivers/gpu/drm/exynos/exynos_drm_plane.c | 14 +-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c 
b/drivers/gpu/drm/exynos/exynos_drm_fb.c
index d346d1e..470456d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
@@ -136,7 +136,7 @@ unsigned int exynos_drm_fb_get_buf_cnt(struct 
drm_framebuffer *fb)

exynos_fb = to_exynos_fb(fb);

-   return exynos_fb->buf_cnt;
+   return exynos_fb ? exynos_fb->buf_cnt : 0;
 }

 struct drm_framebuffer *
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c 
b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index 2c356b9..a3b0687 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -219,7 +219,7 @@ static int exynos_plane_set_property(struct drm_plane 
*plane,

 static struct drm_plane_funcs exynos_plane_funcs = {
.update_plane   = drm_plane_helper_update,
-   .disable_plane  = exynos_disable_plane,
+   .disable_plane  = drm_plane_helper_disable,
.destroy= exynos_plane_destroy,
.set_property   = exynos_plane_set_property,
 };
@@ -242,9 +242,21 @@ static void exynos_plane_atomic_update(struct drm_plane 
*plane,
state->src_w >> 16, state->src_h >> 16);
 }

+static void exynos_plane_atomic_disable(struct drm_plane *plane,
+   struct drm_plane_state *old_state)
+{
+   struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
+   struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(old_state->crtc);
+
+   if (exynos_crtc->ops->win_disable)
+   exynos_crtc->ops->win_disable(exynos_crtc,
+ exynos_plane->zpos);
+}
+
 static const struct drm_plane_helper_funcs plane_helper_funcs = {
.atomic_check = exynos_plane_atomic_check,
.atomic_update = exynos_plane_atomic_update,
+   .atomic_disable = exynos_plane_atomic_disable,
 };

 static void exynos_plane_attach_zpos_property(struct drm_plane *plane)
-- 
1.9.3

[PATCH 06/14] drm/exynos: atomic phase 1: use drm_plane_helper_update()

[Gustavo Padovan]

From: Gustavo Padovan 

Rip out the check from exynos_update_plane() and create
exynos_check_plane() for the check phase enabling use to use
the atomic helpers to call our check and update phases when updating
planes.

Update all users of exynos_update_plane() accordingly to call
exynos_check_plane() before.

Signed-off-by: Gustavo Padovan 
---
 drivers/gpu/drm/exynos/exynos_drm_crtc.c  | 29 
 drivers/gpu/drm/exynos/exynos_drm_plane.c | 37 ++-
 drivers/gpu/drm/exynos/exynos_drm_plane.h |  2 +-
 3 files changed, 43 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c 
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index ecbcade..09d4780 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -110,6 +110,7 @@ static int exynos_drm_crtc_mode_set_base(struct drm_crtc 
*crtc, int x, int y,
struct drm_framebuffer *fb = crtc->primary->fb;
unsigned int crtc_w;
unsigned int crtc_h;
+   int ret;

/* when framebuffer changing is requested, crtc's dpms should be on */
if (exynos_crtc->dpms > DRM_MODE_DPMS_ON) {
@@ -117,11 +118,16 @@ static int exynos_drm_crtc_mode_set_base(struct drm_crtc 
*crtc, int x, int y,
return -EPERM;
}

+   ret = exynos_check_plane(crtc->primary, fb);
+   if (ret)
+   return ret;
+
crtc_w = fb->width - x;
crtc_h = fb->height - y;
+   exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
+   crtc_w, crtc_h, x, y, crtc_w, crtc_h);

-   return exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
-  crtc_w, crtc_h, x, y, crtc_w, crtc_h);
+   return 0;
 }

 static void exynos_drm_crtc_disable(struct drm_crtc *crtc)
@@ -158,7 +164,6 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc,
 {
struct drm_device *dev = crtc->dev;
struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
-   struct drm_framebuffer *old_fb = crtc->primary->fb;
unsigned int crtc_w, crtc_h;
int ret;

@@ -177,6 +182,10 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc,
goto out;
}

+   ret = exynos_check_plane(crtc->primary, fb);
+   if (ret)
+   goto out;
+
ret = drm_vblank_get(dev, exynos_crtc->pipe);
if (ret) {
DRM_DEBUG("failed to acquire vblank counter\n");
@@ -195,17 +204,9 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc,
crtc->primary->fb = fb;
crtc_w = fb->width - crtc->x;
crtc_h = fb->height - crtc->y;
-   ret = exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
- crtc_w, crtc_h, crtc->x, crtc->y,
- crtc_w, crtc_h);
-   if (ret) {
-   crtc->primary->fb = old_fb;
-   spin_lock_irq(>event_lock);
-   exynos_crtc->event = NULL;
-   drm_vblank_put(dev, exynos_crtc->pipe);
-   spin_unlock_irq(>event_lock);
-   return ret;
-   }
+   exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
+   crtc_w, crtc_h, crtc->x, crtc->y,
+   crtc_w, crtc_h);

return 0;

diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c 
b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index dc13621..2c356b9 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -167,21 +167,15 @@ void exynos_plane_dpms(struct drm_plane *plane, int mode)
}
 }

-int
+void
 exynos_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 struct drm_framebuffer *fb, int crtc_x, int crtc_y,
 unsigned int crtc_w, unsigned int crtc_h,
 uint32_t src_x, uint32_t src_y,
 uint32_t src_w, uint32_t src_h)
 {
-
struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
-   int ret;
-
-   ret = exynos_check_plane(plane, fb);
-   if (ret < 0)
-   return ret;

exynos_plane_mode_set(plane, crtc, fb, crtc_x, crtc_y,
  crtc_w, crtc_h, src_x >> 16, src_y >> 16,
@@ -189,8 +183,6 @@ exynos_update_plane(struct drm_plane *plane, struct 
drm_crtc *crtc,

if (exynos_crtc->ops->win_commit)
exynos_crtc->ops->win_commit(exynos_crtc, exynos_plane->zpos);
-
-   return 0;
 }

 static int exynos_disable_plane(struct drm_plane *plane)
@@ -226,12 +218,35 @@ static int exynos_plane_set_property(struct drm_plane 
*plane,
 }

 static struct drm_plane_funcs exynos_plane_funcs = {
-   .update_plane   = exynos_update_plane,
+   .update_plane   = drm_plane_helper_update,
.disable_plane  = 

[PATCH 05/14] drm/exynos: do not copy adjusted mode into mode during crtc mode_set

[Gustavo Padovan]

From: Daniel Kurtz 

The 'mode' is the modeline information which specifies the ideal mode
requested by the mode set initiator (usually userspace).
The 'adjusted_mode' is the actual hardware mode after all the crtcs
and encoders have had a chance to "fix it up".

The adjusted_mode starts as a duplicate of the mode in
drm_crtc_helper_set_mode(), and gets modified as required.  There is no
reason to touch the original requested mode.

In fact, doing so will cause us to think a new mode is being requested
whenever userspace tries to establish a new framebuffer with
drmModeSetCrtc(), since userspace will still be using the ideal mode, but
the crtc will be incorrectly comparing it against the adjusted_mode.

Signed-off-by: Daniel Kurtz 
Signed-off-by: Gustavo Padovan 
---
 drivers/gpu/drm/exynos/exynos_drm_crtc.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c 
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index d504f0b..ecbcade 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -91,12 +91,6 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc, struct 
drm_display_mode *mode,
unsigned int crtc_h;
int ret;

-   /*
-* copy the mode data adjusted by mode_fixup() into crtc->mode
-* so that hardware can be seet to proper mode.
-*/
-   memcpy(>mode, adjusted_mode, sizeof(*adjusted_mode));
-
ret = exynos_check_plane(crtc->primary, fb);
if (ret < 0)
return ret;
-- 
1.9.3

[PATCH 04/14] drm/exynos: remove struct *_win_data abstraction on planes

[Gustavo Padovan]

From: Gustavo Padovan 

struct {fimd,mixer,vidi}_win_data was just keeping the same data
as struct exynos_drm_plane thus get ride of it and use exynos_drm_plane
directly.

It changes how planes are created and remove .win_mode_set() callback
that was only filling all *_win_data structs.

Signed-off-by: Gustavo Padovan 
---
 drivers/gpu/drm/exynos/exynos_drm_crtc.c  |   9 +-
 drivers/gpu/drm/exynos/exynos_drm_crtc.h  |   1 +
 drivers/gpu/drm/exynos/exynos_drm_drv.c   |  14 --
 drivers/gpu/drm/exynos/exynos_drm_drv.h   |   5 +-
 drivers/gpu/drm/exynos/exynos_drm_fimd.c  | 182 ++---
 drivers/gpu/drm/exynos/exynos_drm_plane.c |  20 +--
 drivers/gpu/drm/exynos/exynos_drm_plane.h |   6 +-
 drivers/gpu/drm/exynos/exynos_drm_vidi.c  | 123 +
 drivers/gpu/drm/exynos/exynos_mixer.c | 212 +++---
 9 files changed, 183 insertions(+), 389 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c 
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index ad675fb..d504f0b 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -296,13 +296,13 @@ static void exynos_drm_crtc_attach_mode_property(struct 
drm_crtc *crtc)
 }

 struct exynos_drm_crtc *exynos_drm_crtc_create(struct drm_device *drm_dev,
+  struct drm_plane *plane,
   int pipe,
   enum exynos_drm_output_type type,
   struct exynos_drm_crtc_ops *ops,
   void *ctx)
 {
struct exynos_drm_crtc *exynos_crtc;
-   struct drm_plane *plane;
struct exynos_drm_private *private = drm_dev->dev_private;
struct drm_crtc *crtc;
int ret;
@@ -318,12 +318,6 @@ struct exynos_drm_crtc *exynos_drm_crtc_create(struct 
drm_device *drm_dev,
exynos_crtc->type = type;
exynos_crtc->ops = ops;
exynos_crtc->ctx = ctx;
-   plane = exynos_plane_init(drm_dev, 1 << pipe,
- DRM_PLANE_TYPE_PRIMARY);
-   if (IS_ERR(plane)) {
-   ret = PTR_ERR(plane);
-   goto err_plane;
-   }

crtc = _crtc->base;

@@ -342,7 +336,6 @@ struct exynos_drm_crtc *exynos_drm_crtc_create(struct 
drm_device *drm_dev,

 err_crtc:
plane->funcs->destroy(plane);
-err_plane:
kfree(exynos_crtc);
return ERR_PTR(ret);
 }
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.h 
b/drivers/gpu/drm/exynos/exynos_drm_crtc.h
index 628b787..0ecd8fc 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.h
@@ -18,6 +18,7 @@
 #include "exynos_drm_drv.h"

 struct exynos_drm_crtc *exynos_drm_crtc_create(struct drm_device *drm_dev,
+  struct drm_plane *plane,
   int pipe,
   enum exynos_drm_output_type type,
   struct exynos_drm_crtc_ops *ops,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c 
b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 737164d..778c91e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -55,7 +55,6 @@ static int exynos_drm_load(struct drm_device *dev, unsigned 
long flags)
 {
struct exynos_drm_private *private;
int ret;
-   int nr;

private = kzalloc(sizeof(struct exynos_drm_private), GFP_KERNEL);
if (!private)
@@ -80,19 +79,6 @@ static int exynos_drm_load(struct drm_device *dev, unsigned 
long flags)

exynos_drm_mode_config_init(dev);

-   for (nr = 0; nr < MAX_PLANE; nr++) {
-   struct drm_plane *plane;
-   unsigned long possible_crtcs = (1 << MAX_CRTC) - 1;
-
-   plane = exynos_plane_init(dev, possible_crtcs,
- DRM_PLANE_TYPE_OVERLAY);
-   if (!IS_ERR(plane))
-   continue;
-
-   ret = PTR_ERR(plane);
-   goto err_mode_config_cleanup;
-   }
-
/* setup possible_clones. */
exynos_drm_encoder_setup(dev);

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h 
b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index 7411af2..cad54e7 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -78,6 +78,7 @@ enum exynos_drm_output_type {
  * @transparency: transparency on or off.
  * @activated: activated or not.
  * @enabled: enabled or not.
+ * @resume: to resume or not.
  *
  * this structure is common to exynos SoC and its contents would be copied
  * to hardware specific overlay info.
@@ -112,6 +113,7 @@ struct exynos_drm_plane {
bool transparency:1;
bool activated:1;
bool enabled:1;
+   

[PATCH 03/14] drm/exynos: remove leftover functions declarations

[Gustavo Padovan]

From: Gustavo Padovan 

These functions were already removed by previous cleanup work, but these
ones were left behind.

Signed-off-by: Gustavo Padovan 
Acked-by: Joonyoung Shim 
---
 drivers/gpu/drm/exynos/exynos_drm_crtc.h | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.h 
b/drivers/gpu/drm/exynos/exynos_drm_crtc.h
index 6258b80..628b787 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.h
@@ -27,12 +27,6 @@ void exynos_drm_crtc_disable_vblank(struct drm_device *dev, 
int pipe);
 void exynos_drm_crtc_finish_pageflip(struct drm_device *dev, int pipe);
 void exynos_drm_crtc_complete_scanout(struct drm_framebuffer *fb);

-void exynos_drm_crtc_plane_mode_set(struct drm_crtc *crtc,
-   struct exynos_drm_plane *plane);
-void exynos_drm_crtc_plane_commit(struct drm_crtc *crtc, int zpos);
-void exynos_drm_crtc_plane_enable(struct drm_crtc *crtc, int zpos);
-void exynos_drm_crtc_plane_disable(struct drm_crtc *crtc, int zpos);
-
 /* This function gets pipe value to crtc device matched with out_type. */
 int exynos_drm_crtc_get_pipe_from_type(struct drm_device *drm_dev,
unsigned int out_type);
-- 
1.9.3

[PATCH 02/14] drm/exynos: Remove exynos_plane_dpms() call with no effect

[Gustavo Padovan]

From: Gustavo Padovan 

exynos_plane_dpms(DRM_MODE_DPMS_ON) calls the win_enable()'s callback
from the underlying layer. However neither one of these layers implement
win_enable() - FIMD, Mixer and VIDI. Thus the call to exynos_plane_dpms()
is pointless.

Signed-off-by: Gustavo Padovan 
---
 drivers/gpu/drm/exynos/exynos_drm_crtc.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c 
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index b2a4b84..ad675fb 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -65,8 +65,6 @@ static void exynos_drm_crtc_commit(struct drm_crtc *crtc)

if (exynos_crtc->ops->commit)
exynos_crtc->ops->commit(exynos_crtc);
-
-   exynos_plane_dpms(crtc->primary, DRM_MODE_DPMS_ON);
 }

 static bool
-- 
1.9.3

[PATCH 01/14] drm/exynos: track vblank events on a per crtc basis

[Gustavo Padovan]

From: Mandeep Singh Baines 

The goal of the change is to make sure we send the vblank event on the
current vblank. My hope is to fix any races that might be causing flicker.
After this change I only see a flicker in the transition plymouth and
X11.

Simplified the code by tracking vblank events on a per-crtc basis. This
allowed me to remove all error paths from the callback. It also allowed
me to remove the vblank wait from the callback.

Signed-off-by: Mandeep Singh Baines 
Signed-off-by: Gustavo Padovan 
---
 drivers/gpu/drm/exynos/exynos_drm_crtc.c | 92 +++-
 drivers/gpu/drm/exynos/exynos_drm_drv.c  | 13 -
 drivers/gpu/drm/exynos/exynos_drm_drv.h  |  6 +--
 3 files changed, 44 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c 
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index a85c451..b2a4b84 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -34,9 +34,8 @@ static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int 
mode)
if (mode > DRM_MODE_DPMS_ON) {
/* wait for the completion of page flip. */
if (!wait_event_timeout(exynos_crtc->pending_flip_queue,
-   !atomic_read(_crtc->pending_flip),
-   HZ/20))
-   atomic_set(_crtc->pending_flip, 0);
+   (exynos_crtc->event == NULL), HZ/20))
+   exynos_crtc->event = NULL;
drm_crtc_vblank_off(crtc);
}

@@ -166,11 +165,10 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc 
*crtc,
 uint32_t page_flip_flags)
 {
struct drm_device *dev = crtc->dev;
-   struct exynos_drm_private *dev_priv = dev->dev_private;
struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
struct drm_framebuffer *old_fb = crtc->primary->fb;
unsigned int crtc_w, crtc_h;
-   int ret = -EINVAL;
+   int ret;

/* when the page flip is requested, crtc's dpms should be on */
if (exynos_crtc->dpms > DRM_MODE_DPMS_ON) {
@@ -178,48 +176,49 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc 
*crtc,
return -EINVAL;
}

-   mutex_lock(>struct_mutex);
+   if (!event)
+   return -EINVAL;

-   if (event) {
-   /*
-* the pipe from user always is 0 so we can set pipe number
-* of current owner to event.
-*/
-   event->pipe = exynos_crtc->pipe;
+   spin_lock_irq(>event_lock);
+   if (exynos_crtc->event) {
+   ret = -EBUSY;
+   goto out;
+   }

-   ret = drm_vblank_get(dev, exynos_crtc->pipe);
-   if (ret) {
-   DRM_DEBUG("failed to acquire vblank counter\n");
+   ret = drm_vblank_get(dev, exynos_crtc->pipe);
+   if (ret) {
+   DRM_DEBUG("failed to acquire vblank counter\n");
+   goto out;
+   }

-   goto out;
-   }
+   exynos_crtc->event = event;
+   spin_unlock_irq(>event_lock);

+   /*
+* the pipe from user always is 0 so we can set pipe number
+* of current owner to event.
+*/
+   event->pipe = exynos_crtc->pipe;
+
+   crtc->primary->fb = fb;
+   crtc_w = fb->width - crtc->x;
+   crtc_h = fb->height - crtc->y;
+   ret = exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
+ crtc_w, crtc_h, crtc->x, crtc->y,
+ crtc_w, crtc_h);
+   if (ret) {
+   crtc->primary->fb = old_fb;
spin_lock_irq(>event_lock);
-   list_add_tail(>base.link,
-   _priv->pageflip_event_list);
-   atomic_set(_crtc->pending_flip, 1);
+   exynos_crtc->event = NULL;
+   drm_vblank_put(dev, exynos_crtc->pipe);
spin_unlock_irq(>event_lock);
-
-   crtc->primary->fb = fb;
-   crtc_w = fb->width - crtc->x;
-   crtc_h = fb->height - crtc->y;
-   ret = exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
- crtc_w, crtc_h, crtc->x, crtc->y,
- crtc_w, crtc_h);
-   if (ret) {
-   crtc->primary->fb = old_fb;
-
-   spin_lock_irq(>event_lock);
-   drm_vblank_put(dev, exynos_crtc->pipe);
-   list_del(>base.link);
-   atomic_set(_crtc->pending_flip, 0);
-   spin_unlock_irq(>event_lock);
-
-   goto out;
-   }
+   return ret;
}
+
+   return 0;
+
 out:
-   mutex_unlock(>struct_mutex);
+   spin_unlock_irq(>event_lock);
 

[PATCH 00/14] drm/exynos: cleanups + atomic phases 1 and 2

[Gustavo Padovan]

From: Gustavo Padovan 

Hi,

This series clean ups a few more paths from exynos-drm with the most important
being the removal of the global page flip queue and the removal in driver
internal data (struct *_win_data) that was replicating plane data.

Following these patches comes the first step torwards atomic modesetting
support on exynos.

This series is applied on top of 3 patches[0][1][2] that were sent recently to
dri-devel.

Gustavo

---
[0] http://www.spinics.net/lists/linux-samsung-soc/msg41867.html
[1] http://lists.freedesktop.org/archives/dri-devel/2015-January/076504.html
[2] http://lists.freedesktop.org/archives/dri-devel/2015-January/076505.html


Daniel Kurtz (1):
  drm/exynos: do not copy adjusted mode into mode during crtc mode_set

Gustavo Padovan (12):
  drm/exynos: Remove exynos_plane_dpms() call with no effect
  drm/exynos: remove leftover functions declarations
  drm/exynos: remove struct *_win_data abstraction on planes
  drm/exynos: atomic phase 1: use drm_plane_helper_update()
  drm/exynos: atomic phase 1: use drm_plane_helper_disable()
  drm/exynos: atomic phase 1: add atomic_begin()/atomic_flush()
  drm/exynos: atomic phase 1: add .mode_set_nofb() callback
  drm/exynos: atomic phase 2: wire up state reset(), duplicate() and
destroy()
  drm/exynos: atomic phase 2: keep track of framebuffer pointer
  drm/exynos: make exynos_plane_mode_set() static
  drm/exynos: use correct pipe number on vblank event
  drm/exynos: remove exynos_disable_plane()

Mandeep Singh Baines (1):
  drm/exynos: track vblank events on a per crtc basis

 drivers/gpu/drm/bridge/ptn3460.c  |   4 +
 drivers/gpu/drm/exynos/exynos_dp_core.c   |   4 +
 drivers/gpu/drm/exynos/exynos_drm_connector.c |   4 +
 drivers/gpu/drm/exynos/exynos_drm_crtc.c  | 203 +++---
 drivers/gpu/drm/exynos/exynos_drm_crtc.h  |   7 +-
 drivers/gpu/drm/exynos/exynos_drm_dpi.c   |   4 +
 drivers/gpu/drm/exynos/exynos_drm_drv.c   |  29 +---
 drivers/gpu/drm/exynos/exynos_drm_drv.h   |  15 +-
 drivers/gpu/drm/exynos/exynos_drm_dsi.c   |   4 +
 drivers/gpu/drm/exynos/exynos_drm_fb.c|   2 +-
 drivers/gpu/drm/exynos/exynos_drm_fimd.c  | 235 --
 drivers/gpu/drm/exynos/exynos_drm_plane.c |  99 ++-
 drivers/gpu/drm/exynos/exynos_drm_plane.h |  13 +-
 drivers/gpu/drm/exynos/exynos_drm_vidi.c  | 127 --
 drivers/gpu/drm/exynos/exynos_hdmi.c  |   4 +
 drivers/gpu/drm/exynos/exynos_mixer.c | 212 ---
 16 files changed, 408 insertions(+), 558 deletions(-)

-- 
1.9.3

[Linaro-mm-sig] [RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms

[Arnd Bergmann]

On Tuesday 03 February 2015 15:54:04 Russell King - ARM Linux wrote:
> On Tue, Feb 03, 2015 at 04:31:13PM +0100, Arnd Bergmann wrote:
> > The dma_map_* interfaces assign the virtual addresses internally,
> > using typically either a global address space for all devices, or one
> > address space per device.
> 
> We shouldn't be doing one address space per device for precisely this
> reason.  We should be doing one address space per *bus*.  I did have
> a nice diagram to illustrate the point in my previous email, but I
> deleted it, I wish I hadn't... briefly:
> 
> Fig. 1.
>  +--+
>  |+-+  device   |
> CPU--L1cache--L2cache--Memory--SysMMU---IOMMU--> |
>  |+-+   |
>  +--+
> 
> Fig.1 represents what I'd call the "GPU" issue that we're talking about
> in this thread.
> 
> Fig. 2.
> CPU--L1cache--L2cache--Memory--SysMMU-IOMMU--device
> 
> The DMA API should be responsible (at the very least) for everything on
> the left of "" in and should be providing a dma_addr_t which is
> representative of what the device (in Fig.1) as a whole sees.  That's
> the "system" part.  
> 
> I believe this is the approach which is taken by x86 and similar platforms,
> simply because they tend not to have an IOMMU on individual devices (and
> if they did, eg, on a PCI card, it's clearly the responsibility of the
> device driver.)
> 
> Whether the DMA API also handles the IOMMU in Fig.1 or 2 is questionable.
> For fig.2, it is entirely possible that the same device could appear
> without an IOMMU, and in that scenario, you would want the IOMMU to be
> handled transparently.
> 
> However, by doing so for everything, you run into exactly the problem
> which is being discussed here - the need to separate out the cache
> coherency from the IOMMU aspects.  You probably also have a setup very
> similar to fig.1 (which is certainly true of Vivante GPUs.)
> 
> If you have the need to separately control both, then using the DMA API
> to encapsulate both does not make sense - at which point, the DMA API
> should be responsible for the minimum only - in other words, everything
> to the left of  (so including the system MMU.)  The control of
> the device IOMMU should be the responsibility of device driver in this
> case.
> 
> So, dma_map_sg() would be responsible for dealing with the CPU cache
> coherency issues, and setting up the system MMU.  dma_sync_*() would
> be responsible for the CPU cache coherency issues, and dma_unmap_sg()
> would (again) deal with the CPU cache and tear down the system MMU
> mappings.
> 
> Meanwhile, the device driver has ultimate control over its IOMMU, the
> creation and destruction of mappings and context switches at the
> appropriate times.

I agree for the case you are describing here. From what I understood
from Rob was that he is looking at something more like:

Fig 3
CPU--L1cache--L2cache--Memory--IOMMU-device

where the IOMMU controls one or more contexts per device, and is
shared across GPU and non-GPU devices. Here, we need to use the
dmap-mapping interface to set up the IO page table for any device
that is unable to address all of system RAM, and we can use it
for purposes like isolation of the devices. There are also cases
where using the IOMMU is not optional.

So unlike the scenario you describe, the driver cannot at the
same time control the cache (using the dma-mapping API) and
the I/O page tables (using the iommu API or some internal
functions).

Arnd

[Bug 88947] "Unknown" display appears with no external display connected

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

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

Bug ID: 88947
   Summary: "Unknown" display appears with no external display
connected
   Product: DRI
   Version: DRI git
  Hardware: x86-64 (AMD64)
OS: Linux (All)
Status: NEW
  Severity: normal
  Priority: medium
 Component: General
  Assignee: dri-devel at lists.freedesktop.org
  Reporter: ypwong at gmail.com

1. boot to desktop with no external monitor connected
2. Open the "Display" utility, check whether there is an "Unknown" display. You
will be able to move the cursor out of your screen, even open an application on
the extended display.

Tested with 2015-01-29 drm-intel-nightly.

00:02.0 VGA compatible controller [0300]: Intel Corporation 2nd Generation Core
Processor Family Integrated Graphics Controller [8086:0106] (rev 09) (prog-if
00 [VGA controller])
01:00.0 VGA compatible controller [0300]: NVIDIA Corporation GF108M [GeForce GT
630M] [10de:0de9] (rev a1) (prog-if 00 [VGA controller])

Original bug:
https://bugs.launchpad.net/ubuntu/+source/linux-lts-trusty/+bug/1355562

-- 
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/20150203/f77155e2/attachment.html>

[Linaro-mm-sig] [RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms

[Russell King - ARM Linux]

On Tue, Feb 03, 2015 at 11:22:01AM -0500, Rob Clark wrote:
> On Tue, Feb 3, 2015 at 11:12 AM, Arnd Bergmann  wrote:
> > I agree for the case you are describing here. From what I understood
> > from Rob was that he is looking at something more like:
> >
> > Fig 3
> > CPU--L1cache--L2cache--Memory--IOMMU-device
> >
> > where the IOMMU controls one or more contexts per device, and is
> > shared across GPU and non-GPU devices. Here, we need to use the
> > dmap-mapping interface to set up the IO page table for any device
> > that is unable to address all of system RAM, and we can use it
> > for purposes like isolation of the devices. There are also cases
> > where using the IOMMU is not optional.
> 
> 
> Actually, just to clarify, the IOMMU instance is specific to the GPU..
> not shared with other devices.  Otherwise managing multiple contexts
> would go quite badly..
> 
> But other devices have their own instance of the same IOMMU.. so same
> driver could be used.

Okay, so that is my Fig.2 case, and we don't have to worry about Fig.3.

One thing I forgot in Fig.1/2 which my original did have were to mark
the system MMU as optional.  (Think an ARM64 with SMMU into a 32-bit
peripheral bus.)  Do we support stacked MMUs in the DMA API?  We may
need to if we keep IOMMUs in the DMA API.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[Linaro-mm-sig] [RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms

[Russell King - ARM Linux]

On Tue, Feb 03, 2015 at 05:12:40PM +0100, Arnd Bergmann wrote:
> On Tuesday 03 February 2015 15:54:04 Russell King - ARM Linux wrote:
> > On Tue, Feb 03, 2015 at 04:31:13PM +0100, Arnd Bergmann wrote:
> > > The dma_map_* interfaces assign the virtual addresses internally,
> > > using typically either a global address space for all devices, or one
> > > address space per device.
> > 
> > We shouldn't be doing one address space per device for precisely this
> > reason.  We should be doing one address space per *bus*.  I did have
> > a nice diagram to illustrate the point in my previous email, but I
> > deleted it, I wish I hadn't... briefly:
> > 
> > Fig. 1.
> >  +--+
> >  |+-+  device   |
> > CPU--L1cache--L2cache--Memory--SysMMU---IOMMU--> |
> >  |+-+   |
> >  +--+
> > 
> > Fig.1 represents what I'd call the "GPU" issue that we're talking about
> > in this thread.
> > 
> > Fig. 2.
> > CPU--L1cache--L2cache--Memory--SysMMU-IOMMU--device
> > 
> > The DMA API should be responsible (at the very least) for everything on
> > the left of "" in and should be providing a dma_addr_t which is
> > representative of what the device (in Fig.1) as a whole sees.  That's
> > the "system" part.  
> > 
> > I believe this is the approach which is taken by x86 and similar platforms,
> > simply because they tend not to have an IOMMU on individual devices (and
> > if they did, eg, on a PCI card, it's clearly the responsibility of the
> > device driver.)
> > 
> > Whether the DMA API also handles the IOMMU in Fig.1 or 2 is questionable.
> > For fig.2, it is entirely possible that the same device could appear
> > without an IOMMU, and in that scenario, you would want the IOMMU to be
> > handled transparently.
> > 
> > However, by doing so for everything, you run into exactly the problem
> > which is being discussed here - the need to separate out the cache
> > coherency from the IOMMU aspects.  You probably also have a setup very
> > similar to fig.1 (which is certainly true of Vivante GPUs.)
> > 
> > If you have the need to separately control both, then using the DMA API
> > to encapsulate both does not make sense - at which point, the DMA API
> > should be responsible for the minimum only - in other words, everything
> > to the left of  (so including the system MMU.)  The control of
> > the device IOMMU should be the responsibility of device driver in this
> > case.
> > 
> > So, dma_map_sg() would be responsible for dealing with the CPU cache
> > coherency issues, and setting up the system MMU.  dma_sync_*() would
> > be responsible for the CPU cache coherency issues, and dma_unmap_sg()
> > would (again) deal with the CPU cache and tear down the system MMU
> > mappings.
> > 
> > Meanwhile, the device driver has ultimate control over its IOMMU, the
> > creation and destruction of mappings and context switches at the
> > appropriate times.
> 
> I agree for the case you are describing here. From what I understood
> from Rob was that he is looking at something more like:
> 
> Fig 3
> CPU--L1cache--L2cache--Memory--IOMMU-device
> 
> where the IOMMU controls one or more contexts per device, and is
> shared across GPU and non-GPU devices. Here, we need to use the
> dmap-mapping interface to set up the IO page table for any device
> that is unable to address all of system RAM, and we can use it
> for purposes like isolation of the devices. There are also cases
> where using the IOMMU is not optional.

Okay, but switching contexts is not something which the DMA API has
any knowledge of (so it can't know which context to associate with
which mapping.)  While it knows which device, it has no knowledge
(nor is there any way for it to gain knowledge) about contexts.

My personal view is that extending the DMA API in this way feels quite
dirty - it's a violation of the DMA API design, which is to (a) demark
the buffer ownership between CPU and DMA agent, and (b) to translate
buffer locations into a cookie which device drivers can use to instruct
their device to access that memory.  To see why, consider... that you
map a buffer to a device in context A, and then you switch to context B,
which means the dma_addr_t given previously is no longer valid.  You
then try to unmap it... which is normally done using the (now no longer
valid) dma_addr_t.

It seems to me that to support this at DMA API level, we would need to
completely revamp the DMA API, which IMHO isn't going to be nice.  (It
would mean that we end up with three APIs - the original PCI DMA API,
the existing DMA API, and some new DMA API.)

Do we have any views on how common this feature is?

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[PATCH] drm: remove DRM_FORMAT_NV12MT

[Daniel Vetter]

So this has been merged originally in

commit 83052d4d5cd518332440bb4ee63d68bb5f744e0f
Author: Seung-Woo Kim 
Date:   Thu Dec 15 15:40:55 2011 +0900

drm: Add multi buffer plane pixel formats

which hasn't seen a lot of review really. The problem is that it's not
a real pixel format, but just a different way to lay out NV12 pixels
in macroblocks, i.e. a tiling format.

The new way of doing this is with the soon-to-be-merge fb modifiers.

This was brough up in some long irc discussion around the entire
topic, as an example of where things have gone wrong. Luckily we can
correct the mistake:
- The kms side support for NV12MT is all dead code because
  format_check in drm_crtc.c never accepted NV12MT.
- The gem side for the gsc support doesn't look better: The code
  forgets to set the pixel format and makes a big mess with the tiling
  mode bits, inadvertedly setting them all.

Conclusion: This never really worked (at least not in upstream) and
hence we can savely correct our mistake here.

Cc: Seung-Woo Kim 
Cc: Inki Dae 
Cc: Kyungmin Park 
Cc: Rob Clark 
Cc: Daniel Stone 
Cc: Damien Lespiau 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/exynos/exynos_drm_fimc.c  | 14 ++
 drivers/gpu/drm/exynos/exynos_drm_gsc.c   |  6 --
 drivers/gpu/drm/exynos/exynos_drm_plane.c |  1 -
 drivers/gpu/drm/exynos/exynos_mixer.c |  2 --
 include/uapi/drm/drm_fourcc.h |  3 ---
 5 files changed, 2 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c 
b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index 835b6af00970..842d6b8dc3c4 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
@@ -461,7 +461,6 @@ static int fimc_src_set_fmt_order(struct fimc_context *ctx, 
u32 fmt)
cfg |= EXYNOS_MSCTRL_C_INT_IN_3PLANE;
break;
case DRM_FORMAT_NV12:
-   case DRM_FORMAT_NV12MT:
case DRM_FORMAT_NV16:
cfg |= (EXYNOS_MSCTRL_ORDER2P_LSB_CBCR |
EXYNOS_MSCTRL_C_INT_IN_2PLANE);
@@ -511,7 +510,6 @@ static int fimc_src_set_fmt(struct device *dev, u32 fmt)
case DRM_FORMAT_YVU420:
case DRM_FORMAT_NV12:
case DRM_FORMAT_NV21:
-   case DRM_FORMAT_NV12MT:
cfg |= EXYNOS_MSCTRL_INFORMAT_YCBCR420;
break;
default:
@@ -524,10 +522,7 @@ static int fimc_src_set_fmt(struct device *dev, u32 fmt)
cfg = fimc_read(ctx, EXYNOS_CIDMAPARAM);
cfg &= ~EXYNOS_CIDMAPARAM_R_MODE_MASK;

-   if (fmt == DRM_FORMAT_NV12MT)
-   cfg |= EXYNOS_CIDMAPARAM_R_MODE_64X32;
-   else
-   cfg |= EXYNOS_CIDMAPARAM_R_MODE_LINEAR;
+   cfg |= EXYNOS_CIDMAPARAM_R_MODE_LINEAR;

fimc_write(ctx, cfg, EXYNOS_CIDMAPARAM);

@@ -812,7 +807,6 @@ static int fimc_dst_set_fmt_order(struct fimc_context *ctx, 
u32 fmt)
cfg |= EXYNOS_CIOCTRL_YCBCR_3PLANE;
break;
case DRM_FORMAT_NV12:
-   case DRM_FORMAT_NV12MT:
case DRM_FORMAT_NV16:
cfg |= EXYNOS_CIOCTRL_ORDER2P_LSB_CBCR;
cfg |= EXYNOS_CIOCTRL_YCBCR_2PLANE;
@@ -867,7 +861,6 @@ static int fimc_dst_set_fmt(struct device *dev, u32 fmt)
case DRM_FORMAT_YUV420:
case DRM_FORMAT_YVU420:
case DRM_FORMAT_NV12:
-   case DRM_FORMAT_NV12MT:
case DRM_FORMAT_NV21:
cfg |= EXYNOS_CITRGFMT_OUTFORMAT_YCBCR420;
break;
@@ -883,10 +876,7 @@ static int fimc_dst_set_fmt(struct device *dev, u32 fmt)
cfg = fimc_read(ctx, EXYNOS_CIDMAPARAM);
cfg &= ~EXYNOS_CIDMAPARAM_W_MODE_MASK;

-   if (fmt == DRM_FORMAT_NV12MT)
-   cfg |= EXYNOS_CIDMAPARAM_W_MODE_64X32;
-   else
-   cfg |= EXYNOS_CIDMAPARAM_W_MODE_LINEAR;
+   cfg |= EXYNOS_CIDMAPARAM_W_MODE_LINEAR;

fimc_write(ctx, cfg, EXYNOS_CIDMAPARAM);

diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c 
b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
index 0261468c8019..8040ed2a831f 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
@@ -542,9 +542,6 @@ static int gsc_src_set_fmt(struct device *dev, u32 fmt)
cfg |= (GSC_IN_CHROMA_ORDER_CBCR |
GSC_IN_YUV420_2P);
break;
-   case DRM_FORMAT_NV12MT:
-   cfg |= (GSC_IN_TILE_C_16x8 | GSC_IN_TILE_MODE);
-   break;
default:
dev_err(ippdrv->dev, "inavlid target yuv order 0x%x.\n", fmt);
return -EINVAL;
@@ -809,9 +806,6 @@ static int gsc_dst_set_fmt(struct device *dev, u32 fmt)
cfg |= (GSC_OUT_CHROMA_ORDER_CBCR |
GSC_OUT_YUV420_2P);
break;
-   case DRM_FORMAT_NV12MT:
-   cfg |= (GSC_OUT_TILE_C_16x8 | GSC_OUT_TILE_MODE);
-   break;
default:

Fwd: 3.18.3->4 : external monitor is black after wakeup of a docked system after s2ram

[Toralf Förster]

This commits breaks a working configuration here - reverting it let both 
version 3.18.4 and 3.18.5 working fine again at a hardened Gentoo Linux


commit ddf6f9a4c9fb1c72ea2e8d196c9a580a8e914dbd
Author: Dave Airlie 
Date:   Mon Dec 8 13:23:37 2014 +1000

drm/i915: resume MST after reading back hw state

commit e7d6f7d708290da1b7c92f533444b042c79412e0 upstream.

Otherwise the MST resume paths can hit DPMS paths
which hit state checker paths, which hit WARN_ON,
because the state checker is inconsistent with the
hw.

This fixes a bunch of WARN_ON's on resume after
undocking.

Signed-off-by: Dave Airlie 
Reviewed-by: Daniel Vetter 
Signed-off-by: Jani Nikula 
Signed-off-by: Greg Kroah-Hartman 


On 02/02/2015 07:24 PM, Toralf Förster wrote:
> In the mean while I have to correct it - it is reliable.
> So something between 3.18.3 and 3.18.4 break a docking system configuration 
> in that manenr, that after resume the external (iand internal X11 screen) is 
> black.
> 
> 
>  Forwarded Message 
> Subject: 3.18.3->4 : external monitor is black after wakeup of a docked 
> system after s2ram
> Date: Sat, 31 Jan 2015 16:47:45 +0100
> From: Toralf Förster 
> To: Linux Kernel 
> 
> Happened 2 times here (but not always) that a docked Lenovo T440s (intel 
> graphic) doesn't show anything at the external display (KDE 4.13.3). System 
> is a hardened stable 64 bit Gentoo Linux
> 
> Nothing suspicious in the log however.
> 
> Known issue ?
> 


-- 
Toralf
pgp key: 7B1A 07F4 EC82 0F90 D4C2  8936 872A E508 0076 E94E

[PATCH] RFC: drm: add support for tiled/compressed/etc modifier in addfb2

[Thierry Reding]

On Fri, Jan 30, 2015 at 05:08:23PM +0100, Daniel Vetter wrote:
> From: Rob Clark 
> 
> In DRM/KMS we are lacking a good way to deal with tiled/compressed
> formats.  Especially in the case of dmabuf/prime buffer sharing, where
> we cannot always rely on under-the-hood flags passed to driver specific
> gem-create ioctl to pass around these extra flags.
> 
> The proposal is to add a per-plane format modifier.  This allows to, if
> necessary, use different tiling patters for sub-sampled planes, etc.

"patterns". Alternatively perhaps "modes", which is how I've heard it
referred to most commonly.

> The format modifiers are added at the end of the ioctl struct, so for
> legacy userspace it will be zero padded.
> 
> v1: original
> v1.5: increase modifier to 64b
> 
> v2: Incorporate review comments from the big thread, plus a few more.
> 
> - Add a getcap so that userspace doesn't have to jump through hoops.
> - Allow modifiers only when a flag is set. That way drivers know when
>   they're dealing with old userspace and need to fish out e.g. tiling
>   from other information.
> - After rolling out checks for ->modifier to all drivers I've decided
>   that this is way too fragile and needs an explicit opt-in flag. So
>   do that instead.
> - Add a define (just for documentation really) for the "NONE"
>   modifier. Imo we don't need to add mask #defines since drivers
>   really should only do exact matches against values defined with
>   fourcc_mod_code.
> - Drop the Samsung tiling modifier on Rob's request since he's not yet
>   sure whether that one is accurate.
> 
> v3:
> - Also add a new ->modifier[] array to struct drm_framebuffer and fill
>   it in drm_helper_mode_fill_fb_struct. Requested by Tvrtko Uruslin.
> - Remove TODO in comment and add code comment that modifiers should be
>   properly documented, requested by Rob.
> 
> v4: Balance parens, spotted by Tvrtko.
> 
> Cc: Rob Clark 
> Cc: Tvrtko Ursulin 
> Cc: Laurent Pinchart 
> Cc: Daniel Stone 
> Cc: Ville Syrjälä 
> Cc: Michel Dänzer 
> Signed-off-by: Rob Clark  (v1.5)
> Reviewed-by: Rob Clark 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_crtc.c| 14 +-
>  drivers/gpu/drm/drm_crtc_helper.c |  1 +
>  drivers/gpu/drm/drm_ioctl.c   |  3 +++
>  include/drm/drm_crtc.h|  4 
>  include/uapi/drm/drm.h|  1 +
>  include/uapi/drm/drm_fourcc.h | 32 
>  include/uapi/drm/drm_mode.h   |  9 +
>  7 files changed, 63 insertions(+), 1 deletion(-)

Also as discussed on IRC, I think this would be better in a non-DRM
specific header so that we can have a central, cross-subsystem
authority.

> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 646ae5f39f42..622109677747 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -132,4 +132,36 @@
>  #define DRM_FORMAT_YUV444fourcc_code('Y', 'U', '2', '4') /* 
> non-subsampled Cb (1) and Cr (2) planes */
>  #define DRM_FORMAT_YVU444fourcc_code('Y', 'V', '2', '4') /* 
> non-subsampled Cr (1) and Cb (2) planes */
>  
> +

Possibly unintented extra blank line?

> +/*
> + * Format Modifiers:
> + *
> + * Format modifiers describe, typically, a re-ordering or modification
> + * of the data in a plane of an FB.  This can be used to express tiled/
> + * swizzled formats, or compression, or a combination of the two.
> + *
> + * The upper 8 bits of the format modifier are a vendor-id as assigned
> + * below.  The lower 56 bits are assigned as vendor sees fit.
> + */
> +
> +/* Vendor Ids: */
> +#define DRM_FORMAT_MOD_NONE   0
> +#define DRM_FORMAT_MOD_VENDOR_INTEL   0x01
> +#define DRM_FORMAT_MOD_VENDOR_AMD 0x02
> +#define DRM_FORMAT_MOD_VENDOR_NV  0x03


I think this should be NVIDIA for consistency with other naming in the
kernel, at least on Tegra.

Otherwise:

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

[Linaro-mm-sig] [RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms

[Arnd Bergmann]

On Tuesday 03 February 2015 15:22:05 Russell King - ARM Linux wrote:
> On Tue, Feb 03, 2015 at 03:52:48PM +0100, Arnd Bergmann wrote:
> > On Tuesday 03 February 2015 14:41:09 Russell King - ARM Linux wrote:
> > > I'd go as far as saying that the "DMA API on top of IOMMU" is more
> > > intended to be for a system IOMMU for the bus in question, rather
> > > than a device-level IOMMU.
> > > 
> > > If an IOMMU is part of a device, then the device should handle it
> > > (maybe via an abstraction) and not via the DMA API.  The DMA API should
> > > be handing the bus addresses to the device driver which the device's
> > > IOMMU would need to generate.  (In other words, in this circumstance,
> > > the DMA API shouldn't give you the device internal address.)
> > 
> > Exactly. And the abstraction that people choose at the moment is the
> > iommu API, for better or worse. It makes a lot of sense to use this
> > API if the same iommu is used for other devices as well (which is
> > the case on Tegra and probably a lot of others). Unfortunately the
> > iommu API lacks support for cache management, and probably other things
> > as well, because this was not an issue for the original use case
> > (device assignment on KVM/x86).
> > 
> > This could be done by adding explicit or implied cache management
> > to the IOMMU mapping interfaces, or by extending the dma-mapping
> > interfaces in a way that covers the use case of the device managing
> > its own address space, in addition to the existing coherent and
> > streaming interfaces.
> 
> Don't we already have those in the DMA API?  dma_sync_*() ?
>
> dma_map_sg() - sets up the system MMU and deals with initial cache
> coherency handling.  Device IOMMU being the responsibility of the
> GPU driver.

dma_sync_*() works with whatever comes out of dma_map_*(), true,
but this is not what they want to do here.

> The GPU can then do dma_sync_*() on the scatterlist as is necessary
> to synchronise the cache coherency (while respecting the ownership
> rules - which are very important on ARM to follow as some sync()s are
> destructive to any dirty data in the CPU cache.)
> 
> dma_unmap_sg() tears down the system MMU and deals with the final cache
> handling.
> 
> Why do we need more DMA API interfaces?

The dma_map_* interfaces assign the virtual addresses internally,
using typically either a global address space for all devices, or one
address space per device.

There are multiple things that this cannot do, and that is why the
drivers use the iommu API directly:

- use one address space per 'struct mm'
- map user memory with bus_address == user_address
- map memory into the GPU without having a permanent kernel mapping
- map memory first, and do the initial cache flushes later

Arnd

[Bug 77784] Certain mono based games hang the system with radeonsi (probably llvm backend)

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

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

Alex Deucher  changed:

   What|Removed |Added

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

--- Comment #8 from Alex Deucher  ---
You can mark it as resolved as well.

-- 
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/20150203/ae3a0df0/attachment.html>

[Bug 77784] Certain mono based games hang the system with radeonsi (probably llvm backend)

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

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

--- Comment #7 from José Suárez  ---
Judging from my last testing of both Foosball: street edition and LITTLE racers
street this problem does not show with mesa 10.5 git with llvm 3.6 (oibaf's
ppa), so I guess it could be marked as solved.

-- 
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/20150203/79effc58/attachment.html>

[Linaro-mm-sig] [RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms

[Russell King - ARM Linux]

On Tue, Feb 03, 2015 at 04:31:13PM +0100, Arnd Bergmann wrote:
> On Tuesday 03 February 2015 15:22:05 Russell King - ARM Linux wrote:
> > Don't we already have those in the DMA API?  dma_sync_*() ?
> >
> > dma_map_sg() - sets up the system MMU and deals with initial cache
> > coherency handling.  Device IOMMU being the responsibility of the
> > GPU driver.
> 
> dma_sync_*() works with whatever comes out of dma_map_*(), true,
> but this is not what they want to do here.
>  
> > The GPU can then do dma_sync_*() on the scatterlist as is necessary
> > to synchronise the cache coherency (while respecting the ownership
> > rules - which are very important on ARM to follow as some sync()s are
> > destructive to any dirty data in the CPU cache.)
> > 
> > dma_unmap_sg() tears down the system MMU and deals with the final cache
> > handling.
> > 
> > Why do we need more DMA API interfaces?
> 
> The dma_map_* interfaces assign the virtual addresses internally,
> using typically either a global address space for all devices, or one
> address space per device.

We shouldn't be doing one address space per device for precisely this
reason.  We should be doing one address space per *bus*.  I did have
a nice diagram to illustrate the point in my previous email, but I
deleted it, I wish I hadn't... briefly:

Fig. 1.
 +--+
 |+-+  device   |
CPU--L1cache--L2cache--Memory--SysMMU---IOMMU--> |
 |+-+   |
 +--+

Fig.1 represents what I'd call the "GPU" issue that we're talking about
in this thread.

Fig. 2.
CPU--L1cache--L2cache--Memory--SysMMU-IOMMU--device

The DMA API should be responsible (at the very least) for everything on
the left of "" in and should be providing a dma_addr_t which is
representative of what the device (in Fig.1) as a whole sees.  That's
the "system" part.  

I believe this is the approach which is taken by x86 and similar platforms,
simply because they tend not to have an IOMMU on individual devices (and
if they did, eg, on a PCI card, it's clearly the responsibility of the
device driver.)

Whether the DMA API also handles the IOMMU in Fig.1 or 2 is questionable.
For fig.2, it is entirely possible that the same device could appear
without an IOMMU, and in that scenario, you would want the IOMMU to be
handled transparently.

However, by doing so for everything, you run into exactly the problem
which is being discussed here - the need to separate out the cache
coherency from the IOMMU aspects.  You probably also have a setup very
similar to fig.1 (which is certainly true of Vivante GPUs.)

If you have the need to separately control both, then using the DMA API
to encapsulate both does not make sense - at which point, the DMA API
should be responsible for the minimum only - in other words, everything
to the left of  (so including the system MMU.)  The control of
the device IOMMU should be the responsibility of device driver in this
case.

So, dma_map_sg() would be responsible for dealing with the CPU cache
coherency issues, and setting up the system MMU.  dma_sync_*() would
be responsible for the CPU cache coherency issues, and dma_unmap_sg()
would (again) deal with the CPU cache and tear down the system MMU
mappings.

Meanwhile, the device driver has ultimate control over its IOMMU, the
creation and destruction of mappings and context switches at the
appropriate times.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[PATCH] vt_buffer: drop console buffer copying optimisations

[One Thousand Gnomes]

On Thu, 29 Jan 2015 15:40:33 -0800
Linus Torvalds  wrote:

> On Wed, Jan 28, 2015 at 8:11 PM, Dave Airlie  wrote:
> >
> > Linus, this came up a while back I finally got some confirmation
> > that it fixes those servers.
> 
> I'm certainly ok with this. which way should it go in? The users are:
> 
>  - drivers/tty/vt/vt.c (Greg KH, "tty layer")
> 
>  - drivers/video/console/* (fbcon people: Tomi Valkeinen and friends)
> 
> and it might make sense to have *some* indication of how much worse
> this makes fbcon performance in particular..

For devices that have no hardware scrolling it used to be double digit
percentages difference between 32 and 64bit when reading from the fb
because the reads are not posted and the latency killed you. Writes - not
so big a deal - but the bridge should combine them anyway. I imagine
16bit read would be unprintably bad.

Is it reads or writes that kill the card ?

Also note that switching to lots of small writes may break the 3Dfx
driver for the early 3Dfx PCI cards - they are really quite touchy about
how they are fed.

Unfortunately fbcon still matters for dumb EFI framebuffer fallbacks.

vgacon it doesn't matter (if it was too slow you could make vgacon as
fast as you want by only updating the off screen characters once per
vertical blank). fbcon that is a bit harder as you are allowed to
scribble on the display as well. You can't even check open/mmapped as you
can open, scribble and close.

Alan

--
Dive into the World of Parallel Programming. The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net/
--
___
Dri-devel mailing list
Dri-devel at lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

[RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms

[Arnd Bergmann]

On Tuesday 03 February 2015 14:41:09 Russell King - ARM Linux wrote:
> On Tue, Feb 03, 2015 at 03:17:27PM +0100, Arnd Bergmann wrote:
> > On Tuesday 03 February 2015 09:04:03 Rob Clark wrote:
> > > Since I'm stuck w/ an iommu, instead of built in mmu, my plan was to
> > > drop use of dma-mapping entirely (incl the current call to dma_map_sg,
> > > which I just need until we can use drm_cflush on arm), and
> > > attach/detach iommu domains directly to implement context switches.
> > > At that point, dma_addr_t really has no sensible meaning for me.
> > 
> > I think what you see here is a quite common hardware setup and we really
> > lack the right abstraction for it at the moment. Everybody seems to
> > work around it with a mix of the dma-mapping API and the iommu API.
> > These are doing different things, and even though the dma-mapping API
> > can be implemented on top of the iommu API, they are not really compatible.
> 
> I'd go as far as saying that the "DMA API on top of IOMMU" is more
> intended to be for a system IOMMU for the bus in question, rather
> than a device-level IOMMU.
> 
> If an IOMMU is part of a device, then the device should handle it
> (maybe via an abstraction) and not via the DMA API.  The DMA API should
> be handing the bus addresses to the device driver which the device's
> IOMMU would need to generate.  (In other words, in this circumstance,
> the DMA API shouldn't give you the device internal address.)

Exactly. And the abstraction that people choose at the moment is the
iommu API, for better or worse. It makes a lot of sense to use this
API if the same iommu is used for other devices as well (which is
the case on Tegra and probably a lot of others). Unfortunately the
iommu API lacks support for cache management, and probably other things
as well, because this was not an issue for the original use case
(device assignment on KVM/x86).

This could be done by adding explicit or implied cache management
to the IOMMU mapping interfaces, or by extending the dma-mapping
interfaces in a way that covers the use case of the device managing
its own address space, in addition to the existing coherent and
streaming interfaces.

Arnd

[Bug 87023] Dota 2 crashed and hang on ATi Mobility Radeon HD 5650 (Acer 4745G)

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

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

Romy  changed:

   What|Removed |Added

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

--- Comment #14 from Romy  ---
After I'm playing around with installing various distros until Windows, not
only Dota2 are crashed, but another games too, but not whole games. It's not
overheat issues, nor drivers nor OS, but I have adressed it that the memory
(RAM) causing this problem. I have 2 x 4GB 1600 Sodimm RAM, so that I'm
disasembling one of them, leaving 1 x 4GB 1600 Sodimm. That configuration
doesn't making my pc crash anymore so far. I mark this bug as RESOLVED.

-- 
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/20150203/cbbe19b7/attachment.html>

[RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms

[Russell King - ARM Linux]

On Tue, Feb 03, 2015 at 03:52:48PM +0100, Arnd Bergmann wrote:
> On Tuesday 03 February 2015 14:41:09 Russell King - ARM Linux wrote:
> > I'd go as far as saying that the "DMA API on top of IOMMU" is more
> > intended to be for a system IOMMU for the bus in question, rather
> > than a device-level IOMMU.
> > 
> > If an IOMMU is part of a device, then the device should handle it
> > (maybe via an abstraction) and not via the DMA API.  The DMA API should
> > be handing the bus addresses to the device driver which the device's
> > IOMMU would need to generate.  (In other words, in this circumstance,
> > the DMA API shouldn't give you the device internal address.)
> 
> Exactly. And the abstraction that people choose at the moment is the
> iommu API, for better or worse. It makes a lot of sense to use this
> API if the same iommu is used for other devices as well (which is
> the case on Tegra and probably a lot of others). Unfortunately the
> iommu API lacks support for cache management, and probably other things
> as well, because this was not an issue for the original use case
> (device assignment on KVM/x86).
> 
> This could be done by adding explicit or implied cache management
> to the IOMMU mapping interfaces, or by extending the dma-mapping
> interfaces in a way that covers the use case of the device managing
> its own address space, in addition to the existing coherent and
> streaming interfaces.

Don't we already have those in the DMA API?  dma_sync_*() ?

dma_map_sg() - sets up the system MMU and deals with initial cache
coherency handling.  Device IOMMU being the responsibility of the
GPU driver.

The GPU can then do dma_sync_*() on the scatterlist as is necessary
to synchronise the cache coherency (while respecting the ownership
rules - which are very important on ARM to follow as some sync()s are
destructive to any dirty data in the CPU cache.)

dma_unmap_sg() tears down the system MMU and deals with the final cache
handling.

Why do we need more DMA API interfaces?

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms

[Arnd Bergmann]

On Tuesday 03 February 2015 09:04:03 Rob Clark wrote:
> On Tue, Feb 3, 2015 at 2:48 AM, Daniel Vetter  wrote:
> > On Mon, Feb 02, 2015 at 03:30:21PM -0500, Rob Clark wrote:
> >> On Mon, Feb 2, 2015 at 11:54 AM, Daniel Vetter  wrote:
> >> >> My initial thought is for dma-buf to not try to prevent something than
> >> >> an exporter can actually do.. I think the scenario you describe could
> >> >> be handled by two sg-lists, if the exporter was clever enough.
> >> >
> >> > That's already needed, each attachment has it's own sg-list. After all
> >> > there's no array of dma_addr_t in the sg tables, so you can't use one sg
> >> > for more than one mapping. And due to different iommu different devices
> >> > can easily end up with different addresses.
> >>
> >>
> >> Well, to be fair it may not be explicitly stated, but currently one
> >> should assume the dma_addr_t's in the dmabuf sglist are bogus.  With
> >> gpu's that implement per-process/context page tables, I'm not really
> >> sure that there is a sane way to actually do anything else..
> >
> > Hm, what does per-process/context page tables have to do here? At least on
> > i915 we have a two levels of page tables:
> > - first level for vm/device isolation, used through dma api
> > - 2nd level for per-gpu-context isolation and context switching, handled
> >   internally.
> >
> > Since atm the dma api doesn't have any context of contexts or different
> > pagetables, I don't see who you could use that at all.
> 
> Since I'm stuck w/ an iommu, instead of built in mmu, my plan was to
> drop use of dma-mapping entirely (incl the current call to dma_map_sg,
> which I just need until we can use drm_cflush on arm), and
> attach/detach iommu domains directly to implement context switches.
> At that point, dma_addr_t really has no sensible meaning for me.

I think what you see here is a quite common hardware setup and we really
lack the right abstraction for it at the moment. Everybody seems to
work around it with a mix of the dma-mapping API and the iommu API.
These are doing different things, and even though the dma-mapping API
can be implemented on top of the iommu API, they are not really compatible.

The drm_clflush helpers don't seem like the right solution to me,
because all other devices outside of drm will face the same issue,
and I suspect we should fill the missing gaps in the API in a
more generic way.

Arnd

[PATCH v3 2/3] drm/panel: add s6e63j0x03 LCD panel driver

[Thierry Reding]

On Mon, Jan 19, 2015 at 04:52:32PM +0900, Hyungwon Hwang wrote:
> From: Inki Dae 
> 
> This patch adds MIPI-DSI based S6E63J0X03 AMOLED LCD panel driver
> which uses mipi_dsi bus to communicate with panel. The panel has
> 320×320 resolution in 1.63-inch physical panel. This panel is used in
> Samsung Galaxy Gear 2.
> 
> Signed-off-by: Inki Dae 
> Signed-off-by: Hyungwon Hwang 
> Acked-by: Kyungmin Park 
> ---
> Changes for v2:
> - Change the gamma table to 2-dimensional array
> - Change the way to make index for brightness
> - Make command functions to an array so that it can be called simply
> - Change command id for reading device ID
> - Change the way to handle the error condition
> - Remove power variable, and use the same name variable in bl_dev
> - Add the state FB_BLANK_NORMAL to represent the state which the panel
> is working but blanked
> - Miscellaneous changes to increase the readability and follow the
>   coding-style standard
> Changes for v3:
> - Add DT binding documentation
> - Add the code getting DT properties of 'panel-width-mm' and 'panel-height-mm'
> - Remove the code getting unnecessary DT properties flip-*
>  .../bindings/panel/samsung,s6e63j0x03.txt  |  55 +++
>  drivers/gpu/drm/panel/Kconfig  |   6 +
>  drivers/gpu/drm/panel/Makefile |   1 +
>  drivers/gpu/drm/panel/panel-s6e63j0x03.c   | 546 
> +
>  4 files changed, 608 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/panel/samsung,s6e63j0x03.txt
>  create mode 100644 drivers/gpu/drm/panel/panel-s6e63j0x03.c

I have pretty much the same comments as for the SA6E3HA2 panel driver
that I reviewed earlier.

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

[RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms

[Russell King - ARM Linux]

On Tue, Feb 03, 2015 at 09:44:57AM -0500, Rob Clark wrote:
> On Tue, Feb 3, 2015 at 9:37 AM, Russell King - ARM Linux
>  wrote:
> > On Tue, Feb 03, 2015 at 09:04:03AM -0500, Rob Clark wrote:
> >> Since I'm stuck w/ an iommu, instead of built in mmu, my plan was to
> >> drop use of dma-mapping entirely (incl the current call to dma_map_sg,
> >> which I just need until we can use drm_cflush on arm), and
> >> attach/detach iommu domains directly to implement context switches.
> >> At that point, dma_addr_t really has no sensible meaning for me.
> >
> > So how do you intend to import from a subsystem which only gives you
> > the dma_addr_t?
> >
> > If you aren't passing system memory, you have no struct page.  You can't
> > fake up a struct page.  What this means is that struct scatterlist can't
> > represent it any other way.
> 
> Tell the exporter to stop using carveouts, and give me proper memory
> instead.. ;-)
> 
> Well, at least on these SoC's, I think the only valid use for carveout
> memory is the bootloader splashscreen.  And I was planning on just
> hanging on to that for myself for fbdev scanout buffer or other
> internal (non shared) usage..

I wasn't thinking about carveouts - as I already mentioned earlier in this
thread, it may be memory which couldn't possibly ever be system memory -
for example, a separate chunk of memory which is tightly coupled to the
graphics system but not so to the CPU.

In such a case, we wouldn't want to use that as normal system memory, but
we would want to allocate framebuffers and the like from it, and maybe
pass them around.

While it may not be appropriate for MSM, it's still something that needs
to be considered, because there may be (and I know there are) dmabuf
users which do pass memory this way.

So, what I'm saying is that for the purposes of the dmabuf API, we can't
mandate that the scatterlists will contain a valid struct page pointer.
It'd probably be a good idea for the importer to validate the scatterlist
at import time if it has this requirement.

However, thinking about this more, I think that from a generic design
point of view, we really should limit the "struct page" usage to a
special MSM-ism - something which should definitely not be copied by
other drivers.  As has been mentioned previously, if there is a system
MMU which needs to be programmed to map system memory onto the bus, the
struct page becomes absolutely useless, and the only thing that gives
you the correct "handle" to that memory is the dma_addr_t.

Finally, note that n_mapped = dma_map_sg(dev, sg, n_ent, dir) - n_mapped
can be less than n_ent when there's the presence of an IOMMU, since an
IOMMU is permitted to coalesce individual scatterlist entries if it
so chooses, and when walking the scatterlist for DMA purposes, the
scatterlist sg_dma_*() accessors should be used, and it should be
iterated over from 0 to n_mapped, not 0 to n_ent.  It's important to
realise that in driver code, sg->length may not be the same as
sg_dma_len(sg) for exactly this reason:

#ifdef CONFIG_NEED_SG_DMA_LENGTH
#define sg_dma_len(sg)  ((sg)->dma_length)
#else
#define sg_dma_len(sg)  ((sg)->length)
#endif

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[PATCH 15/15] exynos: fimg2d: introduce G2D_OP_INTERPOLATE

[Tobias Jakobi]

This sets up the blending equation in the following way:
out = src * src_alpha + dst * (1 - src_alpha)

Signed-off-by: Tobias Jakobi 
---
 exynos/exynos_fimg2d.c | 4 
 exynos/exynos_fimg2d.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c
index 1b0bfdf..5829f14 100644
--- a/exynos/exynos_fimg2d.c
+++ b/exynos/exynos_fimg2d.c
@@ -84,6 +84,10 @@ static unsigned int g2d_get_blend_op(enum e_g2d_op op)
SET_BF(val, G2D_COEFF_MODE_ONE, 0, 0, 0,
G2D_COEFF_MODE_SRC_ALPHA, 1, 0, 0);
break;
+   case G2D_OP_INTERPOLATE:
+   SET_BF(val, G2D_COEFF_MODE_SRC_ALPHA, 0, 0, 0,
+   G2D_COEFF_MODE_SRC_ALPHA, 1, 0, 0);
+   break;
default:
fprintf(stderr, "Not support operation(%d).\n", op);
SET_BF(val, G2D_COEFF_MODE_ONE, 0, 0, 0, G2D_COEFF_MODE_ZERO,
diff --git a/exynos/exynos_fimg2d.h b/exynos/exynos_fimg2d.h
index 78cff47..79e6907 100644
--- a/exynos/exynos_fimg2d.h
+++ b/exynos/exynos_fimg2d.h
@@ -149,6 +149,7 @@ enum e_g2d_op {
G2D_OP_SRC  = 0x01,
G2D_OP_DST  = 0x02,
G2D_OP_OVER = 0x03,
+   G2D_OP_INTERPOLATE  = 0x04,
G2D_OP_DISJOINT_CLEAR   = 0x10,
G2D_OP_DISJOINT_SRC = 0x11,
G2D_OP_DISJOINT_DST = 0x12,
-- 
2.0.5

[PATCH 14/15] exynos: fimg2d: unify register style

[Tobias Jakobi]

Register defines all use uppercase hex codes.

Signed-off-by: Tobias Jakobi 
---
 exynos/fimg2d_reg.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/exynos/fimg2d_reg.h b/exynos/fimg2d_reg.h
index 5782488..07dd634 100644
--- a/exynos/fimg2d_reg.h
+++ b/exynos/fimg2d_reg.h
@@ -15,7 +15,7 @@

 #define SOFT_RESET_REG (0x)
 #define INTEN_REG  (0x0004)
-#define INTC_PEND_REG  (0x000c)
+#define INTC_PEND_REG  (0x000C)
 #define FIFO_STAT_REG  (0x0010)
 #define AXI_MODE_REG   (0x001C)
 #define DMA_SFR_BASE_ADDR_REG  (0x0080)
-- 
2.0.5

[PATCH 13/15] exynos: fimg2d: fix comment for G2D_COEFF_MODE_GB_COLOR

[Tobias Jakobi]

The coefficient mode enables use of global color, not alpha.

Signed-off-by: Tobias Jakobi 
---
 exynos/exynos_fimg2d.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/exynos/exynos_fimg2d.h b/exynos/exynos_fimg2d.h
index bd116cf..78cff47 100644
--- a/exynos/exynos_fimg2d.h
+++ b/exynos/exynos_fimg2d.h
@@ -166,7 +166,7 @@ enum e_g2d_coeff_mode {
G2D_COEFF_MODE_DST_COLOR,
/* Global Alpha : Set by ALPHA_REG(0x618) */
G2D_COEFF_MODE_GB_ALPHA,
-   /* Global Alpha : Set by ALPHA_REG(0x618) */
+   /* Global Color : Set by ALPHA_REG(0x618) */
G2D_COEFF_MODE_GB_COLOR,
/* (1-SRC alpha)/DST Alpha */
G2D_COEFF_MODE_DISJOINT_S,
-- 
2.0.5

[PATCH 12/15] exynos: add fimg2d header to common includes

[Tobias Jakobi]

The reason for this change is to let userspace use the header.
Currently 'make install' does not install it.

Signed-off-by: Tobias Jakobi 
---
 exynos/Makefile.am | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/exynos/Makefile.am b/exynos/Makefile.am
index 1715a85..35bc71f 100644
--- a/exynos/Makefile.am
+++ b/exynos/Makefile.am
@@ -14,11 +14,10 @@ libdrm_exynos_la_LIBADD = ../libdrm.la @PTHREADSTUBS_LIBS@
 libdrm_exynos_la_SOURCES = \
exynos_drm.c \
exynos_fimg2d.c \
-   exynos_fimg2d.h \
fimg2d_reg.h

 libdrm_exynoscommonincludedir = ${includedir}/exynos
-libdrm_exynoscommoninclude_HEADERS = exynos_drm.h
+libdrm_exynoscommoninclude_HEADERS = exynos_drm.h exynos_fimg2d.h

 libdrm_exynosincludedir = ${includedir}/libdrm
 libdrm_exynosinclude_HEADERS = exynos_drmif.h
-- 
2.0.5

[PATCH 11/15] exynos: add exynos prefix to fimg2d header

[Tobias Jakobi]

Signed-off-by: Tobias Jakobi 
---
 exynos/Makefile.am|   2 +-
 exynos/exynos_fimg2d.c|   2 +-
 exynos/exynos_fimg2d.h| 328 ++
 exynos/fimg2d.h   | 328 --
 tests/exynos/exynos_fimg2d_test.c |   2 +-
 5 files changed, 331 insertions(+), 331 deletions(-)
 create mode 100644 exynos/exynos_fimg2d.h
 delete mode 100644 exynos/fimg2d.h

diff --git a/exynos/Makefile.am b/exynos/Makefile.am
index 06bee00..1715a85 100644
--- a/exynos/Makefile.am
+++ b/exynos/Makefile.am
@@ -14,7 +14,7 @@ libdrm_exynos_la_LIBADD = ../libdrm.la @PTHREADSTUBS_LIBS@
 libdrm_exynos_la_SOURCES = \
exynos_drm.c \
exynos_fimg2d.c \
-   fimg2d.h \
+   exynos_fimg2d.h \
fimg2d_reg.h

 libdrm_exynoscommonincludedir = ${includedir}/exynos
diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c
index c609053..1b0bfdf 100644
--- a/exynos/exynos_fimg2d.c
+++ b/exynos/exynos_fimg2d.c
@@ -27,7 +27,7 @@
 #include "libdrm.h"
 #include "exynos_drm.h"
 #include "fimg2d_reg.h"
-#include "fimg2d.h"
+#include "exynos_fimg2d.h"

 #defineSET_BF(val, sc, si, scsa, scda, dc, di, dcsa, dcda) \
val.data.src_coeff = sc;\
diff --git a/exynos/exynos_fimg2d.h b/exynos/exynos_fimg2d.h
new file mode 100644
index 000..bd116cf
--- /dev/null
+++ b/exynos/exynos_fimg2d.h
@@ -0,0 +1,328 @@
+/*
+ * Copyright (C) 2013 Samsung Electronics Co.Ltd
+ * Authors:
+ * Inki Dae 
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ *
+ */
+
+#ifndef _FIMG2D_H_
+#define _FIMG2D_H_
+
+#ifndef TRUE
+#define TRUE 0
+#endif
+#ifndef FALSE
+#define FALSE -1
+#endif
+
+#define G2D_MAX_CMD_NR 64
+#define G2D_MAX_GEM_CMD_NR 64
+#define G2D_MAX_CMD_LIST_NR64
+#define G2D_PLANE_MAX_NR   2
+
+enum e_g2d_color_mode {
+   /* COLOR FORMAT */
+   G2D_COLOR_FMT_XRGB,
+   G2D_COLOR_FMT_ARGB,
+   G2D_COLOR_FMT_RGB565,
+   G2D_COLOR_FMT_XRGB1555,
+   G2D_COLOR_FMT_ARGB1555,
+   G2D_COLOR_FMT_XRGB,
+   G2D_COLOR_FMT_ARGB,
+   G2D_COLOR_FMT_PRGB888,
+   G2D_COLOR_FMT_YCbCr444,
+   G2D_COLOR_FMT_YCbCr422,
+   G2D_COLOR_FMT_YCbCr420,
+   /* alpha 8bit */
+   G2D_COLOR_FMT_A8,
+   /* Luminance 8bit: gray color */
+   G2D_COLOR_FMT_L8,
+   /* alpha 1bit */
+   G2D_COLOR_FMT_A1,
+   /* alpha 4bit */
+   G2D_COLOR_FMT_A4,
+   G2D_COLOR_FMT_MASK, /* VER4.1 */
+
+   /* COLOR ORDER */
+   G2D_ORDER_AXRGB = (0 << 4), /* VER4.1 */
+   G2D_ORDER_RGBAX = (1 << 4), /* VER4.1 */
+   G2D_ORDER_AXBGR = (2 << 4), /* VER4.1 */
+   G2D_ORDER_BGRAX = (3 << 4), /* VER4.1 */
+   G2D_ORDER_MASK  = (3 << 4), /* VER4.1 */
+
+   /* Number of YCbCr plane */
+   G2D_YCbCr_1PLANE= (0 << 8), /* VER4.1 */
+   G2D_YCbCr_2PLANE= (1 << 8), /* VER4.1 */
+   G2D_YCbCr_PLANE_MASK= (3 << 8), /* VER4.1 */
+
+   /* Order in YCbCr */
+   G2D_YCbCr_ORDER_CrY1CbY0 = (0 << 12),   /* VER4.1 */
+   G2D_YCbCr_ORDER_CbY1CrY0 = (1 << 12),   /* VER4.1 */
+   G2D_YCbCr_ORDER_Y1CrY0Cb = (2 << 12),   /* VER4.1 */
+   G2D_YCbCr_ORDER_Y1CbY0Cr = (3 << 12),   /* VER4.1 */
+   G2D_YCbCr_ORDER_CrCb = G2D_YCbCr_ORDER_CrY1CbY0,/* VER4.1 */
+   G2D_YCbCr_ORDER_CbCr = G2D_YCbCr_ORDER_CbY1CrY0,/* VER4.1 */
+   G2D_YCbCr_ORDER_MASK = (3 < 12),/* VER4.1 */
+
+   /* CSC */
+   G2D_CSC_601 = (0 << 16),/* VER4.1 */
+   G2D_CSC_709 = (1 << 16),/* VER4.1 */
+   G2D_CSC_MASK = (1 << 16),   /* VER4.1 */
+
+   /* Valid value range of YCbCr */
+   G2D_YCbCr_RANGE_NARROW = (0 << 17), /* VER4.1 */
+   G2D_YCbCr_RANGE_WIDE = (1 << 17),   /* VER4.1 */
+   G2D_YCbCr_RANGE_MASK = (1 << 17),   /* VER4.1 */
+
+   G2D_COLOR_MODE_MASK = 0x,
+};
+
+enum e_g2d_select_mode {
+   G2D_SELECT_MODE_NORMAL  = (0 << 0),
+   G2D_SELECT_MODE_FGCOLOR = (1 << 0),
+   G2D_SELECT_MODE_BGCOLOR = (2 << 0),
+};
+
+enum e_g2d_repeat_mode {
+   G2D_REPEAT_MODE_REPEAT,
+   G2D_REPEAT_MODE_PAD,
+   G2D_REPEAT_MODE_REFLECT,
+   G2D_REPEAT_MODE_CLAMP,
+   G2D_REPEAT_MODE_NONE,
+};
+
+enum e_g2d_scale_mode {
+   G2D_SCALE_MODE_NONE = 0,
+   G2D_SCALE_MODE_NEAREST,
+   G2D_SCALE_MODE_BILINEAR,
+   G2D_SCALE_MODE_MAX,
+};
+
+enum e_g2d_buf_type {
+   

[PATCH 10/15] tests/exynos: improve error handling

[Tobias Jakobi]

Check for a useable connector and also if the resolution is sane
(width and height are both non-zero).

Signed-off-by: Tobias Jakobi 
---
 tests/exynos/exynos_fimg2d_test.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/tests/exynos/exynos_fimg2d_test.c 
b/tests/exynos/exynos_fimg2d_test.c
index cb021ac..d68f3d1 100644
--- a/tests/exynos/exynos_fimg2d_test.c
+++ b/tests/exynos/exynos_fimg2d_test.c
@@ -725,10 +725,22 @@ int main(int argc, char **argv)
connector_find_mode(dev->fd, , resources);
drmModeFreeResources(resources);

+   if (!con.mode) {
+   fprintf(stderr, "failed to find usable connector\n");
+   ret = -EFAULT;
+   goto err_drm_close;
+   }
+
screen_width = con.mode->hdisplay;
screen_height = con.mode->vdisplay;

-   printf("screen width  = %d, screen height = %d\n", screen_width,
+   if (screen_width == 0 || screen_height == 0) {
+   fprintf(stderr, "failed to find sane resolution on 
connector\n");
+   ret = -EFAULT;
+   goto err_drm_close;
+   }
+
+   printf("screen width = %d, screen height = %d\n", screen_width,
screen_height);

bo = exynos_create_buffer(dev, screen_width * screen_height * 4, 0);
-- 
2.0.5

[PATCH 09/15] exynos: use structure initialization instead of memset

[Tobias Jakobi]

Keeps the code cleaner, since the structs have to be initialized
once anyway.

Signed-off-by: Tobias Jakobi 
---
 exynos/exynos_fimg2d.c|  4 +---
 tests/exynos/exynos_fimg2d_test.c | 15 ---
 2 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c
index 7b76eb6..c609053 100644
--- a/exynos/exynos_fimg2d.c
+++ b/exynos/exynos_fimg2d.c
@@ -180,7 +180,7 @@ static void g2d_reset(struct g2d_context *ctx)
 static int g2d_flush(struct g2d_context *ctx)
 {
int ret;
-   struct drm_exynos_g2d_set_cmdlist cmdlist;
+   struct drm_exynos_g2d_set_cmdlist cmdlist = {0};

if (ctx->cmd_nr  == 0 && ctx->cmd_buf_nr == 0)
return FALSE;
@@ -190,8 +190,6 @@ static int g2d_flush(struct g2d_context *ctx)
return -EINVAL;
}

-   memset(, 0, sizeof(struct drm_exynos_g2d_set_cmdlist));
-
cmdlist.cmd = (uint64_t)(uintptr_t)>cmd[0];
cmdlist.cmd_buf = (uint64_t)(uintptr_t)>cmd_buf[0];
cmdlist.cmd_nr = ctx->cmd_nr;
diff --git a/tests/exynos/exynos_fimg2d_test.c 
b/tests/exynos/exynos_fimg2d_test.c
index 20b8e9d..cb021ac 100644
--- a/tests/exynos/exynos_fimg2d_test.c
+++ b/tests/exynos/exynos_fimg2d_test.c
@@ -257,7 +257,7 @@ static void exynos_destroy_buffer(struct exynos_bo *bo)
 static int g2d_solid_fill_test(struct exynos_device *dev, struct exynos_bo 
*dst)
 {
struct g2d_context *ctx;
-   struct g2d_image img;
+   struct g2d_image img = {0};
unsigned int count, img_w, img_h;
int ret = 0;

@@ -265,7 +265,6 @@ static int g2d_solid_fill_test(struct exynos_device *dev, 
struct exynos_bo *dst)
if (!ctx)
return -EFAULT;

-   memset(, 0, sizeof(struct g2d_image));
img.bo[0] = dst->handle;

printf("solid fill test.\n");
@@ -308,7 +307,7 @@ static int g2d_copy_test(struct exynos_device *dev, struct 
exynos_bo *src,
enum e_g2d_buf_type type)
 {
struct g2d_context *ctx;
-   struct g2d_image src_img, dst_img;
+   struct g2d_image src_img = {0}, dst_img = {0};
unsigned int count;
unsigned int src_x, src_y, dst_x, dst_y, img_w, img_h;
unsigned long userptr, size;
@@ -318,8 +317,6 @@ static int g2d_copy_test(struct exynos_device *dev, struct 
exynos_bo *src,
if (!ctx)
return -EFAULT;

-   memset(_img, 0, sizeof(struct g2d_image));
-   memset(_img, 0, sizeof(struct g2d_image));
dst_img.bo[0] = dst->handle;

src_x = 0;
@@ -392,7 +389,7 @@ static int g2d_copy_with_scale_test(struct exynos_device 
*dev,
enum e_g2d_buf_type type)
 {
struct g2d_context *ctx;
-   struct g2d_image src_img, dst_img;
+   struct g2d_image src_img = {0}, dst_img = {0};
unsigned int count;
unsigned int src_x, src_y, dst_x, dst_y, img_w, img_h;
unsigned long userptr, size;
@@ -402,8 +399,6 @@ static int g2d_copy_with_scale_test(struct exynos_device 
*dev,
if (!ctx)
return -EFAULT;

-   memset(_img, 0, sizeof(struct g2d_image));
-   memset(_img, 0, sizeof(struct g2d_image));
dst_img.bo[0] = dst->handle;

src_x = 0;
@@ -481,7 +476,7 @@ static int g2d_blend_test(struct exynos_device *dev,
enum e_g2d_buf_type type)
 {
struct g2d_context *ctx;
-   struct g2d_image src_img, dst_img;
+   struct g2d_image src_img = {0}, dst_img = {0};
unsigned int count;
unsigned int src_x, src_y, dst_x, dst_y, img_w, img_h;
unsigned long userptr, size;
@@ -491,8 +486,6 @@ static int g2d_blend_test(struct exynos_device *dev,
if (!ctx)
return -EFAULT;

-   memset(_img, 0, sizeof(struct g2d_image));
-   memset(_img, 0, sizeof(struct g2d_image));
dst_img.bo[0] = dst->handle;

src_x = 0;
-- 
2.0.5

[PATCH 08/15] exynos: introduce g2d_add_base_addr helper function

[Tobias Jakobi]

In almost all functions the base address register is written, so it
makes sense to have a helper function for this.

Signed-off-by: Tobias Jakobi 
---
 exynos/exynos_fimg2d.c | 87 +++---
 1 file changed, 33 insertions(+), 54 deletions(-)

diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c
index eae6223..7b76eb6 100644
--- a/exynos/exynos_fimg2d.c
+++ b/exynos/exynos_fimg2d.c
@@ -41,6 +41,11 @@

 #define MIN(a, b)  ((a) < (b) ? (a) : (b))

+enum g2d_base_addr_reg {
+   g2d_dst = 0,
+   g2d_src
+};
+
 static unsigned int g2d_get_scaling(unsigned int src, unsigned int dst)
 {
/* The G2D hw scaling factor is a normalized inverse of the scaling 
factor. *
@@ -131,6 +136,25 @@ static int g2d_add_cmd(struct g2d_context *ctx, unsigned 
long cmd,
 }

 /*
+ * g2d_add_base_addr - helper function to set dst/src base address register.
+ *
+ * @ctx: a pointer to g2d_context structure.
+ * @img: a pointer to the dst/src g2d_image structure.
+ * @reg: the register that should be set.
+ */
+static void g2d_add_base_addr(struct g2d_context *ctx, struct g2d_image *img,
+   enum g2d_base_addr_reg reg)
+{
+   const unsigned long cmd = (reg == g2d_dst) ? DST_BASE_ADDR_REG : 
SRC_BASE_ADDR_REG;
+
+   if (img->buf_type == G2D_IMGBUF_USERPTR)
+   g2d_add_cmd(ctx, cmd | G2D_BUF_USERPTR,
+   (unsigned long)>user_ptr[0]);
+   else
+   g2d_add_cmd(ctx, cmd, img->bo[0]);
+}
+
+/*
  * g2d_reset - reset fimg2d hardware.
  *
  * @ctx: a pointer to g2d_context structure.
@@ -275,13 +299,7 @@ g2d_solid_fill(struct g2d_context *ctx, struct g2d_image 
*img,

g2d_add_cmd(ctx, DST_SELECT_REG, G2D_SELECT_MODE_NORMAL);
g2d_add_cmd(ctx, DST_COLOR_MODE_REG, img->color_mode);
-
-   if (img->buf_type == G2D_IMGBUF_USERPTR)
-   g2d_add_cmd(ctx, DST_BASE_ADDR_REG | G2D_BUF_USERPTR,
-   (unsigned long)>user_ptr[0]);
-   else
-   g2d_add_cmd(ctx, DST_BASE_ADDR_REG, img->bo[0]);
-
+   g2d_add_base_addr(ctx, img, g2d_dst);
g2d_add_cmd(ctx, DST_STRIDE_REG, img->stride);

if (x + w > img->width)
@@ -338,22 +356,12 @@ g2d_copy(struct g2d_context *ctx, struct g2d_image *src,

g2d_add_cmd(ctx, DST_SELECT_REG, G2D_SELECT_MODE_BGCOLOR);
g2d_add_cmd(ctx, DST_COLOR_MODE_REG, dst->color_mode);
-   if (dst->buf_type == G2D_IMGBUF_USERPTR)
-   g2d_add_cmd(ctx, DST_BASE_ADDR_REG | G2D_BUF_USERPTR,
-   (unsigned long)>user_ptr[0]);
-   else
-   g2d_add_cmd(ctx, DST_BASE_ADDR_REG, dst->bo[0]);
-
+   g2d_add_base_addr(ctx, dst, g2d_dst);
g2d_add_cmd(ctx, DST_STRIDE_REG, dst->stride);

g2d_add_cmd(ctx, SRC_SELECT_REG, G2D_SELECT_MODE_NORMAL);
g2d_add_cmd(ctx, SRC_COLOR_MODE_REG, src->color_mode);
-   if (src->buf_type == G2D_IMGBUF_USERPTR)
-   g2d_add_cmd(ctx, SRC_BASE_ADDR_REG | G2D_BUF_USERPTR,
-   (unsigned long)>user_ptr[0]);
-   else
-   g2d_add_cmd(ctx, SRC_BASE_ADDR_REG, src->bo[0]);
-
+   g2d_add_base_addr(ctx, src, g2d_src);
g2d_add_cmd(ctx, SRC_STRIDE_REG, src->stride);

src_w = w;
@@ -441,12 +449,7 @@ g2d_copy_with_scale(struct g2d_context *ctx, struct 
g2d_image *src,

g2d_add_cmd(ctx, DST_SELECT_REG, G2D_SELECT_MODE_BGCOLOR);
g2d_add_cmd(ctx, DST_COLOR_MODE_REG, dst->color_mode);
-   if (dst->buf_type == G2D_IMGBUF_USERPTR)
-   g2d_add_cmd(ctx, DST_BASE_ADDR_REG | G2D_BUF_USERPTR,
-   (unsigned long)>user_ptr[0]);
-   else
-   g2d_add_cmd(ctx, DST_BASE_ADDR_REG, dst->bo[0]);
-
+   g2d_add_base_addr(ctx, dst, g2d_dst);
g2d_add_cmd(ctx, DST_STRIDE_REG, dst->stride);

g2d_add_cmd(ctx, SRC_SELECT_REG, G2D_SELECT_MODE_NORMAL);
@@ -456,11 +459,7 @@ g2d_copy_with_scale(struct g2d_context *ctx, struct 
g2d_image *src,
if (src->repeat_mode == G2D_REPEAT_MODE_PAD)
g2d_add_cmd(ctx, SRC_PAD_VALUE_REG, dst->color);

-   if (src->buf_type == G2D_IMGBUF_USERPTR)
-   g2d_add_cmd(ctx, SRC_BASE_ADDR_REG | G2D_BUF_USERPTR,
-   (unsigned long)>user_ptr[0]);
-   else
-   g2d_add_cmd(ctx, SRC_BASE_ADDR_REG, src->bo[0]);
+   g2d_add_base_addr(ctx, src, g2d_src);

g2d_add_cmd(ctx, SRC_STRIDE_REG, src->stride);

@@ -564,12 +563,7 @@ g2d_blend(struct g2d_context *ctx, struct g2d_image *src,
g2d_add_cmd(ctx, DST_SELECT_REG, G2D_SELECT_MODE_NORMAL);

g2d_add_cmd(ctx, DST_COLOR_MODE_REG, dst->color_mode);
-   if (dst->buf_type == G2D_IMGBUF_USERPTR)
-   g2d_add_cmd(ctx, DST_BASE_ADDR_REG | G2D_BUF_USERPTR,
-   (unsigned long)>user_ptr[0]);
-   else
-   g2d_add_cmd(ctx, 

[PATCH 07/15] exynos: honor the repeat mode in g2d_copy_with_scale

[Tobias Jakobi]

This is useful when the default repeat mode, which is 'repeat'
produces artifacts at the borders of the copied image.
Choose the 'pad' mode to make use of the color of the destination
image.

In my usage case the destination is the framebuffer, which is
solid filled with a background color. Scaling with 'pad' mode
would then just do the right thing and also produces nice
borders on the output.

Signed-off-by: Tobias Jakobi 
---
 exynos/exynos_fimg2d.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c
index d6ad9a7..eae6223 100644
--- a/exynos/exynos_fimg2d.c
+++ b/exynos/exynos_fimg2d.c
@@ -451,6 +451,11 @@ g2d_copy_with_scale(struct g2d_context *ctx, struct 
g2d_image *src,

g2d_add_cmd(ctx, SRC_SELECT_REG, G2D_SELECT_MODE_NORMAL);
g2d_add_cmd(ctx, SRC_COLOR_MODE_REG, src->color_mode);
+
+   g2d_add_cmd(ctx, SRC_REPEAT_MODE_REG, src->repeat_mode);
+   if (src->repeat_mode == G2D_REPEAT_MODE_PAD)
+   g2d_add_cmd(ctx, SRC_PAD_VALUE_REG, dst->color);
+
if (src->buf_type == G2D_IMGBUF_USERPTR)
g2d_add_cmd(ctx, SRC_BASE_ADDR_REG | G2D_BUF_USERPTR,
(unsigned long)>user_ptr[0]);
-- 
2.0.5

[PATCH 06/15] tests/exynos: introduce wait_for_user_input

[Tobias Jakobi]

Currently getchar() is used to pause execution after each test.
The user isn't informed if one is supposed to do anything for
the tests to continue, so print a simple message to make this
more clear.

Signed-off-by: Tobias Jakobi 
---
 tests/exynos/exynos_fimg2d_test.c | 20 
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/tests/exynos/exynos_fimg2d_test.c 
b/tests/exynos/exynos_fimg2d_test.c
index 48dfe97..20b8e9d 100644
--- a/tests/exynos/exynos_fimg2d_test.c
+++ b/tests/exynos/exynos_fimg2d_test.c
@@ -237,6 +237,18 @@ void *create_checkerboard_pattern(unsigned int num_tiles_x,
return buf;
 }

+static void wait_for_user_input(int last)
+{
+   printf("press  to ");
+
+   if (last)
+   printf("exit test application\n");
+   else
+   printf("skip to next test\n");
+
+   getchar();
+}
+
 static void exynos_destroy_buffer(struct exynos_bo *bo)
 {
exynos_bo_destroy(bo);
@@ -756,7 +768,7 @@ int main(int argc, char **argv)
goto err_rm_fb;
}

-   getchar();
+   wait_for_user_input(0);

src = exynos_create_buffer(dev, screen_width * screen_height * 4, 0);
if (!src) {
@@ -770,7 +782,7 @@ int main(int argc, char **argv)
goto err_free_src;
}

-   getchar();
+   wait_for_user_input(0);

ret = test_case.copy_with_scale(dev, src, bo, G2D_IMGBUF_GEM);
if (ret < 0) {
@@ -778,7 +790,7 @@ int main(int argc, char **argv)
goto err_free_src;
}

-   getchar();
+   wait_for_user_input(0);

ret = test_case.checkerboard(dev, src, bo, G2D_IMGBUF_GEM);
if (ret < 0) {
@@ -786,7 +798,7 @@ int main(int argc, char **argv)
goto err_free_src;
}

-   getchar();
+   wait_for_user_input(1);

 #if 0
ret  = test_case.blend(dev, src, bo, G2D_IMGBUF_USERPTR);
-- 
2.0.5

[PATCH 05/15] exynos: add g2d_scale_and_blend

[Tobias Jakobi]

This is a combination of g2d_copy_with_scale and g2d_scale.
It is a pretty common operation to scale one buffer and then
blend it on top of another, so provide a direct way to that
operation.

Signed-off-by: Tobias Jakobi 
---
 exynos/exynos_fimg2d.c | 129 +
 exynos/fimg2d.h|   5 ++
 2 files changed, 134 insertions(+)

diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c
index e7b00b1..d6ad9a7 100644
--- a/exynos/exynos_fimg2d.c
+++ b/exynos/exynos_fimg2d.c
@@ -642,3 +642,132 @@ g2d_blend(struct g2d_context *ctx, struct g2d_image *src,
return 0;
 }

+/**
+ * g2d_scale_and_blend - apply scaling to source buffer and then blend to 
destination buffer
+ *
+ * @ctx: a pointer to g2d_context structure.
+ * @src: a pointer to g2d_image structure including image and buffer
+ * information to source.
+ * @dst: a pointer to g2d_image structure including image and buffer
+ * information to destination.
+ * @src_x: x start position to source buffer.
+ * @src_y: y start position to source buffer.
+ * @src_w: width value to source buffer.
+ * @src_h: height value to source buffer.
+ * @dst_x: x start position to destination buffer.
+ * @dst_y: y start position to destination buffer.
+ * @dst_w: width value to destination buffer.
+ * @dst_h: height value to destination buffer.
+ * @op: blend operation type.
+ */
+drm_public int
+g2d_scale_and_blend(struct g2d_context *ctx, struct g2d_image *src,
+   struct g2d_image *dst, unsigned int src_x, unsigned int src_y,
+   unsigned int src_w, unsigned int src_h, unsigned int dst_x,
+   unsigned int dst_y, unsigned int dst_w, unsigned int dst_h,
+   enum e_g2d_op op)
+{
+   union g2d_point_val pt;
+   union g2d_bitblt_cmd_val bitblt;
+   union g2d_blend_func_val blend;
+   unsigned int scale;
+   unsigned int scale_x, scale_y;
+
+   bitblt.val = 0;
+   blend.val = 0;
+
+   if (op == G2D_OP_SRC || op == G2D_OP_CLEAR)
+   g2d_add_cmd(ctx, DST_SELECT_REG, G2D_SELECT_MODE_BGCOLOR);
+   else
+   g2d_add_cmd(ctx, DST_SELECT_REG, G2D_SELECT_MODE_NORMAL);
+
+   g2d_add_cmd(ctx, DST_COLOR_MODE_REG, dst->color_mode);
+   if (dst->buf_type == G2D_IMGBUF_USERPTR)
+   g2d_add_cmd(ctx, DST_BASE_ADDR_REG | G2D_BUF_USERPTR,
+   (unsigned long)>user_ptr[0]);
+   else
+   g2d_add_cmd(ctx, DST_BASE_ADDR_REG, dst->bo[0]);
+
+   g2d_add_cmd(ctx, DST_STRIDE_REG, dst->stride);
+
+   g2d_add_cmd(ctx, SRC_SELECT_REG, src->select_mode);
+   g2d_add_cmd(ctx, SRC_COLOR_MODE_REG, src->color_mode);
+
+   switch (src->select_mode) {
+   case G2D_SELECT_MODE_NORMAL:
+   if (src->buf_type == G2D_IMGBUF_USERPTR)
+   g2d_add_cmd(ctx, SRC_BASE_ADDR_REG | G2D_BUF_USERPTR,
+   (unsigned long)>user_ptr[0]);
+   else
+   g2d_add_cmd(ctx, SRC_BASE_ADDR_REG, src->bo[0]);
+
+   g2d_add_cmd(ctx, SRC_STRIDE_REG, src->stride);
+   break;
+   case G2D_SELECT_MODE_FGCOLOR:
+   g2d_add_cmd(ctx, FG_COLOR_REG, src->color);
+   break;
+   case G2D_SELECT_MODE_BGCOLOR:
+   g2d_add_cmd(ctx, BG_COLOR_REG, src->color);
+   break;
+   default:
+   fprintf(stderr , "failed to set src.\n");
+   return -EINVAL;
+   }
+
+   if (src_w == dst_w && src_h == dst_h)
+   scale = 0;
+   else {
+   scale = 1;
+   scale_x = g2d_get_scaling(src_w, dst_w);
+   scale_y = g2d_get_scaling(src_h, dst_h);
+   }
+
+   if (src_x + src_w > src->width)
+   src_w = src->width - src_x;
+   if (src_y + src_h > src->height)
+   src_h = src->height - src_y;
+
+   if (dst_x + dst_w > dst->width)
+   dst_w = dst->width - dst_x;
+   if (dst_y + dst_h > dst->height)
+   dst_h = dst->height - dst_y;
+
+   if (src_w <= 0 || src_h <= 0 || dst_w <= 0 || dst_h <= 0) {
+   fprintf(stderr, "invalid width or height.\n");
+   g2d_reset(ctx);
+   return -EINVAL;
+   }
+
+   if (scale) {
+   g2d_add_cmd(ctx, SRC_SCALE_CTRL_REG, G2D_SCALE_MODE_BILINEAR);
+   g2d_add_cmd(ctx, SRC_XSCALE_REG, scale_x);
+   g2d_add_cmd(ctx, SRC_YSCALE_REG, scale_y);
+   }
+
+   bitblt.data.alpha_blend_mode = G2D_ALPHA_BLEND_MODE_ENABLE;
+   blend.val = g2d_get_blend_op(op);
+   g2d_add_cmd(ctx, BITBLT_COMMAND_REG, bitblt.val);
+   g2d_add_cmd(ctx, BLEND_FUNCTION_REG, blend.val);
+
+   pt.val = 0;
+   pt.data.x = src_x;
+   pt.data.y = src_y;
+   g2d_add_cmd(ctx, SRC_LEFT_TOP_REG, pt.val);
+   pt.val = 0;
+   pt.data.x = src_x + src_w;
+   pt.data.y = src_y + src_h;
+   

[PATCH 04/15] tests/exynos: disable the G2D userptr/blend test

[Tobias Jakobi]

The blend test uses the userptr functionality of exynos-drm, which
is currently not safe to use. If the kernel hasn't been build with
exynos-iommu support, then the blend test is going to produce (kernel)
memory corruption, eventually leading to a system crash.

Disable the test for now, until the kernel code has been sanitized.

Signed-off-by: Tobias Jakobi 
---
 tests/exynos/exynos_fimg2d_test.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/exynos/exynos_fimg2d_test.c 
b/tests/exynos/exynos_fimg2d_test.c
index aa140e5..48dfe97 100644
--- a/tests/exynos/exynos_fimg2d_test.c
+++ b/tests/exynos/exynos_fimg2d_test.c
@@ -788,11 +788,13 @@ int main(int argc, char **argv)

getchar();

+#if 0
ret  = test_case.blend(dev, src, bo, G2D_IMGBUF_USERPTR);
if (ret < 0)
fprintf(stderr, "failed to test blend operation.\n");

getchar();
+#endif

 err_free_src:
if (src)
-- 
2.0.5

[PATCH 03/15] tests/exynos: fix typos and change wording

[Tobias Jakobi]

No functional changes.

Signed-off-by: Tobias Jakobi 
---
 exynos/exynos_fimg2d.c| 8 
 tests/exynos/exynos_fimg2d_test.c | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c
index d22c893..e7b00b1 100644
--- a/exynos/exynos_fimg2d.c
+++ b/exynos/exynos_fimg2d.c
@@ -145,13 +145,13 @@ static void g2d_reset(struct g2d_context *ctx)
 }

 /*
- * g2d_flush - summit all commands and values in user side command buffer
+ * g2d_flush - submit all commands and values in user side command buffer
  * to command queue aware of fimg2d dma.
  *
  * @ctx: a pointer to g2d_context structure.
  *
  * This function should be called after all commands and values to user
- * side command buffer is set to summit that buffer to kernel side driver.
+ * side command buffer are set. It submits that buffer to the kernel side 
driver.
  */
 static int g2d_flush(struct g2d_context *ctx)
 {
@@ -192,7 +192,7 @@ static int g2d_flush(struct g2d_context *ctx)
 /**
  * g2d_init - create a new g2d context and get hardware version.
  *
- * fd: a file descriptor to drm device driver opened.
+ * fd: a file descriptor to an opened drm device.
  */
 drm_public struct g2d_context *g2d_init(int fd)
 {
@@ -524,7 +524,7 @@ g2d_copy_with_scale(struct g2d_context *ctx, struct 
g2d_image *src,
 }

 /**
- * g2d_blend - blend image data in source and destion buffers
+ * g2d_blend - blend image data in source and destination buffers.
  *
  * @ctx: a pointer to g2d_context structure.
  * @src: a pointer to g2d_image structure including image and buffer
diff --git a/tests/exynos/exynos_fimg2d_test.c 
b/tests/exynos/exynos_fimg2d_test.c
index 41fb869..aa140e5 100644
--- a/tests/exynos/exynos_fimg2d_test.c
+++ b/tests/exynos/exynos_fimg2d_test.c
@@ -39,7 +39,7 @@ static unsigned int screen_width, screen_height;
 /*
  * A structure to test fimg2d hw.
  *
- * @solid_fild: fill given color data to source buffer.
+ * @solid_fill: fill given color data to source buffer.
  * @copy: copy source to destination buffer.
  * @copy_with_scale: copy source to destination buffer scaling up or
  * down properly.
@@ -256,7 +256,7 @@ static int g2d_solid_fill_test(struct exynos_device *dev, 
struct exynos_bo *dst)
memset(, 0, sizeof(struct g2d_image));
img.bo[0] = dst->handle;

-   printf("soild fill test.\n");
+   printf("solid fill test.\n");

srand(time(NULL));
img_w = screen_width;
-- 
2.0.5

[PATCH 02/15] exynos: replace G2D_DOUBLE_TO_FIXED macro with function

[Tobias Jakobi]

This also avoids the floating point conversion steps and just
uses pure integer arithmetic.
Since the G2D hardware scaling approach is a bit unintuitive,
document it in the function as well.

Signed-off-by: Tobias Jakobi 
---
 exynos/exynos_fimg2d.c | 19 ++-
 exynos/fimg2d.h|  2 --
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c
index ce1ba1e..d22c893 100644
--- a/exynos/exynos_fimg2d.c
+++ b/exynos/exynos_fimg2d.c
@@ -41,6 +41,15 @@

 #define MIN(a, b)  ((a) < (b) ? (a) : (b))

+static unsigned int g2d_get_scaling(unsigned int src, unsigned int dst)
+{
+   /* The G2D hw scaling factor is a normalized inverse of the scaling 
factor. *
+* For example: When source width is 100 and destination width is 200   
*
+* (scaling of 2x), then the hw factor is NORMALIZE_CONSTANT * 100 / 
200.   */
+
+   return ((src << 16) / dst);
+}
+
 static unsigned int g2d_get_blend_op(enum e_g2d_op op)
 {
union g2d_blend_func_val val;
@@ -428,7 +437,7 @@ g2d_copy_with_scale(struct g2d_context *ctx, struct 
g2d_image *src,
union g2d_rop4_val rop4;
union g2d_point_val pt;
unsigned int scale;
-   double scale_x = 0.0f, scale_y = 0.0f;
+   unsigned int scale_x, scale_y;

g2d_add_cmd(ctx, DST_SELECT_REG, G2D_SELECT_MODE_BGCOLOR);
g2d_add_cmd(ctx, DST_COLOR_MODE_REG, dst->color_mode);
@@ -454,8 +463,8 @@ g2d_copy_with_scale(struct g2d_context *ctx, struct 
g2d_image *src,
scale = 0;
else {
scale = 1;
-   scale_x = (double)src_w / (double)dst_w;
-   scale_y = (double)src_h / (double)dst_h;
+   scale_x = g2d_get_scaling(src_w, dst_w);
+   scale_y = g2d_get_scaling(src_h, dst_h);
}

if (src_x + src_w > src->width)
@@ -487,8 +496,8 @@ g2d_copy_with_scale(struct g2d_context *ctx, struct 
g2d_image *src,

if (scale) {
g2d_add_cmd(ctx, SRC_SCALE_CTRL_REG, G2D_SCALE_MODE_BILINEAR);
-   g2d_add_cmd(ctx, SRC_XSCALE_REG, G2D_DOUBLE_TO_FIXED(scale_x));
-   g2d_add_cmd(ctx, SRC_YSCALE_REG, G2D_DOUBLE_TO_FIXED(scale_y));
+   g2d_add_cmd(ctx, SRC_XSCALE_REG, scale_x);
+   g2d_add_cmd(ctx, SRC_YSCALE_REG, scale_y);
}

pt.val = 0;
diff --git a/exynos/fimg2d.h b/exynos/fimg2d.h
index 4785e2f..8e0321c 100644
--- a/exynos/fimg2d.h
+++ b/exynos/fimg2d.h
@@ -25,8 +25,6 @@
 #define G2D_MAX_CMD_LIST_NR64
 #define G2D_PLANE_MAX_NR   2

-#define G2D_DOUBLE_TO_FIXED(d) ((unsigned int)((d) * 65536.0))
-
 enum e_g2d_color_mode {
/* COLOR FORMAT */
G2D_COLOR_FMT_XRGB,
-- 
2.0.5

[PATCH 01/15] tests/exynos: fimg2d: add a checkerboard test

[Tobias Jakobi]

This makes it easier to spot memory corruptions which don't become
visible when using a plain buffer filled with a solid color (so
corruptions that are just a permutation of the bytes in the buffer).

Signed-off-by: Tobias Jakobi 
---
 tests/exynos/exynos_fimg2d_test.c | 117 ++
 1 file changed, 117 insertions(+)

diff --git a/tests/exynos/exynos_fimg2d_test.c 
b/tests/exynos/exynos_fimg2d_test.c
index c6bd558..41fb869 100644
--- a/tests/exynos/exynos_fimg2d_test.c
+++ b/tests/exynos/exynos_fimg2d_test.c
@@ -55,6 +55,9 @@ struct fimg2d_test_case {
int (*blend)(struct exynos_device *dev,
struct exynos_bo *src, struct exynos_bo *dst,
enum e_g2d_buf_type);
+   int (*checkerboard)(struct exynos_device *dev,
+   struct exynos_bo *src, struct exynos_bo *dst,
+   enum e_g2d_buf_type);
 };

 struct connector {
@@ -207,6 +210,33 @@ static struct exynos_bo *exynos_create_buffer(struct 
exynos_device *dev,
return bo;
 }

+/* Allocate buffer and fill it with checkerboard pattern, where the tiles *
+ * have a random color. The caller has to free the buffer.*/
+void *create_checkerboard_pattern(unsigned int num_tiles_x,
+   unsigned int num_tiles_y, 
unsigned int tile_size)
+{
+   unsigned int *buf;
+   unsigned int x, y, i, j;
+   const unsigned int stride = num_tiles_x * tile_size;
+
+   if (posix_memalign((void*), 64, num_tiles_y * tile_size * stride * 
4) != 0)
+   return NULL;
+
+   for (x = 0; x < num_tiles_x; ++x) {
+   for (y = 0; y < num_tiles_y; ++y) {
+   const unsigned int color = 0xff00 + (random() & 
0xff);
+
+   for (i = 0; i < tile_size; ++i) {
+   for (j = 0; j < tile_size; ++j) {
+   buf[x * tile_size + y * stride * 
tile_size + i + j * stride] = color;
+   }
+   }
+   }
+   }
+
+   return buf;
+}
+
 static void exynos_destroy_buffer(struct exynos_bo *bo)
 {
exynos_bo_destroy(bo);
@@ -533,11 +563,90 @@ err_free_userptr:
return 0;
 }

+static int g2d_checkerboard_test(struct exynos_device *dev,
+   struct exynos_bo *src,
+   struct exynos_bo *dst,
+   enum e_g2d_buf_type type)
+{
+   struct g2d_context *ctx;
+   struct g2d_image src_img = {0}, dst_img = {0};
+   unsigned int src_x, src_y, dst_x, dst_y, img_w, img_h;
+   void *checkerboard = NULL;
+   int ret;
+
+   ctx = g2d_init(dev->fd);
+   if (!ctx)
+   return -EFAULT;
+
+   dst_img.bo[0] = dst->handle;
+
+   src_x = 0;
+   src_y = 0;
+   dst_x = 0;
+   dst_y = 0;
+
+   checkerboard = create_checkerboard_pattern(screen_width / 32, 
screen_height / 32, 32);
+   if (checkerboard == NULL) {
+   ret = -1;
+   goto fail;
+   }
+
+   img_w = screen_width - (screen_width % 32);
+   img_h = screen_height - (screen_height % 32);
+
+   switch (type) {
+   case G2D_IMGBUF_GEM:
+   memcpy(src->vaddr, checkerboard, img_w * img_h * 4);
+   src_img.bo[0] = src->handle;
+   break;
+   case G2D_IMGBUF_USERPTR:
+   src_img.user_ptr[0].userptr = (unsigned long)checkerboard;
+   src_img.user_ptr[0].size = img_w * img_h * 4;
+   break;
+   default:
+   ret = -EFAULT;
+   goto fail;
+   }
+
+   printf("checkerboard test with %s.\n",
+   type == G2D_IMGBUF_GEM ? "gem" : "userptr");
+
+   src_img.width = img_w;
+   src_img.height = img_h;
+   src_img.stride = src_img.width * 4;
+   src_img.buf_type = type;
+   src_img.color_mode = G2D_COLOR_FMT_ARGB | G2D_ORDER_AXRGB;
+
+   dst_img.width = screen_width;
+   dst_img.height = screen_height;
+   dst_img.stride = dst_img.width * 4;
+   dst_img.buf_type = G2D_IMGBUF_GEM;
+   dst_img.color_mode = G2D_COLOR_FMT_ARGB | G2D_ORDER_AXRGB;
+   src_img.color = 0xff00;
+   ret = g2d_solid_fill(ctx, _img, src_x, src_y, screen_width, 
screen_height);
+   if (ret < 0)
+   goto fail;
+
+   ret = g2d_copy(ctx, _img, _img, src_x, src_y, dst_x, dst_y,
+   img_w, img_h);
+   if (ret < 0)
+   goto fail;
+
+   g2d_exec(ctx);
+
+fail:
+   free(checkerboard);
+   g2d_fini(ctx);
+
+   return ret;
+}
+
 static struct fimg2d_test_case test_case = {
.solid_fill = _solid_fill_test,
.copy = _copy_test,
.copy_with_scale = _copy_with_scale_test,
.blend = _blend_test,
+   

[libdrm] improvements to userspace exynos component

[Tobias Jakobi]

Hello,

here are some miscellaneous improvements (small features, bugfixes, spelling 
fixes, etc.) for the exynos component of libdrm. The general idea is to let 
userspace use the G2D engine functionality more 
efficiently.

If someone is interested in an application that actually makes use of this, the 
RetroArch frontend has a custom video backend:
https://github.com/libretro/RetroArch/blob/master/gfx/drivers/exynos_gfx.c


Please review and let me know what I can improve.

With best wishes,
Tobias

[RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms

[Russell King - ARM Linux]

On Tue, Feb 03, 2015 at 03:17:27PM +0100, Arnd Bergmann wrote:
> On Tuesday 03 February 2015 09:04:03 Rob Clark wrote:
> > Since I'm stuck w/ an iommu, instead of built in mmu, my plan was to
> > drop use of dma-mapping entirely (incl the current call to dma_map_sg,
> > which I just need until we can use drm_cflush on arm), and
> > attach/detach iommu domains directly to implement context switches.
> > At that point, dma_addr_t really has no sensible meaning for me.
> 
> I think what you see here is a quite common hardware setup and we really
> lack the right abstraction for it at the moment. Everybody seems to
> work around it with a mix of the dma-mapping API and the iommu API.
> These are doing different things, and even though the dma-mapping API
> can be implemented on top of the iommu API, they are not really compatible.

I'd go as far as saying that the "DMA API on top of IOMMU" is more
intended to be for a system IOMMU for the bus in question, rather
than a device-level IOMMU.

If an IOMMU is part of a device, then the device should handle it
(maybe via an abstraction) and not via the DMA API.  The DMA API should
be handing the bus addresses to the device driver which the device's
IOMMU would need to generate.  (In other words, in this circumstance,
the DMA API shouldn't give you the device internal address.)

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Can't gmake libdrm-2.4.59 on OmniOS

[Emil Velikov]

On 1 February 2015 at 01:03, CodeSwim OS Development
 wrote:
> I'm trying to build libdrm and have an issue when I gmake after
> configuring.  Steps to reproduce:
>
> # uname -a
> SunOS omnios 5.11 omnios-10b9c79 i86pc i386 i86pc
>
Where can one get a copy of OmniOS ? Free of charge of course :-P
But more seriously

>From the massive log I can spot a few interesting things
 - _FILE_OFFSET_BITS redefinition.
Autotools correctly detects and sets it set to 64 (as you've got a
32bit system/build), yet further down the gcc headers, it's already
set to 32. This won't "break" the build but will likely cause problems
at runtime.

 - A ton of -Wparentheses warnings.
These should be safe - you might also want to silence them so that
serious issues are clearer.

 - And last but not leask the missing _IOC symbol.
Gentoo people have a patch on the topic [1] - might be nice to clean
it up and upstream it.

Cheers,
Emil

[1] 
http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/x11-libs/libdrm/files/libdrm-2.4.58-solaris.patch?view=markup

--
Dive into the World of Parallel Programming. The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net/
--
___
Dri-devel mailing list
Dri-devel at lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

[RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms

[Russell King - ARM Linux]

On Tue, Feb 03, 2015 at 09:04:03AM -0500, Rob Clark wrote:
> Since I'm stuck w/ an iommu, instead of built in mmu, my plan was to
> drop use of dma-mapping entirely (incl the current call to dma_map_sg,
> which I just need until we can use drm_cflush on arm), and
> attach/detach iommu domains directly to implement context switches.
> At that point, dma_addr_t really has no sensible meaning for me.

So how do you intend to import from a subsystem which only gives you
the dma_addr_t?

If you aren't passing system memory, you have no struct page.  You can't
fake up a struct page.  What this means is that struct scatterlist can't
represent it any other way.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms

[Russell King - ARM Linux]

On Tue, Feb 03, 2015 at 02:28:26PM +0100, Christian Gmeiner wrote:
> 2015-02-03 13:28 GMT+01:00 Russell King - ARM Linux  arm.linux.org.uk>:
> > What I've found with *my* etnaviv drm implementation (not Christian's - I
> > found it impossible to work with Christian, especially with the endless
> > "msm doesn't do it that way, so we shouldn't" responses and his attitude
> > towards cherry-picking my development work [*]) is that it's much easier to
> > keep the GPU MMU local to the GPU and under the control of the DRM MM code,
> > rather than attaching the IOMMU to the DMA API and handling it that way.
> >
> 
> Keep in mind that I tried to reach you several times via mail and irc
> and you simply
> ignored me. Did you know that took almost all of your patches (with
> small changes)?
> And I needed to cherry pick you patches as they were a) wrong, b) solved in a
> different way or c) had "hack" in the subject. I am quite sorry that I
> ended that
> way, but it is not only my fault!

Exactly - you *took* every patch that I published whether I was ready
for you to take it or not.

That's not how kernel development works.  Kernel development works by
people working on the code, and *pushing* patches or git trees upstream.
It doesn't work by having people running around *taking* patches from
people just because they feel like it.

I asked you several times not to do that which means the only way I can
control you is by *not* publishing my changes, thereby denying other
people the ability to test my changes.

Another result of you *taking* patches from me is that you totally
*screwed* my ability to work with you.  If you make this stuff
unnecessary hard, you can expect people to walk away, and that's
precisely what I've done.

There's also the issue of you *taking* my patches and then applying
them to your tree with your own modifications, again *screwing* my tree,
and screwing my ability - again - to work with you.

Many of my patches in your repository are also marked as you being the
author of them... which _really_ is not nice.

Your "review" comments of "based 1:1 on the MSM driver" were really crazy.
Just because one DRM driver does something one way does not make it the
only way, nor does it make it suitable for use everywhere, even if you
modelled your driver on MSM.  It certainly doesn't mean that the way the
MSM driver does it is correct either.

And frankly, you calling my patches "wrong" is laughable.  I have a stable
fully functional Xorg DDX driver here which works with my version of your
etnaviv DRM across several Vivante GPUs - GC660 on Dove, and two revisions
of GC320 on iMX6 (which are notoriously buggy).  No kernel oopses, no GPU
lockups.  I've had machines with uptimes of a month here with it, with the
driver being exercised frequently during that period.

You refused to take things such as the DMA address monitoring for GPU
hangups to stop it mis-firing.  This _is_ a bug fix.  Without that, your
driver spits out random GPU hangups when there isn't actually any hangup
at all.  *Reliably* so.  Your excuse for not taking it was "The current
vivante driver doesn't do that."  While that's true, the V2 Vivante
drivers _do_ do it in their "guard thread" - and they do it because -
as I already explained to you - it's entirely possible for the GPU to
take a long time, longer than your hangcheck timeout, to render a
series of 1080p operations.  And again, not everything that the Vivante
drivers do is correct either.  Jon and myself know that very well having
spent a long time debugging their GPL'd offerings.

Even the "hack" patch was mostly correct - the reason that it is labelled
as a "hack" is because - as the commit log said - it should really be
done by the MMUv1 code, but that requires your entire IOMMU _bodge_ to be
rewritten properly.  Even the Vivante drivers use that region when they
can.

Then there's also the compatibility with the etnaviv library - which is
an important thing if you want people to make use of your driver.  You
applied the patches for and then reverted which completely screws the
Xorg DDX driver, making it impossible to support both etnaviv and
etnadrm without having two almost identical copies of the same code.  I
don't want to maintain almost identical copies of that same code, and
no one in their right mind would want that.

Having some level of sane user compatibility between etnaviv and
etnaviv drm will _gain_ you uses as it will allow people to write code
which works on both platforms - and it's really not difficult to do.
(In fact, I've proven it by writing a shim layer between the etnaviv
API and the DRM interfaces in the DDX driver.)

Then there's the round-robin IOMMU address space allocation, which is
required to avoid corrupted pixmaps (which is something that _your_
driver does very very well - in fact, so well that it corrupts system
memory), and the reaping of the IOMMU space when we run out of IOMMU
space to map.

Now, on to things that you do wrong.

[PATCH 1/3] drm/panel: Add display_timing support

[Thierry Reding]

On Thu, Dec 11, 2014 at 06:32:44PM +0100, Philipp Zabel wrote:
> Many panel data sheets additionally to typical values list allowed ranges for
> timings such as hsync/vsync lengths, porches, and the pixel clock rate. These
> can be stored in a struct display_timing, to be used by an encoder mode_fixup
> callback to clamp user provided timing values or to validate workarounds for
> clock source limitations.
> 
> This patch adds a new drm_panel_funcs callback that returns the panels'
> available display_timing entries.
> 
> Signed-off-by: Philipp Zabel 
> ---
>  include/drm/drm_panel.h | 5 +
>  1 file changed, 5 insertions(+)

Sorry for taking so long to get back on this. I generally like the idea,
though a couple of things are unclear to me.

In struct display_timing we have:

struct timing_entry hactive;
...
struct timing_entry vactive;

I wonder if that really makes sense. Are there really datasheets that
provide a valid range for the number of horizontal and vertical active
pixels?

> 
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index 1fbcc96..13ff44b 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -29,6 +29,7 @@
>  struct drm_connector;
>  struct drm_device;
>  struct drm_panel;
> +struct display_timing;
>  
>  /**
>   * struct drm_panel_funcs - perform operations on a given panel
> @@ -38,6 +39,8 @@ struct drm_panel;
>   * @enable: enable panel (turn on back light, etc.)
>   * @get_modes: add modes to the connector that the panel is attached to and
>   * return the number of modes added
> + * @get_timings: copy display timings into the provided array and return
> + * the number of display timings available
>   *
>   * The .prepare() function is typically called before the display controller
>   * starts to transmit video data. Panel drivers can use this to turn the 
> panel
> @@ -68,6 +71,8 @@ struct drm_panel_funcs {
>   int (*prepare)(struct drm_panel *panel);
>   int (*enable)(struct drm_panel *panel);
>   int (*get_modes)(struct drm_panel *panel);
> + int (*get_timings)(struct drm_panel *panel, unsigned int num_timings,
> +struct display_timing *timings);

Also are there even panels that support more than one set of timings?
I've never heard of panels that are actually able to do more than one
mode, so I'm wondering if num_timings isn't overdoing it a little here.
I guess it doesn't hurt all that much, though.

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

[PATCH -v2] drm/exynos: do not disable hdmi clocks for exynos5420

[Joonyoung Shim]

Hi,

On 02/02/2015 11:26 PM, Gustavo Padovan wrote:
> From: Prathyush K 
> 
> When VPLL clock of less than 140 MHz was used and all the three clocks -
> hdmiphy, hdmi, sclk_hdmi are disabled, the system hangs during S2R when
> HDMI is connected. Since we want to use a vpll clock of 70.5 MHz, we
> cannot disable these 3 clocks before suspending.  This patch add an extra
> clk enable/disable of hdmi and sclk_hdmi outside of the pm ops so these
> clocks are always enabled. Now system suspends and resumes with HDMI
> connected with VPLL set at 70.5 MHz.
> 
> This is not the best solution, but it inhibits a crash while we figure out
> what is the correct solution for this issue.
> 
> Signed-off-by: Prathyush K 
> Signed-off-by: Andrew Bresticker 
> Signed-off-by: Gustavo Padovan 
> ---
>  drivers/gpu/drm/exynos/exynos_hdmi.c | 20 
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c 
> b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index 6aa0d65..c6baf64 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -2254,6 +2254,20 @@ static int hdmi_resources_init(struct hdmi_context 
> *hdata)
>   } else
>   res->reg_hdmi_en = NULL;
>  
> + /*
> +  * For these two clocks exynos5420 fails to suspend if VPLL clock of
> +  * 70.5 MHz is used and these clocks are disabled before suspend. So
> +  * enable them here.
> +  * Note that this will keep the clocks enabled for the entire
> +  * lifetime of the driver and waste energy when it is suspended.
> +  * However it prevents the system crash until a better solution is
> +  * found
> +  */
> + if (of_device_is_compatible(dev->of_node, "samsung,exynos5420-hdmi")) {
> + clk_prepare_enable(res->sclk_hdmi);
> + clk_prepare_enable(res->hdmi);
> + }
> +
>   return ret;
>  fail:
>   DRM_ERROR("HDMI resource init - failed\n");
> @@ -2516,6 +2530,12 @@ static int hdmi_remove(struct platform_device *pdev)
>   if (hdata->res.reg_hdmi_en)
>   regulator_disable(hdata->res.reg_hdmi_en);
>  
> + if (of_device_is_compatible(hdata->dev->of_node,
> + "samsung,exynos5420-hdmi")) {
> + clk_disable_unprepare(hdata->res.sclk_hdmi);
> + clk_disable_unprepare(hdata->res.hdmi);
> + }
> +
>   if (hdata->hdmiphy_port)
>   put_device(>hdmiphy_port->dev);
>   put_device(>ddc_adpt->dev);
> 

Looks fine to me but i'm not sure any there are any side effects to
change parent clock of mout_hdmi clock.

Acked-by: Joonyoung Shim 

Thanks.

[RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms

[Christian Gmeiner]

2015-02-03 13:28 GMT+01:00 Russell King - ARM Linux :
> On Tue, Feb 03, 2015 at 08:48:56AM +0100, Daniel Vetter wrote:
>> On Mon, Feb 02, 2015 at 03:30:21PM -0500, Rob Clark wrote:
>> > On Mon, Feb 2, 2015 at 11:54 AM, Daniel Vetter  wrote:
>> > >> My initial thought is for dma-buf to not try to prevent something than
>> > >> an exporter can actually do.. I think the scenario you describe could
>> > >> be handled by two sg-lists, if the exporter was clever enough.
>> > >
>> > > That's already needed, each attachment has it's own sg-list. After all
>> > > there's no array of dma_addr_t in the sg tables, so you can't use one sg
>> > > for more than one mapping. And due to different iommu different devices
>> > > can easily end up with different addresses.
>> >
>> >
>> > Well, to be fair it may not be explicitly stated, but currently one
>> > should assume the dma_addr_t's in the dmabuf sglist are bogus.  With
>> > gpu's that implement per-process/context page tables, I'm not really
>> > sure that there is a sane way to actually do anything else..
>>
>> Hm, what does per-process/context page tables have to do here? At least on
>> i915 we have a two levels of page tables:
>> - first level for vm/device isolation, used through dma api
>> - 2nd level for per-gpu-context isolation and context switching, handled
>>   internally.
>>
>> Since atm the dma api doesn't have any context of contexts or different
>> pagetables, I don't see who you could use that at all.
>
> What I've found with *my* etnaviv drm implementation (not Christian's - I
> found it impossible to work with Christian, especially with the endless
> "msm doesn't do it that way, so we shouldn't" responses and his attitude
> towards cherry-picking my development work [*]) is that it's much easier to
> keep the GPU MMU local to the GPU and under the control of the DRM MM code,
> rather than attaching the IOMMU to the DMA API and handling it that way.
>

Keep in mind that I tried to reach you several times via mail and irc
and you simply
ignored me. Did you know that took almost all of your patches (with
small changes)?
And I needed to cherry pick you patches as they were a) wrong, b) solved in a
different way or c) had "hack" in the subject. I am quite sorry that I
ended that
way, but it is not only my fault!

> There are several reasons for that:
>
> 1. DRM has a better idea about when the memory needs to be mapped to the
>GPU, and it can more effectively manage the GPU MMU.
>
> 2. The GPU MMU may have TLBs which can only be flushed via a command in
>the GPU command stream, so it's fundamentally necessary for the MMU to
>be managed by the GPU driver so that it knows when (and how) to insert
>the flushes.
>
>
> * - as a direct result of that, I've stopped all further development of
> etnaviv drm, and I'm intending to strip it out from my Xorg DDX driver
> as the etnaviv drm API which Christian wants is completely incompatible
> with the non-etnaviv drm, and that just creates far too much pain in the
> DDX driver.
>

That is bad, but life moves on.

greets
--
Christian Gmeiner, MSc

https://soundcloud.com/christian-gmeiner

[PATCH] drm: remove DRM_FORMAT_NV12MT

[Gustavo Padovan]

2015-02-03 Daniel Vetter :

> So this has been merged originally in
> 
> commit 83052d4d5cd518332440bb4ee63d68bb5f744e0f
> Author: Seung-Woo Kim 
> Date:   Thu Dec 15 15:40:55 2011 +0900
> 
> drm: Add multi buffer plane pixel formats
> 
> which hasn't seen a lot of review really. The problem is that it's not
> a real pixel format, but just a different way to lay out NV12 pixels
> in macroblocks, i.e. a tiling format.
> 
> The new way of doing this is with the soon-to-be-merge fb modifiers.
> 
> This was brough up in some long irc discussion around the entire
> topic, as an example of where things have gone wrong. Luckily we can
> correct the mistake:
> - The kms side support for NV12MT is all dead code because
>   format_check in drm_crtc.c never accepted NV12MT.
> - The gem side for the gsc support doesn't look better: The code
>   forgets to set the pixel format and makes a big mess with the tiling
>   mode bits, inadvertedly setting them all.
> 
> Conclusion: This never really worked (at least not in upstream) and
> hence we can savely correct our mistake here.
> 
> Cc: Seung-Woo Kim 
> Cc: Inki Dae 
> Cc: Kyungmin Park 
> Cc: Rob Clark 
> Cc: Daniel Stone 
> Cc: Damien Lespiau 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/exynos/exynos_drm_fimc.c  | 14 ++
>  drivers/gpu/drm/exynos/exynos_drm_gsc.c   |  6 --
>  drivers/gpu/drm/exynos/exynos_drm_plane.c |  1 -
>  drivers/gpu/drm/exynos/exynos_mixer.c |  2 --
>  include/uapi/drm/drm_fourcc.h |  3 ---
>  5 files changed, 2 insertions(+), 24 deletions(-)

That seems good to me.

Reviewed-by: Gustavo Padovan 

Gustavo

[RFC v3 3/4] drm/i915: Add new panel driver based on crystal cove pmic

[Thierry Reding]

On Wed, Jan 21, 2015 at 04:48:12PM +0530, Shobhit Kumar wrote:
> This driver provides support for the "crystal_cove_panel" cell device.
> On BYT-T pmic has to be used to enable/disable panel.
> 
> v2: Addressed Jani's comments
> - Moved inside i915
> - Correct licensing
> - Remove unused stuff
> - Do not initialize prepare/unprepare as they are not needed as of now
> - Correct backlight off delay
> 
> Signed-off-by: Shobhit Kumar 
> ---
>  drivers/gpu/drm/i915/Kconfig   |  12 ++
>  drivers/gpu/drm/i915/Makefile  |   3 +
>  drivers/gpu/drm/i915/intel-panel-crystalcove.c | 159 
> +
>  3 files changed, 174 insertions(+)
>  create mode 100644 drivers/gpu/drm/i915/intel-panel-crystalcove.c
> 
> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index 4e39ab3..0510ef0 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -69,3 +69,15 @@ config DRM_I915_PRELIMINARY_HW_SUPPORT
> option changes the default for that module option.
>  
> If in doubt, say "N".
> +
> +config DRM_I915_PANEL_CRYSTALCOVE_PMIC
> + bool "Enable drm panel for crystal cove pmic based control"
> + depends on DRM_I915
> + depends on DRM_PANEL
> + default n

n is the default default.

> diff --git a/drivers/gpu/drm/i915/intel-panel-crystalcove.c 
> b/drivers/gpu/drm/i915/intel-panel-crystalcove.c
[...]
> +#define PMIC_PANEL_EN0x52
> +#define PMIC_PWM_EN  0x51
> +#define PMIC_BKL_EN  0x4B
> +#define PMIC_PWM_LEVEL   0x4E

These look like they should be GPIOs/regulators and a PWM instead. So I
think you'd need to further split up the MFD device to accomodate for
this.

> +static inline struct crystalcove_panel *to_crystalcove_panel(struct 
> drm_panel *panel)

Please wrap this so it doesn't cross the 80-character boundary.

> +static int crystalcove_panel_disable(struct drm_panel *panel)
> +{
> + struct crystalcove_panel *p = to_crystalcove_panel(panel);
> +
> + if (!p->enabled)
> + return 0;
> +
> + DRM_DEBUG_KMS("\n");
> +
> + /* invoke the pmic driver */
> + regmap_write(p->regmap, PMIC_PANEL_EN, 0x00);

A datasheet would be really good to determine whether this is the
correct place to write this. There are ->prepare() and ->unprepare()
callbacks that get the panel into a state where you can communicate
with it. This being a DSI panel I would assume that you need to enable
the panel to some degree so you can send DSI commands. So most likely
this enable "GPIO" should be set in ->unprepare().

> +static int crystalcove_panel_enable(struct drm_panel *panel)
> +{
> + struct crystalcove_panel *p = to_crystalcove_panel(panel);
> +
> + if (p->enabled)
> + return 0;
> +
> + DRM_DEBUG_KMS("\n");
> +
> + /* invoke the pmic driver */
> + regmap_write(p->regmap, PMIC_PANEL_EN, 0x01);

Similarly I'd expect this to go into ->prepare() to make sure that you
can communicate with the panel after ->prepare().

> + /* Enable BKL as well */
> + regmap_write(p->regmap, PMIC_BKL_EN, 0xFF);

Writing 0xff to an "enable" register seems weird. Again the datasheet
would help in determining the right thing to do here.

> + regmap_write(p->regmap, PMIC_PWM_EN, 0x01);
> + msleep(20);
> + regmap_write(p->regmap, PMIC_PWM_LEVEL, 255);

This definitely looks like it should be a PWM driver. Also how do you
handle backlight brightness control? You only set things to either full
off or full on in this driver.

> +
> + drm_panel_init(>base);
> + panel->base.dev = dev;
> + panel->base.funcs = _panel_funcs;
> +
> + drm_panel_add(>base);

This function can theoretically fail, although it doesn't (at present),
so checking the error might be a good idea.

> +static int crystalcove_panel_remove(struct platform_device *pdev)
> +{
> + struct crystalcove_panel *panel = platform_get_drvdata(pdev);
> +
> + DRM_DEBUG_KMS("\n");
> +
> + drm_panel_detach(>base);
> + drm_panel_remove(>base);
> +
> + crystalcove_panel_disable(>base);
> +
> + return 0;
> +}
> +
> +static struct platform_driver crystalcove_panel_driver = {
> + .probe = crystalcove_panel_probe,
> + .remove = crystalcove_panel_remove,
> + .driver = {
> + .name = "crystal_cove_panel",
> + },
> +};
> +
> +module_platform_driver(crystalcove_panel_driver);

What's also weird here is that you claim this to be a DSI panel, yet you
use a platform driver. The right thing to do would be to instantiate the
device as mipi_dsi_device, with the DSI controller being the parent.

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

[RFC v3 2/4] mfd: Add a new cell device for panel controlled by crystal cove pmic

[Thierry Reding]

On Wed, Jan 21, 2015 at 04:48:11PM +0530, Shobhit Kumar wrote:
> On BYT-T configuration, panel enable/disable signals are routed through
> PMIC. Add a cell device for the same.
> 
> Signed-off-by: Shobhit Kumar 
> ---
>  drivers/mfd/intel_soc_pmic_crc.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/mfd/intel_soc_pmic_crc.c 
> b/drivers/mfd/intel_soc_pmic_crc.c
> index c85e2ec..c8ccc24 100644
> --- a/drivers/mfd/intel_soc_pmic_crc.c
> +++ b/drivers/mfd/intel_soc_pmic_crc.c
> @@ -109,6 +109,9 @@ static struct mfd_cell crystal_cove_dev[] = {
>   {
>   .name = "crystal_cove_pmic",
>   },
> + {
> + .name = "crystal_cove_panel",
> + },
>  };
>  
>  static struct regmap_config crystal_cove_regmap_config = {

This doesn't look at all right. A PMIC doesn't typically contain a panel
so this likely is a wrong description of the hardware. Is the datasheet
for the Crystal Cove PMIC available somewhere? Google doesn't turn any-
thing useful up.

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

[RFC v3 1/4] drm: Add support to find drm_panel by name

[Thierry Reding]

On Wed, Jan 21, 2015 at 04:48:10PM +0530, Shobhit Kumar wrote:
> For scenarios where OF is not available, we can use panel identification by
> name.
> 
> Signed-off-by: Shobhit Kumar 
> ---
>  drivers/gpu/drm/drm_panel.c | 18 ++
>  include/drm/drm_panel.h |  3 +++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> index 2ef988e..e1cb8cf 100644
> --- a/drivers/gpu/drm/drm_panel.c
> +++ b/drivers/gpu/drm/drm_panel.c
> @@ -95,6 +95,24 @@ struct drm_panel *of_drm_find_panel(struct device_node *np)
>  EXPORT_SYMBOL(of_drm_find_panel);
>  #endif
>  
> +struct drm_panel *drm_find_panel_by_name(const char *name)
> +{
> + struct drm_panel *panel;
> +
> + mutex_lock(_lock);
> +
> + list_for_each_entry(panel, _list, list) {
> + if (strcmp(panel->name, name) == 0) {
> + mutex_unlock(_lock);
> + return panel;
> + }
> + }
> +
> + mutex_unlock(_lock);
> + return NULL;
> +}
> +EXPORT_SYMBOL(drm_find_panel_by_name);
> +
>  MODULE_AUTHOR("Thierry Reding ");
>  MODULE_DESCRIPTION("DRM panel infrastructure");
>  MODULE_LICENSE("GPL and additional rights");
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index 1fbcc96..1ef9ff3 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -74,6 +74,7 @@ struct drm_panel {
>   struct drm_device *drm;
>   struct drm_connector *connector;
>   struct device *dev;
> + char name[NAME_MAX];

I thought I had said this before, but I can't find any record, so here
goes again: NAME_MAX is a pretty large number and it increases the size
of this structure a lot. Why not use dev_name(dev) to do the matching?
Or if that doesn't work for some reason, allocate a string of the right
size, or use a static one.

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

[RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms

[Daniel Vetter]

On Tue, Feb 03, 2015 at 12:28:14PM +, Russell King - ARM Linux wrote:
> On Tue, Feb 03, 2015 at 08:48:56AM +0100, Daniel Vetter wrote:
> > On Mon, Feb 02, 2015 at 03:30:21PM -0500, Rob Clark wrote:
> > > On Mon, Feb 2, 2015 at 11:54 AM, Daniel Vetter  wrote:
> > > >> My initial thought is for dma-buf to not try to prevent something than
> > > >> an exporter can actually do.. I think the scenario you describe could
> > > >> be handled by two sg-lists, if the exporter was clever enough.
> > > >
> > > > That's already needed, each attachment has it's own sg-list. After all
> > > > there's no array of dma_addr_t in the sg tables, so you can't use one sg
> > > > for more than one mapping. And due to different iommu different devices
> > > > can easily end up with different addresses.
> > > 
> > > 
> > > Well, to be fair it may not be explicitly stated, but currently one
> > > should assume the dma_addr_t's in the dmabuf sglist are bogus.  With
> > > gpu's that implement per-process/context page tables, I'm not really
> > > sure that there is a sane way to actually do anything else..
> > 
> > Hm, what does per-process/context page tables have to do here? At least on
> > i915 we have a two levels of page tables:
> > - first level for vm/device isolation, used through dma api
> > - 2nd level for per-gpu-context isolation and context switching, handled
> >   internally.
> > 
> > Since atm the dma api doesn't have any context of contexts or different
> > pagetables, I don't see who you could use that at all.
> 
> What I've found with *my* etnaviv drm implementation (not Christian's - I
> found it impossible to work with Christian, especially with the endless
> "msm doesn't do it that way, so we shouldn't" responses and his attitude
> towards cherry-picking my development work [*]) is that it's much easier to
> keep the GPU MMU local to the GPU and under the control of the DRM MM code,
> rather than attaching the IOMMU to the DMA API and handling it that way.
> 
> There are several reasons for that:
> 
> 1. DRM has a better idea about when the memory needs to be mapped to the
>GPU, and it can more effectively manage the GPU MMU.
> 
> 2. The GPU MMU may have TLBs which can only be flushed via a command in
>the GPU command stream, so it's fundamentally necessary for the MMU to
>be managed by the GPU driver so that it knows when (and how) to insert
>the flushes.

3. Switching between different address spaces (for per gpu context
isolation) often requires intricate knowledge about the gpu and close
coordination. Well maybe just a part of 2 really, but an important one.

Fully agree and tbh I'm not sure whether the current push in arm to expose
all gpu mmus as iommus is solid. Even for pasid (per-context iommu tables)
which is a big official standard there's still a lot of open questions
about how to do it properly. And it requires strict hw support so that the
hw always knows which pasid it should use for a given dma access.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH v3] drm/panel: add s6e3ha2 AMOLED panel driver

[Thierry Reding]

tx->supplies);
> + if (ret < 0)
> + return ret;
> +
> + msleep(25);
> +
> + gpiod_set_value(ctx->panel_en_gpio, 0);
> + usleep_range(5000, 6000);
> + gpiod_set_value(ctx->panel_en_gpio, 1);
> +
> + gpiod_set_value(ctx->reset_gpio, 1);
> + usleep_range(5000, 6000);
> + gpiod_set_value(ctx->reset_gpio, 0);
> + usleep_range(5000, 6000);
> + gpiod_set_value(ctx->reset_gpio, 1);

I would expect the value after power on to be 0 for reset. Perhaps you
need GPIO_ACTIVE_LOW in your device tree? Also why do you toggle thrice?
I would assume that putting the peripheral into reset and taking it out
again would be enough.

> +static int s6e3ha2_prepare(struct drm_panel *panel)
> +{
> + struct s6e3ha2 *ctx = panel_to_s6e3ha2(panel);
> + int ret;
> +
> + ret = s6e3ha2_power_on(ctx);
> + if (ret < 0)
> + return ret;
> +
> + s6e3ha2_panel_init(ctx);
> + if (ctx->error < 0)

This is one of the reasons why ctx->error is a bad idea. It's completely
unintuitive to check ctx->error here because nobody's actually setting
it in this context.

> + s6e3ha2_unprepare(panel);

This is somewhat asymmetric. I would expect the s6e3ha2_panel_init() to
undo what it did on failure, so that you would only have to call
s6e3ha2_power_off() here and undo what you did in *this* function.

> +static int s6e3ha2_enable(struct drm_panel *panel)
> +{
> + return 0;
> +}

This is really where you're supposed to turn on the backlight or
similar. Where does that happen?

> +static int s6e3ha2_get_modes(struct drm_panel *panel)
> +{
> + struct drm_connector *connector = panel->connector;
> + struct s6e3ha2 *ctx = panel_to_s6e3ha2(panel);
> + struct drm_display_mode *mode;
> +
> + mode = drm_mode_create(connector->dev);
> + if (!mode) {
> + DRM_ERROR("failed to create a new display mode\n");
> + return 0;
> + }
> +
> + drm_display_mode_from_videomode(>vm, mode);
> + mode->width_mm = ctx->width_mm;
> + mode->height_mm = ctx->height_mm;
> + connector->display_info.width_mm = mode->width_mm;
> + connector->display_info.height_mm = mode->height_mm;
> +
> + mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
> + drm_mode_probed_add(connector, mode);
> +
> + return 1;
> +}

Like I said before, the mode is implied by the compatible value, so no
need to parse it from device tree.

> +static int s6e3ha2_probe(struct mipi_dsi_device *dsi)
> +{
> + struct device *dev = >dev;
> + struct s6e3ha2 *ctx;
> + int ret;
> +
> + ctx = devm_kzalloc(dev, sizeof(struct s6e3ha2), GFP_KERNEL);

sizeof(*ctx) is much shorter.

> + ctx->reset_gpio = devm_gpiod_get(dev, "reset");
> + if (IS_ERR(ctx->reset_gpio)) {
> + dev_err(dev, "cannot get reset-gpios %ld\n",
> + PTR_ERR(ctx->reset_gpio));
> + return PTR_ERR(ctx->reset_gpio);
> + }
> + ret = gpiod_direction_output(ctx->reset_gpio, 1);

For consistency the above two lines should be separated by a blank line.

> + if (ret < 0) {
> + dev_err(dev, "cannot configure reset-gpios %d\n", ret);
> + return ret;
> + }
> +
> + ctx->panel_en_gpio = devm_gpiod_get(dev, "panel-en");
> + if (IS_ERR(ctx->panel_en_gpio)) {
> + dev_err(dev, "cannot get panel-en-gpios %ld\n",
> + PTR_ERR(ctx->panel_en_gpio));
> + return PTR_ERR(ctx->panel_en_gpio);
> + }
> + ret = gpiod_direction_output(ctx->panel_en_gpio, 1);

Same here.

> + if (ret < 0) {
> + dev_err(dev, "cannot configure panel-en-gpios %d\n", ret);
> + return ret;
> + }

You seem to be turning on the panel here. That's wrong because you're
supposed to wait for the display driver to tell you to turn it on via
->prepare() and ->enable().

> +
> + drm_panel_init(>panel);
> + ctx->panel.dev = dev;
> + ctx->panel.funcs = _drm_funcs;

I don't see a use for the _drm in here.

> +static struct of_device_id s6e3ha2_of_match[] = {

static const, please.

> + { .compatible = "samsung,s6e3ha2" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, s6e3ha2_of_match);
> +
> +static struct mipi_dsi_driver s6e3ha2_driver = {
> + .probe = s6e3ha2_probe,
> + .remove = s6e3ha2_remove,
> + .driver = {
> + .name = "panel_s6e3ha2",

Please use a - instead of an _ here, for consistency with other drivers.

> + .owner = THIS_MODULE,

No need for this anymore.

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

[PULL] drm-amdkfd-next-fixes

[Oded Gabbay]

Hi Dave,

Two more fixes for 3.20 amdkfd code:

- Fixing accounting of active queues
- Preserving a register internal state

Thanks,

Oded

The following changes since commit e4bf44b3b558742fb7c58b4d34e206c8942f07e6:

  drm/modes: Print the mode status in human readable form (2015-02-03 11:13:27 
+1000)

are available in the git repository at:

  git://people.freedesktop.org/~gabbayo/linux 
tags/drm-amdkfd-next-fixes-2015-02-03

for you to fetch changes up to d752f95e55ed36648fc2bcbca348f5113920c357:

  drm/amdkfd: Preserve CP_MQD_IQ_RPTR internal state (2015-01-19 11:47:34 -0600)


Jay Cornwall (2):
  drm/amdkfd: Fix dqm->queue_count tracking
  drm/amdkfd: Preserve CP_MQD_IQ_RPTR internal state

 drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 9 ++---
 drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c  | 4 +++-
 2 files changed, 9 insertions(+), 4 deletions(-)

[PATCH 1/4] drm/irq: Add drm_crtc_vblank_reset

[Laurent Pinchart]

Hi Daniel,

Thank you for the patch.

On Tuesday 03 February 2015 11:30:11 Daniel Vetter wrote:
> At driver load we need to tell the vblank code about the state of the
> pipes, so that the logic around reject vblank_get when the pipe is off
> works correctly.
> 
> Thus far i915 used drm_vblank_off, but one of the side-effects of it
> is that it also saves the vblank counter. And for that it calls down
> into the ->get_vblank_counter hook. Which isn't really a good idea
> when the pipe is off for a few reasons:
> - With runtime pm the register might not respond.
> - If the pipe is off some datastructures might not be around or
>   unitialized.
> 
> The later is what blew up on gen3: We look at intel_crtc->config to
> compute the vblank counter, and for a disabled pipe at boot-up that's
> just not there. Thus far this was papered over by a check for
> intel_crtc->active, but I want to get rid of that (since it's fairly
> race, vblank hooks are called from all kinds of places).
> 
> So prep for that by adding a _reset functions which only does what we
> really need to be done at driver load: Mark the vblank pipe as off,
> but don't do any vblank counter saving or event flushing - neither of
> that is required.

(Thinking out loud)

In principle this looks good, but I find the naming pretty confusing. The 
drm_crtc_vblank_* functions now have get, put, on, off and reset variants. The 
fact that on is supposed to be called both at runtime and an init time while 
off is replaced by reset at init time breaks the symmetry between on and off. 
What would you think of a drm_crtc_vblank_init() function instead, used at 
init time only, and that would take an enabled boolean argument ?

On the other hand, calling drm_crtc_vblank_on() at init time also makes sense, 
as even if the CRTC is active the vblank interrupt should be off then, and an 
explicit call to the function means "turn the vblank interrupt on". I'm thus 
not totally opposed to keeping that as-is. Wouldn't it then be better to 
modify the core to default to off, and let drivers call drm_crtc_vblank_on() 
explicitly if the default isn't correct ? I think I like this solution better, 
and it could be conditioned by a driver flag if we don't want to audit all 
drivers for possible breakages.

> Cc: Laurent Pinchart 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_irq.c| 32 
>  drivers/gpu/drm/i915/intel_display.c |  4 ++--
>  include/drm/drmP.h   |  1 +
>  3 files changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 75647e7f012b..1e5fb1b994d7 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -1226,6 +1226,38 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc)
>  EXPORT_SYMBOL(drm_crtc_vblank_off);
> 
>  /**
> + * drm_crtc_vblank_reset - reset vblank state to off on a CRTC
> + * @crtc: CRTC in question
> + *
> + * Drivers can use this function to reset the vblank state to off at load
> time.
> + * Drivers should use this together with the drm_crtc_vblank_off() and
> + * drm_crtc_vblank_on() functions. The diffrence comparet to

Typo: difference compared to

> + * drm_crtc_vblank_off() is that this function doesn't save the vblank
> counter
> + * and hence doesn't need to call any driver hooks.
> + */
> +void drm_crtc_vblank_reset(struct drm_crtc *drm_crtc)
> +{
> + struct drm_device *dev = drm_crtc->dev;
> + unsigned long irqflags;
> + int crtc = drm_crtc_index(drm_crtc);
> + struct drm_vblank_crtc *vblank = >vblank[crtc];
> +
> + spin_lock_irqsave(>vbl_lock, irqflags);
> + /*
> +  * Prevent subsequent drm_vblank_get() from enabling the vblank
> +  * interrupt by bumping the refcount.
> +  */
> + if (!vblank->inmodeset) {
> + atomic_inc(>refcount);
> + vblank->inmodeset = 1;
> + }
> + spin_unlock_irqrestore(>vbl_lock, irqflags);
> +
> + WARN_ON(!list_empty(>vblank_event_list));
> +}
> +EXPORT_SYMBOL(drm_crtc_vblank_reset);
> +
> +/**
>   * drm_vblank_on - enable vblank events on a CRTC
>   * @dev: DRM device
>   * @crtc: CRTC in question
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c index 423ef959264d..f8871a184747
> 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13296,9 +13296,9 @@ static void intel_sanitize_crtc(struct intel_crtc
> *crtc) /* restore vblank interrupts to correct state */
>   if (crtc->active) {
>   update_scanline_offset(crtc);
> - drm_vblank_on(dev, crtc->pipe);
> + drm_crtc_vblank_on(>base);
>   } else
> - drm_vblank_off(dev, crtc->pipe);
> + drm_crtc_vblank_reset(>base);
> 
>   /* We need to sanitize the plane -> pipe mapping first because this will
>* disable the crtc (and hence change the state) if it is wrong. 

[PATCH 03/14] drm/bridge: make bridge registration independent of drm flow

[Thierry Reding]

On Fri, Jan 30, 2015 at 10:37:19AM -0500, Rob Clark wrote:
> On Tue, Jan 20, 2015 at 11:38 AM, Ajay Kumar  
> wrote:
> > Currently, third party bridge drivers(ptn3460) are dependent
> > on the corresponding encoder driver init, since bridge driver
> > needs a drm_device pointer to finish drm initializations.
> > The encoder driver passes the drm_device pointer to the
> > bridge driver. Because of this dependency, third party drivers
> > like ptn3460 doesn't adhere to the driver model.
> >
> > In this patch, we reframe the bridge registration framework
> > so that bridge initialization is split into 2 steps, and
> > bridge registration happens independent of drm flow:
> > --Step 1: gather all the bridge settings independent of drm and
> >   add the bridge onto a global list of bridges.
> > --Step 2: when the encoder driver is probed, call drm_bridge_attach
> >   for the corresponding bridge so that the bridge receives
> >   drm_device pointer and continues with connector and other
> >   drm initializations.
> >
> > The old set of bridge helpers are removed, and a set of new helpers
> > are added to accomplish the 2 step initialization.
> >
> > The bridge devices register themselves onto global list of bridges
> > when they get probed by calling "drm_bridge_add".
> >
> > The parent encoder driver waits till the bridge is available
> > in the lookup table(by calling "of_drm_find_bridge") and then
> > continues with its initialization.
> >
> > The encoder driver should also call "drm_bridge_attach" to pass
> > on the drm_device to the bridge object.
> >
> > drm_bridge_attach inturn calls "bridge->funcs->attach" so that
> > bridge can continue with drm related initializations.
> 
> ok, so I probably should have had a closer look at this before it
> landed in drm-next, so if it is too late to revert (and deal w/
> untangling subsequent patches that depend on this) some of these
> issues we'll just have to fix with follow-on patches.
> 
> 1) needs headerdoc for new fxns in drm_bridge.c, and needs to be added
> in drm.tmpl

drm_panel.c is missing kerneldoc as well, though I have a local patch to
add it. If nobody else steps up I'll take this.

> 2) as far as I can tell, the new approach to cleaning up bridge
> objects is to just let them leak !?!

With this series bridges are no longer part of the DRM device and it's
the driver that provides them that needs to free them (in ->remove()).
It's not a completely perfect solution yet, but with the registry patch
that I proposed a while back all the remaining issues should go away.
Now if I could get anybody to look at that patch...

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

[PATCH v5 0/9] Enable HDMI support on Exynos platforms

[Kukjin Kim]

Marek Szyprowski wrote:
> 
> Hi all,
> 
> This is yet another update on patchset, which enables HDMI support for
> Exynos based platforms.
> 
> Beside DTS changes, this patchset adds parent domain support for Exynos
> PM domains and add support for 'hdmi' clock directly to Exynos DRM mixer
> driver.
> 
> The patchset is based on samsung/for-next branch and 'PM / Domains:
> Export of_genpd_get_from_provider function' patch merged as commit
> 7496fcbe8a643097efc061160e1c3b65ee2fa350 to v3.19-rc4.
> 
> Regards
> Marek Szyprowski
> 
> 
> Changelog:
> 
> v5:
> - fixed error value for clk_get() in mixer patch
> - rebased onto samsung/for-next branch
> 
> v4: (http://www.spinics.net/lists/linux-samsung-soc/msg41375.html)
> - added patches, which add 'hdmi' clock handling to mixed block, this
>   finally solves the initialization related issues, thanks for Tobias
>   Jakobi for testing
> - added acks/reviewed/tested by tags
> 
> v3: (http://www.spinics.net/lists/linux-samsung-soc/msg41020.html)
> - added a note on defining subdomains to generic PM domain binding
>   documentation (requested by Ulf Hansson)
> 
> v2: (http://www.spinics.net/lists/linux-samsung-soc/msg40980.html)
> - rewrote subdomains patch according to suggestions from Geert
>   Uytterhoeven and Amit Daniel Kachhap.
> 
> v1 resend: (http://www.spinics.net/lists/linux-samsung-soc/msg39428.html)
> - added handling of generic 'power-domains' binding in subdomains
> 
> v1: (http://www.spinics.net/lists/linux-samsung-soc/msg38914.html)
> - resolved power domain on/off issue with 'clk: samsung: exynos4: set
>   parent of mixer gate clock to hdmi' patch
> 
> v0: (http://www.spinics.net/lists/linux-samsung-soc/msg33498.html)
> - first attempt, used 'always on' power domains hack
> 
> 
> Patch summary:
> 
> 
> *** BLURB HERE ***
> 
> Andrzej Hajda (1):
>   ARM: dts: exynos5250: add display power domain
> 
> Marek Szyprowski (7):
>   PM / Domains: Add a note about power domain subdomains
>   ARM: Exynos: add support for sub-power domains
>   ARM: dts: exynos4: add hdmi related nodes
>   ARM: dts: exynos4: add dependency between TV and LCD0 power domains
>   ARM: dts: exynos4412-odroid: enable hdmi support
>   ARM: dts: Exynos: add 'hdmi' clock to mixer nodes
>   drm/exynos: add support for 'hdmi' clock
> 
> Tomasz Stanislawski (1):
>   ARM: dts: exynos4210-universal_c210: enable hdmi support
> 
>  .../bindings/arm/exynos/power_domain.txt   |  2 +
>  .../devicetree/bindings/power/power_domain.txt | 29 +++
>  .../devicetree/bindings/video/exynos_mixer.txt |  1 +
>  arch/arm/boot/dts/exynos4.dtsi | 41 
>  arch/arm/boot/dts/exynos4210-universal_c210.dts| 57 
> ++
>  arch/arm/boot/dts/exynos4210.dtsi  |  8 +++
>  arch/arm/boot/dts/exynos4412-odroid-common.dtsi| 44 +
>  arch/arm/boot/dts/exynos4x12.dtsi  | 11 +
>  arch/arm/boot/dts/exynos5250.dtsi  | 15 +-
>  arch/arm/boot/dts/exynos5420.dtsi  |  5 +-
>  arch/arm/mach-exynos/pm_domains.c  | 28 +++
>  drivers/gpu/drm/exynos/exynos_mixer.c  |  9 
>  12 files changed, 246 insertions(+), 4 deletions(-)
> 
> --
I have no objection on this series, but just wondering my tree should be fine
without 9/9 drm patch and it will be handled for 3.20?

I'll take 1 to 8 patches once drm patch is landed for 3.20.

- Kukjin

[PATCH 1/4] drm/irq: Add drm_crtc_vblank_reset

[Daniel Vetter]

On Tue, Feb 03, 2015 at 01:31:34PM +0200, Laurent Pinchart wrote:
> Hi Daniel,
> 
> Thank you for the patch.
> 
> On Tuesday 03 February 2015 11:30:11 Daniel Vetter wrote:
> > At driver load we need to tell the vblank code about the state of the
> > pipes, so that the logic around reject vblank_get when the pipe is off
> > works correctly.
> > 
> > Thus far i915 used drm_vblank_off, but one of the side-effects of it
> > is that it also saves the vblank counter. And for that it calls down
> > into the ->get_vblank_counter hook. Which isn't really a good idea
> > when the pipe is off for a few reasons:
> > - With runtime pm the register might not respond.
> > - If the pipe is off some datastructures might not be around or
> >   unitialized.
> > 
> > The later is what blew up on gen3: We look at intel_crtc->config to
> > compute the vblank counter, and for a disabled pipe at boot-up that's
> > just not there. Thus far this was papered over by a check for
> > intel_crtc->active, but I want to get rid of that (since it's fairly
> > race, vblank hooks are called from all kinds of places).
> > 
> > So prep for that by adding a _reset functions which only does what we
> > really need to be done at driver load: Mark the vblank pipe as off,
> > but don't do any vblank counter saving or event flushing - neither of
> > that is required.
> 
> (Thinking out loud)
> 
> In principle this looks good, but I find the naming pretty confusing. The 
> drm_crtc_vblank_* functions now have get, put, on, off and reset variants. 
> The 
> fact that on is supposed to be called both at runtime and an init time while 
> off is replaced by reset at init time breaks the symmetry between on and off. 
> What would you think of a drm_crtc_vblank_init() function instead, used at 
> init time only, and that would take an enabled boolean argument ?
> 
> On the other hand, calling drm_crtc_vblank_on() at init time also makes 
> sense, 
> as even if the CRTC is active the vblank interrupt should be off then, and an 
> explicit call to the function means "turn the vblank interrupt on". I'm thus 
> not totally opposed to keeping that as-is. Wouldn't it then be better to 
> modify the core to default to off, and let drivers call drm_crtc_vblank_on() 
> explicitly if the default isn't correct ? I think I like this solution 
> better, 
> and it could be conditioned by a driver flag if we don't want to audit all 
> drivers for possible breakages.

Yeah, that's been my thinking with sticking with on and only replacing the
off: For _on the environment exactly matches what we do when enabling the
crtc in a modeset:
- the pipe is actually running
- we want vblanks to start working

Totally different for _off, which assumes:
- pipe is still on
- vblanks should stop running and state should be saved
- any pending
Wheras _reset just disallows vblanks.

Long-term I wonder whether a drm_crtc_vblank_init would be useful, which:
- uses dev->mode_config.num_crtcs to set up the right amount of vblank
  pipes.
- calls _reset directly
- in the future might even store the vblank state in the drm_crtc.

But before we can do that we need to split the vblank code into ums legacy
paths using int pipe and kms paths dealing in struct drm_crtc *. We're not
there yet.

> 
> > Cc: Laurent Pinchart 
> > Signed-off-by: Daniel Vetter 
> > ---
> >  drivers/gpu/drm/drm_irq.c| 32 
> >  drivers/gpu/drm/i915/intel_display.c |  4 ++--
> >  include/drm/drmP.h   |  1 +
> >  3 files changed, 35 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> > index 75647e7f012b..1e5fb1b994d7 100644
> > --- a/drivers/gpu/drm/drm_irq.c
> > +++ b/drivers/gpu/drm/drm_irq.c
> > @@ -1226,6 +1226,38 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc)
> >  EXPORT_SYMBOL(drm_crtc_vblank_off);
> > 
> >  /**
> > + * drm_crtc_vblank_reset - reset vblank state to off on a CRTC
> > + * @crtc: CRTC in question
> > + *
> > + * Drivers can use this function to reset the vblank state to off at load
> > time.
> > + * Drivers should use this together with the drm_crtc_vblank_off() and
> > + * drm_crtc_vblank_on() functions. The diffrence comparet to
> 
> Typo: difference compared to
> 
> > + * drm_crtc_vblank_off() is that this function doesn't save the vblank
> > counter
> > + * and hence doesn't need to call any driver hooks.
> > + */
> > +void drm_crtc_vblank_reset(struct drm_crtc *drm_crtc)
> > +{
> > +   struct drm_device *dev = drm_crtc->dev;
> > +   unsigned long irqflags;
> > +   int crtc = drm_crtc_index(drm_crtc);
> > +   struct drm_vblank_crtc *vblank = >vblank[crtc];
> > +
> > +   spin_lock_irqsave(>vbl_lock, irqflags);
> > +   /*
> > +* Prevent subsequent drm_vblank_get() from enabling the vblank
> > +* interrupt by bumping the refcount.
> > +*/
> > +   if (!vblank->inmodeset) {
> > +   atomic_inc(>refcount);
> > +   vblank->inmodeset = 1;
> > +   }
> > +   

[PATCH] drm: Kconfig: Let all DRM_GEM_CMA_HELPER related macros depend on HAVE_DMA_ATTRS

[Laurent Pinchart]

Hi Chen,

Thank you for the patch.

On Sunday 01 February 2015 22:08:33 Chen Gang S wrote:
> DRM_GEM_CMA_HELPER is depend on HAVE_DMA_ATTRS, or it will break the
> building. The related error (with allmodconfig under xtensa):
> 
> CC [M]  drivers/gpu/drm/drm_gem_cma_helper.o
>   drivers/gpu/drm/drm_gem_cma_helper.c: In function 'drm_gem_cma_create':
>   drivers/gpu/drm/drm_gem_cma_helper.c:110:19: error: implicit declaration
> of function 'dma_alloc_writecombine'
> [-Werror=implicit-function-declaration] cma_obj->vaddr =
> dma_alloc_writecombine(drm->dev, size,
>  ^
>   drivers/gpu/drm/drm_gem_cma_helper.c:110:17: warning: assignment makes
> pointer from integer without a cast [-Wint-conversion] cma_obj->vaddr =
> dma_alloc_writecombine(drm->dev, size,
>^
>   drivers/gpu/drm/drm_gem_cma_helper.c: In function
> 'drm_gem_cma_free_object': drivers/gpu/drm/drm_gem_cma_helper.c:193:3:
> error: implicit declaration of function 'dma_free_writecombine'
> [-Werror=implicit-function-declaration]
> dma_free_writecombine(gem_obj->dev->dev, cma_obj->base.size,
>  ^
>   drivers/gpu/drm/drm_gem_cma_helper.c: In function 'drm_gem_cma_mmap_obj':
>   drivers/gpu/drm/drm_gem_cma_helper.c:330:8: error: implicit declaration of
> function 'dma_mmap_writecombine' [-Werror=implicit-function-declaration]
> ret = dma_mmap_writecombine(cma_obj->base.dev->dev, vma,
>   ^
> 
> Signed-off-by: Chen Gang 
> ---
>  drivers/gpu/drm/Kconfig | 3 ++-
>  drivers/gpu/drm/atmel-hlcdc/Kconfig | 2 +-
>  drivers/gpu/drm/imx/Kconfig | 2 +-
>  drivers/gpu/drm/rcar-du/Kconfig | 2 +-
>  drivers/gpu/drm/shmobile/Kconfig| 2 +-
>  drivers/gpu/drm/sti/Kconfig | 2 +-
>  drivers/gpu/drm/tilcdc/Kconfig  | 2 +-
>  7 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 308c104..151a050 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -62,12 +62,13 @@ config DRM_TTM
> 
>  config DRM_GEM_CMA_HELPER
>   bool
> - depends on DRM
> + depends on DRM && HAVE_DMA_ATTRS
>   help
> Choose this if you need the GEM CMA helper functions
> 
>  config DRM_KMS_CMA_HELPER
>   bool
> + depends on DRM && HAVE_DMA_ATTRS
>   select DRM_GEM_CMA_HELPER
>   select DRM_KMS_FB_HELPER
>   select FB_SYS_FILLRECT
> diff --git a/drivers/gpu/drm/atmel-hlcdc/Kconfig
> b/drivers/gpu/drm/atmel-hlcdc/Kconfig index 1a08562..219fc87 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/Kconfig
> +++ b/drivers/gpu/drm/atmel-hlcdc/Kconfig
> @@ -1,6 +1,6 @@
>  config DRM_ATMEL_HLCDC
>   tristate "DRM Support for ATMEL HLCDC Display Controller"
> - depends on DRM && OF && COMMON_CLK && MFD_ATMEL_HLCDC
> + depends on DRM && OF && COMMON_CLK && MFD_ATMEL_HLCDC && HAVE_DMA_ATTRS

I don't like having to add the explicit dependency to all users of 
DRM_GEM_CMA_HELPER and DRM_KMS_CMA_HELPER, but there's no way around this 
given the select vs. depends issues in Kconfig.

A better solution in the longer term would be to implement HAVE_DMA_ATTRS 
support for xtensa (and all the other architectures that miss it), but this 
patch looks fine to me as an interim solution.

Acked-by: Laurent Pinchart 

>   select DRM_GEM_CMA_HELPER
>   select DRM_KMS_HELPER
>   select DRM_KMS_FB_HELPER
> diff --git a/drivers/gpu/drm/imx/Kconfig b/drivers/gpu/drm/imx/Kconfig
> index 5d5e4092..33cdddf 100644
> --- a/drivers/gpu/drm/imx/Kconfig
> +++ b/drivers/gpu/drm/imx/Kconfig
> @@ -5,7 +5,7 @@ config DRM_IMX
>   select VIDEOMODE_HELPERS
>   select DRM_GEM_CMA_HELPER
>   select DRM_KMS_CMA_HELPER
> - depends on DRM && (ARCH_MXC || ARCH_MULTIPLATFORM)
> + depends on DRM && (ARCH_MXC || ARCH_MULTIPLATFORM) && HAVE_DMA_ATTRS
>   depends on IMX_IPUV3_CORE
>   help
> enable i.MX graphics support
> diff --git a/drivers/gpu/drm/rcar-du/Kconfig
> b/drivers/gpu/drm/rcar-du/Kconfig index 2324a52..11485a4 100644
> --- a/drivers/gpu/drm/rcar-du/Kconfig
> +++ b/drivers/gpu/drm/rcar-du/Kconfig
> @@ -1,6 +1,6 @@
>  config DRM_RCAR_DU
>   tristate "DRM Support for R-Car Display Unit"
> - depends on DRM && ARM
> + depends on DRM && ARM && HAVE_DMA_ATTRS
>   depends on ARCH_SHMOBILE || COMPILE_TEST
>   select DRM_KMS_HELPER
>   select DRM_KMS_CMA_HELPER
> diff --git a/drivers/gpu/drm/shmobile/Kconfig
> b/drivers/gpu/drm/shmobile/Kconfig index a50fe0e..7321c41 100644
> --- a/drivers/gpu/drm/shmobile/Kconfig
> +++ b/drivers/gpu/drm/shmobile/Kconfig
> @@ -1,6 +1,6 @@
>  config DRM_SHMOBILE
>   tristate "DRM Support for SH Mobile"
> - depends on DRM && ARM
> + depends on DRM && ARM && HAVE_DMA_ATTRS
>   depends on ARCH_SHMOBILE || COMPILE_TEST
>   select BACKLIGHT_CLASS_DEVICE
>   select DRM_KMS_HELPER
> diff --git a/drivers/gpu/drm/sti/Kconfig b/drivers/gpu/drm/sti/Kconfig
> index d6d6b70..1fdb74b 100644
> ---