[Bug 110637] Any OpenCL application causes "*ERROR* ring gfx timeout" on Vega 64

[bugzilla-daemon]

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

--- Comment #3 from Alex Deucher  ---
More likely a bug in the mesa OpenCL code.  If you want functional OpenCL, you
should use the ROCm OpenCL packages.

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

[Bug 110641] lm_sensors reports "ERROR: Can't get value of subfeature in0_input: Can't read"

[bugzilla-daemon]

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

--- Comment #6 from Bong Cosca  ---
So does it make sense to expose VDDGFX and VDDNB at all? Or should this be
fixed in lm_sensors?

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

[Bug 110637] Any OpenCL application causes "*ERROR* ring gfx timeout" on Vega 64

[bugzilla-daemon]

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

Alexander Mezin  changed:

   What|Removed |Added

Summary|Enabling OpenCL in  |Any OpenCL application
   |Libreoffice kills Vega 64   |causes "*ERROR* ring gfx
   ||timeout" on Vega 64

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

[Bug 110637] Enabling OpenCL in Libreoffice kills Vega 64

[bugzilla-daemon]

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

--- Comment #1 from Alexander Mezin  ---
And the same thing with 4.19.40. And with manually built drm-next.

Also, not only libreoffice, but literally any application that uses OpenCL
triggers the same problem. It seems that I just can't use OpenCL at all.

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

Re: [git pull] drm for 5.2-rc1

[Linus Torvalds]

Dave,

On Wed, May 8, 2019 at 8:28 PM Dave Airlie  wrote:
>
> This is the main drm pull request for 5.2.

Thanks. I've merged it, but I got a couple of conflicts with fixes
(reverts) to mainline in the meantime.

The one to the i915 driver you seem to have applied again (after the
function was moved and renamed).

The one to the virtgpu driver, I really don't know if is needed any
more. I suspect I completely unnecessarily merged that
virtgpu_gem_prime_import_sg_table() function that came in because I
decided to do the merge of the revert.

It's a trivial function that just returns an error, and your tree just
leaves it as NULL, and I suspect my merge doesn't hurt, but it also
probably doesn't matter.

So you should check my merge.

Thanks,

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

Re: [git pull] drm for 5.2-rc1

[pr-tracker-bot]

The pull request you sent on Thu, 9 May 2019 13:28:07 +1000:

> git://anongit.freedesktop.org/drm/drm tags/drm-next-2019-05-09

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/a2d635decbfa9c1e4ae15cb05b68b2559f7f827c

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 110641] lm_sensors reports "ERROR: Can't get value of subfeature in0_input: Can't read"

[bugzilla-daemon]

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

--- Comment #5 from Alex Deucher  ---
(In reply to Bong Cosca from comment #4)
> AMD A6-7400K Radeon R5

No hwmon support for voltage on that family.

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

[Bug 110641] lm_sensors reports "ERROR: Can't get value of subfeature in0_input: Can't read"

[bugzilla-daemon]

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

--- Comment #4 from Bong Cosca  ---
AMD A6-7400K Radeon R5

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

Re: [PATCH v2 00/17] kunit: introduce KUnit, the Linux kernel unit testing framework

[Theodore Ts'o]

On Wed, May 08, 2019 at 07:13:59PM -0700, Frank Rowand wrote:
> > If you want to use vice grips as a hammer, screwdriver, monkey wrench,
> > etc.  there's nothing stopping you from doing that.  But it's not fair
> > to object to other people who might want to use better tools.
> > 
> > The reality is that we have a lot of testing tools.  It's not just
> > kselftests.  There is xfstests for file system code, blktests for
> > block layer tests, etc.   We use the right tool for the right job.
> 
> More specious arguments.

Well, *I* don't think they are specious; so I think we're going to
have to agree to disagree.

Cheers,

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

[Bug 110472] Graphical Fault (Desktop Freeze) on Specific OpenGL Application

[bugzilla-daemon]

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

--- Comment #4 from gpiza...@javaman.net ---
(In reply to Timothy Arceri from comment #3)
> It would be helpful if you were able to get an apitrace [1] of the problem.
> 
> [1] https://github.com/apitrace/apitrace

Thank you for replying!

My apitrace file is unnaturally large (418MB) so I uploaded it to Google Drive
instead of this site. The apitrace log is also large (24MB), so it's also in
the shared folder.

Link:
https://drive.google.com/drive/folders/1rulVIW3dIB3RYA3RlIorLFufdQxLOYrn?usp=sharing

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

[Bug 110472] Graphical Fault (Desktop Freeze) on Specific OpenGL Application

[bugzilla-daemon]

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

--- Comment #3 from Timothy Arceri  ---
It would be helpful if you were able to get an apitrace [1] of the problem.

[1] https://github.com/apitrace/apitrace

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

Re: [PATCH RFC 4/6] ARM: dts: msm8974: add display support

[Rob Clark]

On Wed, May 8, 2019 at 7:16 PM Brian Masney  wrote:
>
> On Mon, May 06, 2019 at 11:39:02PM -0700, Bjorn Andersson wrote:
> > On Sun 05 May 06:04 PDT 2019, Brian Masney wrote:
> > > diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi 
> > > b/arch/arm/boot/dts/qcom-msm8974.dtsi
> > [..]
> > > +   clocks = < MDSS_MDP_CLK>,
> > > +< MDSS_AHB_CLK>,
> > > +< MDSS_AXI_CLK>,
> > > +< MDSS_BYTE0_CLK>,
> > > +< MDSS_PCLK0_CLK>,
> > > +< MDSS_ESC0_CLK>,
> > > +< MMSS_MISC_AHB_CLK>;
> > > +   clock-names = "mdp_core",
> > > + "iface",
> > > + "bus",
> > > + "byte",
> > > + "pixel",
> > > + "core",
> > > + "core_mmss";
> >
> > Unless I enable MMSS_MMSSNOC_AXI_CLK and MMSS_S0_AXI_CLK I get some
> > underrun error from DSI. You don't see anything like this?
> >
> > (These clocks are controlled by msm_bus downstream and should be driven
> > by interconnect upstream)
> >
> >
> > Apart from this, I think this looks nice. Happy to see the progress.
>
> No, I'm not seeing an underrun errors from the DSI. I think the clocks
> are fine since I'm able to get this working with 4.17 using these same
> clocks. I just sent out v2 and the cover letter has some details, along
> with the full dmesg.

since we don't have interconnect driver for 8974, I guess there is
some chance that things work or not based on how lk leaves things?

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

[Bug 110635] briefly flashing corruption when playing various OGL games

[bugzilla-daemon]

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

--- Comment #1 from Timothy Arceri  ---
Looks like it might be the same as bug #110575.

Do either of these go away when you run steam with the following:

R600_DEBUG=zerovram steam

Or setting the launch options for the game in steam to:

R600_DEBUG=zerovram %command%

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

[Bug 110641] lm_sensors reports "ERROR: Can't get value of subfeature in0_input: Can't read"

[bugzilla-daemon]

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

--- Comment #3 from Alex Deucher  ---
What chip do you have?  Voltage hwmon is not implemented on all APUs.

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

Re: [PATCH v2 00/17] kunit: introduce KUnit, the Linux kernel unit testing framework

[Theodore Ts'o]

On Wed, May 08, 2019 at 05:43:35PM -0700, Frank Rowand wrote:
> kselftest provides a mechanism for in-kernel tests via modules.  For
> example, see:
> 
>   tools/testing/selftests/vm/run_vmtests invokes:
> tools/testing/selftests/vm/test_vmalloc.sh
>   loads module:
> test_vmalloc
> (which is built from lib/test_vmalloc.c if CONFIG_TEST_VMALLOC)

The majority of the kselftests are implemented as userspace programs.

You *can* run in-kernel test using modules; but there is no framework
for the in-kernel code found in the test modules, which means each of
the in-kernel code has to create their own in-kernel test
infrastructure.  

That's much like saying you can use vice grips to turn a nut or
bolt-head.  You *can*, but it might be that using a monkey wrench
would be a much better tool that is much easier.

What would you say to a wood worker objecting that a toolbox should
contain a monkey wrench because he already knows how to use vise
grips, and his tiny brain shouldn't be forced to learn how to use a
wrench when he knows how to use a vise grip, which is a perfectly good
tool?

If you want to use vice grips as a hammer, screwdriver, monkey wrench,
etc.  there's nothing stopping you from doing that.  But it's not fair
to object to other people who might want to use better tools.

The reality is that we have a lot of testing tools.  It's not just
kselftests.  There is xfstests for file system code, blktests for
block layer tests, etc.   We use the right tool for the right job.

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

Re: [PATCH v2 00/17] kunit: introduce KUnit, the Linux kernel unit testing framework

[Theodore Ts'o]

On Wed, May 08, 2019 at 05:58:49PM -0700, Frank Rowand wrote:
> 
> If KUnit is added to the kernel, and a subsystem that I am submitting
> code for has chosen to use KUnit instead of kselftest, then yes, I do
> *have* to use KUnit if my submission needs to contain a test for the
> code unless I want to convince the maintainer that somehow my case
> is special and I prefer to use kselftest instead of KUnittest.

That's going to be between you and the maintainer.  Today, if you want
to submit a substantive change to xfs or ext4, you're going to be
asked to create test for that new feature using xfstests.  It doesn't
matter that xfstests isn't in the kernel --- if that's what is
required by the maintainer.

> > supposed to be a simple way to run a large number of small tests that
> > for specific small components in a system.
> 
> kselftest also supports running a subset of tests.  That subset of tests
> can also be a large number of small tests.  There is nothing inherent
> in KUnit vs kselftest in this regard, as far as I am aware.

The big difference is that kselftests are driven by a C program that
runs in userspace.  Take a look at 
tools/testing/selftests/filesystem/dnotify_test.c
it has a main(int argc, char *argv) function.

In contrast, KUnit are fragments of C code which run in the kernel;
not in userspace.  This allows us to test internal functions inside
complex file system (such as the block allocator in ext4) directly.
This makes it *fundamentally* different from kselftest.

Cheers,

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

[Bug 110641] lm_sensors reports "ERROR: Can't get value of subfeature in0_input: Can't read"

[bugzilla-daemon]

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

--- Comment #2 from Bong Cosca  ---
It wasn't the case before when VDDGFX and VDDNB were not exposed. Are these
(and the missing in0_input/in1_input probes) present on APUs?

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

Re: [PATCH 2/3] drm/bridge: add it6505 driver

[Sam Ravnborg]

Hi allen.

Thanks for this fine patch.
A few comments follows.

Consider to use DRM_DEV_ERROR and friends.
Then you get the devicename included in logging
and this makes it much easier to find relevant entries.

On Wed, May 08, 2019 at 03:48:41PM +0800, allen wrote:
> From: Allen Chen 
> 
> This adds support for the iTE IT6505.
> This device can convert DPI signal to DP output.
> 
> Signed-off-by: Jitao Shi 
> Signed-off-by: Yilun Lin 
> Signed-off-by: Allen Chen 
> ---
>  drivers/gpu/drm/bridge/Kconfig  |   22 +
>  drivers/gpu/drm/bridge/Makefile |1 +
>  drivers/gpu/drm/bridge/ite-it6505.c | 2637 
> +++
>  3 files changed, 2660 insertions(+)
>  create mode 100644 drivers/gpu/drm/bridge/ite-it6505.c
> 
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index 9c9c4df..d12e48c 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -43,6 +43,28 @@ config DRM_DUMB_VGA_DAC
> Support for non-programmable RGB to VGA DAC bridges, such as ADI
> ADV7123, TI THS8134 and THS8135 or passive resistor ladder DACs.
>  
> +config DRM_ITE_IT6505
> + tristate "ITE IT6505 DP bridge"
> + depends on OF
> + select DRM_KMS_HELPER
> + help
> +   ITE IT6505 DP bridge chip driver.

Why is it relevant to have these features as features
that can be enabed/disabled on Kconfig level?
It is likely more flexible to do it run-time
if needed to turn them off.
And it would simplify the code.

> +
> +config DRM_ITE_IT6505_ENHDCP
> + tristate "Enable IT6505 HDCP function"
> + depends on DRM_ITE_IT6505
> + default y
> +
> +config DRM_ITE_IT6505_ENAUD
> +tristate "Enable IT6505 audio function"
> +depends on DRM_ITE_IT6505
> +default y
> +
> +config DRM_ITE_IT6505_ENPWRONOFF
> +tristate "Enable IT6505 power on/off function"
> +depends on DRM_ITE_IT6505
> +default y
> +
>  config DRM_LVDS_ENCODER
>   tristate "Transparent parallel to LVDS encoder support"
>   depends on OF
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index 4934fcf..f5abca5 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -2,6 +2,7 @@
>  obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
>  obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o
>  obj-$(CONFIG_DRM_DUMB_VGA_DAC) += dumb-vga-dac.o
> +obj-$(CONFIG_DRM_ITE_IT6505) += ite-it6505.o
>  obj-$(CONFIG_DRM_LVDS_ENCODER) += lvds-encoder.o
>  obj-$(CONFIG_DRM_MEGACHIPS_STDP_GE_B850V3_FW) += 
> megachips-stdp-ge-b850v3-fw.o
>  obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c 
> b/drivers/gpu/drm/bridge/ite-it6505.c
> new file mode 100644
> index 000..13079a8
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
> @@ -0,0 +1,2637 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
Please put a blank line between the individual blocks
of include files.
So all #include  +
> +#define AX 0
> +#define BX 1
> +#define AUDSEL I2S
> +#define AUDTYPE LPCM
> +#define AUDFS AUD48K
> +#define AUDCH 2
> +/* 0: Standard I2S;1: 32bit I2S */
> +#define I2SINPUTFMT 1
> +/* 0: Left-justified;1: Right-justified */
> +#define I2SJUSTIFIED 0
> +/* 0: Data delay 1T correspond to WS;1: No data delay correspond to WS */
> +#define I2SDATADELAY 0
> +/* 0: is left channel;1: is right channel */
> +#define I2SWSCHANNEL 0
> +/* 0: MSB shift first;1: LSB shift first */
> +#define I2SDATASEQ 0
> +
> +#define LANESWAP 0
> +#define LANE 4
> +#define _HBR 1
> +#define ENHFRAME 1
> +#define ENSSC 1
> +
> +#define FLAGTRAINDOWN 100
> +#define TRAINFAILCNT 5
> +#define AUX_WAIT_TIMEOUT_MS 15
> +#define PCLK_DELAY 1
> +#define PCLK_INV 0
> +#define EDIDRETRYTIME 5
> +#define SHOWVIDEOTIMING 2
> +#define PWROFFRETRYTIME 5
> +
> +/* AX or BX */
> +#define CHIP_VERSION BX

If this driver is for BX only then drop the AX releated code.
This would simplify the driver.

If this is really needed then provide an empty variant of
it6505_termination() for AX, to simplify the call sites.

And it6505_termination() calls for two functions so we
void a bool parameter that makes the function do two
different things.

> +
> +/* if use this define will power on in probe */
> +/* #define TEST_MODE */
> +
> +/* if use this define will enable AUX debug option */
> +/* #define ENAUX_TRANSFER_DEBUG */
> +
> +/* if use this define will enable SHA debug */
> +/* #define SHA_DEBUG */
Consider how much of this really belongs in a production driver.
If relevant consider to add some 

[PULL] drm-misc-next-fixes

[Sean Paul]

Hi Da.*,
So last week when I said we were ready for merge window... I lied. Lots of stuff
to sneak in this week including 6 patches that came from -misc-next. Fortunately
they _just_ missed the feature freeze so I was able to tag and merge them here.
Most of what is here is panfrost fixes, which is understandable given its age, I
expect this trend will continue this release as it becomes more mature.

The sole msm patch is here b/c that's all Rob and I have for -fixes in msm atm
and it was easier to just stuff it in here.

drm-misc-next-fixes-2019-05-08:
- A handful of fixes from -next that just missed feature freeze
- More panfrost fixes that went directly in -misc-next-fixes (various)
- Fix searchpaths during build (Masahiro)
- msm patch to fix the driver for chips without zap shader (Rob)
- Fix freeing imported buffers in drm_gem_cma_free_object() (Noralf)

Cc: Masahiro Yamada 
Cc: Rob Clark 
Cc: Noralf Trønnes 

Cheers, Sean


The following changes since commit 761e473f6b23f206862d904a1a5fcbc012656b47:

  drm/gem: Fix sphinx warnings (2019-04-25 10:02:10 -0400)

are available in the Git repository at:

  git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-fixes-2019-05-08

for you to fetch changes up to 15273ffd7efdf6e9f21c4e4beef6539229167343:

  drm/msm/a6xx: No zap shader is not an error (2019-05-08 16:00:54 -0400)


- A handful of fixes from -next that just missed feature freeze
- More panfrost fixes that went directly in -misc-next-fixes (various)
- Fix searchpaths during build (Masahiro)
- msm patch to fix the driver for chips without zap shader (Rob)
- Fix freeing imported buffers in drm_gem_cma_free_object() (Noralf)

Cc: Masahiro Yamada 
Cc: Rob Clark 
Cc: Noralf Trønnes 


Mario Kleiner (1):
  drm: Fix timestamp docs for variable refresh properties.

Masahiro Yamada (1):
  drm: prefix header search paths with $(srctree)/

Noralf Trønnes (1):
  drm/cma-helper: Fix drm_gem_cma_free_object()

Rob Clark (1):
  drm/msm/a6xx: No zap shader is not an error

Robin Murphy (4):
  drm/panfrost: Set DMA masks earlier
  drm/panfrost: Disable PM on probe failure
  drm/panfrost: Don't scream about deferred probe
  drm/panfrost: Show stored feature registers

Sean Paul (1):
  Merge panfrost-fixes into drm-misc-next-fixes

Steven Price (2):
  drm/panfrost: Add missing include
  drm/panfrost: depend on !GENERIC_ATOMIC64 when using COMPILE_TEST

Tomeu Vizoso (2):
  drm/panfrost: Prevent concurrent resets
  drm/panfrost: Add sanity checks to submit IOCTL

Vicente Bergas (1):
  drm/rockchip: shutdown drm subsystem on shutdown

YueHaibing (1):
  drm/panfrost: Make panfrost_gem_free_object() static

 drivers/gpu/drm/amd/amdgpu/Makefile |  2 +-
 drivers/gpu/drm/arm/display/komeda/Makefile |  4 +--
 drivers/gpu/drm/drm_connector.c |  6 
 drivers/gpu/drm/drm_gem_cma_helper.c|  8 ++---
 drivers/gpu/drm/i915/gvt/Makefile   |  2 +-
 drivers/gpu/drm/msm/Makefile|  6 ++--
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c   |  1 +
 drivers/gpu/drm/nouveau/Kbuild  |  8 ++---
 drivers/gpu/drm/panfrost/Kconfig|  2 +-
 drivers/gpu/drm/panfrost/panfrost_devfreq.c |  1 +
 drivers/gpu/drm/panfrost/panfrost_device.c  |  1 +
 drivers/gpu/drm/panfrost/panfrost_device.h  |  1 +
 drivers/gpu/drm/panfrost/panfrost_drv.c | 47 ++---
 drivers/gpu/drm/panfrost/panfrost_gem.c |  2 +-
 drivers/gpu/drm/panfrost/panfrost_gpu.c | 19 +++-
 drivers/gpu/drm/panfrost/panfrost_job.c |  4 +++
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c |  9 ++
 17 files changed, 75 insertions(+), 48 deletions(-)

-- 
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/5] drm/msm/dpu: clean up references of DPU custom bus scaling

[Rob Clark]

From: Jayant Shekhar 

Since the upstream interconnect bus framework has landed
upstream, the existing references of custom bus scaling
needs to be cleaned up.

Changes in v2:
- Fixed build error due to partial clean up

Changes in v3:
- Condense multiple lines into a single line (Sean Paul)

Changes in v4-v7:
- None

Signed-off-by: Sravanthi Kollukuduru 
Signed-off-by: Jayant Shekhar 
Signed-off-by: Rob Clark 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 174 +++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h |   4 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  |  11 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h |  22 +--
 4 files changed, 80 insertions(+), 131 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
index 9f20f397f77d..e231c37a9dbb 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
@@ -77,7 +77,6 @@ static void _dpu_core_perf_calc_crtc(struct dpu_kms *kms,
struct dpu_core_perf_params *perf)
 {
struct dpu_crtc_state *dpu_cstate;
-   int i;
 
if (!kms || !kms->catalog || !crtc || !state || !perf) {
DPU_ERROR("invalid parameters\n");
@@ -88,35 +87,24 @@ static void _dpu_core_perf_calc_crtc(struct dpu_kms *kms,
memset(perf, 0, sizeof(struct dpu_core_perf_params));
 
if (!dpu_cstate->bw_control) {
-   for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
-   perf->bw_ctl[i] = kms->catalog->perf.max_bw_high *
+   perf->bw_ctl = kms->catalog->perf.max_bw_high *
1000ULL;
-   perf->max_per_pipe_ib[i] = perf->bw_ctl[i];
-   }
+   perf->max_per_pipe_ib = perf->bw_ctl;
perf->core_clk_rate = kms->perf.max_core_clk_rate;
} else if (kms->perf.perf_tune.mode == DPU_PERF_MODE_MINIMUM) {
-   for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
-   perf->bw_ctl[i] = 0;
-   perf->max_per_pipe_ib[i] = 0;
-   }
+   perf->bw_ctl = 0;
+   perf->max_per_pipe_ib = 0;
perf->core_clk_rate = 0;
} else if (kms->perf.perf_tune.mode == DPU_PERF_MODE_FIXED) {
-   for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
-   perf->bw_ctl[i] = kms->perf.fix_core_ab_vote;
-   perf->max_per_pipe_ib[i] = kms->perf.fix_core_ib_vote;
-   }
+   perf->bw_ctl = kms->perf.fix_core_ab_vote;
+   perf->max_per_pipe_ib = kms->perf.fix_core_ib_vote;
perf->core_clk_rate = kms->perf.fix_core_clk_rate;
}
 
DPU_DEBUG(
-   "crtc=%d clk_rate=%llu core_ib=%llu core_ab=%llu llcc_ib=%llu 
llcc_ab=%llu mem_ib=%llu mem_ab=%llu\n",
+   "crtc=%d clk_rate=%llu core_ib=%llu core_ab=%llu\n",
crtc->base.id, perf->core_clk_rate,
-   perf->max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_MNOC],
-   perf->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_MNOC],
-   perf->max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_LLCC],
-   perf->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_LLCC],
-   perf->max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_EBI],
-   perf->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_EBI]);
+   perf->max_per_pipe_ib, perf->bw_ctl);
 }
 
 int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
@@ -129,7 +117,6 @@ int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
struct dpu_crtc_state *dpu_cstate;
struct drm_crtc *tmp_crtc;
struct dpu_kms *kms;
-   int i;
 
