[Bug 196615] amdgpu - resume from suspend is no longer working on rx480

[bugzilla-daemon]

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

cont...@florentflament.com changed:

   What|Removed |Added

 CC||cont...@florentflament.com

--- Comment #25 from cont...@florentflament.com ---
Hi,
Same issue here. OS freezing after resume from suspend with an AMD RX480 GPU.

$ cat /etc/redhat-release 
Fedora release 26 (Twenty Six)
$ uname -a
Linux amn 4.13.4-200.fc26.x86_64 #1 SMP Thu Sep 28 20:46:39 UTC 2017 x86_64
x86_64 x86_64 GNU/Linux
$ journalctl -k -b -2 | grep amdgpu | tail -5
Oct 12 01:46:06 amn kernel: [drm] Initialized amdgpu 3.18.0 20150101 for
:01:00.0 on minor 0
Oct 12 23:16:29 amn kernel: [drm:amdgpu_vce_ring_test_ring [amdgpu]] *ERROR*
amdgpu: ring 14 test failed
Oct 12 23:16:29 amn kernel: [drm:amdgpu_resume_phase2 [amdgpu]] *ERROR* resume
of IP block  failed -110
Oct 12 23:16:29 amn kernel: [drm:amdgpu_device_resume [amdgpu]] *ERROR*
amdgpu_resume failed (-110).
Oct 12 23:16:30 amn kernel: amdgpu :01:00.0: 9514d9161800 unpin not
necessary

Regards

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

[git pull] drm fixes for v4.14-rc5

[Dave Airlie]

Hi Linus,

Couple of the arm people seem to wake up so this has imx and msm
fixes, along with a bunch of i915 stable bounds fixes and an amdgpu
regression fix.

All seems pretty okay for now.

Dave.

The following changes since commit 8a5776a5f49812d29fe4b2d0a2d71675c3facf3f:

  Linux 4.14-rc4 (2017-10-08 20:53:29 -0700)

are available in the git repository at:

  git://people.freedesktop.org/~airlied/linux tags/drm-fixes-for-v4.14-rc5

for you to fetch changes up to a480f30846d19b50106b3243d9d48683d2966249:

  Merge tag 'drm-intel-fixes-2017-10-11' of
git://anongit.freedesktop.org/drm/drm-intel into drm-fixes (2017-10-14
09:59:20 +1000)


drm: msm, i915, amdgpu, imx and sync_file fixes


Archit Taneja (2):
  drm/msm/dsi: Use correct pm_runtime_put variant during host_init
  drm/msm/mdp5: Remove extra pm_runtime_put call in mdp5_crtc_cursor_set()

Chris Wilson (2):
  drm/i915: Silence compiler warning for hsw_power_well_enable()
  drm/i915: Order two completing nop_submit_request

Christian König (1):
  drm/amdgpu: fix placement flags in amdgpu_ttm_bind

Dave Airlie (5):
  Merge tag 'drm-misc-fixes-2017-10-11' of
git://anongit.freedesktop.org/drm/drm-misc into drm-fixes
  Merge branch 'drm-fixes-4.14' of
git://people.freedesktop.org/~agd5f/linux into drm-fixes
  Merge tag 'imx-drm-fixes-2017-10-12' of
git://git.pengutronix.de/git/pza/linux into drm-fixes
  Merge branch 'msm-fixes-4.14-rc4' of
git://people.freedesktop.org/~robclark/linux into drm-fixes
  Merge tag 'drm-intel-fixes-2017-10-11' of
git://anongit.freedesktop.org/drm/drm-intel into drm-fixes

Jani Nikula (1):
  drm/i915/bios: parse DDI ports also for CHV for HDMI DDC pin and
DP AUX channel

Jeffy Chen (1):
  drm/atomic: Unref duplicated drm_atomic_state in
drm_atomic_helper_resume()

John Einar Reitan (1):
  sync_file: Return consistent status in SYNC_IOC_FILE_INFO

Lucas Stach (2):
  gpu: ipu-v3: prg: wait for double buffers to be filled on channel startup
  gpu: ipu-v3: pre: implement workaround for ERR009624

Maarten Lankhorst (1):
  drm/i915: Use crtc_state_is_legacy_gamma in intel_color_check

Manasi Navare (2):
  drm/i915/edp: Get the Panel Power Off timestamp after panel is off
  drm/i915/edp: Increase the T12 delay quirk to 1300ms

Philipp Zabel (1):
  gpu: ipu-v3: Allow channel burst locking on i.MX6 only

Rob Clark (4):
  drm/msm/mdp5: add missing max size for 8x74 v1
  drm/msm: use proper memory barriers for updating tail/head
  drm/msm: fix error path cleanup
  drm/msm: fix _NO_IMPLICIT fencing case

Ville Syrjälä (1):
  drm/i915: Read timings from the correct transcoder in
intel_crtc_mode_get()

Wei Yongjun (1):
  drm/msm: fix return value check in _msm_gem_kernel_new()

 drivers/dma-buf/sync_file.c  | 17 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c  |  2 +-
 drivers/gpu/drm/drm_atomic_helper.c  |  1 +
 drivers/gpu/drm/i915/i915_gem.c  |  7 ++-
 drivers/gpu/drm/i915/intel_bios.c|  2 +-
 drivers/gpu/drm/i915/intel_color.c   | 16 +++-
 drivers/gpu/drm/i915/intel_display.c | 14 +-
 drivers/gpu/drm/i915/intel_dp.c  |  4 ++--
 drivers/gpu/drm/i915/intel_runtime_pm.c  |  2 +-
 drivers/gpu/drm/msm/dsi/dsi_host.c   |  2 +-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c  |  2 ++
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c |  2 --
 drivers/gpu/drm/msm/msm_gem.c| 15 ++-
 drivers/gpu/drm/msm/msm_gem_submit.c | 24 ++--
 drivers/gpu/drm/msm/msm_gpu.c|  3 ++-
 drivers/gpu/drm/msm/msm_rd.c | 12 ++--
 drivers/gpu/ipu-v3/ipu-common.c  |  8 
 drivers/gpu/ipu-v3/ipu-pre.c | 29 +
 drivers/gpu/ipu-v3/ipu-prg.c |  7 +++
 19 files changed, 119 insertions(+), 50 deletions(-)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 1/6] drm/amd/display: Use DRM new-style object iterators.

[Harry Wentland]

On 2017-10-13 03:29 PM, sunpeng...@amd.com wrote:
> From: "Leo (Sunpeng) Li" 
> 
> Use the correct for_each_new/old_* iterators instead of for_each_*
> 
> The following functions were considered:
> 
> amdgpu_dm_find_first_crtc_matching_connector: use for_each_new
> - Old from_state_var flag was always choosing the new state
> 
> amdgpu_dm_display_resume: use for_each_new
> - drm_atomic_helper_duplicate_state is called during suspend to
>   cache the state
> - It sets 'state' within the state triplet to 'new_state'
> 
> amdgpu_dm_commit_planes: use for_each_old
> - Called after the state was swapped (via atomic commit tail)
> 
> amdgpu_dm_atomic_commit: use for_each_new
> - Called before the state is swapped
> 
> amdgpu_dm_atomic_commit_tail: use for_each_old
> - Called after the state was swapped
> 
> dm_update_crtcs_state: use for_each_new
> - Called before the state is swapped (via atomic check)
> 
> amdgpu_dm_atomic_check: use for_each_new
> - Called before the state is swapped
> 
> v2: Split out typo fixes to a new patch.
> 
> v3: Say "functions considered" instead of "affected functions". The
> latter implies that changes are made to each.
> 
> Signed-off-by: Leo (Sunpeng) Li 

Patches 1-2 (v3) are also
Reviewed-by: Harry Wentland 

Harry

> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  3 +--
>  2 files changed, 12 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 9bfe1f9..cc024ab 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -570,23 +570,15 @@ static int dm_suspend(void *handle)
>  
>  struct amdgpu_dm_connector *amdgpu_dm_find_first_crct_matching_connector(
>   struct drm_atomic_state *state,
> - struct drm_crtc *crtc,
> - bool from_state_var)
> + struct drm_crtc *crtc)
>  {
>   uint32_t i;
>   struct drm_connector_state *conn_state;
>   struct drm_connector *connector;
>   struct drm_crtc *crtc_from_state;
>  
> - for_each_new_connector_in_state(
> - state,
> - connector,
> - conn_state,
> - i) {
> - crtc_from_state =
> - from_state_var ?
> - conn_state->crtc :
> - connector->state->crtc;
> + for_each_new_connector_in_state(state, connector, conn_state, i) {
> + crtc_from_state = conn_state->crtc;
>  
>   if (crtc_from_state == crtc)
>   return to_amdgpu_dm_connector(connector);
> @@ -3890,7 +3882,7 @@ static void amdgpu_dm_commit_planes(struct 
> drm_atomic_state *state,
>   unsigned long flags;
>  
>   /* update planes when needed */
> - for_each_new_plane_in_state(state, plane, old_plane_state, i) {
> + for_each_old_plane_in_state(state, plane, old_plane_state, i) {
>   struct drm_plane_state *plane_state = plane->state;
>   struct drm_crtc *crtc = plane_state->crtc;
>   struct drm_framebuffer *fb = plane_state->fb;
> @@ -4024,7 +4016,7 @@ void amdgpu_dm_atomic_commit_tail(
>   dm_state = to_dm_atomic_state(state);
>  
>   /* update changed items */
> - for_each_new_crtc_in_state(state, crtc, old_crtc_state, i) {
> + for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) {
>   struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
>   struct drm_crtc_state *new_state = crtc->state;
>  
> @@ -4113,11 +4105,9 @@ void amdgpu_dm_atomic_commit_tail(
>   new_acrtc_state = 
> to_dm_crtc_state(new_crtcs[i]->base.state);
>  
>   new_stream = new_acrtc_state->stream;
> - aconnector =
> - amdgpu_dm_find_first_crct_matching_connector(
> + aconnector = 
> amdgpu_dm_find_first_crct_matching_connector(
>   state,
> - _crtcs[i]->base,
> - false);
> + _crtcs[i]->base);
>   if (!aconnector) {
>   DRM_DEBUG_DRIVER("Atomic commit: Failed to find 
> connector for acrtc id:%d "
>"skipping freesync init\n",
> @@ -4151,7 +4141,7 @@ void amdgpu_dm_atomic_commit_tail(
>   }
>  
>   /* Handle scaling and undersacn changes*/
> - for_each_new_connector_in_state(state, connector, old_conn_state, i) {
> + for_each_old_connector_in_state(state, connector, old_conn_state, i) {
>   struct amdgpu_dm_connector *aconnector = 
> to_amdgpu_dm_connector(connector);
> 

Re: [Outreachy kernel] [PATCH v13 2/3] drm/tinydrm: Move tinydrm_of_find_backlight to backlight.c

[Sean Paul]

On Fri, Oct 13, 2017 at 04:11:43PM +0530, Meghana Madhyastha wrote:
> Rename tinydrm_of_find_backlight to backlight_get and move it
> to linux/backlight.c so that it can be used by other drivers.

[apologies if this has been brought up in previous versions, I haven't been
following closely]

I don't think "backlight_get" is a good name for this function. How about
of_find_backlight_by_name (since there's already of_find_backlight_by_node)?

> 
> Signed-off-by: Meghana Madhyastha 
> ---
> Changes in v13:
>  -Add backlight_put to backlight.h in this patch
> 
>  drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 40 
> --
>  drivers/gpu/drm/tinydrm/mi0283qt.c |  3 +-
>  drivers/video/backlight/backlight.c| 37 
>  include/drm/tinydrm/tinydrm-helpers.h  |  2 --
>  include/linux/backlight.h  | 19 
>  5 files changed, 58 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c 
> b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
> index a42dee6..cb1a01a 100644
> --- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
> +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
> @@ -236,46 +236,6 @@ void tinydrm_xrgb_to_gray8(u8 *dst, void *vaddr, 
> struct drm_framebuffer *fb,
>  }
>  EXPORT_SYMBOL(tinydrm_xrgb_to_gray8);
>  
> -/**
> - * tinydrm_of_find_backlight - Find backlight device in device-tree
> - * @dev: Device
> - *
> - * This function looks for a DT node pointed to by a property named 
> 'backlight'
> - * and uses of_find_backlight_by_node() to get the backlight device.
> - * Additionally if the brightness property is zero, it is set to
> - * max_brightness.
> - *
> - * Returns:
> - * NULL if there's no backlight property.
> - * Error pointer -EPROBE_DEFER if the DT node is found, but no backlight 
> device
> - * is found.
> - * If the backlight device is found, a pointer to the structure is returned.
> - */
> -struct backlight_device *tinydrm_of_find_backlight(struct device *dev)
> -{
> - struct backlight_device *backlight;
> - struct device_node *np;
> -
> - np = of_parse_phandle(dev->of_node, "backlight", 0);
> - if (!np)
> - return NULL;
> -
> - backlight = of_find_backlight_by_node(np);
> - of_node_put(np);
> -
> - if (!backlight)
> - return ERR_PTR(-EPROBE_DEFER);
> -
> - if (!backlight->props.brightness) {
> - backlight->props.brightness = backlight->props.max_brightness;
> - DRM_DEBUG_KMS("Backlight brightness set to %d\n",
> -   backlight->props.brightness);
> - }
> -
> - return backlight;
> -}
> -EXPORT_SYMBOL(tinydrm_of_find_backlight);
> -
>  #if IS_ENABLED(CONFIG_SPI)
>  
>  /**
> diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c 
> b/drivers/gpu/drm/tinydrm/mi0283qt.c
> index 7fd2691..a932185 100644
> --- a/drivers/gpu/drm/tinydrm/mi0283qt.c
> +++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
> @@ -12,6 +12,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -188,7 +189,7 @@ static int mi0283qt_probe(struct spi_device *spi)
>   if (IS_ERR(mipi->regulator))
>   return PTR_ERR(mipi->regulator);
>  
> - mipi->backlight = tinydrm_of_find_backlight(dev);
> + mipi->backlight = backlight_get(dev);
>   if (IS_ERR(mipi->backlight))
>   return PTR_ERR(mipi->backlight);
>  
> diff --git a/drivers/video/backlight/backlight.c 
> b/drivers/video/backlight/backlight.c
> index 8049e76..c4e94d0 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -580,6 +580,43 @@ struct backlight_device 
> *of_find_backlight_by_node(struct device_node *node)
>  EXPORT_SYMBOL(of_find_backlight_by_node);
>  #endif
>  
> +/**
> + * backlight_get - Get backlight device
> + * @dev: Device
> + *
> + * This function looks for a property named 'backlight' on the DT node
> + * connected to @dev and looks up the backlight device.
> + *
> + * Call backlight_put() to drop the reference on the backlight device.
> + *
> + * Returns:
> + * A pointer to the backlight device if found.
> + * Error pointer -EPROBE_DEFER if the DT property is set, but no backlight
> + * device is found.
> + * NULL if there's no backlight property.
> + */
> +struct backlight_device *backlight_get(struct device *dev)
> +{
> + struct backlight_device *bd = NULL;
> + struct device_node *np;
> +
> + if (!dev)
> + return NULL;
> +
> + if (IS_ENABLED(CONFIG_OF) && dev->of_node) {

I'm not really used to seeing IS_ENABLED(CONFIG_BLAH) inline. The common
patterns seems to be wrapping the actual implementation in #if
IS_ENABLED(CONFIG_BLAH) and then sticking a stub implementation in the #else.

I see below that you already have a stub if backlight is not enabled, so expand
that #if to include CONFIG_OF as well.

> + np 

Re: [PATCH v3 1/6] drm/amd/display: Use DRM new-style object iterators.

[Andrey Grodzovsky]

On 10/13/2017 03:29 PM, sunpeng...@amd.com wrote:

From: "Leo (Sunpeng) Li" 

Use the correct for_each_new/old_* iterators instead of for_each_*

The following functions were considered:

amdgpu_dm_find_first_crtc_matching_connector: use for_each_new
 - Old from_state_var flag was always choosing the new state

amdgpu_dm_display_resume: use for_each_new
 - drm_atomic_helper_duplicate_state is called during suspend to
   cache the state
 - It sets 'state' within the state triplet to 'new_state'

amdgpu_dm_commit_planes: use for_each_old
 - Called after the state was swapped (via atomic commit tail)

amdgpu_dm_atomic_commit: use for_each_new
 - Called before the state is swapped

amdgpu_dm_atomic_commit_tail: use for_each_old
 - Called after the state was swapped

dm_update_crtcs_state: use for_each_new
 - Called before the state is swapped (via atomic check)

amdgpu_dm_atomic_check: use for_each_new
 - Called before the state is swapped

v2: Split out typo fixes to a new patch.

v3: Say "functions considered" instead of "affected functions". The
 latter implies that changes are made to each.

Signed-off-by: Leo (Sunpeng) Li 
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 ---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  3 +--
  2 files changed, 12 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 9bfe1f9..cc024ab 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -570,23 +570,15 @@ static int dm_suspend(void *handle)
  
  struct amdgpu_dm_connector *amdgpu_dm_find_first_crct_matching_connector(

struct drm_atomic_state *state,
-   struct drm_crtc *crtc,
-   bool from_state_var)
+   struct drm_crtc *crtc)
  {
uint32_t i;
struct drm_connector_state *conn_state;
struct drm_connector *connector;
struct drm_crtc *crtc_from_state;
  
-	for_each_new_connector_in_state(

-   state,
-   connector,
-   conn_state,
-   i) {
-   crtc_from_state =
-   from_state_var ?
-   conn_state->crtc :
-   connector->state->crtc;
+   for_each_new_connector_in_state(state, connector, conn_state, i) {
+   crtc_from_state = conn_state->crtc;
  
  		if (crtc_from_state == crtc)

return to_amdgpu_dm_connector(connector);
@@ -3890,7 +3882,7 @@ static void amdgpu_dm_commit_planes(struct 
drm_atomic_state *state,
unsigned long flags;
  
  	/* update planes when needed */

-   for_each_new_plane_in_state(state, plane, old_plane_state, i) {
+   for_each_old_plane_in_state(state, plane, old_plane_state, i) {
struct drm_plane_state *plane_state = plane->state;
struct drm_crtc *crtc = plane_state->crtc;
struct drm_framebuffer *fb = plane_state->fb;
@@ -4024,7 +4016,7 @@ void amdgpu_dm_atomic_commit_tail(
dm_state = to_dm_atomic_state(state);
  
  	/* update changed items */

-   for_each_new_crtc_in_state(state, crtc, old_crtc_state, i) {
+   for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) {


Better just use for_each_new_old here, so you can get both old and new 
from the iterator.



struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
struct drm_crtc_state *new_state = crtc->state;
  
@@ -4113,11 +4105,9 @@ void amdgpu_dm_atomic_commit_tail(

new_acrtc_state = 
to_dm_crtc_state(new_crtcs[i]->base.state);
  
  			new_stream = new_acrtc_state->stream;

-   aconnector =
-   amdgpu_dm_find_first_crct_matching_connector(
+   aconnector = 
amdgpu_dm_find_first_crct_matching_connector(
state,
-   _crtcs[i]->base,
-   false);
+   _crtcs[i]->base);
if (!aconnector) {
DRM_DEBUG_DRIVER("Atomic commit: Failed to find 
connector for acrtc id:%d "
 "skipping freesync init\n",
@@ -4151,7 +4141,7 @@ void amdgpu_dm_atomic_commit_tail(
}
  
  	/* Handle scaling and undersacn changes*/

-   for_each_new_connector_in_state(state, connector, old_conn_state, i) {
+   for_each_old_connector_in_state(state, connector, old_conn_state, i) {
struct amdgpu_dm_connector *aconnector = 
to_amdgpu_dm_connector(connector);
struct dm_connector_state *con_new_state =
to_dm_connector_state(aconnector->base.state);
@@ -4205,7 

Re: [PATCH v3 1/6] drm/amd/display: Use DRM new-style object iterators.

[Leo]

On 2017-10-13 04:36 PM, Andrey Grodzovsky wrote:



On 10/13/2017 03:29 PM, sunpeng...@amd.com wrote:

From: "Leo (Sunpeng) Li" 

Use the correct for_each_new/old_* iterators instead of for_each_*

The following functions were considered:

amdgpu_dm_find_first_crtc_matching_connector: use for_each_new
 - Old from_state_var flag was always choosing the new state

amdgpu_dm_display_resume: use for_each_new
 - drm_atomic_helper_duplicate_state is called during suspend to
   cache the state
 - It sets 'state' within the state triplet to 'new_state'

amdgpu_dm_commit_planes: use for_each_old
 - Called after the state was swapped (via atomic commit tail)

amdgpu_dm_atomic_commit: use for_each_new
 - Called before the state is swapped

amdgpu_dm_atomic_commit_tail: use for_each_old
 - Called after the state was swapped

dm_update_crtcs_state: use for_each_new
 - Called before the state is swapped (via atomic check)

amdgpu_dm_atomic_check: use for_each_new
 - Called before the state is swapped

v2: Split out typo fixes to a new patch.

v3: Say "functions considered" instead of "affected functions". The
 latter implies that changes are made to each.

Signed-off-by: Leo (Sunpeng) Li 
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 
---

  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  3 +--
  2 files changed, 12 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c

index 9bfe1f9..cc024ab 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -570,23 +570,15 @@ static int dm_suspend(void *handle)
  struct amdgpu_dm_connector 
*amdgpu_dm_find_first_crct_matching_connector(

  struct drm_atomic_state *state,
-    struct drm_crtc *crtc,
-    bool from_state_var)
+    struct drm_crtc *crtc)
  {
  uint32_t i;
  struct drm_connector_state *conn_state;
  struct drm_connector *connector;
  struct drm_crtc *crtc_from_state;
-    for_each_new_connector_in_state(
-    state,
-    connector,
-    conn_state,
-    i) {
-    crtc_from_state =
-    from_state_var ?
-    conn_state->crtc :
-    connector->state->crtc;
+    for_each_new_connector_in_state(state, connector, conn_state, i) {
+    crtc_from_state = conn_state->crtc;
  if (crtc_from_state == crtc)
  return to_amdgpu_dm_connector(connector);
@@ -3890,7 +3882,7 @@ static void amdgpu_dm_commit_planes(struct 
drm_atomic_state *state,

  unsigned long flags;
  /* update planes when needed */
-    for_each_new_plane_in_state(state, plane, old_plane_state, i) {
+    for_each_old_plane_in_state(state, plane, old_plane_state, i) {
  struct drm_plane_state *plane_state = plane->state;
  struct drm_crtc *crtc = plane_state->crtc;
  struct drm_framebuffer *fb = plane_state->fb;
@@ -4024,7 +4016,7 @@ void amdgpu_dm_atomic_commit_tail(
  dm_state = to_dm_atomic_state(state);
  /* update changed items */
-    for_each_new_crtc_in_state(state, crtc, old_crtc_state, i) {
+    for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) {


Better just use for_each_new_old here, so you can get both old and new 
from the iterator.



  struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
  struct drm_crtc_state *new_state = crtc->state;
@@ -4113,11 +4105,9 @@ void amdgpu_dm_atomic_commit_tail(
  new_acrtc_state = 
to_dm_crtc_state(new_crtcs[i]->base.state);

  new_stream = new_acrtc_state->stream;
-    aconnector =
-    amdgpu_dm_find_first_crct_matching_connector(
+    aconnector = amdgpu_dm_find_first_crct_matching_connector(
  state,
-    _crtcs[i]->base,
-    false);
+    _crtcs[i]->base);
  if (!aconnector) {
  DRM_DEBUG_DRIVER("Atomic commit: Failed to find 
connector for acrtc id:%d "

   "skipping freesync init\n",
@@ -4151,7 +4141,7 @@ void amdgpu_dm_atomic_commit_tail(
  }
  /* Handle scaling and undersacn changes*/
-    for_each_new_connector_in_state(state, connector, old_conn_state, 
i) {
+    for_each_old_connector_in_state(state, connector, old_conn_state, 
i) {
  struct amdgpu_dm_connector *aconnector = 
to_amdgpu_dm_connector(connector);

  struct dm_connector_state *con_new_state =
  to_dm_connector_state(aconnector->base.state);
@@ -4205,7 +4195,7 @@ void amdgpu_dm_atomic_commit_tail(
  }
  /* update planes when needed per crtc*/
-    for_each_new_crtc_in_state(state, pcrtc, old_crtc_state, j) {
+    for_each_old_crtc_in_state(state, pcrtc, old_crtc_state, j) {
  new_acrtc_state = to_dm_crtc_state(pcrtc->state);


Why did you 

Re: [PATCH v3 1/6] drm/amd/display: Use DRM new-style object iterators.

[Andrey Grodzovsky]

On 10/13/2017 05:01 PM, Leo wrote:



On 2017-10-13 04:36 PM, Andrey Grodzovsky wrote:



On 10/13/2017 03:29 PM, sunpeng...@amd.com wrote:

From: "Leo (Sunpeng) Li" 

Use the correct for_each_new/old_* iterators instead of for_each_*

The following functions were considered:

amdgpu_dm_find_first_crtc_matching_connector: use for_each_new
 - Old from_state_var flag was always choosing the new state

amdgpu_dm_display_resume: use for_each_new
 - drm_atomic_helper_duplicate_state is called during suspend to
   cache the state
 - It sets 'state' within the state triplet to 'new_state'

amdgpu_dm_commit_planes: use for_each_old
 - Called after the state was swapped (via atomic commit tail)

amdgpu_dm_atomic_commit: use for_each_new
 - Called before the state is swapped

amdgpu_dm_atomic_commit_tail: use for_each_old
 - Called after the state was swapped

dm_update_crtcs_state: use for_each_new
 - Called before the state is swapped (via atomic check)

amdgpu_dm_atomic_check: use for_each_new
 - Called before the state is swapped

v2: Split out typo fixes to a new patch.

v3: Say "functions considered" instead of "affected functions". The
 latter implies that changes are made to each.

Signed-off-by: Leo (Sunpeng) Li 
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 
---

  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  3 +--
  2 files changed, 12 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c

index 9bfe1f9..cc024ab 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -570,23 +570,15 @@ static int dm_suspend(void *handle)
  struct amdgpu_dm_connector 
*amdgpu_dm_find_first_crct_matching_connector(

  struct drm_atomic_state *state,
-struct drm_crtc *crtc,
-bool from_state_var)
+struct drm_crtc *crtc)
  {
  uint32_t i;
  struct drm_connector_state *conn_state;
  struct drm_connector *connector;
  struct drm_crtc *crtc_from_state;
-for_each_new_connector_in_state(
-state,
-connector,
-conn_state,
-i) {
-crtc_from_state =
-from_state_var ?
-conn_state->crtc :
-connector->state->crtc;
+for_each_new_connector_in_state(state, connector, conn_state, i) {
+crtc_from_state = conn_state->crtc;
  if (crtc_from_state == crtc)
  return to_amdgpu_dm_connector(connector);
@@ -3890,7 +3882,7 @@ static void amdgpu_dm_commit_planes(struct 
drm_atomic_state *state,

  unsigned long flags;
  /* update planes when needed */
-for_each_new_plane_in_state(state, plane, old_plane_state, i) {
+for_each_old_plane_in_state(state, plane, old_plane_state, i) {
  struct drm_plane_state *plane_state = plane->state;
  struct drm_crtc *crtc = plane_state->crtc;
  struct drm_framebuffer *fb = plane_state->fb;
@@ -4024,7 +4016,7 @@ void amdgpu_dm_atomic_commit_tail(
  dm_state = to_dm_atomic_state(state);
  /* update changed items */
-for_each_new_crtc_in_state(state, crtc, old_crtc_state, i) {
+for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) {


Better just use for_each_new_old here, so you can get both old and 
new from the iterator.



  struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
  struct drm_crtc_state *new_state = crtc->state;
@@ -4113,11 +4105,9 @@ void amdgpu_dm_atomic_commit_tail(
  new_acrtc_state = 
to_dm_crtc_state(new_crtcs[i]->base.state);

  new_stream = new_acrtc_state->stream;
-aconnector =
-amdgpu_dm_find_first_crct_matching_connector(
+aconnector = amdgpu_dm_find_first_crct_matching_connector(
  state,
-_crtcs[i]->base,
-false);
+_crtcs[i]->base);
  if (!aconnector) {
  DRM_DEBUG_DRIVER("Atomic commit: Failed to find 
connector for acrtc id:%d "

   "skipping freesync init\n",
@@ -4151,7 +4141,7 @@ void amdgpu_dm_atomic_commit_tail(
  }
  /* Handle scaling and undersacn changes*/
-for_each_new_connector_in_state(state, connector, 
old_conn_state, i) {
+for_each_old_connector_in_state(state, connector, 
old_conn_state, i) {
  struct amdgpu_dm_connector *aconnector = 
to_amdgpu_dm_connector(connector);

  struct dm_connector_state *con_new_state =
to_dm_connector_state(aconnector->base.state);
@@ -4205,7 +4195,7 @@ void amdgpu_dm_atomic_commit_tail(
  }
  /* update planes when needed per crtc*/
-for_each_new_crtc_in_state(state, pcrtc, old_crtc_state, j) {
+for_each_old_crtc_in_state(state, pcrtc, old_crtc_state, j) {
  new_acrtc_state = 

Re: [PATCH 3/9] drm/panel: simple: make it possible to override LCD bus format

[Rob Herring]

On Wed, Oct 11, 2017 at 01:23:35PM +0200, Lothar Waßmann wrote:
> The baseboards for the Ka-Ro electronics series of i.MX modules
> use a 24bit LCD interface, no matter what LCD bus width the SoC on the
> module provides and what the LCD panel expects. LCDs with 6bit per color
> will ignore the 2 LSBs of each color lane, and modules using a SoC
> that provides only 6bit per color, drive the display information on the
> 6 MSBs of each color lane and tie the 2 LSBs of each color lane to GND.
> 
> Thus, no matter what combination of LCD and SoC is used, the LCD port
> can be used without shuffling bit lanes by always configuring the LCD
> output to 24bit mode.

Thanks for providing good reasoning as to why this is needed.

> 
> Add a function to handle certain quirks of the LCD interface to the
> panel driver to be able to override the bus format specified in a
> panel's display_mode.
> 
> Signed-off-by: Lothar Waßmann 
> ---
>  .../bindings/display/panel/simple-panel.txt|  2 ++
>  drivers/gpu/drm/panel/panel-simple.c   | 40 
> +-
>  2 files changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/simple-panel.txt 
> b/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> index 1341bbf..e2308c3 100644
> --- a/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> +++ b/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> @@ -7,6 +7,8 @@ Optional properties:
>  - ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
>  - enable-gpios: GPIO pin to enable or disable the panel
>  - backlight: phandle of the backlight device attached to the panel
> +- bus-format-override: override the bus_format setting of the panel's
> +  display_mode settings

You need to define valid values here.

However, we also have this proposal[1]. Please align to a common one. 
BTW, we don't need another common panel binding file that [1] added. We 
already have panel-dpi.txt for parallel interface panels. And 
personally, I'd like to see simple-panel.txt disappear.

Rob

[1] https://patchwork.ozlabs.org/patch/823104/
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 196125] [Regression] AMD Radeon RX 480 flickering on HDMI port with Linux 4.11.4

[bugzilla-daemon]

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

--- Comment #7 from cont...@florentflament.com ---
I haven't experienced any flickering for a while (be it with Wayland or Xorg,
one monitor connected to a HDMI port and one another to a DisplayPort). This
seems to have been fixed.

$ cat /etc/fedora-release 
Fedora release 26 (Twenty Six)
$ uname -a
Linux amn 4.13.5-200.fc26.x86_64 #1 SMP Thu Oct 5 16:53:13 UTC 2017 x86_64
x86_64 x86_64 GNU/Linux

-- 
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 102820] [bisected] commit ebbf7337e2daacacef3e01114e6be68a2a4f11b4 prevents X11 from starting

[bugzilla-daemon]

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

--- Comment #6 from dwagner  ---
Just as an update: This very bug still occurs with
https://cgit.freedesktop.org/~agd5f/linux/log/?h=amd-staging-drm-next as of
today, and it is still fixed by reverting commit
ebbf7337e2daacacef3e01114e6be68a2a4f11b4

Could somebody comment what this commit is good for, given that it seems to
only prevent X11 from running with certain 4k HDMI displays?

-- 
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: [Outreachy kernel] [PATCH v13 2/3] drm/tinydrm: Move tinydrm_of_find_backlight to backlight.c

[Noralf Trønnes]

Den 13.10.2017 22.25, skrev Sean Paul:

On Fri, Oct 13, 2017 at 04:11:43PM +0530, Meghana Madhyastha wrote:

Rename tinydrm_of_find_backlight to backlight_get and move it
to linux/backlight.c so that it can be used by other drivers.

[apologies if this has been brought up in previous versions, I haven't been
following closely]

I don't think "backlight_get" is a good name for this function. How about
of_find_backlight_by_name (since there's already of_find_backlight_by_node)?


I came up with that name modelled after gpiod_get() and gpiod_put() and I
deliberately kept the of_ part out of the name like the gpio functions.
gpiod_get() checks OF, ACPI and platform for gpios and calling it
backlight_get() would keep the door open for other ways of connecting
backlight devices in the future, other than Device Tree.

I think of_find_backlight() would be better than *_by_name(), since
'backlight' is the common DT property name, so it wouldn't make much sense
to require every caller to pass in the same name.

Either way is fine with me.

Noralf.



Signed-off-by: Meghana Madhyastha 
---
Changes in v13:
  -Add backlight_put to backlight.h in this patch

  drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 40 --
  drivers/gpu/drm/tinydrm/mi0283qt.c |  3 +-
  drivers/video/backlight/backlight.c| 37 
  include/drm/tinydrm/tinydrm-helpers.h  |  2 --
  include/linux/backlight.h  | 19 
  5 files changed, 58 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c 
b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
index a42dee6..cb1a01a 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
@@ -236,46 +236,6 @@ void tinydrm_xrgb_to_gray8(u8 *dst, void *vaddr, 
struct drm_framebuffer *fb,
  }
  EXPORT_SYMBOL(tinydrm_xrgb_to_gray8);
  
-/**

- * tinydrm_of_find_backlight - Find backlight device in device-tree
- * @dev: Device
- *
- * This function looks for a DT node pointed to by a property named 'backlight'
- * and uses of_find_backlight_by_node() to get the backlight device.
- * Additionally if the brightness property is zero, it is set to
- * max_brightness.
- *
- * Returns:
- * NULL if there's no backlight property.
- * Error pointer -EPROBE_DEFER if the DT node is found, but no backlight device
- * is found.
- * If the backlight device is found, a pointer to the structure is returned.
- */
-struct backlight_device *tinydrm_of_find_backlight(struct device *dev)
-{
-   struct backlight_device *backlight;
-   struct device_node *np;
-
-   np = of_parse_phandle(dev->of_node, "backlight", 0);
-   if (!np)
-   return NULL;
-
-   backlight = of_find_backlight_by_node(np);
-   of_node_put(np);
-
-   if (!backlight)
-   return ERR_PTR(-EPROBE_DEFER);
-
-   if (!backlight->props.brightness) {
-   backlight->props.brightness = backlight->props.max_brightness;
-   DRM_DEBUG_KMS("Backlight brightness set to %d\n",
- backlight->props.brightness);
-   }
-
-   return backlight;
-}
-EXPORT_SYMBOL(tinydrm_of_find_backlight);
-
  #if IS_ENABLED(CONFIG_SPI)
  
  /**

diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c 
b/drivers/gpu/drm/tinydrm/mi0283qt.c
index 7fd2691..a932185 100644
--- a/drivers/gpu/drm/tinydrm/mi0283qt.c
+++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
@@ -12,6 +12,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -188,7 +189,7 @@ static int mi0283qt_probe(struct spi_device *spi)
if (IS_ERR(mipi->regulator))
return PTR_ERR(mipi->regulator);
  
-	mipi->backlight = tinydrm_of_find_backlight(dev);

+   mipi->backlight = backlight_get(dev);
if (IS_ERR(mipi->backlight))
return PTR_ERR(mipi->backlight);
  
diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c

index 8049e76..c4e94d0 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -580,6 +580,43 @@ struct backlight_device *of_find_backlight_by_node(struct 
device_node *node)
  EXPORT_SYMBOL(of_find_backlight_by_node);
  #endif
  
+/**

+ * backlight_get - Get backlight device
+ * @dev: Device
+ *
+ * This function looks for a property named 'backlight' on the DT node
+ * connected to @dev and looks up the backlight device.
+ *
+ * Call backlight_put() to drop the reference on the backlight device.
+ *
+ * Returns:
+ * A pointer to the backlight device if found.
+ * Error pointer -EPROBE_DEFER if the DT property is set, but no backlight
+ * device is found.
+ * NULL if there's no backlight property.
+ */
+struct backlight_device *backlight_get(struct device *dev)
+{
+   struct backlight_device *bd = NULL;
+   struct device_node *np;
+
+   if (!dev)
+ 

Re: [Outreachy kernel] [PATCH v13 3/3] drm/tinydrm: Add devres versions of backlight_get

[Sean Paul]

On Fri, Oct 13, 2017 at 04:12:55PM +0530, Meghana Madhyastha wrote:
> Add devm_backlight_get and the corresponding release
> function because some drivers use devres versions of functions
> for requiring device resources.
> 

This patch looks fine, just update the names to be consistent with the previous
version and account for backlight_put not being present.

Sean

> Signed-off-by: Meghana Madhyastha 
> ---
> Changes in v13:
>  -Add devm_backlight_put to backlight.c
> 
>  drivers/gpu/drm/tinydrm/mi0283qt.c  |  2 +-
>  drivers/video/backlight/backlight.c | 31 +++
>  include/linux/backlight.h   |  6 ++
>  3 files changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c 
> b/drivers/gpu/drm/tinydrm/mi0283qt.c
> index a932185..e3e7583 100644
> --- a/drivers/gpu/drm/tinydrm/mi0283qt.c
> +++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
> @@ -189,7 +189,7 @@ static int mi0283qt_probe(struct spi_device *spi)
>   if (IS_ERR(mipi->regulator))
>   return PTR_ERR(mipi->regulator);
>  
> - mipi->backlight = backlight_get(dev);
> + mipi->backlight = devm_backlight_get(dev);
>   if (IS_ERR(mipi->backlight))
>   return PTR_ERR(mipi->backlight);
>  
> diff --git a/drivers/video/backlight/backlight.c 
> b/drivers/video/backlight/backlight.c
> index c4e94d0..b6c505a 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -617,6 +617,37 @@ struct backlight_device *backlight_get(struct device 
> *dev)
>  }
>  EXPORT_SYMBOL(backlight_get);
>  
> +static void devm_backlight_put(void *data)
> +{
> + backlight_put(data);
> +}
> +
> +/**
> + * devm_backlight_get - Resource-managed backlight_get()
> + * @dev: Device
> + *
> + * Device managed version of backlight_get(). The reference on the backlight
> + * device is automatically dropped on driver detach.
> + */
> +struct backlight_device *devm_backlight_get(struct device *dev)
> +{
> + struct backlight_device *bd;
> + int ret;
> +
> + bd = backlight_get(dev);
> + if (!bd)
> + return NULL;
> +
> + ret = devm_add_action(dev, devm_backlight_put, bd);
> + if (ret) {
> + backlight_put(bd);
> + return ERR_PTR(ret);
> + }
> +
> + return bd;
> +}
> +EXPORT_SYMBOL(devm_backlight_get);
> +
>  static void __exit backlight_class_exit(void)
>  {
>   class_destroy(backlight_class);
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index 987a6d7..8db9ba9 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -214,11 +214,17 @@ of_find_backlight_by_node(struct device_node *node)
>  
>  #if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
>  struct backlight_device *backlight_get(struct device *dev);
> +struct backlight_device *devm_backlight_get(struct device *dev);
>  #else
>  static inline struct backlight_device *backlight_get(struct device *dev)
>  {
>   return NULL;
>  }
> +
> +static inline struct backlight_device *devm_backlight_get(struct device *dev)
> +{
> + return NULL;
> +}
>  #endif
>  
>  #endif
> -- 
> 2.7.4
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/4b1acbfbddd2a123001e035a62801456891276d5.1507890285.git.meghana.madhyastha%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.

-- 
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 196615] amdgpu - resume from suspend is no longer working on rx480

[bugzilla-daemon]

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

--- Comment #24 from klavkala...@gmail.com ---
I would suggest to remove the fix in all kernel versions until we can confirm
it doesn't break anything. Having an LTS kernel break suspend/resume for
polaris users doesn't sound to good.

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

[PATCH] drm/plane: drop num_overlay_planes (v3)

[Dave Airlie]

From: Dave Airlie 

In order to implement plane leasing we need to count things,
just make the code consistent with the counting code currently
used for counting crtcs/encoders/connectors and drop the need
for num_overlay_planes.

v2: don't forget to assign plane_ptr. (keithp)
v3: use correct bounds check, found by igt.

Signed-off-by: Dave Airlie 
---
 drivers/gpu/drm/drm_mode_config.c |  1 -
 drivers/gpu/drm/drm_plane.c   | 46 ++-
 include/drm/drm_mode_config.h | 13 ---
 3 files changed, 16 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/drm_mode_config.c 
b/drivers/gpu/drm/drm_mode_config.c
index 74f6ff5df656..919e78d45ab0 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -385,7 +385,6 @@ void drm_mode_config_init(struct drm_device *dev)
dev->mode_config.num_connector = 0;
dev->mode_config.num_crtc = 0;
dev->mode_config.num_encoder = 0;
-   dev->mode_config.num_overlay_plane = 0;
dev->mode_config.num_total_plane = 0;
 }
 EXPORT_SYMBOL(drm_mode_config_init);
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 574fd1475214..8d9824804b0c 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -241,8 +241,6 @@ int drm_universal_plane_init(struct drm_device *dev, struct 
drm_plane *plane,
 
list_add_tail(>head, >plane_list);
plane->index = config->num_total_plane++;
-   if (plane->type == DRM_PLANE_TYPE_OVERLAY)
-   config->num_overlay_plane++;
 
drm_object_attach_property(>base,
   config->plane_type_property,
@@ -353,8 +351,6 @@ void drm_plane_cleanup(struct drm_plane *plane)
 
list_del(>head);
dev->mode_config.num_total_plane--;
-   if (plane->type == DRM_PLANE_TYPE_OVERLAY)
-   dev->mode_config.num_overlay_plane--;
 
WARN_ON(plane->state && !plane->funcs->atomic_destroy_state);
if (plane->state && plane->funcs->atomic_destroy_state)
@@ -462,43 +458,33 @@ int drm_mode_getplane_res(struct drm_device *dev, void 
*data,
struct drm_mode_config *config;
struct drm_plane *plane;
uint32_t __user *plane_ptr;
-   int copied = 0;
-   unsigned num_planes;
+   int count = 0;
 
if (!drm_core_check_feature(dev, DRIVER_MODESET))
return -EINVAL;
 
config = >mode_config;
-
-   if (file_priv->universal_planes)
-   num_planes = config->num_total_plane;
-   else
-   num_planes = config->num_overlay_plane;
+   plane_ptr = u64_to_user_ptr(plane_resp->plane_id_ptr);
 
/*
 * This ioctl is called twice, once to determine how much space is
 * needed, and the 2nd time to fill it.
 */
-   if (num_planes &&
-   (plane_resp->count_planes >= num_planes)) {
-   plane_ptr = (uint32_t __user *)(unsigned 
long)plane_resp->plane_id_ptr;
-
-   /* Plane lists are invariant, no locking needed. */
-   drm_for_each_plane(plane, dev) {
-   /*
-* Unless userspace set the 'universal planes'
-* capability bit, only advertise overlays.
-*/
-   if (plane->type != DRM_PLANE_TYPE_OVERLAY &&
-   !file_priv->universal_planes)
-   continue;
-
-   if (put_user(plane->base.id, plane_ptr + copied))
-   return -EFAULT;
-   copied++;
-   }
+   drm_for_each_plane(plane, dev) {
+   /*
+* Unless userspace set the 'universal planes'
+* capability bit, only advertise overlays.
+*/
+   if (plane->type != DRM_PLANE_TYPE_OVERLAY &&
+   !file_priv->universal_planes)
+   continue;
+
+   if (count < plane_resp->count_planes &&
+   put_user(plane->base.id, plane_ptr + count))
+   return -EFAULT;
+   count++;
}
-   plane_resp->count_planes = num_planes;
+   plane_resp->count_planes = count;
 
return 0;
 }
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 1b37368416c8..0b4ac2ebc610 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -429,19 +429,6 @@ struct drm_mode_config {
 */
struct list_head encoder_list;
 
-   /**
-* @num_overlay_plane:
-*
-* Number of overlay planes on this device, excluding primary and cursor
-* planes.
-*
-* Track number of overlay planes separately from number of total
-* planes.  By default we only advertise overlay planes to userspace; if
-* userspace 

Re: [PATCH v2] drm/tilcdc: Precalculate total frametime in tilcdc_crtc_set_mode()

[Jyri Sarha]


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

On 10/13/17 15:32, Tomi Valkeinen wrote:
> Where does "uint" come from anyway? I don't think it's part of a C standard.

AFAIK, it is just a Linux commodity typedef for unsigned int, and
because of that a defacto typedef in many other environments too.

Best regards,
Jyri

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

[PATCH v3 1/6] drm/amd/display: Use DRM new-style object iterators.

[sunpeng.li]

From: "Leo (Sunpeng) Li" 

Use the correct for_each_new/old_* iterators instead of for_each_*

The following functions were considered:

amdgpu_dm_find_first_crtc_matching_connector: use for_each_new
- Old from_state_var flag was always choosing the new state

amdgpu_dm_display_resume: use for_each_new
- drm_atomic_helper_duplicate_state is called during suspend to
  cache the state
- It sets 'state' within the state triplet to 'new_state'

amdgpu_dm_commit_planes: use for_each_old
- Called after the state was swapped (via atomic commit tail)

amdgpu_dm_atomic_commit: use for_each_new
- Called before the state is swapped

amdgpu_dm_atomic_commit_tail: use for_each_old
- Called after the state was swapped

dm_update_crtcs_state: use for_each_new
- Called before the state is swapped (via atomic check)

amdgpu_dm_atomic_check: use for_each_new
- Called before the state is swapped

v2: Split out typo fixes to a new patch.

v3: Say "functions considered" instead of "affected functions". The
latter implies that changes are made to each.

Signed-off-by: Leo (Sunpeng) Li 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 ---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  3 +--
 2 files changed, 12 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 9bfe1f9..cc024ab 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -570,23 +570,15 @@ static int dm_suspend(void *handle)
 
 struct amdgpu_dm_connector *amdgpu_dm_find_first_crct_matching_connector(
struct drm_atomic_state *state,
-   struct drm_crtc *crtc,
-   bool from_state_var)
+   struct drm_crtc *crtc)
 {
uint32_t i;
struct drm_connector_state *conn_state;
struct drm_connector *connector;
struct drm_crtc *crtc_from_state;
 
-   for_each_new_connector_in_state(
-   state,
-   connector,
-   conn_state,
-   i) {
-   crtc_from_state =
-   from_state_var ?
-   conn_state->crtc :
-   connector->state->crtc;
+   for_each_new_connector_in_state(state, connector, conn_state, i) {
+   crtc_from_state = conn_state->crtc;
 
if (crtc_from_state == crtc)
return to_amdgpu_dm_connector(connector);
@@ -3890,7 +3882,7 @@ static void amdgpu_dm_commit_planes(struct 
drm_atomic_state *state,
unsigned long flags;
 
/* update planes when needed */
-   for_each_new_plane_in_state(state, plane, old_plane_state, i) {
+   for_each_old_plane_in_state(state, plane, old_plane_state, i) {
struct drm_plane_state *plane_state = plane->state;
struct drm_crtc *crtc = plane_state->crtc;
struct drm_framebuffer *fb = plane_state->fb;
@@ -4024,7 +4016,7 @@ void amdgpu_dm_atomic_commit_tail(
dm_state = to_dm_atomic_state(state);
 
/* update changed items */
-   for_each_new_crtc_in_state(state, crtc, old_crtc_state, i) {
+   for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) {
struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
struct drm_crtc_state *new_state = crtc->state;
 
@@ -4113,11 +4105,9 @@ void amdgpu_dm_atomic_commit_tail(
new_acrtc_state = 
to_dm_crtc_state(new_crtcs[i]->base.state);
 
new_stream = new_acrtc_state->stream;
-   aconnector =
-   amdgpu_dm_find_first_crct_matching_connector(
+   aconnector = 
amdgpu_dm_find_first_crct_matching_connector(
state,
-   _crtcs[i]->base,
-   false);
+   _crtcs[i]->base);
if (!aconnector) {
DRM_DEBUG_DRIVER("Atomic commit: Failed to find 
connector for acrtc id:%d "
 "skipping freesync init\n",
@@ -4151,7 +4141,7 @@ void amdgpu_dm_atomic_commit_tail(
}
 
/* Handle scaling and undersacn changes*/
-   for_each_new_connector_in_state(state, connector, old_conn_state, i) {
+   for_each_old_connector_in_state(state, connector, old_conn_state, i) {
struct amdgpu_dm_connector *aconnector = 
to_amdgpu_dm_connector(connector);
struct dm_connector_state *con_new_state =
to_dm_connector_state(aconnector->base.state);
@@ -4205,7 +4195,7 @@ void amdgpu_dm_atomic_commit_tail(
}
 
/* update planes when needed per crtc*/
-   

Re: [PULL] drm-intel-next

[Dave Airlie]

On 13 October 2017 at 01:23, Jani Nikula  wrote:
> On Wed, 11 Oct 2017, Jani Nikula  wrote:
>> Hi Dave, more v4.15 features.
>
> Okay, so I suck and there's still one more batch to come after this. I'm
> a bit out of rhythm here. When do you want the pull request for that at
> the latest?

Around rc6 time I think is when I generally get the last one. I sometimes let
Daniel slip a week or so once it was all in CI and -next at that time.

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

Re: [PATCH] drm/tilcdc: Force recalculation of vrefresh in tilcdc_crtc_set_mode()

[Kevin Hao]

On Thu, Oct 12, 2017 at 12:39:25PM +0300, Jyri Sarha wrote:
> We are using the vrefresh to check if we are too close to vertical
> sync to update the two framebuffer DMA registers and risk a
> collision. The vrefresh is coming from user space and normally it is
> not used for anything. For instance xserver leaves vrefresh to zero
> causing a division by zero when setting the mode. We want to make sure
> the value is valid and force its recalculation in
> tilcdc_crtc_set_mode().
> 
> Reported-by: Kevin Hao 
> Fixes: 11abbc9f39e0 ("drm/tilcdc: Set framebuffer DMA address to HW only if 
> CRTC is enabled")
> Cc:  # v4.11+
> Signed-off-by: Jyri Sarha 
> ---
>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c 
> b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> index 704db24..aa5e87c 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> @@ -459,6 +459,16 @@ static void tilcdc_crtc_set_mode(struct drm_crtc *crtc)
>   drm_framebuffer_get(fb);
>  
>   crtc->hwmode = crtc->state->adjusted_mode;
> +
> + /* We are using the vrefresh to check if we are too close to
> +  * vertical sync to update the two framebuffer DMA registers
> +  * and risk a collision. The vrefresh is coming from user
> +  * space and normally it is not used for anything. We want to
> +  * make sure the value is valid and force its recalculation
> +  * here.
> +  */
> + crtc->hwmode.vrefresh = 0;

Why do we need this?

Thanks,
Kevin

> + crtc->hwmode.vrefresh = drm_mode_vrefresh(>hwmode);
>  }
>  
>  static void tilcdc_crtc_enable(struct drm_crtc *crtc)
> -- 
> 1.9.1
> 
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 
> 


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

Re: [PATCH 1/2] vgaarb: Select a default VGA device even if there's no legacy VGA

[Lothar Waßmann]

Hi,

On Thu, 12 Oct 2017 12:24:10 +0100 Julien Thierry wrote:
> Hi Bjorn,
> 
> On 06/10/17 23:24, Bjorn Helgaas wrote:
> > From: Bjorn Helgaas 
> > 
> > Daniel Axtens reported that on the HiSilicon D05 board, the VGA device is
> > behind a bridge that doesn't support PCI_BRIDGE_CTL_VGA, so the VGA arbiter
> > never selects it as the default, which means Xorg auto-detection doesn't
> > work.
> > 
> > VGA is a legacy PCI feature: a VGA device can respond to addresses, e.g.,
> > [mem 0xa-0xb], [io 0x3b0-0x3bb], [io 0x3c0-0x3df], etc., that are
> > not configurable by BARs.  Consequently, multiple VGA devices can conflict
> > with each other.  The VGA arbiter avoids conflicts by ensuring that those
> > legacy resources are only routed to one VGA device at a time.
> > 
> > The arbiter identifies the "default VGA" device, i.e., a legacy VGA device
> > that was used by boot firmware.  It selects the first device that:
> > 
> >- is of PCI_CLASS_DISPLAY_VGA,
> >- has both PCI_COMMAND_IO and PCI_COMMAND_MEMORY enabled, and
> >- has PCI_BRIDGE_CTL_VGA set in all upstream bridges.
> > 
> > Some systems don't have such a device.  For example, if a host bridge
> > doesn't support I/O space, PCI_COMMAND_IO probably won't be enabled for any
> > devices below it.  Or, as on the HiSilicon D05, the VGA device may be
> > behind a bridge that doesn't support PCI_BRIDGE_CTL_VGA, so accesses to the
> > legacy VGA resources will never reach the device.
> > 
> > This patch extends the arbiter so that if it doesn't find a device that
> > meets all the above criteria, it selects the first device that:
> > 
> >- is of PCI_CLASS_DISPLAY_VGA and
> >- has PCI_COMMAND_IO or PCI_COMMAND_MEMORY enabled
> > 
> > If it doesn't find even that, it selects the first device that:
> > 
> >- is of class PCI_CLASS_DISPLAY_VGA.
> > 
> > Such a device may not be able to use the legacy VGA resources, but most
> > drivers can operate the device without those.  Setting it as the default
> > device means its "boot_vga" sysfs file will contain "1", which Xorg (via
> > libpciaccess) uses to help select its default output device.
> > 
> > This fixes Xorg auto-detection on some arm64 systems (HiSilicon D05 in
> > particular; see the link below).
> > 
> > It also replaces the powerpc fixup_vga() quirk, albeit with slightly
> > different semantics: the quirk selected the first VGA device we found, and
> > overrode that selection with any enabled VGA device we found.  If there
> > were several enabled VGA devices, the *last* one we found would become the
> > default.
> > 
> > The code here instead selects the *first* enabled VGA device we find, and
> > if none are enabled, the first VGA device we find.
> > 
> > Link: http://lkml.kernel.org/r/20170901072744.2409-1-...@axtens.net
> > Tested-by: Daniel Axtens # arm64, ppc64-qemu-tcg
> > Signed-off-by: Bjorn Helgaas 
> > ---
> >   arch/powerpc/kernel/pci-common.c |   12 
> >   drivers/gpu/vga/vgaarb.c |   25 +
> >   2 files changed, 25 insertions(+), 12 deletions(-)
> > 
> > diff --git a/arch/powerpc/kernel/pci-common.c 
> > b/arch/powerpc/kernel/pci-common.c
> > index 02831a396419..0ac7aa346c69 100644
> > --- a/arch/powerpc/kernel/pci-common.c
> > +++ b/arch/powerpc/kernel/pci-common.c
> > @@ -1740,15 +1740,3 @@ static void fixup_hide_host_resource_fsl(struct 
> > pci_dev *dev)
> >   }
> >   DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MOTOROLA, PCI_ANY_ID, 
> > fixup_hide_host_resource_fsl);
> >   DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID, 
> > fixup_hide_host_resource_fsl);
> > -
> > -static void fixup_vga(struct pci_dev *pdev)
> > -{
> > -   u16 cmd;
> > -
> > -   pci_read_config_word(pdev, PCI_COMMAND, );
> > -   if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || 
> > !vga_default_device())
> > -   vga_set_default_device(pdev);
> > -
> > -}
> > -DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
> > - PCI_CLASS_DISPLAY_VGA, 8, fixup_vga);
> > diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
> > index 76875f6299b8..aeb41f793ed4 100644
> > --- a/drivers/gpu/vga/vgaarb.c
> > +++ b/drivers/gpu/vga/vgaarb.c
> > @@ -1468,6 +1468,31 @@ static int __init vga_arb_device_init(void)
> > vgaarb_info(dev, "no bridge control possible\n");
> > }
> >   
> > +   if (!vga_default_device()) {
> > +   list_for_each_entry(vgadev, _list, list) {
> > +   struct device *dev = >pdev->dev;
> > +   u16 cmd;
> > +
> > +   pdev = vgadev->pdev;
> > +   pci_read_config_word(pdev, PCI_COMMAND, );
> > +   if (cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
> > +   vgaarb_info(dev, "setting as boot device (VGA 
> > legacy resources not available)\n");
> > +   

Re: [PATCH 1/2] vgaarb: Select a default VGA device even if there's no legacy VGA

[Julien Thierry]

On 12/10/17 13:05, Lothar Waßmann wrote:

Hi,

On Thu, 12 Oct 2017 12:24:10 +0100 Julien Thierry wrote:

Hi Bjorn,

On 06/10/17 23:24, Bjorn Helgaas wrote:

From: Bjorn Helgaas 

Daniel Axtens reported that on the HiSilicon D05 board, the VGA device is
behind a bridge that doesn't support PCI_BRIDGE_CTL_VGA, so the VGA arbiter
never selects it as the default, which means Xorg auto-detection doesn't
work.

VGA is a legacy PCI feature: a VGA device can respond to addresses, e.g.,
[mem 0xa-0xb], [io 0x3b0-0x3bb], [io 0x3c0-0x3df], etc., that are
not configurable by BARs.  Consequently, multiple VGA devices can conflict
with each other.  The VGA arbiter avoids conflicts by ensuring that those
legacy resources are only routed to one VGA device at a time.

The arbiter identifies the "default VGA" device, i.e., a legacy VGA device
that was used by boot firmware.  It selects the first device that:

- is of PCI_CLASS_DISPLAY_VGA,
- has both PCI_COMMAND_IO and PCI_COMMAND_MEMORY enabled, and
- has PCI_BRIDGE_CTL_VGA set in all upstream bridges.

Some systems don't have such a device.  For example, if a host bridge
doesn't support I/O space, PCI_COMMAND_IO probably won't be enabled for any
devices below it.  Or, as on the HiSilicon D05, the VGA device may be
behind a bridge that doesn't support PCI_BRIDGE_CTL_VGA, so accesses to the
legacy VGA resources will never reach the device.

This patch extends the arbiter so that if it doesn't find a device that
meets all the above criteria, it selects the first device that:

- is of PCI_CLASS_DISPLAY_VGA and
- has PCI_COMMAND_IO or PCI_COMMAND_MEMORY enabled

If it doesn't find even that, it selects the first device that:

- is of class PCI_CLASS_DISPLAY_VGA.

Such a device may not be able to use the legacy VGA resources, but most
drivers can operate the device without those.  Setting it as the default
device means its "boot_vga" sysfs file will contain "1", which Xorg (via
libpciaccess) uses to help select its default output device.

This fixes Xorg auto-detection on some arm64 systems (HiSilicon D05 in
particular; see the link below).

It also replaces the powerpc fixup_vga() quirk, albeit with slightly
different semantics: the quirk selected the first VGA device we found, and
overrode that selection with any enabled VGA device we found.  If there
were several enabled VGA devices, the *last* one we found would become the
default.

The code here instead selects the *first* enabled VGA device we find, and
if none are enabled, the first VGA device we find.

Link: http://lkml.kernel.org/r/20170901072744.2409-1-...@axtens.net
Tested-by: Daniel Axtens # arm64, ppc64-qemu-tcg
Signed-off-by: Bjorn Helgaas 
---
   arch/powerpc/kernel/pci-common.c |   12 
   drivers/gpu/vga/vgaarb.c |   25 +
   2 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 02831a396419..0ac7aa346c69 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1740,15 +1740,3 @@ static void fixup_hide_host_resource_fsl(struct pci_dev 
*dev)
   }
   DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MOTOROLA, PCI_ANY_ID, 
fixup_hide_host_resource_fsl);
   DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID, 
fixup_hide_host_resource_fsl);
-
-static void fixup_vga(struct pci_dev *pdev)
-{
-   u16 cmd;
-
-   pci_read_config_word(pdev, PCI_COMMAND, );
-   if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || 
!vga_default_device())
-   vga_set_default_device(pdev);
-
-}
-DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
- PCI_CLASS_DISPLAY_VGA, 8, fixup_vga);
diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
index 76875f6299b8..aeb41f793ed4 100644
--- a/drivers/gpu/vga/vgaarb.c
+++ b/drivers/gpu/vga/vgaarb.c
@@ -1468,6 +1468,31 @@ static int __init vga_arb_device_init(void)
vgaarb_info(dev, "no bridge control possible\n");
}
   
+	if (!vga_default_device()) {

+   list_for_each_entry(vgadev, _list, list) {
+   struct device *dev = >pdev->dev;
+   u16 cmd;
+
+   pdev = vgadev->pdev;
+   pci_read_config_word(pdev, PCI_COMMAND, );
+   if (cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
+   vgaarb_info(dev, "setting as boot device (VGA legacy 
resources not available)\n");
+   vga_set_default_device(pdev);
+   break;
+   }
+   }
+   }
+
+   if (!vga_default_device()) {
+   vgadev = list_first_entry_or_null(_list,
+ struct vga_device, list);
+   if (vgadev) {
+ 

Re: [PATCH] staging: vboxvideo: Fix reporting invalid suggested-offset-properties

[Michael Thayer]

Hello Hans,

12.10.2017 20:10, Hans de Goede wrote:
> The x and y hints receives from the host are unsigned 32 bit integers and
> they get set to -1 (0x) when invalid. Before this commit the
> vboxvideo driver was storing them in an u16 causing the -1 to be truncated
> to 65535 which, once reported to userspace, was breaking gnome 3.26+
> in Wayland mode.
> 
> This commit stores the host values in 32 bit variables, removing the
> truncation and checks for -1, replacing it with 0 as -1 is not a valid
> suggested-offset-property value. Likewise the properties are now
> initialized to 0 instead of -1, since -1 is not a valid value.
> This fixes gnome 3.26+ in Wayland mode not working with the vboxvideo
> driver.

Thank you, looks sensible.  Minor quibble: wouldn't it be nicer to
compare the unsigned values vbox_connector->vbox_crtc->x_hint and y_hint
against ~0 below, rather than -1 (see inline below)?  Other than that,
looks good to me.  I will send Gianfranco a patch to test against
Ubuntu's version of the driver.

Regards
Michael

> Reported-by: Gianfranco Costamagna 
> Cc: sta...@vger.kernel.org
> Cc: Michael Thayer 
> Signed-off-by: Hans de Goede 
> ---пишет
>  drivers/staging/vboxvideo/vbox_drv.h  |  8 
>  drivers/staging/vboxvideo/vbox_irq.c  |  4 ++--
>  drivers/staging/vboxvideo/vbox_mode.c | 26 ++
>  3 files changed, 24 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/staging/vboxvideo/vbox_drv.h 
> b/drivers/staging/vboxvideo/vbox_drv.h
> index 4b9302703b36..eeac4f0cb2c6 100644
> --- a/drivers/staging/vboxvideo/vbox_drv.h
> +++ b/drivers/staging/vboxvideo/vbox_drv.h
> @@ -137,8 +137,8 @@ struct vbox_connector {
>   char name[32];
>   struct vbox_crtc *vbox_crtc;
>   struct {
> - u16 width;
> - u16 height;
> + u32 width;
> + u32 height;
>   bool disconnected;
>   } mode_hint;
>  };
> @@ -150,8 +150,8 @@ struct vbox_crtc {
>   unsigned int crtc_id;
>   u32 fb_offset;
>   bool cursor_enabled;
> - u16 x_hint;
> - u16 y_hint;
> + u32 x_hint;
> + u32 y_hint;
>  };
>  
>  struct vbox_encoder {
> diff --git a/drivers/staging/vboxvideo/vbox_irq.c 
> b/drivers/staging/vboxvideo/vbox_irq.c
> index 3ca8bec62ac4..74abdf02d9fd 100644
> --- a/drivers/staging/vboxvideo/vbox_irq.c
> +++ b/drivers/staging/vboxvideo/vbox_irq.c
> @@ -150,8 +150,8 @@ static void vbox_update_mode_hints(struct vbox_private 
> *vbox)
>  
>   disconnected = !(hints->enabled);
>   crtc_id = vbox_conn->vbox_crtc->crtc_id;
> - vbox_conn->mode_hint.width = hints->cx & 0x8fff;
> - vbox_conn->mode_hint.height = hints->cy & 0x8fff;
> + vbox_conn->mode_hint.width = hints->cx;
> + vbox_conn->mode_hint.height = hints->cy;
>   vbox_conn->vbox_crtc->x_hint = hints->dx;
>   vbox_conn->vbox_crtc->y_hint = hints->dy;
>   vbox_conn->mode_hint.disconnected = disconnected;
> diff --git a/drivers/staging/vboxvideo/vbox_mode.c 
> b/drivers/staging/vboxvideo/vbox_mode.c
> index 257a77830410..6f08dc966719 100644
> --- a/drivers/staging/vboxvideo/vbox_mode.c
> +++ b/drivers/staging/vboxvideo/vbox_mode.c
> @@ -553,12 +553,22 @@ static int vbox_get_modes(struct drm_connector 
> *connector)
>   ++num_modes;
>   }
>   vbox_set_edid(connector, preferred_width, preferred_height);
> - drm_object_property_set_value(
> - >base, vbox->dev->mode_config.suggested_x_property,
> - vbox_connector->vbox_crtc->x_hint);
> - drm_object_property_set_value(
> - >base, vbox->dev->mode_config.suggested_y_property,
> - vbox_connector->vbox_crtc->y_hint);
> +
> + if (vbox_connector->vbox_crtc->x_hint != -1)

Unsigned with signed comparison above.

> + drm_object_property_set_value(>base,
> + vbox->dev->mode_config.suggested_x_property,
> + vbox_connector->vbox_crtc->x_hint);
> + else
> + drm_object_property_set_value(>base,
> + vbox->dev->mode_config.suggested_x_property, 0);
> +
> + if (vbox_connector->vbox_crtc->y_hint != -1)

And here.

> + drm_object_property_set_value(>base,
> + vbox->dev->mode_config.suggested_y_property,
> + vbox_connector->vbox_crtc->y_hint);
> + else
> + drm_object_property_set_value(>base,
> + vbox->dev->mode_config.suggested_y_property, 0);
>  
>   return num_modes;
>  }
> @@ -640,9 +650,9 @@ static int vbox_connector_init(struct drm_device *dev,
>  
>   drm_mode_create_suggested_offset_properties(dev);
>   drm_object_attach_property(>base,
> -dev->mode_config.suggested_x_property, -1);
> +

Re: [PATCH 1/2] vgaarb: Select a default VGA device even if there's no legacy VGA

[Bjorn Helgaas]

On Thu, Oct 12, 2017 at 12:24:10PM +0100, Julien Thierry wrote:
> Hi Bjorn,
> 
> On 06/10/17 23:24, Bjorn Helgaas wrote:
> >From: Bjorn Helgaas 
> >
> >Daniel Axtens reported that on the HiSilicon D05 board, the VGA device is
> >behind a bridge that doesn't support PCI_BRIDGE_CTL_VGA, so the VGA arbiter
> >never selects it as the default, which means Xorg auto-detection doesn't
> >work.
> >
> >VGA is a legacy PCI feature: a VGA device can respond to addresses, e.g.,
> >[mem 0xa-0xb], [io 0x3b0-0x3bb], [io 0x3c0-0x3df], etc., that are
> >not configurable by BARs.  Consequently, multiple VGA devices can conflict
> >with each other.  The VGA arbiter avoids conflicts by ensuring that those
> >legacy resources are only routed to one VGA device at a time.
> >
> >The arbiter identifies the "default VGA" device, i.e., a legacy VGA device
> >that was used by boot firmware.  It selects the first device that:
> >
> >   - is of PCI_CLASS_DISPLAY_VGA,
> >   - has both PCI_COMMAND_IO and PCI_COMMAND_MEMORY enabled, and
> >   - has PCI_BRIDGE_CTL_VGA set in all upstream bridges.
> >
> >Some systems don't have such a device.  For example, if a host bridge
> >doesn't support I/O space, PCI_COMMAND_IO probably won't be enabled for any
> >devices below it.  Or, as on the HiSilicon D05, the VGA device may be
> >behind a bridge that doesn't support PCI_BRIDGE_CTL_VGA, so accesses to the
> >legacy VGA resources will never reach the device.
> >
> >This patch extends the arbiter so that if it doesn't find a device that
> >meets all the above criteria, it selects the first device that:
> >
> >   - is of PCI_CLASS_DISPLAY_VGA and
> >   - has PCI_COMMAND_IO or PCI_COMMAND_MEMORY enabled
> >
> >If it doesn't find even that, it selects the first device that:
> >
> >   - is of class PCI_CLASS_DISPLAY_VGA.
> >
> >Such a device may not be able to use the legacy VGA resources, but most
> >drivers can operate the device without those.  Setting it as the default
> >device means its "boot_vga" sysfs file will contain "1", which Xorg (via
> >libpciaccess) uses to help select its default output device.
> >
> >This fixes Xorg auto-detection on some arm64 systems (HiSilicon D05 in
> >particular; see the link below).
> >
> >It also replaces the powerpc fixup_vga() quirk, albeit with slightly
> >different semantics: the quirk selected the first VGA device we found, and
> >overrode that selection with any enabled VGA device we found.  If there
> >were several enabled VGA devices, the *last* one we found would become the
> >default.
> >
> >The code here instead selects the *first* enabled VGA device we find, and
> >if none are enabled, the first VGA device we find.
> >
> >Link: http://lkml.kernel.org/r/20170901072744.2409-1-...@axtens.net
> >Tested-by: Daniel Axtens # arm64, ppc64-qemu-tcg
> >Signed-off-by: Bjorn Helgaas 
> >---
> >  arch/powerpc/kernel/pci-common.c |   12 
> >  drivers/gpu/vga/vgaarb.c |   25 +
> >  2 files changed, 25 insertions(+), 12 deletions(-)
> >
> >diff --git a/arch/powerpc/kernel/pci-common.c 
> >b/arch/powerpc/kernel/pci-common.c
> >index 02831a396419..0ac7aa346c69 100644
> >--- a/arch/powerpc/kernel/pci-common.c
> >+++ b/arch/powerpc/kernel/pci-common.c
> >@@ -1740,15 +1740,3 @@ static void fixup_hide_host_resource_fsl(struct 
> >pci_dev *dev)
> >  }
> >  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MOTOROLA, PCI_ANY_ID, 
> > fixup_hide_host_resource_fsl);
> >  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID, 
> > fixup_hide_host_resource_fsl);
> >-
> >-static void fixup_vga(struct pci_dev *pdev)
> >-{
> >-u16 cmd;
> >-
> >-pci_read_config_word(pdev, PCI_COMMAND, );
> >-if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || 
> >!vga_default_device())
> >-vga_set_default_device(pdev);
> >-
> >-}
> >-DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
> >-  PCI_CLASS_DISPLAY_VGA, 8, fixup_vga);
> >diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
> >index 76875f6299b8..aeb41f793ed4 100644
> >--- a/drivers/gpu/vga/vgaarb.c
> >+++ b/drivers/gpu/vga/vgaarb.c
> >@@ -1468,6 +1468,31 @@ static int __init vga_arb_device_init(void)
> > vgaarb_info(dev, "no bridge control possible\n");
> > }
> >+if (!vga_default_device()) {
> >+list_for_each_entry(vgadev, _list, list) {
> >+struct device *dev = >pdev->dev;
> >+u16 cmd;
> >+
> >+pdev = vgadev->pdev;
> >+pci_read_config_word(pdev, PCI_COMMAND, );
> >+if (cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
> >+vgaarb_info(dev, "setting as boot device (VGA 
> >legacy resources not available)\n");
> >+vga_set_default_device(pdev);
> >+break;
> >+}
> >+}
> >+ 

Re: [PATCH 0/2] vgaarb: Select fallback default VGA device

[Sherlock Wang]

On Fri, Oct 06, 2017 at 05:24:20PM -0500, Bjorn Helgaas wrote:
> These patches are supposed to fix a problem Daniel Axtens found on the
> HiSilicon D05 board.  The VGA device there is behind a bridge that doesn't
> support PCI_BRIDGE_CTL_VGA, so the arbiter never selects the device as the
> default.
> 
> The first patch extends the arbiter so that if it can't find an enabled VGA
> device with legacy resources, it selects the first enabled device *without*
> legacy resources (this is what fixes the D05).  If that fails, it selects
> the first device that isn't enabled.  The combination of both changes
> should make the current powerpc fixup_vga() quirk unnecessary.
> 
> N.B. It changes the powerpc behavior: if there are several enabled VGA
> devices, the current quirk selects the last one, while this patch selects
> the first one.  If this is a problem, I can drop that part of the patch and
> keep the quirk.
> 
> The second patch pulls out this fallback device detection (and the EFI
> override) from vga_arb_device_init() to make it easier to read.

Hi Bjorn,

I tested this series in:

D05 board based on HiSilicon SoC Hip07.
And HiSilicon SoC Hip08 based board.

Both of these two SoCs' PCIe host bridge don't support PCI_BRIDGE_CTL_VGA. One
VGA device which does not support legacy resources has been connected to these
two PCIe host bridges to test. Default VGA device can be selected in above two
systems.

Tested-by: Zhou Wang 
(sorry for my HiSilicon E-mail box miss this serias, so here I have to use my
gmail box)

Thanks,
Zhou

> 
> ---
> 
> Bjorn Helgaas (2):
>   vgaarb: Select a default VGA device even if there's no legacy VGA
>   vgaarb: Factor out EFI and fallback default device selection
> 
> 
>  arch/powerpc/kernel/pci-common.c |   12 --
>  drivers/gpu/vga/vgaarb.c |   72 
> +-
>  2 files changed, 55 insertions(+), 29 deletions(-)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/2] drm/udl: Fixed problem with UDL adpater reconnection

[Robert Tarasov]

Fixed problem with DisplayLink and DisplayLink certified adapers in drm/udl
driver when adapter doesn't want to work if it was initialized with
disconnected DVI cable by enabling drm connectot polling and updating
current connector's state.

Signed-off-by: Robert Tarasov 
---
 drivers/gpu/drm/udl/udl_connector.c | 76 -
 drivers/gpu/drm/udl/udl_connector.h | 13 +++
 drivers/gpu/drm/udl/udl_drv.c   |  4 ++
 drivers/gpu/drm/udl/udl_main.c  |  5 +++
 4 files changed, 72 insertions(+), 26 deletions(-)
 create mode 100644 drivers/gpu/drm/udl/udl_connector.h

diff --git a/drivers/gpu/drm/udl/udl_connector.c 
b/drivers/gpu/drm/udl/udl_connector.c
index 091ca81658eb..6a9250ac8f29 100644
--- a/drivers/gpu/drm/udl/udl_connector.c
+++ b/drivers/gpu/drm/udl/udl_connector.c
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include "udl_connector.h"
 #include "udl_drv.h"
 
 /* dummy connector to just get EDID,
@@ -56,28 +57,15 @@ static u8 *udl_get_edid(struct udl_device *udl)
 
 static int udl_get_modes(struct drm_connector *connector)
 {
-   struct udl_device *udl = connector->dev->dev_private;
-   struct edid *edid;
-   int ret;
-
-   edid = (struct edid *)udl_get_edid(udl);
-   if (!edid) {
-   drm_mode_connector_update_edid_property(connector, NULL);
-   return 0;
-   }
-
-   /*
-* We only read the main block, but if the monitor reports extension
-* blocks then the drm edid code expects them to be present, so patch
-* the extension count to 0.
-*/
-   edid->checksum += edid->extensions;
-   edid->extensions = 0;
-
-   drm_mode_connector_update_edid_property(connector, edid);
-   ret = drm_add_edid_modes(connector, edid);
-   kfree(edid);
-   return ret;
+   struct udl_drm_connector *udl_connector =
+   container_of(connector,
+   struct udl_drm_connector,
+   connector);
+
+   drm_mode_connector_update_edid_property(connector, udl_connector->edid);
+   if (udl_connector->edid)
+   return drm_add_edid_modes(connector, udl_connector->edid);
+   return 0;
 }
 
 static int udl_mode_valid(struct drm_connector *connector,
@@ -96,8 +84,33 @@ static int udl_mode_valid(struct drm_connector *connector,
 static enum drm_connector_status
 udl_detect(struct drm_connector *connector, bool force)
 {
-   if (drm_dev_is_unplugged(connector->dev))
+   struct edid *edid;
+   struct udl_device *udl = connector->dev->dev_private;
+   struct udl_drm_connector *udl_connector =
+   container_of(connector,
+   struct udl_drm_connector,
+   connector);
+
+   if (udl_connector->edid != NULL) {
+   kfree(udl_connector->edid);
+   udl_connector->edid = NULL;
+   }
+
+   edid = (struct edid *)udl_get_edid(udl);
+   if (!edid || !memchr_inv(edid, 0, EDID_LENGTH))
return connector_status_disconnected;
+
+   udl_connector->edid = edid;
+
+   /*
+* We only read the main block, but if the monitor reports extension
+* blocks then the drm edid code expects them to be present, so patch
+* the extension count to 0.
+*/
+   udl_connector->edid->checksum +=
+   udl_connector->edid->extensions;
+   udl_connector->edid->extensions = 0;
+
return connector_status_connected;
 }
 
@@ -117,8 +130,14 @@ static int udl_connector_set_property(struct drm_connector 
*connector,
 
 static void udl_connector_destroy(struct drm_connector *connector)
 {
+   struct udl_drm_connector *udl_connector =
+   container_of(connector,
+   struct udl_drm_connector,
+   connector);
+
drm_connector_unregister(connector);
drm_connector_cleanup(connector);
+   kfree(udl_connector->edid);
kfree(connector);
 }
 
@@ -138,17 +157,22 @@ static const struct drm_connector_funcs 
udl_connector_funcs = {
 
 int udl_connector_init(struct drm_device *dev, struct drm_encoder *encoder)
 {
+   struct udl_drm_connector *udl_connector;
struct drm_connector *connector;
 
-   connector = kzalloc(sizeof(struct drm_connector), GFP_KERNEL);
-   if (!connector)
+   udl_connector = kzalloc(sizeof(struct udl_drm_connector), GFP_KERNEL);
+   if (!udl_connector)
return -ENOMEM;
 
-   drm_connector_init(dev, connector, _connector_funcs, 
DRM_MODE_CONNECTOR_DVII);
+   connector = _connector->connector;
+   drm_connector_init(dev, connector, _connector_funcs,
+  DRM_MODE_CONNECTOR_DVII);
drm_connector_helper_add(connector, 

[PATCH 2/2] drm/udl: Reading all edid blocks in DRM/UDL driver

[Robert Tarasov]

Now DRM/UDL driver retreives all edid data blocks instead of only base one.
Previous approch could lead to improper initialization of video mode with
certain monitors.

Signed-off-by: Robert Tarasov 
---
 drivers/gpu/drm/udl/udl_connector.c | 106 +++-
 1 file changed, 68 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/udl/udl_connector.c 
b/drivers/gpu/drm/udl/udl_connector.c
index 6a9250ac8f29..c3dc1fd20cb4 100644
--- a/drivers/gpu/drm/udl/udl_connector.c
+++ b/drivers/gpu/drm/udl/udl_connector.c
@@ -17,42 +17,79 @@
 #include "udl_connector.h"
 #include "udl_drv.h"
 
-/* dummy connector to just get EDID,
-   all UDL appear to have a DVI-D */
-
-static u8 *udl_get_edid(struct udl_device *udl)
+static bool udl_get_edid_block(struct udl_device *udl, int block_idx,
+  u8 *buff)
 {
-   u8 *block;
-   char *rbuf;
int ret, i;
+   u8 *read_buff;
 
-   block = kmalloc(EDID_LENGTH, GFP_KERNEL);
-   if (block == NULL)
-   return NULL;
-
-   rbuf = kmalloc(2, GFP_KERNEL);
-   if (rbuf == NULL)
-   goto error;
+   read_buff = kmalloc(2, GFP_KERNEL);
+   if (!read_buff)
+   return false;
 
for (i = 0; i < EDID_LENGTH; i++) {
+   int bval = (i + block_idx * EDID_LENGTH) << 8;
ret = usb_control_msg(udl->udev,
- usb_rcvctrlpipe(udl->udev, 0), (0x02),
- (0x80 | (0x02 << 5)), i << 8, 0xA1, rbuf, 
2,
- HZ);
+ usb_rcvctrlpipe(udl->udev, 0),
+ (0x02), (0x80 | (0x02 << 5)), bval,
+ 0xA1, read_buff, 2, HZ);
if (ret < 1) {
DRM_ERROR("Read EDID byte %d failed err %x\n", i, ret);
-   goto error;
+   kfree(read_buff);
+   return false;
}
-   block[i] = rbuf[1];
+   buff[i] = read_buff[1];
}
 
-   kfree(rbuf);
-   return block;
+   kfree(read_buff);
+   return true;
+}
 
-error:
-   kfree(block);
-   kfree(rbuf);
-   return NULL;
+static bool udl_get_edid(struct udl_device *udl, u8 **result_buff,
+int *result_buff_size)
+{
+   int i, extensions;
+   u8 *block_buff = NULL, *buff_ptr;
+
+   block_buff = kmalloc(EDID_LENGTH, GFP_KERNEL);
+   if (block_buff == NULL)
+   return false;
+
+   if (udl_get_edid_block(udl, 0, block_buff) &&
+   memchr_inv(block_buff, 0, EDID_LENGTH)) {
+   extensions = ((struct edid *)block_buff)->extensions;
+   if (extensions > 0) {
+   /* we have to read all extensions one by one */
+   *result_buff_size = EDID_LENGTH * (extensions + 1);
+   *result_buff = kmalloc(*result_buff_size, GFP_KERNEL);
+   buff_ptr = *result_buff;
+   if (buff_ptr == NULL) {
+   kfree(block_buff);
+   return false;
+   }
+   memcpy(buff_ptr, block_buff, EDID_LENGTH);
+   kfree(block_buff);
+   buff_ptr += EDID_LENGTH;
+   for (i = 1; i < extensions; ++i) {
+   if (udl_get_edid_block(udl, i, buff_ptr)) {
+   buff_ptr += EDID_LENGTH;
+   } else {
+   kfree(*result_buff);
+   *result_buff = NULL;
+   return false;
+   }
+   }
+   return true;
+   }
+   /* we have only base edid block */
+   *result_buff = block_buff;
+   *result_buff_size = EDID_LENGTH;
+   return true;
+   }
+
+   kfree(block_buff);
+
+   return false;
 }
 
 static int udl_get_modes(struct drm_connector *connector)
@@ -84,33 +121,26 @@ static int udl_mode_valid(struct drm_connector *connector,
 static enum drm_connector_status
 udl_detect(struct drm_connector *connector, bool force)
 {
-   struct edid *edid;
+   u8 *edid_buff = NULL;
+   int edid_buff_size = 0;
struct udl_device *udl = connector->dev->dev_private;
struct udl_drm_connector *udl_connector =
container_of(connector,
struct udl_drm_connector,
connector);
 
+   /* cleanup previous edid */
if (udl_connector->edid != NULL) {

Re: [PATCH 1/2] vgaarb: Select a default VGA device even if there's no legacy VGA

[Julien Thierry]

Hi Bjorn,

On 06/10/17 23:24, Bjorn Helgaas wrote:

From: Bjorn Helgaas 

Daniel Axtens reported that on the HiSilicon D05 board, the VGA device is
behind a bridge that doesn't support PCI_BRIDGE_CTL_VGA, so the VGA arbiter
never selects it as the default, which means Xorg auto-detection doesn't
work.

VGA is a legacy PCI feature: a VGA device can respond to addresses, e.g.,
[mem 0xa-0xb], [io 0x3b0-0x3bb], [io 0x3c0-0x3df], etc., that are
not configurable by BARs.  Consequently, multiple VGA devices can conflict
with each other.  The VGA arbiter avoids conflicts by ensuring that those
legacy resources are only routed to one VGA device at a time.

The arbiter identifies the "default VGA" device, i.e., a legacy VGA device
that was used by boot firmware.  It selects the first device that:

   - is of PCI_CLASS_DISPLAY_VGA,
   - has both PCI_COMMAND_IO and PCI_COMMAND_MEMORY enabled, and
   - has PCI_BRIDGE_CTL_VGA set in all upstream bridges.

Some systems don't have such a device.  For example, if a host bridge
doesn't support I/O space, PCI_COMMAND_IO probably won't be enabled for any
devices below it.  Or, as on the HiSilicon D05, the VGA device may be
behind a bridge that doesn't support PCI_BRIDGE_CTL_VGA, so accesses to the
legacy VGA resources will never reach the device.

This patch extends the arbiter so that if it doesn't find a device that
meets all the above criteria, it selects the first device that:

   - is of PCI_CLASS_DISPLAY_VGA and
   - has PCI_COMMAND_IO or PCI_COMMAND_MEMORY enabled

If it doesn't find even that, it selects the first device that:

   - is of class PCI_CLASS_DISPLAY_VGA.

Such a device may not be able to use the legacy VGA resources, but most
drivers can operate the device without those.  Setting it as the default
device means its "boot_vga" sysfs file will contain "1", which Xorg (via
libpciaccess) uses to help select its default output device.

This fixes Xorg auto-detection on some arm64 systems (HiSilicon D05 in
particular; see the link below).

It also replaces the powerpc fixup_vga() quirk, albeit with slightly
different semantics: the quirk selected the first VGA device we found, and
overrode that selection with any enabled VGA device we found.  If there
were several enabled VGA devices, the *last* one we found would become the
default.

The code here instead selects the *first* enabled VGA device we find, and
if none are enabled, the first VGA device we find.

Link: http://lkml.kernel.org/r/20170901072744.2409-1-...@axtens.net
Tested-by: Daniel Axtens # arm64, ppc64-qemu-tcg
Signed-off-by: Bjorn Helgaas 
---
  arch/powerpc/kernel/pci-common.c |   12 
  drivers/gpu/vga/vgaarb.c |   25 +
  2 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 02831a396419..0ac7aa346c69 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1740,15 +1740,3 @@ static void fixup_hide_host_resource_fsl(struct pci_dev 
*dev)
  }
  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MOTOROLA, PCI_ANY_ID, 
fixup_hide_host_resource_fsl);
  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID, 
fixup_hide_host_resource_fsl);
-
-static void fixup_vga(struct pci_dev *pdev)
-{
-   u16 cmd;
-
-   pci_read_config_word(pdev, PCI_COMMAND, );
-   if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || 
!vga_default_device())
-   vga_set_default_device(pdev);
-
-}
-DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
- PCI_CLASS_DISPLAY_VGA, 8, fixup_vga);
diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
index 76875f6299b8..aeb41f793ed4 100644
--- a/drivers/gpu/vga/vgaarb.c
+++ b/drivers/gpu/vga/vgaarb.c
@@ -1468,6 +1468,31 @@ static int __init vga_arb_device_init(void)
vgaarb_info(dev, "no bridge control possible\n");
}
  
+	if (!vga_default_device()) {

+   list_for_each_entry(vgadev, _list, list) {
+   struct device *dev = >pdev->dev;
+   u16 cmd;
+
+   pdev = vgadev->pdev;
+   pci_read_config_word(pdev, PCI_COMMAND, );
+   if (cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
+   vgaarb_info(dev, "setting as boot device (VGA legacy 
resources not available)\n");
+   vga_set_default_device(pdev);
+   break;
+   }
+   }
+   }
+
+   if (!vga_default_device()) {
+   vgadev = list_first_entry_or_null(_list,
+ struct vga_device, list);
+   if (vgadev) {
+   struct device *dev = >pdev->dev;
+   vgaarb_info(dev, "setting as boot device (VGA 

[PATCH] drm: Replace kzalloc with kcalloc

[Harsha Sharma]

Prefer kcalloc over kzalloc to allocate an array.
This patch fixes checkcpatch issue.

Signed-off-by: Harsha Sharma 
---
 drivers/gpu/drm/drm_crtc_helper.c  | 4 ++--
 drivers/gpu/drm/drm_fb_helper.c| 2 +-
 drivers/gpu/drm/drm_plane_helper.c | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc_helper.c 
b/drivers/gpu/drm/drm_crtc_helper.c
index eab36a460638..ceb131637e2f 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -562,12 +562,12 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set,
 * Allocate space for the backup of all (non-pointer) encoder and
 * connector data.
 */
-   save_encoder_crtcs = kzalloc(dev->mode_config.num_encoder *
+   save_encoder_crtcs = kcalloc(dev->mode_config.num_encoder *
sizeof(struct drm_crtc *), GFP_KERNEL);
if (!save_encoder_crtcs)
return -ENOMEM;
 
-   save_connector_encoders = kzalloc(dev->mode_config.num_connector *
+   save_connector_encoders = kcalloc(dev->mode_config.num_connector *
sizeof(struct drm_encoder *), GFP_KERNEL);
if (!save_connector_encoders) {
kfree(save_encoder_crtcs);
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 1b8f013ffa65..68d197df89fd 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2266,7 +2266,7 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
if (modes[n] == NULL)
return best_score;
 
-   crtcs = kzalloc(fb_helper->connector_count *
+   crtcs = kcalloc(fb_helper->connector_count *
sizeof(struct drm_fb_helper_crtc *), GFP_KERNEL);
if (!crtcs)
return best_score;
diff --git a/drivers/gpu/drm/drm_plane_helper.c 
b/drivers/gpu/drm/drm_plane_helper.c
index 06aee1741e96..74054653530e 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -354,7 +354,7 @@ int drm_primary_helper_update(struct drm_plane *plane, 
struct drm_crtc *crtc,
/* Find current connectors for CRTC */
num_connectors = get_connectors_for_crtc(crtc, NULL, 0);
BUG_ON(num_connectors == 0);
-   connector_list = kzalloc(num_connectors * sizeof(*connector_list),
+   connector_list = kcalloc(num_connectors * sizeof(*connector_list),
 GFP_KERNEL);
if (!connector_list)
return -ENOMEM;
-- 
2.11.0

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

Re: [PATCH 1/3] drm/omap: work-around for omap3 display enable

[Aaro Koskinen]

Hi,

On Wed, Aug 23, 2017 at 12:33:08PM +0300, Tomi Valkeinen wrote:
> On 13/06/17 12:02, Tomi Valkeinen wrote:
> > Seems that on omap3 enabling a crtc without any planes causes a sync
> > lost flood. This only happens on the first enable, and after that it
> > works. This looks like an HW issue.
> > 
> > It's unclear why this is happening or how to fix it, but as a quick
> > work-around, this patch enables i734 errata work-around for omap2 and
> > omap3 too. The errata work-around enables and disables the LCD output
> > with a plane once when waking up the DSS IP, and it seems to resolve the
> > omap3 problem too. It is unclear if omap2 has the same issue, but it
> > probably has and the WA should have no side effects so it should be safe
> > to enable on omap2 too.
> 
> I was again debugging and testing this problem, and I don't think this patch
> works very well. I'm getting endless sync losts again.
> 
> Here's another try, this time changing how the omapdrm commits the new setup.
> At least for me this seems to work better.
>
> From fc5cc9678e130196012c17b37e555d53d3d3476b Mon Sep 17 00:00:00 2001
> From: Tomi Valkeinen 
> Date: Wed, 23 Aug 2017 12:19:02 +0300
> Subject: [PATCHv2] drm/omap: work-around for omap3 display enable

Thanks! I was finally able to test this (with v4.13). Multiple boots
and now the display is always working fine.

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

Re: [PATCH] drm/udl: Fixed problem with UDL adpater reconnection

[Robert Tarasov]

Fixed.

On Thu, Oct 12, 2017 at 12:56 PM, Alex Deucher 
wrote:

> On Wed, Oct 11, 2017 at 4:41 PM, Robert Tarasov
>  wrote:
> > Fixed problem with DisplayLink and DisplayLink certified adapters when
> they
> > didn't want to work if they were initialized with disconnected DVI
> cable. Now
> > udl driver checks and updates adapter's connection state every 10
> seconds, as
> > well as retreives all the edid data blocks instead of only base one.
> Previous
> > approch could lead to improper initialization of video mode with certain
> > monitors.
> >
>
> Seems like this should be split into two patches:
>
> 1. rework the EDID handling in the driver
> 2. enable drm connector polling.
>
> Alex
>
> > Signed-off-by: Robert Tarasov 
> > ---
> >  drivers/gpu/drm/udl/udl_connector.c | 153
> 
> >  drivers/gpu/drm/udl/udl_connector.h |  13 +++
> >  drivers/gpu/drm/udl/udl_drv.c   |   4 +
> >  drivers/gpu/drm/udl/udl_main.c  |   5 ++
> >  4 files changed, 125 insertions(+), 50 deletions(-)
> >  create mode 100644 drivers/gpu/drm/udl/udl_connector.h
> >
> > diff --git a/drivers/gpu/drm/udl/udl_connector.c
> b/drivers/gpu/drm/udl/udl_connector.c
> > index 9f9a49748d17..b2d9ffcc29aa 100644
> > --- a/drivers/gpu/drm/udl/udl_connector.c
> > +++ b/drivers/gpu/drm/udl/udl_connector.c
> > @@ -14,70 +14,95 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include "udl_connector.h"
> >  #include "udl_drv.h"
> >
> > -/* dummy connector to just get EDID,
> > -   all UDL appear to have a DVI-D */
> > -
> > -static u8 *udl_get_edid(struct udl_device *udl)
> > +static bool udl_get_edid_block(struct udl_device *udl, int block_idx,
> > +  u8 *buff)
> >  {
> > -   u8 *block;
> > -   char *rbuf;
> > int ret, i;
> > +   u8 *read_buff;
> >
> > -   block = kmalloc(EDID_LENGTH, GFP_KERNEL);
> > -   if (block == NULL)
> > -   return NULL;
> > -
> > -   rbuf = kmalloc(2, GFP_KERNEL);
> > -   if (rbuf == NULL)
> > -   goto error;
> > +   read_buff = kmalloc(2, GFP_KERNEL);
> > +   if (!read_buff)
> > +   return false;
> >
> > for (i = 0; i < EDID_LENGTH; i++) {
> > +   int bval = (i + block_idx * EDID_LENGTH) << 8;
> > ret = usb_control_msg(udl->udev,
> > - usb_rcvctrlpipe(udl->udev, 0),
> (0x02),
> > - (0x80 | (0x02 << 5)), i << 8,
> 0xA1, rbuf, 2,
> > - HZ);
> > + usb_rcvctrlpipe(udl->udev, 0),
> > + (0x02), (0x80 | (0x02 << 5)),
> bval,
> > + 0xA1, read_buff, 2, HZ);
> > if (ret < 1) {
> > DRM_ERROR("Read EDID byte %d failed err %x\n",
> i, ret);
> > -   goto error;
> > +   kfree(read_buff);
> > +   return false;
> > }
> > -   block[i] = rbuf[1];
> > +   buff[i] = read_buff[1];
> > }
> >
> > -   kfree(rbuf);
> > -   return block;
> > -
> > -error:
> > -   kfree(block);
> > -   kfree(rbuf);
> > -   return NULL;
> > +   kfree(read_buff);
> > +   return true;
> >  }
> >
> > -static int udl_get_modes(struct drm_connector *connector)
> > +static bool udl_get_edid(struct udl_device *udl, u8 **result_buff,
> > +int *result_buff_size)
> >  {
> > -   struct udl_device *udl = connector->dev->dev_private;
> > -   struct edid *edid;
> > -   int ret;
> > -
> > -   edid = (struct edid *)udl_get_edid(udl);
> > -   if (!edid) {
> > -   drm_mode_connector_update_edid_property(connector,
> NULL);
> > -   return 0;
> > +   int i, extensions;
> > +   u8 *block_buff = NULL, *buff_ptr;
> > +
> > +   block_buff = kmalloc(EDID_LENGTH, GFP_KERNEL);
> > +   if (block_buff == NULL)
> > +   return false;
> > +
> > +   if (udl_get_edid_block(udl, 0, block_buff) &&
> > +   memchr_inv(block_buff, 0, EDID_LENGTH)) {
> > +   extensions = ((struct edid *)block_buff)->extensions;
> > +   if (extensions > 0) {
> > +   /* we have to read all extensions one by one */
> > +   *result_buff_size = EDID_LENGTH * (extensions +
> 1);
> > +   *result_buff = kmalloc(*result_buff_size,
> GFP_KERNEL);
> > +   buff_ptr = *result_buff;
> > +   if (buff_ptr == NULL) {
> > +   kfree(block_buff);
> > +   return false;
> > +   }
> > +   memcpy(buff_ptr, block_buff, EDID_LENGTH);
> > +

Re: [PATCH] drm/tilcdc: Precalculate total frametime in tilcdc_crtc_set_mode()

[Tomi Valkeinen]


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

On 12/10/17 17:16, Jyri Sarha wrote:
> We need the frame refresh time to check if we are too close to
> vertical sync when updating the two framebuffer DMA registers and risk
> a collision. This new method is more accurate that the previous that
> based on mode's vrefresh value, which itself may also be inaccurate
> or not even initialized.
> 
> Reported-by: Kevin Hao 
> Fixes: 11abbc9f39e0 ("drm/tilcdc: Set framebuffer DMA address to HW only if 
> CRTC is enabled")
> Cc:  # v4.11+
> Signed-off-by: Jyri Sarha 
> ---
> This patch is inteded to replace this:
> https://lists.freedesktop.org/archives/dri-devel/2017-October/154556.html
> 
>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 16 +++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c 
> b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> index 704db24..b2f2170 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> @@ -48,6 +48,7 @@ struct tilcdc_crtc {
>   unsigned int lcd_fck_rate;
>  
>   ktime_t last_vblank;
> + unsigned int frametime_us;
>  
>   struct drm_framebuffer *curr_fb;
>   struct drm_framebuffer *next_fb;
> @@ -292,6 +293,16 @@ static void tilcdc_crtc_set_clk(struct drm_crtc *crtc)
>   LCDC_V2_CORE_CLK_EN);
>  }
>  
> +u32 tilcdc_mode_frametime(const struct drm_display_mode *mode)
> +{
> + u32 totalframes = mode->htotal * mode->vtotal;

"totalframes"? Shouldn't it be total pixels (including blanking pixels)?
Or maybe "hvtotal", mimicking the htotal and vtotal.

> +
> + if (totalframes < U32_MAX / 1000u)
> + return (1000u * totalframes) / mode->clock;
> + else
> + return 10u * ((100u * totalframes) / mode->clock);

Just pick one, I don't think there's benefit in the above if. Perhaps go
via u64 div.

 Tomi

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

Re: [PATCH v2 1/8] drm/rockchip/dsi: correct Feedback divider setting

[Archit Taneja]

Hi,

On 09/26/2017 01:25 PM, Nickey Yang wrote:

This patch correct Feedback divider setting:
1、Set Feedback divider [8:5] when HIGH_PROGRAM_EN
2、Due to the use of a "by 2 pre-scaler," the range of the
feedback multiplication Feedback divider is limited to even
division numbers, and Feedback divider must be greater than
12, less than 1000.
3、Make the previously configured Feedback divider(LSB)
factors effective
4、Add the definition of the MIPI PHY register.


Slightly unrelated topic: We now have a generic dw-mipi-dsi bridge
driver. Can we consider moving to that instead of adding new features
within the rockchip kms driver?

Thanks,
Archit



Signed-off-by: Nickey Yang 
---
  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 219 ++---
  1 file changed, 146 insertions(+), 73 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
index 9a20b9d..c933a3a 100644
--- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
@@ -228,7 +228,7 @@
  #define LOW_PROGRAM_EN0
  #define HIGH_PROGRAM_EN   BIT(7)
  #define LOOP_DIV_LOW_SEL(val) (((val) - 1) & 0x1f)
-#define LOOP_DIV_HIGH_SEL(val) val) - 1) >> 5) & 0x1f)
+#define LOOP_DIV_HIGH_SEL(val) val) - 1) >> 5) & 0xf)
  #define PLL_LOOP_DIV_EN   BIT(5)
  #define PLL_INPUT_DIV_EN  BIT(4)
  
@@ -254,6 +254,28 @@

  #define DW_MIPI_NEEDS_PHY_CFG_CLK BIT(0)
  #define DW_MIPI_NEEDS_GRF_CLK BIT(1)
  
+#define PLL_BIAS_CUR_SEL_CAP_VCO_CONTROL 0x10

+#define PLL_CP_CONTROL_PLL_LOCK_BYPASS 0x11
+#define PLL_LPF_AND_CP_CONTROL 0x12
+#define PLL_INPUT_DIVIDER_RATIO 0x17
+#define PLL_LOOP_DIVIDER_RATIO 0x18
+#define PLL_INPUT_AND_LOOP_DIVIDER_RATIOS_CONTROL 0x19
+#define BANDGAP_AND_BIAS_CONTROL 0x20
+#define TERMINATION_RESISTER_CONTROL 0x21
+#define AFE_BIAS_BANDGAP_ANALOG_PROGRAMMABILITY 0x22
+#define HS_RX_CONTROL_OF_LANE_0 0x44
+#define HS_TX_CLOCK_LANE_REQUEST_STATE_TIME_CONTROL 0x60
+#define HS_TX_CLOCK_LANE_PREPARE_STATE_TIME_CONTROL 0x61
+#define HS_TX_CLOCK_LANE_HS_ZERO_STATE_TIME_CONTROL 0x62
+#define HS_TX_CLOCK_LANE_TRAIL_STATE_TIME_CONTROL 0x63
+#define HS_TX_CLOCK_LANE_EXIT_STATE_TIME_CONTROL 0x64
+#define HS_TX_CLOCK_LANE_POST_TIME_CONTROL 0x65
+#define HS_TX_DATA_LANE_REQUEST_STATE_TIME_CONTROL 0x70
+#define HS_TX_DATA_LANE_PREPARE_STATE_TIME_CONTROL 0x71
+#define HS_TX_DATA_LANE_HS_ZERO_STATE_TIME_CONTROL 0x72
+#define HS_TX_DATA_LANE_TRAIL_STATE_TIME_CONTROL 0x73
+#define HS_TX_DATA_LANE_EXIT_STATE_TIME_CONTROL 0x74
+
  enum {
BANDGAP_97_07,
BANDGAP_98_05,
@@ -447,53 +469,79 @@ static int dw_mipi_dsi_phy_init(struct dw_mipi_dsi *dsi)
return ret;
}
  
-	dw_mipi_dsi_phy_write(dsi, 0x10, BYPASS_VCO_RANGE |

-VCO_RANGE_CON_SEL(vco) |
-VCO_IN_CAP_CON_LOW |
-REF_BIAS_CUR_SEL);
-
-   dw_mipi_dsi_phy_write(dsi, 0x11, CP_CURRENT_3MA);
-   dw_mipi_dsi_phy_write(dsi, 0x12, CP_PROGRAM_EN | LPF_PROGRAM_EN |
-LPF_RESISTORS_20_KOHM);
-
-   dw_mipi_dsi_phy_write(dsi, 0x44, HSFREQRANGE_SEL(testdin));
-
-   dw_mipi_dsi_phy_write(dsi, 0x17, INPUT_DIVIDER(dsi->input_div));
-   dw_mipi_dsi_phy_write(dsi, 0x18, LOOP_DIV_LOW_SEL(dsi->feedback_div) |
-LOW_PROGRAM_EN);
-   dw_mipi_dsi_phy_write(dsi, 0x18, LOOP_DIV_HIGH_SEL(dsi->feedback_div) |
-HIGH_PROGRAM_EN);
-   dw_mipi_dsi_phy_write(dsi, 0x19, PLL_LOOP_DIV_EN | PLL_INPUT_DIV_EN);
-
-   dw_mipi_dsi_phy_write(dsi, 0x22, LOW_PROGRAM_EN |
-BIASEXTR_SEL(BIASEXTR_127_7));
-   dw_mipi_dsi_phy_write(dsi, 0x22, HIGH_PROGRAM_EN |
-BANDGAP_SEL(BANDGAP_96_10));
-
-   dw_mipi_dsi_phy_write(dsi, 0x20, POWER_CONTROL | INTERNAL_REG_CURRENT |
-BIAS_BLOCK_ON | BANDGAP_ON);
-
-   dw_mipi_dsi_phy_write(dsi, 0x21, TER_RESISTOR_LOW | TER_CAL_DONE |
-SETRD_MAX | TER_RESISTORS_ON);
-   dw_mipi_dsi_phy_write(dsi, 0x21, TER_RESISTOR_HIGH | LEVEL_SHIFTERS_ON |
-SETRD_MAX | POWER_MANAGE |
-TER_RESISTORS_ON);
-
-   dw_mipi_dsi_phy_write(dsi, 0x60, TLP_PROGRAM_EN | ns2bc(dsi, 500));
-   dw_mipi_dsi_phy_write(dsi, 0x61, THS_PRE_PROGRAM_EN | ns2ui(dsi, 40));
-   dw_mipi_dsi_phy_write(dsi, 0x62, THS_ZERO_PROGRAM_EN | ns2bc(dsi, 300));
-   dw_mipi_dsi_phy_write(dsi, 0x63, THS_PRE_PROGRAM_EN | ns2ui(dsi, 100));
-   dw_mipi_dsi_phy_write(dsi, 0x64, BIT(5) | ns2bc(dsi, 100));
-   dw_mipi_dsi_phy_write(dsi, 0x65, BIT(5) | (ns2bc(dsi, 60) + 7));
-
-   dw_mipi_dsi_phy_write(dsi, 0x70, 

Re: RFC: page-flip with damage?

[Pekka Paalanen]

On Thu, 12 Oct 2017 10:51:22 -0400
Sean Paul  wrote:

> On Thu, Oct 12, 2017 at 01:55:40PM +0300, Pekka Paalanen wrote:
> > On Tue, 26 Sep 2017 09:07:45 -0700
> > Thomas Hellstrom  wrote:
> >   
> > > On 09/26/2017 01:18 AM, Daniel Vetter wrote:  
> > > > On Sun, Sep 24, 2017 at 07:41:45PM +0200, Thomas Hellstrom wrote:
> > > >> Hi, list!
> > > >>
> > > >> Page flips, while efficient on real hardware, aren't that efficient in 
> > > >> other
> > > >> situations, like for virtual devices with local, or even worse, remote
> > > >> desktops.
> > > >> We might ending up forwarding or encoding a couple of full frames 
> > > >> worth of
> > > >> data instead of a small region at a cursor blink.
> > > >>
> > > >> Now there is this extension EGL_KHR_swap_buffers_with_damage, and
> > > >> gnome-shell/wayland on KMS also has a damage region that it forwards 
> > > >> all the
> > > >> way down to the function where page-flip is called.  
> >   
> > > > Wrt single dirty rect vs. rectlist: I'd opt for a single rect since that
> > > > makes the interface easier. Currently most drivers collapes the list
> > > > passed to fb_funcs->dirty to a single rect, so that seems good enough, 
> > > > and
> > > > a nice simplification of the interface (both uapi and driver).
> > > 
> > > I think multiple cliprects had its benefits for old X type rendering: 
> > > Frontbuffer-type diagonal
> > > lines benchmarked with x11perf and similar. Now that everybody's 
> > > compositing that use-case is mostly gone, and as you say
> > > most driver coalesce anyway. Even we to some extent, at least on newer 
> > > "hardware" versions.  
> > 
> > Hi,
> > 
> > it still is awfully easy to create a pathological use case where
> > collapsing into a single rect adds a thousand-fold more pixels to the
> > damage than having more than one rectangle would allow: two tiny
> > animations at the opposite corners of a screen.
> > 
> > I'm thinking Wayland compositors here.
> > 
> > Simplifying damage regions while not crashing down to just one
> > rectangle is quite possible I believe. Let userspace do simplification
> > from hundreds down to a few rectangles,   
> 
> Sounds like you're making a case for overlay planes here :-)
> 
> Perhaps migrating the damage rect to plane state would make everyone happy. In
> the case where the hardware or compositor only supports one plane, there is 
> one
> big rect. In the case where multiple planes are used, you rely on the 
> compositor
> to make plane assignments and pass down the individual damage regions per 
> plane.

Hi Sean,

no, I wasn't really. I was thinking of a single big Wayland app window,
that has small changes roughly in opposite corners. Or two windows far
apart on the same output, updating at the same, and being composited
into a single framebuffer, e.g. because they are software-rendered.

The real world use case I have hit was with Xwayland with a fullscreen
X11 window getting these little updates scattered around. Xwayland
collapsing the damage into a single rect to be submitted to Weston
caused Weston to spend a lot of time in the pixman-renderer (software
compositing). Fixing it to submit detailed damage regions reduced
Weston's CPU usage significantly, down to a quarter or something in
that magnitude, if I recall correctly. This is an extreme example,
comparing a single rect to the original hundreds of rects. In practice,
one should have a damage region simplifier that optimizes the trade-off
between more pixels and more rects.

That example is not directly applicable to the question at hand, but I
do believe it extrapolates somewhat on the downsides of using only a
single rectangle per plane or crtc on cases where damage is a useful
hint.

After all, 2D-compositors should be able to provide detailed damage
regions if they are allowed to. Would this not also be in the interest
of power saving?


Thanks,
pq

> > and then collapse into one
> > rectangle in the driver if absolutely necessary. That's what I'd hope.
> > 
> > It is totally fine to require non-overlapping rectangles. You could
> > even demand a specific layout of rectangles, e.g. the form Pixman
> > regions use. You can also put an arbitrary cap on how many rectangles
> > are allowed.
> > 


pgpxnVDTCEIGb.pgp
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[radeon-alex:amd-staging-drm-next 120/173] drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_vba.c:2943:5-35: duplicated argument to && or || (fwd)

[Julia Lawall]

Hello,

There are two checks for dm_444_16on line 2943.

julia

-- Forwarded message --
Date: Fri, 13 Oct 2017 08:24:34 +0800
From: kbuild test robot 
To: kbu...@01.org
Cc: Julia Lawall 
Subject: [radeon-alex:amd-staging-drm-next 120/173]
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_vba.c:2943:5-35:
duplicated argument to && or ||

CC: kbuild-...@01.org
CC: dri-devel@lists.freedesktop.org
CC: Harry Wentland 
CC: Tony Cheng 

tree:   git://people.freedesktop.org/~agd5f/linux.git amd-staging-drm-next
head:   1b006d838f78d7822f606087fa12ea7ad2c5753b
commit: 3cc51acd2adc70b6ad7dc325ff87e8a253e764d3 [120/173] drm/amd/display: 
Restructuring and cleaning up DML
:: branch date: 7 hours ago
:: commit date: 7 hours ago

>> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_vba.c:2943:5-35: 
>> duplicated argument to && or ||

git remote add radeon-alex git://people.freedesktop.org/~agd5f/linux.git
git remote update radeon-alex
git checkout 3cc51acd2adc70b6ad7dc325ff87e8a253e764d3
vim +2943 drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_vba.c

3cc51acd Dmytro Laktyushkin 2017-08-23  2932
3cc51acd Dmytro Laktyushkin 2017-08-23  2933  bool Calculate256BBlockSizes(enum 
source_format_class SourcePixelFormat,
3cc51acd Dmytro Laktyushkin 2017-08-23  2934enum dm_swizzle_mode 
SurfaceTiling,
3cc51acd Dmytro Laktyushkin 2017-08-23  2935unsigned int BytePerPixelY,
3cc51acd Dmytro Laktyushkin 2017-08-23  2936unsigned int BytePerPixelC,
3cc51acd Dmytro Laktyushkin 2017-08-23  2937unsigned int 
*BlockHeight256BytesY,
3cc51acd Dmytro Laktyushkin 2017-08-23  2938unsigned int 
*BlockHeight256BytesC,
3cc51acd Dmytro Laktyushkin 2017-08-23  2939unsigned int 
*BlockWidth256BytesY,
3cc51acd Dmytro Laktyushkin 2017-08-23  2940unsigned int 
*BlockWidth256BytesC)
3cc51acd Dmytro Laktyushkin 2017-08-23  2941  {
3cc51acd Dmytro Laktyushkin 2017-08-23  2942if ((SourcePixelFormat == 
dm_444_64 || SourcePixelFormat == dm_444_32
3cc51acd Dmytro Laktyushkin 2017-08-23 @2943|| SourcePixelFormat == 
dm_444_16 || SourcePixelFormat == dm_444_16
3cc51acd Dmytro Laktyushkin 2017-08-23  2944|| SourcePixelFormat == 
dm_444_8)) {
3cc51acd Dmytro Laktyushkin 2017-08-23  2945if (SurfaceTiling == 
dm_sw_linear) {
3cc51acd Dmytro Laktyushkin 2017-08-23  2946
*BlockHeight256BytesY = 1;
3cc51acd Dmytro Laktyushkin 2017-08-23  2947} else if 
(SourcePixelFormat == dm_444_64) {
3cc51acd Dmytro Laktyushkin 2017-08-23  2948
*BlockHeight256BytesY = 4;
3cc51acd Dmytro Laktyushkin 2017-08-23  2949} else if 
(SourcePixelFormat == dm_444_8) {
3cc51acd Dmytro Laktyushkin 2017-08-23  2950
*BlockHeight256BytesY = 16;
3cc51acd Dmytro Laktyushkin 2017-08-23  2951} else {
3cc51acd Dmytro Laktyushkin 2017-08-23  2952
*BlockHeight256BytesY = 8;
3cc51acd Dmytro Laktyushkin 2017-08-23  2953}
3cc51acd Dmytro Laktyushkin 2017-08-23  2954*BlockWidth256BytesY = 
256 / BytePerPixelY / *BlockHeight256BytesY;
3cc51acd Dmytro Laktyushkin 2017-08-23  2955*BlockHeight256BytesC = 
0;
3cc51acd Dmytro Laktyushkin 2017-08-23  2956*BlockWidth256BytesC = 
0;
3cc51acd Dmytro Laktyushkin 2017-08-23  2957} else {
3cc51acd Dmytro Laktyushkin 2017-08-23  2958if (SurfaceTiling == 
dm_sw_linear) {
3cc51acd Dmytro Laktyushkin 2017-08-23  2959
*BlockHeight256BytesY = 1;
3cc51acd Dmytro Laktyushkin 2017-08-23  2960
*BlockHeight256BytesC = 1;
3cc51acd Dmytro Laktyushkin 2017-08-23  2961} else if 
(SourcePixelFormat == dm_420_8) {
3cc51acd Dmytro Laktyushkin 2017-08-23  2962
*BlockHeight256BytesY = 16;
3cc51acd Dmytro Laktyushkin 2017-08-23  2963
*BlockHeight256BytesC = 8;
3cc51acd Dmytro Laktyushkin 2017-08-23  2964} else {
3cc51acd Dmytro Laktyushkin 2017-08-23  2965
*BlockHeight256BytesY = 8;
3cc51acd Dmytro Laktyushkin 2017-08-23  2966
*BlockHeight256BytesC = 8;
3cc51acd Dmytro Laktyushkin 2017-08-23  2967}
3cc51acd Dmytro Laktyushkin 2017-08-23  2968*BlockWidth256BytesY = 
256 / BytePerPixelY / *BlockHeight256BytesY;
3cc51acd Dmytro Laktyushkin 2017-08-23  2969*BlockWidth256BytesC = 
256 / BytePerPixelC / *BlockHeight256BytesC;
3cc51acd Dmytro Laktyushkin 2017-08-23  2970}
3cc51acd Dmytro Laktyushkin 2017-08-23  2971return true;
3cc51acd Dmytro Laktyushkin 2017-08-23  2972  }
3cc51acd Dmytro Laktyushkin 2017-08-23  2973

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

[Bug 100443] DMESG: [powerplay] Can't find requested voltage id in vdd_dep_on_sclk table!

[bugzilla-daemon]

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

--- Comment #34 from Devin Prince  ---
Having this same issue on kernel 4.13.5-1-ARCH.

CPU: AMD R7-1700X
Motherboard: AsRock Taichi X370
GPU: XFX RX-480 8GB

I have found a temporary solution that seems to allow me to actually boot and
use the card, however.

Pass the kernel parameter:
amdgpu.dpm=0
to shut off the Dynamic Power Management module, which contains AMD's
powerplay, and everything works fine.

I'm unsure what sort of limitations this introduces, but for the time being, it
will get your system working.

-- 
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 01/10] drm/panel: Keep track of enabled/prepared

[Andrzej Hajda]

On 12.10.2017 19:55, Sean Paul wrote:
> This patch adds state tracking to the drm_panel functions which keep
> track of enabled and prepared. If the calls are unbalanced, a WARNING is
> issued.
>
> The motivation for this change is that a number of panel drivers
> (including panel-simple) all do this to protect their regulator
> refcounts. The atomic framework ensures the calls are balanced, and
> there  aren't any panel drivers being used by legacy drivers. As such,
> these checks are useless, but let's add a WARNING just in case something
> crazy happens (like a legacy driver using a panel).
>
> Less code == better.
>
> Changes in v2:
>  - Reduced enabled/prepared bools to one enum (Andrzej)
>  - Don't update state if the operation fails (Andrzej)
>  - Issue warning even if operation is not implemented
>
> Cc: Andrzej Hajda 
> Signed-off-by: Sean Paul 
> ---
>  drivers/gpu/drm/drm_panel.c |  1 +
>  include/drm/drm_panel.h | 60 
> +++--
>  2 files changed, 53 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> index 308d442a531b..e5957e7da768 100644
> --- a/drivers/gpu/drm/drm_panel.c
> +++ b/drivers/gpu/drm/drm_panel.c
> @@ -48,6 +48,7 @@ static LIST_HEAD(panel_list);
>  void drm_panel_init(struct drm_panel *panel)
>  {
>   INIT_LIST_HEAD(>list);
> + panel->state = DRM_PANEL_POWER_OFF;
>  }
>  EXPORT_SYMBOL(drm_panel_init);
>  
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index 14ac240a1f64..425461c4c574 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -24,6 +24,7 @@
>  #ifndef __DRM_PANEL_H__
>  #define __DRM_PANEL_H__
>  
> +#include 
>  #include 
>  #include 
>  
> @@ -84,6 +85,7 @@ struct drm_panel_funcs {
>   * @dev: parent device of the panel
>   * @funcs: operations that can be performed on the panel
>   * @list: panel entry in registry
> + * @power_state: keeps track of the panel power status
>   */
>  struct drm_panel {
>   struct drm_device *drm;
> @@ -93,6 +95,12 @@ struct drm_panel {
>   const struct drm_panel_funcs *funcs;
>  
>   struct list_head list;
> +
> + enum {
> + DRM_PANEL_POWER_OFF,
> + DRM_PANEL_POWER_PREPARED,
> + DRM_PANEL_POWER_ENABLED
> + } state;
>  };
>  
>  /**
> @@ -104,12 +112,21 @@ struct drm_panel {
>   * is usually no longer possible to communicate with the panel until another
>   * call to drm_panel_prepare().
>   *
> + * Atomic framework should ensure that prepare/unprepare are properly 
> balanced.
> + * If this is not the case, a WARNING will be issued.
> + *
>   * Return: 0 on success or a negative error code on failure.
>   */
>  static inline int drm_panel_unprepare(struct drm_panel *panel)
>  {
> - if (panel && panel->funcs && panel->funcs->unprepare)
> - return panel->funcs->unprepare(panel);
> + WARN_ON(panel->state != DRM_PANEL_POWER_PREPARED);
> +
> + if (panel && panel->funcs && panel->funcs->unprepare) {
> + int ret = panel->funcs->unprepare(panel);
> + if (!ret)
> + panel->state = DRM_PANEL_POWER_OFF;
> + return ret;
> + }
>  
>   return panel ? -ENOSYS : -EINVAL;

State check and state change should be performed also for panels without
callbacks, otherwise it will work only with panels having full set of
callbacks.
Another thing is that missing callback does not mean
function-not-implemented (ENOSYS), it is rather nothing-to-do, in such
case function should return rather 0, this will simplify also error
handling on callers side.

And one thing just to consider: all functions have almost the same body,
maybe it would be good to use some helper, for example:

    #define drm_panel_state_change(panel, from, to, func) ...

    int drm_panel_unprepare(struct drm_panel *panel)
    {
        return drm_panel_state_change(panel, DRM_PANEL_POWER_PREPARED,
DRM_PANEL_POWER_OFF, unprepare);
    }


Regards
Andrzej
>  }
> @@ -122,12 +139,21 @@ static inline int drm_panel_unprepare(struct drm_panel 
> *panel)
>   * drivers. For smart panels it should still be possible to communicate with
>   * the integrated circuitry via any command bus after this call.
>   *
> + * Atomic framework should ensure that enable/disable are properly balanced.
> + * If this is not the case, a WARNING will be issued.
> + *
>   * Return: 0 on success or a negative error code on failure.
>   */
>  static inline int drm_panel_disable(struct drm_panel *panel)
>  {
> - if (panel && panel->funcs && panel->funcs->disable)
> - return panel->funcs->disable(panel);
> + WARN_ON(panel->state != DRM_PANEL_POWER_ENABLED);
> +
> + if (panel && panel->funcs && panel->funcs->disable) {
> + int ret = panel->funcs->disable(panel);
> + if (!ret)
> + panel->state = DRM_PANEL_POWER_PREPARED;
> +

Re: [pull] amdgpu drm-fixes-4.14

[Michel Dänzer]

On 12/10/17 07:49 PM, Alex Deucher wrote:
> On Thu, Oct 12, 2017 at 1:02 PM, Christian König
>  wrote:
>> Am 12.10.2017 um 18:20 schrieb Michel Dänzer:
>>> On 12/10/17 05:58 PM, Alex Deucher wrote:

 Hi Dave,

 One memory management regression fix.

 The following changes since commit
 545036a9944e9d6e50fed4ca03117147c880ff71:

Merge tag 'drm-misc-fixes-2017-10-11' of
 git://anongit.freedesktop.org/drm/drm-misc into drm-fixes (2017-10-12
 10:38:09 +1000)

 are available in the git repository at:

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

 for you to fetch changes up to 27b94b4f1386c3a8181f5a0277434a32e24e7dd7:

drm/amdgpu: fix placement flags in amdgpu_ttm_bind (2017-10-12
 10:34:42 -0400)

 
 Christian König (1):
drm/amdgpu: fix placement flags in amdgpu_ttm_bind
>>>
>>> Thanks Alex, but there's another piglit hang regression in 4.14, caused
>>> by commit 6af0883ed977 "drm/amdgpu: discard commands of killed
>>> processes", fixed by five commits 6b37d03280a4..318d85de9c20 in
>>> amd-staging-drm-next. Either the latter need to be backported to 4.14,
>>> or the former needs to be reverted from it.
>>
>>
>> The revert is probably easier to handle at this point.
>>
>> So to answer your question from the other thread I vote for that.
> 
> Nicolai's patches apply cleanly and I think they change about the same
> amount of code and we don't have to worry about any problems down the
> road when the revert gets merged into drm-next.

That's basically why I asked which way to go. However, Monk just
reported a potential regression in one of Nicolai's changes, so
reverting seems safer for 4.14.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PULL] drm-intel-next

[Jani Nikula]

On Fri, 13 Oct 2017, Dave Airlie  wrote:
> On 13 October 2017 at 01:23, Jani Nikula  wrote:
>> On Wed, 11 Oct 2017, Jani Nikula  wrote:
>>> Hi Dave, more v4.15 features.
>>
>> Okay, so I suck and there's still one more batch to come after this. I'm
>> a bit out of rhythm here. When do you want the pull request for that at
>> the latest?
>
> Around rc6 time I think is when I generally get the last one. I sometimes let
> Daniel slip a week or so once it was all in CI and -next at that time.

Okay, I tagged the last batch yesterday, will send it your way next
week.

Thanks,
Jani.

-- 
Jani Nikula, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm: Replace kzalloc with kcalloc

[Jani Nikula]

On Fri, 13 Oct 2017, Harsha Sharma  wrote:
> Prefer kcalloc over kzalloc to allocate an array.
> This patch fixes checkcpatch issue.
>
> Signed-off-by: Harsha Sharma 

Reviewed-by: Jani Nikula 


> ---
> Changes in v2:
>  -kcalloc will take 3 arguments
>
>  drivers/gpu/drm/drm_crtc_helper.c  | 4 ++--
>  drivers/gpu/drm/drm_fb_helper.c| 2 +-
>  drivers/gpu/drm/drm_plane_helper.c | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c 
> b/drivers/gpu/drm/drm_crtc_helper.c
> index eab36a460638..5a84c3bc915d 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -562,12 +562,12 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set,
>* Allocate space for the backup of all (non-pointer) encoder and
>* connector data.
>*/
> - save_encoder_crtcs = kzalloc(dev->mode_config.num_encoder *
> + save_encoder_crtcs = kcalloc(dev->mode_config.num_encoder,
>   sizeof(struct drm_crtc *), GFP_KERNEL);
>   if (!save_encoder_crtcs)
>   return -ENOMEM;
>  
> - save_connector_encoders = kzalloc(dev->mode_config.num_connector *
> + save_connector_encoders = kcalloc(dev->mode_config.num_connector,
>   sizeof(struct drm_encoder *), GFP_KERNEL);
>   if (!save_connector_encoders) {
>   kfree(save_encoder_crtcs);
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 1b8f013ffa65..de31e52ab9cb 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2266,7 +2266,7 @@ static int drm_pick_crtcs(struct drm_fb_helper 
> *fb_helper,
>   if (modes[n] == NULL)
>   return best_score;
>  
> - crtcs = kzalloc(fb_helper->connector_count *
> + crtcs = kcalloc(fb_helper->connector_count,
>   sizeof(struct drm_fb_helper_crtc *), GFP_KERNEL);
>   if (!crtcs)
>   return best_score;
> diff --git a/drivers/gpu/drm/drm_plane_helper.c 
> b/drivers/gpu/drm/drm_plane_helper.c
> index 06aee1741e96..759ed93f4ba8 100644
> --- a/drivers/gpu/drm/drm_plane_helper.c
> +++ b/drivers/gpu/drm/drm_plane_helper.c
> @@ -354,7 +354,7 @@ int drm_primary_helper_update(struct drm_plane *plane, 
> struct drm_crtc *crtc,
>   /* Find current connectors for CRTC */
>   num_connectors = get_connectors_for_crtc(crtc, NULL, 0);
>   BUG_ON(num_connectors == 0);
> - connector_list = kzalloc(num_connectors * sizeof(*connector_list),
> + connector_list = kcalloc(num_connectors, sizeof(*connector_list),
>GFP_KERNEL);
>   if (!connector_list)
>   return -ENOMEM;

-- 
Jani Nikula, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [pull] amdgpu drm-fixes-4.14

[Christian König]

Am 13.10.2017 um 09:41 schrieb Michel Dänzer:

On 12/10/17 07:49 PM, Alex Deucher wrote:

On Thu, Oct 12, 2017 at 1:02 PM, Christian König
 wrote:

Am 12.10.2017 um 18:20 schrieb Michel Dänzer:

On 12/10/17 05:58 PM, Alex Deucher wrote:

Hi Dave,

One memory management regression fix.

The following changes since commit
545036a9944e9d6e50fed4ca03117147c880ff71:

Merge tag 'drm-misc-fixes-2017-10-11' of
git://anongit.freedesktop.org/drm/drm-misc into drm-fixes (2017-10-12
10:38:09 +1000)

are available in the git repository at:

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

for you to fetch changes up to 27b94b4f1386c3a8181f5a0277434a32e24e7dd7:

drm/amdgpu: fix placement flags in amdgpu_ttm_bind (2017-10-12
10:34:42 -0400)


Christian König (1):
drm/amdgpu: fix placement flags in amdgpu_ttm_bind

Thanks Alex, but there's another piglit hang regression in 4.14, caused
by commit 6af0883ed977 "drm/amdgpu: discard commands of killed
processes", fixed by five commits 6b37d03280a4..318d85de9c20 in
amd-staging-drm-next. Either the latter need to be backported to 4.14,
or the former needs to be reverted from it.


The revert is probably easier to handle at this point.

So to answer your question from the other thread I vote for that.

Nicolai's patches apply cleanly and I think they change about the same
amount of code and we don't have to worry about any problems down the
road when the revert gets merged into drm-next.

That's basically why I asked which way to go. However, Monk just
reported a potential regression in one of Nicolai's changes, so
reverting seems safer for 4.14.


I agree that reverting the original offending patch is probably the 
better approach.


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

Re: [PATCH 2/6] drm/amd/display: Use new DRM API where possible

[Harry Wentland]

On 2017-10-13 01:26 PM, Andrey Grodzovsky wrote:
> 
> 
> On 10/13/2017 12:18 PM, Harry Wentland wrote:
>> On 2017-10-12 05:15 PM, sunpeng...@amd.com wrote:
>>> From: "Leo (Sunpeng) Li" 
>>>
>>> To conform to DRM's new API, we should not be accessing a DRM object's
>>> internal state directly. Rather, the DRM for_each_old/new_* iterators,
>>> and drm_atomic_get_old/new_* interface should be used.
>>>
>>> This is an ongoing process. For now, update the DRM-facing atomic
>>> functions, where the atomic state object is given.
>>>
>>> Signed-off-by: Leo (Sunpeng) Li 
>>> ---
>>>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 131 
>>> +++---
>>>   1 file changed, 66 insertions(+), 65 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
>>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> index cc024ab..d4426b3 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> @@ -3873,28 +3873,31 @@ static void amdgpu_dm_commit_planes(struct 
>>> drm_atomic_state *state,
>>>   {
>>>   uint32_t i;
>>>   struct drm_plane *plane;
>>> -    struct drm_plane_state *old_plane_state;
>>> +    struct drm_plane_state *old_plane_state, *new_plane_state;
>>>   struct dc_stream_state *dc_stream_attach;
>>>   struct dc_plane_state *plane_states_constructed[MAX_SURFACES];
>>>   struct amdgpu_crtc *acrtc_attach = to_amdgpu_crtc(pcrtc);
>>> -    struct dm_crtc_state *acrtc_state = to_dm_crtc_state(pcrtc->state);
>>> +    struct drm_crtc_state *new_pcrtc_state =
>>> +    drm_atomic_get_new_crtc_state(state, pcrtc);
>>> +    struct dm_crtc_state *acrtc_state = to_dm_crtc_state(new_pcrtc_state);
>>>   int planes_count = 0;
>>>   unsigned long flags;
>>>     /* update planes when needed */
>>> -    for_each_old_plane_in_state(state, plane, old_plane_state, i) {
>>> -    struct drm_plane_state *plane_state = plane->state;
>>> -    struct drm_crtc *crtc = plane_state->crtc;
>>> -    struct drm_framebuffer *fb = plane_state->fb;
>>> +    for_each_oldnew_plane_in_state(state, plane, old_plane_state, 
>>> new_plane_state, i) {
>>> +    struct drm_crtc *crtc = new_plane_state->crtc;
>>> +    struct drm_crtc_state *new_crtc_state =
>>> +    drm_atomic_get_new_crtc_state(state, crtc);
>>> +    struct drm_framebuffer *fb = new_plane_state->fb;
>>>   bool pflip_needed;
>>> -    struct dm_plane_state *dm_plane_state = 
>>> to_dm_plane_state(plane_state);
>>> +    struct dm_plane_state *dm_plane_state = 
>>> to_dm_plane_state(new_plane_state);
>>>     if (plane->type == DRM_PLANE_TYPE_CURSOR) {
>>>   handle_cursor_update(plane, old_plane_state);
>>>   continue;
>>>   }
>>>   -    if (!fb || !crtc || pcrtc != crtc || !crtc->state->active)
>>> +    if (!fb || !crtc || pcrtc != crtc || !new_crtc_state->active)
>>>   continue;
>>>     pflip_needed = !state->allow_modeset;
>>> @@ -3918,13 +3921,13 @@ static void amdgpu_dm_commit_planes(struct 
>>> drm_atomic_state *state,
>>>   dc_stream_attach = acrtc_state->stream;
>>>   planes_count++;
>>>   -    } else if (crtc->state->planes_changed) {
>>> +    } else if (new_crtc_state->planes_changed) {
>>>   /* Assume even ONE crtc with immediate flip means
>>>    * entire can't wait for VBLANK
>>>    * TODO Check if it's correct
>>>    */
>>>   *wait_for_vblank =
>>> -    pcrtc->state->pageflip_flags & 
>>> DRM_MODE_PAGE_FLIP_ASYNC ?
>>> +    new_pcrtc_state->pageflip_flags & 
>>> DRM_MODE_PAGE_FLIP_ASYNC ?
>>>   false : true;
>>>     /* TODO: Needs rework for multiplane flip */
>>> @@ -3942,7 +3945,7 @@ static void amdgpu_dm_commit_planes(struct 
>>> drm_atomic_state *state,
>>>   if (planes_count) {
>>>   unsigned long flags;
>>>   -    if (pcrtc->state->event) {
>>> +    if (new_pcrtc_state->event) {
>>>     drm_crtc_vblank_get(pcrtc);
>>>   @@ -3968,7 +3971,7 @@ int amdgpu_dm_atomic_commit(
>>>   bool nonblock)
>>>   {
>>>   struct drm_crtc *crtc;
>>> -    struct drm_crtc_state *new_state;
>>> +    struct drm_crtc_state *old_crtc_state, *new_state;
>>>   struct amdgpu_device *adev = dev->dev_private;
>>>   int i;
>>>   @@ -3979,8 +3982,8 @@ int amdgpu_dm_atomic_commit(
>>>    * it will update crtc->dm_crtc_state->stream pointer which is used in
>>>    * the ISRs.
>>>    */
>>> -    for_each_new_crtc_in_state(state, crtc, new_state, i) {
>>> -    struct dm_crtc_state *old_acrtc_state = 
>>> to_dm_crtc_state(crtc->state);
>>> +    for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_state, 
>>> i) {
>>> +    struct dm_crtc_state *old_acrtc_state = 
>>> 

Re: [PATCH v2 2/8] drm/rockchip/dsi: add dual mipi channel support

[Archit Taneja]

Hi,

Comment below.

On 09/26/2017 01:25 PM, Nickey Yang wrote:

This patch add dual mipi channel support:
1.add definition of dsi1 register and grf operation.
2.dsi0 and dsi1 will work in master and slave mode
when driving dual mipi panel.




  
@@ -1226,6 +1367,13 @@ static int rockchip_mipi_parse_dt(struct dw_mipi_dsi *dsi)

.grf_switch_reg = RK3399_GRF_SOC_CON20,
.grf_dsi0_mode = RK3399_GRF_DSI_MODE,
.grf_dsi0_mode_reg = RK3399_GRF_SOC_CON22,
+   .grf_dsi1_mode = RK3399_GRF_DSI1_MODE,
+   .grf_dsi1_enable = RK3399_GRF_DSI1_ENABLE,
+   .grf_dsi1_mode_reg1 = RK3399_GRF_SOC_CON23,
+   .dsi1_basedir = RK3399_TXRX_BASEDIR,
+   .dsi1_masterslavez = RK3399_TXRX_MASTERSLAVEZ,
+   .dsi1_enableclk = RK3399_TXRX_ENABLECLK,
+   .grf_dsi1_mode_reg2 = RK3399_GRF_SOC_CON24,
.flags = DW_MIPI_NEEDS_PHY_CFG_CLK | DW_MIPI_NEEDS_GRF_CLK,
.max_data_lanes = 4,
  };
@@ -1242,17 +1390,107 @@ static int rockchip_mipi_parse_dt(struct dw_mipi_dsi 
*dsi)
  };
  MODULE_DEVICE_TABLE(of, dw_mipi_dsi_dt_ids);
  
+

+static int rockchip_dsi_dual_channel_probe(struct dw_mipi_dsi *dsi)
+{
+   struct device_node *np;
+   struct platform_device *secondary;
+
+   np = of_parse_phandle(dsi->dev->of_node, "rockchip,dual-channel", 0);
+   if (np) {
+   secondary = of_find_device_by_node(np);
+   dsi->slave = platform_get_drvdata(secondary);
+   of_node_put(np);
+
+   if (!dsi->slave)
+   return -EPROBE_DEFER;
+
+   dsi->slave->master = dsi;


This is not a suitable DT binding. It doesn't really represent HW, it's more of
a configuration that allows 2 DSIs to work together.

You can use the of-graph bindings to figure out whether you are operating in 
dual
mode or not: If the output ports of both the DSIs are connected to the same DT
node (i.e, both DSIs driving the same bridge chip/panel), then you know that you
need to configure the DSIs in dual channel mode.

Thanks,
Archit

--
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: [PATCH v4 1/3] drm: Add retries for lspcon mode detection

[Jani Nikula]

On Thu, 12 Oct 2017, Shashank Sharma  wrote:
> From the CI builds, its been observed that during a driver
> reload/insert, dp dual mode read function sometimes fails to
> read from LSPCON device over i2c-over-aux channel.
>
> This patch:
> - adds some delay and few retries, allowing a scope for these
>   devices to settle down and respond.
> - changes one error log's level from ERROR->DEBUG as we want
>   to call it an error only after all the retries are exhausted.
>
> V2: Addressed review comments from Jani (for loop for retry)
> V3: Addressed review comments from Imre (break on partial read too)
> V3: Addressed review comments from Ville/Imre (Add the retries
> exclusively for LSPCON, not for all dp_dual_mode devices)
> V4: Added r-b from Imre, sending it to dri-devel (Jani)
>
> Cc: Ville Syrjala 
> Cc: Imre Deak 
> Cc: Jani Nikula 
>
> Reviewed-by: Imre Deak 
> Signed-off-by: Shashank Sharma 

Pushed via drm-intel with Dave's IRC ack. Thanks for the patch and
review.

BR,
Jani.

> ---
>  drivers/gpu/drm/drm_dp_dual_mode_helper.c | 16 +---
>  1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_dual_mode_helper.c 
> b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> index 0ef9011..02a5092 100644
> --- a/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> +++ b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> @@ -410,6 +410,7 @@ int drm_lspcon_get_mode(struct i2c_adapter *adapter,
>  {
>   u8 data;
>   int ret = 0;
> + int retry;
>  
>   if (!mode) {
>   DRM_ERROR("NULL input\n");
> @@ -417,10 +418,19 @@ int drm_lspcon_get_mode(struct i2c_adapter *adapter,
>   }
>  
>   /* Read Status: i2c over aux */
> - ret = drm_dp_dual_mode_read(adapter, DP_DUAL_MODE_LSPCON_CURRENT_MODE,
> - , sizeof(data));
> + for (retry = 0; retry < 6; retry++) {
> + if (retry)
> + usleep_range(500, 1000);
> +
> + ret = drm_dp_dual_mode_read(adapter,
> + DP_DUAL_MODE_LSPCON_CURRENT_MODE,
> + , sizeof(data));
> + if (!ret)
> + break;
> + }
> +
>   if (ret < 0) {
> - DRM_ERROR("LSPCON read(0x80, 0x41) failed\n");
> + DRM_DEBUG_KMS("LSPCON read(0x80, 0x41) failed\n");
>   return -EFAULT;
>   }

-- 
Jani Nikula, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v13 1/3] drm/tinydrm: Move helper functions from tinydrm-helpers to backlight.h

[Meghana Madhyastha]

Move the helper functions enable_backlight and disable_backlight
from tinydrm-helpers.c to backlight.h as static inline functions so
that they can be used by other drivers.

Signed-off-by: Meghana Madhyastha 
---
Changes in v13:
 -None

 drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 55 --
 drivers/gpu/drm/tinydrm/mipi-dbi.c |  5 ++-
 include/drm/tinydrm/tinydrm-helpers.h  |  2 -
 include/linux/backlight.h  | 30 ++
 4 files changed, 33 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c 
b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
index bd6cce0..a42dee6 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
@@ -276,61 +276,6 @@ struct backlight_device *tinydrm_of_find_backlight(struct 
device *dev)
 }
 EXPORT_SYMBOL(tinydrm_of_find_backlight);
 
-/**
- * tinydrm_enable_backlight - Enable backlight helper
- * @backlight: Backlight device
- *
- * Returns:
- * Zero on success, negative error code on failure.
- */
-int tinydrm_enable_backlight(struct backlight_device *backlight)
-{
-   unsigned int old_state;
-   int ret;
-
-   if (!backlight)
-   return 0;
-
-   old_state = backlight->props.state;
-   backlight->props.state &= ~BL_CORE_FBBLANK;
-   DRM_DEBUG_KMS("Backlight state: 0x%x -> 0x%x\n", old_state,
- backlight->props.state);
-
-   ret = backlight_update_status(backlight);
-   if (ret)
-   DRM_ERROR("Failed to enable backlight %d\n", ret);
-
-   return ret;
-}
-EXPORT_SYMBOL(tinydrm_enable_backlight);
-
-/**
- * tinydrm_disable_backlight - Disable backlight helper
- * @backlight: Backlight device
- *
- * Returns:
- * Zero on success, negative error code on failure.
- */
-int tinydrm_disable_backlight(struct backlight_device *backlight)
-{
-   unsigned int old_state;
-   int ret;
-
-   if (!backlight)
-   return 0;
-
-   old_state = backlight->props.state;
-   backlight->props.state |= BL_CORE_FBBLANK;
-   DRM_DEBUG_KMS("Backlight state: 0x%x -> 0x%x\n", old_state,
- backlight->props.state);
-   ret = backlight_update_status(backlight);
-   if (ret)
-   DRM_ERROR("Failed to disable backlight %d\n", ret);
-
-   return ret;
-}
-EXPORT_SYMBOL(tinydrm_disable_backlight);
-
 #if IS_ENABLED(CONFIG_SPI)
 
 /**
diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c 
b/drivers/gpu/drm/tinydrm/mipi-dbi.c
index d43e992..2561549 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -280,7 +281,7 @@ void mipi_dbi_pipe_enable(struct drm_simple_display_pipe 
*pipe,
if (fb)
fb->funcs->dirty(fb, NULL, 0, 0, NULL, 0);
 
-   tinydrm_enable_backlight(mipi->backlight);
+   backlight_enable(mipi->backlight);
 }
 EXPORT_SYMBOL(mipi_dbi_pipe_enable);
 
@@ -319,7 +320,7 @@ void mipi_dbi_pipe_disable(struct drm_simple_display_pipe 
*pipe)
mipi->enabled = false;
 
if (mipi->backlight)
-   tinydrm_disable_backlight(mipi->backlight);
+   backlight_disable(mipi->backlight);
else
mipi_dbi_blank(mipi);
 }
diff --git a/include/drm/tinydrm/tinydrm-helpers.h 
b/include/drm/tinydrm/tinydrm-helpers.h
index d554ded..f54fae0 100644
--- a/include/drm/tinydrm/tinydrm-helpers.h
+++ b/include/drm/tinydrm/tinydrm-helpers.h
@@ -47,8 +47,6 @@ void tinydrm_xrgb_to_gray8(u8 *dst, void *vaddr, struct 
drm_framebuffer *fb,
   struct drm_clip_rect *clip);
 
 struct backlight_device *tinydrm_of_find_backlight(struct device *dev);
-int tinydrm_enable_backlight(struct backlight_device *backlight);
-int tinydrm_disable_backlight(struct backlight_device *backlight);
 
 size_t tinydrm_spi_max_transfer_size(struct spi_device *spi, size_t max_len);
 bool tinydrm_spi_bpw_supported(struct spi_device *spi, u8 bpw);
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 5f2fd61..b88fabb 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -129,6 +129,36 @@ static inline int backlight_update_status(struct 
backlight_device *bd)
return ret;
 }
 
+/**
+ * backlight_enable - Enable backlight
+ * @bd: the backlight device to enable
+ */
+static inline int backlight_enable(struct backlight_device *bd)
+{
+   if (!bd)
+   return 0;
+
+   bd->props.power = FB_BLANK_UNBLANK;
+   bd->props.state &= ~BL_CORE_FBBLANK;
+
+   return backlight_update_status(bd);
+}
+
+/**
+ * backlight_disable - Disable backlight
+ * @bd: the backlight device to disable
+ */
+static inline int backlight_disable(struct backlight_device *bd)
+{
+   if (!bd)
+   return 0;
+
+   

[PATCH v13 0/3] Move backlight helper functions from tinydrm-helpers to linux/backlight

[Meghana Madhyastha]

Move drm helper functions from tinydrm-helpers to linux/backlight for
ease of use by callers in other drivers.

Changes in v13:
-Rebase against drm-misc
-Put devm_backlight_put back

Meghana Madhyastha (3):
  drm/tinydrm: Move helper functions from tinydrm-helpers to backlight.h
  drm/tinydrm: Move tinydrm_of_find_backlight to backlight.c
  drm/tinydrm: Add devres versions of backlight_get

 drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 95 --
 drivers/gpu/drm/tinydrm/mi0283qt.c |  3 +-
 drivers/gpu/drm/tinydrm/mipi-dbi.c |  5 +-
 drivers/video/backlight/backlight.c| 68 ++
 include/drm/tinydrm/tinydrm-helpers.h  |  4 --
 include/linux/backlight.h  | 55 +++
 6 files changed, 128 insertions(+), 102 deletions(-)

-- 
2.7.4

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

Re: [PATCH 0/6] Use new DRM API where possible, and cleanups.

[Maarten Lankhorst]

Op 12-10-17 om 23:15 schreef sunpeng...@amd.com:
> From: "Leo (Sunpeng) Li" 
>
> Hi Dave,
>
> This series reworks the previous patch. Patch 1 is a v2 of the previous,
> and additional patches are from the feedback received. They apply on top
> of your drm-next-amd-dc-staging branch.
>
> Thanks,
> Leo
>
> Leo (Sunpeng) Li (6):
>   drm/amd/display: Use DRM new-style object iterators.
>   drm/amd/display: Use new DRM API where possible
>   drm/amd/display: Unify DRM state variable namings.
>   drm/amd/display: Unify amdgpu_dm state variable namings.
>   drm/amd/display: Fix typo
>   drm/amd/display: Remove useless pcrtc pointer
>
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 320 
> +++---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   3 +-
>  2 files changed, 156 insertions(+), 167 deletions(-)
>
Better, only scanned through the series but

Reviewed-by: Maarten Lankhorst 

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

Re: [PATCH v2 1/6] drm/amd/display: Use DRM new-style object iterators.

[Andrey Grodzovsky]

On 10/13/2017 12:35 PM, Leo wrote:



On 2017-10-13 11:56 AM, Andrey Grodzovsky wrote:



On 10/13/2017 11:41 AM, Leo wrote:



On 2017-10-13 11:03 AM, Andrey Grodzovsky wrote:



On 10/12/2017 05:15 PM, sunpeng...@amd.com wrote:

From: "Leo (Sunpeng) Li"

Use the correct for_each_new/old_* iterators instead of for_each_*

List of affected functions:

amdgpu_dm_find_first_crtc_matching_connector: use for_each_new
 - Old from_state_var flag was always choosing the new state

amdgpu_dm_display_resume: use for_each_new
 - drm_atomic_helper_duplicate_state is called during suspend to
   cache the state
 - It sets 'state' within the state triplet to 'new_state'


It seems to me you missed that one.

Thanks,
Andrey



Good catch, seems like that change was stripped out while I was cp-ing
from the internal tree. Some changes in this function have not been 
promoted to Dave's branch yet.


I'll remove this comment for now, I think it makes sense to have a 
follow-up patch to this after more changes have been promoted.


Thanks,
Leo


With that fixed the change is Reviewed-by: Andrey Grodzovsky 



On second look, this comment is addressing the change within Dave's
patch, on which this series apply. I was trying to justify all the 
changes made, including the ones already done by Dave. See here:


https://cgit.freedesktop.org/~airlied/linux/commit/?h=drm-next-amd-dc-staging=e7b8e99bed73e9c42f1c074ad6009cb59a79bd52 



I think changing "List of affected functions" to "The following 
functions were considered" would make it less confusing, and will

still make sense if it gets squashed with Dave's patch.

Leo


Makes sense to me to.

Thanks,
Andrey







amdgpu_dm_commit_planes: use for_each_old
 - Called after the state was swapped (via atomic commit tail)

amdgpu_dm_atomic_commit: use for_each_new
 - Called before the state is swapped

amdgpu_dm_atomic_commit_tail: use for_each_old
 - Called after the state was swapped

dm_update_crtcs_state: use for_each_new
 - Called before the state is swapped (via atomic check)

amdgpu_dm_atomic_check: use for_each_new
 - Called before the state is swapped

v2: Split out typo fixes to a new patch.

Signed-off-by: Leo (Sunpeng) Li
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 
---

  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  3 +--
  2 files changed, 12 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c

index 9bfe1f9..cc024ab 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -570,23 +570,15 @@ static int dm_suspend(void *handle)
  struct amdgpu_dm_connector 
*amdgpu_dm_find_first_crct_matching_connector(

  struct drm_atomic_state *state,
-struct drm_crtc *crtc,
-bool from_state_var)
+struct drm_crtc *crtc)
  {
  uint32_t i;
  struct drm_connector_state *conn_state;
  struct drm_connector *connector;
  struct drm_crtc *crtc_from_state;
-for_each_new_connector_in_state(
-state,
-connector,
-conn_state,
-i) {
-crtc_from_state =
-from_state_var ?
-conn_state->crtc :
-connector->state->crtc;
+for_each_new_connector_in_state(state, connector, conn_state, 
i) {

+crtc_from_state = conn_state->crtc;
  if (crtc_from_state == crtc)
  return to_amdgpu_dm_connector(connector);
@@ -3890,7 +3882,7 @@ static void amdgpu_dm_commit_planes(struct 
drm_atomic_state *state,

  unsigned long flags;
  /* update planes when needed */
-for_each_new_plane_in_state(state, plane, old_plane_state, i) {
+for_each_old_plane_in_state(state, plane, old_plane_state, i) {
  struct drm_plane_state *plane_state = plane->state;
  struct drm_crtc *crtc = plane_state->crtc;
  struct drm_framebuffer *fb = plane_state->fb;
@@ -4024,7 +4016,7 @@ void amdgpu_dm_atomic_commit_tail(
  dm_state = to_dm_atomic_state(state);
  /* update changed items */
-for_each_new_crtc_in_state(state, crtc, old_crtc_state, i) {
+for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) {
  struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
  struct drm_crtc_state *new_state = crtc->state;
@@ -4113,11 +4105,9 @@ void amdgpu_dm_atomic_commit_tail(
  new_acrtc_state = 
to_dm_crtc_state(new_crtcs[i]->base.state);

  new_stream = new_acrtc_state->stream;
-aconnector =
- amdgpu_dm_find_first_crct_matching_connector(
+aconnector = 
amdgpu_dm_find_first_crct_matching_connector(

  state,
-_crtcs[i]->base,
-false);
+_crtcs[i]->base);
  if (!aconnector) {
  

Re: [PATCH 2/6] drm/amd/display: Use new DRM API where possible

[Andrey Grodzovsky]

On 10/13/2017 12:18 PM, Harry Wentland wrote:

On 2017-10-12 05:15 PM, sunpeng...@amd.com wrote:

From: "Leo (Sunpeng) Li" 

To conform to DRM's new API, we should not be accessing a DRM object's
internal state directly. Rather, the DRM for_each_old/new_* iterators,
and drm_atomic_get_old/new_* interface should be used.

This is an ongoing process. For now, update the DRM-facing atomic
functions, where the atomic state object is given.

Signed-off-by: Leo (Sunpeng) Li 
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 131 +++---
  1 file changed, 66 insertions(+), 65 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index cc024ab..d4426b3 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3873,28 +3873,31 @@ static void amdgpu_dm_commit_planes(struct 
drm_atomic_state *state,
  {
uint32_t i;
struct drm_plane *plane;
-   struct drm_plane_state *old_plane_state;
+   struct drm_plane_state *old_plane_state, *new_plane_state;
struct dc_stream_state *dc_stream_attach;
struct dc_plane_state *plane_states_constructed[MAX_SURFACES];
struct amdgpu_crtc *acrtc_attach = to_amdgpu_crtc(pcrtc);
-   struct dm_crtc_state *acrtc_state = to_dm_crtc_state(pcrtc->state);
+   struct drm_crtc_state *new_pcrtc_state =
+   drm_atomic_get_new_crtc_state(state, pcrtc);
+   struct dm_crtc_state *acrtc_state = to_dm_crtc_state(new_pcrtc_state);
int planes_count = 0;
unsigned long flags;
  
  	/* update planes when needed */

-   for_each_old_plane_in_state(state, plane, old_plane_state, i) {
-   struct drm_plane_state *plane_state = plane->state;
-   struct drm_crtc *crtc = plane_state->crtc;
-   struct drm_framebuffer *fb = plane_state->fb;
+   for_each_oldnew_plane_in_state(state, plane, old_plane_state, 
new_plane_state, i) {
+   struct drm_crtc *crtc = new_plane_state->crtc;
+   struct drm_crtc_state *new_crtc_state =
+   drm_atomic_get_new_crtc_state(state, crtc);
+   struct drm_framebuffer *fb = new_plane_state->fb;
bool pflip_needed;
-   struct dm_plane_state *dm_plane_state = 
to_dm_plane_state(plane_state);
+   struct dm_plane_state *dm_plane_state = 
to_dm_plane_state(new_plane_state);
  
  		if (plane->type == DRM_PLANE_TYPE_CURSOR) {

handle_cursor_update(plane, old_plane_state);
continue;
}
  
-		if (!fb || !crtc || pcrtc != crtc || !crtc->state->active)

+   if (!fb || !crtc || pcrtc != crtc || !new_crtc_state->active)
continue;
  
  		pflip_needed = !state->allow_modeset;

@@ -3918,13 +3921,13 @@ static void amdgpu_dm_commit_planes(struct 
drm_atomic_state *state,
dc_stream_attach = acrtc_state->stream;
planes_count++;
  
-		} else if (crtc->state->planes_changed) {

+   } else if (new_crtc_state->planes_changed) {
/* Assume even ONE crtc with immediate flip means
 * entire can't wait for VBLANK
 * TODO Check if it's correct
 */
*wait_for_vblank =
-   pcrtc->state->pageflip_flags & 
DRM_MODE_PAGE_FLIP_ASYNC ?
+   new_pcrtc_state->pageflip_flags & 
DRM_MODE_PAGE_FLIP_ASYNC ?
false : true;
  
  			/* TODO: Needs rework for multiplane flip */

@@ -3942,7 +3945,7 @@ static void amdgpu_dm_commit_planes(struct 
drm_atomic_state *state,
if (planes_count) {
unsigned long flags;
  
-		if (pcrtc->state->event) {

+   if (new_pcrtc_state->event) {
  
  			drm_crtc_vblank_get(pcrtc);
  
@@ -3968,7 +3971,7 @@ int amdgpu_dm_atomic_commit(

bool nonblock)
  {
struct drm_crtc *crtc;
-   struct drm_crtc_state *new_state;
+   struct drm_crtc_state *old_crtc_state, *new_state;
struct amdgpu_device *adev = dev->dev_private;
int i;
  
@@ -3979,8 +3982,8 @@ int amdgpu_dm_atomic_commit(

 * it will update crtc->dm_crtc_state->stream pointer which is used in
 * the ISRs.
 */
-   for_each_new_crtc_in_state(state, crtc, new_state, i) {
-   struct dm_crtc_state *old_acrtc_state = 
to_dm_crtc_state(crtc->state);
+   for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_state, 
i) {
+   struct dm_crtc_state *old_acrtc_state = 
to_dm_crtc_state(old_crtc_state);
struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
  
  		if 

Re: [Outreachy kernel] [PATCH v2] drm/amd/powerplay: Remove unnecessary cast on void pointer

[Julia Lawall]

> @@ -3400,7 +3400,7 @@ static int smu7_read_sensor(struct pp_hwmgr *hwmgr, int 
> idx,
>  static int smu7_find_dpm_states_clocks_in_dpm_table(struct pp_hwmgr *hwmgr, 
> const void *input)
>  {
>   const struct phm_set_power_state_input *states =
> - (const struct phm_set_power_state_input *)input;
> + input;

Actually, there are some more cleanup opportunities heer and in some other
cases.  There is no need for the newline before input.

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

Re: [PATCH] drm/amd/dc: fix semicolon.cocci warnings

[Harry Wentland]

On 2017-10-12 07:35 PM, kbuild test robot wrote:
> drivers/gpu/drm/amd/amdgpu/../display/dc/gpio/gpio_service.c:134:2-3: 
> Unneeded semicolon
> 
> 
>  Remove unneeded semicolon.
> 
> Generated by: scripts/coccinelle/misc/semicolon.cocci
> 
> Fixes: 80be23c57868 ("drm/amd/dc: Add dc display driver (v2)")
> CC: Harry Wentland 
> Signed-off-by: Fengguang Wu 

Reviewed-by: Harry Wentland 

Harry

> ---
> 
>  gpio_service.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/drivers/gpu/drm/amd/amdgpu/../display/dc/gpio/gpio_service.c
> +++ b/drivers/gpu/drm/amd/amdgpu/../display/dc/gpio/gpio_service.c
> @@ -131,7 +131,7 @@ failure_2:
>  
>   if (slot)
>   dm_free(slot);
> - };
> + }
>  
>  failure_1:
>   dm_free(service);
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 101900] No HDMI HBR audio on Polaris (no TrueHD, no Atmos, no Neo:X, no HD Master audio)

[bugzilla-daemon]

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

--- Comment #11 from Harry Wentland  ---
So, unbeknowst to me, we never supported HBR audio (i.e. TrueHD, etc.) with DC.
We would like to enable it but it's not top priority right now.

-- 
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] drm: Replace kzalloc with kcalloc

[Sean Paul]

On Fri, Oct 13, 2017 at 10:58:33AM +0300, Jani Nikula wrote:
> On Fri, 13 Oct 2017, Harsha Sharma  wrote:
> > Prefer kcalloc over kzalloc to allocate an array.
> > This patch fixes checkcpatch issue.
> >
> > Signed-off-by: Harsha Sharma 
> 
> Reviewed-by: Jani Nikula 
> 

Applied to -misc-next, thanks!

Sean

> 
> > ---
> > Changes in v2:
> >  -kcalloc will take 3 arguments
> >
> >  drivers/gpu/drm/drm_crtc_helper.c  | 4 ++--
> >  drivers/gpu/drm/drm_fb_helper.c| 2 +-
> >  drivers/gpu/drm/drm_plane_helper.c | 2 +-
> >  3 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_crtc_helper.c 
> > b/drivers/gpu/drm/drm_crtc_helper.c
> > index eab36a460638..5a84c3bc915d 100644
> > --- a/drivers/gpu/drm/drm_crtc_helper.c
> > +++ b/drivers/gpu/drm/drm_crtc_helper.c
> > @@ -562,12 +562,12 @@ int drm_crtc_helper_set_config(struct drm_mode_set 
> > *set,
> >  * Allocate space for the backup of all (non-pointer) encoder and
> >  * connector data.
> >  */
> > -   save_encoder_crtcs = kzalloc(dev->mode_config.num_encoder *
> > +   save_encoder_crtcs = kcalloc(dev->mode_config.num_encoder,
> > sizeof(struct drm_crtc *), GFP_KERNEL);
> > if (!save_encoder_crtcs)
> > return -ENOMEM;
> >  
> > -   save_connector_encoders = kzalloc(dev->mode_config.num_connector *
> > +   save_connector_encoders = kcalloc(dev->mode_config.num_connector,
> > sizeof(struct drm_encoder *), GFP_KERNEL);
> > if (!save_connector_encoders) {
> > kfree(save_encoder_crtcs);
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c 
> > b/drivers/gpu/drm/drm_fb_helper.c
> > index 1b8f013ffa65..de31e52ab9cb 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -2266,7 +2266,7 @@ static int drm_pick_crtcs(struct drm_fb_helper 
> > *fb_helper,
> > if (modes[n] == NULL)
> > return best_score;
> >  
> > -   crtcs = kzalloc(fb_helper->connector_count *
> > +   crtcs = kcalloc(fb_helper->connector_count,
> > sizeof(struct drm_fb_helper_crtc *), GFP_KERNEL);
> > if (!crtcs)
> > return best_score;
> > diff --git a/drivers/gpu/drm/drm_plane_helper.c 
> > b/drivers/gpu/drm/drm_plane_helper.c
> > index 06aee1741e96..759ed93f4ba8 100644
> > --- a/drivers/gpu/drm/drm_plane_helper.c
> > +++ b/drivers/gpu/drm/drm_plane_helper.c
> > @@ -354,7 +354,7 @@ int drm_primary_helper_update(struct drm_plane *plane, 
> > struct drm_crtc *crtc,
> > /* Find current connectors for CRTC */
> > num_connectors = get_connectors_for_crtc(crtc, NULL, 0);
> > BUG_ON(num_connectors == 0);
> > -   connector_list = kzalloc(num_connectors * sizeof(*connector_list),
> > +   connector_list = kcalloc(num_connectors, sizeof(*connector_list),
> >  GFP_KERNEL);
> > if (!connector_list)
> > return -ENOMEM;
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

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

[pull] amdgpu drm-next-4.15-dc

[Alex Deucher]

Hi Dave,

Updates for DC against your drm-next-amd-dc-staging branch.
- Fix for iterator changes
- Misc cleanups

The following changes since commit e7b8e99bed73e9c42f1c074ad6009cb59a79bd52:

  amdgpu/dc: fixup for new apis - probably wrong (2017-10-09 11:22:07 +1000)

are available in the git repository at:

  git://people.freedesktop.org/~agd5f/linux drm-next-4.15-dc

for you to fetch changes up to b175d392cfb28a9d260904bbb330917efe039331:

  drm/amd/display: drop unused dm_delay_in_microseconds (2017-10-13 15:49:42 
-0400)


Alex Deucher (12):
  drm/amd/display: fix typo in function name
  drm/amd/display: whitespace cleanup in amdgpu_dm.c/h
  drm/amd/display: make a bunch of stuff in amdgpu_dm.c static
  drm/amd/display: drop unused functions in amdgpu_dm.c
  drm/amd/display: drop unused functions in amdgpu_dm_services.c
  drm/amd/display: whitespace cleanup in amdgpu_dm_mst_types.c/h
  drm/amd/display: make log_dpcd static
  drm/amd/display: whitespace cleanup in amdgpu_dm_irq.c/h
  drm/amd/display: remove unused functions in amdgpu_dm_irq.c
  drm/amd/display: make amdgpu_dm_irq_handler static
  drm/amd/display/dc: drop dm_delay_in_microseconds
  drm/amd/display: drop unused dm_delay_in_microseconds

Leo (Sunpeng) Li (6):
  drm/amd/display: Use DRM new-style object iterators.
  drm/amd/display: Use new DRM API where possible
  drm/amd/display: Unify DRM state variable namings.
  drm/amd/display: Unify amdgpu_dm state variable namings.
  drm/amd/display: Fix typo
  drm/amd/display: Remove useless pcrtc pointer

 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 812 ++---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h  | 122 +---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c  | 163 ++---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.h  |  38 +-
 .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c|  41 +-
 .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.h|   5 +-
 .../drm/amd/display/amdgpu_dm/amdgpu_dm_services.c |  60 --
 drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c |   4 +-
 drivers/gpu/drm/amd/display/dc/dce/dce_dmcu.c  |   4 +-
 drivers/gpu/drm/amd/display/dc/dm_services.h   |   3 -
 10 files changed, 472 insertions(+), 780 deletions(-)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/amd/display: fix ifnullfree.cocci warnings

[Harry Wentland]

On 2017-10-12 07:17 PM, kbuild test robot wrote:
> drivers/gpu/drm/amd/amdgpu/../display/dc/gpio/gpio_service.c:134:3-8: 
> WARNING: NULL check before freeing functions like kfree, debugfs_remove, 
> debugfs_remove_recursive or usb_free_urb is not needed. Maybe consider 
> reorganizing relevant code to avoid passing NULL values.
> drivers/gpu/drm/amd/amdgpu/../display/dc/gpio/gpio_service.c:175:4-9: 
> WARNING: NULL check before freeing functions like kfree, debugfs_remove, 
> debugfs_remove_recursive or usb_free_urb is not needed. Maybe consider 
> reorganizing relevant code to avoid passing NULL values.
> 
>  NULL check before some freeing functions is not needed.
> 
>  Based on checkpatch warning
>  "kfree(NULL) is safe this check is probably not required"
>  and kfreeaddr.cocci by Julia Lawall.
> 
> Generated by: scripts/coccinelle/free/ifnullfree.cocci
> 
> Fixes: f09cd1f46388 ("drm/amd/display: Use kernel alloc/free")
> Signed-off-by: Fengguang Wu 

Reviewed-by: Harry Wentland 

Harry

> ---
> 
>  gpio_service.c |6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> --- a/drivers/gpu/drm/amd/amdgpu/../display/dc/gpio/gpio_service.c
> +++ b/drivers/gpu/drm/amd/amdgpu/../display/dc/gpio/gpio_service.c
> @@ -130,8 +130,7 @@ failure_2:
>  
>   slot = service->busyness[index_of_id];
>  
> - if (slot)
> - kfree(slot);
> + kfree(slot);
>   };
>  
>  failure_1:
> @@ -171,8 +170,7 @@ void dal_gpio_service_destroy(
>   do {
>   uint32_t *slot = (*ptr)->busyness[index_of_id];
>  
> - if (slot)
> - kfree(slot);
> + kfree(slot);
>  
>   ++index_of_id;
>   } while (index_of_id < GPIO_ID_COUNT);
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 103102] Cannot wake-up with an AMD RX 480 on Linux 4.13 and Linux 4.14

[bugzilla-daemon]

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

--- Comment #10 from Hadrien  ---
I built v4.12.5 from
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git instead
of using a prebuilt kernel from
http://kernel.ubuntu.com/~kernel-ppa/mainline/v4.12.5/ and I unexpectedly faced
the problem.

I wonder why I cannot reproduce the problem with Ubuntu's version of kernel
4.12.5. Maybe there are some Ubuntu specific patches that hide the problem?

Anyway, I rebuilt v4.2.5 again but this time I removed the code from the commit
referenced on kernel.org Bugzilla:

--
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
index c0a806280257..f862e3d9cd93 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
@@ -838,9 +838,10 @@ static int amdgpu_cgs_get_active_displays_info(struct
cgs_device *cgs_device,
return -EINVAL;

mode_info = info->mode_info;
+
if (mode_info) {
/* if the displays are off, vblank time is max */
-   mode_info->vblank_time_us = 0x;
+   /*mode_info->vblank_time_us = 0x;*/
/* always set the reference clock */
mode_info->ref_clock = adev->clock.spll.reference_freq;
}

--

And I cannot reproduce the problem anymore. So I guess this is actually the
same problem than https://bugzilla.kernel.org/show_bug.cgi?id=196615

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