[PATCH v3 1/2] dt-bindings: drm/bridge: adv7511: Add regulator bindings

[Laurent Pinchart]

Hi Bjorn,

On Monday 05 Dec 2016 13:11:51 Bjorn Andersson wrote:
> On Tue 29 Nov 01:11 PST 2016, Laurent Pinchart wrote:
> > On Tuesday 29 Nov 2016 13:41:33 Archit Taneja wrote:
> >> On 11/29/2016 12:03 PM, Laurent Pinchart wrote:
> >>> On Tuesday 29 Nov 2016 11:37:41 Archit Taneja wrote:
>  Add the regulator supply properties needed by ADV7511 and ADV7533.
>  
>  The regulators are specified as optional properties since there can
>  be boards which have a fixed supply directly routed to the pins, and
>  these may not be modelled as regulator supplies.
> >>> 
> >>> That's why we have support for dummy supplies in the kernel, isn't it
> >>> ? Isn't it better to make the supplies mandatory in the bindings (and
> >>> obviously handling them as optional in the driver for
> >>> backward-compatibility) ?
> >> 
> >> I'm a bit unclear on this.
> >> 
> >> I thought we couldn't add mandatory properties once the device is
> >> already present in DT for one or more platforms.
> > 
> > You can, as long as you treat them as optional in the driver to retain
> > backward compatibility. The DT bindings should document the properties
> > expected from a new platform (older versions of the bindings will always
> > be available in the git history).
> 
> If you document them as required and don't do anything special in the
> implementation (i.e. just call devm_regulator_get() as usual) it will
> just work, in the absence of the property you will get a dummy regulator
> from the framework.
> 
> And then add the fixed-voltage regulators to the new DT to make that
> properly describe the hardware.
> 
> >> Say, if we do make it mandatory for future additions, we would need to
> >> have DT property for the supplies for the new platforms. If the
> >> regulators on these boards are fixed supplies, they would be need to be
> >> modeled using "regulator-fixed", possibly without any input supply. Is
> >> that what you're suggesting?
> > 
> > That's the idea, yes. Clock maintainers have a similar opinion regarding
> > the clock bindings, where a clock that is not optional at the hardware
> > level should be specified in DT even if it's always present.
> 
> Further more, a DT binding for a particular block should describe that
> block; so if we have three different 1.8V pins then the DT binding
> should reflect this - even if our current platform have them wired to
> the same regulator.

This has been discussed previously, and Rob agreed that if the datasheet 
recommends to power all supplies from the same regulator we can take that as a 
good hint that a single supply should be enough. In the very unlikely event 
that a board would require control of more regulators we can always extend the 
DT bindings later without breaking backward compatibility.

> (And the supply names would preferably be based on the pin names in the
> component data sheet)

-- 
Regards,

Laurent Pinchart

[Powerpc] Sam460ex Canyonlands issue -Kernel 4.4.36

[Deucher, Alexander]

> -Original Message-
> From: Julian Margetson [mailto:runaway at candw.ms]
> Sent: Monday, December 05, 2016 4:59 PM
> To: Maling list - DRI developers
> Cc: Deucher, Alexander; Daenzer, Michel; daniel.vetter at ffwll.ch
> Subject: [Powerpc] Sam460ex Canyonlands issue -Kernel 4.4.36
> 
> Following  appears with kernel 4.4.36 .
> 
> Maybe as a result of following patch ?
> 
> drm/radeon: Ensure vblank interrupt is enabled on DPMS transition to on

Possibly.  Your platform appears to have problems in general:

[4.733843] [drm:r600_ring_test] *ERROR* radeon: ring 0 test failed 
(scratch(0x850C)=0xCAFEDEAD)
[4.742686] radeon 0001:81:00.0: disabling GPU acceleration

When that happens the driver tears down a bunch of stuff including interrupts 
and basically falls back to dumb framebuffer mode.  The warning is harmless in 
this case.  It's basically telling you that the driver refused to enable any 
interrupts because the interrupt handler is not registered because a bunch of 
stuff failed to initialize.

Alex

> 
> 
> [5.331666] Can't enable IRQ/MSI because no handler is installed
> [5.331842] [ cut here ]
> [5.331847] WARNING: at c048a53c [verbose debug info unavailable]
> [5.331851] Modules linked in:
> [5.331859] CPU: 0 PID: 1 Comm: swapper Not tainted 4.4.36-sam460ex-jm
> #1
> [5.331864] task: ea848000 ti: ea84c000 task.ti: ea84c000
> [5.331867] NIP: c048a53c LR: c048a53c CTR: 
> [5.331870] REGS: ea84d6e0 TRAP: 0700   Not tainted (4.4.36-sam460ex-jm)
> [5.331879] MSR: 00029000   CR: 24008222  XER: 
> [5.331914]
> [5.331914] GPR00: c048a53c ea84d790 ea848000 0034 
>  c0ea7a2c 
> [5.331914] GPR08: 00029000 0800 ea84c000 ea84d790 24008222
>  eaa8096c 0001
> [5.331914] GPR16: eaacd9d4 c08e6c20   c0c678d7
>  c0da ea982048
> [5.331914] GPR24: c08f24c0 ea982108 eaa80984 ea9d4d80 eaa80800
> eaa80800 eabdc000 ea982000
> [5.331932] NIP [c048a53c] si_irq_set+0x28/0x9e4
> [5.331937] LR [c048a53c] si_irq_set+0x28/0x9e4
> [5.331938] Call Trace:
> [5.331945] [ea84d790] [c048a53c] si_irq_set+0x28/0x9e4 (unreliable)
> [5.331958] [ea84d7f0] [c0409b20] atombios_crtc_dpms+0x8c/0x104
> [5.331964] [ea84d810] [c0409bb4] atombios_crtc_commit+0x1c/0x38
> [5.331972] [ea84d820] [c03c9d90]
> drm_crtc_helper_set_mode+0x36c/0x410
> [5.331978] [ea84d9f0] [c03ca628]
> drm_crtc_helper_set_config+0x6dc/0x934
> [5.331991] [ea84da60] [c0423a70] radeon_crtc_set_config+0x50/0xf8
> [5.332007] [ea84da80] [c03e47b0]
> drm_mode_set_config_internal+0x5c/0xe0
> [5.332021] [ea84daa0] [c03d4704] restore_fbdev_mode+0x20c/0x25c
> [5.332030] [ea84dad0] [c03d61ec]
> drm_fb_helper_restore_fbdev_mode_unlocked+0x40/0x8c
> [5.332036] [ea84daf0] [c03d6284] drm_fb_helper_set_par+0x4c/0x60
> [5.332047] [ea84db00] [c0388144] fbcon_init+0x318/0x410
> [5.332056] [ea84db50] [c03b3024] visual_init+0xac/0xf8
> [5.332063] [ea84db60] [c03b487c] do_bind_con_driver+0x1e8/0x2d8
> [5.332070] [ea84dbb0] [c03b4cd0] do_take_over_console+0x19c/0x1ac
> [5.332075] [ea84dbe0] [c03878bc] do_fbcon_takeover+0x80/0xe4
> [5.332087] [ea84dbf0] [c003c3fc] notifier_call_chain+0x60/0x94
> [5.332094] [ea84dc10] [c003c71c]
> __blocking_notifier_call_chain+0x50/0x68
> [5.332104] [ea84dc30] [c038f3a8] register_framebuffer+0x248/0x26c
> [5.332111] [ea84dc90] [c03d65b4]
> drm_fb_helper_initial_config+0x31c/0x39c
> [5.332120] [ea84dcd0] [c042bc78] radeon_fbdev_init+0xcc/0xf8
> [5.332126] [ea84dcf0] [c0425dd0] radeon_modeset_init+0x5a8/0x938
> [5.332132] [ea84dd50] [c040316c] radeon_driver_load_kms+0xcc/0x184
> [5.332138] [ea84dd70] [c03df0b8] drm_dev_register+0x84/0xc8
> [5.332144] [ea84dd90] [c03e0e78] drm_get_pci_dev+0xf4/0x18c
> [5.332156] [ea84ddb0] [c03719e4] pci_device_probe+0x88/0xe8
> [5.332164] [ea84ddd0] [c05436f4] driver_probe_device+0x130/0x290
> [5.332170] [ea84de00] [c05438cc] __driver_attach+0x78/0xa0
> [5.332188] [ea84de20] [c0541bec] bus_for_each_dev+0x90/0xa0
> [5.332193] [ea84de50] [c0542dc8] bus_add_driver+0xe0/0x1f0
> [5.332199] [ea84de70] [c054400c] driver_register+0xb4/0xf8
> [5.332207] [ea84de90] [c00016f8] do_one_initcall+0x11c/0x1a0
> [5.332214] [ea84df00] [c0d08acc] kernel_init_freeable+0x130/0x1cc
> [5.332220] [ea84df30] [c0001cf4] kernel_init+0x14/0xf8
> [5.332228] [ea84df40] [c000ade4] ret_from_kernel_thread+0x5c/0x64
> [5.332230] Instruction dump:
> [5.332241] 7c0803a6 4bb7c1ec 9421ffa0 7c0802a6 bdc10018 90010064
> 894313d0 2f8a
> [5.332251] 40be001c 3c60c0c7 38631c16 483ea6c5 <0fe0> 3860ffea
> 480009ac 89431aa8
> [5.332274] ---[ end trace 3fbb08226210324a ]---
> [5.413526] Console: switching to colour frame buffer device 240x67

[Bug 98964] Chromium complains about glXGetSyncValuesOML in 13.0.2

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

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