if (!crtc || !state) {
DPU_ERROR("invalid crtc\n");
@@ -151,31 +138,25 @@ int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
/* obtain new values */
_dpu_core_perf_calc_crtc(kms, crtc, state, _cstate->new_perf);
 
-   for (i = DPU_CORE_PERF_DATA_BUS_ID_MNOC;
-   i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
-   bw_sum_of_intfs = dpu_cstate->new_perf.bw_ctl[i];
-   curr_client_type = dpu_crtc_get_client_type(crtc);
+   bw_sum_of_intfs = dpu_cstate->new_perf.bw_ctl;
+   curr_client_type = dpu_crtc_get_client_type(crtc);
 
-   drm_for_each_crtc(tmp_crtc, crtc->dev) {
-   if (tmp_crtc->enabled &&
-   (dpu_crtc_get_client_type(tmp_crtc) ==
-   curr_client_type) &&
-   (tmp_crtc != crtc)) {
-   struct dpu_crtc_state *tmp_cstate =
-   to_dpu_crtc_state(tmp_crtc->state);
-
-   DPU_DEBUG("crtc:%d bw:%llu ctrl:%d\n",
-

[PATCH 2/5] drm/msm/dpu: Integrate interconnect API in MDSS

[Rob Clark]

From: Jayant Shekhar 

The interconnect framework is designed to provide a
standard kernel interface to control the settings of
the interconnects on a SoC.

The interconnect API uses a consumer/provider-based model,
where the providers are the interconnect buses and the
consumers could be various drivers.

MDSS is one of the interconnect consumers which uses the
interconnect APIs to get the path between endpoints and
set its bandwidth requirement for the given interconnected
path.

Changes in v2:
- Remove error log and unnecessary check (Jordan Crouse)

Changes in v3:
- Code clean involving variable name change, removal
  of extra paranthesis and variables (Matthias Kaehlcke)

Changes in v4:
- Add comments, spacings, tabs, proper port name
  and icc macro (Georgi Djakov)

Changes in v5:
- Commit text and parenthesis alignment (Georgi Djakov)

Changes in v6:
- Change to new icc_set API's (Doug Anderson)

Changes in v7:
- Fixed a typo

Signed-off-by: Sravanthi Kollukuduru 
Signed-off-by: Jayant Shekhar 
Signed-off-by: Rob Clark 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 49 ++--
 1 file changed, 45 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
index 7316b4ab1b85..e3c56ccd7357 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
@@ -4,11 +4,15 @@
  */
 
 #include "dpu_kms.h"
+#include 
 
 #define to_dpu_mdss(x) container_of(x, struct dpu_mdss, base)
 
 #define HW_INTR_STATUS 0x0010
 
+/* Max BW defined in KBps */
+#define MAX_BW 680
+
 struct dpu_irq_controller {
unsigned long enabled_mask;
struct irq_domain *domain;
@@ -21,8 +25,30 @@ struct dpu_mdss {
u32 hwversion;
struct dss_module_power mp;
struct dpu_irq_controller irq_controller;
+   struct icc_path *path[2];
+   u32 num_paths;
 };
 
+static int dpu_mdss_parse_data_bus_icc_path(struct drm_device *dev,
+   struct dpu_mdss *dpu_mdss)
+{
+   struct icc_path *path0 = of_icc_get(dev->dev, "mdp0-mem");
+   struct icc_path *path1 = of_icc_get(dev->dev, "mdp1-mem");
+
+   if (IS_ERR(path0))
+   return PTR_ERR(path0);
+
+   dpu_mdss->path[0] = path0;
+   dpu_mdss->num_paths = 1;
+
+   if (!IS_ERR(path1)) {
+   dpu_mdss->path[1] = path1;
+   dpu_mdss->num_paths++;
+   }
+
+   return 0;
+}
+
 static void dpu_mdss_irq(struct irq_desc *desc)
 {
struct dpu_mdss *dpu_mdss = irq_desc_get_handler_data(desc);
@@ -134,7 +160,11 @@ static int dpu_mdss_enable(struct msm_mdss *mdss)
 {
struct dpu_mdss *dpu_mdss = to_dpu_mdss(mdss);
struct dss_module_power *mp = _mdss->mp;
-   int ret;
+   int ret, i;
+   u64 avg_bw = dpu_mdss->num_paths ? MAX_BW / dpu_mdss->num_paths : 0;
+
+   for (i = 0; i < dpu_mdss->num_paths; i++)
+   icc_set_bw(dpu_mdss->path[i], avg_bw, kBps_to_icc(MAX_BW));
 
ret = msm_dss_enable_clk(mp->clk_config, mp->num_clk, true);
if (ret)
@@ -147,12 +177,15 @@ static int dpu_mdss_disable(struct msm_mdss *mdss)
 {
struct dpu_mdss *dpu_mdss = to_dpu_mdss(mdss);
struct dss_module_power *mp = _mdss->mp;
-   int ret;
+   int ret, i;
 
ret = msm_dss_enable_clk(mp->clk_config, mp->num_clk, false);
if (ret)
DPU_ERROR("clock disable failed, ret:%d\n", ret);
 
+   for (i = 0; i < dpu_mdss->num_paths; i++)
+   icc_set_bw(dpu_mdss->path[i], 0, 0);
+
return ret;
 }
 
@@ -163,6 +196,7 @@ static void dpu_mdss_destroy(struct drm_device *dev)
struct dpu_mdss *dpu_mdss = to_dpu_mdss(priv->mdss);
struct dss_module_power *mp = _mdss->mp;
int irq;
+   int i;
 
pm_runtime_suspend(dev->dev);
pm_runtime_disable(dev->dev);
@@ -172,6 +206,9 @@ static void dpu_mdss_destroy(struct drm_device *dev)
msm_dss_put_clk(mp->clk_config, mp->num_clk);
devm_kfree(>dev, mp->clk_config);
 
+   for (i = 0; i < dpu_mdss->num_paths; i++)
+   icc_put(dpu_mdss->path[i]);
+
if (dpu_mdss->mmio)
devm_iounmap(>dev, dpu_mdss->mmio);
dpu_mdss->mmio = NULL;
@@ -211,6 +248,10 @@ int dpu_mdss_init(struct drm_device *dev)
}
dpu_mdss->mmio_len = resource_size(res);
 
+   ret = dpu_mdss_parse_data_bus_icc_path(dev, dpu_mdss);
+   if (ret)
+   return ret;
+
mp = _mdss->mp;
ret = msm_dss_parse_clock(pdev, mp);
if (ret) {
@@ -232,14 +273,14 @@ int dpu_mdss_init(struct drm_device *dev)
irq_set_chained_handler_and_data(irq, dpu_mdss_irq,
 dpu_mdss);
 
+   priv->mdss = _mdss->base;
+
pm_runtime_enable(dev->dev);
 

[PATCH 5/5] drm/msm/mdp5: Use the interconnect API

[Rob Clark]

From: Georgi Djakov 

Signed-off-by: Georgi Djakov 
Signed-off-by: Rob Clark 
---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c 
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index 97179bec8902..54d2b4c2b09f 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -16,6 +16,7 @@
  * this program.  If not, see .
  */
 
+#include 
 #include 
 
 #include "msm_drv.h"
@@ -1050,6 +1051,19 @@ static const struct component_ops mdp5_ops = {
 
 static int mdp5_dev_probe(struct platform_device *pdev)
 {
+   struct icc_path *path0 = of_icc_get(>dev, "port0");
+   struct icc_path *path1 = of_icc_get(>dev, "port1");
+   struct icc_path *path_rot = of_icc_get(>dev, "rotator");
+
+   if (IS_ERR(path0))
+   return PTR_ERR(path0);
+   icc_set_bw(path0, 0, MBps_to_icc(6400));
+
+   if (!IS_ERR(path1))
+   icc_set_bw(path1, 0, MBps_to_icc(6400));
+   if (!IS_ERR(path_rot))
+   icc_set_bw(path_rot, 0, MBps_to_icc(6400));
+
DBG("");
return component_add(>dev, _ops);
 }
-- 
2.20.1

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

[PATCH 4/5] drm/msm/dpu: add icc voting in dpu_mdss_init

[Rob Clark]

From: Abhinav Kumar 

dpu_mdss_destroy() can get called not just from
msm_drm_uninit() but also from msm_drm_bind() in case
of any failures.

dpu_mdss_destroy() removes the icc voting by calling
icc_put. This could accidentally remove the voting
done by pm_runtime_enable.

To make the voting balanced add a minimum vote in
dpu_mdss_init() to avoid any unclocked access.

This change depends on the following patch which
introduces interconnect binding to MDSS driver:

https://patchwork.codeaurora.org/patch/708155/

Signed-off-by: Abhinav Kumar 
Reviewed-by: Sean Paul 
Signed-off-by: Rob Clark 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
index e3c56ccd7357..b42537a663f7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
@@ -49,6 +49,16 @@ static int dpu_mdss_parse_data_bus_icc_path(struct 
drm_device *dev,
return 0;
 }
 
+static void dpu_mdss_icc_request_bw(struct msm_mdss *mdss)
+{
+   struct dpu_mdss *dpu_mdss = to_dpu_mdss(mdss);
+   int i;
+   u64 avg_bw = dpu_mdss->num_paths ? MAX_BW / dpu_mdss->num_paths : 0;
+
+   for (i = 0; i < dpu_mdss->num_paths; i++)
+   icc_set_bw(dpu_mdss->path[i], avg_bw, kBps_to_icc(MAX_BW));
+}
+
 static void dpu_mdss_irq(struct irq_desc *desc)
 {
struct dpu_mdss *dpu_mdss = irq_desc_get_handler_data(desc);
@@ -160,11 +170,9 @@ static int dpu_mdss_enable(struct msm_mdss *mdss)
 {
struct dpu_mdss *dpu_mdss = to_dpu_mdss(mdss);
struct dss_module_power *mp = _mdss->mp;
-   int ret, i;
-   u64 avg_bw = dpu_mdss->num_paths ? MAX_BW / dpu_mdss->num_paths : 0;
+   int ret;
 
-   for (i = 0; i < dpu_mdss->num_paths; i++)
-   icc_set_bw(dpu_mdss->path[i], avg_bw, kBps_to_icc(MAX_BW));
+   dpu_mdss_icc_request_bw(mdss);
 
ret = msm_dss_enable_clk(mp->clk_config, mp->num_clk, true);
if (ret)
@@ -277,6 +285,8 @@ int dpu_mdss_init(struct drm_device *dev)
 
pm_runtime_enable(dev->dev);
 
+   dpu_mdss_icc_request_bw(priv->mdss);
+
pm_runtime_get_sync(dev->dev);
dpu_mdss->hwversion = readl_relaxed(dpu_mdss->mmio);
pm_runtime_put_sync(dev->dev);
-- 
2.20.1

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

[PATCH 3/5] dt-bindings: msm/disp: Introduce interconnect bindings for MDSS on SDM845

[Rob Clark]

From: Jayant Shekhar 

Add interconnect properties such as interconnect provider specifier
, the edge source and destination ports which are required by the
interconnect API to configure interconnect path for MDSS.

Changes in v2:
- None

Changes in v3:
- Remove common property definitions (Rob Herring)

Changes in v4:
- Use port macros and change port string names (Georgi Djakov)

Changes in v5-v7:
- None

Signed-off-by: Sravanthi Kollukuduru 
Signed-off-by: Jayant Shekhar 
Reviewed-by: Rob Herring 
Signed-off-by: Rob Clark 
---
 Documentation/devicetree/bindings/display/msm/dpu.txt | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt 
b/Documentation/devicetree/bindings/display/msm/dpu.txt
index ad2e8830324e..a61dd40f3792 100644
--- a/Documentation/devicetree/bindings/display/msm/dpu.txt
+++ b/Documentation/devicetree/bindings/display/msm/dpu.txt
@@ -28,6 +28,11 @@ Required properties:
 - #address-cells: number of address cells for the MDSS children. Should be 1.
 - #size-cells: Should be 1.
 - ranges: parent bus address space is the same as the child bus address space.
+- interconnects : interconnect path specifier for MDSS according to
+  Documentation/devicetree/bindings/interconnect/interconnect.txt. Should be
+  2 paths corresponding to 2 AXI ports.
+- interconnect-names : MDSS will have 2 port names to differentiate between the
+  2 interconnect paths defined with interconnect specifier.
 
 Optional properties:
 - assigned-clocks: list of clock specifiers for clocks needing rate assignment
@@ -86,6 +91,11 @@ Example:
interrupt-controller;
#interrupt-cells = <1>;
 
+   interconnects = <_hlos MASTER_MDP0 _hlos SLAVE_EBI1>,
+   <_hlos MASTER_MDP1 _hlos SLAVE_EBI1>;
+
+   interconnect-names = "mdp0-mem", "mdp1-mem";
+
iommus = <_iommu 0>;
 
#address-cells = <2>;
-- 
2.20.1

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

[PATCH 0/5] drm/msm: mdp5+dpu interconnect support

[Rob Clark]

From: Rob Clark 

Most of this is a resend of things that have already been posted to
list.  I've rebased the DPU patches, which was somewhat conflicty due to
other changes and refactoring in the DPU code.

Probably wouldn't hurt for someone to look over my rebases of the first
two patches in particular.  But I don't think I missed anything, and
they get display back to working without underflow.

I apologize if I missed any r-b/a-b tags, patchwork didn't seem to pick
them up for some reason.  I looked thru earlier versions of the DPU
patches, but I might have missed tags.

Abhinav Kumar (1):
  drm/msm/dpu: add icc voting in dpu_mdss_init

Georgi Djakov (1):
  drm/msm/mdp5: Use the interconnect API

Jayant Shekhar (3):
  drm/msm/dpu: clean up references of DPU custom bus scaling
  drm/msm/dpu: Integrate interconnect API in MDSS
  dt-bindings: msm/disp: Introduce interconnect bindings for MDSS on
SDM845

 .../devicetree/bindings/display/msm/dpu.txt   |  10 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 174 +++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h |   4 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  |  11 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c  |  57 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h |  22 +--
 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c  |  14 ++
 7 files changed, 158 insertions(+), 134 deletions(-)

-- 
2.20.1

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

Re: [PATCH 1/2] drm/i915: Seal races between async GPU cancellation, retirement and signaling

[Chris Wilson]

Quoting Daniel Vetter (2019-05-08 13:53:30)
> On Wed, May 8, 2019 at 2:06 PM Chris Wilson  wrote:
> >
> > Currently there is an underlying assumption that i915_request_unsubmit()
> > is synchronous wrt the GPU -- that is the request is no longer in flight
> > as we remove it. In the near future that may change, and this may upset
> > our signaling as we can process an interrupt for that request while it
> > is no longer in flight.
> >
> > CPU0CPU1
> > intel_engine_breadcrumbs_irq
> > (queue request completion)
> > i915_request_cancel_signaling
> > ... ...
> > i915_request_enable_signaling
> > dma_fence_signal
> >
> > Hence in the time it took us to drop the lock to signal the request, a
> > preemption event may have occurred and re-queued the request. In the
> > process, that request would have seen I915_FENCE_FLAG_SIGNAL clear and
> > so reused the rq->signal_link that was in use on CPU0, leading to bad
> > pointer chasing in intel_engine_breadcrumbs_irq.
> >
> > A related issue was that if someone started listening for a signal on a
> > completed but no longer in-flight request, we missed the opportunity to
> > immediately signal that request.
> >
> > Furthermore, as intel_contexts may be immediately released during
> > request retirement, in order to be entirely sure that
> > intel_engine_breadcrumbs_irq may no longer dereference the intel_context
> > (ce->signals and ce->signal_link), we must wait for irq spinlock.
> >
> > In order to prevent the race, we use a bit in the fence.flags to signal
> > the transfer onto the signal list inside intel_engine_breadcrumbs_irq.
> > For simplicity, we use the DMA_FENCE_FLAG_SIGNALED_BIT as it then
> > quickly signals to any outside observer that the fence is indeed signaled.
> >
> > v2: Sketch out potential dma-fence API for manual signaling
> > v3: And the test_and_set_bit()
> >
> > Fixes: 52c0fdb25c7c ("drm/i915: Replace global breadcrumbs with per-context 
> > interrupt tracking")
> > Signed-off-by: Chris Wilson 
> > Cc: Tvrtko Ursulin 
> > Reviewed-by: Tvrtko Ursulin 
> 
> Ok chatted a bit with Chris on irc, and we already allow drivers to
> pass whatever spinlock is suitable for their request/fence
> handling/retiring, so allowing to split up this all makes sense I
> think. Has my ack on the approach.
> 
> I think it'd be good to spell out the optimization this allows
> (synchronous cleanup instead of refcounting all. the. things.) plus
> showcase the link between the fence->lock pointer and the split-up
> __dma_signal functions in the kerneldoc. Definitely for the core
> patch.
> 
> Also not sure why __notify and __timestamp has a double underscore in
> the middle. That color choice confuses me a bit :-)

I like it for subphases. The overall action here is still the dma-fence
'signal', but now we are doing the 'set timestamp', 'notify callbacks'
etc. Otherwise we gain a subject to the verb, 'signal_notify' which says
to go and signal the notify, rather than do the notifications; for me
the '__' breaks the association. Maybe dma_fence_signal_do_notifies.
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/msm/a6xx: No zap shader is not an error

[Sean Paul]

On Wed, May 08, 2019 at 06:06:52AM -0700, Rob Clark wrote:
> From: Rob Clark 
> 
> Depending on platform firmware, a zap shader may not be required to take
> the GPU out of secure mode on boot, in which case we can just write
> RBBM_SECVID_TRUST_CNTL directly.  Which we *mostly* handled, but missed
> clearing 'ret' resulting that hw_init() returned an error on these
> devices.
> 
> Fixes: abccb9fe3267 drm/msm/a6xx: Add zap shader load
> Signed-off-by: Rob Clark 

As discussed on IRC, I've stuffed this in -misc-next-fixes for the next PR I'm
sending out.

Sean

> ---
>  drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
> b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index ec24508b9d68..e74dce474250 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -527,6 +527,7 @@ static int a6xx_hw_init(struct msm_gpu *gpu)
>   dev_warn_once(gpu->dev->dev,
>   "Zap shader not enabled - using SECVID_TRUST_CNTL 
> instead\n");
>   gpu_write(gpu, REG_A6XX_RBBM_SECVID_TRUST_CNTL, 0x0);
> + ret = 0;
>   }
>  
>  out:
> -- 
> 2.20.1
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 110630] Random Horizontal green lines after screen resize

[bugzilla-daemon]

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

--- Comment #7 from Axel Davy  ---
amdgpu.dc=0 makes things worse.

http://dev.ipol.im/~adavy/green_lines2.mp4

As you can see, instead of a few green lines, there are instead a few full bad
frames.

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

[Bug 110443] vaapi/vpp: wrong output for non 64-bytes align width (ex: 1200)

[bugzilla-daemon]

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

--- Comment #14 from Julien Isorce  ---
https://gitlab.freedesktop.org/mesa/mesa/merge_requests/796
https://gitlab.freedesktop.org/mesa/mesa/merge_requests/797
https://gitlab.freedesktop.org/mesa/mesa/merge_requests/842

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

Re: [PATCH v2 00/17] kunit: introduce KUnit, the Linux kernel unit testing framework

[Brendan Higgins]

> On Tue, May 07, 2019 at 10:01:19AM +0200, Greg KH wrote:
> > > My understanding is that the intent of KUnit is to avoid booting a kernel 
> > > on
> > > real hardware or in a virtual machine.  That seems to be a matter of 
> > > semantics
> > > to me because isn't invoking a UML Linux just running the Linux kernel in
> > > a different form of virtualization?
> > >
> > > So I do not understand why KUnit is an improvement over kselftest.
> > >
> > > It seems to me that KUnit is just another piece of infrastructure that I
> > > am going to have to be familiar with as a kernel developer.  More 
> > > overhead,
> > > more information to stuff into my tiny little brain.
> > >
> > > I would guess that some developers will focus on just one of the two test
> > > environments (and some will focus on both), splitting the development
> > > resources instead of pooling them on a common infrastructure.
> > >
> > > What am I missing?
> >
> > kselftest provides no in-kernel framework for testing kernel code
> > specifically.  That should be what kunit provides, an "easy" way to
> > write in-kernel tests for things.
> >
> > Brendan, did I get it right?
>
> Yes, that's basically right.  You don't *have* to use KUnit.  It's
> supposed to be a simple way to run a large number of small tests that
> for specific small components in a system.
>
> For example, I currently use xfstests using KVM and GCE to test all of
> ext4.  These tests require using multiple 5 GB and 20GB virtual disks,
> and it works by mounting ext4 file systems and exercising ext4 through
> the system call interfaces, using userspace tools such as fsstress,
> fsx, fio, etc.  It requires time overhead to start the VM, create and
> allocate virtual disks, etc.  For example, to run a single 3 seconds
> xfstest (generic/001), it requires full 10 seconds to run it via
> kvm-xfstests.
>
> KUnit is something else; it's specifically intended to allow you to
> create lightweight tests quickly and easily, and by reducing the
> effort needed to write and run unit tests, hopefully we'll have a lot
> more of them and thus improve kernel quality.
>
> As an example, I have a volunteer working on developing KUinit tests
> for ext4.  We're going to start by testing the ext4 extent status
> tree.  The source code is at fs/ext4/extent_status.c; it's
> approximately 1800 LOC.  The Kunit tests for the extent status tree
> will exercise all of the corner cases for the various extent status
> tree functions --- e.g., ext4_es_insert_delayed_block(),
> ext4_es_remove_extent(), ext4_es_cache_extent(), etc.  And it will do
> this in isolation without our needing to create a test file system or
> using a test block device.
>
> Next we'll test the ext4 block allocator, again in isolation.  To test
> the block allocator we will have to write "mock functions" which
> simulate reading allocation bitmaps from disk.  Again, this will allow
> the test writer to explicitly construct corner cases and validate that
> the block allocator works as expected without having to reverese
> engineer file system data structures which will force a particular
> code path to be executed.
>
> So this is why it's largely irrelevant to me that KUinit uses UML.  In
> fact, it's a feature.  We're not testing device drivers, or the
> scheduler, or anything else architecture-specific.  UML is not about
> virtualization.  What it's about in this context is allowing us to
> start running test code as quickly as possible.  Booting KVM takes
> about 3-4 seconds, and this includes initializing virtio_scsi and
> other device drivers.  If by using UML we can hold the amount of
> unnecessary kernel subsystem initialization down to the absolute
> minimum, and if it means that we can communicating to the test
> framework via a userspace "printf" from UML/KUnit code, as opposed to
> via a virtual serial port to KVM's virtual console, it all makes for
> lighter weight testing.
>
> Why did I go looking for a volunteer to write KUnit tests for ext4?
> Well, I have a plan to make some changes in restructing how ext4's
> write path works, in order to support things like copy-on-write, a
> more efficient delayed allocation system, etc.  This will require
> making changes to the extent status tree, and by having unit tests for
> the extent status tree, we'll be able to detect any bugs that we might
> accidentally introduce in the es tree far more quickly than if we
> didn't have those tests available.  Google has long found that having
> these sorts of unit tests is a real win for developer velocity for any
> non-trivial code module (or C++ class), even when you take into
> account the time it takes to create the unit tests.
>
> - Ted
>
> P.S.  Many thanks to Brendan for finding such a volunteer for me; the
> person in question is a SRE from Switzerland who is interested in
> getting involved with kernel testing, and this is going to be their
> 20% project.  :-)

Thanks Ted, I really 

[v9 05/13] drm/i915: Attach HDR metadata property to connector

[Uma Shankar]

Attach HDR metadata property to connector object.

v2: Rebase

v3: Updated the property name as per updated name
while creating hdr metadata property

Signed-off-by: Uma Shankar 
Reviewed-by: Shashank Sharma 
---
 drivers/gpu/drm/i915/intel_hdmi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index 2a4086c..92597d8 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2724,6 +2724,8 @@ static void intel_hdmi_destroy(struct drm_connector 
*connector)
 
drm_connector_attach_content_type_property(connector);
connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
+   drm_object_attach_property(>base,
+  
connector->dev->mode_config.hdr_output_metadata_property, 0);
 
if (!HAS_GMCH(dev_priv))
drm_connector_attach_max_bpc_property(connector, 8, 12);
-- 
1.9.1

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

[v9 07/13] drm: Add HLG EOTF

[Uma Shankar]

From: Ville Syrjälä 

ADD HLG EOTF to the list of EOTF transfer functions supported.
Hybrid Log-Gamma (HLG) is a high dynamic range (HDR) standard.
HLG defines a nonlinear transfer function in which the lower
half of the signal values use a gamma curve and the upper half
of the signal values use a logarithmic curve.

v2: Rebase

v3: Fixed a warning message

v4: Addressed Shashank's review comments

v5: Addressed Jonas Karlman's review comment and dropped the i915
tag from header.

Signed-off-by: Ville Syrjälä 
Signed-off-by: Uma Shankar 
Reviewed-by: Shashank Sharma 
---
 drivers/gpu/drm/drm_edid.c | 3 ++-
 include/linux/hdmi.h   | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 5f48965..73b33ad 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3857,7 +3857,8 @@ static uint8_t eotf_supported(const u8 *edid_ext)
return edid_ext[2] &
(BIT(HDMI_EOTF_TRADITIONAL_GAMMA_SDR) |
 BIT(HDMI_EOTF_TRADITIONAL_GAMMA_HDR) |
-BIT(HDMI_EOTF_SMPTE_ST2084));
+BIT(HDMI_EOTF_SMPTE_ST2084) |
+BIT(HDMI_EOTF_BT_2100_HLG));
 }
 
 static uint8_t hdr_metadata_type(const u8 *edid_ext)
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index 7edafcf..3d7f10f 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -161,6 +161,7 @@ enum hdmi_eotf {
HDMI_EOTF_TRADITIONAL_GAMMA_SDR,
HDMI_EOTF_TRADITIONAL_GAMMA_HDR,
HDMI_EOTF_SMPTE_ST2084,
+   HDMI_EOTF_BT_2100_HLG,
 };
 
 struct hdmi_avi_infoframe {
-- 
1.9.1

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

[v9 00/13] Add HDR Metadata Parsing and handling in DRM layer

[Uma Shankar]

This patch series enables HDR support in drm. It basically defines
HDR metadata structures, property to pass content (after blending)
metadata from user space compositors to driver.

Dynamic Range and Mastering infoframe creation and sending.

ToDo:
1. We need to get the color framework in place for all planes
   which support HDR content in hardware. This is already in progres
   and patches are out for review in mailing list.
2. UserSpace/Compositors: Blending policies and metadata blob
   creation and passing to driver. Work is already in progress
   by Intel's middleware teams on wayland and the patches for
   the same are in review.

A POC has already been developed by Ville based on wayland. Please refer
below link to see the component interactions and usage:
https://lists.freedesktop.org/archives/wayland-devel/2017-December/036403.html

v2: Updated Ville's POC changes to the patch series.Incorporated cleanups
and fixes from Ville. Rebase on latest drm-tip.

v3: Fixed a warning causing builds to break on CI. No major change.

v4: Addressed Shashank's review comments.

v5: Rebase on top of Ville's infoframe refactoring changes. Fixed non modeset
case for HDR metadata update. Dropped a redundant patch.

v6: Addressed Shashank's review comments and added RB's received.

v7: Squashed 2 patches, dropped 1 change and addressed Brian Starkey's and
Shashank's review comments.

v8: Addressed Jonas Karlman review comments. Added Shashank's RB to the series,
fixed a WARN_ON on BYT/CHT.

v9: Addressed Ville and Jonas Karlman's review comments. Added the infoframe
state readout and metadata reference count.

Note: This is already tested with Kodi and a confirmation from team kodi has 
been
received. Branch details for the same as below:
https://github.com/xbmc/xbmc/tree/feature_drmprime-vaapi

Jonas Karlman (1):
  drm: Add reference counting on HDR metadata blob

Uma Shankar (10):
  drm: Add HDR source metadata property
  drm: Parse HDR metadata info from EDID
  drm: Enable HDR infoframe support
  drm/i915: Attach HDR metadata property to connector
  drm/i915: Write HDR infoframe and send to panel
  drm/i915:Enabled Modeset when HDR Infoframe changes
  drm/i915: Set Infoframe for non modeset case for HDR
  drm/i915: Added DRM Infoframe handling for BYT/CHT
  video/hdmi: Add Unpack function for DRM infoframe
  drm/i915: Add state readout for DRM infoframe

Ville Syrjälä (2):
  drm: Add HLG EOTF
  drm/i915: Enable infoframes on GLK+ for HDR

 drivers/gpu/drm/drm_atomic.c  |   2 +
 drivers/gpu/drm/drm_atomic_state_helper.c |   6 +
 drivers/gpu/drm/drm_atomic_uapi.c |  13 ++
 drivers/gpu/drm/drm_connector.c   |   6 +
 drivers/gpu/drm/drm_edid.c| 101 +
 drivers/gpu/drm/i915/i915_reg.h   |   4 +
 drivers/gpu/drm/i915/intel_atomic.c   |  14 +-
 drivers/gpu/drm/i915/intel_ddi.c  |  17 +++
 drivers/gpu/drm/i915/intel_display.c  |   1 +
 drivers/gpu/drm/i915/intel_drv.h  |   1 +
 drivers/gpu/drm/i915/intel_hdmi.c | 100 -
 drivers/video/hdmi.c  | 240 ++
 include/drm/drm_connector.h   |  11 ++
 include/drm/drm_edid.h|   5 +
 include/drm/drm_mode_config.h |   6 +
 include/linux/hdmi.h  |  55 +++
 include/uapi/drm/drm_mode.h   |  23 +++
 17 files changed, 598 insertions(+), 7 deletions(-)

-- 
1.9.1

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

[v9 11/13] drm/i915: Added DRM Infoframe handling for BYT/CHT

[Uma Shankar]

BYT/CHT doesn't support DRM Infoframe. This caused
a WARN_ON due to a missing CASE while executing
intel_hdmi_infoframes_enabled function. This patch
fixes the same.

Signed-off-by: Uma Shankar 
---
 drivers/gpu/drm/i915/intel_hdmi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index e559a940..224d33e 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -129,6 +129,8 @@ static u32 g4x_infoframe_enable(unsigned int type)
return VIDEO_DIP_ENABLE_SPD;
case HDMI_INFOFRAME_TYPE_VENDOR:
return VIDEO_DIP_ENABLE_VENDOR;
+   case HDMI_INFOFRAME_TYPE_DRM:
+   return 0;
default:
MISSING_CASE(type);
return 0;
-- 
1.9.1

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

[v9 13/13] drm/i915: Add state readout for DRM infoframe

[Uma Shankar]

Added state readout for DRM infoframe and enabled
state validation for DRM infoframe.

Signed-off-by: Uma Shankar 
---
 drivers/gpu/drm/i915/intel_ddi.c | 4 
 drivers/gpu/drm/i915/intel_display.c | 1 +
 drivers/gpu/drm/i915/intel_hdmi.c| 4 
 3 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index d37526b..3a38f32 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3849,6 +3849,10 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
intel_read_infoframe(encoder, pipe_config,
 HDMI_INFOFRAME_TYPE_VENDOR,
 _config->infoframes.hdmi);
+   if ((INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)))
+   intel_read_infoframe(encoder, pipe_config,
+HDMI_INFOFRAME_TYPE_DRM,
+_config->infoframes.drm);
 }
 
 static enum intel_output_type
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index a351c8e..74b5189 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12231,6 +12231,7 @@ static bool fastboot_enabled(struct drm_i915_private 
*dev_priv)
PIPE_CONF_CHECK_INFOFRAME(avi);
PIPE_CONF_CHECK_INFOFRAME(spd);
PIPE_CONF_CHECK_INFOFRAME(hdmi);
+   PIPE_CONF_CHECK_INFOFRAME(drm);
 
 #undef PIPE_CONF_CHECK_X
 #undef PIPE_CONF_CHECK_I
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index 224d33e..3886065 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1867,6 +1867,10 @@ static void intel_hdmi_get_config(struct intel_encoder 
*encoder,
intel_read_infoframe(encoder, pipe_config,
 HDMI_INFOFRAME_TYPE_VENDOR,
 _config->infoframes.hdmi);
+   if ((INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)))
+   intel_read_infoframe(encoder, pipe_config,
+HDMI_INFOFRAME_TYPE_DRM,
+_config->infoframes.drm);
 }
 
 static void intel_enable_hdmi_audio(struct intel_encoder *encoder,
-- 
1.9.1

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

Re: [PATCH v4 01/11] drm: Add atomic variants of enable/disable to encoder helper funcs

[Sean Paul]

On Wed, May 08, 2019 at 06:31:24PM +0200, Daniel Vetter wrote:
> On Wed, May 08, 2019 at 12:09:06PM -0400, Sean Paul wrote:
> > From: Sean Paul 
> > 
> > This patch adds atomic_enable and atomic_disable callbacks to the
> > encoder helpers. This will allow encoders to make informed decisions in
> > their start-up/shutdown based on the committed state.
> > 
> > Aside from the new hooks, this patch also introduces the new signature
> > for .atomic_* functions going forward. Instead of passing object state
> > (well, encoders don't have atomic state, but let's ignore that), we pass
> > the entire atomic state so the driver can inspect more than what's
> > happening locally.
> > 
> > This is particularly important for the upcoming self refresh helpers.
> > 
> > Changes in v3:
> > - Added patch to the set
> > Changes in v4:
> > - Move atomic_disable above prepare (Daniel)
> > - Add breadcrumb to .enable() docbook (Daniel)
> 
> Why no r-b: me or did you not apply all my suggestions? Too lazy to read
> it all again :-)

Sorry, I was being a bit conservative slapping your R-b on the patches I
changed. I've incorporated all of your suggestions AFAICT, so I've applied R-b
on patches 01/03/04/05 (conveniently the same patch numbers between v3 & v4).
Unless there's other review feedback I'll probably not send those out. I will
send out an update to Laurent's patch with your suggestions and R-b after a bit
of soak time on the list.

Sean


> -Daniel
> 
> > 
> > Link to v3: 
> > https://patchwork.freedesktop.org/patch/msgid/20190502194956.218441-2-s...@poorly.run
> > 
> > Cc: Daniel Vetter 
> > Cc: Ville Syrjälä 
> > Signed-off-by: Sean Paul 
> > ---
> >  drivers/gpu/drm/drm_atomic_helper.c  |  8 +++-
> >  include/drm/drm_modeset_helper_vtables.h | 48 
> >  2 files changed, 54 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> > b/drivers/gpu/drm/drm_atomic_helper.c
> > index 553415fe8ede..ccf01831f265 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -999,7 +999,9 @@ disable_outputs(struct drm_device *dev, struct 
> > drm_atomic_state *old_state)
> >  
> > /* Right function depends upon target state. */
> > if (funcs) {
> > -   if (new_conn_state->crtc && funcs->prepare)
> > +   if (funcs->atomic_disable)
> > +   funcs->atomic_disable(encoder, old_state);
> > +   else if (new_conn_state->crtc && funcs->prepare)
> > funcs->prepare(encoder);
> > else if (funcs->disable)
> > funcs->disable(encoder);
> > @@ -1309,7 +1311,9 @@ void drm_atomic_helper_commit_modeset_enables(struct 
> > drm_device *dev,
> > drm_bridge_pre_enable(encoder->bridge);
> >  
> > if (funcs) {
> > -   if (funcs->enable)
> > +   if (funcs->atomic_enable)
> > +   funcs->atomic_enable(encoder, old_state);
> > +   else if (funcs->enable)
> > funcs->enable(encoder);
> > else if (funcs->commit)
> > funcs->commit(encoder);
> > diff --git a/include/drm/drm_modeset_helper_vtables.h 
> > b/include/drm/drm_modeset_helper_vtables.h
> > index 8f3602811eb5..aa509c107083 100644
> > --- a/include/drm/drm_modeset_helper_vtables.h
> > +++ b/include/drm/drm_modeset_helper_vtables.h
> > @@ -675,6 +675,51 @@ struct drm_encoder_helper_funcs {
> > enum drm_connector_status (*detect)(struct drm_encoder *encoder,
> > struct drm_connector *connector);
> >  
> > +   /**
> > +* @atomic_disable:
> > +*
> > +* This callback should be used to disable the encoder. With the atomic
> > +* drivers it is called before this encoder's CRTC has been shut off
> > +* using their own _crtc_helper_funcs.atomic_disable hook. If that
> > +* sequence is too simple drivers can just add their own driver private
> > +* encoder hooks and call them from CRTC's callback by looping over all
> > +* encoders connected to it using for_each_encoder_on_crtc().
> > +*
> > +* This callback is a variant of @disable that provides the atomic state
> > +* to the driver. It takes priority over @disable during atomic commits.
> > +*
> > +* This hook is used only by atomic helpers. Atomic drivers don't need
> > +* to implement it if there's no need to disable anything at the encoder
> > +* level. To ensure that runtime PM handling (using either DPMS or the
> > +* new "ACTIVE" property) works @atomic_disable must be the inverse of
> > +* @atomic_enable.
> > +*/
> > +   void (*atomic_disable)(struct drm_encoder *encoder,
> > +  struct drm_atomic_state *state);
> > +
> > +   /**
> > +* @atomic_enable:
> > +*
> > +  

[v9 02/13] drm: Add reference counting on HDR metadata blob

[Uma Shankar]

From: Jonas Karlman 

This adds reference count for HDR metadata blob,
handled as part of duplicate and destroy connector
state functions.

Signed-off-by: Jonas Karlman 
Signed-off-by: Uma Shankar 
---
 drivers/gpu/drm/drm_atomic_state_helper.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c 
b/drivers/gpu/drm/drm_atomic_state_helper.c
index ac929f6..8f49952 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -391,6 +391,10 @@ void drm_atomic_helper_connector_reset(struct 
drm_connector *connector)
drm_connector_get(connector);
state->commit = NULL;
 
+   if (state->hdr_output_metadata)
+   drm_property_blob_get(state->hdr_output_metadata);
+   state->hdr_metadata_changed = false;
+
/* Don't copy over a writeback job, they are used only once */
state->writeback_job = NULL;
 }
@@ -438,6 +442,8 @@ struct drm_connector_state *
 
if (state->writeback_job)
drm_writeback_cleanup_job(state->writeback_job);
+
+   drm_property_blob_put(state->hdr_output_metadata);
 }
 EXPORT_SYMBOL(__drm_atomic_helper_connector_destroy_state);
 
-- 
1.9.1

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

[v9 09/13] drm/i915:Enabled Modeset when HDR Infoframe changes

[Uma Shankar]

This patch enables modeset whenever HDR metadata
needs to be updated to sink.

v2: Addressed Shashank's review comments.

v3: Added Shashank's RB.

v4: Addressed Ville's review comments.

Signed-off-by: Ville Syrjälä 
Signed-off-by: Uma Shankar 
Reviewed-by: Shashank Sharma 
---
 drivers/gpu/drm/i915/intel_atomic.c | 14 +-
 drivers/gpu/drm/i915/intel_hdmi.c   | 17 +
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic.c 
b/drivers/gpu/drm/i915/intel_atomic.c
index 58b8049..6b985e8 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -105,6 +105,16 @@ int intel_digital_connector_atomic_set_property(struct 
drm_connector *connector,
return -EINVAL;
 }
 
+static bool blob_equal(const struct drm_property_blob *a,
+  const struct drm_property_blob *b)
+{
+   if (a && b)
+   return a->length == b->length &&
+   !memcmp(a->data, b->data, a->length);
+
+   return !a == !b;
+}
+
 int intel_digital_connector_atomic_check(struct drm_connector *conn,
 struct drm_connector_state *new_state)
 {
@@ -132,7 +142,9 @@ int intel_digital_connector_atomic_check(struct 
drm_connector *conn,
new_conn_state->base.colorspace != old_conn_state->base.colorspace 
||
new_conn_state->base.picture_aspect_ratio != 
old_conn_state->base.picture_aspect_ratio ||
new_conn_state->base.content_type != 
old_conn_state->base.content_type ||
-   new_conn_state->base.scaling_mode != 
old_conn_state->base.scaling_mode)
+   new_conn_state->base.scaling_mode != 
old_conn_state->base.scaling_mode ||
+   !blob_equal(new_conn_state->base.hdr_output_metadata,
+   old_conn_state->base.hdr_output_metadata))
crtc_state->mode_changed = true;
 
return 0;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index 92bc347..db9c82b 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -806,6 +806,11 @@ void intel_read_infoframe(struct intel_encoder *encoder,
return true;
 }
 
+static inline bool is_eotf_supported(u8 output_eotf, u8 sink_eotf)
+{
+   return sink_eotf & BIT(output_eotf);
+}
+
 static bool
 intel_hdmi_compute_drm_infoframe(struct intel_encoder *encoder,
 struct intel_crtc_state *crtc_state,
@@ -813,11 +818,23 @@ void intel_read_infoframe(struct intel_encoder *encoder,
 {
struct hdmi_drm_infoframe *frame = _state->infoframes.drm.drm;
struct hdr_output_metadata *hdr_metadata;
+   struct drm_connector *connector = conn_state->connector;
int ret;
 
+   if (!conn_state->hdr_output_metadata ||
+   conn_state->hdr_output_metadata->length == 0)
+   return true;
+
hdr_metadata = (struct hdr_output_metadata *)
conn_state->hdr_output_metadata->data;
 
+   /* Sink EOTF is Bit map while infoframe is absolute values */
+   if (!is_eotf_supported(hdr_metadata->hdmi_metadata_type1.eotf,
+   connector->hdr_sink_metadata.hdmi_type1.eotf)) {
+   DRM_ERROR("EOTF Not Supported\n");
+   return true;
+   }
+
ret = drm_hdmi_infoframe_set_hdr_metadata(frame, hdr_metadata);
if (ret < 0) {
DRM_ERROR("couldn't set HDR metadata in infoframe\n");
-- 
1.9.1

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

[v9 08/13] drm/i915: Enable infoframes on GLK+ for HDR

[Uma Shankar]

From: Ville Syrjälä 

This patch enables infoframes on GLK+ to be
used to send HDR metadata to HDMI sink.

v2: Addressed Shashank's review comment.

v3: Addressed Shashank's review comment.

v4: Added Shashank's RB.

Signed-off-by: Ville Syrjälä 
Signed-off-by: Uma Shankar 
Reviewed-by: Shashank Sharma 
---
 drivers/gpu/drm/i915/i915_reg.h   |  4 
 drivers/gpu/drm/i915/intel_hdmi.c | 22 +-
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index e97c47f..d3f5510 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4694,6 +4694,7 @@ enum {
 #define   VIDEO_DIP_FREQ_MASK  (3 << 16)
 /* HSW and later: */
 #define   DRM_DIP_ENABLE   (1 << 28)
+#define   VIDEO_DIP_ENABLE_DRM_GLK (1 << 28)
 #define   PSR_VSC_BIT_7_SET(1 << 27)
 #define   VSC_SELECT_MASK  (0x3 << 25)
 #define   VSC_SELECT_SHIFT 25
@@ -8146,6 +8147,7 @@ enum {
 #define _HSW_VIDEO_DIP_SPD_DATA_A  0x602A0
 #define _HSW_VIDEO_DIP_GMP_DATA_A  0x602E0
 #define _HSW_VIDEO_DIP_VSC_DATA_A  0x60320
+#define _GLK_VIDEO_DIP_DRM_DATA_A  0x60440
 #define _HSW_VIDEO_DIP_AVI_ECC_A   0x60240
 #define _HSW_VIDEO_DIP_VS_ECC_A0x60280
 #define _HSW_VIDEO_DIP_SPD_ECC_A   0x602C0
@@ -8159,6 +8161,7 @@ enum {
 #define _HSW_VIDEO_DIP_SPD_DATA_B  0x612A0
 #define _HSW_VIDEO_DIP_GMP_DATA_B  0x612E0
 #define _HSW_VIDEO_DIP_VSC_DATA_B  0x61320
+#define _GLK_VIDEO_DIP_DRM_DATA_B  0x61440
 #define _HSW_VIDEO_DIP_BVI_ECC_B   0x61240
 #define _HSW_VIDEO_DIP_VS_ECC_B0x61280
 #define _HSW_VIDEO_DIP_SPD_ECC_B   0x612C0
@@ -8184,6 +8187,7 @@ enum {
 #define HSW_TVIDEO_DIP_SPD_DATA(trans, i)  _MMIO_TRANS2(trans, 
_HSW_VIDEO_DIP_SPD_DATA_A + (i) * 4)
 #define HSW_TVIDEO_DIP_GMP_DATA(trans, i)  _MMIO_TRANS2(trans, 
_HSW_VIDEO_DIP_GMP_DATA_A + (i) * 4)
 #define HSW_TVIDEO_DIP_VSC_DATA(trans, i)  _MMIO_TRANS2(trans, 
_HSW_VIDEO_DIP_VSC_DATA_A + (i) * 4)
+#define GLK_TVIDEO_DIP_DRM_DATA(trans, i)  _MMIO_TRANS2(trans, 
_GLK_VIDEO_DIP_DRM_DATA_A + (i) * 4)
 #define ICL_VIDEO_DIP_PPS_DATA(trans, i)   _MMIO_TRANS2(trans, 
_ICL_VIDEO_DIP_PPS_DATA_A + (i) * 4)
 #define ICL_VIDEO_DIP_PPS_ECC(trans, i)_MMIO_TRANS2(trans, 
_ICL_VIDEO_DIP_PPS_ECC_A + (i) * 4)
 
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index 980900b..92bc347 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -152,6 +152,8 @@ static u32 hsw_infoframe_enable(unsigned int type)
return VIDEO_DIP_ENABLE_SPD_HSW;
case HDMI_INFOFRAME_TYPE_VENDOR:
return VIDEO_DIP_ENABLE_VS_HSW;
+   case HDMI_INFOFRAME_TYPE_DRM:
+   return VIDEO_DIP_ENABLE_DRM_GLK;
default:
MISSING_CASE(type);
return 0;
@@ -177,6 +179,8 @@ static u32 hsw_infoframe_enable(unsigned int type)
return HSW_TVIDEO_DIP_SPD_DATA(cpu_transcoder, i);
case HDMI_INFOFRAME_TYPE_VENDOR:
return HSW_TVIDEO_DIP_VS_DATA(cpu_transcoder, i);
+   case HDMI_INFOFRAME_TYPE_DRM:
+   return GLK_TVIDEO_DIP_DRM_DATA(cpu_transcoder, i);
default:
MISSING_CASE(type);
return INVALID_MMIO_REG;
@@ -560,10 +564,16 @@ static u32 hsw_infoframes_enabled(struct intel_encoder 
*encoder,
 {
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
u32 val = I915_READ(HSW_TVIDEO_DIP_CTL(pipe_config->cpu_transcoder));
+   u32 mask;
 
-   return val & (VIDEO_DIP_ENABLE_VSC_HSW | VIDEO_DIP_ENABLE_AVI_HSW |
- VIDEO_DIP_ENABLE_GCP_HSW | VIDEO_DIP_ENABLE_VS_HSW |
- VIDEO_DIP_ENABLE_GMP_HSW | VIDEO_DIP_ENABLE_SPD_HSW);
+   mask = (VIDEO_DIP_ENABLE_VSC_HSW | VIDEO_DIP_ENABLE_AVI_HSW |
+   VIDEO_DIP_ENABLE_GCP_HSW | VIDEO_DIP_ENABLE_VS_HSW |
+   VIDEO_DIP_ENABLE_GMP_HSW | VIDEO_DIP_ENABLE_SPD_HSW);
+
+   if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
+   mask |= VIDEO_DIP_ENABLE_DRM_GLK;
+
+   return val & mask;
 }
 
 static const u8 infoframe_type_to_idx[] = {
@@ -1182,7 +1192,8 @@ static void hsw_set_infoframes(struct intel_encoder 
*encoder,
 
val &= ~(VIDEO_DIP_ENABLE_VSC_HSW | VIDEO_DIP_ENABLE_AVI_HSW |
 VIDEO_DIP_ENABLE_GCP_HSW | VIDEO_DIP_ENABLE_VS_HSW |
-VIDEO_DIP_ENABLE_GMP_HSW | VIDEO_DIP_ENABLE_SPD_HSW);
+VIDEO_DIP_ENABLE_GMP_HSW | VIDEO_DIP_ENABLE_SPD_HSW |
+VIDEO_DIP_ENABLE_DRM_GLK);
 
if (!enable) {
I915_WRITE(reg, val);
@@ -1211,7 +1222,8 @@ static void hsw_set_infoframes(struct intel_encoder 
*encoder,
 * ToDo: Gen9 also can support HDR with LSPCON.
 * Support for the same to be enabled later.
 

[v9 10/13] drm/i915: Set Infoframe for non modeset case for HDR

[Uma Shankar]

HDR metadata requires a infoframe to be set. Due to fastset,
full modeset is not performed hence adding it to update_pipe
to handle that.

Signed-off-by: Uma Shankar 
Reviewed-by: Shashank Sharma 
---
 drivers/gpu/drm/i915/intel_ddi.c  | 13 +
 drivers/gpu/drm/i915/intel_hdmi.c |  7 +--
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index cd5277d..d37526b 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3559,6 +3559,10 @@ static void intel_ddi_update_pipe(struct intel_encoder 
*encoder,
  const struct intel_crtc_state *crtc_state,
  const struct drm_connector_state *conn_state)
 {
+   struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+   struct intel_digital_port *intel_dig_port =
+   enc_to_dig_port(>base);
+
if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state);
 
@@ -3568,6 +3572,15 @@ static void intel_ddi_update_pipe(struct intel_encoder 
*encoder,
else if (conn_state->content_protection ==
 DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
intel_hdcp_disable(to_intel_connector(conn_state->connector));
+
+   /* Set the infoframe for NON modeset cases as well */
+   if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
+   if ((INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) &&
+   conn_state->hdr_metadata_changed)
+   intel_dig_port->set_infoframes(encoder,
+  
crtc_state->has_infoframe,
+  crtc_state, conn_state);
+   }
 }
 
 static void intel_ddi_set_fia_lane_count(struct intel_encoder *encoder,
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index db9c82b..e559a940 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1204,8 +1204,11 @@ static void hsw_set_infoframes(struct intel_encoder 
*encoder,
i915_reg_t reg = HSW_TVIDEO_DIP_CTL(crtc_state->cpu_transcoder);
u32 val = I915_READ(reg);
 
-   assert_hdmi_transcoder_func_disabled(dev_priv,
-crtc_state->cpu_transcoder);
+   /* DRM Infoframe can be send with transcoder enabled */
+   if (!((INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) &&
+ conn_state->hdr_metadata_changed))
+   assert_hdmi_transcoder_func_disabled(dev_priv,
+
crtc_state->cpu_transcoder);
 
val &= ~(VIDEO_DIP_ENABLE_VSC_HSW | VIDEO_DIP_ENABLE_AVI_HSW |
 VIDEO_DIP_ENABLE_GCP_HSW | VIDEO_DIP_ENABLE_VS_HSW |
-- 
1.9.1

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

[v9 12/13] video/hdmi: Add Unpack function for DRM infoframe

[Uma Shankar]

Added unpack function for DRM infoframe for dynamic
range and mastering infoframe readout.

Suggested-by: Ville Syrjälä 
Signed-off-by: Uma Shankar 
---
 drivers/video/hdmi.c | 54 
 include/linux/hdmi.h |  1 +
 2 files changed, 55 insertions(+)

diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index 717ed7fb..110d405 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -1801,6 +1801,57 @@ static int hdmi_audio_infoframe_unpack(struct 
hdmi_audio_infoframe *frame,
 }
 
 /**
+ * hdmi_drm_infoframe_unpack() - unpack binary buffer to a HDMI DRM infoframe
+ * @frame: HDMI DRM infoframe
+ * @buffer: source buffer
+ * @size: size of buffer
+ *
+ * Unpacks the information contained in binary @buffer into a structured
+ * @frame of the HDMI Dynamic Range and Mastering (DRM) information frame.
+ * Also verifies the checksum as required by section 5.3.5 of the HDMI 1.4
+ * specification.
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+static int hdmi_drm_infoframe_unpack(struct hdmi_drm_infoframe *frame,
+const void *buffer, size_t size)
+{
+   const u8 *ptr = buffer;
+   int ret;
+
+   if (size < HDMI_INFOFRAME_SIZE(DRM))
+   return -EINVAL;
+
+   if (ptr[0] != HDMI_INFOFRAME_TYPE_DRM ||
+   ptr[1] != 1 ||
+   ptr[2] != HDMI_DRM_INFOFRAME_SIZE)
+   return -EINVAL;
+
+   if (hdmi_infoframe_checksum(buffer, HDMI_INFOFRAME_SIZE(DRM)) != 0)
+   return -EINVAL;
+
+   ret = hdmi_drm_infoframe_init(frame);
+   if (ret)
+   return ret;
+
+   frame->length = ptr[2];
+   ptr += HDMI_INFOFRAME_HEADER_SIZE;
+
+   frame->eotf = ptr[0] & 0x7;
+   frame->metadata_type = ptr[1] & 0x7;
+
+   memcpy(>display_primaries, [2], 12);
+   memcpy(>white_point, [14], 4);
+
+   frame->max_display_mastering_luminance = (ptr[19] << 8) | ptr[18];
+   frame->min_display_mastering_luminance = (ptr[21] << 8) | ptr[20];
+   frame->max_cll = (ptr[23] << 8) | ptr[22];
+   frame->max_fall = (ptr[25] << 8) | ptr[24];
+
+   return 0;
+}
+
+/**
  * hdmi_infoframe_unpack() - unpack binary buffer to a HDMI infoframe
  * @frame: HDMI infoframe
  * @buffer: source buffer
@@ -1826,6 +1877,9 @@ int hdmi_infoframe_unpack(union hdmi_infoframe *frame,
case HDMI_INFOFRAME_TYPE_AVI:
ret = hdmi_avi_infoframe_unpack(>avi, buffer, size);
break;
+   case HDMI_INFOFRAME_TYPE_DRM:
+   ret = hdmi_drm_infoframe_unpack(>drm, buffer, size);
+   break;
case HDMI_INFOFRAME_TYPE_SPD:
ret = hdmi_spd_infoframe_unpack(>spd, buffer, size);
break;
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index 3d7f10f..ee55ba5 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -56,6 +56,7 @@ enum hdmi_infoframe_type {
 #define HDMI_AVI_INFOFRAME_SIZE13
 #define HDMI_SPD_INFOFRAME_SIZE25
 #define HDMI_AUDIO_INFOFRAME_SIZE  10
+#define HDMI_DRM_INFOFRAME_SIZE26
 
 #define HDMI_INFOFRAME_SIZE(type)  \
(HDMI_INFOFRAME_HEADER_SIZE + HDMI_ ## type ## _INFOFRAME_SIZE)
-- 
1.9.1

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

[v9 06/13] drm/i915: Write HDR infoframe and send to panel

[Uma Shankar]

Enable writing of HDR metadata infoframe to panel.
The data will be provid by usersapace compositors, based
on blending policies and passsed to driver through a blob
property.

v2: Rebase

v3: Fixed a warning message

v4: Addressed Shashank's review comments

v5: Rebase. Added infoframe calculation in compute config.

v6: Addressed Shashank's review comment. Added HDR metadata
support from GEN10 onwards as per Shashank's recommendation.

v7: Addressed Shashank's review comments

v8: Added Shashank's RB.

Signed-off-by: Uma Shankar 
Reviewed-by: Shashank Sharma 
---
 drivers/gpu/drm/i915/intel_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_hdmi.c | 48 +++
 2 files changed, 49 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 247893e..bc32b2c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -910,6 +910,7 @@ struct intel_crtc_state {
union hdmi_infoframe avi;
union hdmi_infoframe spd;
union hdmi_infoframe hdmi;
+   union hdmi_infoframe drm;
} infoframes;
 
/* HDMI scrambling status */
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index 92597d8..980900b 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -573,6 +573,7 @@ static u32 hsw_infoframes_enabled(struct intel_encoder 
*encoder,
HDMI_INFOFRAME_TYPE_AVI,
HDMI_INFOFRAME_TYPE_SPD,
HDMI_INFOFRAME_TYPE_VENDOR,
+   HDMI_INFOFRAME_TYPE_DRM,
 };
 
 u32 intel_hdmi_infoframe_enable(unsigned int type)
@@ -795,6 +796,30 @@ void intel_read_infoframe(struct intel_encoder *encoder,
return true;
 }
 
+static bool
+intel_hdmi_compute_drm_infoframe(struct intel_encoder *encoder,
+struct intel_crtc_state *crtc_state,
+struct drm_connector_state *conn_state)
+{
+   struct hdmi_drm_infoframe *frame = _state->infoframes.drm.drm;
+   struct hdr_output_metadata *hdr_metadata;
+   int ret;
+
+   hdr_metadata = (struct hdr_output_metadata *)
+   conn_state->hdr_output_metadata->data;
+
+   ret = drm_hdmi_infoframe_set_hdr_metadata(frame, hdr_metadata);
+   if (ret < 0) {
+   DRM_ERROR("couldn't set HDR metadata in infoframe\n");
+   return false;
+   }
+
+   crtc_state->infoframes.enable |=
+   intel_hdmi_infoframe_enable(HDMI_INFOFRAME_TYPE_DRM);
+
+   return true;
+}
+
 static void g4x_set_infoframes(struct intel_encoder *encoder,
   bool enable,
   const struct intel_crtc_state *crtc_state,
@@ -1180,6 +1205,16 @@ static void hsw_set_infoframes(struct intel_encoder 
*encoder,
intel_write_infoframe(encoder, crtc_state,
  HDMI_INFOFRAME_TYPE_VENDOR,
  _state->infoframes.hdmi);
+
+   /*
+* Support HDR Metadata from Gen10 onwards
+* ToDo: Gen9 also can support HDR with LSPCON.
+* Support for the same to be enabled later.
+*/
+   if (INTEL_GEN(dev_priv) >= 10)
+   intel_write_infoframe(encoder, crtc_state,
+ HDMI_INFOFRAME_TYPE_DRM,
+ _state->infoframes.drm);
 }
 
 void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable)
@@ -2386,6 +2421,19 @@ int intel_hdmi_compute_config(struct intel_encoder 
*encoder,
return -EINVAL;
}
 
+   /*
+* Support HDR Metadata from Gen10 onwards
+* ToDo: Gen9 also can support HDR with LSPCON.
+* Support for the same to be enabled later.
+*/
+   if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
+   if (!intel_hdmi_compute_drm_infoframe(encoder, pipe_config,
+ conn_state)) {
+   DRM_DEBUG_KMS("bad DRM infoframe\n");
+   return -EINVAL;
+   }
+   }
+
return 0;
 }
 
-- 
1.9.1

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

[v9 04/13] drm: Enable HDR infoframe support

[Uma Shankar]

Enable Dynamic Range and Mastering Infoframe for HDR
content, which is defined in CEA 861.3 spec.

The metadata will be computed based on blending
policy in userspace compositors and passed as a connector
property blob to driver. The same will be sent as infoframe
to panel which support HDR.

Added the const version of infoframe for DRM metadata
for HDR.

v2: Rebase and added Ville's POC changes.

v3: No Change

v4: Addressed Shashank's review comments and merged the
patch making drm infoframe function arguments as constant.

v5: Rebase

v6: Fixed checkpatch warnings with --strict option. Addressed
Shashank's review comments and added his RB.

v7: Addressed Brian Starkey's review comments. Merged 2 patches
into one.

v8: Addressed Jonas Karlman review comments.

Signed-off-by: Uma Shankar 
Signed-off-by: Ville Syrjälä 
Reviewed-by: Shashank Sharma 
---
 drivers/gpu/drm/drm_edid.c |  48 
 drivers/video/hdmi.c   | 186 +
 include/drm/drm_edid.h |   5 ++
 include/linux/hdmi.h   |  27 +++
 4 files changed, 266 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index fe2c29b..5f48965 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4906,6 +4906,54 @@ static bool is_hdmi2_sink(struct drm_connector 
*connector)
 }
 
 /**
+ * drm_hdmi_infoframe_set_hdr_metadata() - fill an HDMI AVI infoframe with
+ * HDR metadata from userspace
+ * @frame: HDMI AVI infoframe
+ * @hdr_source_metadata: hdr_source_metadata info from userspace
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+int
+drm_hdmi_infoframe_set_hdr_metadata(struct hdmi_drm_infoframe *frame,
+   struct hdr_output_metadata *hdr_metadata)
+{
+   int err;
+
+   if (!frame || !hdr_metadata)
+   return true;
+
+   err = hdmi_drm_infoframe_init(frame);
+   if (err < 0)
+   return err;
+
+   DRM_DEBUG_KMS("type = %x\n", frame->type);
+
+   frame->length = sizeof(struct hdr_metadata_infoframe);
+
+   frame->eotf = hdr_metadata->hdmi_metadata_type1.eotf;
+   frame->metadata_type = hdr_metadata->hdmi_metadata_type1.metadata_type;
+
+   memcpy(>display_primaries,
+  _metadata->hdmi_metadata_type1.display_primaries, 12);
+
+   memcpy(>white_point,
+  _metadata->hdmi_metadata_type1.white_point, 4);
+
+   frame->max_display_mastering_luminance =
+   
hdr_metadata->hdmi_metadata_type1.max_display_mastering_luminance;
+   frame->min_display_mastering_luminance =
+   
hdr_metadata->hdmi_metadata_type1.min_display_mastering_luminance;
+   frame->max_fall = hdr_metadata->hdmi_metadata_type1.max_fall;
+   frame->max_cll = hdr_metadata->hdmi_metadata_type1.max_cll;
+
+   hdmi_infoframe_log(KERN_CRIT, NULL,
+  (union hdmi_infoframe *)frame);
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_hdmi_infoframe_set_hdr_metadata);
+
+/**
  * drm_hdmi_avi_infoframe_from_display_mode() - fill an HDMI AVI infoframe with
  *  data from a DRM display mode
  * @frame: HDMI AVI infoframe
diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index 799ae49..717ed7fb 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -650,6 +650,146 @@ ssize_t hdmi_vendor_infoframe_pack(struct 
hdmi_vendor_infoframe *frame,
return 0;
 }
 
+/**
+ * hdmi_drm_infoframe_init() - initialize an HDMI Dynaminc Range and
+ * mastering infoframe
+ * @frame: HDMI DRM infoframe
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int hdmi_drm_infoframe_init(struct hdmi_drm_infoframe *frame)
+{
+   memset(frame, 0, sizeof(*frame));
+
+   frame->type = HDMI_INFOFRAME_TYPE_DRM;
+   frame->version = 1;
+
+   return 0;
+}
+EXPORT_SYMBOL(hdmi_drm_infoframe_init);
+
+static int hdmi_drm_infoframe_check_only(const struct hdmi_drm_infoframe 
*frame)
+{
+   if (frame->type != HDMI_INFOFRAME_TYPE_DRM ||
+   frame->version != 1)
+   return -EINVAL;
+
+   return 0;
+}
+
+/**
+ * hdmi_drm_infoframe_check() - check a HDMI DRM infoframe
+ * @frame: HDMI DRM infoframe
+ *
+ * Validates that the infoframe is consistent.
+ * Returns 0 on success or a negative error code on failure.
+ */
+int hdmi_drm_infoframe_check(struct hdmi_drm_infoframe *frame)
+{
+   return hdmi_drm_infoframe_check_only(frame);
+}
+EXPORT_SYMBOL(hdmi_drm_infoframe_check);
+
+/**
+ * hdmi_drm_infoframe_pack() - write HDMI DRM infoframe to binary buffer
+ * @frame: HDMI DRM infoframe
+ * @buffer: destination buffer
+ * @size: size of buffer
+ *
+ * Packs the information contained in the @frame structure into a binary
+ * representation that can be written into the corresponding controller
+ * registers. Also computes the checksum as required by section 5.3.5 of
+ * the HDMI 

[v9 03/13] drm: Parse HDR metadata info from EDID

[Uma Shankar]

HDR metadata block is introduced in CEA-861.3 spec.
Parsing the same to get the panel's HDR metadata.

v2: Rebase and added Ville's POC changes to the patch.

v3: No Change

v4: Addressed Shashank's review comments

v5: Addressed Shashank's comment and added his RB.

v6: Addressed Jonas Karlman review comments.

Signed-off-by: Uma Shankar 
Reviewed-by: Shashank Sharma 
---
 drivers/gpu/drm/drm_edid.c | 52 ++
 1 file changed, 52 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 852bdd8..fe2c29b 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2852,6 +2852,7 @@ static int drm_cvt_modes(struct drm_connector *connector,
 #define VIDEO_BLOCK 0x02
 #define VENDOR_BLOCK0x03
 #define SPEAKER_BLOCK  0x04
+#define HDR_STATIC_METADATA_BLOCK  0x6
 #define USE_EXTENDED_TAG 0x07
 #define EXT_VIDEO_CAPABILITY_BLOCK 0x00
 #define EXT_VIDEO_DATA_BLOCK_420   0x0E
@@ -3599,6 +3600,12 @@ static int add_3d_struct_modes(struct drm_connector 
*connector, u16 structure,
 }
 
 static int
+cea_db_payload_len_ext(const u8 *db)
+{
+   return (db[0] & 0x1f) - 1;
+}
+
+static int
 cea_db_extended_tag(const u8 *db)
 {
return db[1];
@@ -3834,6 +3841,49 @@ static void fixup_detailed_cea_mode_clock(struct 
drm_display_mode *mode)
mode->clock = clock;
 }
 
+static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db)
+{
+   if (cea_db_tag(db) != USE_EXTENDED_TAG)
+   return false;
+
+   if (db[1] != HDR_STATIC_METADATA_BLOCK)
+   return false;
+
+   return true;
+}
+
+static uint8_t eotf_supported(const u8 *edid_ext)
+{
+   return edid_ext[2] &
+   (BIT(HDMI_EOTF_TRADITIONAL_GAMMA_SDR) |
+BIT(HDMI_EOTF_TRADITIONAL_GAMMA_HDR) |
+BIT(HDMI_EOTF_SMPTE_ST2084));
+}
+
+static uint8_t hdr_metadata_type(const u8 *edid_ext)
+{
+   return edid_ext[3] &
+   BIT(HDMI_STATIC_METADATA_TYPE1);
+}
+
+static void
+drm_parse_hdr_metadata_block(struct drm_connector *connector, const u8 *db)
+{
+   u16 len;
+
+   len = cea_db_payload_len_ext(db);
+   connector->hdr_sink_metadata.hdmi_type1.eotf = eotf_supported(db);
+   connector->hdr_sink_metadata.hdmi_type1.metadata_type =
+   hdr_metadata_type(db);
+
+   if (len >= 4)
+   connector->hdr_sink_metadata.hdmi_type1.max_cll = db[4];
+   if (len >= 5)
+   connector->hdr_sink_metadata.hdmi_type1.max_fall = db[5];
+   if (len >= 6)
+   connector->hdr_sink_metadata.hdmi_type1.min_cll = db[6];
+}
+
 static void
 drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db)
 {
@@ -4461,6 +4511,8 @@ static void drm_parse_cea_ext(struct drm_connector 
*connector,
drm_parse_y420cmdb_bitmap(connector, db);
if (cea_db_is_vcdb(db))
drm_parse_vcdb(connector, db);
+   if (cea_db_is_hdmi_hdr_metadata_block(db))
+   drm_parse_hdr_metadata_block(connector, db);
}
 }
 
-- 
1.9.1

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

[v9 01/13] drm: Add HDR source metadata property

[Uma Shankar]

This patch adds a blob property to get HDR metadata
information from userspace. This will be send as part
of AVI Infoframe to panel.

It also implements get() and set() functions for HDR output
metadata property.The blob data is received from userspace and
saved in connector state, the same is returned as blob in get
property call to userspace.

v2: Rebase and modified the metadata structure elements
as per Ville's POC changes.

v3: No Change

v4: Addressed Shashank's review comments

v5: Rebase.

v6: Addressed Brian Starkey's review comments, defined
new structure with header for dynamic metadata scalability.
Merge get/set property functions for metadata in this patch.

v7: Addressed Jonas Karlman review comments and defined separate
structure for infoframe to better align with CTA 861.G spec. Added
Shashank's RB.

v8: Addressed Ville's review comments. Moved sink metadata structure
out of uapi headers as suggested by Jonas Karlman.

Signed-off-by: Uma Shankar 
Reviewed-by: Shashank Sharma 
---
 drivers/gpu/drm/drm_atomic.c  |  2 ++
 drivers/gpu/drm/drm_atomic_uapi.c | 13 +
 drivers/gpu/drm/drm_connector.c   |  6 ++
 include/drm/drm_connector.h   | 11 +++
 include/drm/drm_mode_config.h |  6 ++
 include/linux/hdmi.h  | 26 ++
 include/uapi/drm/drm_mode.h   | 23 +++
 7 files changed, 87 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 5eb4013..8b9c126 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -881,6 +881,8 @@ static void drm_atomic_connector_print_state(struct 
drm_printer *p,
 
drm_printf(p, "connector[%u]: %s\n", connector->base.id, 
connector->name);
drm_printf(p, "\tcrtc=%s\n", state->crtc ? state->crtc->name : 
"(null)");
+   drm_printf(p, "\thdr_metadata_changed=%d\n",
+  state->hdr_metadata_changed);
 
if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK)
if (state->writeback_job && state->writeback_job->fb)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 428d826..2ecc79e 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -676,6 +676,8 @@ static int drm_atomic_connector_set_property(struct 
drm_connector *connector,
 {
struct drm_device *dev = connector->dev;
struct drm_mode_config *config = >mode_config;
+   bool replaced = false;
+   int ret;
 
if (property == config->prop_crtc_id) {
struct drm_crtc *crtc = drm_crtc_find(dev, file_priv, val);
@@ -726,6 +728,14 @@ static int drm_atomic_connector_set_property(struct 
drm_connector *connector,
 */
if (state->link_status != DRM_LINK_STATUS_GOOD)
state->link_status = val;
+   } else if (property == config->hdr_output_metadata_property) {
+   ret = drm_atomic_replace_property_blob_from_id(dev,
+   >hdr_output_metadata,
+   val,
+   -1, sizeof(struct hdr_output_metadata),
+   );
+   state->hdr_metadata_changed |= replaced;
+   return ret;
} else if (property == config->aspect_ratio_property) {
state->picture_aspect_ratio = val;
} else if (property == config->content_type_property) {
@@ -814,6 +824,9 @@ static int drm_atomic_connector_set_property(struct 
drm_connector *connector,
*val = state->colorspace;
} else if (property == connector->scaling_mode_property) {
*val = state->scaling_mode;
+   } else if (property == config->hdr_output_metadata_property) {
+   *val = state->hdr_output_metadata ?
+   state->hdr_output_metadata->base.id : 0;
} else if (property == connector->content_protection_property) {
*val = state->content_protection;
} else if (property == config->writeback_fb_id_property) {
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index b34c3d3..365ace0 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1058,6 +1058,12 @@ int drm_connector_create_standard_properties(struct 
drm_device *dev)
return -ENOMEM;
dev->mode_config.non_desktop_property = prop;
 
+   prop = drm_property_create(dev, DRM_MODE_PROP_BLOB,
+  "HDR_OUTPUT_METADATA", 0);
+   if (!prop)
+   return -ENOMEM;
+   dev->mode_config.hdr_output_metadata_property = prop;
+
return 0;
 }
 
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index f43f40d..e54674b 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -603,6 +603,13 @@ struct drm_connector_state {
 * and 

Re: [PATCH v6 5/6] drm/i915/dp: Change a link bandwidth computation for DP

[Ville Syrjälä]

On Wed, May 08, 2019 at 11:17:56AM +0300, Gwan-gyeong Mun wrote:
> Data M/N calculations were assumed a bpp as RGB format. But when we are
> using YCbCr 4:2:0 output format on DP, we should change bpp calculations
> as YCbCr 4:2:0 format. The pipe_bpp value was assumed RGB format,
> therefore, it was multiplied with 3. But YCbCr 4:2:0 requires a multiplier
> value to 1.5.
> Therefore we need to divide pipe_bpp to 2 while DP output uses YCbCr4:2:0
> format.
>  - RGB format bpp = bpc x 3
>  - YCbCr 4:2:0 format bpp = bpc x 1.5
> 
> But Link M/N values are calculated and applied based on the Full Clock for
> YCbCr 4:2:0. And DP YCbCr 4:2:0 does not need to pixel clock double for
> a dotclock caluation. Only for HDMI YCbCr 4:2:0 needs to pixel clock double
> for a dot clock calculation.
> 
> And it adds missed bpc values for a programming of VSC Header.
> It only affects dp and edp port which use YCbCr 4:2:0 output format.
> And for now, it does not consider a use case of DSC + YCbCr 4:2:0.
> 
> v2:
>   Addressed review comments from Ville.
>   Remove a changing of pipe_bpp on intel_ddi_set_pipe_settings().
>   Because the pipe is running at the full bpp, keep pipe_bpp as RGB
>   even though YCbCr 4:2:0 output format is used.
>   Add a link bandwidth computation for YCbCr4:2:0 output format.
> 
> v3:
>   Addressed reivew comments from Ville.
>   In order to make codes simple, it adds and uses intel_dp_output_bpp()
>   function.
> 
> v6:
>   Link M/N values are calculated and applied based on the Full Clock for
>   YCbCr420. The Bit per Pixel needs to be adjusted for YUV420 mode as it
>   requires only half of the RGB case.
> - Link M/N values are calculated and applied based on the Full Clock
> - Data M/N values needs to be calculated considering the data is half
>   due to subsampling
>   Remove a doubling of pixel clock on a dot clock calculator for
>   DP YCbCr 4:2:0.
>   Rebase and remove a duplicate setting of vsc_sdp.DB17.
>   Add a setting of dynamic range bit to  vsc_sdp.DB17.
>   Change Content Type bit to "Graphics" from "Not defined".
>   Change a dividing of pipe_bpp to muliplying to constant values on a
>   switch-case statement.
> 
> Cc: Ville Syrjälä 
> Signed-off-by: Gwan-gyeong Mun 
> ---
>  drivers/gpu/drm/i915/intel_ddi.c |  3 ++-
>  drivers/gpu/drm/i915/intel_dp.c  | 42 +---
>  2 files changed, 41 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 4441c5ba71fb..e22a0898b957 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1457,7 +1457,8 @@ static void ddi_dotclock_get(struct intel_crtc_state 
> *pipe_config)
>   else
>   dotclock = pipe_config->port_clock;
>  
> - if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
> + if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 &&
> + !intel_crtc_has_dp_encoder(pipe_config))
>   dotclock *= 2;
>  
>   if (pipe_config->pixel_multiplier)
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 74aad8830a80..c75e2bbe612a 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1842,6 +1842,19 @@ intel_dp_adjust_compliance_config(struct intel_dp 
> *intel_dp,
>   }
>  }
>  
> +static int intel_dp_output_bpp(const struct intel_crtc_state *crtc_state, 
> int bpp)
> +{
> + /*
> +  * bpp value was assumed to RGB format. And YCbCr 4:2:0 output
> +  * format of the number of bytes per pixel will be half the number
> +  * of bytes of RGB pixel.
> +  */
> + if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
> + bpp /= 2;
> +
> + return bpp;
> +}
> +
>  /* Optimize link config in order: max bpp, min clock, min lanes */
>  static int
>  intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
> @@ -2212,7 +2225,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>   if (pipe_config->dsc_params.compression_enable)
>   output_bpp = pipe_config->dsc_params.compressed_bpp;
>   else
> - output_bpp = pipe_config->pipe_bpp;
> + output_bpp = intel_dp_output_bpp(pipe_config, 
> pipe_config->pipe_bpp);
>  
>   intel_link_compute_m_n(output_bpp,
>  pipe_config->lane_count,
> @@ -4439,7 +4452,30 @@ intel_pixel_encoding_setup_vsc(struct intel_dp 
> *intel_dp,
>* 011b = 12bpc.
>* 100b = 16bpc.
>*/
> - vsc_sdp.DB17 = 0x1;
> + switch (crtc_state->pipe_bpp) {
> + case 24: /* 8bpc */
> + vsc_sdp.DB17 = 0x1;
> + break;
> + case 30: /* 10bpc */
> + vsc_sdp.DB17 = 0x2;
> + break;
> + case 36: /* 12bpc */
> + vsc_sdp.DB17 = 0x3;
> + break;
> + case 48: /* 16bpc */
> + vsc_sdp.DB17 = 0x4;
> + break;
> +

Re: [PATCH v6 3/6] drm/i915/dp: Program VSC Header and DB for Pixel Encoding/Colorimetry Format

[Ville Syrjälä]

On Wed, May 08, 2019 at 11:17:54AM +0300, Gwan-gyeong Mun wrote:
> Function intel_pixel_encoding_setup_vsc handles vsc header and data block
> setup for pixel encoding / colorimetry format.
> 
> Setup VSC header and data block in function intel_pixel_encoding_setup_vsc
> for pixel encoding / colorimetry format as per dp 1.4a spec,
> section 2.2.5.7.1, table 2-119: VSC SDP Header Bytes, section 2.2.5.7.5,
> table 2-120:VSC SDP Payload for DB16 through DB18.
> 
> v2:
>   Minor style fix. [Maarten]
>   Refer to commit ids instead of patchwork. [Maarten]
> 
> v6: Rebase
> 
> Cc: Maarten Lankhorst 
> Signed-off-by: Gwan-gyeong Mun 
> ---
>  drivers/gpu/drm/i915/intel_ddi.c |  1 +
>  drivers/gpu/drm/i915/intel_dp.c  | 73 
>  drivers/gpu/drm/i915/intel_drv.h |  2 +
>  3 files changed, 76 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> b/drivers/gpu/drm/i915/intel_ddi.c
> index cd5277d98b03..2f1688ea5a2c 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3391,6 +3391,7 @@ static void intel_enable_ddi_dp(struct intel_encoder 
> *encoder,
>  
>   intel_edp_backlight_on(crtc_state, conn_state);
>   intel_psr_enable(intel_dp, crtc_state);
> + intel_dp_ycbcr_420_enable(intel_dp, crtc_state);

I wonder if this is a bit too late. But we do it for PSR here, so I
guess we should think about this for both cases. We should actually
add full readout + state checker for the VSC SDP for both cases as
well. But that can be done later.

>   intel_edp_drrs_enable(intel_dp, crtc_state);
>  
>   if (crtc_state->has_audio)
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 06a3417a88d1..74aad8830a80 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4394,6 +4394,79 @@ u8 intel_dp_dsc_get_slice_count(struct intel_dp 
> *intel_dp,
>   return 0;
>  }
>  
> +static void
> +intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp,
> +const struct intel_crtc_state *crtc_state)
> +{
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct dp_vsc_sdp vsc_sdp;
> +
> + if (!intel_dp->attached_connector->base.ycbcr_420_allowed)
> + return;
> +
> + /* Prepare VSC Header for SU as per DP 1.4a spec, Table 2-119 */
> + memset(_sdp, 0, sizeof(vsc_sdp));

Can be replaced with '= {}' in the declaration.

> + vsc_sdp.sdp_header.HB0 = 0;
> + vsc_sdp.sdp_header.HB1 = 0x7;
> +
> + /* VSC SDP supporting 3D stereo, PSR2, and Pixel Encoding/
> +  * Colorimetry Format indication. A DP Source device is allowed
> +  * to indicate the pixel encoding/colorimetry format to the DP Sink
> +  * device with VSC SDP only when the DP Sink device supports it
> +  * (i.e., VSC_SDP_EXTENSION_FOR_COLORIMETRY_SUPPORTED bit in the 
> register
> +  * DPRX_FEATURE_ENUMERATION_LIST (DPCD Address 02210h, bit 3) is set to 
> 1)
> +  */

Are we checking that bit somewhere? I suppose the sink might a bit nuts
if it declares 420_only modes without that set, but maybe it should be
checked anyway.

Also non-standard comment format all over. Should be
/*
 * blah
 */

> + vsc_sdp.sdp_header.HB2 = 0x5;
> +
> + /* VSC SDP supporting 3D stereo, + PSR2, + Pixel Encoding/
> +  * Colorimetry Format indication (HB2 = 05h).
> +  */
> + vsc_sdp.sdp_header.HB3 = 0x13;
> + /* YCbCr 420 = 3h DB16[7:4] ITU-R BT.601 = 0h, ITU-R BT.709 = 1h
> +  * DB16[3:0] DP 1.4a spec, Table 2-120
> +  */
> +
> + /* Commit id (25edf91501b8 "drm/i915: prepare csc unit for YCBCR420 
> output")
> +  * uses the BT.709 color space to perform RGB->YCBCR conversion.
> +  */

I don't think referring to specific commit here is particularly helpful.
The situation will change anyway at some point.

> + vsc_sdp.DB16 = 0x3 << 4; /* 0x3 << 4 , YCbCr 420*/
> + vsc_sdp.DB16 |= 0x1; /* 0x1, ITU-R BT.709 */
> +
> + /* For pixel encoding formats YCbCr444, YCbCr422, YCbCr420, and Y Only,
> +  * the following Component Bit Depth values are defined:
> +  * 001b = 8bpc.
> +  * 010b = 10bpc.
> +  * 011b = 12bpc.
> +  * 100b = 16bpc.
> +  */
> + vsc_sdp.DB17 = 0x1;

Don't we need to base this on pipe_bpp? 

And I'm thinking we want the CEA bit set here as well.

> +
> + /*
> +  * Content Type (Bits 2:0)
> +  * 000b = Not defined.
> +  * 001b = Graphics.
> +  * 010b = Photo.
> +  * 011b = Video.
> +  * 100b = Game
> +  * All other values are RESERVED.
> +  * Note: See CTA-861-G for the definition and expected
> +  * processing by a stream sink for the above contect types.
> +  */
> + vsc_sdp.DB18 = 0;

Hmm. I guess we could add the content type prop for DP too.
But that's something for the future.

All the magic numbers should be defined somewhere in the core dp header.
But that could be done 

Re: [PATCH AUTOSEL 4.19 29/53] drm/amdkfd: Add picasso pci id

[Sasha Levin]

On Sat, Apr 27, 2019 at 01:49:27PM +, Deucher, Alexander wrote:

NACK.  4.19 did not contain support for picasso. Please drop this patch for 
4.19.


Dropped, thank you!

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

Re: [PATCH v6 2/6] drm: Add a VSC structure for handling Pixel Encoding/Colorimetry Formats

[Ville Syrjälä]

On Wed, May 08, 2019 at 11:17:53AM +0300, Gwan-gyeong Mun wrote:
> SDP VSC Header and Data Block follow DP 1.4a spec, section 2.2.5.7.5,
> chapter "VSC SDP Payload for Pixel Encoding/Colorimetry Format".
> 
> Signed-off-by: Gwan-gyeong Mun 
> Reviewed-by: Maarten Lankhorst 
> ---
>  include/drm/drm_dp_helper.h | 17 +
>  1 file changed, 17 insertions(+)
> 
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 97ce790a5b5a..3793bea7b7fe 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1096,6 +1096,23 @@ struct edp_vsc_psr {
>   u8 DB8_31[24]; /* Reserved */
>  } __packed;
>  
> +struct dp_vsc_sdp {
> + struct dp_sdp_header sdp_header;
> + u8 DB0; /* Stereo Interface */
> + u8 DB1; /* 0 - PSR State; 1 - Update RFB; 2 - CRC Valid */
> + u8 DB2; /* CRC value bits 7:0 of the R or Cr component */
> + u8 DB3; /* CRC value bits 15:8 of the R or Cr component */
> + u8 DB4; /* CRC value bits 7:0 of the G or Y component */
> + u8 DB5; /* CRC value bits 15:8 of the G or Y component */
> + u8 DB6; /* CRC value bits 7:0 of the B or Cb component */
> + u8 DB7; /* CRC value bits 15:8 of the B or Cb component */
> + u8 DB8_15[8];  /* Reserved */
> + u8 DB16; /* Pixel Encoding and Colorimetry Formats */
> + u8 DB17; /* Dynamic Range and Component Bit Depth */
> + u8 DB18; /* Content Type */
> + u8 DB19_31[13]; /* Reserved */
> +} __packed;

Isn't this the same thing we have for edp already? Just rename the edp
struct and add the missing stuff?

> +
>  #define EDP_VSC_PSR_STATE_ACTIVE (1<<0)
>  #define EDP_VSC_PSR_UPDATE_RFB   (1<<1)
>  #define EDP_VSC_PSR_CRC_VALUES_VALID (1<<2)
> -- 
> 2.21.0

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

[Bug 109526] [CARRIZO] amdgpu fails to resume from S3, atombios stuck executing C554 (len 629, WS 0, PS 0)

[bugzilla-daemon]

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

--- Comment #6 from Johannes Hirte  ---
ping? still an issue with kernel 5.1

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

Re: [PATCH 0/2] drm: imx: Add NWL MIPI DSI host controller support

[Guido Günther]

Hi,
On Thu, Mar 07, 2019 at 11:30:51AM +0100, Guido Günther wrote:
> This adds initial support for the NWL MIPI DSI Host controller found on i.MX8
> SoCs.
> 
> It adds support for the i.MX8MQ but the same IP core can also be found on e.g.
> i.MX8QXP. I added the necessary hooks to support other imx8 variants but since
> I only have imx8mq boards to test I omitted the platform data for other SoCs.
> 
> The code is based on NXPs BSP so I added Robert Chiras as Co-authored-by but
> I'm happy to swap Author: and Co-authored-by: if that looks more appropriate.
> The most notable changes over the BSP driver are
>  - Calculate HS mode timing from phy_configure_opts_mipi_dphy
>  - Perform all clock setup via DT
>  - Merge nwl-imx and nwl drivers
>  - Add B0 silion revision quirk
> 
> Posting this is likely a bit premature (hence v0) but I wanted for one show 
> how
> this hooks into the mixel dphy posted earlier [1] and avoid duplicating work.
> So if there's other code out there doing the same I'm be happy to merge
> efforts.

Since this is likely not going anywhere until we have a dcss driver aimed
for mainline I'm not going spam the list with further revisions. However
the 5.x version is maintained here:


https://source.puri.sm/guido.gunther/linux-imx8/tree/forward-upstream/next-20190506/imx-nwl/v1-wip

Feedback is still welcome. It'll eventually be forwarded to newer
linux-next versions.

Changes over the posted version are:

- Add quirk for IMQ8MQ silicon B0 revision to not mess with the
  system reset controller on power down since enable won't work
  afterwards otherwise.
- Disable tx esc clock *after* the phy power down to unbreak
  disable/enable (unblank/blank)
- Drop devm_free_irq() handled by the device driver core
- Add ports to dt binding docs
- Select GENERIC_PHY_MIPI_DPHY instead of GENERIC_PHY for
  phy_mipi_dphy_get_default_config
- Include drm_print.h to fix build on next-20190408
- Drop some debugging messages
- Newline terminate all DRM_ printouts

If somebody is working on DCSS support it'd be cool to know since this
driver is currently a component of imx-display-subsystem which will only
work out if dcss is handled like this as well.
Cheers,
 -- Guido

> 
> It has been tested quite bit (in a version backported to 4.18) on Librem 5
> devkit using DCSS (which is not mainlined yet) and a MIPI DSI panel[2]. In
> principle LCDIF can also act as input source. I intend look into next so this
> can actually be tested without further patches on mainline kernels.
> 
> [1]: https://lists.freedesktop.org/archives/dri-devel/2019-March/209680.html
> [2]: 
> https://source.puri.sm/guido.gunther/linux-imx8/tree/imx8-4.18-wip-nwl-dsi-rework
> 
> Guido Günther (2):
>   dt-bindings: imx: Add binding for IMX NWL mipi dsi host controller
>   drm/imx: Add NWL MIPI DSI host controller support
> 
>  .../bindings/display/imx/imx-nwl-dsi.txt  |  72 ++
>  drivers/gpu/drm/Kconfig   |   2 +
>  drivers/gpu/drm/Makefile  |   1 +
>  drivers/gpu/drm/nwl/Kconfig   |  12 +
>  drivers/gpu/drm/nwl/Makefile  |   2 +
>  drivers/gpu/drm/nwl/nwl-drv.c | 594 ++
>  drivers/gpu/drm/nwl/nwl-drv.h |  68 ++
>  drivers/gpu/drm/nwl/nwl-dsi.c | 752 ++
>  drivers/gpu/drm/nwl/nwl-dsi.h | 105 +++
>  9 files changed, 1608 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/imx/imx-nwl-dsi.txt
>  create mode 100644 drivers/gpu/drm/nwl/Kconfig
>  create mode 100644 drivers/gpu/drm/nwl/Makefile
>  create mode 100644 drivers/gpu/drm/nwl/nwl-drv.c
>  create mode 100644 drivers/gpu/drm/nwl/nwl-drv.h
>  create mode 100644 drivers/gpu/drm/nwl/nwl-dsi.c
>  create mode 100644 drivers/gpu/drm/nwl/nwl-dsi.h
> 
> -- 
> 2.20.1
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 05/11] drm: Add helpers to kick off self refresh mode in drivers

[Daniel Vetter]

On Wed, May 08, 2019 at 12:09:10PM -0400, Sean Paul wrote:
> From: Sean Paul 
> 
> This patch adds a new drm helper library to help drivers implement
> self refresh. Drivers choosing to use it will register crtcs and
> will receive callbacks when it's time to enter or exit self refresh
> mode.
> 
> In its current form, it has a timer which will trigger after a
> driver-specified amount of inactivity. When the timer triggers, the
> helpers will submit a new atomic commit to shut the refreshing pipe
> off. On the next atomic commit, the drm core will revert the self
> refresh state and bring everything back up to be actively driven.
> 
> From the driver's perspective, this works like a regular disable/enable
> cycle. The driver need only check the 'self_refresh_active' state in
> crtc_state. It should initiate self refresh mode on the panel and enter
> an off or low-power state.
> 
> Changes in v2:
> - s/psr/self_refresh/ (Daniel)
> - integrated the psr exit into the commit that wakes it up (Jose/Daniel)
> - made the psr state per-crtc (Jose/Daniel)
> Changes in v3:
> - Remove the self_refresh_(active|changed) from connector state (Daniel)
> - Simplify loop in drm_self_refresh_helper_alter_state (Daniel)
> - Improve self_refresh_aware comment (Daniel)
> - s/self_refresh_state/self_refresh_data/ (Daniel)
> Changes in v4:
> - Move docbook location below panel (Daniel)
> - Improve docbook with references and more detailed explanation (Daniel)
> - Instead of register/unregister, use init/cleanup (Daniel)

Again missed my r-b or didn't apply all my suggestions? I'm feeling a bit
blue and all that today so don't want to do more than necessary with
actual patch review :-)

Cheers, Daniel


> 
> Link to v1: 
> https://patchwork.freedesktop.org/patch/msgid/20190228210939.83386-2-s...@poorly.run
> Link to v2: 
> https://patchwork.freedesktop.org/patch/msgid/20190326204509.96515-1-s...@poorly.run
> Link to v3: 
> https://patchwork.freedesktop.org/patch/msgid/20190502194956.218441-6-s...@poorly.run
> 
> Cc: Daniel Vetter 
> Cc: Jose Souza 
> Cc: Zain Wang 
> Cc: Tomasz Figa 
> Cc: Ville Syrjälä 
> Signed-off-by: Sean Paul 
> ---
>  Documentation/gpu/drm-kms-helpers.rst |   9 +
>  drivers/gpu/drm/Makefile  |   2 +-
>  drivers/gpu/drm/drm_atomic.c  |   2 +
>  drivers/gpu/drm/drm_atomic_helper.c   |  35 +++-
>  drivers/gpu/drm/drm_atomic_state_helper.c |   4 +
>  drivers/gpu/drm/drm_atomic_uapi.c |   7 +-
>  drivers/gpu/drm/drm_self_refresh_helper.c | 213 ++
>  include/drm/drm_atomic.h  |  15 ++
>  include/drm/drm_connector.h   |  14 ++
>  include/drm/drm_crtc.h|  19 ++
>  include/drm/drm_self_refresh_helper.h |  22 +++
>  11 files changed, 337 insertions(+), 5 deletions(-)
>  create mode 100644 drivers/gpu/drm/drm_self_refresh_helper.c
>  create mode 100644 include/drm/drm_self_refresh_helper.h
> 
> diff --git a/Documentation/gpu/drm-kms-helpers.rst 
> b/Documentation/gpu/drm-kms-helpers.rst
> index 14102ae035dc..86dbb56aef51 100644
> --- a/Documentation/gpu/drm-kms-helpers.rst
> +++ b/Documentation/gpu/drm-kms-helpers.rst
> @@ -181,6 +181,15 @@ Panel Helper Reference
>  .. kernel-doc:: drivers/gpu/drm/drm_panel_orientation_quirks.c
> :export:
>  
> +Panel Self Refresh Helper Reference
> +===
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_self_refresh_helper.c
> +   :doc: overview
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_self_refresh_helper.c
> +   :export:
> +
>  Display Port Helper Functions Reference
>  ===
>  
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 3d0c75cd687c..c4852604fc1d 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -39,7 +39,7 @@ drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o 
> drm_dsc.o drm_probe_helper
>   drm_simple_kms_helper.o drm_modeset_helper.o \
>   drm_scdc_helper.o drm_gem_framebuffer_helper.o \
>   drm_atomic_state_helper.o drm_damage_helper.o \
> - drm_format_helper.o
> + drm_format_helper.o drm_self_refresh_helper.o
>  
>  drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o
>  drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 936002495523..2b92648ce196 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -379,6 +379,7 @@ static void drm_atomic_crtc_print_state(struct 
> drm_printer *p,
>   drm_printf(p, "crtc[%u]: %s\n", crtc->base.id, crtc->name);
>   drm_printf(p, "\tenable=%d\n", state->enable);
>   drm_printf(p, "\tactive=%d\n", state->active);
> + drm_printf(p, "\tself_refresh_active=%d\n", state->self_refresh_active);
>   drm_printf(p, "\tplanes_changed=%d\n", state->planes_changed);
>   drm_printf(p, 

Re: [PATCH v4 02/11] drm: Add drm_atomic_get_(old|new-_connector_for_encoder() helpers

[Daniel Vetter]

On Wed, May 08, 2019 at 12:09:07PM -0400, Sean Paul wrote:
> From: Laurent Pinchart 
> 
> Add functions to the atomic core to retrieve the old and new connectors
> associated with an encoder in a drm_atomic_state. This is useful for
> encoders and bridges that need to access the connector, for instance for
> the drm_display_info.
> 
> The CRTC associated with the encoder can also be retrieved through the
> connector state, and from it, the old and new CRTC states.
> 
> Changed in v4:
> - Added to the set
> 
> Cc: Daniel Vetter 
> Signed-off-by: Laurent Pinchart 
> 
> [seanpaul removed WARNs from helpers and added docs to explain why
> returning NULL might be valid]
> Signed-off-by: Sean Paul 
> ---
>  drivers/gpu/drm/drm_atomic.c | 70 
>  include/drm/drm_atomic.h |  7 
>  2 files changed, 77 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 5eb40130fafb..936002495523 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -797,6 +797,76 @@ drm_atomic_get_private_obj_state(struct drm_atomic_state 
> *state,
>  }
>  EXPORT_SYMBOL(drm_atomic_get_private_obj_state);
>  
> +/**
> + * drm_atomic_get_old_connector_for_encoder - Get old connector for an 
> encoder
> + * @state: Atomic state
> + * @encoder: The encoder to fetch the connector state for
> + *
> + * This function finds and returns the connector that was connected to 
> @encoder
> + * as specified by the @state.
> + *
> + * If there is no connector in @state which previously had @encoder 
> connected to
> + * it, this function will return NULL. While this may seem like an invalid 
> use
> + * case, it is sometimes a useful to differentiate commits which had no prior
> + * connectors attached to @encoder vs ones that did (and to inspect their
> + * @state). This is especially true in enable hooks because the pipeline has
> + * changed/will change.

s/has changed// ... I meant you'll pick the right ver tense :-)

> + *
> + * Returns: The old connector connected to @encoder, or NULL if the encoder 
> is
> + * not connected.
> + */
> +struct drm_connector *
> +drm_atomic_get_old_connector_for_encoder(struct drm_atomic_state *state,
> +  struct drm_encoder *encoder)
> +{
> + struct drm_connector_state *conn_state;
> + struct drm_connector *connector;
> + unsigned int i;
> +
> + for_each_old_connector_in_state(state, connector, conn_state, i) {
> + if (conn_state->best_encoder == encoder)
> + return connector;
> + }
> +
> + return NULL;
> +}
> +EXPORT_SYMBOL(drm_atomic_get_old_connector_for_encoder);
> +
> +/**
> + * drm_atomic_get_new_connector_for_encoder - Get new connector for an 
> encoder
> + * @state: Atomic state
> + * @encoder: The encoder to fetch the connector state for
> + *
> + * This function finds and returns the connector that will be connected to
> + * @encoder as specified by the @state.
> + *
> + * If there is no connector in @state which will have @encoder connected to 
> it,
> + * this function will return NULL. While this may seem like an invalid use 
> case,
> + * it is sometimes a useful to differentiate commits which have no connectors
> + * attached to @encoder vs ones that do (and to inspect their state). This is
> + * especially true in disable hooks because the pipeline has changed/will
> + * change.

s/will change//

> + *
> + * Returns: The new connector connected to @encoder, or NULL if the encoder 
> is
> + * not connected.
> + */
> +struct drm_connector *
> +drm_atomic_get_new_connector_for_encoder(struct drm_atomic_state *state,
> +  struct drm_encoder *encoder)
> +{
> + struct drm_connector_state *conn_state;
> + struct drm_connector *connector;
> + unsigned int i;
> +
> + for_each_new_connector_in_state(state, connector, conn_state, i) {
> + if (conn_state->best_encoder == encoder)
> + return connector;
> + }
> +
> + return NULL;
> +}
> +EXPORT_SYMBOL(drm_atomic_get_new_connector_for_encoder);

Maybe also add a "See also drm_atomic_get_old_connector_for_encoder() and
drm_atomic_get_new_connector_for_encoder()." to the kerneldoc of
drm_connector_state.best_encoder.

With these doc nits:

Reviewed-by: Daniel Vetter 

> +
>  /**
>   * drm_atomic_get_connector_state - get connector state
>   * @state: global atomic state object
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index 824a5ed4e216..d6b3acd34e1c 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -453,6 +453,13 @@ struct drm_private_state * __must_check
>  drm_atomic_get_private_obj_state(struct drm_atomic_state *state,
>struct drm_private_obj *obj);
>  
> +struct drm_connector *
> +drm_atomic_get_old_connector_for_encoder(struct drm_atomic_state *state,
> +   

Re: [PATCH v4 01/11] drm: Add atomic variants of enable/disable to encoder helper funcs

[Daniel Vetter]

On Wed, May 08, 2019 at 12:09:06PM -0400, Sean Paul wrote:
> From: Sean Paul 
> 
> This patch adds atomic_enable and atomic_disable callbacks to the
> encoder helpers. This will allow encoders to make informed decisions in
> their start-up/shutdown based on the committed state.
> 
> Aside from the new hooks, this patch also introduces the new signature
> for .atomic_* functions going forward. Instead of passing object state
> (well, encoders don't have atomic state, but let's ignore that), we pass
> the entire atomic state so the driver can inspect more than what's
> happening locally.
> 
> This is particularly important for the upcoming self refresh helpers.
> 
> Changes in v3:
> - Added patch to the set
> Changes in v4:
> - Move atomic_disable above prepare (Daniel)
> - Add breadcrumb to .enable() docbook (Daniel)

Why no r-b: me or did you not apply all my suggestions? Too lazy to read
it all again :-)
-Daniel

> 
> Link to v3: 
> https://patchwork.freedesktop.org/patch/msgid/20190502194956.218441-2-s...@poorly.run
> 
> Cc: Daniel Vetter 
> Cc: Ville Syrjälä 
> Signed-off-by: Sean Paul 
> ---
>  drivers/gpu/drm/drm_atomic_helper.c  |  8 +++-
>  include/drm/drm_modeset_helper_vtables.h | 48 
>  2 files changed, 54 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 553415fe8ede..ccf01831f265 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -999,7 +999,9 @@ disable_outputs(struct drm_device *dev, struct 
> drm_atomic_state *old_state)
>  
>   /* Right function depends upon target state. */
>   if (funcs) {
> - if (new_conn_state->crtc && funcs->prepare)
> + if (funcs->atomic_disable)
> + funcs->atomic_disable(encoder, old_state);
> + else if (new_conn_state->crtc && funcs->prepare)
>   funcs->prepare(encoder);
>   else if (funcs->disable)
>   funcs->disable(encoder);
> @@ -1309,7 +1311,9 @@ void drm_atomic_helper_commit_modeset_enables(struct 
> drm_device *dev,
>   drm_bridge_pre_enable(encoder->bridge);
>  
>   if (funcs) {
> - if (funcs->enable)
> + if (funcs->atomic_enable)
> + funcs->atomic_enable(encoder, old_state);
> + else if (funcs->enable)
>   funcs->enable(encoder);
>   else if (funcs->commit)
>   funcs->commit(encoder);
> diff --git a/include/drm/drm_modeset_helper_vtables.h 
> b/include/drm/drm_modeset_helper_vtables.h
> index 8f3602811eb5..aa509c107083 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -675,6 +675,51 @@ struct drm_encoder_helper_funcs {
>   enum drm_connector_status (*detect)(struct drm_encoder *encoder,
>   struct drm_connector *connector);
>  
> + /**
> +  * @atomic_disable:
> +  *
> +  * This callback should be used to disable the encoder. With the atomic
> +  * drivers it is called before this encoder's CRTC has been shut off
> +  * using their own _crtc_helper_funcs.atomic_disable hook. If that
> +  * sequence is too simple drivers can just add their own driver private
> +  * encoder hooks and call them from CRTC's callback by looping over all
> +  * encoders connected to it using for_each_encoder_on_crtc().
> +  *
> +  * This callback is a variant of @disable that provides the atomic state
> +  * to the driver. It takes priority over @disable during atomic commits.
> +  *
> +  * This hook is used only by atomic helpers. Atomic drivers don't need
> +  * to implement it if there's no need to disable anything at the encoder
> +  * level. To ensure that runtime PM handling (using either DPMS or the
> +  * new "ACTIVE" property) works @atomic_disable must be the inverse of
> +  * @atomic_enable.
> +  */
> + void (*atomic_disable)(struct drm_encoder *encoder,
> +struct drm_atomic_state *state);
> +
> + /**
> +  * @atomic_enable:
> +  *
> +  * This callback should be used to enable the encoder. It is called
> +  * after this encoder's CRTC has been enabled using their own
> +  * _crtc_helper_funcs.atomic_enable hook. If that sequence is
> +  * too simple drivers can just add their own driver private encoder
> +  * hooks and call them from CRTC's callback by looping over all encoders
> +  * connected to it using for_each_encoder_on_crtc().
> +  *
> +  * This callback is a variant of @enable that provides the atomic state
> +  * to the driver. It is called in place of @enable during atomic
> +

[PATCH v4 05/11] drm: Add helpers to kick off self refresh mode in drivers

[Sean Paul]

From: Sean Paul 

This patch adds a new drm helper library to help drivers implement
self refresh. Drivers choosing to use it will register crtcs and
will receive callbacks when it's time to enter or exit self refresh
mode.

In its current form, it has a timer which will trigger after a
driver-specified amount of inactivity. When the timer triggers, the
helpers will submit a new atomic commit to shut the refreshing pipe
off. On the next atomic commit, the drm core will revert the self
refresh state and bring everything back up to be actively driven.

From the driver's perspective, this works like a regular disable/enable
cycle. The driver need only check the 'self_refresh_active' state in
crtc_state. It should initiate self refresh mode on the panel and enter
an off or low-power state.

Changes in v2:
- s/psr/self_refresh/ (Daniel)
- integrated the psr exit into the commit that wakes it up (Jose/Daniel)
- made the psr state per-crtc (Jose/Daniel)
Changes in v3:
- Remove the self_refresh_(active|changed) from connector state (Daniel)
- Simplify loop in drm_self_refresh_helper_alter_state (Daniel)
- Improve self_refresh_aware comment (Daniel)
- s/self_refresh_state/self_refresh_data/ (Daniel)
Changes in v4:
- Move docbook location below panel (Daniel)
- Improve docbook with references and more detailed explanation (Daniel)
- Instead of register/unregister, use init/cleanup (Daniel)

Link to v1: 
https://patchwork.freedesktop.org/patch/msgid/20190228210939.83386-2-s...@poorly.run
Link to v2: 
https://patchwork.freedesktop.org/patch/msgid/20190326204509.96515-1-s...@poorly.run
Link to v3: 
https://patchwork.freedesktop.org/patch/msgid/20190502194956.218441-6-s...@poorly.run

Cc: Daniel Vetter 
Cc: Jose Souza 
Cc: Zain Wang 
Cc: Tomasz Figa 
Cc: Ville Syrjälä 
Signed-off-by: Sean Paul 
---
 Documentation/gpu/drm-kms-helpers.rst |   9 +
 drivers/gpu/drm/Makefile  |   2 +-
 drivers/gpu/drm/drm_atomic.c  |   2 +
 drivers/gpu/drm/drm_atomic_helper.c   |  35 +++-
 drivers/gpu/drm/drm_atomic_state_helper.c |   4 +
 drivers/gpu/drm/drm_atomic_uapi.c |   7 +-
 drivers/gpu/drm/drm_self_refresh_helper.c | 213 ++
 include/drm/drm_atomic.h  |  15 ++
 include/drm/drm_connector.h   |  14 ++
 include/drm/drm_crtc.h|  19 ++
 include/drm/drm_self_refresh_helper.h |  22 +++
 11 files changed, 337 insertions(+), 5 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_self_refresh_helper.c
 create mode 100644 include/drm/drm_self_refresh_helper.h

diff --git a/Documentation/gpu/drm-kms-helpers.rst 
b/Documentation/gpu/drm-kms-helpers.rst
index 14102ae035dc..86dbb56aef51 100644
--- a/Documentation/gpu/drm-kms-helpers.rst
+++ b/Documentation/gpu/drm-kms-helpers.rst
@@ -181,6 +181,15 @@ Panel Helper Reference
 .. kernel-doc:: drivers/gpu/drm/drm_panel_orientation_quirks.c
:export:
 
+Panel Self Refresh Helper Reference
+===
+
+.. kernel-doc:: drivers/gpu/drm/drm_self_refresh_helper.c
+   :doc: overview
+
+.. kernel-doc:: drivers/gpu/drm/drm_self_refresh_helper.c
+   :export:
+
 Display Port Helper Functions Reference
 ===
 
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 3d0c75cd687c..c4852604fc1d 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -39,7 +39,7 @@ drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o 
drm_dsc.o drm_probe_helper
drm_simple_kms_helper.o drm_modeset_helper.o \
drm_scdc_helper.o drm_gem_framebuffer_helper.o \
drm_atomic_state_helper.o drm_damage_helper.o \
-   drm_format_helper.o
+   drm_format_helper.o drm_self_refresh_helper.o
 
 drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o
 drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 936002495523..2b92648ce196 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -379,6 +379,7 @@ static void drm_atomic_crtc_print_state(struct drm_printer 
*p,
drm_printf(p, "crtc[%u]: %s\n", crtc->base.id, crtc->name);
drm_printf(p, "\tenable=%d\n", state->enable);
drm_printf(p, "\tactive=%d\n", state->active);
+   drm_printf(p, "\tself_refresh_active=%d\n", state->self_refresh_active);
drm_printf(p, "\tplanes_changed=%d\n", state->planes_changed);
drm_printf(p, "\tmode_changed=%d\n", state->mode_changed);
drm_printf(p, "\tactive_changed=%d\n", state->active_changed);
@@ -951,6 +952,7 @@ static void drm_atomic_connector_print_state(struct 
drm_printer *p,
 
drm_printf(p, "connector[%u]: %s\n", connector->base.id, 
connector->name);
drm_printf(p, "\tcrtc=%s\n", state->crtc ? state->crtc->name : 
"(null)");
+   drm_printf(p, "\tself_refresh_aware=%d\n", state->self_refresh_aware);
 
 

[PATCH v4 06/11] drm/rockchip: Use dirtyfb helper

[Sean Paul]

From: Sean Paul 

Instead of flushing all vops every time we get a dirtyfb call, use the
damage helper to kick off an atomic commit. Even though we don't use
damage clips, the helper commit will force us through the normal
psr_inhibit_get/put sequence.

Changes in v3:
- Added to the set
Changes in v4:
- None

Link to v3: 
https://patchwork.freedesktop.org/patch/msgid/20190502194956.218441-7-s...@poorly.run

Suggested-by: Daniel Vetter 
Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 13 ++---
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
index 97438bbbe389..02e81ca2d933 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -25,20 +26,10 @@
 #include "rockchip_drm_gem.h"
 #include "rockchip_drm_psr.h"
 
-static int rockchip_drm_fb_dirty(struct drm_framebuffer *fb,
-struct drm_file *file,
-unsigned int flags, unsigned int color,
-struct drm_clip_rect *clips,
-unsigned int num_clips)
-{
-   rockchip_drm_psr_flush_all(fb->dev);
-   return 0;
-}
-
 static const struct drm_framebuffer_funcs rockchip_drm_fb_funcs = {
.destroy   = drm_gem_fb_destroy,
.create_handle = drm_gem_fb_create_handle,
-   .dirty = rockchip_drm_fb_dirty,
+   .dirty = drm_atomic_helper_dirtyfb,
 };
 
 static struct drm_framebuffer *
-- 
Sean Paul, Software Engineer, Google / Chromium OS

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

[PATCH v4 10/11] drm/rockchip: Don't fully disable vop on self refresh

[Sean Paul]

From: Sean Paul 

Instead of fully disabling and re-enabling the vop on self refresh
transitions, only disable the active windows. This will speed up
self refresh exits substantially and is still a power-savings win.

This patch integrates portions of Zain's patch from here:
https://patchwork.kernel.org/patch/9615063/

Changes in v2:
- None
Changes in v3:
- None
Changes in v4:
- Adjust for preceding vop_win_disable changes

Link to v1: 
https://patchwork.freedesktop.org/patch/msgid/20190228210939.83386-5-s...@poorly.run
Link to v2: 
https://patchwork.freedesktop.org/patch/msgid/20190326204509.96515-4-s...@poorly.run
Link to v3: 
https://patchwork.freedesktop.org/patch/msgid/20190502194956.218441-10-s...@poorly.run

Cc: Zain Wang 
Cc: Tomasz Figa 
Cc: Kristian H. Kristensen 
Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 41 ++---
 1 file changed, 36 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 15a5b44eb7e7..acdc86a9144b 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -134,6 +134,7 @@ struct vop {
bool is_enabled;
 
struct completion dsp_hold_completion;
+   unsigned int win_enabled;
 
/* protected by dev->event_lock */
struct drm_pending_vblank_event *event;
@@ -555,6 +556,7 @@ static void vop_win_disable(struct vop *vop, const struct 
vop_win *vop_win)
}
 
VOP_WIN_SET(vop, win, enable, 0);
+   vop->win_enabled &= ~BIT(VOP_WIN_TO_INDEX(vop_win));
 }
 
 static int vop_enable(struct drm_crtc *crtc, struct drm_crtc_state *old_state)
@@ -637,6 +639,25 @@ static int vop_enable(struct drm_crtc *crtc, struct 
drm_crtc_state *old_state)
return ret;
 }
 
+static void rockchip_drm_set_win_enabled(struct drm_crtc *crtc, bool enabled)
+{
+struct vop *vop = to_vop(crtc);
+int i;
+
+spin_lock(>reg_lock);
+
+for (i = 0; i < vop->data->win_size; i++) {
+struct vop_win *vop_win = >win[i];
+const struct vop_win_data *win = vop_win->data;
+
+VOP_WIN_SET(vop, win, enable,
+enabled && (vop->win_enabled & BIT(i)));
+}
+vop_cfg_done(vop);
+
+spin_unlock(>reg_lock);
+}
+
 static void vop_crtc_atomic_disable(struct drm_crtc *crtc,
struct drm_crtc_state *old_state)
 {
@@ -644,15 +665,16 @@ static void vop_crtc_atomic_disable(struct drm_crtc *crtc,
 
WARN_ON(vop->event);
 
-   mutex_lock(>vop_lock);
+   if (crtc->state->self_refresh_active)
+   rockchip_drm_set_win_enabled(crtc, false);
 
-   if (!vop->is_enabled) {
-   mutex_unlock(>vop_lock);
-   return;
-   }
+   mutex_lock(>vop_lock);
 
drm_crtc_vblank_off(crtc);
 
+   if (crtc->state->self_refresh_active)
+   goto out;
+
/*
 * Vop standby will take effect at end of current frame,
 * if dsp hold valid irq happen, it means standby complete.
@@ -683,6 +705,8 @@ static void vop_crtc_atomic_disable(struct drm_crtc *crtc,
clk_disable(vop->dclk);
vop_core_clks_disable(vop);
pm_runtime_put(vop->dev);
+
+out:
mutex_unlock(>vop_lock);
 
if (crtc->state->event && !crtc->state->active) {
@@ -900,6 +924,7 @@ static void vop_plane_atomic_update(struct drm_plane *plane,
}
 
VOP_WIN_SET(vop, win, enable, 1);
+   vop->win_enabled |= BIT(win_index);
spin_unlock(>reg_lock);
 }
 
@@ -1056,6 +1081,12 @@ static void vop_crtc_atomic_enable(struct drm_crtc *crtc,
int dither_bpc = s->output_bpc ? s->output_bpc : 10;
int ret;
 
+   if (old_state && old_state->self_refresh_active) {
+   drm_crtc_vblank_on(crtc);
+   rockchip_drm_set_win_enabled(crtc, true);
+   return;
+   }
+
mutex_lock(>vop_lock);
 
WARN_ON(vop->event);
-- 
Sean Paul, Software Engineer, Google / Chromium OS

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

[PATCH v4 11/11] drm/rockchip: Use drm_atomic_helper_commit_tail_rpm

[Sean Paul]

From: Sean Paul 

Now that we use the drm psr helpers, we no longer need to hand-roll our
atomic_commit_tail implementation. So use the helper

Changes in v2:
- None
Changes in v3:
- None
Changes in v4:
- None

Link to v1: 
https://patchwork.freedesktop.org/patch/msgid/20190228210939.83386-6-s...@poorly.run
Link to v2: 
https://patchwork.freedesktop.org/patch/msgid/20190326204509.96515-5-s...@poorly.run
Link to v3: 
https://patchwork.freedesktop.org/patch/msgid/20190502194956.218441-11-s...@poorly.run

Cc: Zain Wang 
Cc: Tomasz Figa 
Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 21 +
 1 file changed, 1 insertion(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
index 214064d599ee..1c63d9e833bc 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
@@ -117,27 +117,8 @@ rockchip_user_fb_create(struct drm_device *dev, struct 
drm_file *file_priv,
return ERR_PTR(ret);
 }
 
-static void
-rockchip_atomic_helper_commit_tail_rpm(struct drm_atomic_state *old_state)
-{
-   struct drm_device *dev = old_state->dev;
-
-   drm_atomic_helper_commit_modeset_disables(dev, old_state);
-
-   drm_atomic_helper_commit_modeset_enables(dev, old_state);
-
-   drm_atomic_helper_commit_planes(dev, old_state,
-   DRM_PLANE_COMMIT_ACTIVE_ONLY);
-
-   drm_atomic_helper_commit_hw_done(old_state);
-
-   drm_atomic_helper_wait_for_vblanks(dev, old_state);
-
-   drm_atomic_helper_cleanup_planes(dev, old_state);
-}
-
 static const struct drm_mode_config_helper_funcs rockchip_mode_config_helpers 
= {
-   .atomic_commit_tail = rockchip_atomic_helper_commit_tail_rpm,
+   .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
 };
 
 static const struct drm_mode_config_funcs rockchip_drm_mode_config_funcs = {
-- 
Sean Paul, Software Engineer, Google / Chromium OS

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

[PATCH v4 07/11] drm/rockchip: Check for fast link training before enabling psr

[Sean Paul]

From: Sean Paul 

Once we start shutting off the link during PSR, we're going to want fast
training to work. If the display doesn't support fast training, don't
enable psr.

Changes in v2:
- None
Changes in v3:
- None
Changes in v4:
- None

Link to v1: 
https://patchwork.freedesktop.org/patch/msgid/20190228210939.83386-3-s...@poorly.run
Link to v2: 
https://patchwork.freedesktop.org/patch/msgid/20190326204509.96515-2-s...@poorly.run
Link to v3: 
https://patchwork.freedesktop.org/patch/msgid/20190502194956.218441-9-s...@poorly.run

Cc: Zain Wang 
Cc: Tomasz Figa 
Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c 
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 225f5e5dd69b..af34554a5a02 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1040,16 +1040,17 @@ static int analogix_dp_commit(struct analogix_dp_device 
*dp)
if (ret)
return ret;
 
+   /* Check whether panel supports fast training */
+   ret = analogix_dp_fast_link_train_detection(dp);
+   if (ret)
+   dp->psr_enable = false;
+
if (dp->psr_enable) {
ret = analogix_dp_enable_sink_psr(dp);
if (ret)
return ret;
}
 
-   /* Check whether panel supports fast training */
-   ret =  analogix_dp_fast_link_train_detection(dp);
-   if (ret)
-   dp->psr_enable = false;
 
return ret;
 }
-- 
Sean Paul, Software Engineer, Google / Chromium OS

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

[PATCH v4 04/11] drm: Convert connector_helper_funcs->atomic_check to accept drm_atomic_state

[Sean Paul]

From: Sean Paul 

Everyone who implements connector_helper_funcs->atomic_check reaches
into the connector state to get the atomic state. Instead of continuing
this pattern, change the callback signature to just give atomic state
and let the driver determine what it does and does not need from it.

Eventually all atomic functions should do this, but that's just too much
busy work for me.

Changes in v3:
- Added to the set
Changes in v4:
- None

Link to v3: 
https://patchwork.freedesktop.org/patch/msgid/20190502194956.218441-5-s...@poorly.run

Cc: Daniel Vetter 
Cc: Ville Syrjälä 
Cc: Jani Nikula 
Cc: Joonas Lahtinen 
Cc: Rodrigo Vivi 
Cc: Ben Skeggs 
Cc: Laurent Pinchart 
Cc: Kieran Bingham 
Cc: Eric Anholt 
Acked-by: Daniel Vetter 
Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/drm_atomic_helper.c  |  4 ++--
 drivers/gpu/drm/i915/intel_atomic.c  |  8 +---
 drivers/gpu/drm/i915/intel_dp_mst.c  |  7 ---
 drivers/gpu/drm/i915/intel_drv.h |  2 +-
 drivers/gpu/drm/i915/intel_sdvo.c|  9 +
 drivers/gpu/drm/i915/intel_tv.c  |  8 +---
 drivers/gpu/drm/nouveau/dispnv50/disp.c  |  5 +++--
 drivers/gpu/drm/rcar-du/rcar_lvds.c  | 12 +++-
 drivers/gpu/drm/vc4/vc4_txp.c|  7 ---
 include/drm/drm_modeset_helper_vtables.h |  2 +-
 10 files changed, 37 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index e8b7187a8494..ee945d6f1cba 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -683,7 +683,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
}
 
if (funcs->atomic_check)
-   ret = funcs->atomic_check(connector, 
new_connector_state);
+   ret = funcs->atomic_check(connector, state);
if (ret)
return ret;
 
@@ -725,7 +725,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
continue;
 
if (funcs->atomic_check)
-   ret = funcs->atomic_check(connector, 
new_connector_state);
+   ret = funcs->atomic_check(connector, state);
if (ret)
return ret;
}
diff --git a/drivers/gpu/drm/i915/intel_atomic.c 
b/drivers/gpu/drm/i915/intel_atomic.c
index b844e8840c6f..e8a5b82e9242 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -103,12 +103,14 @@ int intel_digital_connector_atomic_set_property(struct 
drm_connector *connector,
 }
 
 int intel_digital_connector_atomic_check(struct drm_connector *conn,
-struct drm_connector_state *new_state)
+struct drm_atomic_state *state)
 {
+   struct drm_connector_state *new_state =
+   drm_atomic_get_new_connector_state(state, conn);
struct intel_digital_connector_state *new_conn_state =
to_intel_digital_connector_state(new_state);
struct drm_connector_state *old_state =
-   drm_atomic_get_old_connector_state(new_state->state, conn);
+   drm_atomic_get_old_connector_state(state, conn);
struct intel_digital_connector_state *old_conn_state =
to_intel_digital_connector_state(old_state);
struct drm_crtc_state *crtc_state;
@@ -118,7 +120,7 @@ int intel_digital_connector_atomic_check(struct 
drm_connector *conn,
if (!new_state->crtc)
return 0;
 
-   crtc_state = drm_atomic_get_new_crtc_state(new_state->state, 
new_state->crtc);
+   crtc_state = drm_atomic_get_new_crtc_state(state, new_state->crtc);
 
/*
 * These properties are handled by fastset, and might not end
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c 
b/drivers/gpu/drm/i915/intel_dp_mst.c
index 19d81cef2ab6..89cfec128ba0 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -143,9 +143,10 @@ static int intel_dp_mst_compute_config(struct 
intel_encoder *encoder,
 
 static int
 intel_dp_mst_atomic_check(struct drm_connector *connector,
- struct drm_connector_state *new_conn_state)
+ struct drm_atomic_state *state)
 {
-   struct drm_atomic_state *state = new_conn_state->state;
+   struct drm_connector_state *new_conn_state =
+   drm_atomic_get_new_connector_state(state, connector);
struct drm_connector_state *old_conn_state =
drm_atomic_get_old_connector_state(state, connector);
struct intel_connector *intel_connector =
@@ -155,7 +156,7 @@ intel_dp_mst_atomic_check(struct drm_connector *connector,
struct drm_dp_mst_topology_mgr *mgr;
int ret;
 
-   ret = intel_digital_connector_atomic_check(connector, new_conn_state);
+   ret = 

[PATCH v4 09/11] drm/rockchip: Use vop_win in vop_win_disable instead of vop_win_data

[Sean Paul]

From: Sean Paul 

Change the argument to vop_win_disable to vop_win to accomodate future
changes to the function.

Changes in v4:
- Added to the patchset

Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index f89d41425be0..15a5b44eb7e7 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -543,8 +543,10 @@ static void vop_core_clks_disable(struct vop *vop)
clk_disable(vop->hclk);
 }
 
-static void vop_win_disable(struct vop *vop, const struct vop_win_data *win)
+static void vop_win_disable(struct vop *vop, const struct vop_win *vop_win)
 {
+   const struct vop_win_data *win = vop_win->data;
+
if (win->phy->scl && win->phy->scl->ext) {
VOP_SCL_SET_EXT(vop, win, yrgb_hor_scl_mode, SCALE_NONE);
VOP_SCL_SET_EXT(vop, win, yrgb_ver_scl_mode, SCALE_NONE);
@@ -603,9 +605,8 @@ static int vop_enable(struct drm_crtc *crtc, struct 
drm_crtc_state *old_state)
if (!old_state || !old_state->self_refresh_active) {
for (i = 0; i < vop->data->win_size; i++) {
struct vop_win *vop_win = >win[i];
-   const struct vop_win_data *win = vop_win->data;
 
-   vop_win_disable(vop, win);
+   vop_win_disable(vop, vop_win);
}
}
spin_unlock(>reg_lock);
@@ -753,7 +754,6 @@ static void vop_plane_atomic_disable(struct drm_plane 
*plane,
 struct drm_plane_state *old_state)
 {
struct vop_win *vop_win = to_vop_win(plane);
-   const struct vop_win_data *win = vop_win->data;
struct vop *vop = to_vop(old_state->crtc);
 
if (!old_state->crtc)
@@ -761,7 +761,7 @@ static void vop_plane_atomic_disable(struct drm_plane 
*plane,
 
spin_lock(>reg_lock);
 
-   vop_win_disable(vop, win);
+   vop_win_disable(vop, vop_win);
 
spin_unlock(>reg_lock);
 }
@@ -1592,7 +1592,6 @@ static void vop_destroy_crtc(struct vop *vop)
 
 static int vop_initial(struct vop *vop)
 {
-   const struct vop_data *vop_data = vop->data;
struct reset_control *ahb_rst;
int i, ret;
 
@@ -1659,12 +1658,13 @@ static int vop_initial(struct vop *vop)
VOP_REG_SET(vop, misc, global_regdone_en, 1);
VOP_REG_SET(vop, common, dsp_blank, 0);
 
-   for (i = 0; i < vop_data->win_size; i++) {
-   const struct vop_win_data *win = _data->win[i];
+   for (i = 0; i < vop->data->win_size; i++) {
+   struct vop_win *vop_win = >win[i];
+   const struct vop_win_data *win = vop_win->data;
int channel = i * 2 + 1;
 
VOP_WIN_SET(vop, win, channel, (channel + 1) << 4 | channel);
-   vop_win_disable(vop, win);
+   vop_win_disable(vop, vop_win);
VOP_WIN_SET(vop, win, gate, 1);
}
 
-- 
Sean Paul, Software Engineer, Google / Chromium OS

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

[PATCH v4 08/11] drm/rockchip: Use the helpers for PSR

[Sean Paul]

From: Sean Paul 

Instead of rolling our own implementation for tracking when PSR should
be [in]active, use the new self refresh helpers to do the heavy lifting.

Changes in v2:
- updated to reflect changes made in the helpers
Changes in v3:
- use the new atomic hooks to inspect crtc state instead of needing conn state 
(Daniel)
Changes in v4:
- Use Laurent's get_new_connector_for_encoder helper (Daniel)
- Exit vop disable early if it's already off

Link to v1: 
https://patchwork.freedesktop.org/patch/msgid/20190228210939.83386-4-s...@poorly.run
Link to v2: 
https://patchwork.freedesktop.org/patch/msgid/20190326204509.96515-3-s...@poorly.run
Link to v3: 
https://patchwork.freedesktop.org/patch/msgid/20190502194956.218441-9-s...@poorly.run

Cc: Zain Wang 
Cc: Tomasz Figa 
Cc: Daniel Vetter 
Cc: Laurent Pinchart 
Signed-off-by: Sean Paul 
---
 .../drm/bridge/analogix/analogix_dp_core.c| 291 +-
 .../drm/bridge/analogix/analogix_dp_core.h|   2 +-
 drivers/gpu/drm/rockchip/Makefile |   3 +-
 .../gpu/drm/rockchip/analogix_dp-rockchip.c   | 106 ---
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c|   5 -
 drivers/gpu/drm/rockchip/rockchip_drm_psr.c   | 290 -
 drivers/gpu/drm/rockchip/rockchip_drm_psr.h   |  30 --
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |  39 ++-
 include/drm/bridge/analogix_dp.h  |   4 -
 9 files changed, 295 insertions(+), 475 deletions(-)
 delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_psr.c
 delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_psr.h

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c 
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index af34554a5a02..1c1472ed2856 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -24,6 +24,7 @@
 #include 
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -106,63 +107,7 @@ static int analogix_dp_detect_hpd(struct 
analogix_dp_device *dp)
return 0;
 }
 
-int analogix_dp_psr_enabled(struct analogix_dp_device *dp)
-{
-
-   return dp->psr_enable;
-}
-EXPORT_SYMBOL_GPL(analogix_dp_psr_enabled);
-
-int analogix_dp_enable_psr(struct analogix_dp_device *dp)
-{
-   struct edp_vsc_psr psr_vsc;
-
-   if (!dp->psr_enable)
-   return 0;
-
-   /* Prepare VSC packet as per EDP 1.4 spec, Table 6.9 */
-   memset(_vsc, 0, sizeof(psr_vsc));
-   psr_vsc.sdp_header.HB0 = 0;
-   psr_vsc.sdp_header.HB1 = 0x7;
-   psr_vsc.sdp_header.HB2 = 0x2;
-   psr_vsc.sdp_header.HB3 = 0x8;
-
-   psr_vsc.DB0 = 0;
-   psr_vsc.DB1 = EDP_VSC_PSR_STATE_ACTIVE | EDP_VSC_PSR_CRC_VALUES_VALID;
-
-   return analogix_dp_send_psr_spd(dp, _vsc, true);
-}
-EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
-
-int analogix_dp_disable_psr(struct analogix_dp_device *dp)
-{
-   struct edp_vsc_psr psr_vsc;
-   int ret;
-
-   if (!dp->psr_enable)
-   return 0;
-
-   /* Prepare VSC packet as per EDP 1.4 spec, Table 6.9 */
-   memset(_vsc, 0, sizeof(psr_vsc));
-   psr_vsc.sdp_header.HB0 = 0;
-   psr_vsc.sdp_header.HB1 = 0x7;
-   psr_vsc.sdp_header.HB2 = 0x2;
-   psr_vsc.sdp_header.HB3 = 0x8;
-
-   psr_vsc.DB0 = 0;
-   psr_vsc.DB1 = 0;
-
-   ret = drm_dp_dpcd_writeb(>aux, DP_SET_POWER, DP_SET_POWER_D0);
-   if (ret != 1) {
-   dev_err(dp->dev, "Failed to set DP Power0 %d\n", ret);
-   return ret;
-   }
-
-   return analogix_dp_send_psr_spd(dp, _vsc, false);
-}
-EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
-
-static int analogix_dp_detect_sink_psr(struct analogix_dp_device *dp)
+static bool analogix_dp_detect_sink_psr(struct analogix_dp_device *dp)
 {
unsigned char psr_version;
int ret;
@@ -170,14 +115,11 @@ static int analogix_dp_detect_sink_psr(struct 
analogix_dp_device *dp)
ret = drm_dp_dpcd_readb(>aux, DP_PSR_SUPPORT, _version);
if (ret != 1) {
dev_err(dp->dev, "failed to get PSR version, disable it\n");
-   return ret;
+   return false;
}
 
dev_dbg(dp->dev, "Panel PSR version : %x\n", psr_version);
-
-   dp->psr_enable = (psr_version & DP_PSR_IS_SUPPORTED) ? true : false;
-
-   return 0;
+   return psr_version & DP_PSR_IS_SUPPORTED;
 }
 
 static int analogix_dp_enable_sink_psr(struct analogix_dp_device *dp)
@@ -200,7 +142,7 @@ static int analogix_dp_enable_sink_psr(struct 
analogix_dp_device *dp)
}
 
/* Main-Link transmitter remains active during PSR active states */
-   psr_en = DP_PSR_MAIN_LINK_ACTIVE | DP_PSR_CRC_VERIFICATION;
+   psr_en = DP_PSR_CRC_VERIFICATION;
ret = drm_dp_dpcd_writeb(>aux, DP_PSR_EN_CFG, psr_en);
if (ret != 1) {
dev_err(dp->dev, "failed to set panel psr\n");
@@ -208,8 +150,7 @@ static int analogix_dp_enable_sink_psr(struct 
analogix_dp_device *dp)
}
 
/* Enable psr 

[PATCH v4 03/11] drm: Add atomic variants for bridge enable/disable

[Sean Paul]

From: Sean Paul 

This patch adds atomic variants for all of
pre_enable/enable/disable/post_disable bridge functions. These will be
called from the appropriate atomic helper functions. If the bridge
driver doesn't implement the atomic version of the function, we will
fall back to the vanilla implementation.

Note that some drivers call drm_bridge_disable directly, and these cases
are not covered. It's up to the driver to decide whether to implement
both atomic_disable and disable, or if it's not necessary.

Changes in v3:
- Added to the patchset
Changes in v4:
- Fix up docbook references (Daniel)

Link to v3: 
https://patchwork.freedesktop.org/patch/msgid/20190502194956.218441-4-s...@poorly.run

Cc: Daniel Vetter 
Cc: Ville Syrjälä 
Reviewed-by: Daniel Vetter 
Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/drm_atomic_helper.c |   8 +-
 drivers/gpu/drm/drm_bridge.c| 110 
 include/drm/drm_bridge.h| 106 +++
 3 files changed, 220 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index ccf01831f265..e8b7187a8494 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -995,7 +995,7 @@ disable_outputs(struct drm_device *dev, struct 
drm_atomic_state *old_state)
 * Each encoder has at most one connector (since we always steal
 * it away), so we won't call disable hooks twice.
 */
-   drm_bridge_disable(encoder->bridge);
+   drm_atomic_bridge_disable(encoder->bridge, old_state);
 
/* Right function depends upon target state. */
if (funcs) {
@@ -1009,7 +1009,7 @@ disable_outputs(struct drm_device *dev, struct 
drm_atomic_state *old_state)
funcs->dpms(encoder, DRM_MODE_DPMS_OFF);
}
 
-   drm_bridge_post_disable(encoder->bridge);
+   drm_atomic_bridge_post_disable(encoder->bridge, old_state);
}
 
for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, 
new_crtc_state, i) {
@@ -1308,7 +1308,7 @@ void drm_atomic_helper_commit_modeset_enables(struct 
drm_device *dev,
 * Each encoder has at most one connector (since we always steal
 * it away), so we won't call enable hooks twice.
 */
-   drm_bridge_pre_enable(encoder->bridge);
+   drm_atomic_bridge_pre_enable(encoder->bridge, old_state);
 
if (funcs) {
if (funcs->atomic_enable)
@@ -1319,7 +1319,7 @@ void drm_atomic_helper_commit_modeset_enables(struct 
drm_device *dev,
funcs->commit(encoder);
}
 
-   drm_bridge_enable(encoder->bridge);
+   drm_atomic_bridge_enable(encoder->bridge, old_state);
}
 
drm_atomic_helper_commit_writebacks(dev, old_state);
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 138b2711d389..cba537c99e43 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -352,6 +352,116 @@ void drm_bridge_enable(struct drm_bridge *bridge)
 }
 EXPORT_SYMBOL(drm_bridge_enable);
 
+/**
+ * drm_atomic_bridge_disable - disables all bridges in the encoder chain
+ * @bridge: bridge control structure
+ * @state: atomic state being committed
+ *
+ * Calls _bridge_funcs.atomic_disable (falls back on
+ * _bridge_funcs.disable) op for all the bridges in the encoder chain,
+ * starting from the last bridge to the first. These are called before calling
+ * _encoder_helper_funcs.atomic_disable
+ *
+ * Note: the bridge passed should be the one closest to the encoder
+ */
+void drm_atomic_bridge_disable(struct drm_bridge *bridge,
+  struct drm_atomic_state *state)
+{
+   if (!bridge)
+   return;
+
+   drm_atomic_bridge_disable(bridge->next, state);
+
+   if (bridge->funcs->atomic_disable)
+   bridge->funcs->atomic_disable(bridge, state);
+   else if (bridge->funcs->disable)
+   bridge->funcs->disable(bridge);
+}
+EXPORT_SYMBOL(drm_atomic_bridge_disable);
+
+/**
+ * drm_atomic_bridge_post_disable - cleans up after disabling all bridges in 
the
+ * encoder chain
+ * @bridge: bridge control structure
+ * @state: atomic state being committed
+ *
+ * Calls _bridge_funcs.atomic_post_disable (falls back on
+ * _bridge_funcs.post_disable) op for all the bridges in the encoder chain,
+ * starting from the first bridge to the last. These are called after 
completing
+ * _encoder_helper_funcs.atomic_disable
+ *
+ * Note: the bridge passed should be the one closest to the encoder
+ */
+void drm_atomic_bridge_post_disable(struct drm_bridge *bridge,
+   struct drm_atomic_state *state)
+{
+   if (!bridge)
+   return;

[PATCH v4 02/11] drm: Add drm_atomic_get_(old|new-_connector_for_encoder() helpers

[Sean Paul]

From: Laurent Pinchart 

Add functions to the atomic core to retrieve the old and new connectors
associated with an encoder in a drm_atomic_state. This is useful for
encoders and bridges that need to access the connector, for instance for
the drm_display_info.

The CRTC associated with the encoder can also be retrieved through the
connector state, and from it, the old and new CRTC states.

Changed in v4:
- Added to the set

Cc: Daniel Vetter 
Signed-off-by: Laurent Pinchart 

[seanpaul removed WARNs from helpers and added docs to explain why
returning NULL might be valid]
Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/drm_atomic.c | 70 
 include/drm/drm_atomic.h |  7 
 2 files changed, 77 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 5eb40130fafb..936002495523 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -797,6 +797,76 @@ drm_atomic_get_private_obj_state(struct drm_atomic_state 
*state,
 }
 EXPORT_SYMBOL(drm_atomic_get_private_obj_state);
 
+/**
+ * drm_atomic_get_old_connector_for_encoder - Get old connector for an encoder
+ * @state: Atomic state
+ * @encoder: The encoder to fetch the connector state for
+ *
+ * This function finds and returns the connector that was connected to @encoder
+ * as specified by the @state.
+ *
+ * If there is no connector in @state which previously had @encoder connected 
to
+ * it, this function will return NULL. While this may seem like an invalid use
+ * case, it is sometimes a useful to differentiate commits which had no prior
+ * connectors attached to @encoder vs ones that did (and to inspect their
+ * @state). This is especially true in enable hooks because the pipeline has
+ * changed/will change.
+ *
+ * Returns: The old connector connected to @encoder, or NULL if the encoder is
+ * not connected.
+ */
+struct drm_connector *
+drm_atomic_get_old_connector_for_encoder(struct drm_atomic_state *state,
+struct drm_encoder *encoder)
+{
+   struct drm_connector_state *conn_state;
+   struct drm_connector *connector;
+   unsigned int i;
+
+   for_each_old_connector_in_state(state, connector, conn_state, i) {
+   if (conn_state->best_encoder == encoder)
+   return connector;
+   }
+
+   return NULL;
+}
+EXPORT_SYMBOL(drm_atomic_get_old_connector_for_encoder);
+
+/**
+ * drm_atomic_get_new_connector_for_encoder - Get new connector for an encoder
+ * @state: Atomic state
+ * @encoder: The encoder to fetch the connector state for
+ *
+ * This function finds and returns the connector that will be connected to
+ * @encoder as specified by the @state.
+ *
+ * If there is no connector in @state which will have @encoder connected to it,
+ * this function will return NULL. While this may seem like an invalid use 
case,
+ * it is sometimes a useful to differentiate commits which have no connectors
+ * attached to @encoder vs ones that do (and to inspect their state). This is
+ * especially true in disable hooks because the pipeline has changed/will
+ * change.
+ *
+ * Returns: The new connector connected to @encoder, or NULL if the encoder is
+ * not connected.
+ */
+struct drm_connector *
+drm_atomic_get_new_connector_for_encoder(struct drm_atomic_state *state,
+struct drm_encoder *encoder)
+{
+   struct drm_connector_state *conn_state;
+   struct drm_connector *connector;
+   unsigned int i;
+
+   for_each_new_connector_in_state(state, connector, conn_state, i) {
+   if (conn_state->best_encoder == encoder)
+   return connector;
+   }
+
+   return NULL;
+}
+EXPORT_SYMBOL(drm_atomic_get_new_connector_for_encoder);
+
 /**
  * drm_atomic_get_connector_state - get connector state
  * @state: global atomic state object
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index 824a5ed4e216..d6b3acd34e1c 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -453,6 +453,13 @@ struct drm_private_state * __must_check
 drm_atomic_get_private_obj_state(struct drm_atomic_state *state,
 struct drm_private_obj *obj);
 
+struct drm_connector *
+drm_atomic_get_old_connector_for_encoder(struct drm_atomic_state *state,
+struct drm_encoder *encoder);
+struct drm_connector *
+drm_atomic_get_new_connector_for_encoder(struct drm_atomic_state *state,
+struct drm_encoder *encoder);
+
 /**
  * drm_atomic_get_existing_crtc_state - get crtc state, if it exists
  * @state: global atomic state object
-- 
Sean Paul, Software Engineer, Google / Chromium OS

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

[PATCH v4 00/12] drm: Add self refresh helpers

[Sean Paul]

From: Sean Paul 

Another version of the SR helpers for your consumption.

Pretty minor differences between v4 and v3:
- lots of documentation changes
- Use connector to get at crtc state in encoders
- Use the damage helpers in rockchip to fix fbdev

Note that the rockchip patches require
e9abc611a941d4051cde1d94b2ab7473fdb50102 which is making its way through
the -fixes branches.

PTAL

Laurent Pinchart (1):
  drm: Add drm_atomic_get_(old|new-_connector_for_encoder() helpers

Sean Paul (10):
  drm: Add atomic variants of enable/disable to encoder helper funcs
  drm: Add atomic variants for bridge enable/disable
  drm: Convert connector_helper_funcs->atomic_check to accept
drm_atomic_state
  drm: Add helpers to kick off self refresh mode in drivers
  drm/rockchip: Use dirtyfb helper
  drm/rockchip: Check for fast link training before enabling psr
  drm/rockchip: Use the helpers for PSR
  drm/rockchip: Use vop_win in vop_win_disable instead of vop_win_data
  drm/rockchip: Don't fully disable vop on self refresh
  drm/rockchip: Use drm_atomic_helper_commit_tail_rpm

 Documentation/gpu/drm-kms-helpers.rst |   9 +
 drivers/gpu/drm/Makefile  |   2 +-
 .../drm/bridge/analogix/analogix_dp_core.c| 292 +-
 .../drm/bridge/analogix/analogix_dp_core.h|   2 +-
 drivers/gpu/drm/drm_atomic.c  |  72 +
 drivers/gpu/drm/drm_atomic_helper.c   |  55 +++-
 drivers/gpu/drm/drm_atomic_state_helper.c |   4 +
 drivers/gpu/drm/drm_atomic_uapi.c |   7 +-
 drivers/gpu/drm/drm_bridge.c  | 110 +++
 drivers/gpu/drm/drm_self_refresh_helper.c | 213 +
 drivers/gpu/drm/i915/intel_atomic.c   |   8 +-
 drivers/gpu/drm/i915/intel_dp_mst.c   |   7 +-
 drivers/gpu/drm/i915/intel_drv.h  |   2 +-
 drivers/gpu/drm/i915/intel_sdvo.c |   9 +-
 drivers/gpu/drm/i915/intel_tv.c   |   8 +-
 drivers/gpu/drm/nouveau/dispnv50/disp.c   |   5 +-
 drivers/gpu/drm/rcar-du/rcar_lvds.c   |  12 +-
 drivers/gpu/drm/rockchip/Makefile |   3 +-
 .../gpu/drm/rockchip/analogix_dp-rockchip.c   | 106 ---
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c|  39 +--
 drivers/gpu/drm/rockchip/rockchip_drm_psr.c   | 290 -
 drivers/gpu/drm/rockchip/rockchip_drm_psr.h   |  30 --
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |  84 -
 drivers/gpu/drm/vc4/vc4_txp.c |   7 +-
 include/drm/bridge/analogix_dp.h  |   4 -
 include/drm/drm_atomic.h  |  22 ++
 include/drm/drm_bridge.h  | 106 +++
 include/drm/drm_connector.h   |  14 +
 include/drm/drm_crtc.h|  19 ++
 include/drm/drm_modeset_helper_vtables.h  |  50 ++-
 include/drm/drm_self_refresh_helper.h |  22 ++
 31 files changed, 1062 insertions(+), 551 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_self_refresh_helper.c
 delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_psr.c
 delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_psr.h
 create mode 100644 include/drm/drm_self_refresh_helper.h

-- 
Sean Paul, Software Engineer, Google / Chromium OS

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

[PATCH v4 01/11] drm: Add atomic variants of enable/disable to encoder helper funcs

[Sean Paul]

From: Sean Paul 

This patch adds atomic_enable and atomic_disable callbacks to the
encoder helpers. This will allow encoders to make informed decisions in
their start-up/shutdown based on the committed state.

Aside from the new hooks, this patch also introduces the new signature
for .atomic_* functions going forward. Instead of passing object state
(well, encoders don't have atomic state, but let's ignore that), we pass
the entire atomic state so the driver can inspect more than what's
happening locally.

This is particularly important for the upcoming self refresh helpers.

Changes in v3:
- Added patch to the set
Changes in v4:
- Move atomic_disable above prepare (Daniel)
- Add breadcrumb to .enable() docbook (Daniel)

Link to v3: 
https://patchwork.freedesktop.org/patch/msgid/20190502194956.218441-2-s...@poorly.run

Cc: Daniel Vetter 
Cc: Ville Syrjälä 
Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/drm_atomic_helper.c  |  8 +++-
 include/drm/drm_modeset_helper_vtables.h | 48 
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 553415fe8ede..ccf01831f265 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -999,7 +999,9 @@ disable_outputs(struct drm_device *dev, struct 
drm_atomic_state *old_state)
 
/* Right function depends upon target state. */
if (funcs) {
-   if (new_conn_state->crtc && funcs->prepare)
+   if (funcs->atomic_disable)
+   funcs->atomic_disable(encoder, old_state);
+   else if (new_conn_state->crtc && funcs->prepare)
funcs->prepare(encoder);
else if (funcs->disable)
funcs->disable(encoder);
@@ -1309,7 +1311,9 @@ void drm_atomic_helper_commit_modeset_enables(struct 
drm_device *dev,
drm_bridge_pre_enable(encoder->bridge);
 
if (funcs) {
-   if (funcs->enable)
+   if (funcs->atomic_enable)
+   funcs->atomic_enable(encoder, old_state);
+   else if (funcs->enable)
funcs->enable(encoder);
else if (funcs->commit)
funcs->commit(encoder);
diff --git a/include/drm/drm_modeset_helper_vtables.h 
b/include/drm/drm_modeset_helper_vtables.h
index 8f3602811eb5..aa509c107083 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -675,6 +675,51 @@ struct drm_encoder_helper_funcs {
enum drm_connector_status (*detect)(struct drm_encoder *encoder,
struct drm_connector *connector);
 
+   /**
+* @atomic_disable:
+*
+* This callback should be used to disable the encoder. With the atomic
+* drivers it is called before this encoder's CRTC has been shut off
+* using their own _crtc_helper_funcs.atomic_disable hook. If that
+* sequence is too simple drivers can just add their own driver private
+* encoder hooks and call them from CRTC's callback by looping over all
+* encoders connected to it using for_each_encoder_on_crtc().
+*
+* This callback is a variant of @disable that provides the atomic state
+* to the driver. It takes priority over @disable during atomic commits.
+*
+* This hook is used only by atomic helpers. Atomic drivers don't need
+* to implement it if there's no need to disable anything at the encoder
+* level. To ensure that runtime PM handling (using either DPMS or the
+* new "ACTIVE" property) works @atomic_disable must be the inverse of
+* @atomic_enable.
+*/
+   void (*atomic_disable)(struct drm_encoder *encoder,
+  struct drm_atomic_state *state);
+
+   /**
+* @atomic_enable:
+*
+* This callback should be used to enable the encoder. It is called
+* after this encoder's CRTC has been enabled using their own
+* _crtc_helper_funcs.atomic_enable hook. If that sequence is
+* too simple drivers can just add their own driver private encoder
+* hooks and call them from CRTC's callback by looping over all encoders
+* connected to it using for_each_encoder_on_crtc().
+*
+* This callback is a variant of @enable that provides the atomic state
+* to the driver. It is called in place of @enable during atomic
+* commits.
+*
+* This hook is used only by atomic helpers, for symmetry with @disable.
+* Atomic drivers don't need to implement it if there's no need to
+* enable anything at the encoder level. To ensure that runtime PM
+   

[Bug 110472] Graphical Fault (Desktop Freeze) on Specific OpenGL Application

[bugzilla-daemon]

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

--- Comment #2 from gpiza...@javaman.net ---
I really hope someone sees this. Bump x2?

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

[Bug 110641] lm_sensors reports "ERROR: Can't get value of subfeature in0_input: Can't read"

[bugzilla-daemon]

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

--- Comment #1 from Alex Deucher  ---
That commit just exposes the temperature via an ioctl interface.  It doesn't
affect the hwmon sysfs API.  How did you determine that that was the
problematic one?

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

Re: [PATCH] drm/msm/a6xx: No zap shader is not an error

[Jordan Crouse]

On Wed, May 08, 2019 at 06:06:52AM -0700, Rob Clark wrote:
> From: Rob Clark 
> 
> Depending on platform firmware, a zap shader may not be required to take
> the GPU out of secure mode on boot, in which case we can just write
> RBBM_SECVID_TRUST_CNTL directly.  Which we *mostly* handled, but missed
> clearing 'ret' resulting that hw_init() returned an error on these
> devices.
> 
> Fixes: abccb9fe3267 drm/msm/a6xx: Add zap shader load
> Signed-off-by: Rob Clark 

Woo, I'm glad we finally got a chance to verify this on both types of systems.

Acked-by: Jordan Crouse 

> ---
>  drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
> b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index ec24508b9d68..e74dce474250 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -527,6 +527,7 @@ static int a6xx_hw_init(struct msm_gpu *gpu)
>   dev_warn_once(gpu->dev->dev,
>   "Zap shader not enabled - using SECVID_TRUST_CNTL 
> instead\n");
>   gpu_write(gpu, REG_A6XX_RBBM_SECVID_TRUST_CNTL, 0x0);
> + ret = 0;
>   }
>  
>  out:
> -- 
> 2.20.1
> 

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: 2019 Xorg Election Results

[Luc Verhaegen]

On Wed, May 08, 2019 at 02:06:37PM +, Harry Wentland wrote:
>
> Due to the lack of 
> controversy about the candidates [...]

Such a statement, when talking about election results, very much sounds 
like "the election committees favourite candidates won anyway, so..."

p.

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

Re: 2019 Xorg Election Results

[Luc Verhaegen]

On Wed, May 08, 2019 at 02:06:37PM +, Harry Wentland wrote:
> 
> In the interest of transparency I should mention that one person 
> accidentally signed up twice and voted twice. Luckily this doesn't 
> change the results of the election since there is more than a 6-point 
> gap between 4th and 5th place. We'll take this as a reminder to have a 
> better look at membership sign-ups.

From private conversation with Stefan, i learned that the second 
membership application was approved between the first round of voting 
and the second round of voting.

How many members were added between the two rounds?

Is the vote on the fd.o merger counted against the first set of members, 
or against the second set of members.

Does that not raise questions about the validity of the fd.o bylaws 
change?

Imho, these are 3 significant irregularities with these elections and 
the subsequent results.

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

Re: 2019 Xorg Election Results

[Luc Verhaegen]

On Wed, May 08, 2019 at 02:06:37PM +, Harry Wentland wrote:
> 
> It should also be noted that even though our election process as 
> outlined at https://www.x.org/wiki/BoardOfDirectors/Elections/ allows 
> denying candidates any points our website didn't take this into account 
> and forced voters to rank every candidate. Due to the lack of 
> controversy about the candidates we don't expect this to have had any 
> impact on the final results.

Does this change not massively skew results?

Lack of controversy has absolutely nothing to do with it.

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

Re: 2019 Xorg Election Results

[Trevor Woerner]

On Wed, May 8, 2019 at 10:06 AM Harry Wentland  wrote:

>Trevor Woerner 4  14  10  10   8  19   199
>

I'd like to truly thank the other 3 people who chose me as their 1st pick,
and the 14 (:-O !!) who chose me as their first 2nd-place pick!
Considering I'm not an active contributor of code to this project, I think
this is an amazing result! Thank you :-D

Although I didn't make it on the board, I remain committed to running GSoC
and EVoC.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/msm/a6xx: No zap shader is not an error

[Sean Paul]

On Wed, May 08, 2019 at 06:06:52AM -0700, Rob Clark wrote:
> From: Rob Clark 
> 
> Depending on platform firmware, a zap shader may not be required to take
> the GPU out of secure mode on boot, in which case we can just write
> RBBM_SECVID_TRUST_CNTL directly.  Which we *mostly* handled, but missed
> clearing 'ret' resulting that hw_init() returned an error on these
> devices.
> 
> Fixes: abccb9fe3267 drm/msm/a6xx: Add zap shader load
> Signed-off-by: Rob Clark 

Reviewed-by: Sean Paul 

> ---
>  drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
> b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index ec24508b9d68..e74dce474250 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -527,6 +527,7 @@ static int a6xx_hw_init(struct msm_gpu *gpu)
>   dev_warn_once(gpu->dev->dev,
>   "Zap shader not enabled - using SECVID_TRUST_CNTL 
> instead\n");
>   gpu_write(gpu, REG_A6XX_RBBM_SECVID_TRUST_CNTL, 0x0);
> + ret = 0;
>   }
>  
>  out:
> -- 
> 2.20.1
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 203525] brightness with function buttons Acer Aspire A315-41 notebook with AMD Ryzen 5 2500U

[bugzilla-daemon]

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

--- Comment #7 from Erik (erikjohans...@flashbox.5july.org) ---
Is there anything more i can do to find out what's going on?

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

Re: [PATCH] drm/cma-helper: Fix drm_gem_cma_free_object()

[Noralf Trønnes]

Den 08.05.2019 08.33, skrev Oleksandr Andrushchenko:
> On 5/7/19 7:04 PM, Noralf Trønnes wrote:
>> Hi,
>>
>> Could someone please have a look at this one?
>>
>> Noralf.
>>
>> Den 26.04.2019 14.47, skrev Noralf Trønnes:
>>> The logic for freeing an imported buffer with a virtual address is
>>> broken. It will free the buffer instead of unmapping the dma buf.
>>> Fix by reversing the if ladder and first check if the buffer is
>>> imported.
>>>
>>> Fixes: b9068cde51ee ("drm/cma-helper: Add DRM_GEM_CMA_VMAP_DRIVER_OPS")
>>> Cc: sta...@vger.kernel.org
>>> Reported-by: "Li, Tingqian" 
>>> Signed-off-by: Noralf Trønnes 
> Reviewed-by: Oleksandr Andrushchenko 

Thanks Oleksandr, applied to drm-misc-next-fixes.

Noralf.

>>> ---
>>>
>>>   drivers/gpu/drm/drm_gem_cma_helper.c | 8 
>>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c
>>> b/drivers/gpu/drm/drm_gem_cma_helper.c
>>> index cc26625b4b33..e01ceed09e67 100644
>>> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
>>> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
>>> @@ -186,13 +186,13 @@ void drm_gem_cma_free_object(struct
>>> drm_gem_object *gem_obj)
>>>     cma_obj = to_drm_gem_cma_obj(gem_obj);
>>>   -    if (cma_obj->vaddr) {
>>> -    dma_free_wc(gem_obj->dev->dev, cma_obj->base.size,
>>> -    cma_obj->vaddr, cma_obj->paddr);
>>> -    } else if (gem_obj->import_attach) {
>>> +    if (gem_obj->import_attach) {
>>>   if (cma_obj->vaddr)
>>>   dma_buf_vunmap(gem_obj->import_attach->dmabuf,
>>> cma_obj->vaddr);
>>>   drm_prime_gem_destroy(gem_obj, cma_obj->sgt);
>>> +    } else if (cma_obj->vaddr) {
>>> +    dma_free_wc(gem_obj->dev->dev, cma_obj->base.size,
>>> +    cma_obj->vaddr, cma_obj->paddr);
>>>   }
>>>     drm_gem_object_release(gem_obj);
>>>
>> ___
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

2019 Xorg Election Results

[Harry Wentland]

Total membership 84; total votes 65; which makes for a 77.4% voter 
turnout. Here are the results:

Board Members:
   Name   1   2   3   4   5   6  Score
   Daniel Vetter 27  10  14   6   2   6   296
   Samuel Iglesias Gonsálvez 11  10  13  15  10   6   239
   Lyude Paul10  12  13  12  12   6   238
   Manasi Navare  8  13   7  16  18   3   228
   Trevor Woerner 4  14  10  10   8  19   199
   Arkadiusz Hiler5   6   8   6  15  25   165

So that means our new board members are:

   Daniel Vetter
   Samuel Iglesias Gonsálvez
   Lyude Paul
   Manasi Navare

Please welcome our new members to the board!

In the interest of transparency I should mention that one person 
accidentally signed up twice and voted twice. Luckily this doesn't 
change the results of the election since there is more than a 6-point 
gap between 4th and 5th place. We'll take this as a reminder to have a 
better look at membership sign-ups.

It should also be noted that even though our election process as 
outlined at https://www.x.org/wiki/BoardOfDirectors/Elections/ allows 
denying candidates any points our website didn't take this into account 
and forced voters to rank every candidate. Due to the lack of 
controversy about the candidates we don't expect this to have had any 
impact on the final results.

Thanks,
Harry Wentland,
On behalf of the Xorg Elections Committee   
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 110645] Blender EEVEE World Volumetric flickering

[bugzilla-daemon]

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

Michel Dänzer  changed:

   What|Removed |Added

 QA Contact|mesa-dev@lists.freedesktop. |dri-devel@lists.freedesktop
   |org |.org
   Assignee|mesa-dev@lists.freedesktop. |dri-devel@lists.freedesktop
   |org |.org
  Component|Mesa core   |Drivers/Gallium/radeonsi

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

Re: [PATCH 03/16] lib,treewide: add new match_string() helper/macro

[Greg KH]

On Wed, May 08, 2019 at 04:11:28PM +0300, Andy Shevchenko wrote:
> On Wed, May 08, 2019 at 02:28:29PM +0300, Alexandru Ardelean wrote:
> > This change re-introduces `match_string()` as a macro that uses
> > ARRAY_SIZE() to compute the size of the array.
> > The macro is added in all the places that do
> > `match_string(_a, ARRAY_SIZE(_a), s)`, since the change is pretty
> > straightforward.
> 
> Can you split include/linux/ change from the rest?

That would break the build, why do you want it split out?  This makes
sense all as a single patch to me.

thanks,

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

[PATCH] drm/msm/a6xx: No zap shader is not an error

[Rob Clark]

From: Rob Clark 

Depending on platform firmware, a zap shader may not be required to take
the GPU out of secure mode on boot, in which case we can just write
RBBM_SECVID_TRUST_CNTL directly.  Which we *mostly* handled, but missed
clearing 'ret' resulting that hw_init() returned an error on these
devices.

Fixes: abccb9fe3267 drm/msm/a6xx: Add zap shader load
Signed-off-by: Rob Clark 
---
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index ec24508b9d68..e74dce474250 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -527,6 +527,7 @@ static int a6xx_hw_init(struct msm_gpu *gpu)
dev_warn_once(gpu->dev->dev,
"Zap shader not enabled - using SECVID_TRUST_CNTL 
instead\n");
gpu_write(gpu, REG_A6XX_RBBM_SECVID_TRUST_CNTL, 0x0);
+   ret = 0;
}
 
 out:
-- 
2.20.1

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

Re: [PATCH 1/2] drm/i915: Seal races between async GPU cancellation, retirement and signaling

[Daniel Vetter]

On Wed, May 8, 2019 at 2:06 PM Chris Wilson  wrote:
>
> Currently there is an underlying assumption that i915_request_unsubmit()
> is synchronous wrt the GPU -- that is the request is no longer in flight
> as we remove it. In the near future that may change, and this may upset
> our signaling as we can process an interrupt for that request while it
> is no longer in flight.
>
> CPU0CPU1
> intel_engine_breadcrumbs_irq
> (queue request completion)
> i915_request_cancel_signaling
> ... ...
> i915_request_enable_signaling
> dma_fence_signal
>
> Hence in the time it took us to drop the lock to signal the request, a
> preemption event may have occurred and re-queued the request. In the
> process, that request would have seen I915_FENCE_FLAG_SIGNAL clear and
> so reused the rq->signal_link that was in use on CPU0, leading to bad
> pointer chasing in intel_engine_breadcrumbs_irq.
>
> A related issue was that if someone started listening for a signal on a
> completed but no longer in-flight request, we missed the opportunity to
> immediately signal that request.
>
> Furthermore, as intel_contexts may be immediately released during
> request retirement, in order to be entirely sure that
> intel_engine_breadcrumbs_irq may no longer dereference the intel_context
> (ce->signals and ce->signal_link), we must wait for irq spinlock.
>
> In order to prevent the race, we use a bit in the fence.flags to signal
> the transfer onto the signal list inside intel_engine_breadcrumbs_irq.
> For simplicity, we use the DMA_FENCE_FLAG_SIGNALED_BIT as it then
> quickly signals to any outside observer that the fence is indeed signaled.
>
> v2: Sketch out potential dma-fence API for manual signaling
> v3: And the test_and_set_bit()
>
> Fixes: 52c0fdb25c7c ("drm/i915: Replace global breadcrumbs with per-context 
> interrupt tracking")
> Signed-off-by: Chris Wilson 
> Cc: Tvrtko Ursulin 
> Reviewed-by: Tvrtko Ursulin 

Ok chatted a bit with Chris on irc, and we already allow drivers to
pass whatever spinlock is suitable for their request/fence
handling/retiring, so allowing to split up this all makes sense I
think. Has my ack on the approach.

I think it'd be good to spell out the optimization this allows
(synchronous cleanup instead of refcounting all. the. things.) plus
showcase the link between the fence->lock pointer and the split-up
__dma_signal functions in the kerneldoc. Definitely for the core
patch.

Also not sure why __notify and __timestamp has a double underscore in
the middle. That color choice confuses me a bit :-)

Cheers, Daniel

> ---
>  drivers/dma-buf/dma-fence.c |  1 +
>  drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 78 +++--
>  drivers/gpu/drm/i915/i915_request.c |  1 +
>  drivers/gpu/drm/i915/intel_guc_submission.c |  1 -
>  4 files changed, 59 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index 3aa8733f832a..9bf06042619a 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -29,6 +29,7 @@
>
>  EXPORT_TRACEPOINT_SYMBOL(dma_fence_emit);
>  EXPORT_TRACEPOINT_SYMBOL(dma_fence_enable_signal);
> +EXPORT_TRACEPOINT_SYMBOL(dma_fence_signaled);
>
>  static DEFINE_SPINLOCK(dma_fence_stub_lock);
>  static struct dma_fence dma_fence_stub;
> diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c 
> b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
> index fe455f01aa65..c092bdf5f0bf 100644
> --- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
> @@ -23,6 +23,7 @@
>   */
>
>  #include 
> +#include 
>  #include 
>
>  #include "i915_drv.h"
> @@ -96,9 +97,39 @@ check_signal_order(struct intel_context *ce, struct 
> i915_request *rq)
> return true;
>  }
>
> +static bool
> +__dma_fence_signal(struct dma_fence *fence)
> +{
> +   return !test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags);
> +}
> +
> +static void
> +__dma_fence_signal__timestamp(struct dma_fence *fence, ktime_t timestamp)
> +{
> +   fence->timestamp = timestamp;
> +   set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags);
> +   trace_dma_fence_signaled(fence);
> +}
> +
> +static void
> +__dma_fence_signal__notify(struct dma_fence *fence)
> +{
> +   struct dma_fence_cb *cur, *tmp;
> +
> +   lockdep_assert_held(fence->lock);
> +   lockdep_assert_irqs_disabled();
> +
> +   list_for_each_entry_safe(cur, tmp, >cb_list, node) {
> +   INIT_LIST_HEAD(>node);
> +   cur->func(fence, cur);
> +   }
> +   INIT_LIST_HEAD(>cb_list);
> +}
> +
>  void intel_engine_breadcrumbs_irq(struct intel_engine_cs *engine)
>  {
> struct intel_breadcrumbs *b = >breadcrumbs;
> +   const ktime_t timestamp = ktime_get();
> struct intel_context *ce, *cn;
> struct list_head *pos, *next;
> 

Re: [PATCH 1/9] dma-buf: start caching of sg_table objects

[Daniel Vetter]

On Wed, May 8, 2019 at 2:23 PM Koenig, Christian
 wrote:
>
> Am 08.05.19 um 14:15 schrieb Daniel Vetter:
> > [CAUTION: External Email]
> >
> > On Wed, May 8, 2019 at 2:00 PM Christian König
> >  wrote:
> >> Am 08.05.19 um 12:11 schrieb Daniel Vetter:
> >>> On Wed, May 08, 2019 at 11:15:33AM +0200, Daniel Vetter wrote:
>  On Tue, May 07, 2019 at 10:13:30AM +0200, Christian König wrote:
> > To allow a smooth transition from pinning buffer objects to dynamic
> > invalidation we first start to cache the sg_table for an attachment.
> >
> > Signed-off-by: Christian König 
> > ---
> >drivers/dma-buf/dma-buf.c | 24 
> >include/linux/dma-buf.h   | 14 ++
> >2 files changed, 38 insertions(+)
> >
> > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > index 7c858020d14b..775e13f54083 100644
> > --- a/drivers/dma-buf/dma-buf.c
> > +++ b/drivers/dma-buf/dma-buf.c
> > @@ -573,6 +573,20 @@ struct dma_buf_attachment *dma_buf_attach(struct 
> > dma_buf *dmabuf,
> >  list_add(>node, >attachments);
> >
> >  mutex_unlock(>lock);
> > +
> > +   if (!dma_buf_is_dynamic(dmabuf)) {
> > +   struct sg_table *sgt;
> > +
> > +   sgt = dmabuf->ops->map_dma_buf(attach, DMA_BIDIRECTIONAL);
> > +   if (!sgt)
> > +   sgt = ERR_PTR(-ENOMEM);
> > +   if (IS_ERR(sgt)) {
> > +   dma_buf_detach(dmabuf, attach);
> > +   return ERR_CAST(sgt);
> > +   }
> > +   attach->sgt = sgt;
> > +   }
> > +
> >  return attach;
> >
> >err_attach:
> > @@ -595,6 +609,10 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct 
> > dma_buf_attachment *attach)
> >  if (WARN_ON(!dmabuf || !attach))
> >  return;
> >
> > +   if (attach->sgt)
> > +   dmabuf->ops->unmap_dma_buf(attach, attach->sgt,
> > +  DMA_BIDIRECTIONAL);
> > +
> >  mutex_lock(>lock);
> >  list_del(>node);
> >  if (dmabuf->ops->detach)
> > @@ -630,6 +648,9 @@ struct sg_table *dma_buf_map_attachment(struct 
> > dma_buf_attachment *attach,
> >  if (WARN_ON(!attach || !attach->dmabuf))
> >  return ERR_PTR(-EINVAL);
> >
> > +   if (attach->sgt)
> > +   return attach->sgt;
> > +
> >  sg_table = attach->dmabuf->ops->map_dma_buf(attach, direction);
> >  if (!sg_table)
> >  sg_table = ERR_PTR(-ENOMEM);
> > @@ -657,6 +678,9 @@ void dma_buf_unmap_attachment(struct 
> > dma_buf_attachment *attach,
> >  if (WARN_ON(!attach || !attach->dmabuf || !sg_table))
> >  return;
> >
> > +   if (attach->sgt == sg_table)
> > +   return;
> > +
> >  attach->dmabuf->ops->unmap_dma_buf(attach, sg_table,
> >  direction);
> >}
> > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> > index 58725f890b5b..52031fdc75bb 100644
> > --- a/include/linux/dma-buf.h
> > +++ b/include/linux/dma-buf.h
> > @@ -322,6 +322,7 @@ struct dma_buf_attachment {
> >  struct dma_buf *dmabuf;
> >  struct device *dev;
> >  struct list_head node;
> > +   struct sg_table *sgt;
> >  void *priv;
> >};
> >
> > @@ -373,6 +374,19 @@ static inline void get_dma_buf(struct dma_buf 
> > *dmabuf)
> >  get_file(dmabuf->file);
> >}
> >
> > +/**
> > + * dma_buf_is_dynamic - check if a DMA-buf uses dynamic mappings.
> > + * @dmabuf: the DMA-buf to check
> > + *
> > + * Returns true if a DMA-buf exporter wants to create dynamic sg table 
> > mappings
> > + * for each attachment. False if only a single static sg table should 
> > be used.
> > + */
> > +static inline bool dma_buf_is_dynamic(struct dma_buf *dmabuf)
> > +{
> > +   /* Always use a static mapping for now */
> > +   return false;
>  Hm I still expect that later on we'll want this to be decided by the
>  attachment: It's only dynamic if both the exporter and the importer
>  support dynamic dma-buf management, otherwise we need to pin.
> 
>  But anyway, I feel like we need to go over the entire thing anyway once
>  more when p2p has landed, on this:
> 
>  Reviewed-by: Daniel Vetter 
> >>> Correction that I only just realized, but need to retract that r-b on this
> >>> and the drm sgt cache removal patch: You now hardcode the direction to
> >>> DMA_BIDIRECTIONAL, drm_prime did only keep the cache for a given
> >>> direction.
> >>>
> >>> Now x86 drivers always set DMA_BIDIRECTIONAL, but arm soc drivers (and
> >>> also v4l videobuf layer) try to guess whether it should be DMA_TO_DEVICE
> 

Re: [PATCH 2/2] dma-fence: Refactor signaling for manual invocation

[Daniel Vetter]

On Wed, May 8, 2019 at 2:05 PM Chris Wilson  wrote:
>
> Move the duplicated code within dma-fence.c into the header for wider
> reuse. In the process apply a small micro-optimisation to only prune the
> fence->cb_list once rather than use list_del on every entry.
>
> Signed-off-by: Chris Wilson 
> Reviewed-by: Tvrtko Ursulin 

I have no opinion on the change itself, but spotted two things while
trying to understand what's going on:

- Please update Documentation/driver-api/dma-buf.rst to keep the
kerneldoc in the newly extracted file included.

- The DMA_FENCE_FLAG_TIMESTAMP_BIT trickery added in 76250f2b743b7
seems to have lost the memory barriers in the process. I think we need
to re-add them. Altough looking at the old code we lacked them on the
reader side since forever :-/

Cheers, Daniel

> ---
>  drivers/dma-buf/Makefile|  10 +-
>  drivers/dma-buf/dma-fence-trace.c   |  28 +++
>  drivers/dma-buf/dma-fence.c |  32 +--
>  drivers/gpu/drm/i915/gt/intel_breadcrumbs.c |  30 ---
>  include/linux/dma-fence-types.h | 248 +++
>  include/linux/dma-fence.h   | 251 +++-
>  6 files changed, 321 insertions(+), 278 deletions(-)
>  create mode 100644 drivers/dma-buf/dma-fence-trace.c
>  create mode 100644 include/linux/dma-fence-types.h
>
> diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile
> index 1f006e083eb9..56e579878f26 100644
> --- a/drivers/dma-buf/Makefile
> +++ b/drivers/dma-buf/Makefile
> @@ -1,5 +1,11 @@
> -obj-y := dma-buf.o dma-fence.o dma-fence-array.o dma-fence-chain.o \
> -reservation.o seqno-fence.o
> +obj-y := \
> +   dma-buf.o \
> +   dma-fence.o \
> +   dma-fence-array.o \
> +   dma-fence-chain.o \
> +   dma-fence-trace.o \
> +   reservation.o \
> +   seqno-fence.o
>  obj-$(CONFIG_SYNC_FILE)+= sync_file.o
>  obj-$(CONFIG_SW_SYNC)  += sw_sync.o sync_debug.o
>  obj-$(CONFIG_UDMABUF)  += udmabuf.o
> diff --git a/drivers/dma-buf/dma-fence-trace.c 
> b/drivers/dma-buf/dma-fence-trace.c
> new file mode 100644
> index ..eb6f282be4c0
> --- /dev/null
> +++ b/drivers/dma-buf/dma-fence-trace.c
> @@ -0,0 +1,28 @@
> +/*
> + * Fence mechanism for dma-buf and to allow for asynchronous dma access
> + *
> + * Copyright (C) 2012 Canonical Ltd
> + * Copyright (C) 2012 Texas Instruments
> + *
> + * Authors:
> + * Rob Clark 
> + * Maarten Lankhorst 
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published 
> by
> + * the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but 
> WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + */
> +
> +#include 
> +
> +#define CREATE_TRACE_POINTS
> +#include 
> +
> +EXPORT_TRACEPOINT_SYMBOL(dma_fence_emit);
> +EXPORT_TRACEPOINT_SYMBOL(dma_fence_enable_signal);
> +EXPORT_TRACEPOINT_SYMBOL(dma_fence_signaled);
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index 9bf06042619a..8196a179fdc2 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -24,13 +24,6 @@
>  #include 
>  #include 
>
> -#define CREATE_TRACE_POINTS
> -#include 
> -
> -EXPORT_TRACEPOINT_SYMBOL(dma_fence_emit);
> -EXPORT_TRACEPOINT_SYMBOL(dma_fence_enable_signal);
> -EXPORT_TRACEPOINT_SYMBOL(dma_fence_signaled);
> -
>  static DEFINE_SPINLOCK(dma_fence_stub_lock);
>  static struct dma_fence dma_fence_stub;
>
> @@ -136,7 +129,6 @@ EXPORT_SYMBOL(dma_fence_context_alloc);
>   */
>  int dma_fence_signal_locked(struct dma_fence *fence)
>  {
> -   struct dma_fence_cb *cur, *tmp;
> int ret = 0;
>
> lockdep_assert_held(fence->lock);
> @@ -144,7 +136,7 @@ int dma_fence_signal_locked(struct dma_fence *fence)
> if (WARN_ON(!fence))
> return -EINVAL;
>
> -   if (test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags)) {
> +   if (!__dma_fence_signal(fence)) {
> ret = -EINVAL;
>
> /*
> @@ -152,15 +144,10 @@ int dma_fence_signal_locked(struct dma_fence *fence)
>  * still run through all callbacks
>  */
> } else {
> -   fence->timestamp = ktime_get();
> -   set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags);
> -   trace_dma_fence_signaled(fence);
> +   __dma_fence_signal__timestamp(fence, ktime_get());
> }
>
> -   list_for_each_entry_safe(cur, tmp, >cb_list, node) {
> -   list_del_init(>node);
> -   cur->func(fence, cur);
> -   }
> +   __dma_fence_signal__notify(fence);
> return ret;
>  }
>  EXPORT_SYMBOL(dma_fence_signal_locked);
> @@ -185,21 +172,14 @@ int 

Re: [PATCH 1/9] dma-buf: start caching of sg_table objects

[Koenig, Christian]

Am 08.05.19 um 14:15 schrieb Daniel Vetter:
> [CAUTION: External Email]
>
> On Wed, May 8, 2019 at 2:00 PM Christian König
>  wrote:
>> Am 08.05.19 um 12:11 schrieb Daniel Vetter:
>>> On Wed, May 08, 2019 at 11:15:33AM +0200, Daniel Vetter wrote:
 On Tue, May 07, 2019 at 10:13:30AM +0200, Christian König wrote:
> To allow a smooth transition from pinning buffer objects to dynamic
> invalidation we first start to cache the sg_table for an attachment.
>
> Signed-off-by: Christian König 
> ---
>drivers/dma-buf/dma-buf.c | 24 
>include/linux/dma-buf.h   | 14 ++
>2 files changed, 38 insertions(+)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 7c858020d14b..775e13f54083 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -573,6 +573,20 @@ struct dma_buf_attachment *dma_buf_attach(struct 
> dma_buf *dmabuf,
>  list_add(>node, >attachments);
>
>  mutex_unlock(>lock);
> +
> +   if (!dma_buf_is_dynamic(dmabuf)) {
> +   struct sg_table *sgt;
> +
> +   sgt = dmabuf->ops->map_dma_buf(attach, DMA_BIDIRECTIONAL);
> +   if (!sgt)
> +   sgt = ERR_PTR(-ENOMEM);
> +   if (IS_ERR(sgt)) {
> +   dma_buf_detach(dmabuf, attach);
> +   return ERR_CAST(sgt);
> +   }
> +   attach->sgt = sgt;
> +   }
> +
>  return attach;
>
>err_attach:
> @@ -595,6 +609,10 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct 
> dma_buf_attachment *attach)
>  if (WARN_ON(!dmabuf || !attach))
>  return;
>
> +   if (attach->sgt)
> +   dmabuf->ops->unmap_dma_buf(attach, attach->sgt,
> +  DMA_BIDIRECTIONAL);
> +
>  mutex_lock(>lock);
>  list_del(>node);
>  if (dmabuf->ops->detach)
> @@ -630,6 +648,9 @@ struct sg_table *dma_buf_map_attachment(struct 
> dma_buf_attachment *attach,
>  if (WARN_ON(!attach || !attach->dmabuf))
>  return ERR_PTR(-EINVAL);
>
> +   if (attach->sgt)
> +   return attach->sgt;
> +
>  sg_table = attach->dmabuf->ops->map_dma_buf(attach, direction);
>  if (!sg_table)
>  sg_table = ERR_PTR(-ENOMEM);
> @@ -657,6 +678,9 @@ void dma_buf_unmap_attachment(struct 
> dma_buf_attachment *attach,
>  if (WARN_ON(!attach || !attach->dmabuf || !sg_table))
>  return;
>
> +   if (attach->sgt == sg_table)
> +   return;
> +
>  attach->dmabuf->ops->unmap_dma_buf(attach, sg_table,
>  direction);
>}
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index 58725f890b5b..52031fdc75bb 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -322,6 +322,7 @@ struct dma_buf_attachment {
>  struct dma_buf *dmabuf;
>  struct device *dev;
>  struct list_head node;
> +   struct sg_table *sgt;
>  void *priv;
>};
>
> @@ -373,6 +374,19 @@ static inline void get_dma_buf(struct dma_buf 
> *dmabuf)
>  get_file(dmabuf->file);
>}
>
> +/**
> + * dma_buf_is_dynamic - check if a DMA-buf uses dynamic mappings.
> + * @dmabuf: the DMA-buf to check
> + *
> + * Returns true if a DMA-buf exporter wants to create dynamic sg table 
> mappings
> + * for each attachment. False if only a single static sg table should be 
> used.
> + */
> +static inline bool dma_buf_is_dynamic(struct dma_buf *dmabuf)
> +{
> +   /* Always use a static mapping for now */
> +   return false;
 Hm I still expect that later on we'll want this to be decided by the
 attachment: It's only dynamic if both the exporter and the importer
 support dynamic dma-buf management, otherwise we need to pin.

 But anyway, I feel like we need to go over the entire thing anyway once
 more when p2p has landed, on this:

 Reviewed-by: Daniel Vetter 
>>> Correction that I only just realized, but need to retract that r-b on this
>>> and the drm sgt cache removal patch: You now hardcode the direction to
>>> DMA_BIDIRECTIONAL, drm_prime did only keep the cache for a given
>>> direction.
>>>
>>> Now x86 drivers always set DMA_BIDIRECTIONAL, but arm soc drivers (and
>>> also v4l videobuf layer) try to guess whether it should be DMA_TO_DEVICE
>>> or DMA_FROM_DEVICE. I have no idea what the implications are, also all the
>>> cache coherency on dma-bufs is kinda ill-defined.
>>>
>>> And we can't throw the sgt cache away at map time if it doesn't fit like
>>> drm_prime does, because that reintroduces the reservation object lock,
>>> defeating the 

Re: [PATCH 09/16] mmc: sdhci-xenon: use new match_string() helper/macro

[Dan Carpenter]

On Wed, May 08, 2019 at 02:28:35PM +0300, Alexandru Ardelean wrote:
> -static const char * const phy_types[] = {
> - "emmc 5.0 phy",
> - "emmc 5.1 phy"
> -};
> -
>  enum xenon_phy_type_enum {
>   EMMC_5_0_PHY,
>   EMMC_5_1_PHY,
>   NR_PHY_TYPES

There is no need for NR_PHY_TYPES now so you could remove that as well.

regards,
dan carpenter

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

Re: [PATCH 2/2] dma-fence: Refactor signaling for manual invocation

[Tvrtko Ursulin]

On 08/05/2019 13:05, Chris Wilson wrote:

Move the duplicated code within dma-fence.c into the header for wider
reuse. In the process apply a small micro-optimisation to only prune the
fence->cb_list once rather than use list_del on every entry.

Signed-off-by: Chris Wilson 
Reviewed-by: Tvrtko Ursulin 


I did not give r-b yet, this was probably a misunderstanding.

I said the general idea sounds right to me, but I was unsure whether to 
go the static inline route, or maybe EXPORT_SYMBOL the decomposed helpers.


Regards,

Tvrtko


---
  drivers/dma-buf/Makefile|  10 +-
  drivers/dma-buf/dma-fence-trace.c   |  28 +++
  drivers/dma-buf/dma-fence.c |  32 +--
  drivers/gpu/drm/i915/gt/intel_breadcrumbs.c |  30 ---
  include/linux/dma-fence-types.h | 248 +++
  include/linux/dma-fence.h   | 251 +++-
  6 files changed, 321 insertions(+), 278 deletions(-)
  create mode 100644 drivers/dma-buf/dma-fence-trace.c
  create mode 100644 include/linux/dma-fence-types.h

diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile
index 1f006e083eb9..56e579878f26 100644
--- a/drivers/dma-buf/Makefile
+++ b/drivers/dma-buf/Makefile
@@ -1,5 +1,11 @@
-obj-y := dma-buf.o dma-fence.o dma-fence-array.o dma-fence-chain.o \
-reservation.o seqno-fence.o
+obj-y := \
+   dma-buf.o \
+   dma-fence.o \
+   dma-fence-array.o \
+   dma-fence-chain.o \
+   dma-fence-trace.o \
+   reservation.o \
+   seqno-fence.o
  obj-$(CONFIG_SYNC_FILE)   += sync_file.o
  obj-$(CONFIG_SW_SYNC) += sw_sync.o sync_debug.o
  obj-$(CONFIG_UDMABUF) += udmabuf.o
diff --git a/drivers/dma-buf/dma-fence-trace.c 
b/drivers/dma-buf/dma-fence-trace.c
new file mode 100644
index ..eb6f282be4c0
--- /dev/null
+++ b/drivers/dma-buf/dma-fence-trace.c
@@ -0,0 +1,28 @@
+/*
+ * Fence mechanism for dma-buf and to allow for asynchronous dma access
+ *
+ * Copyright (C) 2012 Canonical Ltd
+ * Copyright (C) 2012 Texas Instruments
+ *
+ * Authors:
+ * Rob Clark 
+ * Maarten Lankhorst 
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+
+#include 
+
+#define CREATE_TRACE_POINTS
+#include 
+
+EXPORT_TRACEPOINT_SYMBOL(dma_fence_emit);
+EXPORT_TRACEPOINT_SYMBOL(dma_fence_enable_signal);
+EXPORT_TRACEPOINT_SYMBOL(dma_fence_signaled);
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 9bf06042619a..8196a179fdc2 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -24,13 +24,6 @@
  #include 
  #include 
  
-#define CREATE_TRACE_POINTS

-#include 
-
-EXPORT_TRACEPOINT_SYMBOL(dma_fence_emit);
-EXPORT_TRACEPOINT_SYMBOL(dma_fence_enable_signal);
-EXPORT_TRACEPOINT_SYMBOL(dma_fence_signaled);
-
  static DEFINE_SPINLOCK(dma_fence_stub_lock);
  static struct dma_fence dma_fence_stub;
  
@@ -136,7 +129,6 @@ EXPORT_SYMBOL(dma_fence_context_alloc);

   */
  int dma_fence_signal_locked(struct dma_fence *fence)
  {
-   struct dma_fence_cb *cur, *tmp;
int ret = 0;
  
  	lockdep_assert_held(fence->lock);

@@ -144,7 +136,7 @@ int dma_fence_signal_locked(struct dma_fence *fence)
if (WARN_ON(!fence))
return -EINVAL;
  
-	if (test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags)) {

+   if (!__dma_fence_signal(fence)) {
ret = -EINVAL;
  
  		/*

@@ -152,15 +144,10 @@ int dma_fence_signal_locked(struct dma_fence *fence)
 * still run through all callbacks
 */
} else {
-   fence->timestamp = ktime_get();
-   set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags);
-   trace_dma_fence_signaled(fence);
+   __dma_fence_signal__timestamp(fence, ktime_get());
}
  
-	list_for_each_entry_safe(cur, tmp, >cb_list, node) {

-   list_del_init(>node);
-   cur->func(fence, cur);
-   }
+   __dma_fence_signal__notify(fence);
return ret;
  }
  EXPORT_SYMBOL(dma_fence_signal_locked);
@@ -185,21 +172,14 @@ int dma_fence_signal(struct dma_fence *fence)
if (!fence)
return -EINVAL;
  
-	if (test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags))

+   if (!__dma_fence_signal(fence))
return -EINVAL;
  
-	fence->timestamp = ktime_get();

-   set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags);
-   trace_dma_fence_signaled(fence);
+   __dma_fence_signal__timestamp(fence, ktime_get());
  
  	if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, >flags)) {

-   struct 

Re: [PATCH 1/9] dma-buf: start caching of sg_table objects

[Daniel Vetter]

On Wed, May 8, 2019 at 2:00 PM Christian König
 wrote:
> Am 08.05.19 um 12:11 schrieb Daniel Vetter:
> > On Wed, May 08, 2019 at 11:15:33AM +0200, Daniel Vetter wrote:
> >> On Tue, May 07, 2019 at 10:13:30AM +0200, Christian König wrote:
> >>> To allow a smooth transition from pinning buffer objects to dynamic
> >>> invalidation we first start to cache the sg_table for an attachment.
> >>>
> >>> Signed-off-by: Christian König 
> >>> ---
> >>>   drivers/dma-buf/dma-buf.c | 24 
> >>>   include/linux/dma-buf.h   | 14 ++
> >>>   2 files changed, 38 insertions(+)
> >>>
> >>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> >>> index 7c858020d14b..775e13f54083 100644
> >>> --- a/drivers/dma-buf/dma-buf.c
> >>> +++ b/drivers/dma-buf/dma-buf.c
> >>> @@ -573,6 +573,20 @@ struct dma_buf_attachment *dma_buf_attach(struct 
> >>> dma_buf *dmabuf,
> >>> list_add(>node, >attachments);
> >>>
> >>> mutex_unlock(>lock);
> >>> +
> >>> +   if (!dma_buf_is_dynamic(dmabuf)) {
> >>> +   struct sg_table *sgt;
> >>> +
> >>> +   sgt = dmabuf->ops->map_dma_buf(attach, DMA_BIDIRECTIONAL);
> >>> +   if (!sgt)
> >>> +   sgt = ERR_PTR(-ENOMEM);
> >>> +   if (IS_ERR(sgt)) {
> >>> +   dma_buf_detach(dmabuf, attach);
> >>> +   return ERR_CAST(sgt);
> >>> +   }
> >>> +   attach->sgt = sgt;
> >>> +   }
> >>> +
> >>> return attach;
> >>>
> >>>   err_attach:
> >>> @@ -595,6 +609,10 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct 
> >>> dma_buf_attachment *attach)
> >>> if (WARN_ON(!dmabuf || !attach))
> >>> return;
> >>>
> >>> +   if (attach->sgt)
> >>> +   dmabuf->ops->unmap_dma_buf(attach, attach->sgt,
> >>> +  DMA_BIDIRECTIONAL);
> >>> +
> >>> mutex_lock(>lock);
> >>> list_del(>node);
> >>> if (dmabuf->ops->detach)
> >>> @@ -630,6 +648,9 @@ struct sg_table *dma_buf_map_attachment(struct 
> >>> dma_buf_attachment *attach,
> >>> if (WARN_ON(!attach || !attach->dmabuf))
> >>> return ERR_PTR(-EINVAL);
> >>>
> >>> +   if (attach->sgt)
> >>> +   return attach->sgt;
> >>> +
> >>> sg_table = attach->dmabuf->ops->map_dma_buf(attach, direction);
> >>> if (!sg_table)
> >>> sg_table = ERR_PTR(-ENOMEM);
> >>> @@ -657,6 +678,9 @@ void dma_buf_unmap_attachment(struct 
> >>> dma_buf_attachment *attach,
> >>> if (WARN_ON(!attach || !attach->dmabuf || !sg_table))
> >>> return;
> >>>
> >>> +   if (attach->sgt == sg_table)
> >>> +   return;
> >>> +
> >>> attach->dmabuf->ops->unmap_dma_buf(attach, sg_table,
> >>> direction);
> >>>   }
> >>> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> >>> index 58725f890b5b..52031fdc75bb 100644
> >>> --- a/include/linux/dma-buf.h
> >>> +++ b/include/linux/dma-buf.h
> >>> @@ -322,6 +322,7 @@ struct dma_buf_attachment {
> >>> struct dma_buf *dmabuf;
> >>> struct device *dev;
> >>> struct list_head node;
> >>> +   struct sg_table *sgt;
> >>> void *priv;
> >>>   };
> >>>
> >>> @@ -373,6 +374,19 @@ static inline void get_dma_buf(struct dma_buf 
> >>> *dmabuf)
> >>> get_file(dmabuf->file);
> >>>   }
> >>>
> >>> +/**
> >>> + * dma_buf_is_dynamic - check if a DMA-buf uses dynamic mappings.
> >>> + * @dmabuf: the DMA-buf to check
> >>> + *
> >>> + * Returns true if a DMA-buf exporter wants to create dynamic sg table 
> >>> mappings
> >>> + * for each attachment. False if only a single static sg table should be 
> >>> used.
> >>> + */
> >>> +static inline bool dma_buf_is_dynamic(struct dma_buf *dmabuf)
> >>> +{
> >>> +   /* Always use a static mapping for now */
> >>> +   return false;
> >> Hm I still expect that later on we'll want this to be decided by the
> >> attachment: It's only dynamic if both the exporter and the importer
> >> support dynamic dma-buf management, otherwise we need to pin.
> >>
> >> But anyway, I feel like we need to go over the entire thing anyway once
> >> more when p2p has landed, on this:
> >>
> >> Reviewed-by: Daniel Vetter 
> > Correction that I only just realized, but need to retract that r-b on this
> > and the drm sgt cache removal patch: You now hardcode the direction to
> > DMA_BIDIRECTIONAL, drm_prime did only keep the cache for a given
> > direction.
> >
> > Now x86 drivers always set DMA_BIDIRECTIONAL, but arm soc drivers (and
> > also v4l videobuf layer) try to guess whether it should be DMA_TO_DEVICE
> > or DMA_FROM_DEVICE. I have no idea what the implications are, also all the
> > cache coherency on dma-bufs is kinda ill-defined.
> >
> > And we can't throw the sgt cache away at map time if it doesn't fit like
> > drm_prime does, because that reintroduces the reservation object lock,
> > defeating the entire purpose of this. Also we can't just assume drm_prime
> > works for everyone, since the special cases 

Re: [PATCH 2/2] dma-fence: Refactor signaling for manual invocation

[Chris Wilson]

Quoting Chris Wilson (2019-05-08 13:05:42)
> Move the duplicated code within dma-fence.c into the header for wider
> reuse. In the process apply a small micro-optimisation to only prune the
> fence->cb_list once rather than use list_del on every entry.
> 
> Signed-off-by: Chris Wilson 
> Reviewed-by: Tvrtko Ursulin 

Not yet r-b, just Tvrtko gave a nod of approval that this would be
suitable discussion material.
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Manual dma-fence signaling

[Chris Wilson]

In order to fix a bug in i915, I would like to opencode the dma fence
signal processing (to close a race condition with preemption and signal
enabling). Tvrtko quite rightly objected to the opencoding and suggested
that dma-fence should provide the helpers, so here's my suggestion wrt
the interface used by dma_fence.c and desired by i915.ko
-Chris


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

[PATCH 1/2] drm/i915: Seal races between async GPU cancellation, retirement and signaling

[Chris Wilson]

Currently there is an underlying assumption that i915_request_unsubmit()
is synchronous wrt the GPU -- that is the request is no longer in flight
as we remove it. In the near future that may change, and this may upset
our signaling as we can process an interrupt for that request while it
is no longer in flight.

CPU0CPU1
intel_engine_breadcrumbs_irq
(queue request completion)
i915_request_cancel_signaling
... ...
i915_request_enable_signaling
dma_fence_signal

Hence in the time it took us to drop the lock to signal the request, a
preemption event may have occurred and re-queued the request. In the
process, that request would have seen I915_FENCE_FLAG_SIGNAL clear and
so reused the rq->signal_link that was in use on CPU0, leading to bad
pointer chasing in intel_engine_breadcrumbs_irq.

A related issue was that if someone started listening for a signal on a
completed but no longer in-flight request, we missed the opportunity to
immediately signal that request.

Furthermore, as intel_contexts may be immediately released during
request retirement, in order to be entirely sure that
intel_engine_breadcrumbs_irq may no longer dereference the intel_context
(ce->signals and ce->signal_link), we must wait for irq spinlock.

In order to prevent the race, we use a bit in the fence.flags to signal
the transfer onto the signal list inside intel_engine_breadcrumbs_irq.
For simplicity, we use the DMA_FENCE_FLAG_SIGNALED_BIT as it then
quickly signals to any outside observer that the fence is indeed signaled.

v2: Sketch out potential dma-fence API for manual signaling
v3: And the test_and_set_bit()

Fixes: 52c0fdb25c7c ("drm/i915: Replace global breadcrumbs with per-context 
interrupt tracking")
Signed-off-by: Chris Wilson 
Cc: Tvrtko Ursulin 
Reviewed-by: Tvrtko Ursulin 
---
 drivers/dma-buf/dma-fence.c |  1 +
 drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 78 +++--
 drivers/gpu/drm/i915/i915_request.c |  1 +
 drivers/gpu/drm/i915/intel_guc_submission.c |  1 -
 4 files changed, 59 insertions(+), 22 deletions(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 3aa8733f832a..9bf06042619a 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -29,6 +29,7 @@
 
 EXPORT_TRACEPOINT_SYMBOL(dma_fence_emit);
 EXPORT_TRACEPOINT_SYMBOL(dma_fence_enable_signal);
+EXPORT_TRACEPOINT_SYMBOL(dma_fence_signaled);
 
 static DEFINE_SPINLOCK(dma_fence_stub_lock);
 static struct dma_fence dma_fence_stub;
diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c 
b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
index fe455f01aa65..c092bdf5f0bf 100644
--- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
@@ -23,6 +23,7 @@
  */
 
 #include 
+#include 
 #include 
 
 #include "i915_drv.h"
@@ -96,9 +97,39 @@ check_signal_order(struct intel_context *ce, struct 
i915_request *rq)
return true;
 }
 
+static bool
+__dma_fence_signal(struct dma_fence *fence)
+{
+   return !test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags);
+}
+
+static void
+__dma_fence_signal__timestamp(struct dma_fence *fence, ktime_t timestamp)
+{
+   fence->timestamp = timestamp;
+   set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags);
+   trace_dma_fence_signaled(fence);
+}
+
+static void
+__dma_fence_signal__notify(struct dma_fence *fence)
+{
+   struct dma_fence_cb *cur, *tmp;
+
+   lockdep_assert_held(fence->lock);
+   lockdep_assert_irqs_disabled();
+
+   list_for_each_entry_safe(cur, tmp, >cb_list, node) {
+   INIT_LIST_HEAD(>node);
+   cur->func(fence, cur);
+   }
+   INIT_LIST_HEAD(>cb_list);
+}
+
 void intel_engine_breadcrumbs_irq(struct intel_engine_cs *engine)
 {
struct intel_breadcrumbs *b = >breadcrumbs;
+   const ktime_t timestamp = ktime_get();
struct intel_context *ce, *cn;
struct list_head *pos, *next;
LIST_HEAD(signal);
@@ -122,6 +153,10 @@ void intel_engine_breadcrumbs_irq(struct intel_engine_cs 
*engine)
 
GEM_BUG_ON(!test_bit(I915_FENCE_FLAG_SIGNAL,
 >fence.flags));
+   clear_bit(I915_FENCE_FLAG_SIGNAL, >fence.flags);
+
+   if (!__dma_fence_signal(>fence))
+   continue;
 
/*
 * Queue for execution after dropping the signaling
@@ -129,14 +164,6 @@ void intel_engine_breadcrumbs_irq(struct intel_engine_cs 
*engine)
 * more signalers to the same context or engine.
 */
i915_request_get(rq);
-
-   /*
-* We may race with direct invocation of
-* dma_fence_signal(), 

[PATCH 2/2] dma-fence: Refactor signaling for manual invocation

[Chris Wilson]

Move the duplicated code within dma-fence.c into the header for wider
reuse. In the process apply a small micro-optimisation to only prune the
fence->cb_list once rather than use list_del on every entry.

Signed-off-by: Chris Wilson 
Reviewed-by: Tvrtko Ursulin 
---
 drivers/dma-buf/Makefile|  10 +-
 drivers/dma-buf/dma-fence-trace.c   |  28 +++
 drivers/dma-buf/dma-fence.c |  32 +--
 drivers/gpu/drm/i915/gt/intel_breadcrumbs.c |  30 ---
 include/linux/dma-fence-types.h | 248 +++
 include/linux/dma-fence.h   | 251 +++-
 6 files changed, 321 insertions(+), 278 deletions(-)
 create mode 100644 drivers/dma-buf/dma-fence-trace.c
 create mode 100644 include/linux/dma-fence-types.h

diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile
index 1f006e083eb9..56e579878f26 100644
--- a/drivers/dma-buf/Makefile
+++ b/drivers/dma-buf/Makefile
@@ -1,5 +1,11 @@
-obj-y := dma-buf.o dma-fence.o dma-fence-array.o dma-fence-chain.o \
-reservation.o seqno-fence.o
+obj-y := \
+   dma-buf.o \
+   dma-fence.o \
+   dma-fence-array.o \
+   dma-fence-chain.o \
+   dma-fence-trace.o \
+   reservation.o \
+   seqno-fence.o
 obj-$(CONFIG_SYNC_FILE)+= sync_file.o
 obj-$(CONFIG_SW_SYNC)  += sw_sync.o sync_debug.o
 obj-$(CONFIG_UDMABUF)  += udmabuf.o
diff --git a/drivers/dma-buf/dma-fence-trace.c 
b/drivers/dma-buf/dma-fence-trace.c
new file mode 100644
index ..eb6f282be4c0
--- /dev/null
+++ b/drivers/dma-buf/dma-fence-trace.c
@@ -0,0 +1,28 @@
+/*
+ * Fence mechanism for dma-buf and to allow for asynchronous dma access
+ *
+ * Copyright (C) 2012 Canonical Ltd
+ * Copyright (C) 2012 Texas Instruments
+ *
+ * Authors:
+ * Rob Clark 
+ * Maarten Lankhorst 
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+
+#include 
+
+#define CREATE_TRACE_POINTS
+#include 
+
+EXPORT_TRACEPOINT_SYMBOL(dma_fence_emit);
+EXPORT_TRACEPOINT_SYMBOL(dma_fence_enable_signal);
+EXPORT_TRACEPOINT_SYMBOL(dma_fence_signaled);
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 9bf06042619a..8196a179fdc2 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -24,13 +24,6 @@
 #include 
 #include 
 
-#define CREATE_TRACE_POINTS
-#include 
-
-EXPORT_TRACEPOINT_SYMBOL(dma_fence_emit);
-EXPORT_TRACEPOINT_SYMBOL(dma_fence_enable_signal);
-EXPORT_TRACEPOINT_SYMBOL(dma_fence_signaled);
-
 static DEFINE_SPINLOCK(dma_fence_stub_lock);
 static struct dma_fence dma_fence_stub;
 
@@ -136,7 +129,6 @@ EXPORT_SYMBOL(dma_fence_context_alloc);
  */
 int dma_fence_signal_locked(struct dma_fence *fence)
 {
-   struct dma_fence_cb *cur, *tmp;
int ret = 0;
 
lockdep_assert_held(fence->lock);
@@ -144,7 +136,7 @@ int dma_fence_signal_locked(struct dma_fence *fence)
if (WARN_ON(!fence))
return -EINVAL;
 
-   if (test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags)) {
+   if (!__dma_fence_signal(fence)) {
ret = -EINVAL;
 
/*
@@ -152,15 +144,10 @@ int dma_fence_signal_locked(struct dma_fence *fence)
 * still run through all callbacks
 */
} else {
-   fence->timestamp = ktime_get();
-   set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags);
-   trace_dma_fence_signaled(fence);
+   __dma_fence_signal__timestamp(fence, ktime_get());
}
 
-   list_for_each_entry_safe(cur, tmp, >cb_list, node) {
-   list_del_init(>node);
-   cur->func(fence, cur);
-   }
+   __dma_fence_signal__notify(fence);
return ret;
 }
 EXPORT_SYMBOL(dma_fence_signal_locked);
@@ -185,21 +172,14 @@ int dma_fence_signal(struct dma_fence *fence)
if (!fence)
return -EINVAL;
 
-   if (test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags))
+   if (!__dma_fence_signal(fence))
return -EINVAL;
 
-   fence->timestamp = ktime_get();
-   set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags);
-   trace_dma_fence_signaled(fence);
+   __dma_fence_signal__timestamp(fence, ktime_get());
 
if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, >flags)) {
-   struct dma_fence_cb *cur, *tmp;
-
spin_lock_irqsave(fence->lock, flags);
-   list_for_each_entry_safe(cur, tmp, >cb_list, node) {
-   list_del_init(>node);
-   cur->func(fence, cur);
-   }
+ 

Re: [PATCH 1/9] dma-buf: start caching of sg_table objects

[Christian König]

Am 08.05.19 um 12:11 schrieb Daniel Vetter:

On Wed, May 08, 2019 at 11:15:33AM +0200, Daniel Vetter wrote:

On Tue, May 07, 2019 at 10:13:30AM +0200, Christian König wrote:

To allow a smooth transition from pinning buffer objects to dynamic
invalidation we first start to cache the sg_table for an attachment.

Signed-off-by: Christian König 
---
  drivers/dma-buf/dma-buf.c | 24 
  include/linux/dma-buf.h   | 14 ++
  2 files changed, 38 insertions(+)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 7c858020d14b..775e13f54083 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -573,6 +573,20 @@ struct dma_buf_attachment *dma_buf_attach(struct dma_buf 
*dmabuf,
list_add(>node, >attachments);
  
  	mutex_unlock(>lock);

+
+   if (!dma_buf_is_dynamic(dmabuf)) {
+   struct sg_table *sgt;
+
+   sgt = dmabuf->ops->map_dma_buf(attach, DMA_BIDIRECTIONAL);
+   if (!sgt)
+   sgt = ERR_PTR(-ENOMEM);
+   if (IS_ERR(sgt)) {
+   dma_buf_detach(dmabuf, attach);
+   return ERR_CAST(sgt);
+   }
+   attach->sgt = sgt;
+   }
+
return attach;
  
  err_attach:

@@ -595,6 +609,10 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct 
dma_buf_attachment *attach)
if (WARN_ON(!dmabuf || !attach))
return;
  
+	if (attach->sgt)

+   dmabuf->ops->unmap_dma_buf(attach, attach->sgt,
+  DMA_BIDIRECTIONAL);
+
mutex_lock(>lock);
list_del(>node);
if (dmabuf->ops->detach)
@@ -630,6 +648,9 @@ struct sg_table *dma_buf_map_attachment(struct 
dma_buf_attachment *attach,
if (WARN_ON(!attach || !attach->dmabuf))
return ERR_PTR(-EINVAL);
  
+	if (attach->sgt)

+   return attach->sgt;
+
sg_table = attach->dmabuf->ops->map_dma_buf(attach, direction);
if (!sg_table)
sg_table = ERR_PTR(-ENOMEM);
@@ -657,6 +678,9 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment 
*attach,
if (WARN_ON(!attach || !attach->dmabuf || !sg_table))
return;
  
+	if (attach->sgt == sg_table)

+   return;
+
attach->dmabuf->ops->unmap_dma_buf(attach, sg_table,
direction);
  }
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index 58725f890b5b..52031fdc75bb 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -322,6 +322,7 @@ struct dma_buf_attachment {
struct dma_buf *dmabuf;
struct device *dev;
struct list_head node;
+   struct sg_table *sgt;
void *priv;
  };
  
@@ -373,6 +374,19 @@ static inline void get_dma_buf(struct dma_buf *dmabuf)

get_file(dmabuf->file);
  }
  
+/**

+ * dma_buf_is_dynamic - check if a DMA-buf uses dynamic mappings.
+ * @dmabuf: the DMA-buf to check
+ *
+ * Returns true if a DMA-buf exporter wants to create dynamic sg table mappings
+ * for each attachment. False if only a single static sg table should be used.
+ */
+static inline bool dma_buf_is_dynamic(struct dma_buf *dmabuf)
+{
+   /* Always use a static mapping for now */
+   return false;

Hm I still expect that later on we'll want this to be decided by the
attachment: It's only dynamic if both the exporter and the importer
support dynamic dma-buf management, otherwise we need to pin.

But anyway, I feel like we need to go over the entire thing anyway once
more when p2p has landed, on this:

Reviewed-by: Daniel Vetter 

Correction that I only just realized, but need to retract that r-b on this
and the drm sgt cache removal patch: You now hardcode the direction to
DMA_BIDIRECTIONAL, drm_prime did only keep the cache for a given
direction.

Now x86 drivers always set DMA_BIDIRECTIONAL, but arm soc drivers (and
also v4l videobuf layer) try to guess whether it should be DMA_TO_DEVICE
or DMA_FROM_DEVICE. I have no idea what the implications are, also all the
cache coherency on dma-bufs is kinda ill-defined.

And we can't throw the sgt cache away at map time if it doesn't fit like
drm_prime does, because that reintroduces the reservation object lock,
defeating the entire purpose of this. Also we can't just assume drm_prime
works for everyone, since the special cases all roll their own dma-buf
import. I have also not checked what exactly exporters do. No idea what to
do here now.

/me cries


Well it is kind of abusing to use the DMA direction here in the first 
place and I actually considered to completely remove it.


The key problem is that the DMA direction defines things from the view 
point of the CPU, e.g. DMA_TO_DEVICE means the data has been accessed by 
the CPU and we are now pushing it to a device.


But DMA-buf is essentially about device to device transfers, so 
DMA_TO_DEVICE as well as DMA_FROM_DEVICE is 

Re: [PATCH 1/9] dma-buf: start caching of sg_table objects

[Christian König]

Am 08.05.19 um 11:15 schrieb Daniel Vetter:

On Tue, May 07, 2019 at 10:13:30AM +0200, Christian König wrote:

To allow a smooth transition from pinning buffer objects to dynamic
invalidation we first start to cache the sg_table for an attachment.

Signed-off-by: Christian König 
---
  drivers/dma-buf/dma-buf.c | 24 
  include/linux/dma-buf.h   | 14 ++
  2 files changed, 38 insertions(+)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 7c858020d14b..775e13f54083 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -573,6 +573,20 @@ struct dma_buf_attachment *dma_buf_attach(struct dma_buf 
*dmabuf,
list_add(>node, >attachments);
  
  	mutex_unlock(>lock);

+
+   if (!dma_buf_is_dynamic(dmabuf)) {
+   struct sg_table *sgt;
+
+   sgt = dmabuf->ops->map_dma_buf(attach, DMA_BIDIRECTIONAL);
+   if (!sgt)
+   sgt = ERR_PTR(-ENOMEM);
+   if (IS_ERR(sgt)) {
+   dma_buf_detach(dmabuf, attach);
+   return ERR_CAST(sgt);
+   }
+   attach->sgt = sgt;
+   }
+
return attach;
  
  err_attach:

@@ -595,6 +609,10 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct 
dma_buf_attachment *attach)
if (WARN_ON(!dmabuf || !attach))
return;
  
+	if (attach->sgt)

+   dmabuf->ops->unmap_dma_buf(attach, attach->sgt,
+  DMA_BIDIRECTIONAL);
+
mutex_lock(>lock);
list_del(>node);
if (dmabuf->ops->detach)
@@ -630,6 +648,9 @@ struct sg_table *dma_buf_map_attachment(struct 
dma_buf_attachment *attach,
if (WARN_ON(!attach || !attach->dmabuf))
return ERR_PTR(-EINVAL);
  
+	if (attach->sgt)

+   return attach->sgt;
+
sg_table = attach->dmabuf->ops->map_dma_buf(attach, direction);
if (!sg_table)
sg_table = ERR_PTR(-ENOMEM);
@@ -657,6 +678,9 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment 
*attach,
if (WARN_ON(!attach || !attach->dmabuf || !sg_table))
return;
  
+	if (attach->sgt == sg_table)

+   return;
+
attach->dmabuf->ops->unmap_dma_buf(attach, sg_table,
direction);
  }
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index 58725f890b5b..52031fdc75bb 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -322,6 +322,7 @@ struct dma_buf_attachment {
struct dma_buf *dmabuf;
struct device *dev;
struct list_head node;
+   struct sg_table *sgt;
void *priv;
  };
  
@@ -373,6 +374,19 @@ static inline void get_dma_buf(struct dma_buf *dmabuf)

get_file(dmabuf->file);
  }
  
+/**

+ * dma_buf_is_dynamic - check if a DMA-buf uses dynamic mappings.
+ * @dmabuf: the DMA-buf to check
+ *
+ * Returns true if a DMA-buf exporter wants to create dynamic sg table mappings
+ * for each attachment. False if only a single static sg table should be used.
+ */
+static inline bool dma_buf_is_dynamic(struct dma_buf *dmabuf)
+{
+   /* Always use a static mapping for now */
+   return false;

Hm I still expect that later on we'll want this to be decided by the
attachment: It's only dynamic if both the exporter and the importer
support dynamic dma-buf management, otherwise we need to pin.


Yeah, essentially we need to handle the following cases:

1. Static DMA-buf and the exporter doesn't want caching.
2. Static DMA-buf and the exporter does want caching.
3. Dynamic DMA-buf and we use caching as a workaround for smooth transition.
4. Dynamic DMA-buf and caching doesn't make much sense.

So far we only support 3 & 4 and not 1 & 2. Could be that this gets much 
more complicated, but I would only want to add this complicated code if 
we really find that we need it.



But anyway, I feel like we need to go over the entire thing anyway once
more when p2p has landed, on this:

Reviewed-by: Daniel Vetter 


Thanks for the review, next question how do we merge this? Through drm-misc?

Regards,
Christian.




+}
+
  struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
struct device *dev);
  void dma_buf_detach(struct dma_buf *dmabuf,
--
2.17.1

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


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

Re: [PATCH 3/9] drm: remove prime sg_table caching

[Christian König]

Am 08.05.19 um 11:57 schrieb Daniel Vetter:

On Tue, May 07, 2019 at 10:13:32AM +0200, Christian König wrote:

That is now done by the DMA-buf helpers instead.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/drm_prime.c | 76 -
  1 file changed, 16 insertions(+), 60 deletions(-)

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 231e3f6d5f41..90f5230cc0d5 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -86,11 +86,6 @@ struct drm_prime_member {
struct rb_node handle_rb;
  };
  
-struct drm_prime_attachment {

-   struct sg_table *sgt;
-   enum dma_data_direction dir;
-};
-
  static int drm_prime_add_buf_handle(struct drm_prime_file_private 
*prime_fpriv,
struct dma_buf *dma_buf, uint32_t handle)
  {
@@ -188,25 +183,16 @@ static int drm_prime_lookup_buf_handle(struct 
drm_prime_file_private *prime_fpri
   * @dma_buf: buffer to attach device to
   * @attach: buffer attachment data
   *
- * Allocates _prime_attachment and calls _driver.gem_prime_pin for
- * device specific attachment. This can be used as the _buf_ops.attach
- * callback.
+ * Calls _driver.gem_prime_pin for device specific handling. This can be
+ * used as the _buf_ops.attach callback.
   *
   * Returns 0 on success, negative error code on failure.
   */
  int drm_gem_map_attach(struct dma_buf *dma_buf,
   struct dma_buf_attachment *attach)
  {
-   struct drm_prime_attachment *prime_attach;
struct drm_gem_object *obj = dma_buf->priv;
  
-	prime_attach = kzalloc(sizeof(*prime_attach), GFP_KERNEL);

-   if (!prime_attach)
-   return -ENOMEM;
-
-   prime_attach->dir = DMA_NONE;
-   attach->priv = prime_attach;
-
return drm_gem_pin(obj);
  }
  EXPORT_SYMBOL(drm_gem_map_attach);
@@ -222,26 +208,8 @@ EXPORT_SYMBOL(drm_gem_map_attach);
  void drm_gem_map_detach(struct dma_buf *dma_buf,
struct dma_buf_attachment *attach)
  {
-   struct drm_prime_attachment *prime_attach = attach->priv;
struct drm_gem_object *obj = dma_buf->priv;
  
-	if (prime_attach) {

-   struct sg_table *sgt = prime_attach->sgt;
-
-   if (sgt) {
-   if (prime_attach->dir != DMA_NONE)
-   dma_unmap_sg_attrs(attach->dev, sgt->sgl,
-  sgt->nents,
-  prime_attach->dir,
-  DMA_ATTR_SKIP_CPU_SYNC);
-   sg_free_table(sgt);
-   }
-
-   kfree(sgt);
-   kfree(prime_attach);
-   attach->priv = NULL;
-   }
-
drm_gem_unpin(obj);
  }
  EXPORT_SYMBOL(drm_gem_map_detach);
@@ -286,39 +254,22 @@ void drm_prime_remove_buf_handle_locked(struct 
drm_prime_file_private *prime_fpr
  struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach,
 enum dma_data_direction dir)
  {
-   struct drm_prime_attachment *prime_attach = attach->priv;
struct drm_gem_object *obj = attach->dmabuf->priv;
struct sg_table *sgt;
  
-	if (WARN_ON(dir == DMA_NONE || !prime_attach))

+   if (WARN_ON(dir == DMA_NONE))
return ERR_PTR(-EINVAL);
  
-	/* return the cached mapping when possible */

-   if (prime_attach->dir == dir)
-   return prime_attach->sgt;
-
-   /*
-* two mappings with different directions for the same attachment are
-* not allowed
-*/
-   if (WARN_ON(prime_attach->dir != DMA_NONE))
-   return ERR_PTR(-EBUSY);
-
if (obj->funcs)
sgt = obj->funcs->get_sg_table(obj);
else
sgt = obj->dev->driver->gem_prime_get_sg_table(obj);
  
-	if (!IS_ERR(sgt)) {

-   if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir,
- DMA_ATTR_SKIP_CPU_SYNC)) {
-   sg_free_table(sgt);
-   kfree(sgt);
-   sgt = ERR_PTR(-ENOMEM);
-   } else {
-   prime_attach->sgt = sgt;
-   prime_attach->dir = dir;
-   }
+   if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir,
+ DMA_ATTR_SKIP_CPU_SYNC)) {
+   sg_free_table(sgt);
+   kfree(sgt);
+   sgt = ERR_PTR(-ENOMEM);
}
  
  	return sgt;

@@ -331,14 +282,19 @@ EXPORT_SYMBOL(drm_gem_map_dma_buf);
   * @sgt: scatterlist info of the buffer to unmap
   * @dir: direction of DMA transfer
   *
- * Not implemented. The unmap is done at drm_gem_map_detach().  This can be
- * used as the _buf_ops.unmap_dma_buf callback.
+ * This can be used as the _buf_ops.unmap_dma_buf callback.
   */
  void drm_gem_unmap_dma_buf(struct 

[Bug 109345] drm-next-2018-12-14 -Linux PPC

[bugzilla-daemon]

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

--- Comment #37 from Christian Zigotzky  ---
Hi All,

Allan has tested the ninth test kernel.

He wrote:

Christian
DRM9 boots to SI card.

ace

--

This step has been marked as bad because the ninth test kernel doesn't boot to
the FirePro.

Next step:

git bisect bad

Output:

Bisecting: 0 revisions left to test after this (roughly 1 step)
[3d42f1ddc47a69c0ce155f9f30d764c4d689a5fa] vgaarb: Keep adding VGA device in
queue

make CROSS_COMPILE=powerpc-linux-gnu- ARCH=powerpc oldconfig

make CROSS_COMPILE=powerpc-linux-gnu- ARCH=powerpc uImage

Download: http://www.xenosoft.de/uImage-drm10

@Allan (acefnq/ace)
Please test it.

Thanks,
Christian

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

[Bug 203525] brightness with function buttons Acer Aspire A315-41 notebook with AMD Ryzen 5 2500U

[bugzilla-daemon]

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

--- Comment #6 from Erik (erikjohans...@flashbox.5july.org) ---
The bisect was working i only didn't trust it.
What causes the change in behavior wasn't caused by resent fixes, well it was
but it wasn't.
What makes it work is bumping SUBLEVEL above 36.
I tried bumping SUBLEVEL of 5.1 and 5.1.36 was broken while 5.1.37 was working.

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

[Bug 202873] (amdgpu) Screen flickering when using a 75Hz monitor paired with an RX 480 GPU

[bugzilla-daemon]

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

--- Comment #7 from Haxk20 (haxk...@gmail.com) ---
Ah sorry this is the old bug where clocks cause flickering. But still try 5.1.
Maybe it will help.

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

[Bug 202873] (amdgpu) Screen flickering when using a 75Hz monitor paired with an RX 480 GPU

[bugzilla-daemon]

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

Haxk20 (haxk...@gmail.com) changed:

   What|Removed |Added

 CC||haxk...@gmail.com

--- Comment #6 from Haxk20 (haxk...@gmail.com) ---
Could you try Kernel 5.1 ?
It has a fix for flickering when Freesync is enabled.

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

Re: [PATCH 1/9] dma-buf: start caching of sg_table objects

[Daniel Vetter]

On Wed, May 08, 2019 at 11:15:33AM +0200, Daniel Vetter wrote:
> On Tue, May 07, 2019 at 10:13:30AM +0200, Christian König wrote:
> > To allow a smooth transition from pinning buffer objects to dynamic
> > invalidation we first start to cache the sg_table for an attachment.
> > 
> > Signed-off-by: Christian König 
> > ---
> >  drivers/dma-buf/dma-buf.c | 24 
> >  include/linux/dma-buf.h   | 14 ++
> >  2 files changed, 38 insertions(+)
> > 
> > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > index 7c858020d14b..775e13f54083 100644
> > --- a/drivers/dma-buf/dma-buf.c
> > +++ b/drivers/dma-buf/dma-buf.c
> > @@ -573,6 +573,20 @@ struct dma_buf_attachment *dma_buf_attach(struct 
> > dma_buf *dmabuf,
> > list_add(>node, >attachments);
> >  
> > mutex_unlock(>lock);
> > +
> > +   if (!dma_buf_is_dynamic(dmabuf)) {
> > +   struct sg_table *sgt;
> > +
> > +   sgt = dmabuf->ops->map_dma_buf(attach, DMA_BIDIRECTIONAL);
> > +   if (!sgt)
> > +   sgt = ERR_PTR(-ENOMEM);
> > +   if (IS_ERR(sgt)) {
> > +   dma_buf_detach(dmabuf, attach);
> > +   return ERR_CAST(sgt);
> > +   }
> > +   attach->sgt = sgt;
> > +   }
> > +
> > return attach;
> >  
> >  err_attach:
> > @@ -595,6 +609,10 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct 
> > dma_buf_attachment *attach)
> > if (WARN_ON(!dmabuf || !attach))
> > return;
> >  
> > +   if (attach->sgt)
> > +   dmabuf->ops->unmap_dma_buf(attach, attach->sgt,
> > +  DMA_BIDIRECTIONAL);
> > +
> > mutex_lock(>lock);
> > list_del(>node);
> > if (dmabuf->ops->detach)
> > @@ -630,6 +648,9 @@ struct sg_table *dma_buf_map_attachment(struct 
> > dma_buf_attachment *attach,
> > if (WARN_ON(!attach || !attach->dmabuf))
> > return ERR_PTR(-EINVAL);
> >  
> > +   if (attach->sgt)
> > +   return attach->sgt;
> > +
> > sg_table = attach->dmabuf->ops->map_dma_buf(attach, direction);
> > if (!sg_table)
> > sg_table = ERR_PTR(-ENOMEM);
> > @@ -657,6 +678,9 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment 
> > *attach,
> > if (WARN_ON(!attach || !attach->dmabuf || !sg_table))
> > return;
> >  
> > +   if (attach->sgt == sg_table)
> > +   return;
> > +
> > attach->dmabuf->ops->unmap_dma_buf(attach, sg_table,
> > direction);
> >  }
> > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> > index 58725f890b5b..52031fdc75bb 100644
> > --- a/include/linux/dma-buf.h
> > +++ b/include/linux/dma-buf.h
> > @@ -322,6 +322,7 @@ struct dma_buf_attachment {
> > struct dma_buf *dmabuf;
> > struct device *dev;
> > struct list_head node;
> > +   struct sg_table *sgt;
> > void *priv;
> >  };
> >  
> > @@ -373,6 +374,19 @@ static inline void get_dma_buf(struct dma_buf *dmabuf)
> > get_file(dmabuf->file);
> >  }
> >  
> > +/**
> > + * dma_buf_is_dynamic - check if a DMA-buf uses dynamic mappings.
> > + * @dmabuf: the DMA-buf to check
> > + *
> > + * Returns true if a DMA-buf exporter wants to create dynamic sg table 
> > mappings
> > + * for each attachment. False if only a single static sg table should be 
> > used.
> > + */
> > +static inline bool dma_buf_is_dynamic(struct dma_buf *dmabuf)
> > +{
> > +   /* Always use a static mapping for now */
> > +   return false;
> 
> Hm I still expect that later on we'll want this to be decided by the
> attachment: It's only dynamic if both the exporter and the importer
> support dynamic dma-buf management, otherwise we need to pin.
> 
> But anyway, I feel like we need to go over the entire thing anyway once
> more when p2p has landed, on this:
> 
> Reviewed-by: Daniel Vetter 

Correction that I only just realized, but need to retract that r-b on this
and the drm sgt cache removal patch: You now hardcode the direction to
DMA_BIDIRECTIONAL, drm_prime did only keep the cache for a given
direction.

Now x86 drivers always set DMA_BIDIRECTIONAL, but arm soc drivers (and
also v4l videobuf layer) try to guess whether it should be DMA_TO_DEVICE
or DMA_FROM_DEVICE. I have no idea what the implications are, also all the
cache coherency on dma-bufs is kinda ill-defined.

And we can't throw the sgt cache away at map time if it doesn't fit like
drm_prime does, because that reintroduces the reservation object lock,
defeating the entire purpose of this. Also we can't just assume drm_prime
works for everyone, since the special cases all roll their own dma-buf
import. I have also not checked what exactly exporters do. No idea what to
do here now.

/me cries
-Daniel

> 
> > +}
> > +
> >  struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
> > struct device *dev);
> >  void dma_buf_detach(struct dma_buf *dmabuf,
> > -- 
> > 2.17.1
> > 
> 

Re: [PATCH 3/9] drm: remove prime sg_table caching

[Daniel Vetter]

On Tue, May 07, 2019 at 10:13:32AM +0200, Christian König wrote:
> That is now done by the DMA-buf helpers instead.
> 
> Signed-off-by: Christian König 
> ---
>  drivers/gpu/drm/drm_prime.c | 76 -
>  1 file changed, 16 insertions(+), 60 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 231e3f6d5f41..90f5230cc0d5 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -86,11 +86,6 @@ struct drm_prime_member {
>   struct rb_node handle_rb;
>  };
>  
> -struct drm_prime_attachment {
> - struct sg_table *sgt;
> - enum dma_data_direction dir;
> -};
> -
>  static int drm_prime_add_buf_handle(struct drm_prime_file_private 
> *prime_fpriv,
>   struct dma_buf *dma_buf, uint32_t handle)
>  {
> @@ -188,25 +183,16 @@ static int drm_prime_lookup_buf_handle(struct 
> drm_prime_file_private *prime_fpri
>   * @dma_buf: buffer to attach device to
>   * @attach: buffer attachment data
>   *
> - * Allocates _prime_attachment and calls _driver.gem_prime_pin for
> - * device specific attachment. This can be used as the _buf_ops.attach
> - * callback.
> + * Calls _driver.gem_prime_pin for device specific handling. This can be
> + * used as the _buf_ops.attach callback.
>   *
>   * Returns 0 on success, negative error code on failure.
>   */
>  int drm_gem_map_attach(struct dma_buf *dma_buf,
>  struct dma_buf_attachment *attach)
>  {
> - struct drm_prime_attachment *prime_attach;
>   struct drm_gem_object *obj = dma_buf->priv;
>  
> - prime_attach = kzalloc(sizeof(*prime_attach), GFP_KERNEL);
> - if (!prime_attach)
> - return -ENOMEM;
> -
> - prime_attach->dir = DMA_NONE;
> - attach->priv = prime_attach;
> -
>   return drm_gem_pin(obj);
>  }
>  EXPORT_SYMBOL(drm_gem_map_attach);
> @@ -222,26 +208,8 @@ EXPORT_SYMBOL(drm_gem_map_attach);
>  void drm_gem_map_detach(struct dma_buf *dma_buf,
>   struct dma_buf_attachment *attach)
>  {
> - struct drm_prime_attachment *prime_attach = attach->priv;
>   struct drm_gem_object *obj = dma_buf->priv;
>  
> - if (prime_attach) {
> - struct sg_table *sgt = prime_attach->sgt;
> -
> - if (sgt) {
> - if (prime_attach->dir != DMA_NONE)
> - dma_unmap_sg_attrs(attach->dev, sgt->sgl,
> -sgt->nents,
> -prime_attach->dir,
> -DMA_ATTR_SKIP_CPU_SYNC);
> - sg_free_table(sgt);
> - }
> -
> - kfree(sgt);
> - kfree(prime_attach);
> - attach->priv = NULL;
> - }
> -
>   drm_gem_unpin(obj);
>  }
>  EXPORT_SYMBOL(drm_gem_map_detach);
> @@ -286,39 +254,22 @@ void drm_prime_remove_buf_handle_locked(struct 
> drm_prime_file_private *prime_fpr
>  struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach,
>enum dma_data_direction dir)
>  {
> - struct drm_prime_attachment *prime_attach = attach->priv;
>   struct drm_gem_object *obj = attach->dmabuf->priv;
>   struct sg_table *sgt;
>  
> - if (WARN_ON(dir == DMA_NONE || !prime_attach))
> + if (WARN_ON(dir == DMA_NONE))
>   return ERR_PTR(-EINVAL);
>  
> - /* return the cached mapping when possible */
> - if (prime_attach->dir == dir)
> - return prime_attach->sgt;
> -
> - /*
> -  * two mappings with different directions for the same attachment are
> -  * not allowed
> -  */
> - if (WARN_ON(prime_attach->dir != DMA_NONE))
> - return ERR_PTR(-EBUSY);
> -
>   if (obj->funcs)
>   sgt = obj->funcs->get_sg_table(obj);
>   else
>   sgt = obj->dev->driver->gem_prime_get_sg_table(obj);
>  
> - if (!IS_ERR(sgt)) {
> - if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir,
> -   DMA_ATTR_SKIP_CPU_SYNC)) {
> - sg_free_table(sgt);
> - kfree(sgt);
> - sgt = ERR_PTR(-ENOMEM);
> - } else {
> - prime_attach->sgt = sgt;
> - prime_attach->dir = dir;
> - }
> + if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir,
> +   DMA_ATTR_SKIP_CPU_SYNC)) {
> + sg_free_table(sgt);
> + kfree(sgt);
> + sgt = ERR_PTR(-ENOMEM);
>   }
>  
>   return sgt;
> @@ -331,14 +282,19 @@ EXPORT_SYMBOL(drm_gem_map_dma_buf);
>   * @sgt: scatterlist info of the buffer to unmap
>   * @dir: direction of DMA transfer
>   *
> - * Not implemented. The unmap is done at drm_gem_map_detach().  This can be
> - * used as the _buf_ops.unmap_dma_buf callback.
> + * This can be used as