--- Comment #8 from Emil Velikov  ---
(In reply to Zoltán Böszörményi from comment #6)
> When did Chromium started to use the vertical sync?
> 
Don't any of us know this one. As Matt mentioned your help in tracking the
issue down will be appreciated.

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

[Bug 98964] Chromium complains about glXGetSyncValuesOML in 13.0.2

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

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

--- Comment #7 from Emil Velikov  ---
Zoltán - the "g" in "i915g" implies the driver type rather than the actual
filename.

Humble (off topic) request: please look into the the Yocto recipe to create
hard links for the files as opposed to symlinks. The latter can cause issues in
many scenarios. Thanks

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

[PATCH 0/8] Host1x IOMMU support + VIC support

[Mikko Perttunen]

On 12/05/2016 09:51 PM, Thierry Reding wrote:
> On Thu, Nov 10, 2016 at 08:23:37PM +0200, Mikko Perttunen wrote:
>> This series adds IOMMU support to Host1x and TegraDRM
>> and adds support for the VIC host1x client so that
>> host1x can be tested on modern Tegra platforms.
>> It depends on the previous fix series. The whole thing
>> (modulo patch order) is available as a git repository at
>> git://github.com/cyndis/linux.git; branch vic-v1.
>>
>> IO memory management is organized such that there are
>> two domains: the host1x domain and the tegradrm domain.
>> The host1x domain is used by the host1x engine and
>> contains the host1x CDMA and pushbuffers for submitted
>> jobs.
>>
>> The tegradrm domain is shared by all host1x units and
>> contains GEM objects and memory allocated by the
>> separate tegra_drm_alloc function. This function is
>> currently used to allocate space for firmware blobs
>> in the tegradrm domain.
>>
>> A userspace test case for VIC can be found at
>> https://github.com/cyndis/drm/tree/work/tegra.
>> The testcase is in tests/tegra and is called submit_vic.
>> The in-kernel firewall is not implemented for VIC;
>> therefore, IOMMU must be enabled for the test to pass.
>>
>> Tested with Jetson TX1 (T210). Probably works also
>> with Jetson TK1 (T124). Note that due to hardware changes
>> the testcase also needs to be changed to run properly
>> on T124.
>
> What's the scope of the changes required for Tegra124? If we add the
> kernel bits for Tegra124 we should also have a userspace test program to
> exercise it.

There should be no change needed to the kernel driver for T124. For the 
userspace test, removing the one or two topmost commits in my repo will 
put the testcase to a form working on T124 - I was lazy and just 
modified the original file. Should be reasonably easy to have the test 
case support both chips, though.

>
> Thanks,
> Thierry
>

Cheers,
Mikko.

[PATCH] drm/amdgpu: don't add files at control minor debugfs directory

[Nicolai Stange]

Since commit 8a357d10043c ("drm: Nerf DRM_CONTROL nodes"), a
struct drm_device's ->control member is always NULL.

In the case of CONFIG_DEBUG_FS=y, amdgpu_debugfs_add_files() accesses
->control->debugfs_root though. This results in a NULL pointer
dereference.

Fix this by omitting the drm_debugfs_create_files() call for the
control minor debugfs directory which is now non-existent anyway.

Fixes: 8a357d10043c ("drm: Nerf DRM_CONTROL nodes")
Signed-off-by: Nicolai Stange 
---
Applicable to next-20161202. Compile-only tested.

 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index deee2db..0cb3e82 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2493,9 +2493,6 @@ int amdgpu_debugfs_add_files(struct amdgpu_device *adev,
adev->debugfs_count = i;
 #if defined(CONFIG_DEBUG_FS)
drm_debugfs_create_files(files, nfiles,
-adev->ddev->control->debugfs_root,
-adev->ddev->control);
-   drm_debugfs_create_files(files, nfiles,
 adev->ddev->primary->debugfs_root,
 adev->ddev->primary);
 #endif
@@ -2510,9 +2507,6 @@ static void amdgpu_debugfs_remove_files(struct 
amdgpu_device *adev)
for (i = 0; i < adev->debugfs_count; i++) {
drm_debugfs_remove_files(adev->debugfs[i].files,
 adev->debugfs[i].num_files,
-adev->ddev->control);
-   drm_debugfs_remove_files(adev->debugfs[i].files,
-adev->debugfs[i].num_files,
 adev->ddev->primary);
}
 #endif
-- 
2.10.2

[PATCH v10 13/13] drm/mediatek: add support for Mediatek SoC MT2701

[YT Shen]

On Wed, 2016-11-30 at 15:03 +0100, Matthias Brugger wrote:
> 
> On 25/11/16 11:34, YT Shen wrote:
> 
> >  static const struct of_device_id mtk_disp_rdma_driver_dt_match[] = {
> > +   { .compatible = "mediatek,mt2701-disp-rdma",
> > + .data = _rdma_driver_data},
> > { .compatible = "mediatek,mt8173-disp-rdma",
> >   .data = _rdma_driver_data},
> > {},
> 
> [...]
> 
> >  static const struct of_device_id ddp_driver_dt_match[] = {
> > +   { .compatible = "mediatek,mt2701-disp-mutex", .data = mt2701_mutex_mod},
> > { .compatible = "mediatek,mt8173-disp-mutex", .data = mt8173_mutex_mod},
> > {},
> >  };
> 
> [...]
> 
> >
> >  static const struct of_device_id mtk_disp_color_driver_dt_match[] = {
> > +   { .compatible = "mediatek,mt2701-disp-color",
> > + .data = _color_driver_data},
> > { .compatible = "mediatek,mt8173-disp-color",
> >   .data = _color_driver_data},
> > {},
> 
> [...]
> 
> >  static const struct of_device_id mtk_drm_of_ids[] = {
> > +   { .compatible = "mediatek,mt2701-mmsys",
> > + .data = _mmsys_driver_data},
> > { .compatible = "mediatek,mt8173-mmsys",
> >   .data = _mmsys_driver_data},
> > { }
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c 
> > b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > index 0569f2e..f63cc91 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > @@ -1203,6 +1203,7 @@ static int mtk_dsi_remove(struct platform_device 
> > *pdev)
> >  }
> >
> >  static const struct of_device_id mtk_dsi_of_match[] = {
> > +   { .compatible = "mediatek,mt2701-dsi" },
> > { .compatible = "mediatek,mt8173-dsi" },
> > { },
> >  };
> 
> [...]
> 
> >
> >  static const struct of_device_id mtk_mipi_tx_match[] = {
> > +   { .compatible = "mediatek,mt2701-mipi-tx",
> > + .data = _mipitx_data },
> > { .compatible = "mediatek,mt8173-mipi-tx",
> >   .data = _mipitx_data },
> > {},
> 
> I'm not sure if I missed some, but you should update the binding 
> description for newly added bindings.
> 
> Thanks a lot,
> Matthias
Oh, you are right.  I thought there is already sent binding description
but actually no.  I will update this part for newly added bindings.

Thanks for informing.
yt.shen

[PATCH v2] drm/i915/dsi: Do not clear DPOUNIT_CLOCK_GATE_DISABLE from vlv_init_display_clock_gating

[Ville Syrjälä]

On Fri, Dec 02, 2016 at 03:29:04PM +0100, Hans de Goede wrote:
> On my Cherrytrail CUBE iwork8 Air tablet PIPE-A would get stuck on loading
> i915 at boot 1 out of every 3 boots, resulting in a non functional LCD.
> Once the i915 driver has successfully loaded, the panel can be disabled /
> enabled without hitting this issue.
> 
> The getting stuck is caused by vlv_init_display_clock_gating() clearing
> the DPOUNIT_CLOCK_GATE_DISABLE bit in DSPCLK_GATE_D when called from
> chv_pipe_power_well_ops.enable() on driver load, while a pipe is enabled
> driving the DSI LCD by the BIOS.
> 
> Clearing this bit while DSI is in use is a known issue and
> intel_dsi_pre_enable() / intel_dsi_post_disable() already set / clear it
> as appropriate.
> 
> This commit modifies vlv_init_display_clock_gating() to leave the
> DPOUNIT_CLOCK_GATE_DISABLE bit alone fixing the pipe getting stuck.
> 
> BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=97330
> Cc: stable at vger.kernel.org
> Signed-off-by: Hans de Goede 
> Reviewed-by: Ville Syrjälä 
> ---
> Changes in v2:
> -Replace PIPE-A with "a pipe" or "the pipe" in the commit msg and comment

Pushed to dinq with s/BugLink/Bugzilla/ and changelog moved into the
commit message proper. Thanks for the patch.

> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 356c662..87b4af0 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -1039,7 +1039,18 @@ static bool vlv_power_well_enabled(struct 
> drm_i915_private *dev_priv,
>  
>  static void vlv_init_display_clock_gating(struct drm_i915_private *dev_priv)
>  {
> - I915_WRITE(DSPCLK_GATE_D, VRHUNIT_CLOCK_GATE_DISABLE);
> + u32 val;
> +
> + /*
> +  * On driver load, a pipe may be active and driving a DSI display.
> +  * Preserve DPOUNIT_CLOCK_GATE_DISABLE to avoid the pipe getting stuck
> +  * (and never recovering) in this case. intel_dsi_post_disable() will
> +  * clear it when we turn off the display.
> +  */
> + val = I915_READ(DSPCLK_GATE_D);
> + val &= DPOUNIT_CLOCK_GATE_DISABLE;
> + val |= VRHUNIT_CLOCK_GATE_DISABLE;
> + I915_WRITE(DSPCLK_GATE_D, val);
>  
>   /*
>* Disable trickle feed and enable pnd deadline calculation
> -- 
> 2.9.3

-- 
Ville Syrjälä
Intel OTC

[PATCH 0/8] Host1x IOMMU support + VIC support

[Thierry Reding]

On Thu, Nov 10, 2016 at 08:23:37PM +0200, Mikko Perttunen wrote:
> This series adds IOMMU support to Host1x and TegraDRM
> and adds support for the VIC host1x client so that
> host1x can be tested on modern Tegra platforms.
> It depends on the previous fix series. The whole thing
> (modulo patch order) is available as a git repository at
> git://github.com/cyndis/linux.git; branch vic-v1.
> 
> IO memory management is organized such that there are
> two domains: the host1x domain and the tegradrm domain.
> The host1x domain is used by the host1x engine and
> contains the host1x CDMA and pushbuffers for submitted
> jobs.
> 
> The tegradrm domain is shared by all host1x units and
> contains GEM objects and memory allocated by the
> separate tegra_drm_alloc function. This function is
> currently used to allocate space for firmware blobs
> in the tegradrm domain.
> 
> A userspace test case for VIC can be found at
> https://github.com/cyndis/drm/tree/work/tegra.
> The testcase is in tests/tegra and is called submit_vic.
> The in-kernel firewall is not implemented for VIC;
> therefore, IOMMU must be enabled for the test to pass.
> 
> Tested with Jetson TX1 (T210). Probably works also
> with Jetson TK1 (T124). Note that due to hardware changes
> the testcase also needs to be changed to run properly
> on T124.

What's the scope of the changes required for Tegra124? If we add the
kernel bits for Tegra124 we should also have a userspace test program to
exercise it.

Thanks,
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161205/aef4b768/attachment.sig>

[PATCH v10 02/13] drm/mediatek: add *driver_data for different hardware settings

[YT Shen]

Hi Daniel,

On Wed, 2016-11-30 at 14:42 +0800, Daniel Kurtz wrote:
> Hi YT,
> 
> 
> On Fri, Nov 25, 2016 at 6:34 PM, YT Shen  wrote:
> >
> > There are some hardware settings changed, between MT8173 & MT2701:
> > DISP_OVL address offset changed, color format definition changed.
> > DISP_RDMA fifo size changed.
> > DISP_COLOR offset changed.
> > MIPI_TX pll setting changed.
> > And add prefix for mtk_ddp_main & mtk_ddp_ext & mutex_mod.
> 
> Sorry, I have not had time to thoroughly review the new patch set, but
> one quick comment:
> 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h 
> > b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > index 22a33ee..cfaa760 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > @@ -78,6 +78,10 @@ struct mtk_ddp_comp_funcs {
> >   struct drm_crtc_state *state);
> >  };
> >
> > +struct mtk_ddp_comp_driver_data {
> > +   unsigned int color_offset;
> > +};
> > +
> >  struct mtk_ddp_comp {
> > struct clk *clk;
> > void __iomem *regs;
> > @@ -85,6 +89,7 @@ struct mtk_ddp_comp {
> > struct device *larb_dev;
> > enum mtk_ddp_comp_id id;
> > const struct mtk_ddp_comp_funcs *funcs;
> > +   const struct mtk_ddp_comp_driver_data *data;
> 
> I want to avoid adding this generic pointer here to all mtk_ddp_comp,
> since this is only used by the color component.
> Instead, define color specific types:
> 
> struct mtk_disp_color_data {
>unsigned int offset;
> };
> 
> struct mtk_disp_color {
>struct mtk_ddp_comp ddp_comp;
>const struct mtk_disp_color_data *data;
> };
> 
> Then, add another comp_type check and fetch the dev data in
> mtk_drm_probe()  or maybe mtk_ddp_comp_init().
> 
> -Dan
OK, we will remove the color data pointer from generic mtk_ddp_comp.
Thanks.

Regards,
yt.shen

[PATCH 5/8] gpu: host1x: Add IOMMU support

[Thierry Reding]

 /* set base, put and end pointer */
> - host1x_ch_writel(ch, cdma->push_buffer.phys, HOST1X_CHANNEL_DMASTART);
> + host1x_ch_writel(ch, cdma->push_buffer.dma, HOST1X_CHANNEL_DMASTART);
>   host1x_ch_writel(ch, cdma->push_buffer.pos, HOST1X_CHANNEL_DMAPUT);
> - host1x_ch_writel(ch, cdma->push_buffer.phys +
> + host1x_ch_writel(ch, cdma->push_buffer.dma +
>cdma->push_buffer.size_bytes + 4,
>HOST1X_CHANNEL_DMAEND);
>  
> @@ -115,8 +115,8 @@ static void cdma_timeout_restart(struct host1x_cdma 
> *cdma, u32 getptr)
>HOST1X_CHANNEL_DMACTRL);
>  
>   /* set base, end pointer (all of memory) */
> - host1x_ch_writel(ch, cdma->push_buffer.phys, HOST1X_CHANNEL_DMASTART);
> - host1x_ch_writel(ch, cdma->push_buffer.phys +
> + host1x_ch_writel(ch, cdma->push_buffer.dma, HOST1X_CHANNEL_DMASTART);
> + host1x_ch_writel(ch, cdma->push_buffer.dma +
>cdma->push_buffer.size_bytes,
>HOST1X_CHANNEL_DMAEND);
>  
> diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c
> index a91b7c4..222d930 100644
> --- a/drivers/gpu/host1x/job.c
> +++ b/drivers/gpu/host1x/job.c
> @@ -174,8 +174,9 @@ static int do_waitchks(struct host1x_job *job, struct 
> host1x *host,
>   return 0;
>  }
>  
> -static unsigned int pin_job(struct host1x_job *job)
> +static unsigned int pin_job(struct host1x *host, struct host1x_job *job)
>  {
> + int err;
>   unsigned int i;

Maybe use an inverse christmas tree for variables as in other parts of
this driver? That is, add the new int err belowe the existing unsined
int i.

>  
>   job->num_unpins = 0;
> @@ -186,12 +187,16 @@ static unsigned int pin_job(struct host1x_job *job)
>   dma_addr_t phys_addr;
>  
>   reloc->target.bo = host1x_bo_get(reloc->target.bo);
> - if (!reloc->target.bo)
> + if (!reloc->target.bo) {
> + err = -EINVAL;
>   goto unpin;
> + }
>  
>   phys_addr = host1x_bo_pin(reloc->target.bo, );
> - if (!phys_addr)
> + if (!phys_addr) {
> + err = -EINVAL;
>   goto unpin;
> + }
>  
>   job->addr_phys[job->num_unpins] = phys_addr;
>   job->unpins[job->num_unpins].bo = reloc->target.bo;
> @@ -204,25 +209,68 @@ static unsigned int pin_job(struct host1x_job *job)
>   struct sg_table *sgt;
>   dma_addr_t phys_addr;
>  
> + size_t gather_size = 0;
> + struct scatterlist *sg;
> + struct iova *alloc;
> + unsigned long shift;
> + int j;

There should be no blank line between blocks of variable declarations.
Also, make j an unsigned int.

> +
>   g->bo = host1x_bo_get(g->bo);
> - if (!g->bo)
> + if (!g->bo) {
> + err = -EINVAL;
>   goto unpin;
> + }
>  
>   phys_addr = host1x_bo_pin(g->bo, );
> - if (!phys_addr)
> + if (!phys_addr) {
> + err = -EINVAL;
>   goto unpin;
> + }
> +
> + if (!IS_ENABLED(CONFIG_TEGRA_HOST1X_FIREWALL) && host->domain) {
> + for_each_sg(sgt->sgl, sg, sgt->nents, j) {
> + gather_size += sg->length;
> + }

No need for curly brackets here.

> + gather_size = iova_align(>iova, gather_size);
> +
> + shift = iova_shift(>iova);
> + alloc = alloc_iova(
> + >iova, gather_size >> shift,
> + host->domain->geometry.aperture_end >> shift,
> + true);

This could be made a little more compact as well.

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161205/59910d4a/attachment.sig>

[PATCH v10 12/13] drm/mediatek: update DSI sub driver flow for sending commands to panel

[YT Shen]

Hi CK,

On Wed, 2016-11-30 at 15:58 +0800, CK Hu wrote:
> Hi, YT:
> 
> some comments inline.
> 
> On Fri, 2016-11-25 at 18:34 +0800, YT Shen wrote:
> > This patch update enable/disable flow of DSI module.
> > Original flow works on there is a bridge chip: DSI -> bridge -> panel.
> > In this case: DSI -> panel, the DSI sub driver flow should be updated.
> > We need to initialize DSI first so that we can send commands to panel.
> > 
> > Signed-off-by: shaoming chen 
> > Signed-off-by: YT Shen 
> > ---
> >  drivers/gpu/drm/mediatek/mtk_dsi.c | 101 
> > +
> >  1 file changed, 80 insertions(+), 21 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c 
> > b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > index ded4202..0569f2e 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > @@ -126,6 +126,10 @@
> >  #define CLK_HS_POST(0xff << 8)
> >  #define CLK_HS_EXIT(0xff << 16)
> >  
> > +#define DSI_VM_CMD_CON 0x130
> > +#define VM_CMD_EN  BIT(0)
> > +#define TS_VFP_EN  BIT(5)
> > +
> >  #define DSI_CMDQ0  0x180
> >  #define CONFIG (0xff << 0)
> >  #define SHORT_PACKET   0
> > @@ -249,7 +253,9 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
> >  * mipi_ratio is mipi clk coefficient for balance the pixel clk in mipi.
> >  * we set mipi_ratio is 1.05.
> >  */
> > -   dsi->data_rate = dsi->vm.pixelclock * 3 * 21 / (1 * 1000 * 10);
> > +   dsi->data_rate = dsi->vm.pixelclock * 12 * 21;
> > +   dsi->data_rate /= (dsi->lanes * 1000 * 10);
> 
> This looks like a bug fix that use lanes to calculate data rate.
This part add support when dsi->lanes != 4.

> 
> > +   DRM_DEBUG_DRIVER("set mipitx's data rate: %dMHz\n", dsi->data_rate);
> >  
> > ret = clk_set_rate(dsi->hs_clk, dsi->data_rate * 100);
> > if (ret < 0) {
> > @@ -333,16 +339,23 @@ static void mtk_dsi_set_mode(struct mtk_dsi *dsi)
> > u32 vid_mode = CMD_MODE;
> >  
> > if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO) {
> > -   vid_mode = SYNC_PULSE_MODE;
> > -
> > -   if ((dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) &&
> > -   !(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE))
> > +   if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST)
> > vid_mode = BURST_MODE;
> > +   else if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)
> > +   vid_mode = SYNC_PULSE_MODE;
> > +   else
> > +   vid_mode = SYNC_EVENT_MODE;
> > }
> >  
> > writel(vid_mode, dsi->regs + DSI_MODE_CTRL);
> >  }
> >  
> > +static void mtk_dsi_set_vm_cmd(struct mtk_dsi *dsi)
> > +{
> > +   mtk_dsi_mask(dsi, DSI_VM_CMD_CON, VM_CMD_EN, VM_CMD_EN);
> > +   mtk_dsi_mask(dsi, DSI_VM_CMD_CON, TS_VFP_EN, TS_VFP_EN);
> > +}
> > +
> >  static void mtk_dsi_ps_control_vact(struct mtk_dsi *dsi)
> >  {
> > struct videomode *vm = >vm;
> > @@ -480,6 +493,16 @@ static void mtk_dsi_start(struct mtk_dsi *dsi)
> > writel(1, dsi->regs + DSI_START);
> >  }
> >  
> > +static void mtk_dsi_stop(struct mtk_dsi *dsi)
> > +{
> > +   writel(0, dsi->regs + DSI_START);
> > +}
> > +
> > +static void mtk_dsi_set_cmd_mode(struct mtk_dsi *dsi)
> > +{
> > +   writel(CMD_MODE, dsi->regs + DSI_MODE_CTRL);
> > +}
> > +
> >  static void mtk_dsi_set_interrupt_enable(struct mtk_dsi *dsi)
> >  {
> > u32 inten = LPRX_RD_RDY_INT_FLAG | CMD_DONE_INT_FLAG | VM_DONE_INT_FLAG;
> > @@ -538,6 +561,19 @@ static irqreturn_t mtk_dsi_irq(int irq, void *dev_id)
> > return IRQ_HANDLED;
> >  }
> >  
> > +static s32 mtk_dsi_switch_to_cmd_mode(struct mtk_dsi *dsi, u8 irq_flag, 
> > u32 t)
> > +{
> > +   mtk_dsi_irq_data_clear(dsi, irq_flag);
> > +   mtk_dsi_set_cmd_mode(dsi);
> > +
> > +   if (!mtk_dsi_wait_for_irq_done(dsi, irq_flag, t)) {
> > +   DRM_ERROR("failed to switch cmd mode\n");
> > +   return -ETIME;
> > +   } else {
> > +   return 0;
> > +   }
> > +}
> > +
> >  static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
> >  {
> > if (WARN_ON(dsi->refcount == 0))
> > @@ -546,11 +582,6 @@ static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
> > if (--dsi->refcount != 0)
> > return;
> >  
> > -   mtk_dsi_lane0_ulp_mode_enter(dsi);
> > -   mtk_dsi_clk_ulp_mode_enter(dsi);
> > -
> > -   mtk_dsi_disable(dsi);
> > -
> > clk_disable_unprepare(dsi->engine_clk);
> > clk_disable_unprepare(dsi->digital_clk);
> >  
> > @@ -564,13 +595,6 @@ static void mtk_output_dsi_enable(struct mtk_dsi *dsi)
> > if (dsi->enabled)
> > return;
> >  
> > -   if (dsi->panel) {
> > -   if (drm_panel_prepare(dsi->panel)) {
> > -   DRM_ERROR("failed to setup the panel\n");
> > -   return;
> > -   }
> > -   }
> > -
> > ret = mtk_dsi_poweron(dsi);
> > if (ret < 0) {
> > 

[PATCH v2] drm/i915/dsi: Do not clear DPOUNIT_CLOCK_GATE_DISABLE from vlv_init_display_clock_gating

[Hans de Goede]

Hi,

On 05-12-16 19:52, Ville Syrjälä wrote:
> On Fri, Dec 02, 2016 at 03:29:04PM +0100, Hans de Goede wrote:
>> On my Cherrytrail CUBE iwork8 Air tablet PIPE-A would get stuck on loading
>> i915 at boot 1 out of every 3 boots, resulting in a non functional LCD.
>> Once the i915 driver has successfully loaded, the panel can be disabled /
>> enabled without hitting this issue.
>>
>> The getting stuck is caused by vlv_init_display_clock_gating() clearing
>> the DPOUNIT_CLOCK_GATE_DISABLE bit in DSPCLK_GATE_D when called from
>> chv_pipe_power_well_ops.enable() on driver load, while a pipe is enabled
>> driving the DSI LCD by the BIOS.
>>
>> Clearing this bit while DSI is in use is a known issue and
>> intel_dsi_pre_enable() / intel_dsi_post_disable() already set / clear it
>> as appropriate.
>>
>> This commit modifies vlv_init_display_clock_gating() to leave the
>> DPOUNIT_CLOCK_GATE_DISABLE bit alone fixing the pipe getting stuck.
>>
>> BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=97330
>> Cc: stable at vger.kernel.org
>> Signed-off-by: Hans de Goede 
>> Reviewed-by: Ville Syrjälä 
>> ---
>> Changes in v2:
>> -Replace PIPE-A with "a pipe" or "the pipe" in the commit msg and comment
>
> Pushed to dinq with s/BugLink/Bugzilla/ and changelog moved into the
> commit message proper. Thanks for the patch.

Thank you, what about the other 2 bug-fix patches, the
assert/deassert swap one and the one setting the gpio_en bit for
cherryview ?

Regards,

Hans


>
>> ---
>>  drivers/gpu/drm/i915/intel_runtime_pm.c | 13 -
>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
>> b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> index 356c662..87b4af0 100644
>> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> @@ -1039,7 +1039,18 @@ static bool vlv_power_well_enabled(struct 
>> drm_i915_private *dev_priv,
>>
>>  static void vlv_init_display_clock_gating(struct drm_i915_private *dev_priv)
>>  {
>> -I915_WRITE(DSPCLK_GATE_D, VRHUNIT_CLOCK_GATE_DISABLE);
>> +u32 val;
>> +
>> +/*
>> + * On driver load, a pipe may be active and driving a DSI display.
>> + * Preserve DPOUNIT_CLOCK_GATE_DISABLE to avoid the pipe getting stuck
>> + * (and never recovering) in this case. intel_dsi_post_disable() will
>> + * clear it when we turn off the display.
>> + */
>> +val = I915_READ(DSPCLK_GATE_D);
>> +val &= DPOUNIT_CLOCK_GATE_DISABLE;
>> +val |= VRHUNIT_CLOCK_GATE_DISABLE;
>> +I915_WRITE(DSPCLK_GATE_D, val);
>>
>>  /*
>>   * Disable trickle feed and enable pnd deadline calculation
>> --
>> 2.9.3
>

[PATCH 4/8] drm/tegra: Add VIC support

[Thierry Reding]

ice:
> + if (tegra->domain)
> + iommu_detach_device(tegra->domain, vic->dev);
> +
> + return err;
> +}
> +
> +static int vic_exit(struct host1x_client *client)
> +{
> + struct tegra_drm_client *drm = host1x_to_drm_client(client);
> + struct drm_device *dev = dev_get_drvdata(client->parent);
> + struct tegra_drm *tegra = dev->dev_private;
> + struct vic *vic = to_vic(drm);
> + int err;
> +
> + err = tegra_drm_unregister_client(tegra, drm);
> + if (err < 0)
> + return err;
> +
> + host1x_syncpt_free(client->syncpts[0]);
> + host1x_channel_free(vic->channel);
> +
> + falcon_exit(>falcon);
> +
> + if (vic->domain) {
> + iommu_detach_device(vic->domain, vic->dev);
> + vic->domain = NULL;
> + }
> +
> + return 0;
> +}
> +
> +static const struct host1x_client_ops vic_client_ops = {
> + .init = vic_init,
> + .exit = vic_exit,
> +};
> +
> +static int vic_open_channel(struct tegra_drm_client *client,
> + struct tegra_drm_context *context)
> +{
> + struct vic *vic = to_vic(client);
> + int err;
> +
> + err = pm_runtime_get_sync(vic->dev);
> + if (err < 0)
> + return err;
> +
> + /*
> +  * Try to boot the Falcon microcontroller. Booting is deferred until
> +  * here because the firmware might not yet be available during system
> +  * boot, for example if it's on remote storage.
> +  */

That sounds like a hack. Typically you're supposed to have the firmware
on the same medium as the module that requests it. That is, if the
driver is built-in, then you need to make the firmware available as
built-in as well or stash it into an initrd. If the driver is built as a
loadable module and is on the root filesystem, that's where the firmware
should be as well. There's also the possibility to put the loadable
module into an initrd, in which case that's where the firmware should go
as well.

That said, I think it makes sense to defer the actual booting until this
point. Loading the firmware seems unappropriate to me. It really should
be done in ->probe().

> + err = vic_boot(vic);
> + if (err < 0) {
> + pm_runtime_put(vic->dev);
> + return err;
> + }
> +
> + context->channel = host1x_channel_get(vic->channel);
> + if (!context->channel) {
> + pm_runtime_put(vic->dev);
> + return -ENOMEM;
> + }
> +
> + return 0;
> +}
> +
> +static void vic_close_channel(struct tegra_drm_context *context)
> +{
> + struct vic *vic = to_vic(context->client);
> +
> + host1x_channel_put(context->channel);
> +
> + pm_runtime_put(vic->dev);
> +}
> +
> +static const struct tegra_drm_client_ops vic_ops = {
> + .open_channel = vic_open_channel,
> + .close_channel = vic_close_channel,
> + .submit = tegra_drm_submit,
> +};
> +
> +static const struct vic_config vic_t124_config = {
> + .ucode_name = "nvidia/tegra124/vic03_ucode.bin",
> +};
> +
> +static const struct vic_config vic_t210_config = {
> + .ucode_name = "nvidia/tegra210/vic04_ucode.bin",
> +};
> +
> +static const struct of_device_id vic_match[] = {
> + { .compatible = "nvidia,tegra124-vic", .data = _t124_config },
> + { .compatible = "nvidia,tegra210-vic", .data = _t210_config },
> + { },
> +};
> +
> +static int vic_probe(struct platform_device *pdev)
> +{
> + struct vic_config *vic_config = NULL;
> + struct device *dev = >dev;
> + struct host1x_syncpt **syncpts;
> + struct resource *regs;
> + const struct of_device_id *match;
> + struct vic *vic;
> + int err;
> +
> + match = of_match_device(vic_match, dev);
> + vic_config = (struct vic_config *)match->data;
> +
> + vic = devm_kzalloc(dev, sizeof(*vic), GFP_KERNEL);
> + if (!vic)
> + return -ENOMEM;
> +
> + syncpts = devm_kzalloc(dev, sizeof(*syncpts), GFP_KERNEL);
> + if (!syncpts)
> + return -ENOMEM;
> +
> + regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!regs) {
> + dev_err(>dev, "failed to get registers\n");
> + return -ENXIO;
> + }
> +
> + vic->regs = devm_ioremap_resource(dev, regs);
> + if (IS_ERR(vic->regs))
> + return PTR_ERR(vic->regs);
> +
> + vic->clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(vic->clk)) {
> + dev_err(>dev, "failed to get clock\n");
> + return PTR_ERR(vic->clk);
> + }
> +
> + platform_set_drvdata(pdev, vic);
> +
> + INIT_LIST_HEAD(>client.base.list);
> + vic->client.base.ops = _client_ops;
> + vic->client.base.dev = dev;
> + vic->client.base.class = HOST1X_CLASS_VIC;
> + vic->client.base.syncpts = syncpts;
> + vic->client.base.num_syncpts = 1;
> + vic->dev = dev;
> + vic->config = vic_config;
> +
> + INIT_LIST_HEAD(>client.list);
> + vic->client.ops = _ops;
> +
> + err = host1x_client_register(>client.base);
> + if (err < 0) {
> + dev_err(dev, "failed to register host1x client: %d\n", err);
> + platform_set_drvdata(pdev, NULL);
> + return err;
> + }
> +
> + pm_runtime_enable(>dev);
> + if (!pm_runtime_enabled(>dev)) {
> + err = vic_runtime_resume(>dev);
> + if (err < 0)
> + goto unregister_client;
> + }

That's a slightly ugly construct, but I can't think of an easy way to
get rid of it (other than to depend on PM, which actually might be the
right thing to do here, it's already selected by ARCH_TEGRA on 64-bit
ARM). I can't think of a scenario where we'd want to run without runtime
PM.

> +
> + dev_info(>dev, "initialized");

Again, no need for this.

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161205/f141df14/attachment-0001.sig>

[PATCH] drm/amdgpu: don't add files at control minor debugfs directory

[Deucher, Alexander]

> -Original Message-
> From: Nicolai Stange [mailto:nicstange at gmail.com]
> Sent: Monday, December 05, 2016 3:30 PM
> To: Daniel Vetter
> Cc: Deucher, Alexander; Koenig, Christian; Michel Dänzer; linux-
> kernel at vger.kernel.org; dri-devel at lists.freedesktop.org; Nicolai Stange
> Subject: [PATCH] drm/amdgpu: don't add files at control minor debugfs
> directory
> 
> Since commit 8a357d10043c ("drm: Nerf DRM_CONTROL nodes"), a
> struct drm_device's ->control member is always NULL.
> 
> In the case of CONFIG_DEBUG_FS=y, amdgpu_debugfs_add_files() accesses
> ->control->debugfs_root though. This results in a NULL pointer
> dereference.
> 
> Fix this by omitting the drm_debugfs_create_files() call for the
> control minor debugfs directory which is now non-existent anyway.
> 
> Fixes: 8a357d10043c ("drm: Nerf DRM_CONTROL nodes")
> Signed-off-by: Nicolai Stange 

Please add the bugzilla:
https://bugs.freedesktop.org/show_bug.cgi?id=98915
With that,
Reviewed-by: Alex Deucher 

> ---
> Applicable to next-20161202. Compile-only tested.
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 --
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index deee2db..0cb3e82 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2493,9 +2493,6 @@ int amdgpu_debugfs_add_files(struct
> amdgpu_device *adev,
>   adev->debugfs_count = i;
>  #if defined(CONFIG_DEBUG_FS)
>   drm_debugfs_create_files(files, nfiles,
> -  adev->ddev->control->debugfs_root,
> -  adev->ddev->control);
> - drm_debugfs_create_files(files, nfiles,
>adev->ddev->primary->debugfs_root,
>adev->ddev->primary);
>  #endif
> @@ -2510,9 +2507,6 @@ static void amdgpu_debugfs_remove_files(struct
> amdgpu_device *adev)
>   for (i = 0; i < adev->debugfs_count; i++) {
>   drm_debugfs_remove_files(adev->debugfs[i].files,
>adev->debugfs[i].num_files,
> -  adev->ddev->control);
> - drm_debugfs_remove_files(adev->debugfs[i].files,
> -  adev->debugfs[i].num_files,
>adev->ddev->primary);
>   }
>  #endif
> --
> 2.10.2

[PATCH 3/8] drm/tegra: Add falcon helper library

[Thierry Reding]

_IRQDEST   0x101c
> +#define FALCON_IRQDEST_HALT  (1 << 4)
> +#define FALCON_IRQDEST_EXTERR(1 << 5)
> +#define FALCON_IRQDEST_SWGEN0(1 << 6)
> +#define FALCON_IRQDEST_SWGEN1(1 << 7)
> +#define FALCON_IRQDEST_EXT(v)(((v) & 0xff) << 8)
> +
> +#define FALCON_ITFEN 0x1048
> +#define FALCON_ITFEN_CTXEN   (1 << 0)
> +#define FALCON_ITFEN_MTHDEN  (1 << 1)
> +
> +#define FALCON_IDLESTATE 0x104c
> +
> +#define FALCON_CPUCTL0x1100
> +#define FALCON_CPUCTL_STARTCPU   (1 << 1)
> +
> +#define FALCON_BOOTVEC   0x1104
> +
> +#define FALCON_DMACTL0x110c
> +#define FALCON_DMACTL_DMEM_SCRUBBING (1 << 1)
> +#define FALCON_DMACTL_IMEM_SCRUBBING (1 << 2)
> +
> +#define FALCON_DMATRFBASE0x1110
> +
> +#define FALCON_DMATRFMOFFS   0x1114
> +
> +#define FALCON_DMATRFCMD 0x1118
> +#define FALCON_DMATRFCMD_IDLE(1 << 1)
> +#define FALCON_DMATRFCMD_IMEM(1 << 4)
> +#define FALCON_DMATRFCMD_SIZE_256B   (6 << 8)
> +
> +#define FALCON_DMATRFFBOFFS  0x111c
> +
> +struct falcon_firmware_bin_header_v1 {
> + u32 bin_magic;  /* 0x10de */
> + u32 bin_ver;/* cya, versioning of bin format (1) */

Not sure everyone understands what cya means, here. Perhaps just drop
it?

> + u32 bin_size;   /* entire image size including this header */
> + u32 os_bin_header_offset;
> + u32 os_bin_data_offset;
> + u32 os_bin_size;
> +};
> +
> +struct falcon_firmware_os_app_v1 {
> + u32 offset;
> + u32 size;
> +};
> +
> +struct falcon_firmware_os_header_v1 {
> + u32 os_code_offset;
> + u32 os_code_size;
> + u32 os_data_offset;
> + u32 os_data_size;
> + u32 num_apps;
> + struct falcon_firmware_os_app_v1 *app_code;
> + struct falcon_firmware_os_app_v1 *app_data;

The above two seem to be completel unused. Should we drop them?

> + u32 *os_ovl_offset;
> + u32 *os_ovl_size;
> +};

Can we in general sanitize the names of these a little? It's redundent
to add a os_ prefix in a structure that contains an _os_ infix.

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161205/30b6599a/attachment.sig>

[PATCH 2/8] drm/tegra: Allocate BOs from lower 4G when without IOMMU

[Thierry Reding]

On Thu, Nov 10, 2016 at 08:23:39PM +0200, Mikko Perttunen wrote:
> On 64-bit Tegras, buffer object memory allocation may
> return memory above 4G that units behind Host1x cannot
> access. Add the GFP_DMA flag to these allocation when
> IOMMU is not enabled to ensure units can always access
> BO memory.

I don't think that's entirely true. The display controller can address
more than 32 bits. It's perhaps slightly different from other units that
are behind host1x, but maybe we need a special case here to allow frame-
buffers to reside in higher memory?

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161205/ebea832e/attachment.sig>

[PATCH 1/8] drm/tegra: Add Tegra DRM allocation API

[Thierry Reding]

cal, too.

> +{
> + struct iova *alloc;
> + unsigned long shift;
> + void *virt;
> + gfp_t gfp;
> + int err;
> +
> + if (tegra->domain)
> + size = iova_align(>iova, size);
> + else
> + size = PAGE_ALIGN(size);
> +
> + gfp = GFP_KERNEL | __GFP_ZERO;
> + if (!tegra->domain) {
> + /*
> +  * Tegra210 has 64-bit physical addresses but units only support
> +  * 32-bit addresses, so if we don't have an IOMMU to do
> +  * translation, force allocations to be in the 32-bit range.
> +  */
> + gfp |= GFP_DMA;

Technically we do support Tegra132 and that already has 64-bit physical
addresses, so perhaps include that in the comment, or make it more
generic:

/*
 * Many units only support 32-bit addresses, even on 64-bit SoCs.
 * If there is no IOMMU to translate into a 32-bit IO virtual address
 * address space, force allocations to be in the lower 32-bit range.
 */

> + }
> +
> + virt = (void *)__get_free_pages(gfp, get_order(size));
> + if (!virt)
> + return NULL;

Perhaps we want to return an ERR_PTR()-encoded error code here so we can
differentiate between out-of-memory and out-of-virtual-address-space
conditions in the caller? Or perhaps other conditions as well.

Also, is __get_free_pages() the right API here? I thought that it always
returned contiguous memory, which, in the presence of an IOMMU, will be
completely unnecessary.

> +
> + if (!tegra->domain) {
> + /*
> +  * If IOMMU is disabled, devices address physical memory
> +  * directly.
> +  */
> + *iova = virt_to_phys(virt);
> + return virt;
> + }
> +
> + shift = iova_shift(>iova);
> + alloc = alloc_iova(>iova, size >> shift,
> +tegra->domain->geometry.aperture_end >> shift, true);

This is fairly cumbersome to use. Maybe a drm_mm would be easier in this
case? If not, could we at least store the upper limit of the carveout so
that we don't have to use the really long expression above?

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161205/090b6341/attachment.sig>

[Bug 98505] [radeon, amdgpu] Regression introduced in 4.8-rc3

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

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

Jari Tahvanainen  changed:

   What|Removed |Added

 Status|RESOLVED|CLOSED

--- Comment #37 from Jari Tahvanainen  ---
Closing resolved+Fixed. Verified by Reporter.

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

[Bug 98993] dosbox artefacts when using opengl

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

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

--- Comment #2 from smoki  ---
 BTW i tried not with dosbox git and not just git mesa, dosbox package from
Debian Sid, mesa currently there is 13.0.2, etc...

 I also have DRI3 flashing with rtcw game, this looks like same issue to me :)

 https://packages.debian.org/sid/dosbox
 https://packages.debian.org/sid/rtcw

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

[Powerpc] Sam460ex Canyonlands issue -Kernel 4.4.36

[Julian Margetson]

0
>> [5.332193] [ea84de50] [c0542dc8] bus_add_driver+0xe0/0x1f0
>> [5.332199] [ea84de70] [c054400c] driver_register+0xb4/0xf8
>> [5.332207] [ea84de90] [c00016f8] do_one_initcall+0x11c/0x1a0
>> [5.332214] [ea84df00] [c0d08acc] kernel_init_freeable+0x130/0x1cc
>> [5.332220] [ea84df30] [c0001cf4] kernel_init+0x14/0xf8
>> [5.332228] [ea84df40] [c000ade4] ret_from_kernel_thread+0x5c/0x64
>> [5.332230] Instruction dump:
>> [5.332241] 7c0803a6 4bb7c1ec 9421ffa0 7c0802a6 bdc10018 90010064
>> 894313d0 2f8a
>> [5.332251] 40be001c 3c60c0c7 38631c16 483ea6c5 <0fe0> 3860ffea
>> 480009ac 89431aa8
>> [5.332274] ---[ end trace 3fbb08226210324a ]---
>> [5.413526] Console: switching to colour frame buffer device 240x67


-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161205/b58b1493/attachment.html>

[PATCH libdrm 5/5] tests/drmdevice: use drmGetDevice[s]2

[Michel Dänzer]

On 03/12/16 01:32 AM, Emil Velikov wrote:
> From: Emil Velikov 
> 
> Pass along DRM_DEVICE_GET_PCI_REVISION only when the individual nodes
> are opened and update the printed messages accordingly.
> 
> v2: Attribute for the flag rename, call drmGetDevices2 w/o the flag.
> 
> Signed-off-by: Emil Velikov 
> Reviewed-by: Michel Dänzer 

[...]

> diff --git a/xf86drm.c b/xf86drm.c
> index 52e9b9f..a886768 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -3019,7 +3019,7 @@ static int drmParsePciDeviceInfo(int maj, int min,
>   uint32_t flags)
>  {
>  #ifdef __linux__
> -if (flags & DRM_DEVICE_GET_PCI_REVISION == 0)
> +if (!(flags & DRM_DEVICE_GET_PCI_REVISION))
>  return parse_separate_sysfs_files(maj, min, device, true);
>  
>  if (parse_separate_sysfs_files(maj, min, device, false))
> 

Please squash this hunk into patch 4 before pushing.


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

[Bug 98993] dosbox artefacts when using opengl

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

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

--- Comment #1 from smoki  ---

 Reproduced it on Kabini APU, does not happen under LIBGL_DRI3_DISABLE=1
variable... so one of those DRI3 issues.

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

[PATCH v3 3/4] dt-bindings: display: add Amlogic Meson DRM Bindings

[Rob Herring]

On Thu, Dec 01, 2016 at 10:05:59AM +0100, Neil Armstrong wrote:
> Reviewed-by: Laurent Pinchart 
> Signed-off-by: Neil Armstrong 
> ---
>  .../bindings/display/amlogic,meson-vpu.txt | 112 
> +
>  1 file changed, 112 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/amlogic,meson-vpu.txt

Acked-by: Rob Herring 

[PATCH v2 2/3] drm/edid: Implement SCDC support detection

[Thierry Reding]

On Mon, Dec 05, 2016 at 05:21:24PM +0100, Daniel Vetter wrote:
> On Mon, Dec 05, 2016 at 12:11:46PM +0100, Thierry Reding wrote:
> > On Mon, Dec 05, 2016 at 09:16:27AM +0100, Daniel Vetter wrote:
> > > On Mon, Dec 05, 2016 at 08:57:43AM +0100, Thierry Reding wrote:
> > > > On Sat, Dec 03, 2016 at 04:35:24AM +, Sharma, Shashank wrote:
> > > > > Hi Thierry, 
> > > > > 
> > > > > If you can please have a look on this patch, I had written one to 
> > > > > parse HF-VSDB, which was covering SCDC detection too. 
> > > > > https://patchwork.kernel.org/patch/9452259/ 
> > > > 
> > > > I think there had been pushback before about caching capabilities from
> > > > EDID, so from that point of view my patch is more inline with existing
> > > > EDID parsing API.
> > > 
> > > Hm, where was that pushback? We do have a bit a mess between explicitly
> > > parsing stuff (e.g. eld) and stuffing parsed data into drm_display_info.
> > 
> > You did object to a very similar patch some time ago that did a similar
> > thing for DPCD stuff. And also Villa had commented on an earlier patch
> > from Jose about concerns of bloating core structures:
> > 
> > https://patchwork.freedesktop.org/patch/104806/
> 
> DPCD I complained about because somehow we ended up with 2 sets of
> helpers, one filling a struct and the others returning directly. I
> objected to the fact that there's 2 (and imo your patch duplicated even
> more), not that I think one approach is clearly inferior to the other.

My recollection is that I had proposed that I do the work of
transitioning users of the parsers to the cached information but you had
said that it wasn't worth the churn and that we should just go with the
existing scheme of passing around the DPCD buffer and extending the
parsers as necessary.

[PATCH] drm/radeon: don't add files at control minor debugfs directory

[Michel Dänzer]

On 05/12/16 05:48 PM, Christian König wrote:
> Am 05.12.2016 um 09:39 schrieb Nicolai Stange:>>
>> I'll send compile-only tested patches either tonight or tomorrow. Shall
>> they cover the oopses only or the dead code as well?
> 
> Please start with the ops, cause we certainly will get complains about
> that rather fast.

Suspect we already did:

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


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

tilcdc: vblank wait timed out

[Bartosz Golaszewski]

2016-12-05 12:01 GMT+01:00 Bartosz Golaszewski :
> Hi Jyri,
>
> I pulled your recent tilcdc pull request on top of v4.9 and Sekhar's
> davinci branch (+ vga dac DT)[1].
>
> I'm getting "vblank wait timed out" errors when running simple modetest[2].
>
> This error happened before with the drm_bridge series[3], but went
> away at one of the subsequent patch versions.
>
> Could you please verify that you see the same error and advise on what
> could be the reason? I'm investigating on my own as well.
>
> Best regards,
> Bartosz Golaszewski
>
> [1] https://github.com/brgl/linux/tree/tilcdc/modetest_error
> [2] http://pastebin.com/rCM44Uds
> [3] http://www.spinics.net/lists/dri-devel/msg123732.html

This seems like some END_OF_FRAME0 interrupt-related race condition.
Increasing the timeout in drm_atomic_helper_wait_for_vblanks() from 50
to 100 and dropping drm_modeset_lock_crtc()/drm_modeset_unlock_crtc()
in tilcdc_crtc_recover_work() makes the warning disappear. Also:
calling drm_crtc_vblank_off() additionally before locking the crtc in
tilcdc_crtc_recover_work() also seems to fix the issue 90% of times. I
have been unable to figure out a reliable solution today though.

Best regards,
Bartosz Golaszewski

[PATCH] drm: parse hf-vsdb

[Thierry Reding]

   drm_parse_hdmi_vsdb_video(connector, db);
> + if (cea_db_is_hf_vsdb(db))
> + drm_parse_hf_vsdb(connector, db);
>   }
>  }
>  
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 34f9741..d011dd5 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -167,6 +167,46 @@ struct drm_display_info {
>*/
>   u32 bus_flags;
>  
> +#define DRM_HFVSDB_SCDC_SUPPORT  (1<<7)
> +#define DRM_HFVSDB_SCDC_RR_CAP   (1<<6)
> +#define DRM_HFVSDB_SCRAMBLING(1<<3)
> +#define DRM_HFVSDB_INDEPENDENT_VIEW  (1<<2)
> +#define DRM_HFVSDB_DUAL_VIEW (1<<1)
> +#define DRM_HFVSDB_3D_OSD(1<<0)

This looks to me like the wrong place for these defines. They should
probably go into include/drm/drm_edid.h instead.

> +
> + /**
> +  * @scdc_supported: Sink supports SCDC functionality.
> +  */
> + bool scdc_supported;
> +
> + /**
> +  * @scdc_rr_cap: Sink has SCDC read request capability.
> +  */
> + bool scdc_rr_cap;
> +
> + /**
> +  * @scrambling: Sync supports scrambling for <=340 Mcsc TMDS
> +  * char rates. Above 340 Mcsc rates, scrambling is always reqd.
> +  */
> + bool scrambling;
> +
> + /**
> +  * @independent_view_3d: Sink supports 3d independent view signaling
> +  * in HF-VSIF.
> +  */
> + bool independent_view_3d;
> +
> + /**
> +  * @dual_view_3d: Sink supports 3d dual view signaling in HF-VSIF.
> +  */
> + bool dual_view_3d;
> +
> + /**
> +  * @osd_disparity_3d: Sink supports 3d osd disparity indication
> +  * in HF-VSIF.
> +  */
> + bool osd_disparity_3d;
> +
>   /**
>* @max_tmds_clock: Maximum TMDS clock rate supported by the
>* sink in kHz. 0 means undefined.
> @@ -185,6 +225,14 @@ struct drm_display_info {
>   u8 edid_hdmi_dc_modes;
>  
>   /**
> +  * @edid_yuv420_dc_modes: bpc for deep color yuv420 encoding.
> +  * various sinks can support 10/12/16 bit per channel deep
> +  * color encoding. edid_yuv420_dc_modes = 0 means sink doesn't
> +  * support deep color yuv420 encoding.
> +  */
> + u8 edid_yuv420_dc_modes;

Why not store the additional formats in the edid_hdmi_dc_modes field?

> +
> + /**
>* @cea_rev: CEA revision of the HDMI sink.
>*/
>   u8 cea_rev;
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 38eabf6..5df8b9c 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -212,6 +212,11 @@ struct detailed_timing {
>  #define DRM_EDID_HDMI_DC_30   (1 << 4)
>  #define DRM_EDID_HDMI_DC_Y444 (1 << 3)
>  
> +/* YUV 420 deep color modes */
> +#define DRM_EDID_YUV420_DC_48  (1 << 6)
> +#define DRM_EDID_YUV420_DC_36  (1 << 5)
> +#define DRM_EDID_YUV420_DC_30  (1 << 5)

36- and 30-bit depths have the same value. That probably wasn't
intended.

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161205/4d0149c1/attachment.sig>

[Powerpc] Sam460ex Canyonlands issue -Kernel 4.4.36

[Julian Margetson]

Following  appears with kernel 4.4.36 .

Maybe as a result of following patch ?

drm/radeon: Ensure vblank interrupt is enabled on DPMS transition to on


[5.331666] Can't enable IRQ/MSI because no handler is installed
[5.331842] [ cut here ]
[5.331847] WARNING: at c048a53c [verbose debug info unavailable]
[5.331851] Modules linked in:
[5.331859] CPU: 0 PID: 1 Comm: swapper Not tainted 4.4.36-sam460ex-jm #1
[5.331864] task: ea848000 ti: ea84c000 task.ti: ea84c000
[5.331867] NIP: c048a53c LR: c048a53c CTR: 
[5.331870] REGS: ea84d6e0 TRAP: 0700   Not tainted (4.4.36-sam460ex-jm)
[5.331879] MSR: 00029000   CR: 24008222  XER: 
[5.331914]
[5.331914] GPR00: c048a53c ea84d790 ea848000 0034  
 c0ea7a2c 
[5.331914] GPR08: 00029000 0800 ea84c000 ea84d790 24008222 
 eaa8096c 0001
[5.331914] GPR16: eaacd9d4 c08e6c20   c0c678d7 
 c0da ea982048
[5.331914] GPR24: c08f24c0 ea982108 eaa80984 ea9d4d80 eaa80800 
eaa80800 eabdc000 ea982000
[5.331932] NIP [c048a53c] si_irq_set+0x28/0x9e4
[5.331937] LR [c048a53c] si_irq_set+0x28/0x9e4
[5.331938] Call Trace:
[5.331945] [ea84d790] [c048a53c] si_irq_set+0x28/0x9e4 (unreliable)
[5.331958] [ea84d7f0] [c0409b20] atombios_crtc_dpms+0x8c/0x104
[5.331964] [ea84d810] [c0409bb4] atombios_crtc_commit+0x1c/0x38
[5.331972] [ea84d820] [c03c9d90] drm_crtc_helper_set_mode+0x36c/0x410
[5.331978] [ea84d9f0] [c03ca628] drm_crtc_helper_set_config+0x6dc/0x934
[5.331991] [ea84da60] [c0423a70] radeon_crtc_set_config+0x50/0xf8
[5.332007] [ea84da80] [c03e47b0] drm_mode_set_config_internal+0x5c/0xe0
[5.332021] [ea84daa0] [c03d4704] restore_fbdev_mode+0x20c/0x25c
[5.332030] [ea84dad0] [c03d61ec] 
drm_fb_helper_restore_fbdev_mode_unlocked+0x40/0x8c
[5.332036] [ea84daf0] [c03d6284] drm_fb_helper_set_par+0x4c/0x60
[5.332047] [ea84db00] [c0388144] fbcon_init+0x318/0x410
[5.332056] [ea84db50] [c03b3024] visual_init+0xac/0xf8
[5.332063] [ea84db60] [c03b487c] do_bind_con_driver+0x1e8/0x2d8
[5.332070] [ea84dbb0] [c03b4cd0] do_take_over_console+0x19c/0x1ac
[5.332075] [ea84dbe0] [c03878bc] do_fbcon_takeover+0x80/0xe4
[5.332087] [ea84dbf0] [c003c3fc] notifier_call_chain+0x60/0x94
[5.332094] [ea84dc10] [c003c71c] 
__blocking_notifier_call_chain+0x50/0x68
[5.332104] [ea84dc30] [c038f3a8] register_framebuffer+0x248/0x26c
[5.332111] [ea84dc90] [c03d65b4] 
drm_fb_helper_initial_config+0x31c/0x39c
[5.332120] [ea84dcd0] [c042bc78] radeon_fbdev_init+0xcc/0xf8
[5.332126] [ea84dcf0] [c0425dd0] radeon_modeset_init+0x5a8/0x938
[5.332132] [ea84dd50] [c040316c] radeon_driver_load_kms+0xcc/0x184
[5.332138] [ea84dd70] [c03df0b8] drm_dev_register+0x84/0xc8
[5.332144] [ea84dd90] [c03e0e78] drm_get_pci_dev+0xf4/0x18c
[5.332156] [ea84ddb0] [c03719e4] pci_device_probe+0x88/0xe8
[5.332164] [ea84ddd0] [c05436f4] driver_probe_device+0x130/0x290
[5.332170] [ea84de00] [c05438cc] __driver_attach+0x78/0xa0
[5.332188] [ea84de20] [c0541bec] bus_for_each_dev+0x90/0xa0
[5.332193] [ea84de50] [c0542dc8] bus_add_driver+0xe0/0x1f0
[5.332199] [ea84de70] [c054400c] driver_register+0xb4/0xf8
[5.332207] [ea84de90] [c00016f8] do_one_initcall+0x11c/0x1a0
[5.332214] [ea84df00] [c0d08acc] kernel_init_freeable+0x130/0x1cc
[5.332220] [ea84df30] [c0001cf4] kernel_init+0x14/0xf8
[5.332228] [ea84df40] [c000ade4] ret_from_kernel_thread+0x5c/0x64
[5.332230] Instruction dump:
[5.332241] 7c0803a6 4bb7c1ec 9421ffa0 7c0802a6 bdc10018 90010064 
894313d0 2f8a
[5.332251] 40be001c 3c60c0c7 38631c16 483ea6c5 <0fe0> 3860ffea 
480009ac 89431aa8
[5.332274] ---[ end trace 3fbb08226210324a ]---
[5.413526] Console: switching to colour frame buffer device 240x67

-- next part --
[0.00] Using Canyonlands machine description
[0.00] Initializing cgroup subsys cpu
[0.00] Linux version 4.4.36-sam460ex-jm (root at julian-VirtualBox) 
(gcc version 5.4.0 20160609 (Ubuntu 5.4.0-6ubuntu1~16.04.1) ) #1 PREEMPT Mon 
Dec 5 17:16:35 AST 2016
[0.00] Zone ranges:
[0.00]   DMA  [mem 0x-0x2fff]
[0.00]   Normal   empty
[0.00]   HighMem  [mem 0x3000-0x7fff]
[0.00] Movable zone start for each node
[0.00] Early memory node ranges
[0.00]   node   0: [mem 0x-0x7fff]
[0.00] Initmem setup node 0 [mem 0x-0x7fff]
[0.00] MMU: Allocated 1088 bytes of context maps for 255 contexts
[0.00] Built 1 zonelists in Zone order, mobility grouping on.  Total 
pages: 522752
[0.00] Kernel command line: root=/dev/sda6 console=ttyS0,115200 
console=tty0
[0.00] PID hash table entries: 4096 (order: 2, 16384 bytes)
[0.00] 

[PATCH libdrm v2 5/5] xf86drm: implement an OpenBSD specific drmGetDevice2

[Emil Velikov]

On 1 December 2016 at 04:18, Jonathan Gray  wrote:
> DRI devices on OpenBSD are not in their own directory.  They reside in
> /dev with a large number of statically generated /dev nodes.
>
> Avoid stat'ing all of /dev on OpenBSD by implementing this custom path.
>
> v2:
>- use drmGetMinorType to get node type
>- adapt to drmProcessPciDevice changes
>- verify drmParseSubsystemType type is PCI
>- add a comment describing why this was added
>
Thanks for the update Jonathan.

I've pulled v2 in master,
Emil

[RESEND PATCH v2 4/7] drm/vc4: Add support for the VEC (Video Encoder) IP

[Florian Fainelli]

On 12/02/2016 05:48 AM, Boris Brezillon wrote:
> The VEC IP is a TV DAC, providing support for PAL and NTSC standards.
> 
> Signed-off-by: Boris Brezillon 
> ---

> diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c
> new file mode 100644
> index ..2d4256fcc6f2
> --- /dev/null
> +++ b/drivers/gpu/drm/vc4/vc4_vec.c
> @@ -0,0 +1,657 @@
> +/*
> + * Copyright (C) 2016 Broadcom Limited

The standard copyright template post acquisition is just Broadcom, not
Broadcom Limited, nor Broadcom corporation. Can you audit your entire
submission and fix this up accordingly?

Thanks!
-- 
Florian

[PATCH v2 3/3] drm/edid: Implement SCDC Read Request capability detection

[Thierry Reding]

On Mon, Dec 05, 2016 at 02:19:48PM +, Jose Abreu wrote:
> Hi Thierry,
> 
> 
> On 05-12-2016 11:14, Thierry Reding wrote:
> > On Mon, Dec 05, 2016 at 11:06:15AM +, Jose Abreu wrote:
> >> Hi Thierry,
> >>
> >>
> >> Do you think while you are at it you could implement a
> >> set_scrambling() callback? It should be pretty straight forward:
> >> you read the SCDC_TMDS_CONFIG reg, do a mask, and then write it
> >> again.
> >>
> >>
> >> I think this is an important feature that we should have.
> > Yeah, agreed. I was actually thinking about going one step further and
> > provide more of the polling functionality as a helper. Even if we have
> > accessors that wrap the low-level functionality, most drivers would
> > still have to provide their own delayed workqueue to deal with sinks
> > (or sources) that don't support read requests. Having this in standard
> > helpers would help reduce the boilerplate a lot further.
> >
> > Does your hardware by any chance support read requests on SCDC? It'd be
> > interesting to see how that works in practice. Unfortunately Tegra does
> > not seem to support it.
> >
> > Thierry
> 
> Yes, my HW supports it though I don't have the setup here to test
> right now (but it was used before). In our controller it is
> pretty straight forward:
> 1) Enable interrupt for rr indication [source]
> 2) Enable update read on a rr [source]
> 3) Set rr flag in the sink [sink]
> 
> Point 2) makes it clear: Whenever we get a rr then the source
> will automatically read the sink and dump the contents read in
> the regbank. Then the SW gets an interrupt and it should read the
> contents of the registers.

Yes, I suspect that there will be three types of setups:

1) fully supported and integrated RR capability (such as in your
   case)

2) external I2C controller with the means of detecting the read
   request (and signal via an interrupt)

3) no read request support at all, in which case a delayed work
   queue is needed to poll periodically


For cases 1) and 3) it's probably only useful to have a helper to enable
scrambling. For 2) there might be some use in having a helper that can
setup the delayed work queue.

Given that we don't have any implementation yet, maybe it's better to
defer the creation of helpers until we see common patterns emerge. The
helper to enable scrambling could be useful in any case, though, so I'll
add one.

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161205/0dae8e12/attachment.sig>

[PATCH] drm/atomic: doc: remove old comment about nonblocking commits

[Daniel Vetter]

On Mon, Dec 05, 2016 at 12:03:46PM -0200, Gustavo Padovan wrote:
> From: Gustavo Padovan 
> 
> We now support nonblocking commits on drm_atomic_helper_commit()
> so the comment is not valid anymore.
> 
> Signed-off-by: Gustavo Padovan 

Oops. Reviewed-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 0b16587..95e2984 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1226,9 +1226,6 @@ static void commit_work(struct work_struct *work)
>   * function implements nonblocking commits, using
>   * drm_atomic_helper_setup_commit() and related functions.
>   *
> - * Note that right now this function does not support nonblocking commits, 
> hence
> - * driver writers must implement their own version for now.
> - *
>   * Committing the actual hardware state is done through the
>   * ->atomic_commit_tail() callback of the _mode_config_helper_funcs 
> vtable,
>   * or it's default implementation drm_atomic_helper_commit_tail().
> -- 
> 2.5.5
> 
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[RFC] drm: Enable dynamic debug for DRM_[DEV]_DEBUG*

[Daniel Vetter]

On Mon, Dec 05, 2016 at 11:24:44AM +, Robert Bragg wrote:
> Forgot to send to dri-devel when I first sent this out...
> 
> The few times I've looked at using DRM_DEBUG messages, I haven't found
> them very helpful considering how noisy some of the categories are. More
> than once now I've preferred to go in and modify individual files to
> affect what messages I see and re-build.
> 
> After recently converting some of the i915_perf.c messages to use
> DRM_DEBUG, I thought I'd see if DRM_DEBUG could be updated to have a bit
> more fine grained control than the current category flags.
> 
> A few things to note with this first iteration:
> 
> - I haven't looked to see what affect the change has on linked object
>   sizes.
> 
> - It seems like it could be nice if dynamic debug could effectively make
>   the drm_debug parameter redundant but dynamic debug doesn't give us a
>   way to categorise messages so maybe we'd want to consider including
>   categories in messages something like:
> 
>   "[drm][kms] No FB found"
> 
>   This way all kms messages could be enabled via:
>   echo "format [kms] +p" > dynamic_debug/control
> 
>   Note with this simple scheme categories would no longer be mutually
>   exclusive which could be a nice bonus.

Really nice idea, and I agree that unifying drm.debug with dynamic debug
in some way would be useful. We could implement your idea by reworking the
existing debug helpers to auto-prepend the right string. That also opens
up the door for much more fine-grained bucketing maybe, only challenge is
that we should document things somewhere.

>   Since it would involve changing the output format, I wonder how
>   concerned others might be about breaking some userspace (maybe CI test
>   runners) that for some reason grep for specific messages?

I think the only thing we have to keep working (somehow) is drm.debug. The
exact output format doesn't really matter at all. Getting drm.debug to
work when dynamic debugging is enabled probably requires exporting some
functions, so that we can set the right ddebug options from the drm.debug
mod-option write handler. There's special mod-option macros that allow you
to specify write handlers, so that part is ok.

The other bit of backwards compat we imo need is that by default we should
still keep drm.debug working, even when dynamic debugging is disabled.
Having a third option that uses no_printk or similar (to get rid of all
the debug strings and dead-code-eliminate all the related output code)

> --- >8 --- (git am --scissors)
> 
> Dynamic debug messages (ref: Documentation/dynamic-debug-howto.txt)
> allow fine grained control over which debug messages are enabled with
> runtime control through /sysfs/kernel/debug/dynamic_debug/control
> 
> This provides more control than the current drm.drm_debug parameter
> which for some use cases is impractical to use given how chatty
> some drm debug categories are.
> 
> For example all debug messages in i915_drm.c can be enabled with:
> echo "file i915_perf.c +p" > dynamic_debug/control
> 
> This aims to maintain compatibility with controlling debug messages
> using the drm_debug parameter. The new dynamic debug macros are called
> by default but conditionally calling [dev_]printk if the category flag
> is set (side stepping the dynamic debug condition in that case)
> 
> This removes the drm_[dev_]printk wrappers considering that the dynamic
> debug macros are only useful if they can track the __FILE__, __func__
> and __LINE__ where they are called. The wrapper didn't seem necessary in
> the DRM_UT_NONE case with no category flag.
> 
> The output format should be compatible, unless the _DEV macros are
> passed a NULL dev pointer considering how the core.c dev_printk
> implementation adds "(NULL device *)" to the message in that case while
> the drm wrapper would fallback to a plain printk in this case.
> Previously some of non-dev drm debug macros were defined in terms of
> passing NULL to a dev version but that's avoided now due to this
> difference.
> 
> Signed-off-by: Robert Bragg 
> Cc: dri-devel at lists.freedesktop.org
> Cc: Daniel Vetter 
> Cc: Chris Wilson 
> ---
>  drivers/gpu/drm/drm_drv.c |  47 -
>  include/drm/drmP.h| 168 
> +-
>  2 files changed, 108 insertions(+), 107 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index f74b7d0..25d00aa 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -65,53 +65,6 @@ static struct idr drm_minors_idr;
>  
>  static struct dentry *drm_debugfs_root;
>  
> -#define DRM_PRINTK_FMT "[" DRM_NAME ":%s]%s %pV"
> -
> -void drm_dev_printk(const struct device *dev, const char *level,
> - unsigned int category, const char *function_name,
> - const char *prefix, const char *format, ...)
> -{
> - struct va_format vaf;
> - va_list args;
> -
> - if (category != DRM_UT_NONE && !(drm_debug & 

[PATCH v2 2/3] drm/edid: Implement SCDC support detection

[Daniel Vetter]

On Mon, Dec 05, 2016 at 12:11:46PM +0100, Thierry Reding wrote:
> On Mon, Dec 05, 2016 at 09:16:27AM +0100, Daniel Vetter wrote:
> > On Mon, Dec 05, 2016 at 08:57:43AM +0100, Thierry Reding wrote:
> > > On Sat, Dec 03, 2016 at 04:35:24AM +, Sharma, Shashank wrote:
> > > > Hi Thierry, 
> > > > 
> > > > If you can please have a look on this patch, I had written one to parse 
> > > > HF-VSDB, which was covering SCDC detection too. 
> > > > https://patchwork.kernel.org/patch/9452259/ 
> > > 
> > > I think there had been pushback before about caching capabilities from
> > > EDID, so from that point of view my patch is more inline with existing
> > > EDID parsing API.
> > 
> > Hm, where was that pushback? We do have a bit a mess between explicitly
> > parsing stuff (e.g. eld) and stuffing parsed data into drm_display_info.
> 
> You did object to a very similar patch some time ago that did a similar
> thing for DPCD stuff. And also Villa had commented on an earlier patch
> from Jose about concerns of bloating core structures:
> 
>   https://patchwork.freedesktop.org/patch/104806/

DPCD I complained about because somehow we ended up with 2 sets of
helpers, one filling a struct and the others returning directly. I
objected to the fact that there's 2 (and imo your patch duplicated even
more), not that I think one approach is clearly inferior to the other.

Demanding that there's some real users is also a valid point.

> > I think long-term stuffing it into drm_display_info is probably better,
> > since then we only have 1 interaction point between the probe code and the
> > atomic_check code. That should be useful for eventually fixing the lack of
> > locking between the two, if I ever get around to that ;-)
> 
> I don't really have objections to caching the results of parsing, it's
> what I had proposed and what seemed most natural back when I was working
> on the DPCD helpers. But if we now agree that this is the preferred way
> to do things, then we should at least agree that it applies to all areas
> for the sake of consistency.
> 
> Also, it might be worth looking into improving the structures, and maybe
> adding new ones to order things more conveniently or at least group them
> in some logical way. In my opinion some of our data structures are
> becoming somewhat... unwieldy.

We're pretty good at consuming fairly invasive refactorings in drm-misc.
So it just boils down to get some agreement on what things should look
like (+1 from my side to parsing stuff into structs as a general idea),
and then massaging all the existing users of the "wrong" interface using
cocci and sed.

Unfortunately "just" ;-)

> > > Other than that the patches are mostly equivalent, except yours parses
> > > more information than just the SCDC bits.
> > 
> > So merge patch 1 from your series + Shashank's parsing patch? Everyone
> > agrees and can you pls cross-r-b stamp so I can apply it all?
> 
> I think I spotted a mistake in Shashank's parsing patch. Let me take
> another look.
> 
> If we can agree on a common way forward on how to deal with this kind of
> static data, I have no objections to caching data for the duration of a
> hotplug period.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH v2] drm: allow changing DPMS mode

[Daniel Vetter]

On Mon, Dec 05, 2016 at 02:04:08PM +0200, Marta Lofstedt wrote:
> The drm_atomic_helper_connector_dpms
> will set the connector back the old DPMS state
> before returning. This makes it impossible to change
> DPMS state of the connector.
> 
> Fixes: 0853695c3ba46f97dfc0b5885f7b7e640ca212dd
> v2: edit of commit message
> Cc: Chris Wilson 
> Cc: Daniel Vetter 
> Cc: Eric Engestrom 
> Cc: Sean Paul 
> Cc: dri-devel at lists.freedesktop.org
> Cc: 
> Signed-off-by: Marta Lofstedt 

Applied to drm-misc, thanks.
-Daniel

> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 494680c..6a5acb9 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2885,8 +2885,8 @@ int drm_atomic_helper_connector_dpms(struct 
> drm_connector *connector,
>  fail:
>   if (ret == -EDEADLK)
>   goto backoff;
> -
> - connector->dpms = old_mode;
> + if (ret != 0)
> + connector->dpms = old_mode;
>   drm_atomic_state_put(state);
>   return ret;
>  
> -- 
> 2.9.3
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[RFC 4/4] drm/i915: Creating guc log file in drmfs instead of debugfs

[swati.dhin...@intel.com]

From: Sourab Gupta 

In the current scenario, the relay API fit well only with debugfs, due to
availability of parent dentry. Any other existing filesystem was not feasible 
for
holding guc logs, due to incompatibility with relay. But this makes the  guc_log
file unavailable on the production kernels.

GuC log file can therefore be one of candidates for movement to the drmfs
filesystem, which can satisfy all the requirements needed by relay API, and can
house any relayfs based output file.

The patch moves the parent directory of guc 'log_dir' from debugfs_root to
drmfs_root, while using the drmfs api's to create the requisite files.

Signed-off-by: Sourab Gupta 
Signed-off-by: Swati Dhingra 
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 33 +++---
 1 file changed, 12 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
b/drivers/gpu/drm/i915/i915_guc_submission.c
index 5841380..3f5978a 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -22,8 +22,8 @@
  *
  */
 #include 
-#include 
 #include 
+#include 
 #include "i915_drv.h"
 #include "intel_uc.h"

@@ -812,7 +812,7 @@ static int subbuf_start_callback(struct rchan_buf *buf,
 }

 /*
- * file_create() callback. Creates relay file in debugfs.
+ * file_create() callback. Creates relay file.
  */
 static struct dentry *create_buf_file_callback(const char *filename,
   struct dentry *parent,
@@ -838,17 +838,19 @@ static struct dentry *create_buf_file_callback(const char 
*filename,
 * dentry of the file associated with the channel buffer and that file's
 * name need not be same as the filename passed as an argument.
 */
-   buf_file = debugfs_create_file("guc_log", mode,
+
+   buf_file = drmfs_create_file("guc_log", mode,
   parent, buf, _file_operations);
+
return buf_file;
 }

 /*
- * file_remove() default callback. Removes relay file in debugfs.
+ * file_remove() default callback. Removes relay file.
  */
 static int remove_buf_file_callback(struct dentry *dentry)
 {
-   debugfs_remove(dentry);
+   drmfs_remove(dentry);
return 0;
 }

@@ -898,22 +900,11 @@ static int guc_log_create_relay_file(struct intel_guc 
*guc)
struct dentry *log_dir;
int ret;

-   /* For now create the log file in /sys/kernel/debug/dri/0 dir */
-   log_dir = dev_priv->drm.primary->debugfs_root;
-
-   /* If /sys/kernel/debug/dri/0 location do not exist, then debugfs is
-* not mounted and so can't create the relay file.
-* The relay API seems to fit well with debugfs only, for availing relay
-* there are 3 requirements which can be met for debugfs file only in a
-* straightforward/clean manner :-
-* i)   Need the associated dentry pointer of the file, while opening 
the
-*  relay channel.
-* ii)  Should be able to use 'relay_file_operations' fops for the file.
-* iii) Set the 'i_private' field of file's inode to the pointer of
-*  relay channel buffer.
-*/
+   /* Create the log file in drmfs dir: /sys/kernel/drm/i915/ */
+   log_dir = dev_priv->drm.driver->drmfs_root;
+
if (!log_dir) {
-   DRM_ERROR("Debugfs dir not available yet for GuC log file\n");
+   DRM_ERROR("Root dir not available yet for GuC log file\n");
return -ENODEV;
}

@@ -1154,7 +1145,7 @@ static int guc_log_create_extras(struct intel_guc *guc)
if (!guc->log.relay_chan) {
/* Create a relay channel, so that we have buffers for storing
 * the GuC firmware logs, the channel will be linked with a file
-* later on when debugfs is registered.
+* later on when drmfs is registered.
 */
ret = guc_log_create_relay_channel(guc);
if (ret)
-- 
2.7.4

[RFC 3/4] drm: Create driver specific root directory inside drmfs

[swati.dhin...@intel.com]

From: Sourab Gupta 

A driver specific directory is created inside drmfs root, and dentry of
this directory is saved for subsequent use by the driver (e.g. i915).
The driver can then create files/directories inside this root directory
directly.

In case of i915 driver, the top directory is created at '/sys/kernel/drm/i915'.

Signed-off-by: Sourab Gupta 
Signed-off-by: Swati Dhingra 
---
 drivers/gpu/drm/drm_drv.c | 6 ++
 include/drm/drm_drv.h | 3 +++
 2 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 298013c..4c32cf8 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -671,6 +671,11 @@ EXPORT_SYMBOL(drm_dev_unref);
 int drm_dev_register(struct drm_device *dev, unsigned long flags)
 {
int ret;
+   struct dentry *drmfs_root;
+
+   drmfs_root = drmfs_create_dir(dev->driver->name, NULL);
+   if (!IS_ERR(drmfs_root))
+   dev->driver->drmfs_root = drmfs_root;

mutex_lock(_global_mutex);

@@ -742,6 +747,7 @@ void drm_dev_unregister(struct drm_device *dev)
drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
drm_minor_unregister(dev, DRM_MINOR_RENDER);
drm_minor_unregister(dev, DRM_MINOR_CONTROL);
+   drmfs_remove(dev->driver->drmfs_root);
 }
 EXPORT_SYMBOL(drm_dev_unregister);

diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index c4fc495..d646296 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -405,6 +405,9 @@ struct drm_driver {

/* List of devices hanging off this driver with stealth attach. */
struct list_head legacy_dev_list;
+
+   /* drmfs parent directory dentry for this driver */
+   struct dentry *drmfs_root;
 };

 extern __printf(6, 7)
-- 
2.7.4

[RFC 2/4] drm: Register drmfs filesystem from drm init

[swati.dhin...@intel.com]

From: Sourab Gupta 

The drmfs filesystem will not be registered standalone during kernel init time,
instead it is intended to be initialized/registered during drm initialization.

Signed-off-by: Sourab Gupta 
Signed-off-by: Swati Dhingra 
---
 drivers/gpu/drm/drm_drv.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index f74b7d0..298013c 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -34,6 +34,7 @@
 #include 

 #include 
+#include 
 #include 

 #include "drm_crtc_internal.h"
@@ -828,6 +829,7 @@ static void drm_core_exit(void)
 {
unregister_chrdev(DRM_MAJOR, "drm");
debugfs_remove(drm_debugfs_root);
+   drmfs_fini();
drm_sysfs_destroy();
idr_destroy(_minors_idr);
drm_connector_ida_destroy();
@@ -848,6 +850,10 @@ static int __init drm_core_init(void)
goto error;
}

+   ret = drmfs_init();
+   if (ret < 0)
+   DRM_ERROR("Cannot create DRM FS: %d\n", ret);
+
drm_debugfs_root = debugfs_create_dir("dri", NULL);
if (!drm_debugfs_root) {
ret = -ENOMEM;
-- 
2.7.4

[RFC 1/4] drm: Introduce drmfs pseudo filesystem interfaces

[swati.dhin...@intel.com]

From: Sourab Gupta 

The patch introduces a new pseudo filesystem type, named 'drmfs' which is
intended to house the files for the data generated by drm subsystem that
cannot be accommodated by any of the existing filesystems.
The filesystem is modelled on the lines of existing pseudo-filesystems such
as debugfs/tracefs, and borrows ideas from their implementation.
This filesystem will be appearing at sys/kernel/drm.

A new config 'CONFIG_DRMFS' is introduced to enable/disable the filesystem,
which is dependent on CONFIG_DRM.
The filesystem will not be registered standalone during kernel init time,
instead it is intended to be initialized/registered during drm initialization.

The intent for introduction of the filesystem is to act as a location to hold
various kinds of data output from Linux DRM subsystems, which can't really fit
anywhere else into the existing filesystems such as debugfs/sysfs etc. All these
filesystems have their own constraints and are intended to output a particular
type of data such as attributes and small debug parameter data. Due to these
constraints, there is a need for a new pseudo filesytem, customizable to DRM
specific requirements and catering to the needs to DRM subsystem components

Signed-off-by: Sourab Gupta 
Signed-off-by: Swati Dhingra 
---
 drivers/gpu/drm/Kconfig|   9 +
 drivers/gpu/drm/Makefile   |   1 +
 drivers/gpu/drm/drmfs.c| 555 +
 include/drm/drmfs.h|  77 +++
 include/uapi/linux/magic.h |   3 +
 5 files changed, 645 insertions(+)
 create mode 100644 drivers/gpu/drm/drmfs.c
 create mode 100644 include/drm/drmfs.h

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 95fc041..5082840 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -26,6 +26,15 @@ config DRM_MIPI_DSI
bool
depends on DRM

+config DRMFS
+   bool "Drmfs file system support"
+   depends on DRM
+   help
+ Drmfs is a pseudo file system for drm subsystem output data.
+
+ drmfs is a filesystem which serves as a stable ABI to hold
+ miscellaneous output data from drm subsystems.
+
 config DRM_DP_AUX_CHARDEV
bool "DRM DP AUX Interface"
depends on DRM
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 883f3e7..5021225 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -42,6 +42,7 @@ CFLAGS_drm_trace_points.o := -I$(src)

 obj-$(CONFIG_DRM)  += drm.o
 obj-$(CONFIG_DRM_MIPI_DSI) += drm_mipi_dsi.o
+obj-$(CONFIG_DRMFS)+= drmfs.o
 obj-$(CONFIG_DRM_ARM)  += arm/
 obj-$(CONFIG_DRM_TTM)  += ttm/
 obj-$(CONFIG_DRM_TDFX) += tdfx/
diff --git a/drivers/gpu/drm/drmfs.c b/drivers/gpu/drm/drmfs.c
new file mode 100644
index 000..bdb0e7b
--- /dev/null
+++ b/drivers/gpu/drm/drmfs.c
@@ -0,0 +1,555 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Authors:
+ * Swati Dhingra 
+ * Sourab Gupta 
+ * Akash Goel 
+ */
+
+/*
+ * drmfs is the filesystem used for output of drm subsystem data
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DRMFS_DEFAULT_MODE 0700
+
+static struct vfsmount *drmfs_mount;
+static int drmfs_mount_count;
+static bool drmfs_registered;
+
+static ssize_t default_read_file(struct file *file, char __user *buf,
+   size_t count, loff_t *ppos)
+{
+   return 0;
+}
+
+static ssize_t default_write_file(struct file *file, const char __user *buf,
+   size_t count, loff_t *ppos)
+{
+   return count;
+}
+
+static const struct file_operations drmfs_default_file_operations = {
+   .read = default_read_file,
+   .write =default_write_file,
+   .open = simple_open,
+  

[RFC 0/4] Introduce drmfs pseudo filesystem for drm subsystem

[swati.dhin...@intel.com]

From: Swati Dhingra 

Currently, we don't have a stable ABI which can be used for the purpose of
providing output debug/loggging/crc and other such data from DRM.
The ABI in current use (filesystems, ioctls, et al.) have their own
constraints and are intended to output a particular type of data.
Few cases in point:
sysfs   - stable ABI, but constrained to one textual value per file
debugfs - unstable ABI, free-for-all
ioctls  - not as suitable to many single purpose continuous data
  dumping, we would very quickly run out ioctl space; requires more
  userspace support than "cat"
device nodes -  a real possibilty, kernel instantiation is more tricky,
requires udev (+udev.rules) or userspace discovery of the
dynamic major:minor (via sysfs) [mounting a registered
filesystem is easy in comparison]
netlink - stream based, therefore involves numerous copies.

Debugfs is the lesser among the evils here, thereby we have grown used to the
convenience and flexibility in presentation that debugfs gives us
(including relayfs inodes) that we want some of that hierachy in stable user
ABI form.

Due to these limitations, there is a need for a new pseudo filesytem, that
would act as a stable 'free-for-all' ABI, with the heirarchial structure and
thus convenience of debugfs. This will be managed by drm, thus named 'drmfs'.
DRM would register this filesystem to manage a canonical mountpoint, but this
wouldn't limit everyone to only using that pseudofs underneath.

This can serve to hold various kinds of output data from Linux DRM subsystems,
for the files which can't truely fit anywhere else with existing ABI's but
present so, for the lack of a better place.

In this patch series, we have introduced a pseudo filesystem named as 'drmfs'
for now. The filesystem is introduced in the first patch, and the subsequent
patches make use of the filesystem interfaces, in drm driver, and making them
available for use by the drm subsystem components, one of which is i915.
We've moved the location of i915 GuC logs from debugfs to drmfs in the last
patch. Subsequently, more such files such as pipe_crc, error states, memory
stats, etc. can be move to this filesystem, if the idea introduced here is
acceptable per se. The filesystem introduced is being used to house the data
generated by i915 driver in this patch series, but will hopefully be generic
enough to provide scope for usage by any other drm subsystem component.

The patch series is being floated as RFC to gather feedback on the idea and
infrastructure proposed here and it's suitability to address the specific
problem statement/use case.

TODO: Create documentation. Will do so in next version.

v2: fix the bat failures caused due to missing config check

v3: Changes made:
- Move the location of drmfs from fs/ to drivers/gpu/drm/ (Chris)
- Moving config checks to header (Chris,Daniel)


Sourab Gupta (4):
  drm: Introduce drmfs pseudo filesystem interfaces
  drm: Register drmfs filesystem from drm init
  drm: Create driver specific root directory inside drmfs
  drm/i915: Creating guc log file in drmfs instead of debugfs

 drivers/gpu/drm/Kconfig|   9 +
 drivers/gpu/drm/Makefile   |   1 +
 drivers/gpu/drm/drm_drv.c  |  12 +
 drivers/gpu/drm/drmfs.c| 555 +
 drivers/gpu/drm/i915/i915_guc_submission.c |  33 +-
 include/drm/drm_drv.h  |   3 +
 include/drm/drmfs.h|  77 
 include/uapi/linux/magic.h |   3 +
 8 files changed, 672 insertions(+), 21 deletions(-)
 create mode 100644 drivers/gpu/drm/drmfs.c
 create mode 100644 include/drm/drmfs.h

-- 
2.7.4

[PATCH 4/4] drm/i915: Implement Link Rate fallback on Link training failure

[Manasi Navare]

If link training at a link rate optimal for a particular
mode fails during modeset's atomic commit phase, then we
let the modeset complete and then retry. We save the link rate
value at which link training failed, update the link status property
to "BAD" and use a lower link rate to prune the modes. It will redo
the modeset on the current mode at lower link rate or if the current
mode gets pruned due to lower link constraints then, it will send a
hotplug uevent for userspace to handle it.

This is also required to pass DP CTS tests 4.3.1.3, 4.3.1.4,
4.3.1.6.

v9:
* Use the trimmed max values of link rate/lane count based on
link train fallback (Daniel Vetter)
v8:
* Set link_status to BAD first and then call mode_valid (Jani Nikula)
v7:
Remove the redundant variable in previous patch itself
v6:
* Obtain link rate index from fallback_link_rate using
the helper intel_dp_link_rate_index (Jani Nikula)
* Include fallback within intel_dp_start_link_train (Jani Nikula)
v5:
* Move set link status to drm core (Daniel Vetter, Jani Nikula)
v4:
* Add fallback support for non DDI platforms too
* Set connector->link status inside set_link_status function
(Jani Nikula)
v3:
* Set link status property to BAd unconditionally (Jani Nikula)
* Dont use two separate variables link_train_failed and link_status
to indicate same thing (Jani Nikula)
v2:
* Squashed a few patches (Jani Nikula)

Acked-by: Tony Cheng 
Acked-by: Harry Wentland 
Cc: Jani Nikula 
Cc: Daniel Vetter 
Cc: Ville Syrjala 
Signed-off-by: Manasi Navare 
---
 drivers/gpu/drm/i915/intel_dp.c   | 27 +++
 drivers/gpu/drm/i915/intel_dp_link_training.c | 22 --
 drivers/gpu/drm/i915/intel_drv.h  |  3 +++
 3 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b5c7526f..404da32 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5676,6 +5676,29 @@ static bool intel_edp_init_connector(struct intel_dp 
*intel_dp,
return false;
 }

+static void intel_dp_modeset_retry_work_fn(struct work_struct *work)
+{
+   struct intel_connector *intel_connector;
+   struct drm_connector *connector;
+
+   intel_connector = container_of(work, typeof(*intel_connector),
+  modeset_retry_work);
+   connector = _connector->base;
+   DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id,
+ connector->name);
+
+   /* Grab the locks before changing connector property*/
+   mutex_lock(>dev->mode_config.mutex);
+   /* Set connector link status to BAD and send a Uevent to notify
+* userspace to do a modeset.
+*/
+   drm_mode_connector_set_link_status_property(connector,
+   DRM_MODE_LINK_STATUS_BAD);
+   mutex_unlock(>dev->mode_config.mutex);
+   /* Send Hotplug uevent so userspace can reprobe */
+   drm_kms_helper_hotplug_event(connector->dev);
+}
+
 bool
 intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
struct intel_connector *intel_connector)
@@ -5688,6 +5711,10 @@ static bool intel_edp_init_connector(struct intel_dp 
*intel_dp,
enum port port = intel_dig_port->port;
int type;

+   /* Initialize the work for modeset in case of link train failure */
+   INIT_WORK(_connector->modeset_retry_work,
+ intel_dp_modeset_retry_work_fn);
+
if (WARN(intel_dig_port->max_lanes < 1,
 "Not enough lanes (%d) for DP on port %c\n",
 intel_dig_port->max_lanes, port_name(port)))
diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c 
b/drivers/gpu/drm/i915/intel_dp_link_training.c
index 0048b52..955b239 100644
--- a/drivers/gpu/drm/i915/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
@@ -313,6 +313,24 @@ void intel_dp_stop_link_train(struct intel_dp *intel_dp)
 void
 intel_dp_start_link_train(struct intel_dp *intel_dp)
 {
-   intel_dp_link_training_clock_recovery(intel_dp);
-   intel_dp_link_training_channel_equalization(intel_dp);
+   struct intel_connector *intel_connector = intel_dp->attached_connector;
+
+   if (!intel_dp_link_training_clock_recovery(intel_dp))
+   goto failure_handling;
+   if (!intel_dp_link_training_channel_equalization(intel_dp))
+   goto failure_handling;
+
+   DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane count = %d",
+ intel_dp->link_rate, intel_dp->lane_count);
+   return;
+
+ failure_handling:
+   DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane count = %d",
+ intel_dp->link_rate, intel_dp->lane_count);
+   if (!intel_dp_get_link_train_fallback_values(intel_dp,
+intel_dp->link_rate,
+   

[PATCH 3/4] drm/i915: Find fallback link rate/lane count

[Manasi Navare]

If link training fails, then we need to fallback to lower
link rate first and if link training fails at RBR, then
fallback to lower lane count.
This function finds the next lower link rate/lane count
value after link training failure and limits the max
link_rate and lane_count values to these fallback values.

v6:
* Cap the max link rate and lane count to the max
values obtained during fallback link training (Daniel Vetter)
v5:
* Start the fallback at the lane count value passed not
the max lane count (Jani Nikula)
v4:
* Remove the redundant variable link_train_failed
v3:
* Remove fallback_link_rate_index variable, just obtain
that using the helper intel_dp_link_rate_index (Jani Nikula)
v2:
Squash the patch that returns the link rate index (Jani Nikula)

Acked-by: Tony Cheng 
Acked-by: Harry Wentland 
Cc: Ville Syrjala 
Cc: Jani Nikula 
Cc: Daniel Vetter 
Signed-off-by: Manasi Navare 
---
 drivers/gpu/drm/i915/intel_dp.c  | 40 
 drivers/gpu/drm/i915/intel_drv.h |  2 ++
 2 files changed, 42 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 434dc7d..b5c7526f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -278,6 +278,46 @@ static int intel_dp_common_rates(struct intel_dp *intel_dp,
   common_rates);
 }

+static int intel_dp_link_rate_index(struct intel_dp *intel_dp,
+   int *common_rates, int link_rate)
+{
+   int common_len;
+   int index;
+
+   common_len = intel_dp_common_rates(intel_dp, common_rates);
+   for (index = 0; index < common_len; index++) {
+   if (link_rate == common_rates[common_len - index - 1])
+   return common_len - index - 1;
+   }
+
+   return -1;
+}
+
+int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
+   int link_rate, uint8_t lane_count)
+{
+   int common_rates[DP_MAX_SUPPORTED_RATES] = {};
+   int common_len;
+   int link_rate_index = -1;
+
+   common_len = intel_dp_common_rates(intel_dp, common_rates);
+   link_rate_index = intel_dp_link_rate_index(intel_dp,
+  common_rates,
+  link_rate);
+   if (link_rate_index > 0) {
+   intel_dp->max_sink_link_bw = 
drm_dp_link_rate_to_bw_code(common_rates[link_rate_index - 1]);
+   intel_dp->max_sink_lane_count = lane_count;
+   } else if (lane_count > 1) {
+   intel_dp->max_sink_link_bw = intel_dp_max_link_bw(intel_dp);
+   intel_dp->max_sink_lane_count = lane_count >> 1;
+   } else {
+   DRM_ERROR("Link Training Unsuccessful\n");
+   return -1;
+   }
+
+   return 0;
+}
+
 static enum drm_mode_status
 intel_dp_mode_valid(struct drm_connector *connector,
struct drm_display_mode *mode)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index b6526ad..47e3671 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1400,6 +1400,8 @@ bool intel_dp_init_connector(struct intel_digital_port 
*intel_dig_port,
 void intel_dp_set_link_params(struct intel_dp *intel_dp,
  int link_rate, uint8_t lane_count,
  bool link_mst);
+int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
+   int link_rate, uint8_t lane_count);
 void intel_dp_start_link_train(struct intel_dp *intel_dp);
 void intel_dp_stop_link_train(struct intel_dp *intel_dp);
 void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode);
-- 
1.9.1

[PATCH 2/4] drm/i915: Compute sink's max lane count/link BW at Hotplug

[Manasi Navare]

Sink's capabilities are advertised through DPCD registers and get
updated only on hotplug. So they should be computed only once in the
long pulse handler and saved off in intel_dp structure for the use
later. For this reason two new fields max_sink_lane_count and
max_sink_link_bw are added to intel_dp structure.

This also simplifies the fallback link rate/lane count logic
to handle link training failure. In that case, the max_sink_link_bw
and max_sink_lane_count can be reccomputed to match the fallback
values lowering the sink capabilities due to link train failure.

Cc: Ville Syrjala 
Cc: Jani Nikula 
Cc: Daniel Vetter 
Signed-off-by: Manasi Navare 
---
 drivers/gpu/drm/i915/intel_dp.c  | 10 --
 drivers/gpu/drm/i915/intel_drv.h |  4 
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index db75bb9..434dc7d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -156,7 +156,7 @@ static u8 intel_dp_max_lane_count(struct intel_dp *intel_dp)
u8 source_max, sink_max;

source_max = intel_dig_port->max_lanes;
-   sink_max = drm_dp_max_lane_count(intel_dp->dpcd);
+   sink_max = intel_dp->max_sink_lane_count;

return min(source_max, sink_max);
 }
@@ -213,7 +213,7 @@ static u8 intel_dp_max_lane_count(struct intel_dp *intel_dp)

*sink_rates = default_rates;

-   return (intel_dp_max_link_bw(intel_dp) >> 3) + 1;
+   return (intel_dp->max_sink_link_bw >> 3) + 1;
 }

 static int
@@ -4395,6 +4395,12 @@ static bool intel_digital_port_connected(struct 
drm_i915_private *dev_priv,
  yesno(intel_dp_source_supports_hbr2(intel_dp)),
  yesno(drm_dp_tps3_supported(intel_dp->dpcd)));

+   /* Set the max lane count for sink */
+   intel_dp->max_sink_lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
+
+   /* Set the max link BW for sink */
+   intel_dp->max_sink_link_bw = intel_dp_max_link_bw(intel_dp);
+
intel_dp_print_rates(intel_dp);

intel_dp_read_desc(intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index fd77a3b..b6526ad 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -906,6 +906,10 @@ struct intel_dp {
/* sink rates as reported by DP_SUPPORTED_LINK_RATES */
uint8_t num_sink_rates;
int sink_rates[DP_MAX_SUPPORTED_RATES];
+   /* Max lane count for the sink as per DPCD registers */
+   uint8_t max_sink_lane_count;
+   /* Max link BW for the sink as per DPCD registers */
+   int max_sink_link_bw;
/* sink or branch descriptor */
struct intel_dp_desc desc;
struct drm_dp_aux aux;
-- 
1.9.1

[PATCH 1/4] drm: Add a new connector atomic property for link status

[Manasi Navare]

At the time userspace does setcrtc, we've already promised the mode
would work. The promise is based on the theoretical capabilities of
the link, but it's possible we can't reach this in practice. The DP
spec describes how the link should be reduced, but we can't reduce
the link below the requirements of the mode. Black screen follows.

One idea would be to have setcrtc return a failure. However, it
already should not fail as the atomic checks have passed. It would
also conflict with the idea of making setcrtc asynchronous in the
future, returning before the actual mode setting and link training.

Another idea is to train the link "upfront" at hotplug time, before
pruning the mode list, so that we can do the pruning based on
practical not theoretical capabilities. However, the changes for link
training are pretty drastic, all for the sake of error handling and
DP compliance, when the most common happy day scenario is the current
approach of link training at mode setting time, using the optimal
parameters for the mode. It is also not certain all hardware could do
this without the pipe on; not even all our hardware can do this. Some
of this can be solved, but not trivially.

Both of the above ideas also fail to address link degradation *during*
operation.

The solution is to add a new "link-status" connector property in order
to address link training failure in a way that:
a) changes the current happy day scenario as little as possible, to
avoid regressions, b) can be implemented the same way by all drm
drivers, c) is still opt-in for the drivers and userspace, and opting
out doesn't regress the user experience, d) doesn't prevent drivers
from implementing better or alternate approaches, possibly without
userspace involvement. And, of course, handles all the issues presented.
In the usual happy day scenario, this is always "good". If something
fails during or after a mode set, the kernel driver can set the link
status to "bad" and issue a hotplug uevent for userspace to have it
re-check the valid modes through GET_CONNECTOR IOCTL, and try modeset
again. If the theoretical capabilities of the link can't be reached,
the mode list is trimmed based on that.

v4:
* Add comments in kernel-doc format (Daniel Vetter)
* Update the kernel-doc for link-status (Sean Paul)
v3:
* Fixed a build error (Jani Saarinen)
v2:
* Removed connector->link_status (Daniel Vetter)
* Set connector->state->link_status in 
drm_mode_connector_set_link_status_property
(Daniel Vetter)
* Set the connector_changed flag to true if connector->state->link_status 
changed.
* Reset link_status to GOOD in update_output_state (Daniel Vetter)
* Never allow userspace to set link status from Good To Bad (Daniel Vetter)

Acked-by: Tony Cheng 
Acked-by: Harry Wentland 
Cc: Jani Nikula 
Cc: Daniel Vetter 
Cc: Ville Syrjala 
Cc: Chris Wilson 
Cc: Sean Paul 
Signed-off-by: Manasi Navare 
---
 drivers/gpu/drm/drm_atomic.c| 10 +++
 drivers/gpu/drm/drm_atomic_helper.c | 15 +++
 drivers/gpu/drm/drm_connector.c | 53 +
 include/drm/drm_connector.h | 19 +
 include/drm/drm_mode_config.h   |  5 
 include/uapi/drm/drm_mode.h |  4 +++
 6 files changed, 106 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 19d7bcb..70a5152 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1087,6 +1087,14 @@ int drm_atomic_connector_set_property(struct 
drm_connector *connector,
 * now?) atomic writes to DPMS property:
 */
return -EINVAL;
+   } else if (property == config->link_status_property) {
+   /* Never downgrade from GOOD to BAD on userspace's request here,
+* only hw issues can do that.
+*/
+   if (state->link_status == DRM_LINK_STATUS_GOOD)
+   return 0;
+   state->link_status = val;
+   return 0;
} else if (connector->funcs->atomic_set_property) {
return connector->funcs->atomic_set_property(connector,
state, property, val);
@@ -1135,6 +1143,8 @@ static void drm_atomic_connector_print_state(struct 
drm_printer *p,
*val = (state->crtc) ? state->crtc->base.id : 0;
} else if (property == config->dpms_property) {
*val = connector->dpms;
+   } else if (property == config->link_status_property) {
+   *val = state->link_status;
} else if (connector->funcs->atomic_get_property) {
return connector->funcs->atomic_get_property(connector,
state, property, val);
diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 494680c..3d6c1ce 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -519,6 +519,13 @@ static int 

[PATCH 0/4] Link Training failure handling by sending Hotplug Uevent

[Manasi Navare]

This patch series is a final revision of previous patch series for handling
link training failure. This has been polished in terms of the kernel 
documentation
based on review feedback.

One more patch is added to move sink's max link rate and lane count
to intel_dp and compute it at hotplug and only if link training fails else
keept it constant. This greatly simplifies the link train fallback logic
by changing the max link rate/lane count to the fallback values instead
of creating separate fallback values for link rate/lane count.

In general the idea is to if link training fails during or after atomic modeset,
then the driver sets the link-status property to BAD and sends a HOTPLUG
uevent to notify userspace that link status is BAD. The userspace can be 
modified
to be "link-status" property aware and on detecting link-status as BAD, it 
should
revalidate the modes and redo a modeset which in turn will retrain the link
at lower fallback values.

This has been validated and is proved to pass DP compliance uisng DPR-120.

Manasi Navare (4):
  drm: Add a new connector atomic property for link status
  drm/i915: Compute sink's max lane count/link BW at Hotplug
  drm/i915: Find fallback link rate/lane count
  drm/i915: Implement Link Rate fallback on Link training failure

 drivers/gpu/drm/drm_atomic.c  | 10 
 drivers/gpu/drm/drm_atomic_helper.c   | 15 ++
 drivers/gpu/drm/drm_connector.c   | 53 ++
 drivers/gpu/drm/i915/intel_dp.c   | 77 ++-
 drivers/gpu/drm/i915/intel_dp_link_training.c | 22 +++-
 drivers/gpu/drm/i915/intel_drv.h  |  9 
 include/drm/drm_connector.h   | 19 +++
 include/drm/drm_mode_config.h |  5 ++
 include/uapi/drm/drm_mode.h   |  4 ++
 9 files changed, 210 insertions(+), 4 deletions(-)

-- 
1.9.1

[PATCH 2/2] drm/sti: do not post HQVDP command if no update

[Fabien Dessenne]

Do not process update requests with unmodified parameters.

Since the HQVDP command queue is limited to 2, we shall take care of
not posting unneeded commands, which would abusively fill the command
queue leading to frame update skip.
This typically happens when the driver is called with legacy
(non-atomic) IOCTL : in that case atomic_update() is called multiple
times with the same parameters.

Signed-off-by: Fabien Dessenne 
---
 drivers/gpu/drm/sti/sti_hqvdp.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c
index a547723..55cbaea 100644
--- a/drivers/gpu/drm/sti/sti_hqvdp.c
+++ b/drivers/gpu/drm/sti/sti_hqvdp.c
@@ -1117,6 +1117,21 @@ static void sti_hqvdp_atomic_update(struct drm_plane 
*drm_plane,
if (!crtc || !fb)
return;

+   if ((oldstate->fb == state->fb) &&
+   (oldstate->crtc_x == state->crtc_x) &&
+   (oldstate->crtc_y == state->crtc_y) &&
+   (oldstate->crtc_w == state->crtc_w) &&
+   (oldstate->crtc_h == state->crtc_h) &&
+   (oldstate->src_x == state->src_x) &&
+   (oldstate->src_y == state->src_y) &&
+   (oldstate->src_w == state->src_w) &&
+   (oldstate->src_h == state->src_h)) {
+   /* No change since last update, do not post cmd */
+   DRM_DEBUG_DRIVER("No change, not posting cmd\n");
+   plane->status = STI_PLANE_UPDATED;
+   return;
+   }
+
mode = >mode;
dst_x = state->crtc_x;
dst_y = state->crtc_y;
-- 
2.7.4

[PATCH 1/2] drm/sti: load XP70 firmware only once

[Fabien Dessenne]

When a plane is enabled, after having been disabled, do not reload XP70
firmware again, but only register VTG again

Signed-off-by: Fabien Dessenne 
---
 drivers/gpu/drm/sti/sti_hqvdp.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c
index f88130f..a547723 100644
--- a/drivers/gpu/drm/sti/sti_hqvdp.c
+++ b/drivers/gpu/drm/sti/sti_hqvdp.c
@@ -332,6 +332,7 @@ struct sti_hqvdp_cmd {
  * @hqvdp_cmd_paddr:   physical address of hqvdp_cmd
  * @vtg:   vtg for main data path
  * @xp70_initialized:  true if xp70 is already initialized
+ * @vtg_registered:true if registered to VTG
  */
 struct sti_hqvdp {
struct device *dev;
@@ -347,6 +348,7 @@ struct sti_hqvdp {
u32 hqvdp_cmd_paddr;
struct sti_vtg *vtg;
bool xp70_initialized;
+   bool vtg_registered;
 };

 #define to_sti_hqvdp(x) container_of(x, struct sti_hqvdp, plane)
@@ -771,7 +773,7 @@ static void sti_hqvdp_disable(struct sti_hqvdp *hqvdp)
DRM_ERROR("XP70 could not revert to idle\n");

hqvdp->plane.status = STI_PLANE_DISABLED;
-   hqvdp->xp70_initialized = false;
+   hqvdp->vtg_registered = false;
 }

 /**
@@ -1064,10 +1066,11 @@ static int sti_hqvdp_atomic_check(struct drm_plane 
*drm_plane,
return -EINVAL;
}

-   if (!hqvdp->xp70_initialized) {
+   if (!hqvdp->xp70_initialized)
/* Start HQVDP XP70 coprocessor */
sti_hqvdp_start_xp70(hqvdp);

+   if (!hqvdp->vtg_registered) {
/* Prevent VTG shutdown */
if (clk_prepare_enable(hqvdp->clk_pix_main)) {
DRM_ERROR("Failed to prepare/enable pix main clk\n");
@@ -1081,6 +1084,7 @@ static int sti_hqvdp_atomic_check(struct drm_plane 
*drm_plane,
DRM_ERROR("Cannot register VTG notifier\n");
return -EINVAL;
}
+   hqvdp->vtg_registered = true;
}

DRM_DEBUG_KMS("CRTC:%d (%s) drm plane:%d (%s)\n",
-- 
2.7.4

[PATCH 0/2] drm/sti: HQVDP plane improvements

[Fabien Dessenne]

These patches fix HQVDP plane performance issues
- [PATCH 1/2] drm/sti: load XP70 firmware only once
- [PATCH 2/2] drm/sti: do not post HQVDP command if no update
Fabien

[Bug 98987] [RS690] BUG: soft lockup when radeon modesetting

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

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

--- Comment #7 from Václav Ovsík  ---
Bing - HDMI audio is probably a source of problem.

I tried to add radeon.audio=0. I tested this before already, but I tried this
time again with 4.9-rc7. No bug in console appeared, but kernel lock down
(--B don't worked). I tried I think 3-times - no OOPS on console,
only hang. Then I get idea to look into BIOS if HDMI audio is enabled there and
really, HDMI was disabled completely. I enable HDMI in the BIOS and disable
HDMI audio only  and system started normaly. I tried combinations:

BIOSBIOSradeon.audio=0
HDMIHDMI audio

0   -   no  OOPS
0   -   yes OOPS
yes yes no  OOPS
yes yes yes OOPS
yes no  no  OK

So to avoid problems with the radeon driver the HDMI audio must be disabled in
BIOS and it is possible only if the HDMI connector is enabled, because
otherwise the option (HDMI audio) is greyed.
I can't remember if I had this setting in the past, maybe yes. Disabled HDMI
may be the result of BIOS "Load system default", because other flaws on this
mobo with networking some time ago (.

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

[PATCH v2 2/3] drm/edid: Implement SCDC support detection

[Ville Syrjälä]

On Mon, Dec 05, 2016 at 12:11:46PM +0100, Thierry Reding wrote:
> On Mon, Dec 05, 2016 at 09:16:27AM +0100, Daniel Vetter wrote:
> > On Mon, Dec 05, 2016 at 08:57:43AM +0100, Thierry Reding wrote:
> > > On Sat, Dec 03, 2016 at 04:35:24AM +, Sharma, Shashank wrote:
> > > > Hi Thierry, 
> > > > 
> > > > If you can please have a look on this patch, I had written one to parse 
> > > > HF-VSDB, which was covering SCDC detection too. 
> > > > https://patchwork.kernel.org/patch/9452259/ 
> > > 
> > > I think there had been pushback before about caching capabilities from
> > > EDID, so from that point of view my patch is more inline with existing
> > > EDID parsing API.
> > 
> > Hm, where was that pushback? We do have a bit a mess between explicitly
> > parsing stuff (e.g. eld) and stuffing parsed data into drm_display_info.
> 
> You did object to a very similar patch some time ago that did a similar
> thing for DPCD stuff. And also Villa had commented on an earlier patch
> from Jose about concerns of bloating core structures:
> 
>   https://patchwork.freedesktop.org/patch/104806/

Yeah, the same old "adding stuff all over without actual users" story.
It's near impossible to judge whether the added things are actually
useful (or if the implementation is the best) without seeing how it's
going to be used.

-- 
Ville Syrjälä
Intel OTC

[PATCH v2 1/3] drm: Add SCDC helpers

[Ville Syrjälä]

On Mon, Dec 05, 2016 at 12:16:52PM +0100, Thierry Reding wrote:
> On Mon, Dec 05, 2016 at 10:12:39AM +, Jose Abreu wrote:
> > On 02-12-2016 19:24, Thierry Reding wrote:
> [...]
> > > +/**
> > > + * drm_scdc_write - write a block of data to SCDC
> > > + * @adapter: I2C controller
> > > + * @offset: start offset of block to write
> > > + * @buffer: block of data to write
> > > + * @size: size of the block to write
> > > + *
> > > + * Writes a block of data to SCDC, starting at a given offset.
> > > + *
> > > + * Returns:
> > > + * The number of bytes written to SCDC or a negative error code on 
> > > failure.
> > > + */
> > > +ssize_t drm_scdc_write(struct i2c_adapter *adapter, u8 offset,
> > > +const void *buffer, size_t size)
> > > +{
> > > + struct i2c_msg msg = {
> > > + .addr = SCDC_I2C_SLAVE_ADDRESS,
> > > + .flags = 0,
> > > + .len = 1 + size,
> > > + .buf = NULL,
> > > + };
> > > + void *data;
> > > + int err;
> > > +
> > > + data = kmalloc(1 + size, GFP_TEMPORARY);
> > > + if (!data)
> > > + return -ENOMEM;
> > > +
> > > + msg.buf = data;
> > > +
> > > + memcpy(data, , sizeof(offset));
> > > + memcpy(data + 1, buffer, size);
> > 
> > Don't you agree it would be better if you use the same scheme as
> > drm_scdc_read()? Something like:
> > 
> > struct i2c_msg msgs[] = {
> > {
> > .addr = SCDC_I2C_SLAVE_ADDRESS,
> > .flags = 0,
> > .len = 1,
> > .buf = ,
> > }, {
> > .addr = SCDC_I2C_SLAVE_ADDRESS,
> > .flags = 0,
> > .len = size,
> > .buf = buffer,
> > },
> > };
> 
> Ville had a similar comment on a prior iteration. It looks as if the
> above should work, but it's probably best to test it a little more
> widely to make sure we're not running into cases where it breaks.

AFAICS the spec says we shouldn't use a repeated start for writes.
I guess it might work for some devices, but going against the spec
sounds questionable to me.

> 
> Have you by any chance verified that it works on your hardware?
> 
> Thierry



-- 
Ville Syrjälä
Intel OTC

[PATCH 0/8] Host1x IOMMU support + VIC support

[Mikko Perttunen]

Bump

On 10.11.2016 20:23, Mikko Perttunen wrote:
> This series adds IOMMU support to Host1x and TegraDRM
> and adds support for the VIC host1x client so that
> host1x can be tested on modern Tegra platforms.
> It depends on the previous fix series. The whole thing
> (modulo patch order) is available as a git repository at
> git://github.com/cyndis/linux.git; branch vic-v1.
>
> IO memory management is organized such that there are
> two domains: the host1x domain and the tegradrm domain.
> The host1x domain is used by the host1x engine and
> contains the host1x CDMA and pushbuffers for submitted
> jobs.
>
> The tegradrm domain is shared by all host1x units and
> contains GEM objects and memory allocated by the
> separate tegra_drm_alloc function. This function is
> currently used to allocate space for firmware blobs
> in the tegradrm domain.
>
> A userspace test case for VIC can be found at
> https://github.com/cyndis/drm/tree/work/tegra.
> The testcase is in tests/tegra and is called submit_vic.
> The in-kernel firewall is not implemented for VIC;
> therefore, IOMMU must be enabled for the test to pass.
>
> Tested with Jetson TX1 (T210). Probably works also
> with Jetson TK1 (T124). Note that due to hardware changes
> the testcase also needs to be changed to run properly
> on T124.
>
> Thanks,
>   Mikko.
>
> Arto Merilainen (2):
>   drm/tegra: Add falcon helper library
>   drm/tegra: Add VIC support
>
> Mikko Perttunen (6):
>   drm/tegra: Add Tegra DRM allocation API
>   drm/tegra: Allocate BOs from lower 4G when without IOMMU
>   gpu: host1x: Add IOMMU support
>   dt-bindings: Add bindings for the Tegra VIC
>   arm64: tegra: Enable VIC on T210
>   arm64: tegra: Enable IOMMU for Host1x on Tegra210
>
>  .../display/tegra/nvidia,tegra20-host1x.txt|  13 +
>  arch/arm64/boot/dts/nvidia/tegra210.dtsi   |  19 +-
>  drivers/gpu/drm/tegra/Makefile |   4 +-
>  drivers/gpu/drm/tegra/drm.c| 101 +-
>  drivers/gpu/drm/tegra/drm.h|   8 +
>  drivers/gpu/drm/tegra/falcon.c | 256 +
>  drivers/gpu/drm/tegra/falcon.h | 130 +++
>  drivers/gpu/drm/tegra/gem.c|   2 +-
>  drivers/gpu/drm/tegra/vic.c| 400 
> +
>  drivers/gpu/drm/tegra/vic.h|  31 ++
>  drivers/gpu/host1x/cdma.c  |  65 +++-
>  drivers/gpu/host1x/cdma.h  |   4 +-
>  drivers/gpu/host1x/dev.c   |  39 +-
>  drivers/gpu/host1x/dev.h   |   5 +
>  drivers/gpu/host1x/hw/cdma_hw.c|  10 +-
>  drivers/gpu/host1x/job.c   |  76 +++-
>  drivers/gpu/host1x/job.h   |   1 +
>  include/linux/host1x.h |   1 +
>  18 files changed, 1128 insertions(+), 37 deletions(-)
>  create mode 100644 drivers/gpu/drm/tegra/falcon.c
>  create mode 100644 drivers/gpu/drm/tegra/falcon.h
>  create mode 100644 drivers/gpu/drm/tegra/vic.c
>  create mode 100644 drivers/gpu/drm/tegra/vic.h
>

[PATCH] drm/amdgpu: don't add files at control minor debugfs directory

[Alex Deucher]

Since commit 8a357d10043c ("drm: Nerf DRM_CONTROL nodes"), a
struct drm_device's ->control member is always NULL.

In the case of CONFIG_DEBUG_FS=y, amdgpu_debugfs_add_files() accesses
->control->debugfs_root though. This results in the following Oops:

[9.627636] BUG: unable to handle kernel NULL pointer dereference at 
0018
[9.628274] IP: amdgpu_debugfs_add_files+0x8d/0x100 [amdgpu]
[9.628867] PGD 2514c7067
[9.628876] PUD 2514c8067
[9.629448] PMD 0
[9.630026] Oops:  [#1] PREEMPT SMP
[...]
[9.639906] Call Trace:
[9.640538]  amdgpu_fence_driver_init+0x1e/0x40 [amdgpu]
[9.641186]  amdgpu_device_init+0xa6d/0x1410 [amdgpu]
[9.641900]  ? kmalloc_order_trace+0x2e/0x100
[9.642587]  amdgpu_driver_load_kms+0x5b/0x200 [amdgpu]
[9.643355]  drm_dev_register+0xa7/0xd0
[9.644016]  drm_get_pci_dev+0xde/0x1d0
[9.644659]  amdgpu_pci_probe+0xbe/0xf0 [amdgpu]
[...]

Fix this by omitting the drm_debugfs_create_files() call for the
control minor debugfs directory which is now non-existent anyway.

Port of Nicolai Stange's radeon patch to amdgpu.

bug: https://bugs.freedesktop.org/show_bug.cgi?id=98915
Fixes: 8a357d10043c ("drm: Nerf DRM_CONTROL nodes")
Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 4701d94..a578540 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2512,9 +2512,6 @@ int amdgpu_debugfs_add_files(struct amdgpu_device *adev,
adev->debugfs_count = i;
 #if defined(CONFIG_DEBUG_FS)
drm_debugfs_create_files(files, nfiles,
-adev->ddev->control->debugfs_root,
-adev->ddev->control);
-   drm_debugfs_create_files(files, nfiles,
 adev->ddev->primary->debugfs_root,
 adev->ddev->primary);
 #endif
@@ -2529,9 +2526,6 @@ static void amdgpu_debugfs_remove_files(struct 
amdgpu_device *adev)
for (i = 0; i < adev->debugfs_count; i++) {
drm_debugfs_remove_files(adev->debugfs[i].files,
 adev->debugfs[i].num_files,
-adev->ddev->control);
-   drm_debugfs_remove_files(adev->debugfs[i].files,
-adev->debugfs[i].num_files,
 adev->ddev->primary);
}
 #endif
-- 
2.5.5

[PATCH v3 1/2] ARM: dts: da850-lcdk: add the dumb-vga-dac node

[Tomi Valkeinen]

On 29/11/16 13:57, Bartosz Golaszewski wrote:
> Add the dumb-vga-dac node to the board DT together with corresponding
> ports and vga connector. This allows to retrieve the edid info from
> the display automatically.
> 
> Signed-off-by: Bartosz Golaszewski 
> ---
>  arch/arm/boot/dts/da850-lcdk.dts | 58 
> 
>  arch/arm/boot/dts/da850.dtsi | 17 
>  2 files changed, 75 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/da850-lcdk.dts 
> b/arch/arm/boot/dts/da850-lcdk.dts
> index 711b9ad..d864f11 100644
> --- a/arch/arm/boot/dts/da850-lcdk.dts
> +++ b/arch/arm/boot/dts/da850-lcdk.dts
> @@ -50,6 +50,53 @@
>   system-clock-frequency = <24576000>;
>   };
>   };
> +
> + vga_bridge {
> + compatible = "dumb-vga-dac";
> + pinctrl-names = "default";
> + pinctrl-0 = <_pins>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port at 0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0>;
> +
> + vga_bridge_in: endpoint at 0 {
> + reg = <0>;
> + remote-endpoint = <_out_vga>;
> + };
> + };
> +
> + port at 1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <1>;
> +
> + vga_bridge_out: endpoint at 0 {
> + reg = <0>;
> + remote-endpoint = <_con_in>;
> + };
> + };
> + };
> + };
> +
> + vga {
> + compatible = "vga-connector";
> +
> + ddc-i2c-bus = <>;
> +
> + port {
> + vga_con_in: endpoint {
> + remote-endpoint = <_bridge_out>;
> + };
> + };
> + };
>  };
>  
>  _core {
> @@ -235,3 +282,14 @@
>   {
>   status = "okay";
>  };
> +
> + {
> + status = "okay";
> +};
> +
> +_out {
> + display_out_vga: endpoint at 0 {
> + reg = <0>;
> + remote-endpoint = <_bridge_in>;
> + };
> +};
> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> index 4070619..5f4ba2e 100644
> --- a/arch/arm/boot/dts/da850.dtsi
> +++ b/arch/arm/boot/dts/da850.dtsi
> @@ -454,6 +454,23 @@
>   reg = <0x213000 0x1000>;
>   interrupts = <52>;
>   status = "disabled";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + display_in: port at 0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0>;
> + };
> +
> + display_out: port at 1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <1>;
> + };
> + };
>   };

It's a bit difficult to follow this as there's been so many patches
going around. But I take the above is the LCDC node in the base da850
dtsi file? In that case, what is the display_in supposed to present?
It's the first node in the "display chain", so it has no input.

Also, don't touch da850.dtsi here, just add the "ports" node in the
da850-lcdk.dts file.

If the da850.dtsi has not been merged yet, I'd change the name of the
lcdc node to something else than "display". It's rather vague. If it's
named "lcdc", reading da850-lcdk.dts becomes much easier, as you'll
refer to "lcdc".

 Tomi

-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161205/8a7836a6/attachment-0001.sig>

[PATCH 13/22] drm: bridge: dw-hdmi: Replace device type with platform quirks

[Laurent Pinchart]

Hi Jose,

On Monday 05 Dec 2016 12:31:30 Jose Abreu wrote:
> On 05-12-2016 11:32, Laurent Pinchart wrote:
> > On Monday 05 Dec 2016 10:50:19 Jose Abreu wrote:
> >> On 02-12-2016 15:43, Laurent Pinchart wrote:
> >>> On Friday 02 Dec 2016 14:24:01 Russell King - ARM Linux wrote:
>  On Fri, Dec 02, 2016 at 01:43:28AM +0200, Laurent Pinchart wrote:
> > From: Kieran Bingham 
> > 
> > The dw-hdmi driver declares a dev_type to distinguish platform
> > specific changes. Replace this with a quirk field, so that the
> > platform can specify the required quirks for the driver, rather than
> > the driver becoming conditional on multiple platforms.
> > 
> > As part of this, we rename the dw-hdmi 'spare' which is defined as the
> > SVSRET bit in later documentation.
>  
>  I'd really prefer that we did not go down the broken route of adding
>  a set of "quirk" flags - look at what a mess SDHCI has become through
>  allowing that kind of practice.
>  
>  I'd much rather we find a saner structure to this - and we know that
>  the hardware has ID registers in it which can be used (so far) to
>  identify the buggy hardware.
> >>> 
> >>> I'd much prefer something that would allow runtime identification of the
> >>> device and the corresponding actions to be taken. However, the amount of
> >>> documentation we have on the DWC HDMI TX IP core (and the associated
> >>> PHY) is pretty limited, given that Synopsys doesn't make the
> >>> documentation available publicly. Changes made to the IP core by
> >>> integrators could complicate this further. I'm trying to gather as much
> >>> information as possible to make clean the code up, for instance by
> >>> trying to identify the PHYs used on the various platforms we support.
> >>> Progress is slow on that front, there isn't enough leaked information
> >>> available online :-) I haven't given up though, but I'll need more time.
> >>> 
> >>> I don't like quirks much either. They are however already used today,
> >>> even if we trigger them through dev_type instead of quirk flags. This
> >>> patch came from a previous version found in a BSP that simply sprinkled
> >>> several if (hdmi-> dev_type == RCAR_HDMI) through the code. For
> >>> instance,
> >>> 
> >>> - if (hdmi->dev_type == RK3288_HDMI)
> >>> + if (hdmi->dev_type == RK3288_HDMI || hdmi->dev_type == RCAR_HDMI)
> >>>   dw_hdmi_phy_enable_spare(hdmi, 1);
> >>> 
> >>> which I think is worse than flags as it would quickly degenerate to
> >>> spaghetti code.
> >>> 
> >>> For this specific case, we've managed to identify that on Renesas
> >>> platforms the bit set by this function is called SVSRET. Its usage isn't
> >>> clear yet, but I suspect it to control one of the PHY input control
> >>> signals, like the other bits in the same register. I'm trying to get
> >>> more information to clean the implementation further, hopefully with a
> >>> way to determine whether the signal is used based on PHY identification.
> >> 
> >> SVSRET is a low power mode consumption and is a PHY input signal
> >> as you suggested.
> > 
> > Thank you for the confirmation. Would you happen to know what SVSRET
> > stands for ?
> 
> Have no info about that. Sorry.
> 
> >> Most of the configurable input signals of the PHY are available by the
> >> controller regbank. I don't think it is possible to detect this at
> >> runtime, I think you have at least to hardcode which version of the PHY
> >> you are using.
> >> 
> >> I would suggest that maybe all the PHY logic should be extracted and then
> >> use callbacks to glue controller and phy. Then, depending on the PHY you
> >> could use empty stubs if, for example, a given PHY did not support
> >> SVSRET. Still, I don't know if this is the best option. What I do know is
> >> that there are a large number of PHY's with different flavors that can
> >> use the same controller. The controller has different versions also, and
> >> each version can have quirks but I think it would be easier to manage
> >> this driver if we had a clear distinction between PHY and controller.
> > 
> > Agreed, I'd like to go in that direction. What makes it quite difficult is
> > the lack of documentation about the PHYs :-) I've found six different PHY
> > types that can be identified by the CONFIG2_ID register:
> > 
> > Bits| Field | Description
> > --
> > 7-0 | phytype   | PHY interface
> > |   | 0x00: Legacy PHY (HDMI TX PHY)
> > |   | 0xb2: MHL PHY + HEAC PHY
> > |   | 0xc2: MHL PHY
> > |   | 0xe2: HDMI 3D TX PHY + HEAC PHY
> > |   | 0xf2: HDMI 3D TX PHY
> > |   | 0xf3: HDMI2 TX PHY
> > 
> > I'm sure there's more than that. In particular I wonder how external
> > vendor 

[PATCH 1/2] pwm: lpss: Make builtin and add lookup-table so that i915 can find the pwm_backlight

[Hans de Goede]

Hi,

On 05-12-16 11:59, Thierry Reding wrote:
> On Mon, Dec 05, 2016 at 09:18:03AM +0100, Hans de Goede wrote:
>> Hi,
>>
>> On 05-12-16 08:46, Thierry Reding wrote:
>>> On Fri, Dec 02, 2016 at 11:17:35AM +0100, Hans de Goede wrote:
 The primary consumer of the lpss pwm is the i915 kms driver, but
 currently that driver cannot get the pwm because i915 platforms are
 not using devicetree and pwm-lpss does not call pwm_add_table.

 Another problem is that i915 does not support get_pwm returning
 -EPROBE_DEFER and i915's init is very complex and this is almost
 impossible to fix.

 This commit changes the PWM_LPSS Kconfig from a tristate to a bool, so
 that when the i915 driver loads the lpss pwm will be available avoiding
 the -EPROBE_DEFER issue. Note that this is identical to how the same
 problem was solved for the pwm-crc driver.

 Being builtin also allows calling pwm_add_table directly from the
 pwm-lpss code, otherwise the pwm_add_table call would need to be put
 somewhere else to ensure it happens before i915 calls pwm_get,
 even if i915 would support -EPROBE_DEFER.

 Signed-off-by: Hans de Goede 
 ---
  drivers/pwm/Kconfig| 12 +++-
  drivers/pwm/pwm-lpss.c | 11 +++
  2 files changed, 14 insertions(+), 9 deletions(-)
>>>
>>> This is completely backwards. You're putting board-specific bits into a
>>> generic driver.
>>
>> This is not really board specific I'm advertising that the goal of
>> the pwm is to be used to control a backlight.
>
> pwm_add_table() is a board-specific API. Documentation/pwm.txt says that
> "board setup code" should be using pwm_add_table(). Using it from within
> the provider is completely the opposite.

The problem here really is that there is no such thing as
board setup code on x86 + EFI/ACPI, that is supposedly all
handled by the EFI/ACPI code there.

>> Before typing this reply I've been thinking about another place
>> to put the pwm_add_table call put I cannot come up with any.
>
> I suggested drivers/platform/x86. A bunch of code in there is doing
> exactly the kind of board/platform setup stuff that you're trying to do
> here.

All drivers under drivers/platform/x86 bind to something, be it
an ACPI interface or an actual platform device. In the case of the
pwm-lpss we have an actual platform or pci device and a driver binding
to it, that is the only common code path I see where I can add the
pwm_add_table.

Sure I can put a built-in bit of code under drivers/platform/x86
which checks from its module_init() that there is an pwm-lpss controller
present (either listed under ACPI or through PCI) and then calls
pwm_add_table, but seems silly. Note as said this then must be
built-in, because if it is a module nothing will trigger the
loading of the module, unless I add duplicate MODULE_DEVICE_TABLE
tables in there with the code under drivers/pwm/pwm-lpss.

TL;DR: the problem is that something needs to trigger / activate
the code doing the pwm_add_table() and AFAICT we have no other
trigger then the presence of the pwm-lpss device, at which point
the pwm_lpss_probe function becomes the best place to do the
pwm_add_table call.

Regards,

Hans





>
>>
>> drivers/acpi/acpi_lpss.c comes to mind, but that would only work
>> with the pwm device in acpi mode and not with it in pci mode.
>>
>> Another candidate would be to put the pwm_add_table call in the
>> i915 driver itself, when the vbt says we need to use an external
>> pwm and the plaform is cherrytrail, but it does not know if the
>> pwm device is in pci or acpi modes which requires a different
>> provider entry in the table.
>
> i915 isn't a good location for that either. This should really be driven
> by code external to any generic drivers. It's exactly the kind of glue
> that platform or board setup code is used for.
>
> If you look at drivers/platform/x86/intel_oaktrail.c, it does very
> similar things. If this is specific to Cherry Trail, I'm sure you can
> find ways to identify the platform as such and drive it in a similar way
> from drivers/platform/x86/intel_cherrytrail.c.
>
>> Besides that having the pwm in the table will not do anything
>> unless the i915 driver does a pwm_get, so this does not gain us
>> anything.
>
> It will keep the driver generic and put the board code where it belongs.
> I call that very much of a gain.
>
>> Maybe we need to rename the con_id from "pwm_backlight" to
>> "pwm_lpss0", that way it is very clear which pwm any pwm_get calls
>> are getting (and lpss-pwm.c is obviously the correct place to
>> do the add_table), and then we can teach the i915 code to look
>> for "pwm_lpss0" based on vbt info ?
>
> pwm_backlight is the much better consumer name because by your own words
> that's exactly what the PWM is used for. Obfuscating this by turning the
> name into something unrecognizable such as pwm_lpss0 isn't going to
> change any of the above.
>
> Thierry
>

[PATCH v2 3/3] drm/edid: Implement SCDC Read Request capability detection

[Jose Abreu]

Hi Thierry,


On 05-12-2016 11:14, Thierry Reding wrote:
> On Mon, Dec 05, 2016 at 11:06:15AM +, Jose Abreu wrote:
>> Hi Thierry,
>>
>>
>> Do you think while you are at it you could implement a
>> set_scrambling() callback? It should be pretty straight forward:
>> you read the SCDC_TMDS_CONFIG reg, do a mask, and then write it
>> again.
>>
>>
>> I think this is an important feature that we should have.
> Yeah, agreed. I was actually thinking about going one step further and
> provide more of the polling functionality as a helper. Even if we have
> accessors that wrap the low-level functionality, most drivers would
> still have to provide their own delayed workqueue to deal with sinks
> (or sources) that don't support read requests. Having this in standard
> helpers would help reduce the boilerplate a lot further.
>
> Does your hardware by any chance support read requests on SCDC? It'd be
> interesting to see how that works in practice. Unfortunately Tegra does
> not seem to support it.
>
> Thierry

Yes, my HW supports it though I don't have the setup here to test
right now (but it was used before). In our controller it is
pretty straight forward:
1) Enable interrupt for rr indication [source]
2) Enable update read on a rr [source]
3) Set rr flag in the sink [sink]

Point 2) makes it clear: Whenever we get a rr then the source
will automatically read the sink and dump the contents read in
the regbank. Then the SW gets an interrupt and it should read the
contents of the registers.

>
>> On 02-12-2016 19:24, Thierry Reding wrote:
>>> From: Thierry Reding 
>>>
>>> Sinks that support SCDC can optionally have the capability to initiate
>>> read requests, which are a mechanism by which a sink can notify its
>>> source that it should read the Update Flags. If either the sink or the
>>> source are not Read Request capable, polling of the Update Flags shall
>>> be employed.
>>>
>>> Signed-off-by: Thierry Reding 
>>> ---
>>> Changes in v2:
>>> - new patch
>>>
>>>  drivers/gpu/drm/drm_edid.c | 36 
>>>  include/drm/drm_edid.h |  1 +
>>>  2 files changed, 37 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>>> index 369961597ee5..8211cce3e09e 100644
>>> --- a/drivers/gpu/drm/drm_edid.c
>>> +++ b/drivers/gpu/drm/drm_edid.c
>>> @@ -3736,6 +3736,42 @@ bool drm_detect_hdmi_scdc(struct edid *edid)
>>>  EXPORT_SYMBOL(drm_detect_hdmi_scdc);
>>>  
>>>  /**
>>> + * drm_detect_hdmi_scdc_rr_capable - detect whether an HDMI sink is 
>>> capable of
>>> + *initiating an SCDC Read Request
>>> + * @edid: sink EDID information
>>> + *
>>> + * Parse the CEA extension according to CEA-861-B to find an HF-VSDB as
>>> + * defined in HDMI 2.0, section 10.3.2 "HDMI Forum Vendor Specific Data
>>> + * Block" and checks if the RR_Capable bit (bit 6 of byte 6) is set.
>>> + *
>>> + * Returns:
>>> + * True if the sink is capable of initiating an SCDC Read Request, false
>>> + * otherwise.
>>> + */
>>> +bool drm_detect_hdmi_scdc_rr_capable(struct edid *edid)
>>> +{
>>> +   unsigned int start, end, i;
>>> +   const u8 *cea;
>>> +
>>> +   cea = drm_find_cea_extension(edid);
>>> +   if (!cea)
>>> +   return false;
>>> +
>>> +   if (cea_db_offsets(cea, , ))
>>> +   return false;
>>> +
>>> +   for_each_cea_db(cea, i, start, end) {
>>> +   if (cea_db_is_hdmi_forum_vsdb([i])) {
>>> +   if (cea[i + 6] & 0x40)
>>> +   return true;
>>> +   }
>>> +   }
>>> +
>>> +   return false;
>>> +}
>>> +EXPORT_SYMBOL(drm_detect_hdmi_scdc_rr_capable);
>>> +
>>> +/**
>>>   * drm_detect_monitor_audio - check monitor audio capability
>>>   * @edid: EDID block to scan
>>>   *
>>> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
>>> index 7ea7e90846d8..d1c29586035e 100644
>>> --- a/include/drm/drm_edid.h
>>> +++ b/include/drm/drm_edid.h
>>> @@ -441,6 +441,7 @@ u8 drm_match_cea_mode(const struct drm_display_mode 
>>> *to_match);
>>>  enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code);
>>>  bool drm_detect_hdmi_monitor(struct edid *edid);
>>>  bool drm_detect_hdmi_scdc(struct edid *edid);
>>> +bool drm_detect_hdmi_scdc_rr_capable(struct edid *edid);
>>>  bool drm_detect_monitor_audio(struct edid *edid);
>>>  bool drm_rgb_quant_range_selectable(struct edid *edid);
>>>  int drm_add_modes_noedid(struct drm_connector *connector,

Best regards,
Jose Miguel Abreu

[PATCH v2 1/3] drm: Add SCDC helpers

[Jose Abreu]

Hi,


On 05-12-2016 13:31, Ville Syrjälä wrote:
> On Mon, Dec 05, 2016 at 12:16:52PM +0100, Thierry Reding wrote:
>> On Mon, Dec 05, 2016 at 10:12:39AM +, Jose Abreu wrote:
>>> On 02-12-2016 19:24, Thierry Reding wrote:
>> [...]
 +/**
 + * drm_scdc_write - write a block of data to SCDC
 + * @adapter: I2C controller
 + * @offset: start offset of block to write
 + * @buffer: block of data to write
 + * @size: size of the block to write
 + *
 + * Writes a block of data to SCDC, starting at a given offset.
 + *
 + * Returns:
 + * The number of bytes written to SCDC or a negative error code on 
 failure.
 + */
 +ssize_t drm_scdc_write(struct i2c_adapter *adapter, u8 offset,
 + const void *buffer, size_t size)
 +{
 +  struct i2c_msg msg = {
 +  .addr = SCDC_I2C_SLAVE_ADDRESS,
 +  .flags = 0,
 +  .len = 1 + size,
 +  .buf = NULL,
 +  };
 +  void *data;
 +  int err;
 +
 +  data = kmalloc(1 + size, GFP_TEMPORARY);
 +  if (!data)
 +  return -ENOMEM;
 +
 +  msg.buf = data;
 +
 +  memcpy(data, , sizeof(offset));
 +  memcpy(data + 1, buffer, size);
>>> Don't you agree it would be better if you use the same scheme as
>>> drm_scdc_read()? Something like:
>>>
>>> struct i2c_msg msgs[] = {
>>> {
>>> .addr = SCDC_I2C_SLAVE_ADDRESS,
>>> .flags = 0,
>>> .len = 1,
>>> .buf = ,
>>> }, {
>>> .addr = SCDC_I2C_SLAVE_ADDRESS,
>>> .flags = 0,
>>> .len = size,
>>> .buf = buffer,
>>> },
>>> };
>> Ville had a similar comment on a prior iteration. It looks as if the
>> above should work, but it's probably best to test it a little more
>> widely to make sure we're not running into cases where it breaks.
> AFAICS the spec says we shouldn't use a repeated start for writes.
> I guess it might work for some devices, but going against the spec
> sounds questionable to me.
>
>> Have you by any chance verified that it works on your hardware?
>>
>> Thierry

Actually, I do not do two writes. My I2C controller is accessible
through the HDMI controller and has direct mapping in the regbank
of the slave address, reg address and data. I am using a I2C
adapter driver but it was modified to when the slave address is
SCDC and we are doing the first write then store the data and
only send it in the next write. Something like this:
http://lxr.free-electrons.com/source/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/anx9805.c#L43
(but modified to SCDC).

Anyway, I was not remembering this so just disregard my comment.
We should follow the spec as Ville said.

>
>

Best regards,
Jose Miguel Abreu

[PATCH v2] drm: allow changing DPMS mode

[Marta Lofstedt]

The drm_atomic_helper_connector_dpms
will set the connector back the old DPMS state
before returning. This makes it impossible to change
DPMS state of the connector.

Fixes: 0853695c3ba46f97dfc0b5885f7b7e640ca212dd
v2: edit of commit message
Cc: Chris Wilson 
Cc: Daniel Vetter 
Cc: Eric Engestrom 
Cc: Sean Paul 
Cc: dri-devel at lists.freedesktop.org
Cc: 
Signed-off-by: Marta Lofstedt 
---
 drivers/gpu/drm/drm_atomic_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 494680c..6a5acb9 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2885,8 +2885,8 @@ int drm_atomic_helper_connector_dpms(struct drm_connector 
*connector,
 fail:
if (ret == -EDEADLK)
goto backoff;
-
-   connector->dpms = old_mode;
+   if (ret != 0)
+   connector->dpms = old_mode;
drm_atomic_state_put(state);
return ret;

-- 
2.9.3

[PATCH 13/22] drm: bridge: dw-hdmi: Replace device type with platform quirks

[Laurent Pinchart]

Hello,

On Friday 02 Dec 2016 18:45:46 Laurent Pinchart wrote:
> On Friday 02 Dec 2016 16:08:17 Russell King - ARM Linux wrote:
> > On Fri, Dec 02, 2016 at 05:43:43PM +0200, Laurent Pinchart wrote:
> >> DW_HDMI_QUIRK_FC_INVIDCONF is indeed a bad name, I'll fix that.
> >> 
> >> Do you know why this function needs to write to the HDMI_FC_INVIDCONF
> >> register  four times in the normal case, and once only for IMX6DL ?
> > 
> > (I don't have much time at present, I'm buried in ARM64 crud trying to
> > get a platform to boot, and working out how to debug an ARM64 platform
> > that even earlycon doesn't work on... no printascii() types of easy
> > debug facilities on ARM64 make this job several orders of magnitude
> > harder than it needs to be...)
> 
> Thanks all the more for taking time to reply.
> 
> > It prevents a magenta line down the left hand side of the screen, which
> > is caused when the frame composer in the HDMI Tx gets confused -
> > according to the errata, it does a load of maths when you write to the
> > frame composer registers, and sometimes it doesn't update properly.
> > 
> > So, the four writes (and the number is critical) is there to persuade
> > the IP to do the maths with the right values so the internal timings
> > are correct.
> > 
> > The rather confusing thing is - it's actually IMX6Q which has the more
> > severe errata, not IMX6DL - the workaround of four writes is applied
> > in the 6Q case.
> > 
> > It's covered by ERR004308 in the IMX6Q Errata document (search for
> > IMX6DQCE).  It isn't mentioned in the IMX6DL documentation, but it
> > seems that similar workaround of one write is necessary there.
> > 
> > Some of this was determined by experimentation in conjunction with the
> > errata documentation - I remember it took a while to get it right so
> > that we didn't ever see the magenta line.
> 
> That's interesting. I'll test the different options on my Renesas platform
> (no write, one write, four writes) and see what is needed. Could anyone
> perform the same tests on a Rockchip RK3288 system ?

Daniel Stone has been nice enough to test HDMI output without the errata 
workaround on RK3288 and hasn't noticed any issue. I've performed the same 
test on R-Car H3 and haven't noticed any issue either.

The i.MX6DL and i.MX6Q use a DWC HDMI TX version 1.31a and 1.30a respectively, 
while RK3288 and R-Car H3 uses v2.00a and v2.01a. We could enable the 
workaround based on the HDMI TX version.

-- 
Regards,

Laurent Pinchart

[Bug 98357] [amdgpu] Topaz fails to boot with error in powerplay initialization

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

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

Nayan Deshmukh  changed:

   What|Removed |Added

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

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

[Bug 98505] [radeon, amdgpu] Regression introduced in 4.8-rc3

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

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

Nayan Deshmukh  changed:

   What|Removed |Added

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

--- Comment #36 from Nayan Deshmukh  ---
The bug is fixed in 4.9-rc7. Thanks for the effort Peter.

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

[PATCH 13/22] drm: bridge: dw-hdmi: Replace device type with platform quirks

[Laurent Pinchart]

Hi Jose,

On Monday 05 Dec 2016 10:50:19 Jose Abreu wrote:
> On 02-12-2016 15:43, Laurent Pinchart wrote:
> > On Friday 02 Dec 2016 14:24:01 Russell King - ARM Linux wrote:
> >> On Fri, Dec 02, 2016 at 01:43:28AM +0200, Laurent Pinchart wrote:
> >>> From: Kieran Bingham 
> >>> 
> >>> The dw-hdmi driver declares a dev_type to distinguish platform specific
> >>> changes. Replace this with a quirk field, so that the platform can
> >>> specify the required quirks for the driver, rather than the driver
> >>> becoming conditional on multiple platforms.
> >>> 
> >>> As part of this, we rename the dw-hdmi 'spare' which is defined as the
> >>> SVSRET bit in later documentation.
> >> 
> >> I'd really prefer that we did not go down the broken route of adding
> >> a set of "quirk" flags - look at what a mess SDHCI has become through
> >> allowing that kind of practice.
> >> 
> >> I'd much rather we find a saner structure to this - and we know that
> >> the hardware has ID registers in it which can be used (so far) to
> >> identify the buggy hardware.
> > 
> > I'd much prefer something that would allow runtime identification of the
> > device and the corresponding actions to be taken. However, the amount of
> > documentation we have on the DWC HDMI TX IP core (and the associated PHY)
> > is pretty limited, given that Synopsys doesn't make the documentation
> > available publicly. Changes made to the IP core by integrators could
> > complicate this further. I'm trying to gather as much information as
> > possible to make clean the code up, for instance by trying to identify
> > the PHYs used on the various platforms we support. Progress is slow on
> > that front, there isn't enough leaked information available online :-) I
> > haven't given up though, but I'll need more time.
> > 
> > I don't like quirks much either. They are however already used today, even
> > if we trigger them through dev_type instead of quirk flags. This patch
> > came from a previous version found in a BSP that simply sprinkled several
> > if (hdmi-> dev_type == RCAR_HDMI) through the code. For instance,
> > 
> > -   if (hdmi->dev_type == RK3288_HDMI)
> > +   if (hdmi->dev_type == RK3288_HDMI || hdmi->dev_type == RCAR_HDMI)
> > dw_hdmi_phy_enable_spare(hdmi, 1);
> > 
> > which I think is worse than flags as it would quickly degenerate to
> > spaghetti code.
> > 
> > For this specific case, we've managed to identify that on Renesas
> > platforms the bit set by this function is called SVSRET. Its usage isn't
> > clear yet, but I suspect it to control one of the PHY input control
> > signals, like the other bits in the same register. I'm trying to get more
> > information to clean the implementation further, hopefully with a way to
> > determine whether the signal is used based on PHY identification.
> 
> SVSRET is a low power mode consumption and is a PHY input signal
> as you suggested.

Thank you for the confirmation. Would you happen to know what SVSRET stands 
for ?

> Most of the configurable input signals of the PHY are available by the
> controller regbank. I don't think it is possible to detect this at runtime,
> I think you have at least to hardcode which version of the PHY you are
> using.
>
> I would suggest that maybe all the PHY logic should be extracted and then
> use callbacks to glue controller and phy. Then, depending on the PHY you
> could use empty stubs if, for example, a given PHY did not support SVSRET.
> Still, I don't know if this is the best option. What I do know is that there
> are a large number of PHY's with different flavors that can use the same
> controller. The controller has different versions also, and each version can
> have quirks but I think it would be easier to manage this driver if we had a
> clear distinction between PHY and controller.

Agreed, I'd like to go in that direction. What makes it quite difficult is the 
lack of documentation about the PHYs :-) I've found six different PHY types 
that can be identified by the CONFIG2_ID register:

Bits| Field | Description
--
7-0 | phytype   | PHY interface
|   | 0x00: Legacy PHY (HDMI TX PHY)
|   | 0xb2: MHL PHY + HEAC PHY
|   | 0xc2: MHL PHY
|   | 0xe2: HDMI 3D TX PHY + HEAC PHY
|   | 0xf2: HDMI 3D TX PHY
|   | 0xf3: HDMI2 TX PHY

I'm sure there's more than that. In particular I wonder how external vendor 
PHYs are identified.

I'm also wondering whether there's a need to keep support for the legacy PHY 
signals (ENTMDS and PDZ in the PHY_CONF0 register). As far as I understand 
they're not used by the Gen2 PHYs (including the external vendor PHYs), but I 
can't confirm that without more documentation (although I could test that 

[PATCH v4 2/2] drm/bridge: adv7511: Initialize regulators

[Archit Taneja]

Maintain a table of regulator names expect by ADV7511 and ADV7533.
Use regulator_bulk_* api to configure these.

Initialize and enable the regulators during probe itself. Controlling
these dynamically is left for later.

Reviewed-by: Laurent Pinchart 
Signed-off-by: Archit Taneja 
---
 drivers/gpu/drm/bridge/adv7511/adv7511.h |  4 ++
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 80 
 2 files changed, 75 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h 
b/drivers/gpu/drm/bridge/adv7511/adv7511.h
index 992d76c..039147f 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 

 #include 
 #include 
@@ -329,6 +330,9 @@ struct adv7511 {

struct gpio_desc *gpio_pd;

+   struct regulator_bulk_data *supplies;
+   unsigned int num_supplies;
+
/* ADV7533 DSI RX related params */
struct device_node *host_node;
struct mipi_dsi_device *dsi;
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c 
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 2177440..b5e61be 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -843,6 +843,52 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge)
  * Probe & remove
  */

+static const char * const adv7511_supply_names[] = {
+   "avdd",
+   "v3p3",
+};
+
+static const char * const adv7533_supply_names[] = {
+   "avdd",
+   "v3p3",
+   "v1p2",
+};
+
+static int adv7511_init_regulators(struct adv7511 *adv)
+{
+   struct device *dev = >i2c_main->dev;
+   const char * const *supply_names;
+   unsigned int i;
+   int ret;
+
+   if (adv->type == ADV7511) {
+   supply_names = adv7511_supply_names;
+   adv->num_supplies = ARRAY_SIZE(adv7511_supply_names);
+   } else {
+   supply_names = adv7533_supply_names;
+   adv->num_supplies = ARRAY_SIZE(adv7533_supply_names);
+   }
+
+   adv->supplies = devm_kcalloc(dev, adv->num_supplies,
+sizeof(*adv->supplies), GFP_KERNEL);
+   if (!adv->supplies)
+   return -ENOMEM;
+
+   for (i = 0; i < adv->num_supplies; i++)
+   adv->supplies[i].supply = supply_names[i];
+
+   ret = devm_regulator_bulk_get(dev, adv->num_supplies, adv->supplies);
+   if (ret)
+   return ret;
+
+   return regulator_bulk_enable(adv->num_supplies, adv->supplies);
+}
+
+static void adv7511_uninit_regulators(struct adv7511 *adv)
+{
+   regulator_bulk_disable(adv->num_supplies, adv->supplies);
+}
+
 static int adv7511_parse_dt(struct device_node *np,
struct adv7511_link_config *config)
 {
@@ -943,6 +989,7 @@ static int adv7511_probe(struct i2c_client *i2c, const 
struct i2c_device_id *id)
if (!adv7511)
return -ENOMEM;

+   adv7511->i2c_main = i2c;
adv7511->powered = false;
adv7511->status = connector_status_disconnected;

@@ -960,13 +1007,21 @@ static int adv7511_probe(struct i2c_client *i2c, const 
struct i2c_device_id *id)
if (ret)
return ret;

+   ret = adv7511_init_regulators(adv7511);
+   if (ret) {
+   dev_err(dev, "failed to init regulators\n");
+   return ret;
+   }
+
/*
 * The power down GPIO is optional. If present, toggle it from active to
 * inactive to wake up the encoder.
 */
adv7511->gpio_pd = devm_gpiod_get_optional(dev, "pd", GPIOD_OUT_HIGH);
-   if (IS_ERR(adv7511->gpio_pd))
-   return PTR_ERR(adv7511->gpio_pd);
+   if (IS_ERR(adv7511->gpio_pd)) {
+   ret = PTR_ERR(adv7511->gpio_pd);
+   goto uninit_regulators;
+   }

if (adv7511->gpio_pd) {
mdelay(5);
@@ -974,12 +1029,14 @@ static int adv7511_probe(struct i2c_client *i2c, const 
struct i2c_device_id *id)
}

adv7511->regmap = devm_regmap_init_i2c(i2c, _regmap_config);
-   if (IS_ERR(adv7511->regmap))
-   return PTR_ERR(adv7511->regmap);
+   if (IS_ERR(adv7511->regmap)) {
+   ret = PTR_ERR(adv7511->regmap);
+   goto uninit_regulators;
+   }

ret = regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, );
if (ret)
-   return ret;
+   goto uninit_regulators;
dev_dbg(dev, "Rev. %d\n", val);

if (adv7511->type == ADV7511)
@@ -989,7 +1046,7 @@ static int adv7511_probe(struct i2c_client *i2c, const 
struct i2c_device_id *id)
else
ret = adv7533_patch_registers(adv7511);
if (ret)
-   return ret;
+   goto uninit_regulators;

regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, edid_i2c_addr);

[PATCH v4 1/2] dt-bindings: drm/bridge: adv7511: Add regulator bindings

[Archit Taneja]

Add the regulator supply properties needed by ADV7511 and ADV7533.

Cc: devicetree at vger.kernel.org
Acked-by: Laurent Pinchart 
Signed-off-by: Archit Taneja 
---
 Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt | 8 
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt 
b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
index 6532a59..00ce479 100644
--- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
+++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
@@ -38,10 +38,18 @@ The following input format properties are required except 
in "rgb 1x" and
 - adi,input-justification: The input bit justification ("left", "evenly",
   "right").

+- avdd-supply: A common 1.8V supply that powers up the AVDD, DVDD and PVDD
+  pins. On ADV7511, it also feeds to the BGVDD pin. On ADV7533, it also powers
+  up the A2VDD pin.
+- v3p3-supply: A 3.3V supply that powers up the pin called DVDD_3V on
+  ADV7511 and V3P3 on ADV7533.
+
 The following properties are required for ADV7533:

 - adi,dsi-lanes: Number of DSI data lanes connected to the DSI host. It should
   be one of 1, 2, 3 or 4.
+- v1p2-supply: A supply that powers up the V1P2 pin on the chip. It can be
+  either 1.2V or 1.8V. This supply is specific to ADV7533.

 Optional properties:

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

[PATCH v4 0/2] drm/bridge: adv7511: Enable regulators

[Archit Taneja]

The patch below added regulator supplies to the ADV533 HDMI bridge
on the DB410c, but the corresponding DT binding doc wasn't updated
to list them:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=28546b09551190c727c94d1c5c96ca609065beb2

The first patch adds the regulator supply props to the binding
docs for both ADV7511 and ADV7533. The second patch updates the drm
bridge driver to enable these regulators.

Changes in v4:
- Moved the regulator supply bindings from optional to mandatory.
- Used unsigned int for num_supplies and iterator used for parsing
the regulator supply properties.

Changes in v3:
- Revert back to having a common supply for AVDD, DVDD, PVDD pins.
- Dropped the DB410c DT patches.

Changes in v2:
- Provide the regulator supply for ADV7511 too in the binding docs.
- Update the driver to manage regulators for both ADV7511 and ADV7533.
- Have separate supply entries for AVDD, DVDD, PVDD, A2VDD pins.
- Use regulator_bulk_* API to configure regulators.


Archit Taneja (2):
  dt-bindings: drm/bridge: adv7511: Add regulator bindings
  drm/bridge: adv7511: Initialize regulators

 .../bindings/display/bridge/adi,adv7511.txt|  8 +++
 drivers/gpu/drm/bridge/adv7511/adv7511.h   |  4 ++
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   | 80 +++---
 3 files changed, 83 insertions(+), 9 deletions(-)

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

[PATCH 1/1] gpu: drm: qxl: fix use of uninitialized variable

[Sean Paul]

On Sat, Dec 3, 2016 at 10:11 AM, Pan Bian  wrote:
> In function qxl_release_alloc(), when kmalloc() returns a NULL pointer,
> it returns value 0 and parameter *ret is uninitialized. 0 means no error
> to the callers of qxl_release_alloc(). The callers keep going and will
> try to reference the uninitialized variable. This patch fixes the bug,
> returning "-ENOMEM" when kmalloc() fails.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=188911
>
> Signed-off-by: Pan Bian 

Applied to drm-misc, with subject prefix tweak s_gpu: drm: qxl_drm/qxl_

Thanks!

Sean

> ---
>  drivers/gpu/drm/qxl/qxl_release.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_release.c 
> b/drivers/gpu/drm/qxl/qxl_release.c
> index cd83f05..e6daa70 100644
> --- a/drivers/gpu/drm/qxl/qxl_release.c
> +++ b/drivers/gpu/drm/qxl/qxl_release.c
> @@ -133,7 +133,7 @@ static long qxl_fence_wait(struct fence *fence, bool 
> intr, signed long timeout)
> release = kmalloc(size, GFP_KERNEL);
> if (!release) {
> DRM_ERROR("Out of memory\n");
> -   return 0;
> +   return -ENOMEM;
> }
> release->base.ops = NULL;
> release->type = type;
> --
> 1.9.1
>
>

[PATCH v3 1/2] dt-bindings: drm/bridge: adv7511: Add regulator bindings

[Bjorn Andersson]

On Tue 29 Nov 01:11 PST 2016, Laurent Pinchart wrote:

> Hi Archit,
> 
> (CC'ing Mark Brown)
> 
> On Tuesday 29 Nov 2016 13:41:33 Archit Taneja wrote:
> > On 11/29/2016 12:03 PM, Laurent Pinchart wrote:
> > > On Tuesday 29 Nov 2016 11:37:41 Archit Taneja wrote:
> > >> Add the regulator supply properties needed by ADV7511 and ADV7533.
> > >> 
> > >> The regulators are specified as optional properties since there can
> > >> be boards which have a fixed supply directly routed to the pins, and
> > >> these may not be modelled as regulator supplies.
> > > 
> > > That's why we have support for dummy supplies in the kernel, isn't it ?
> > > Isn't it better to make the supplies mandatory in the bindings (and
> > > obviously handling them as optional in the driver for
> > > backward-compatibility) ?
> >
> > I'm a bit unclear on this.
> > 
> > I thought we couldn't add mandatory properties once the device is already
> > present in DT for one or more platforms.
> 
> You can, as long as you treat them as optional in the driver to retain 
> backward compatibility. The DT bindings should document the properties 
> expected from a new platform (older versions of the bindings will always be 
> available in the git history).

If you document them as required and don't do anything special in the
implementation (i.e. just call devm_regulator_get() as usual) it will
just work, in the absence of the property you will get a dummy regulator
from the framework.

And then add the fixed-voltage regulators to the new DT to make that
properly describe the hardware.

> 
> > Say, if we do make it mandatory for future additions, we would need to have
> > DT property for the supplies for the new platforms. If the regulators on
> > these boards are fixed supplies, they would be need to be modeled
> > using "regulator-fixed", possibly without any input supply. Is that
> > what you're suggesting?
> 
> That's the idea, yes. Clock maintainers have a similar opinion regarding the 
> clock bindings, where a clock that is not optional at the hardware level 
> should be specified in DT even if it's always present.
> 

Further more, a DT binding for a particular block should describe that
block; so if we have three different 1.8V pins then the DT binding
should reflect this - even if our current platform have them wired to
the same regulator.

(And the supply names would preferably be based on the pin names in the
component data sheet)

Regards,
Bjorn

Enabling peer to peer device transactions for PCIe devices

[Logan Gunthorpe]

On 05/12/16 12:46 PM, Jason Gunthorpe wrote:
> NVMe might have to deal with pci-e hot-unplug, which is a similar
> problem-class to the GPU case..

Sure, but if the NVMe device gets hot-unplugged it means that all the 
CMB mappings are useless and need to be torn down. This probably means 
killing any process that has mappings open.

> In any event the allocator still needs to track which regions are in
> use and be able to hook 'free' from userspace. That does suggest it
> should be integrated into the nvme driver and not a bolt on driver..

Yup, that's correct. And yes, I've never suggested this to be a bolt on 
driver -- I always expected for it to get integrated into the nvme 
driver. (iopmem was not meant for this.)

Logan

Enabling peer to peer device transactions for PCIe devices

[Jason Gunthorpe]

On Mon, Dec 05, 2016 at 12:27:20PM -0700, Logan Gunthorpe wrote:
> 
> 
> On 05/12/16 12:14 PM, Jason Gunthorpe wrote:
> >But CMB sounds much more like the GPU case where there is a
> >specialized allocator handing out the BAR to consumers, so I'm not
> >sure a general purpose chardev makes a lot of sense?
> 
> I don't think it will ever need to be as complicated as the GPU case. There
> will probably only ever be a relatively small amount of memory behind the
> CMB and really the only users are those doing P2P work. Thus the specialized
> allocator could be pretty simple and I expect it would be fine to just
> return -ENOMEM if there is not enough memory.

NVMe might have to deal with pci-e hot-unplug, which is a similar
problem-class to the GPU case..

In any event the allocator still needs to track which regions are in
use and be able to hook 'free' from userspace. That does suggest it
should be integrated into the nvme driver and not a bolt on driver..

Jason

[Intel-gfx] [PATCH 1/3] drm: Add a new connector atomic property for link status

[Manasi Navare]

On Fri, Dec 02, 2016 at 05:26:35PM +0100, Daniel Vetter wrote:
> On Tue, Nov 29, 2016 at 11:30:31PM -0800, Manasi Navare wrote:
> > At the time userspace does setcrtc, we've already promised the mode
> > would work. The promise is based on the theoretical capabilities of
> > the link, but it's possible we can't reach this in practice. The DP
> > spec describes how the link should be reduced, but we can't reduce
> > the link below the requirements of the mode. Black screen follows.
> > 
> > One idea would be to have setcrtc return a failure. However, it
> > already should not fail as the atomic checks have passed. It would
> > also conflict with the idea of making setcrtc asynchronous in the
> > future, returning before the actual mode setting and link training.
> > 
> > Another idea is to train the link "upfront" at hotplug time, before
> > pruning the mode list, so that we can do the pruning based on
> > practical not theoretical capabilities. However, the changes for link
> > training are pretty drastic, all for the sake of error handling and
> > DP compliance, when the most common happy day scenario is the current
> > approach of link training at mode setting time, using the optimal
> > parameters for the mode. It is also not certain all hardware could do
> > this without the pipe on; not even all our hardware can do this. Some
> > of this can be solved, but not trivially.
> > 
> > Both of the above ideas also fail to address link degradation *during*
> > operation.
> > 
> > The solution is to add a new "link-status" connector property in order
> > to address link training failure in a way that:
> > a) changes the current happy day scenario as little as possible, to
> > avoid regressions, b) can be implemented the same way by all drm
> > drivers, c) is still opt-in for the drivers and userspace, and opting
> > out doesn't regress the user experience, d) doesn't prevent drivers
> > from implementing better or alternate approaches, possibly without
> > userspace involvement. And, of course, handles all the issues presented.
> > In the usual happy day scenario, this is always "good". If something
> > fails during or after a mode set, the kernel driver can set the link
> > status to "bad" and issue a hotplug uevent for userspace to have it
> > re-check the valid modes through GET_CONNECTOR IOCTL, and try modeset
> > again. If the theoretical capabilities of the link can't be reached,
> > the mode list is trimmed based on that.
> > 
> > v4:
> > * Add comments in kernel-doc format (Daniel Vetter)
> > * Update the kernel-doc for link-status (Sean Paul)
> > v3:
> > * Fixed a build error (Jani Saarinen)
> > v2:
> > * Removed connector->link_status (Daniel Vetter)
> > * Set connector->state->link_status in 
> > drm_mode_connector_set_link_status_property
> > (Daniel Vetter)
> > * Set the connector_changed flag to true if connector->state->link_status 
> > changed.
> > * Reset link_status to GOOD in update_output_state (Daniel Vetter)
> > * Never allow userspace to set link status from Good To Bad (Daniel Vetter)
> > 
> > Acked-by: Tony Cheng 
> > Acked-by: Harry Wentland 
> > Cc: Jani Nikula 
> > Cc: Daniel Vetter 
> > Cc: Ville Syrjala 
> > Cc: Chris Wilson 
> > Cc: Sean Paul 
> > Signed-off-by: Manasi Navare 
> > ---
> >  drivers/gpu/drm/drm_atomic.c| 10 +++
> >  drivers/gpu/drm/drm_atomic_helper.c |  8 ++
> >  drivers/gpu/drm/drm_connector.c | 54 
> > -
> >  include/drm/drm_connector.h | 19 +
> >  include/drm/drm_mode_config.h   |  5 
> >  include/uapi/drm/drm_mode.h |  4 +++
> >  6 files changed, 99 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 89737e4..990f013 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -1087,6 +1087,14 @@ int drm_atomic_connector_set_property(struct 
> > drm_connector *connector,
> >  * now?) atomic writes to DPMS property:
> >  */
> > return -EINVAL;
> > +   } else if (property == config->link_status_property) {
> > +   /* Never downgrade from GOOD to BAD on userspace's request here,
> > +* only hw issues can do that.
> > +*/
> > +   if (state->link_status == DRM_LINK_STATUS_GOOD)
> > +   return 0;
> > +   state->link_status = val;
> > +   return 0;
> > } else if (connector->funcs->atomic_set_property) {
> > return connector->funcs->atomic_set_property(connector,
> > state, property, val);
> > @@ -1135,6 +1143,8 @@ static void drm_atomic_connector_print_state(struct 
> > drm_printer *p,
> > *val = (state->crtc) ? state->crtc->base.id : 0;
> > } else if (property == config->dpms_property) {
> > *val = connector->dpms;
> > +   } else if (property == config->link_status_property) {
> > +   *val = 

[PATCH 13/22] drm: bridge: dw-hdmi: Replace device type with platform quirks

[Jose Abreu]

Hi Laurent,


On 05-12-2016 11:32, Laurent Pinchart wrote:
> Hi Jose,
>
> On Monday 05 Dec 2016 10:50:19 Jose Abreu wrote:
>> On 02-12-2016 15:43, Laurent Pinchart wrote:
>>> On Friday 02 Dec 2016 14:24:01 Russell King - ARM Linux wrote:
 On Fri, Dec 02, 2016 at 01:43:28AM +0200, Laurent Pinchart wrote:
> From: Kieran Bingham 
>
> The dw-hdmi driver declares a dev_type to distinguish platform specific
> changes. Replace this with a quirk field, so that the platform can
> specify the required quirks for the driver, rather than the driver
> becoming conditional on multiple platforms.
>
> As part of this, we rename the dw-hdmi 'spare' which is defined as the
> SVSRET bit in later documentation.
 I'd really prefer that we did not go down the broken route of adding
 a set of "quirk" flags - look at what a mess SDHCI has become through
 allowing that kind of practice.

 I'd much rather we find a saner structure to this - and we know that
 the hardware has ID registers in it which can be used (so far) to
 identify the buggy hardware.
>>> I'd much prefer something that would allow runtime identification of the
>>> device and the corresponding actions to be taken. However, the amount of
>>> documentation we have on the DWC HDMI TX IP core (and the associated PHY)
>>> is pretty limited, given that Synopsys doesn't make the documentation
>>> available publicly. Changes made to the IP core by integrators could
>>> complicate this further. I'm trying to gather as much information as
>>> possible to make clean the code up, for instance by trying to identify
>>> the PHYs used on the various platforms we support. Progress is slow on
>>> that front, there isn't enough leaked information available online :-) I
>>> haven't given up though, but I'll need more time.
>>>
>>> I don't like quirks much either. They are however already used today, even
>>> if we trigger them through dev_type instead of quirk flags. This patch
>>> came from a previous version found in a BSP that simply sprinkled several
>>> if (hdmi-> dev_type == RCAR_HDMI) through the code. For instance,
>>>
>>> -   if (hdmi->dev_type == RK3288_HDMI)
>>> +   if (hdmi->dev_type == RK3288_HDMI || hdmi->dev_type == RCAR_HDMI)
>>> dw_hdmi_phy_enable_spare(hdmi, 1);
>>>
>>> which I think is worse than flags as it would quickly degenerate to
>>> spaghetti code.
>>>
>>> For this specific case, we've managed to identify that on Renesas
>>> platforms the bit set by this function is called SVSRET. Its usage isn't
>>> clear yet, but I suspect it to control one of the PHY input control
>>> signals, like the other bits in the same register. I'm trying to get more
>>> information to clean the implementation further, hopefully with a way to
>>> determine whether the signal is used based on PHY identification.
>> SVSRET is a low power mode consumption and is a PHY input signal
>> as you suggested.
> Thank you for the confirmation. Would you happen to know what SVSRET stands 
> for ?

Have no info about that. Sorry.

>
>> Most of the configurable input signals of the PHY are available by the
>> controller regbank. I don't think it is possible to detect this at runtime,
>> I think you have at least to hardcode which version of the PHY you are
>> using.
>>
>> I would suggest that maybe all the PHY logic should be extracted and then
>> use callbacks to glue controller and phy. Then, depending on the PHY you
>> could use empty stubs if, for example, a given PHY did not support SVSRET.
>> Still, I don't know if this is the best option. What I do know is that there
>> are a large number of PHY's with different flavors that can use the same
>> controller. The controller has different versions also, and each version can
>> have quirks but I think it would be easier to manage this driver if we had a
>> clear distinction between PHY and controller.
> Agreed, I'd like to go in that direction. What makes it quite difficult is 
> the 
> lack of documentation about the PHYs :-) I've found six different PHY types 
> that can be identified by the CONFIG2_ID register:
>
> Bits| Field   | Description
> --
> 7-0 | phytype | PHY interface
> | | 0x00: Legacy PHY (HDMI TX PHY)
> | | 0xb2: MHL PHY + HEAC PHY
> | | 0xc2: MHL PHY
> | | 0xe2: HDMI 3D TX PHY + HEAC PHY
> | | 0xf2: HDMI 3D TX PHY
> | | 0xf3: HDMI2 TX PHY
>
> I'm sure there's more than that. In particular I wonder how external vendor 
> PHYs are identified.

0xFE.

>
> I'm also wondering whether there's a need to keep support for the legacy PHY 
> signals (ENTMDS and PDZ in the PHY_CONF0 register). As far as I understand 
> they're not used by the 

[PATCH] drm/bridge: analogix: Don't return -EINVAL when panel not support PSR in PSR functions

[Sean Paul]

On Sun, Dec 4, 2016 at 10:13 PM, Archit Taneja  
wrote:
>
>
> On 12/02/2016 09:33 PM, Sean Paul wrote:
>>
>> On Thu, Dec 1, 2016 at 10:54 PM, Archit Taneja 
>> wrote:
>>>
>>> Hi,
>>>
>>> On 12/02/2016 08:02 AM, zain wang wrote:


 We will ignored PSR setting if panel not support it. So, in this case,
 we
 should
 return from analogix_dp_enable/disable_psr() without any error code.
 Let's retrun 0 instead of -EINVAL when panel not support PSR in
 analogix_dp_enable/disable_psr().

 Signed-off-by: zain wang 
 ---
  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

 diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
 b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
 index 6e0447f..0cb3695 100644
 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
 +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
 @@ -112,7 +112,7 @@ int analogix_dp_enable_psr(struct device *dev)
 struct edp_vsc_psr psr_vsc;

 if (!dp->psr_support)
 -   return -EINVAL;
 +   return 0;
>>>
>>>
>>>
>>> Looking at the rockchip analogix dp code, in analogix_dp_psr_set, the
>>> worker
>>> that calls
>>> analogix_dp_enable/disable_psr isn't even if psr isn't enabled. So, the
>>> bridge funcs
>>> shouldn't be called in the first place. I think the error handling is
>>> fine
>>> to have
>>> here.
>>>
>>
>> Hi Archit,
>>
>> This was my first impression, too, and the complexity of the various
>> psr abstraction layers don't help :)
>>
>> However, this code path will be hit if the source supports psr, but
>> the sink doesn't. The rockchip_drm_psr code doesn't know if the sink
>> supports psr, so it will end up calling this.
>
>
> Okay, thanks for the explanation. The dev_warn() below still seems
> unnecessary, right?
>

Yeah, one could make a case for dev_info (disclaimer: I have a high
tolerance for noisy logs), but a warning does seem excessive.

Sean

> Archit
>
>
>>
>> Sean
>>
>>

 /* Prepare VSC packet as per EDP 1.4 spec, Table 6.9 */
 memset(_vsc, 0, sizeof(psr_vsc));
 @@ -135,7 +135,7 @@ int analogix_dp_disable_psr(struct device *dev)
 struct edp_vsc_psr psr_vsc;

 if (!dp->psr_support)
 -   return -EINVAL;
 +   return 0;

 /* Prepare VSC packet as per EDP 1.4 spec, Table 6.9 */
 memset(_vsc, 0, sizeof(psr_vsc));
 @@ -878,6 +878,8 @@ static void analogix_dp_commit(struct
 analogix_dp_device *dp)
 dp->psr_support = analogix_dp_detect_sink_psr(dp);
 if (dp->psr_support)
 analogix_dp_enable_sink_psr(dp);
 +   else
 +   dev_warn(dp->dev, "Sink not support PSR\n");
>>>
>>>
>>>
>>> This doesn't seem beneficial either. There seems to be a debug
>>> print already in analogix_dp_detect_sink_psr which reports PSR
>>> related info.
>>>
>>> Archit
>>>
  }

  /*

>>>
>>> --
>>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>>> a Linux Foundation Collaborative Project
>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project

Enabling peer to peer device transactions for PCIe devices

[Logan Gunthorpe]

On 05/12/16 12:14 PM, Jason Gunthorpe wrote:
> But CMB sounds much more like the GPU case where there is a
> specialized allocator handing out the BAR to consumers, so I'm not
> sure a general purpose chardev makes a lot of sense?

I don't think it will ever need to be as complicated as the GPU case. 
There will probably only ever be a relatively small amount of memory 
behind the CMB and really the only users are those doing P2P work. Thus 
the specialized allocator could be pretty simple and I expect it would 
be fine to just return -ENOMEM if there is not enough memory.

Also, if it was implemented this way, if there was a need to make the 
allocator more complicated it could easily be added later as the 
userspace interface is just mmap to obtain a buffer.

Logan

[PATCH v2 1/3] drm: Add SCDC helpers

[Thierry Reding]

On Mon, Dec 05, 2016 at 10:12:39AM +, Jose Abreu wrote:
> On 02-12-2016 19:24, Thierry Reding wrote:
[...]
> > +/**
> > + * drm_scdc_write - write a block of data to SCDC
> > + * @adapter: I2C controller
> > + * @offset: start offset of block to write
> > + * @buffer: block of data to write
> > + * @size: size of the block to write
> > + *
> > + * Writes a block of data to SCDC, starting at a given offset.
> > + *
> > + * Returns:
> > + * The number of bytes written to SCDC or a negative error code on failure.
> > + */
> > +ssize_t drm_scdc_write(struct i2c_adapter *adapter, u8 offset,
> > +  const void *buffer, size_t size)
> > +{
> > +   struct i2c_msg msg = {
> > +   .addr = SCDC_I2C_SLAVE_ADDRESS,
> > +   .flags = 0,
> > +   .len = 1 + size,
> > +   .buf = NULL,
> > +   };
> > +   void *data;
> > +   int err;
> > +
> > +   data = kmalloc(1 + size, GFP_TEMPORARY);
> > +   if (!data)
> > +   return -ENOMEM;
> > +
> > +   msg.buf = data;
> > +
> > +   memcpy(data, , sizeof(offset));
> > +   memcpy(data + 1, buffer, size);
> 
> Don't you agree it would be better if you use the same scheme as
> drm_scdc_read()? Something like:
> 
> struct i2c_msg msgs[] = {
> {
> .addr = SCDC_I2C_SLAVE_ADDRESS,
> .flags = 0,
> .len = 1,
> .buf = ,
> }, {
> .addr = SCDC_I2C_SLAVE_ADDRESS,
> .flags = 0,
> .len = size,
> .buf = buffer,
> },
> };

Ville had a similar comment on a prior iteration. It looks as if the
above should work, but it's probably best to test it a little more
widely to make sure we're not running into cases where it breaks.

Have you by any chance verified that it works on your hardware?

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161205/25f08c7f/attachment.sig>

[PATCH v2 3/3] drm/edid: Implement SCDC Read Request capability detection

[Thierry Reding]

On Mon, Dec 05, 2016 at 11:06:15AM +, Jose Abreu wrote:
> Hi Thierry,
> 
> 
> Do you think while you are at it you could implement a
> set_scrambling() callback? It should be pretty straight forward:
> you read the SCDC_TMDS_CONFIG reg, do a mask, and then write it
> again.
> 
> 
> I think this is an important feature that we should have.

Yeah, agreed. I was actually thinking about going one step further and
provide more of the polling functionality as a helper. Even if we have
accessors that wrap the low-level functionality, most drivers would
still have to provide their own delayed workqueue to deal with sinks
(or sources) that don't support read requests. Having this in standard
helpers would help reduce the boilerplate a lot further.

Does your hardware by any chance support read requests on SCDC? It'd be
interesting to see how that works in practice. Unfortunately Tegra does
not seem to support it.

Thierry

> On 02-12-2016 19:24, Thierry Reding wrote:
> > From: Thierry Reding 
> >
> > Sinks that support SCDC can optionally have the capability to initiate
> > read requests, which are a mechanism by which a sink can notify its
> > source that it should read the Update Flags. If either the sink or the
> > source are not Read Request capable, polling of the Update Flags shall
> > be employed.
> >
> > Signed-off-by: Thierry Reding 
> > ---
> > Changes in v2:
> > - new patch
> >
> >  drivers/gpu/drm/drm_edid.c | 36 
> >  include/drm/drm_edid.h |  1 +
> >  2 files changed, 37 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 369961597ee5..8211cce3e09e 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -3736,6 +3736,42 @@ bool drm_detect_hdmi_scdc(struct edid *edid)
> >  EXPORT_SYMBOL(drm_detect_hdmi_scdc);
> >  
> >  /**
> > + * drm_detect_hdmi_scdc_rr_capable - detect whether an HDMI sink is 
> > capable of
> > + *initiating an SCDC Read Request
> > + * @edid: sink EDID information
> > + *
> > + * Parse the CEA extension according to CEA-861-B to find an HF-VSDB as
> > + * defined in HDMI 2.0, section 10.3.2 "HDMI Forum Vendor Specific Data
> > + * Block" and checks if the RR_Capable bit (bit 6 of byte 6) is set.
> > + *
> > + * Returns:
> > + * True if the sink is capable of initiating an SCDC Read Request, false
> > + * otherwise.
> > + */
> > +bool drm_detect_hdmi_scdc_rr_capable(struct edid *edid)
> > +{
> > +   unsigned int start, end, i;
> > +   const u8 *cea;
> > +
> > +   cea = drm_find_cea_extension(edid);
> > +   if (!cea)
> > +   return false;
> > +
> > +   if (cea_db_offsets(cea, , ))
> > +   return false;
> > +
> > +   for_each_cea_db(cea, i, start, end) {
> > +   if (cea_db_is_hdmi_forum_vsdb([i])) {
> > +   if (cea[i + 6] & 0x40)
> > +   return true;
> > +   }
> > +   }
> > +
> > +   return false;
> > +}
> > +EXPORT_SYMBOL(drm_detect_hdmi_scdc_rr_capable);
> > +
> > +/**
> >   * drm_detect_monitor_audio - check monitor audio capability
> >   * @edid: EDID block to scan
> >   *
> > diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> > index 7ea7e90846d8..d1c29586035e 100644
> > --- a/include/drm/drm_edid.h
> > +++ b/include/drm/drm_edid.h
> > @@ -441,6 +441,7 @@ u8 drm_match_cea_mode(const struct drm_display_mode 
> > *to_match);
> >  enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code);
> >  bool drm_detect_hdmi_monitor(struct edid *edid);
> >  bool drm_detect_hdmi_scdc(struct edid *edid);
> > +bool drm_detect_hdmi_scdc_rr_capable(struct edid *edid);
> >  bool drm_detect_monitor_audio(struct edid *edid);
> >  bool drm_rgb_quant_range_selectable(struct edid *edid);
> >  int drm_add_modes_noedid(struct drm_connector *connector,
> 
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161205/fc80b324/attachment.sig>

Enabling peer to peer device transactions for PCIe devices

[Jason Gunthorpe]

On Mon, Dec 05, 2016 at 10:48:58AM -0800, Dan Williams wrote:
> On Mon, Dec 5, 2016 at 10:39 AM, Logan Gunthorpe  
> wrote:
> > On 05/12/16 11:08 AM, Dan Williams wrote:
> >>
> >> I've already recommended that iopmem not be a block device and instead
> >> be a device-dax instance. I also don't think it should claim the PCI
> >> ID, rather the driver that wants to map one of its bars this way can
> >> register the memory region with the device-dax core.
> >>
> >> I'm not sure there are enough device drivers that want to do this to
> >> have it be a generic /sys/.../resource_dmableX capability. It still
> >> seems to be an exotic one-off type of configuration.
> >
> >
> > Yes, this is essentially my thinking. Except I think the userspace interface
> > should really depend on the device itself. Device dax is a good  choice for
> > many and I agree the block device approach wouldn't be ideal.
> >
> > Specifically for NVME CMB: I think it would make a lot of sense to just hand
> > out these mappings with an mmap call on /dev/nvmeX. I expect CMB buffers
> > would be volatile and thus you wouldn't need to keep track of where in the
> > BAR the region came from. Thus, the mmap call would just be an allocator
> > from BAR memory. If device-dax were used, userspace would need to lookup
> > which device-dax instance corresponds to which nvme drive.
> 
> I'm not opposed to mapping /dev/nvmeX.  However, the lookup is trivial
> to accomplish in sysfs through /sys/dev/char to find the sysfs path
> of

But CMB sounds much more like the GPU case where there is a
specialized allocator handing out the BAR to consumers, so I'm not
sure a general purpose chardev makes a lot of sense?

Jason

[PATCH v2 2/3] drm/edid: Implement SCDC support detection

[Thierry Reding]

tart, end) \
> > >   for ((i) = (start); (i) < (end) && (i) + 
> > > cea_db_payload_len(&(cea)[(i)]) < (end); (i) += 
> > > cea_db_payload_len(&(cea)[(i)]) + 1)
> > >  
> > > @@ -3687,6 +3702,40 @@ bool drm_detect_hdmi_monitor(struct edid *edid)  
> > > EXPORT_SYMBOL(drm_detect_hdmi_monitor);
> > >  
> > >  /**
> > > + * drm_detect_hdmi_scdc - detect whether an HDMI sink supports SCDC
> > > + * @edid: sink EDID information
> > > + *
> > > + * Parse the CEA extension according to CEA-861-B to find an HF-VSDB as
> > > + * defined in HDMI 2.0, section 10.3.2 "HDMI Forum Vendor Specific Data
> > > + * Block" and checks if the SCDC_Present bit (bit 7 of byte 6) is set.
> > > + *
> > > + * Returns:
> > > + * True if the sink supports SCDC, false otherwise.
> > > + */
> > > +bool drm_detect_hdmi_scdc(struct edid *edid) {
> > > + unsigned int start, end, i;
> > > + const u8 *cea;
> > > +
> > > + cea = drm_find_cea_extension(edid);
> > > + if (!cea)
> > > + return false;
> > > +
> > > + if (cea_db_offsets(cea, , ))
> > > + return false;
> > > +
> > > + for_each_cea_db(cea, i, start, end) {
> > > + if (cea_db_is_hdmi_forum_vsdb([i])) {
> > > + if (cea[i + 6] & 0x80)
> > > + return true;
> > > + }
> > > + }
> > > +
> > > + return false;
> > > +}
> > > +EXPORT_SYMBOL(drm_detect_hdmi_scdc);
> > > +
> > > +/**
> > >   * drm_detect_monitor_audio - check monitor audio capability
> > >   * @edid: EDID block to scan
> > >   *
> > > diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index 
> > > 38eabf65f19d..7ea7e90846d8 100644
> > > --- a/include/drm/drm_edid.h
> > > +++ b/include/drm/drm_edid.h
> > > @@ -440,6 +440,7 @@ int drm_add_edid_modes(struct drm_connector 
> > > *connector, struct edid *edid);
> > >  u8 drm_match_cea_mode(const struct drm_display_mode *to_match);  enum 
> > > hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code);  bool 
> > > drm_detect_hdmi_monitor(struct edid *edid);
> > > +bool drm_detect_hdmi_scdc(struct edid *edid);
> > >  bool drm_detect_monitor_audio(struct edid *edid);  bool 
> > > drm_rgb_quant_range_selectable(struct edid *edid);  int 
> > > drm_add_modes_noedid(struct drm_connector *connector, diff --git 
> > > a/include/linux/hdmi.h b/include/linux/hdmi.h index 
> > > edbb4fc674ed..d271ff23984f 100644
> > > --- a/include/linux/hdmi.h
> > > +++ b/include/linux/hdmi.h
> > > @@ -35,6 +35,7 @@ enum hdmi_infoframe_type {  };
> > >  
> > >  #define HDMI_IEEE_OUI 0x000c03
> > > +#define HDMI_FORUM_IEEE_OUI 0xc45dd8
> > >  #define HDMI_INFOFRAME_HEADER_SIZE  4
> > >  #define HDMI_AVI_INFOFRAME_SIZE13
> > >  #define HDMI_SPD_INFOFRAME_SIZE25
> > > --
> > > 2.10.2
> > > 
> 
> 
> 
> > ___
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161205/0e84bba0/attachment-0001.sig>

[PATCH 12/12] drm/msm: gpu: Use the zap shader on 5XX if we can

[Bjorn Andersson]

On Mon 05 Dec 11:57 PST 2016, Bjorn Andersson wrote:

[..]
> You should rather add a struct device zap_dev to your adreno context, do
> minimal initialization (name and a parent I think is enough), call
> device_register(_dev);, of_reserved_mem_device_init() and then use
> that for your dma allocation.

You would of course need to set the of_node on the zap_dev for
of_reserved_mem_device_init() to work. Sorry for missing that.

Regards,
Bjorn

Enabling peer to peer device transactions for PCIe devices

[Christoph Hellwig]

On Mon, Dec 05, 2016 at 12:46:14PM -0700, Jason Gunthorpe wrote:
> In any event the allocator still needs to track which regions are in
> use and be able to hook 'free' from userspace. That does suggest it
> should be integrated into the nvme driver and not a bolt on driver..

Two totally different use cases:

 - a card that exposes directly byte addressable storage as a PCI-e
   bar.  Thin of it as a nvdimm on a PCI-e card.  That's the iopmem
   case.
 - the NVMe CMB which exposes a byte addressable indirection buffer for
   I/O, but does not actually provide byte addressable persistent
   storage.  This is something that needs to be added to the NVMe driver
   (and the block layer for the abstraction probably).

[PATCH] drm/atomic: doc: remove old comment about nonblocking commits

[Gustavo Padovan]

From: Gustavo Padovan 

We now support nonblocking commits on drm_atomic_helper_commit()
so the comment is not valid anymore.

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

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 0b16587..95e2984 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1226,9 +1226,6 @@ static void commit_work(struct work_struct *work)
  * function implements nonblocking commits, using
  * drm_atomic_helper_setup_commit() and related functions.
  *
- * Note that right now this function does not support nonblocking commits, 
hence
- * driver writers must implement their own version for now.
- *
  * Committing the actual hardware state is done through the
  * ->atomic_commit_tail() callback of the _mode_config_helper_funcs vtable,
  * or it's default implementation drm_atomic_helper_commit_tail().
-- 
2.5.5

[GIT PULL] imx-drm: fix plane atomic_update regression

[Philipp Zabel]

Hi Dave,

this tag fixes a regression in the imx-drm YUV plane support [1] that
was introduced in v4.9-rc4, as well as an error returned when changing
U/V offsets of an active plane to otherwise valid values.

[1] https://lkml.org/lkml/2016/11/17/133

Both commits are already contained in drm-next.

regards
Philipp

The following changes since commit 86126748cd5063aa888ce252f16b89b35e7d4707:

  drm/imx: ipuv3-plane: disable local alpha for planes without alpha channel 
(2016-10-20 14:39:51 +0200)

are available in the git repository at:

  git://git.pengutronix.de/pza/linux.git tags/imx-drm-fixes-2016-12-02

for you to fetch changes up to 3fd8b292ae6b99e3e6e96df3e470b25b100741a8:

  drm/imx: ipuv3-plane: merge ipu_plane_atomic_set_base into atomic_update 
(2016-10-20 14:39:53 +0200)


imx-drm: fix plane atomic_update regression

- request modeset if plane offsets changed, only the plane base
  address can be changed without disabling the plane IDMAC channel,
  stop returning an error if a valid YUV plane configuration with
  different U/V offsets from the currently active plane is requested.
- merge ipu_plane_atomic_set_base into atomic_update, also fixes a
  regression introduced by commit 81d553545a15 ("drm/imx: ipuv3-plane: Skip
  setting u/vbo only when we don't need modeset") where U/V offsets were not
  set corretly when enabling YUV planes without a modeset.


Philipp Zabel (2):
  drm/imx: ipuv3-plane: request modeset if plane offsets changed
  drm/imx: ipuv3-plane: merge ipu_plane_atomic_set_base into atomic_update

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

tilcdc: vblank wait timed out

[Bartosz Golaszewski]

Hi Jyri,

I pulled your recent tilcdc pull request on top of v4.9 and Sekhar's
davinci branch (+ vga dac DT)[1].

I'm getting "vblank wait timed out" errors when running simple modetest[2].

This error happened before with the drm_bridge series[3], but went
away at one of the subsequent patch versions.

Could you please verify that you see the same error and advise on what
could be the reason? I'm investigating on my own as well.

Best regards,
Bartosz Golaszewski

[1] https://github.com/brgl/linux/tree/tilcdc/modetest_error
[2] http://pastebin.com/rCM44Uds
[3] http://www.spinics.net/lists/dri-devel/msg123732.html

[PATCH 1/2] pwm: lpss: Make builtin and add lookup-table so that i915 can find the pwm_backlight

[Thierry Reding]

On Mon, Dec 05, 2016 at 09:18:03AM +0100, Hans de Goede wrote:
> Hi,
> 
> On 05-12-16 08:46, Thierry Reding wrote:
> > On Fri, Dec 02, 2016 at 11:17:35AM +0100, Hans de Goede wrote:
> > > The primary consumer of the lpss pwm is the i915 kms driver, but
> > > currently that driver cannot get the pwm because i915 platforms are
> > > not using devicetree and pwm-lpss does not call pwm_add_table.
> > > 
> > > Another problem is that i915 does not support get_pwm returning
> > > -EPROBE_DEFER and i915's init is very complex and this is almost
> > > impossible to fix.
> > > 
> > > This commit changes the PWM_LPSS Kconfig from a tristate to a bool, so
> > > that when the i915 driver loads the lpss pwm will be available avoiding
> > > the -EPROBE_DEFER issue. Note that this is identical to how the same
> > > problem was solved for the pwm-crc driver.
> > > 
> > > Being builtin also allows calling pwm_add_table directly from the
> > > pwm-lpss code, otherwise the pwm_add_table call would need to be put
> > > somewhere else to ensure it happens before i915 calls pwm_get,
> > > even if i915 would support -EPROBE_DEFER.
> > > 
> > > Signed-off-by: Hans de Goede 
> > > ---
> > >  drivers/pwm/Kconfig| 12 +++-
> > >  drivers/pwm/pwm-lpss.c | 11 +++
> > >  2 files changed, 14 insertions(+), 9 deletions(-)
> > 
> > This is completely backwards. You're putting board-specific bits into a
> > generic driver.
> 
> This is not really board specific I'm advertising that the goal of
> the pwm is to be used to control a backlight.

pwm_add_table() is a board-specific API. Documentation/pwm.txt says that
"board setup code" should be using pwm_add_table(). Using it from within
the provider is completely the opposite.

> Before typing this reply I've been thinking about another place
> to put the pwm_add_table call put I cannot come up with any.

I suggested drivers/platform/x86. A bunch of code in there is doing
exactly the kind of board/platform setup stuff that you're trying to do
here.

> 
> drivers/acpi/acpi_lpss.c comes to mind, but that would only work
> with the pwm device in acpi mode and not with it in pci mode.
> 
> Another candidate would be to put the pwm_add_table call in the
> i915 driver itself, when the vbt says we need to use an external
> pwm and the plaform is cherrytrail, but it does not know if the
> pwm device is in pci or acpi modes which requires a different
> provider entry in the table.

i915 isn't a good location for that either. This should really be driven
by code external to any generic drivers. It's exactly the kind of glue
that platform or board setup code is used for.

If you look at drivers/platform/x86/intel_oaktrail.c, it does very
similar things. If this is specific to Cherry Trail, I'm sure you can
find ways to identify the platform as such and drive it in a similar way
from drivers/platform/x86/intel_cherrytrail.c.

> Besides that having the pwm in the table will not do anything
> unless the i915 driver does a pwm_get, so this does not gain us
> anything.

It will keep the driver generic and put the board code where it belongs.
I call that very much of a gain.

> Maybe we need to rename the con_id from "pwm_backlight" to
> "pwm_lpss0", that way it is very clear which pwm any pwm_get calls
> are getting (and lpss-pwm.c is obviously the correct place to
> do the add_table), and then we can teach the i915 code to look
> for "pwm_lpss0" based on vbt info ?

pwm_backlight is the much better consumer name because by your own words
that's exactly what the PWM is used for. Obfuscating this by turning the
name into something unrecognizable such as pwm_lpss0 isn't going to
change any of the above.

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161205/fe3da6c5/attachment.sig>

[PATCH 11/12] drm/msm: Add a quick and dirty PIL loader

[Bjorn Andersson]

On Mon 28 Nov 11:28 PST 2016, Jordan Crouse wrote:

> In order to switch the GPU out of secure mode on most systems we
> need to load a zap shader into memory and get it authenticated
> and into the secure world.  All the bits and pieces to do
> the load are scattered throughout the kernel, but we need to
> bring everything together.
>
> Add a semi-custom loader that will read a MDT file and get
> it loaded and authenticated through SCM.
>

I've been trying to figure out a reasonable way to provide a
scm/pas/mdt-loader, but haven't come up with anything sane yet. Perhaps
it's better to provide helpers for the scm case and open code the MDT
loading in the non-scm driver. I think this is an okay approach for now.

But it's not a "PIL loader", that's just a side effect of reusing parts
of the existing PIL load in the Qualcomm tree. I would suggest just
making the subject "Add ZAP MDT loader".

Some minor comments below.

> Signed-off-by: Jordan Crouse 
> ---
>  drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 166 
> ++
>  1 file changed, 166 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c 
> b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> index 3824bc4..eefe197 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> @@ -11,12 +11,178 @@
>   *
>   */
>  
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
>  #include "msm_gem.h"
>  #include "a5xx_gpu.h"
>  
>  extern bool hang_debug;
>  static void a5xx_dump(struct msm_gpu *gpu);
>  
> +static inline bool _check_segment(const struct elf32_phdr *phdr)
> +{
> + return ((phdr->p_type == PT_LOAD) &&
> + ((phdr->p_flags & (7 << 24)) != (2 << 24)) &&
> + phdr->p_memsz);
> +}
> +
> +static int __pil_tz_load_image(struct platform_device *pdev,

Rather than throwing more underscores at it I would have named this
"zap_load_segments".

> + const struct firmware *mdt, const char *fwname,
> + void *fwptr, size_t fw_size, unsigned long fw_min_addr)
> +{
> + char str[64] = { 0 };

Name this filename or similar and no need to initialize it.

> + const struct elf32_hdr *ehdr = (struct elf32_hdr *) mdt->data;
> + const struct elf32_phdr *phdrs = (struct elf32_phdr *) (ehdr + 1);
> + const struct firmware *fw;
> + int i, ret = 0;
> +
> + for (i = 0; i < ehdr->e_phnum; i++) {
> + const struct elf32_phdr *phdr = [i];
> + size_t offset;
> +
> + /* Make sure the segment is loadable */
> + if (!_check_segment(phdr))
> + continue; > +
> + /* Get the offset of the segment within the region */
> + offset = (phdr->p_paddr - fw_min_addr);
> +
> + /* Request the file containing the segment */
> + snprintf(str, sizeof(str) - 1, "%s.b%02d", fwname, i);

snprintf(, size, ) writes at most size bytes and includes null
termination, so drop the "- 1".

> +
> + ret = request_firmware(, str, >dev);
> + if (ret) {
> + dev_err(>dev, "Failed to load segment %s\n", str);
> + break;
> + }
> +
> + if (offset + fw->size > fw_size) {
> + dev_err(>dev, "Segment %s is too big\n", str);
> + ret = -EINVAL;
> + release_firmware(fw);
> + break;
> + }
> +
> + /* Copy the segment into place */
> + memcpy(fwptr + offset, fw->data, fw->size);

Just to be on the safe side, please add:

if (phdr->p_memsz > phdr->p_filesz)
memset(fwptr + fw->size, 0, phdr->p_memsz - phdr->p_filesz);

> + release_firmware(fw);
> + }
> +
> + return ret;
> +}
> +
> +static int _pil_tz_load_image(struct platform_device *pdev)

zap_load_mdt() ?

> +{
> + char str[64] = { 0 };
> + const char *fwname;
> + const struct elf32_hdr *ehdr;
> + const struct elf32_phdr *phdrs;
> + const struct firmware *mdt;
> + phys_addr_t fw_min_addr, fw_max_addr;
> + dma_addr_t fw_phys;
> + size_t fw_size;
> + u32 pas_id;
> + void *ptr;
> + int i, ret;
> +
> + if (pdev == NULL)
> + return -ENODEV;

This should not happen.

> +
> + if (!qcom_scm_is_available()) {
> + dev_err(>dev, "SCM is not available\n");
> + return -EINVAL;

We generally return -EPROBE_DEFER here.

> + }
> +
> + ret = of_reserved_mem_device_init(>dev);
> +

Please drop the extra newline.

> + if (ret) {
> + dev_err(>dev, "Unable to set up the reserved memory\n");
> + return ret;
> + }
> +
> + /* Get the firmware and PAS id from the device node */
> + if (of_property_read_string(pdev->dev.of_node, "qcom,firmware",
> + )) {
> + dev_err(>dev, "Could not read a firmware name\n");
> + return -EINVAL;
> + }
> +
> + if (of_property_read_u32(pdev->dev.of_node, "qcom,pas-id", _id)) {

This is constant, so define it in the driver.

> + dev_err(>dev, "Could not read the pas ID\n");
> + return -EINVAL;
> + }
> +
> + snprintf(str, sizeof(str) - 1, "%s.mdt", fwname);

As above, re "- 1", name of str and initialization.

> +
> + /* Request the MDT file for the firmware */
> + ret = request_firmware(, str, >dev);
> + if (ret) {
> + dev_err(>dev, "Unable to load %s\n", str);
> + return ret;
> + }
> +
> + ehdr = (struct elf32_hdr *) mdt->data;
> + phdrs = (struct elf32_phdr *) (ehdr + 1);
> +
> + /* Get 

[PATCH 12/12] drm/msm: gpu: Use the zap shader on 5XX if we can

[Bjorn Andersson]

On Mon 28 Nov 11:28 PST 2016, Jordan Crouse wrote:

> The A5XX GPU powers on in "secure" mode. In secure mode the GPU can
> only render to buffers that are marked as secure and inaccessible
> to the kernel and user through a series of hardware protections. In
> practice secure mode is used to draw things like a UI on a secure
> video frame.
> 
> In order to switch out of secure mode the GPU executes a special
> shader that clears out the GMEM and other sensitve registers and
> then writes a register. Because the kernel can't be trusted the
> shader binary is signed and verified and programmed by the
> secure world. To do this we need to read the MDT header and the
> segments from the firmware location and put them in memory and
> present them for approval.
> 
> For targets without secure support there is an out: if the
> secure world doesn't support secure then there are no hardware
> protections and we can freely write the SECVID_TRUST register from
> the CPU. We don't have 100% confidence that we can query the
> secure capabilities at run time but we have enough calls that
> need to go right to give us some confidence that we're at least doing
> something useful.
> 
> Of course if we guess wrong you trigger a permissions violation
> which usually ends up in a system crash but thats a problem
> that shows up immediately.
> 
> Signed-off-by: Jordan Crouse 
> ---
>  drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 72 
> ++-
>  1 file changed, 70 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c 
> b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> index eefe197..a7a58ec 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> @@ -469,6 +469,55 @@ static int a5xx_ucode_init(struct msm_gpu *gpu)
>   return 0;
>  }
>  
> +static int a5xx_zap_shader_resume(struct msm_gpu *gpu)
> +{
> + int ret;
> +
> + ret = qcom_scm_gpu_zap_resume();
> + if (ret)
> + DRM_ERROR("%s: zap-shader resume failed: %d\n",
> + gpu->name, ret);
> +
> + return ret;
> +}
> +
> +static int a5xx_zap_shader_init(struct msm_gpu *gpu)
> +{
> + static bool loaded;
> + struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
> + struct a5xx_gpu *a5xx_gpu = to_a5xx_gpu(adreno_gpu);
> + struct platform_device *pdev = a5xx_gpu->pdev;
> + struct device_node *node;
> + int ret;
> +
> + /*
> +  * If the zap shader is already loaded into memory we just need to kick
> +  * the remote processor to reinitialize it
> +  */
> + if (loaded)

Why is this handling needed? Why can init be called multiple times?

> + return a5xx_zap_shader_resume(gpu);
> +
> + /* Populate the sub-nodes if they haven't already been done */
> + of_platform_populate(pdev->dev.of_node, NULL, NULL, >dev);

I haven't been able to find the qcom,zap-shader platform driver, but I
presume you have something like:

adreno {
qcom,zap-shader {
compatible = "qcom,zap-shader";

firmware = "zapfw";
memory-region = <_region>;
};
};

I presume this is done to not "taint" the adreno device's with the zap
memory region, but I don't think you should (ab)use a platform driver
for this.

You should rather add a struct device zap_dev to your adreno context, do
minimal initialization (name and a parent I think is enough), call
device_register(_dev);, of_reserved_mem_device_init() and then use
that for your dma allocation.

This saves you from creating a platform_driver, instantiating a
platform_device and the worry of the race between the creation of that
device and the of_find_device_by_node() below.

> +
> + /* Find the sub-node for the zap shader */
> + node = of_find_node_by_name(pdev->dev.of_node, "qcom,zap-shader");

If you're looking for immediate children use of_get_child_by_name()

And no "qcom," in node names please.

> + if (!node) {
> + DRM_ERROR("%s: qcom,zap-shader not found in device tree\n",
> + gpu->name);
> + return -ENODEV;
> + }
> +
> + ret = _pil_tz_load_image(of_find_device_by_node(node));
> + if (ret)
> + DRM_ERROR("%s: Unable to load the zap shader\n",
> + gpu->name);
> +
> + loaded = !ret;
> +
> + return ret;
> +}

Regards,
Bjorn

[Bug 98915] NULL pointer dereference on boot - amdgpu_debugfs_add_files

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

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

--- Comment #3 from Nicolai Stange  ---
(In reply to Alex Deucher from comment #2)
> Can you bisect?

No need: most likely, the offending commit is 8a357d10043c ("drm: Nerf
DRM_CONTROL nodes").

C.f. the discussion at
http://lkml.kernel.org/r/20161203144700.2307-1-nicstange at gmail.com

A patch for amdgpu is in the works as well.

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

Enabling peer to peer device transactions for PCIe devices

[Logan Gunthorpe]

On 05/12/16 11:08 AM, Dan Williams wrote:
> I've already recommended that iopmem not be a block device and instead
> be a device-dax instance. I also don't think it should claim the PCI
> ID, rather the driver that wants to map one of its bars this way can
> register the memory region with the device-dax core.
>
> I'm not sure there are enough device drivers that want to do this to
> have it be a generic /sys/.../resource_dmableX capability. It still
> seems to be an exotic one-off type of configuration.

Yes, this is essentially my thinking. Except I think the userspace 
interface should really depend on the device itself. Device dax is a 
good  choice for many and I agree the block device approach wouldn't be 
ideal.

Specifically for NVME CMB: I think it would make a lot of sense to just 
hand out these mappings with an mmap call on /dev/nvmeX. I expect CMB 
buffers would be volatile and thus you wouldn't need to keep track of 
where in the BAR the region came from. Thus, the mmap call would just be 
an allocator from BAR memory. If device-dax were used, userspace would 
need to lookup which device-dax instance corresponds to which nvme drive.

Logan

[RFC] drm: Enable dynamic debug for DRM_[DEV]_DEBUG*

[Robert Bragg]

Forgot to send to dri-devel when I first sent this out...

The few times I've looked at using DRM_DEBUG messages, I haven't found
them very helpful considering how noisy some of the categories are. More
than once now I've preferred to go in and modify individual files to
affect what messages I see and re-build.

After recently converting some of the i915_perf.c messages to use
DRM_DEBUG, I thought I'd see if DRM_DEBUG could be updated to have a bit
more fine grained control than the current category flags.

A few things to note with this first iteration:

- I haven't looked to see what affect the change has on linked object
  sizes.

- It seems like it could be nice if dynamic debug could effectively make
  the drm_debug parameter redundant but dynamic debug doesn't give us a
  way to categorise messages so maybe we'd want to consider including
  categories in messages something like:

  "[drm][kms] No FB found"

  This way all kms messages could be enabled via:
  echo "format [kms] +p" > dynamic_debug/control

  Note with this simple scheme categories would no longer be mutually
  exclusive which could be a nice bonus.

  Since it would involve changing the output format, I wonder how
  concerned others might be about breaking some userspace (maybe CI test
  runners) that for some reason grep for specific messages?

--- >8 --- (git am --scissors)

Dynamic debug messages (ref: Documentation/dynamic-debug-howto.txt)
allow fine grained control over which debug messages are enabled with
runtime control through /sysfs/kernel/debug/dynamic_debug/control

This provides more control than the current drm.drm_debug parameter
which for some use cases is impractical to use given how chatty
some drm debug categories are.

For example all debug messages in i915_drm.c can be enabled with:
echo "file i915_perf.c +p" > dynamic_debug/control

This aims to maintain compatibility with controlling debug messages
using the drm_debug parameter. The new dynamic debug macros are called
by default but conditionally calling [dev_]printk if the category flag
is set (side stepping the dynamic debug condition in that case)

This removes the drm_[dev_]printk wrappers considering that the dynamic
debug macros are only useful if they can track the __FILE__, __func__
and __LINE__ where they are called. The wrapper didn't seem necessary in
the DRM_UT_NONE case with no category flag.

The output format should be compatible, unless the _DEV macros are
passed a NULL dev pointer considering how the core.c dev_printk
implementation adds "(NULL device *)" to the message in that case while
the drm wrapper would fallback to a plain printk in this case.
Previously some of non-dev drm debug macros were defined in terms of
passing NULL to a dev version but that's avoided now due to this
difference.

Signed-off-by: Robert Bragg 
Cc: dri-devel at lists.freedesktop.org
Cc: Daniel Vetter 
Cc: Chris Wilson 
---
 drivers/gpu/drm/drm_drv.c |  47 -
 include/drm/drmP.h| 168 +-
 2 files changed, 108 insertions(+), 107 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index f74b7d0..25d00aa 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -65,53 +65,6 @@ static struct idr drm_minors_idr;

 static struct dentry *drm_debugfs_root;

-#define DRM_PRINTK_FMT "[" DRM_NAME ":%s]%s %pV"
-
-void drm_dev_printk(const struct device *dev, const char *level,
-   unsigned int category, const char *function_name,
-   const char *prefix, const char *format, ...)
-{
-   struct va_format vaf;
-   va_list args;
-
-   if (category != DRM_UT_NONE && !(drm_debug & category))
-   return;
-
-   va_start(args, format);
-   vaf.fmt = format;
-   vaf.va = 
-
-   if (dev)
-   dev_printk(level, dev, DRM_PRINTK_FMT, function_name, prefix,
-  );
-   else
-   printk("%s" DRM_PRINTK_FMT, level, function_name, prefix, );
-
-   va_end(args);
-}
-EXPORT_SYMBOL(drm_dev_printk);
-
-void drm_printk(const char *level, unsigned int category,
-   const char *format, ...)
-{
-   struct va_format vaf;
-   va_list args;
-
-   if (category != DRM_UT_NONE && !(drm_debug & category))
-   return;
-
-   va_start(args, format);
-   vaf.fmt = format;
-   vaf.va = 
-
-   printk("%s" "[" DRM_NAME ":%ps]%s %pV",
-  level, __builtin_return_address(0),
-  strcmp(level, KERN_ERR) == 0 ? " *ERROR*" : "", );
-
-   va_end(args);
-}
-EXPORT_SYMBOL(drm_printk);
-
 /*
  * DRM Minors
  * A DRM device can provide several char-dev interfaces on the DRM-Major. Each
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index a9cfd33..680f359 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -58,6 +58,7 @@
 #include 
 #include 
 #include 
+#include 

 #include 
 #include 
@@ -129,7 +130,6 @@ struct 

[Bug 98996] Counter Strike: Global Offensive performance

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

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

--- Comment #1 from Christoph Haag  ---
Created attachment 128345
  --> https://bugs.freedesktop.org/attachment.cgi?id=128345=edit
screenshot csgo gallium nine

For comparison purposes, here is a screenshot of the windows version with
gallium nine. Same hardware, same software.

Note the high GPU load (It only has large drops when you die and it plays the
"killcam" animation), higher CPU load and much smaller buffer wait times.

Valve said lower performance is expected because of their shader translation
with togl, but this seems extreme.

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

[PATCH v2 3/3] drm/edid: Implement SCDC Read Request capability detection

[Jose Abreu]

Hi Thierry,


Do you think while you are at it you could implement a
set_scrambling() callback? It should be pretty straight forward:
you read the SCDC_TMDS_CONFIG reg, do a mask, and then write it
again.


I think this is an important feature that we should have.


Best regards,

Jose Miguel Abreu


On 02-12-2016 19:24, Thierry Reding wrote:
> From: Thierry Reding 
>
> Sinks that support SCDC can optionally have the capability to initiate
> read requests, which are a mechanism by which a sink can notify its
> source that it should read the Update Flags. If either the sink or the
> source are not Read Request capable, polling of the Update Flags shall
> be employed.
>
> Signed-off-by: Thierry Reding 
> ---
> Changes in v2:
> - new patch
>
>  drivers/gpu/drm/drm_edid.c | 36 
>  include/drm/drm_edid.h |  1 +
>  2 files changed, 37 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 369961597ee5..8211cce3e09e 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3736,6 +3736,42 @@ bool drm_detect_hdmi_scdc(struct edid *edid)
>  EXPORT_SYMBOL(drm_detect_hdmi_scdc);
>  
>  /**
> + * drm_detect_hdmi_scdc_rr_capable - detect whether an HDMI sink is capable 
> of
> + *initiating an SCDC Read Request
> + * @edid: sink EDID information
> + *
> + * Parse the CEA extension according to CEA-861-B to find an HF-VSDB as
> + * defined in HDMI 2.0, section 10.3.2 "HDMI Forum Vendor Specific Data
> + * Block" and checks if the RR_Capable bit (bit 6 of byte 6) is set.
> + *
> + * Returns:
> + * True if the sink is capable of initiating an SCDC Read Request, false
> + * otherwise.
> + */
> +bool drm_detect_hdmi_scdc_rr_capable(struct edid *edid)
> +{
> + unsigned int start, end, i;
> + const u8 *cea;
> +
> + cea = drm_find_cea_extension(edid);
> + if (!cea)
> + return false;
> +
> + if (cea_db_offsets(cea, , ))
> + return false;
> +
> + for_each_cea_db(cea, i, start, end) {
> + if (cea_db_is_hdmi_forum_vsdb([i])) {
> + if (cea[i + 6] & 0x40)
> + return true;
> + }
> + }
> +
> + return false;
> +}
> +EXPORT_SYMBOL(drm_detect_hdmi_scdc_rr_capable);
> +
> +/**
>   * drm_detect_monitor_audio - check monitor audio capability
>   * @edid: EDID block to scan
>   *
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 7ea7e90846d8..d1c29586035e 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -441,6 +441,7 @@ u8 drm_match_cea_mode(const struct drm_display_mode 
> *to_match);
>  enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code);
>  bool drm_detect_hdmi_monitor(struct edid *edid);
>  bool drm_detect_hdmi_scdc(struct edid *edid);
> +bool drm_detect_hdmi_scdc_rr_capable(struct edid *edid);
>  bool drm_detect_monitor_audio(struct edid *edid);
>  bool drm_rgb_quant_range_selectable(struct edid *edid);
>  int drm_add_modes_noedid(struct drm_connector *connector,

[Bug 98996] Counter Strike: Global Offensive performance

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

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

Bug ID: 98996
   Summary: Counter Strike: Global Offensive performance
   Product: Mesa
   Version: git
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: Drivers/Gallium/radeonsi
  Assignee: dri-devel at lists.freedesktop.org
  Reporter: haagch at frickel.club
QA Contact: dri-devel at lists.freedesktop.org

Created attachment 128344
  --> https://bugs.freedesktop.org/attachment.cgi?id=128344=edit
screenshot csgo native

This has always been problematic and since csgo is the second most played game
on Steam I do believe it warrants some attention.

Attached is a screenshot of csgo made with Linux 4.8 and latest mesa git with
gallium hud visible. It's 1920x1080 with the ingame settings set to lowest
(only multithreaded rendering enabled).

I believe the average FPS score pretty well, but the actual low FPS when there
is a lot of action on the screen makes it almost unplayable. The main issue
will probably be the low GPU load, it appears severely bottlenecked.

Hardware should be more than enough to get a smooth experience:
Intel Core i5-6500
XFX Radeon RX 480 XXX OC
2x 8GB Ballistix Sport LT Red DDR4-2400 UDIMM

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

Enabling peer to peer device transactions for PCIe devices

[Jason Gunthorpe]

On Mon, Dec 05, 2016 at 09:40:38AM -0800, Dan Williams wrote:

> > If it is kernel only with physical addresess we don't need a uAPI for
> > it, so I'm not sure #1 is at all related to iopmem.
> >
> > Most people who want #1 probably can just mmap
> > /sys/../pci/../resourceX to get a user handle to it, or pass around
> > __iomem pointers in the kernel. This has been asked for before with
> > RDMA.
> >
> > I'm still not really clear what iopmem is for, or why DAX should ever
> > be involved in this..
> 
> Right, by default remap_pfn_range() does not establish DMA capable
> mappings. You can think of iopmem as remap_pfn_range() converted to
> use devm_memremap_pages(). Given the extra constraints of
> devm_memremap_pages() it seems reasonable to have those DMA capable
> mappings be optionally established via a separate driver.

Except the iopmem driver claims the PCI ID, and presents a block
interface which is really *NOT* what people who have asked for this in
the past have wanted. IIRC it was embedded stuff eg RDMA video
directly out of a capture card or a similar kind of thinking.

It is a good point about devm_memremap_pages limitations, but maybe
that just says to create a /sys/.../resource_dmableX ?

Or is there some reason why people want a filesystem on top of BAR
memory? That does not seem to have been covered yet..

Jason

[PATCH] drm/radeon: don't add files at control minor debugfs directory

[Daniel Vetter]

On Mon, Dec 05, 2016 at 09:48:02AM +0100, Christian König wrote:
> Am 05.12.2016 um 09:39 schrieb Nicolai Stange:
> > Christian König  writes:
> > 
> > > Am 05.12.2016 um 08:27 schrieb Daniel Vetter:
> > > > On Sat, Dec 03, 2016 at 03:47:00PM +0100, Nicolai Stange wrote:
> > > > > Since commit 8a357d10043c ("drm: Nerf DRM_CONTROL nodes"), a
> > > > > struct drm_device's ->control member is always NULL.
> > > > > 
> > > > > In the case of CONFIG_DEBUG_FS=y, radeon_debugfs_add_files() accesses
> > > > > ->control->debugfs_root though. This results in the following Oops:
> > > > > 
> > > > > BUG: unable to handle kernel NULL pointer dereference at 
> > > > > 0018
> > > > > IP: radeon_debugfs_add_files+0x90/0x100 [radeon]
> > > > > PGD 0
> > > > > Oops:  [#1] SMP
> > > > > [...]
> > > > > Call Trace:
> > > > >  ? work_on_cpu+0xb0/0xb0
> > > > >  radeon_fence_driver_init+0x120/0x150 [radeon]
> > > > >  si_init+0x122/0xd50 [radeon]
> > > > >  ? _raw_spin_unlock_irq+0x2c/0x40
> > > > >  ? device_pm_check_callbacks+0xb3/0xc0
> > > > >  radeon_device_init+0x958/0xda0 [radeon]
> > > > >  radeon_driver_load_kms+0x9a/0x210 [radeon]
> > > > >  drm_dev_register+0xa9/0xd0 [drm]
> > > > >  drm_get_pci_dev+0x9c/0x1e0 [drm]
> > > > >  radeon_pci_probe+0xb8/0xe0 [radeon]
> > > > > [...]
> > > > > 
> > > > > Fix this by omitting the drm_debugfs_create_files() call for the
> > > > > control minor debugfs directory which is now non-existent anyway.
> > > > > 
> > > > > Fixes: 8a357d10043c ("drm: Nerf DRM_CONTROL nodes")
> > > > > Signed-off-by: Nicolai Stange 
> > > > Applied to drm-misc with Dave's irc ack, thanks for your patch.
> > > If it's still worth it the patch is Reviewed-by: Christian König
> > > .
> > > 
> > > On the other hand when ->control is always NULL, why do we still have
> > > ->control anyway?
> > Yes, I was wondering about that, too.
> > 
> > Quoting from 8a357d10043c ("drm: Nerf DRM_CONTROL nodes"):
> > 
> >Since I don't like dead uabi, let's remove it. But since this would be
> >a really big change I think it's better to start out small by simply
> >not registering anything. We can garbage-collect the dead code later
> >on, once we're sure it's really not used anywhere.
> > 
> > I'd too prefer compile time errors by purging ->control here. Daniel?
> 
> Seconded.

We're super-late for 4.10, I think that'd would need to be postponened for
4.11. Not need to wait with creating the patches (drm-misc is always
open), but wee need to plug the oopses first.

> > > And BTW: Please double check the other drivers as well.
> ># git grep '\->control' -- drivers/gpu/
> >drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:  
> > adev->ddev->control->debugfs_root,
> >drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:  
> > adev->ddev->control);
> >drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:  
> > adev->ddev->control);
> > 
> > Oops.
> 
> Yeah, that's what I expected as well but Daniel said it would only affect
> qxl.

No idea why I've missed that, I guess coffee wasn't working.

> >drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c:ib_packet->control = (1 
> > << 23) | (1 << 31) |
> >drivers/gpu/drm/drm_drv.c:  return >control;
> > 
> > That's drm_minor_get_slot(dev, type), but grepping for DRM_MINOR_CONTROL
> > doesn't yield anything -> dead code.
> > 
> >drivers/gpu/drm/gma500/psb_intel_sdvo.c:switch 
> > (sdvo->controlled_output) {
> >drivers/gpu/drm/gma500/psb_intel_sdvo.c:
> > psb_intel_sdvo->controlled_output |= SDVO_OUTPUT_TMDS0;
> >drivers/gpu/drm/gma500/psb_intel_sdvo.c:
> > psb_intel_sdvo->controlled_output |= SDVO_OUTPUT_TMDS1;
> >drivers/gpu/drm/gma500/psb_intel_sdvo.c:
> > psb_intel_sdvo->controlled_output |= type;
> >drivers/gpu/drm/gma500/psb_intel_sdvo.c:
> > psb_intel_sdvo->controlled_output |= SDVO_OUTPUT_RGB0;
> >drivers/gpu/drm/gma500/psb_intel_sdvo.c:
> > psb_intel_sdvo->controlled_output |= SDVO_OUTPUT_RGB1;
> >drivers/gpu/drm/gma500/psb_intel_sdvo.c:
> > psb_intel_sdvo->controlled_output |= SDVO_OUTPUT_LVDS0;
> >drivers/gpu/drm/gma500/psb_intel_sdvo.c:
> > psb_intel_sdvo->controlled_output |= SDVO_OUTPUT_LVDS1;
> >drivers/gpu/drm/gma500/psb_intel_sdvo.c:
> > psb_intel_sdvo->controlled_output = 0;
> >drivers/gpu/drm/i915/intel_sdvo.c:  switch (sdvo->controlled_output) 
> > {
> >drivers/gpu/drm/i915/intel_sdvo.c:  
> > intel_sdvo->controlled_output |= SDVO_OUTPUT_TMDS0;
> >drivers/gpu/drm/i915/intel_sdvo.c:  
> > intel_sdvo->controlled_output |= SDVO_OUTPUT_TMDS1;
> >drivers/gpu/drm/i915/intel_sdvo.c:  intel_sdvo->controlled_output |= 
> > type;
> >

[PATCH 13/22] drm: bridge: dw-hdmi: Replace device type with platform quirks

[Jose Abreu]

Hi Laurent,


On 02-12-2016 15:43, Laurent Pinchart wrote:
> Hi Russell,
>
> On Friday 02 Dec 2016 14:24:01 Russell King - ARM Linux wrote:
>> On Fri, Dec 02, 2016 at 01:43:28AM +0200, Laurent Pinchart wrote:
>>> From: Kieran Bingham 
>>>
>>> The dw-hdmi driver declares a dev_type to distinguish platform specific
>>> changes. Replace this with a quirk field, so that the platform can
>>> specify the required quirks for the driver, rather than the driver
>>> becoming conditional on multiple platforms.
>>>
>>> As part of this, we rename the dw-hdmi 'spare' which is defined as the
>>> SVSRET bit in later documentation.
>> I'd really prefer that we did not go down the broken route of adding
>> a set of "quirk" flags - look at what a mess SDHCI has become through
>> allowing that kind of practice.
>>
>> I'd much rather we find a saner structure to this - and we know that
>> the hardware has ID registers in it which can be used (so far) to
>> identify the buggy hardware.
> I'd much prefer something that would allow runtime identification of the 
> device and the corresponding actions to be taken. However, the amount of 
> documentation we have on the DWC HDMI TX IP core (and the associated PHY) is 
> pretty limited, given that Synopsys doesn't make the documentation available 
> publicly. Changes made to the IP core by integrators could complicate this 
> further. I'm trying to gather as much information as possible to make clean 
> the code up, for instance by trying to identify the PHYs used on the various 
> platforms we support. Progress is slow on that front, there isn't enough 
> leaked information available online :-) I haven't given up though, but I'll 
> need more time.
>
> I don't like quirks much either. They are however already used today, even if 
> we trigger them through dev_type instead of quirk flags. This patch came from 
> a previous version found in a BSP that simply sprinkled several if (hdmi-
>> dev_type == RCAR_HDMI) through the code. For instance,
> - if (hdmi->dev_type == RK3288_HDMI)
> + if (hdmi->dev_type == RK3288_HDMI || hdmi->dev_type == RCAR_HDMI)
>   dw_hdmi_phy_enable_spare(hdmi, 1);
>
> which I think is worse than flags as it would quickly degenerate to spaghetti 
> code.
>
> For this specific case, we've managed to identify that on Renesas platforms 
> the bit set by this function is called SVSRET. Its usage isn't clear yet, but 
> I suspect it to control one of the PHY input control signals, like the other 
> bits in the same register. I'm trying to get more information to clean the 
> implementation further, hopefully with a way to determine whether the signal 
> is used based on PHY identification.

SVSRET is a low power mode consumption and is a PHY input signal
as you suggested. Most of the configurable input signals of the
PHY are available by the controller regbank. I don't think it is
possible to detect this at runtime, I think you have at least to
hardcode which version of the PHY you are using.

I would suggest that maybe all the PHY logic should be extracted
and then use callbacks to glue controller and phy. Then,
depending on the PHY you could use empty stubs if, for example, a
given PHY did not support SVSRET. Still, I don't know if this is
the best option. What I do know is that there are a large number
of PHY's with different flavors that can use the same controller.
The controller has different versions also, and each version can
have quirks but I think it would be easier to manage this driver
if we had a clear distinction between PHY and controller.


>
> This is all work in progress, and if anyone has access to any documentation 
> and can provide additional information I'll be grateful.
>
>>> Signed-off-by: Kieran Bingham 
>>> Signed-off-by: Laurent Pinchart
>>> 
>>> ---
>>>
>>>  drivers/gpu/drm/bridge/dw-hdmi.c| 14 ++
>>>  drivers/gpu/drm/bridge/dw-hdmi.h|  4 ++--
>>>  drivers/gpu/drm/imx/dw_hdmi-imx.c   |  3 +--
>>>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c |  2 +-
>>>  include/drm/bridge/dw_hdmi.h| 12 +---
>>>  5 files changed, 15 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c
>>> b/drivers/gpu/drm/bridge/dw-hdmi.c index 06a44a2cdf3b..26607185722f
>>> 100644
>>> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
>>> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
>>> @@ -118,7 +118,6 @@ struct dw_hdmi {
>>> struct drm_bridge bridge;
>>> struct platform_device *audio;
>>>
>>> -   enum dw_hdmi_devtype dev_type;
>>> struct device *dev;
>>> struct clk *isfr_clk;
>>> struct clk *iahb_clk;
>>> @@ -896,11 +895,11 @@ static void dw_hdmi_phy_enable_tmds(struct dw_hdmi
>>> *hdmi, u8 enable)
>>>  HDMI_PHY_CONF0_ENTMDS_MASK);
>>>  }
>>>
>>> -static void dw_hdmi_phy_enable_spare(struct 

Enabling peer to peer device transactions for PCIe devices

[Dan Williams]

On Mon, Dec 5, 2016 at 10:39 AM, Logan Gunthorpe  wrote:
> On 05/12/16 11:08 AM, Dan Williams wrote:
>>
>> I've already recommended that iopmem not be a block device and instead
>> be a device-dax instance. I also don't think it should claim the PCI
>> ID, rather the driver that wants to map one of its bars this way can
>> register the memory region with the device-dax core.
>>
>> I'm not sure there are enough device drivers that want to do this to
>> have it be a generic /sys/.../resource_dmableX capability. It still
>> seems to be an exotic one-off type of configuration.
>
>
> Yes, this is essentially my thinking. Except I think the userspace interface
> should really depend on the device itself. Device dax is a good  choice for
> many and I agree the block device approach wouldn't be ideal.
>
> Specifically for NVME CMB: I think it would make a lot of sense to just hand
> out these mappings with an mmap call on /dev/nvmeX. I expect CMB buffers
> would be volatile and thus you wouldn't need to keep track of where in the
> BAR the region came from. Thus, the mmap call would just be an allocator
> from BAR memory. If device-dax were used, userspace would need to lookup
> which device-dax instance corresponds to which nvme drive.
>

I'm not opposed to mapping /dev/nvmeX.  However, the lookup is trivial
to accomplish in sysfs through /sys/dev/char to find the sysfs path of
the device-dax instance under the nvme device, or if you already have
the nvme sysfs path the dax instance(s) will appear under the "dax"
sub-directory.

[PATCH] drm/i915/gvt: fix deadlock in dispatch_workload()'s error path

[Zhenyu Wang]

On 2016.12.04 23:57:18 +, Eric Engestrom wrote:
> 90d27a1 moved the lock before this error path but forgot to add an
> unlock here.
> 
> Fixes: 90d27a1b180e51ef0713 ("drm/i915/gvt: fix deadlock in workload_thread")
> Cc: Pei Zhang 
> Cc: Zhenyu Wang 
> Signed-off-by: Eric Engestrom 
> ---

Hi, this has been fixed on 
https://cgit.freedesktop.org/drm/drm-intel/commit/?h=drm-intel-next-fixes=53d6f812c0dbf1c9cad89b1c2118e61c13ca9677

Thanks!

>  drivers/gpu/drm/i915/gvt/scheduler.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c 
> b/drivers/gpu/drm/i915/gvt/scheduler.c
> index f898df3..cd13c4b 100644
> --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> @@ -177,6 +177,7 @@ static int dispatch_workload(struct intel_vgpu_workload 
> *workload)
>   rq = i915_gem_request_alloc(dev_priv->engine[ring_id], shadow_ctx);
>   if (IS_ERR(rq)) {
>   gvt_err("fail to allocate gem request\n");
> + mutex_unlock(_priv->drm.struct_mutex);
>   workload->status = PTR_ERR(rq);
>   return workload->status;
>   }
> -- 
> Cheers,
>   Eric
> 

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 163 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161205/9b21b23d/attachment.sig>

Enabling peer to peer device transactions for PCIe devices

[Jason Gunthorpe]

On Sun, Dec 04, 2016 at 07:23:00AM -0600, Stephen Bates wrote:
> Hi All
> 
> This has been a great thread (thanks to Alex for kicking it off) and I
> wanted to jump in and maybe try and put some summary around the
> discussion. I also wanted to propose we include this as a topic for LFS/MM
> because I think we need more discussion on the best way to add this
> functionality to the kernel.
> 
> As far as I can tell the people looking for P2P support in the kernel fall
> into two main camps:
> 
> 1. Those who simply want to expose static BARs on PCIe devices that can be
> used as the source/destination for DMAs from another PCIe device. This
> group has no need for memory invalidation and are happy to use
> physical/bus addresses and not virtual addresses.

I didn't think there was much on this topic except for the CMB
thing.. Even that is really a mapped kernel address..

> I think something like the iopmem patches Logan and I submitted recently
> come close to addressing use case 1. There are some issues around
> routability but based on feedback to date that does not seem to be a
> show-stopper for an initial inclusion.

If it is kernel only with physical addresess we don't need a uAPI for
it, so I'm not sure #1 is at all related to iopmem.

Most people who want #1 probably can just mmap
/sys/../pci/../resourceX to get a user handle to it, or pass around
__iomem pointers in the kernel. This has been asked for before with
RDMA.

I'm still not really clear what iopmem is for, or why DAX should ever
be involved in this..

> For use-case 2 it looks like there are several options and some of them
> (like HMM) have been around for quite some time without gaining
> acceptance. I think there needs to be more discussion on this usecase and
> it could be some time before we get something upstreamable.

AFAIK, hmm makes parts easier, but isn't directly addressing this
need..

I think you need to get ZONE_DEVICE accepted for non-cachable PCI BARs
as the first step.

>From there is pretty clear we the DMA API needs to be updated to
support that use and work can be done to solve the various problems
there on the basis of using ZONE_DEVICE pages to figure out to the
PCI-E end points

Jason

[PATCH v2 1/3] drm: Add SCDC helpers

[Jose Abreu]

Hi Thierry,


On 02-12-2016 19:24, Thierry Reding wrote:
> From: Thierry Reding 
>
> SCDC is a mechanism defined in the HDMI 2.0 specification that allows
> the source and sink devices to communicate.
>
> This commit introduces helpers to access the SCDC and provides the
> symbolic names for the various registers defined in the specification.
>
> Signed-off-by: Thierry Reding 
> ---
> Changes in v2:
> - indent register field definitions for readability (Ville Syrjälä)
> - use GFP_TEMPORARY for temporary buffer allocation (Jani Nikula)
>
>  Documentation/gpu/drm-kms-helpers.rst |  12 
>  drivers/gpu/drm/Kconfig   |   4 ++
>  drivers/gpu/drm/Makefile  |   1 +
>  drivers/gpu/drm/drm_scdc_helper.c | 111 
>  include/drm/drm_scdc_helper.h | 132 
> ++
>  5 files changed, 260 insertions(+)
>  create mode 100644 drivers/gpu/drm/drm_scdc_helper.c
>  create mode 100644 include/drm/drm_scdc_helper.h
>
> diff --git a/Documentation/gpu/drm-kms-helpers.rst 
> b/Documentation/gpu/drm-kms-helpers.rst
> index 03040aa14fe8..759275629fcf 100644
> --- a/Documentation/gpu/drm-kms-helpers.rst
> +++ b/Documentation/gpu/drm-kms-helpers.rst
> @@ -217,6 +217,18 @@ EDID Helper Functions Reference
>  .. kernel-doc:: drivers/gpu/drm/drm_edid.c
> :export:
>  
> +SCDC Helper Functions Reference
> +===
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_scdc_helper.c
> +   :doc: scdc helpers
> +
> +.. kernel-doc:: include/drm/drm_scdc_helper.h
> +   :internal:
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_scdc_helper.c
> +   :export:
> +
>  Rectangle Utilities Reference
>  =
>  
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 95fc0410e129..d0031fe45bab 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -84,6 +84,10 @@ config DRM_FBDEV_EMULATION
>  
> If in doubt, say "Y".
>  
> +config DRM_SCDC
> + bool
> + depends on DRM_KMS_HELPER
> +
>  config DRM_LOAD_EDID_FIRMWARE
>   bool "Allow to specify an EDID data set instead of probing for it"
>   depends on DRM_KMS_HELPER
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 883f3e75cfbc..71c38b6dd546 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -31,6 +31,7 @@ drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o 
> drm_probe_helper.o \
>   drm_kms_helper_common.o drm_dp_dual_mode_helper.o \
>   drm_simple_kms_helper.o drm_modeset_helper.o
>  
> +drm_kms_helper-$(CONFIG_DRM_SCDC) += drm_scdc_helper.o
>  drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
>  drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
>  drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
> diff --git a/drivers/gpu/drm/drm_scdc_helper.c 
> b/drivers/gpu/drm/drm_scdc_helper.c
> new file mode 100644
> index ..fe0e1211e873
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_scdc_helper.c
> @@ -0,0 +1,111 @@
> +/*
> + * Copyright (c) 2015 NVIDIA Corporation. All rights reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sub license,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include 
> +
> +#include 
> +
> +/**
> + * DOC: scdc helpers
> + *
> + * Status and Control Data Channel (SCDC) is a mechanism introduced by the
> + * HDMI 2.0 specification. It is a point-to-point protocol that allows the
> + * HDMI source and HDMI sink to exchange data. The same I2C interface that
> + * is used to access EDID serves as the transport mechanism for SCDC.
> + */
> +
> +#define SCDC_I2C_SLAVE_ADDRESS 0x54
> +
> +/**
> + * drm_scdc_read - read a block of data from SCDC
> + * @adapter: I2C controller
> + * @offset: start offset of block to read
> + * @buffer: return location for the block to read
> + * @size: size of the block to