Re: [Intel-gfx] i915 PSR test results and cursor lag

[Pandiyan, Dhinakaran]

On Fri, 2018-02-02 at 19:18 +, Andy Lutomirski wrote:
> On Fri, Feb 2, 2018 at 1:24 AM, Andy Lutomirski  wrote:
> > On Thu, Feb 1, 2018 at 9:20 PM, Chris Wilson  
> > wrote:
> >> Quoting Andy Lutomirski (2018-02-01 21:04:30)
> >>> I got this after a recent suspend/resume:
> >>>
> >>> Feb 01 09:44:34 laptop systemd-logind[2412]: Lid closed.
> >>> Feb 01 09:44:34 laptop systemd-logind[2412]: device-enumerator: scan all 
> >>> dirs
> >>> Feb 01 09:44:34 laptop systemd-logind[2412]:   device-enumerator:
> >>> scanning /sys/bus
> >>> Feb 01 09:44:34 laptop systemd-logind[2412]:   device-enumerator:
> >>> scanning /sys/class
> >>> Feb 01 09:44:34 laptop systemd-logind[2412]: Failed to open
> >>> configuration file '/etc/systemd/sleep.conf': No such file or
> >>> directory
> >>> Feb 01 09:44:34 laptop systemd-logind[2412]: Suspending...
> >>> Feb 01 09:44:34 laptop systemd-logind[2412]: Sent message type=signal
> >>> sender=n/a destination=n/a object=/org/freedesktop/login1
> >>> interface=org.freedesktop.login1.Manager member=PrepareForSleep
> >>> cookie=570 reply
> >>> Feb 01 09:44:34 laptop systemd-logind[2412]: Got message
> >>> type=method_call sender=:1.46 destination=:1.1
> >>> object=/org/freedesktop/login1/session/_32
> >>> interface=org.freedesktop.login1.Session member=ReleaseDevice
> >>> Feb 01 09:44:34 laptop systemd-logind[2412]: Sent message type=signal
> >>> sender=n/a destination=:1.46
> >>> object=/org/freedesktop/login1/session/_32
> >>> interface=org.freedesktop.login1.Session member=PauseDevice cookie
> >>> Feb 01 09:44:34 laptop gnome-shell[2630]: Failed to apply DRM plane
> >>> transform 0: Permission denied
> >>> Feb 01 09:44:34 laptop gnome-shell[2630]: drmModeSetCursor2 failed
> >>> with (Permission denied), drawing cursor with OpenGL from now on
> >>>
> >>> But I don't see the word "cursor" in my system logs before the first
> >>> suspend.  What am I looking for?  This is Fedora 27 running a Gnome
> >>> Wayland session, but it hasn't been reinstalled in some time, so it's
> >>> possible that there are some weird settings sitting around.  But I did
> >>> check and I have no weird i915 parameters.
> >>
> >> You are using gnome-shell as the display server. From that it appears to
> >> have started off with a HW cursor and switched to a SW cursor after
> >> suspend. Did you notice a change in behaviour? After rebooting or just
> >> restarting gnome-shell?
> >
> > I think it's less consistently bad after a reboot before suspending.
> >
> >>
> >>> Also, are these things potentially related:
> >>>
> >>> [ 3067.702527] [drm:intel_pipe_update_start [i915]] *ERROR* Potential
> >>> atomic update failure on pipe A
> >>
> >> They are just "missed the immediate vblank for the screen update"
> >> messages. Should not be related to PSR, but may cause jitter by delaying
> >> the odd screen update.
> >
> > I just got this one, and the timestamp is at least reasonably close to
> > a giant latency spike:
> >
> > [  288.799654] [drm:intel_pipe_update_end [i915]] *ERROR* Atomic
> > update failure on pipe A (start=31 end=32) time 15 us, min 1073, max
> > 1079, scanline start 1087, end 1088
> >
> >>
> >>> As I'm typing this, I've seen a couple instances of what seems like a
> >>> full *second* of cursor latency, but I've only gotten the potential
> >>> atomic update failure once.
> >>>
> >>> And is there any straightforward tracing to do to distinguish between
> >>> PSR exit latency and other potential sources of latency?
> >>
> >> It looks plausible that we could at least report how long it takes the
> >> registers to reflect the change in state (but we don't). The best source
> >> of information atm is /sys/kernel/debug/dri/0/i915_edp_psr_status.
> >
> > Hmm.
> >
> > I went and looked at the code, and I noticed what could be bugs or
> > could (more likely) be my confusion since I don't know this code at
> > all:
> >
> > intel_single_frame_update() does something inscrutable to me, but I
> > imagine it does something that causes the next page flip to get
> > noticed by the panel even with PSR on.  But how does the code that
> > calls it know that anything happened?  (Looking at the commit history,
> > maybe this is something special that's only needed on some platforms
> > but doesn't replace the normal PSR exit sequence.)
> >
> > Perhaps more interestingly, intel_psr_flush() does this:
> >
> > /* By definition flush = invalidate + flush */
> > if (frontbuffer_bits)
> > intel_psr_exit(dev_priv);
> >
> > if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits)
> > if (!work_busy(_priv->psr.work.work))
> > schedule_delayed_work(_priv->psr.work,
> >   msecs_to_jiffies(100));
> >
> > I'm guessing that the idea is that we're turning off PSR because we
> > want the panel to update and we expect that, in 100ms, the update will
> > have hit the panel and we'll have been idle long enough for it to 

[PATCH 07/10] drm/atomic: Handle 64-bit return from drm_crtc_vblank_count()

[Dhinakaran Pandiyan]

570e86963a51 ("drm: Widen vblank count to 64-bits [v3]") changed the
return type for drm_crtc_vblank_count() to u64.

The flip ioctl receives a 32-bit target sequence from user space and is
compared against the current sequence from drm_crtc_vblank_count(). So,
typecast return from drm_crtc_vblank_count() explicitly to add clarity.

__drm_crtcs_state.last_vblank_count however only ever stores the value from
drm_crtc_vblank_count() and can be upgraded to u64.

Cc: Keith Packard 
Cc: Rodrigo Vivi 
Signed-off-by: Dhinakaran Pandiyan 
---
 drivers/gpu/drm/drm_plane.c | 2 +-
 include/drm/drm_atomic.h| 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 22b54663b6e7..09de6ecb3968 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -948,7 +948,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
if (r)
return r;
 
-   current_vblank = drm_crtc_vblank_count(crtc);
+   current_vblank = (u32)drm_crtc_vblank_count(crtc);
 
switch (page_flip->flags & DRM_MODE_PAGE_FLIP_TARGET) {
case DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE:
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index cf13842a6dbd..2c711a24c80c 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -154,7 +154,7 @@ struct __drm_crtcs_state {
struct drm_crtc *ptr;
struct drm_crtc_state *state, *old_state, *new_state;
s32 __user *out_fence_ptr;
-   unsigned last_vblank_count;
+   u64 last_vblank_count;
 };
 
 struct __drm_connnectors_state {
-- 
2.14.1

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

[PATCH 09/10] drm/vblank: Restoring vblank counts after device PM events.

[Dhinakaran Pandiyan]

From: "Pandiyan, Dhinakaran" 

The HW frame counter can get reset if device enters a low power state after
vblank interrupts were disabled. This messes up any following vblank count
update as a negative diff (huge unsigned diff) is calculated from the HW
frame counter change. We cannot ignore negative diffs altogther as there
could be legitimate wrap arounds. So, allow drivers to update vblank->count
with missed vblanks for the time interrupts were disabled. This is similar
to _crtc_vblank_on() except that vblanks interrupts are not enabled at the
end as this function is expected to be called from the driver
_enable_vblank() vfunc.

v2: drm_crtc_vblank_restore should take crtc as arg. (Chris)
Add docs and sprinkle some asserts.

Cc: Daniel Vetter 
Cc: Chris Wilson 
Cc: Michel Dänzer 
Signed-off-by: Dhinakaran Pandiyan 
Reviewed-by: Rodrigo Vivi 
Acked-by: Daniel Vetter 
---
 drivers/gpu/drm/drm_vblank.c | 59 
 include/drm/drm_vblank.h |  2 ++
 2 files changed, 61 insertions(+)

diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 913954765d9e..c781cb426bf1 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -1237,6 +1237,65 @@ void drm_crtc_vblank_on(struct drm_crtc *crtc)
 }
 EXPORT_SYMBOL(drm_crtc_vblank_on);
 
+/**
+ * drm_vblank_restore - estimated vblanks using timestamps and update it.
+ *
+ * Power manamement features can cause frame counter resets between vblank
+ * disable and enable. Drivers can then use this function in their
+ * _crtc_funcs.enable_vblank implementation to estimate the vblanks since
+ * the last _crtc_funcs.disable_vblank.
+ *
+ * This function is the legacy version of drm_crtc_vblank_restore().
+ */
+void drm_vblank_restore(struct drm_device *dev, unsigned int pipe)
+{
+   ktime_t t_vblank;
+   struct drm_vblank_crtc *vblank;
+   int framedur_ns;
+   u64 diff_ns;
+   u32 cur_vblank, diff = 1;
+   int count = DRM_TIMESTAMP_MAXRETRIES;
+
+   if (WARN_ON(pipe >= dev->num_crtcs))
+   return;
+
+   assert_spin_locked(>vbl_lock);
+   assert_spin_locked(>vblank_time_lock);
+
+   vblank = >vblank[pipe];
+   WARN_ONCE((drm_debug & DRM_UT_VBL) && !vblank->framedur_ns,
+ "Cannot compute missed vblanks without frame duration\n");
+   framedur_ns = vblank->framedur_ns;
+
+   do {
+   cur_vblank = __get_vblank_counter(dev, pipe);
+   drm_get_last_vbltimestamp(dev, pipe, _vblank, false);
+   } while (cur_vblank != __get_vblank_counter(dev, pipe) && --count > 0);
+
+   diff_ns = ktime_to_ns(ktime_sub(t_vblank, vblank->time));
+   if (framedur_ns)
+   diff = DIV_ROUND_CLOSEST_ULL(diff_ns, framedur_ns);
+
+
+   DRM_DEBUG_VBL("missed %d vblanks in %lld ns, frame duration=%d ns, 
hw_diff=%d\n",
+ diff, diff_ns, framedur_ns, cur_vblank - vblank->last);
+   store_vblank(dev, pipe, diff, t_vblank, cur_vblank);
+}
+EXPORT_SYMBOL(drm_vblank_restore);
+
+/**
+ * drm_crtc_vblank_restore - estimate vblanks using timestamps and update it.
+ * Power manamement features can cause frame counter resets between vblank
+ * disable and enable. Drivers can then use this function in their
+ * _crtc_funcs.enable_vblank implementation to estimate the vblanks since
+ * the last _crtc_funcs.disable_vblank.
+ */
+void drm_crtc_vblank_restore(struct drm_crtc *crtc)
+{
+   drm_vblank_restore(crtc->dev, drm_crtc_index(crtc));
+}
+EXPORT_SYMBOL(drm_crtc_vblank_restore);
+
 static void drm_legacy_vblank_pre_modeset(struct drm_device *dev,
  unsigned int pipe)
 {
diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
index a4c3b0a0a197..16d46e2a6854 100644
--- a/include/drm/drm_vblank.h
+++ b/include/drm/drm_vblank.h
@@ -180,6 +180,8 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc);
 void drm_crtc_vblank_reset(struct drm_crtc *crtc);
 void drm_crtc_vblank_on(struct drm_crtc *crtc);
 u64 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc);
+void drm_vblank_restore(struct drm_device *dev, unsigned int pipe);
+void drm_crtc_vblank_restore(struct drm_crtc *crtc);
 
 bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
   unsigned int pipe, int *max_error,
-- 
2.14.1

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

[PATCH 08/10] drm/vblank: Do not update vblank count if interrupts are already disabled.

[Dhinakaran Pandiyan]

From: "Pandiyan, Dhinakaran" 

Updating vblank counts requires register reads and these reads may not
return meaningful values if the device was in a low power state after
vblank interrupts were last disabled. So, update the count only if vblank
interrupts are enabled. Secondly, this means the registers should be read
before disabling vblank interrupts.

v2: Don't check vblank->enabled outside it's lock (Chris)

Cc: Chris Wilson 
Cc: Daniel Vetter 
Cc: Michel Dänzer 
Signed-off-by: Dhinakaran Pandiyan 
Reviewed-by: Rodrigo Vivi 
Acked-by: Daniel Vetter 
---
 drivers/gpu/drm/drm_vblank.c | 18 ++
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index f0d3ed5f2528..913954765d9e 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -347,23 +347,25 @@ void drm_vblank_disable_and_save(struct drm_device *dev, 
unsigned int pipe)
spin_lock_irqsave(>vblank_time_lock, irqflags);
 
/*
-* Only disable vblank interrupts if they're enabled. This avoids
-* calling the ->disable_vblank() operation in atomic context with the
-* hardware potentially runtime suspended.
+* Update vblank count and disable vblank interrupts only if the
+* interrupts were enabled. This avoids calling the ->disable_vblank()
+* operation in atomic context with the hardware potentially runtime
+* suspended.
 */
-   if (vblank->enabled) {
-   __disable_vblank(dev, pipe);
-   vblank->enabled = false;
-   }
+   if (!vblank->enabled)
+   goto out;
 
/*
-* Always update the count and timestamp to maintain the
+* Update the count and timestamp to maintain the
 * appearance that the counter has been ticking all along until
 * this time. This makes the count account for the entire time
 * between drm_crtc_vblank_on() and drm_crtc_vblank_off().
 */
drm_update_vblank_count(dev, pipe, false);
+   __disable_vblank(dev, pipe);
+   vblank->enabled = false;
 
+out:
spin_unlock_irqrestore(>vblank_time_lock, irqflags);
 }
 
-- 
2.14.1

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

[PATCH 06/10] drm/tegra: Handle 64-bit return from drm_crtc_vblank_count()

[Dhinakaran Pandiyan]

570e86963a51 ("drm: Widen vblank count to 64-bits [v3]") changed the
return type for drm_crtc_vblank_count() to u64. This could cause
potential problems if the return value is used in arithmetic operations
with a 32-bit reference HW vblank count. Explicitly typecasting this
down to u32 either fixes a potential problem or serves to add clarity in
case the implicit typecasting was already correct.

Cc: Keith Packard 
Cc: Thierry Reding 
Signed-off-by: Dhinakaran Pandiyan 
---
 drivers/gpu/drm/tegra/dc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index b8403ed48285..49df2db2ad46 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -1359,7 +1359,7 @@ static u32 tegra_dc_get_vblank_counter(struct drm_crtc 
*crtc)
return host1x_syncpt_read(dc->syncpt);
 
/* fallback to software emulated VBLANK counter */
-   return drm_crtc_vblank_count(>base);
+   return (u32)drm_crtc_vblank_count(>base);
 }
 
 static int tegra_dc_enable_vblank(struct drm_crtc *crtc)
-- 
2.14.1

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

[PATCH 02/10] drm/i915/vblank: Make the vblank counter u64 -> u32 typecast explicit

[Dhinakaran Pandiyan]

Core returns a u64 vblank count and intel_crtc_get_vblank_counter()
expects a 32-bit value. Make the typecast explicit to add clarity.

Cc: Keith Packard 
Cc: Rodrigo Vivi 
Signed-off-by: Dhinakaran Pandiyan 
---
 drivers/gpu/drm/i915/intel_display.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index ad8d9c6c40e4..f6b450de653c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12075,7 +12075,7 @@ u32 intel_crtc_get_vblank_counter(struct intel_crtc 
*crtc)
struct drm_device *dev = crtc->base.dev;
 
if (!dev->max_vblank_count)
-   return drm_crtc_accurate_vblank_count(>base);
+   return (u32)drm_crtc_accurate_vblank_count(>base);
 
return dev->driver->get_vblank_counter(dev, crtc->pipe);
 }
-- 
2.14.1

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

[PATCH 05/10] drm/radeon: Handle 64-bit return from drm_crtc_vblank_count()

[Dhinakaran Pandiyan]

570e86963a51 ("drm: Widen vblank count to 64-bits [v3]") changed the
return type for drm_crtc_vblank_count() to u64. This could cause
potential problems if the return value is used in arithmetic operations
with a 32-bit reference HW vblank count. Explicitly typecasting this down
to u32 either fixes a potential problem or serves to add clarity in case
the implicit typecasting was already correct.

Cc: Keith Packard 
Cc: Alex Deucher 
Cc: Harry Wentland 
Signed-off-by: Dhinakaran Pandiyan 
---
 drivers/gpu/drm/radeon/radeon_display.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_display.c 
b/drivers/gpu/drm/radeon/radeon_display.c
index dfda5e0ed166..26129b2b082d 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -570,7 +570,7 @@ static int radeon_crtc_page_flip_target(struct drm_crtc 
*crtc,
base &= ~7;
}
work->base = base;
-   work->target_vblank = target - drm_crtc_vblank_count(crtc) +
+   work->target_vblank = target - (uint32_t)drm_crtc_vblank_count(crtc) +
dev->driver->get_vblank_counter(dev, work->crtc_id);
 
/* We borrow the event spin lock for protecting flip_work */
-- 
2.14.1

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

[PATCH 10/10] drm/i915: Estimate and update missed vblanks.

[Dhinakaran Pandiyan]

From: "Pandiyan, Dhinakaran" 

The frame counter may have got reset between disabling and enabling vblank
interrupts due to DMC putting the hardware to DC5/6 states if PSR was
active. The frame counter could also have stalled if PSR was active in case
there was no DMC. The frame counter resetting has a user visible impact
of screen freezes.

Make use of drm_vblank_restore() to compute missed vblanks for the duration
in which vblank interrupts were disabled and update the vblank counter with
this value as diff. There's no need to check if PSR was actually active in
the interrupt disabled duration, so simplify the check to a feature check.

Enabling vblank interrupts wakes up the hardware from DC5/6 and prevents it
from going back again as long as the there are pending interrupts. So, we
don't have to explicity disallow DC5/6 after enabling vblank interrupts to
keep the counter running.

This change is not applicable to CHV, as enabling interrupts does not
prevent the hardware from activating PSR.

v2: Added comments(Rodrigo) and rewrote commit message.

Cc: Ville Syrjälä 
Cc: Rodrigo Vivi 
Signed-off-by: Dhinakaran Pandiyan 
Reviewed-by: Rodrigo Vivi 
Acked-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_irq.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 252feff2892d..e86c645b6b07 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2968,6 +2968,12 @@ static int ironlake_enable_vblank(struct drm_device 
*dev, unsigned int pipe)
ilk_enable_display_irq(dev_priv, bit);
spin_unlock_irqrestore(_priv->irq_lock, irqflags);
 
+   /* Even though there is no DMC, frame counter can get stuck when
+* PSR is active as no frames are generated.
+*/
+   if (HAS_PSR(dev_priv))
+   drm_vblank_restore(dev, pipe);
+
return 0;
 }
 
@@ -2980,6 +2986,12 @@ static int gen8_enable_vblank(struct drm_device *dev, 
unsigned int pipe)
bdw_enable_pipe_irq(dev_priv, pipe, GEN8_PIPE_VBLANK);
spin_unlock_irqrestore(_priv->irq_lock, irqflags);
 
+   /* Even if there is no DMC, frame counter can get stuck when
+* PSR is active as no frames are generated, so check only for PSR.
+*/
+   if (HAS_PSR(dev_priv))
+   drm_vblank_restore(dev, pipe);
+
return 0;
 }
 
-- 
2.14.1

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

[PATCH 04/10] drm/amdgpu: Handle 64-bit return from drm_crtc_vblank_count()

[Dhinakaran Pandiyan]

570e86963a51 ("drm: Widen vblank count to 64-bits [v3]") changed the
return type for drm_crtc_vblank_count() to u64. This could cause
potential problems if the return value is used in arithmetic operations
with a 32-bit reference HW vblank count. Explicitly typecasting this down
to u32 either fixes a potential problem or serves to add clarity in case
the typecasting was implicitly done.

Cc: Keith Packard 
Cc: Alex Deucher 
Cc: Harry Wentland 
Signed-off-by: Dhinakaran Pandiyan 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   | 2 +-
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 38d47559f098..c2fa5d55f04e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -207,7 +207,7 @@ int amdgpu_crtc_page_flip_target(struct drm_crtc *crtc,
amdgpu_bo_unreserve(new_abo);
 
work->base = base;
-   work->target_vblank = target - drm_crtc_vblank_count(crtc) +
+   work->target_vblank = target - (uint32_t)drm_crtc_vblank_count(crtc) +
amdgpu_get_vblank_counter_kms(dev, work->crtc_id);
 
/* we borrow the event spin lock for protecting flip_wrok */
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 1ce4c98385e3..b7254a29b34a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3836,7 +3836,7 @@ static void amdgpu_dm_do_flip(struct drm_crtc *crtc,
 
 
/* Prepare wait for target vblank early - before the fence-waits */
-   target_vblank = target - drm_crtc_vblank_count(crtc) +
+   target_vblank = target - (uint32_t)drm_crtc_vblank_count(crtc) +
amdgpu_get_vblank_counter_kms(crtc->dev, 
acrtc->crtc_id);
 
/* TODO This might fail and hence better not used, wait
@@ -3982,7 +3982,7 @@ static void amdgpu_dm_commit_planes(struct 
drm_atomic_state *state,
amdgpu_dm_do_flip(
crtc,
fb,
-   drm_crtc_vblank_count(crtc) + *wait_for_vblank,
+   (uint32_t)drm_crtc_vblank_count(crtc) + 
*wait_for_vblank,
dm_state->context);
}
 
-- 
2.14.1

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

[PATCH 01/10] drm/vblank: Data type fixes for 64-bit vblank sequences.

[Dhinakaran Pandiyan]

From: "Pandiyan, Dhinakaran" 

drm_vblank_count() has an u32 type returning what is a 64-bit vblank count.
The effect of this is when drm_wait_vblank_ioctl() tries to widen the user
space requested vblank sequence using this clipped 32-bit count(when the
value is >= 2^32) as reference, the requested sequence remains a 32-bit
value and gets queued like that. However, the code that checks if the
requested sequence has passed compares this against the 64-bit vblank
count.

With drm_vblank_count() returning all bits of the vblank count, update
drm_crtc_accurate_vblank_count() so that drm_crtc_arm_vblank_event() queues
the correct sequence. Otherwise, this leads to prolonged waits for a vblank
sequence when the current count is >=2^32.

Finally, fix drm_wait_one_vblank() too.

v2: Commit message fix (Keith)
Squash commits (Rodrigo)

Fixes: 570e86963a51 ("drm: Widen vblank count to 64-bits [v3]")
Cc: Keith Packard 
Cc: Michel Dänzer 
Cc: Daniel Vetter 
Cc: Rodrigo Vivi 
Signed-off-by: Dhinakaran Pandiyan 
Acked-by: Daniel Vetter 
---
 drivers/gpu/drm/drm_vblank.c | 8 
 include/drm/drm_vblank.h | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 32d9bcf5be7f..f0d3ed5f2528 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -271,7 +271,7 @@ static void drm_update_vblank_count(struct drm_device *dev, 
unsigned int pipe,
store_vblank(dev, pipe, diff, t_vblank, cur_vblank);
 }
 
-static u32 drm_vblank_count(struct drm_device *dev, unsigned int pipe)
+static u64 drm_vblank_count(struct drm_device *dev, unsigned int pipe)
 {
struct drm_vblank_crtc *vblank = >vblank[pipe];
 
@@ -292,11 +292,11 @@ static u32 drm_vblank_count(struct drm_device *dev, 
unsigned int pipe)
  * This is mostly useful for hardware that can obtain the scanout position, but
  * doesn't have a hardware frame counter.
  */
-u32 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc)
+u64 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc)
 {
struct drm_device *dev = crtc->dev;
unsigned int pipe = drm_crtc_index(crtc);
-   u32 vblank;
+   u64 vblank;
unsigned long flags;
 
WARN_ONCE(drm_debug & DRM_UT_VBL && !dev->driver->get_vblank_timestamp,
@@ -1055,7 +1055,7 @@ void drm_wait_one_vblank(struct drm_device *dev, unsigned 
int pipe)
 {
struct drm_vblank_crtc *vblank = >vblank[pipe];
int ret;
-   u32 last;
+   u64 last;
 
if (WARN_ON(pipe >= dev->num_crtcs))
return;
diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
index 848b463a0af5..a4c3b0a0a197 100644
--- a/include/drm/drm_vblank.h
+++ b/include/drm/drm_vblank.h
@@ -179,7 +179,7 @@ void drm_crtc_wait_one_vblank(struct drm_crtc *crtc);
 void drm_crtc_vblank_off(struct drm_crtc *crtc);
 void drm_crtc_vblank_reset(struct drm_crtc *crtc);
 void drm_crtc_vblank_on(struct drm_crtc *crtc);
-u32 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc);
+u64 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc);
 
 bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
   unsigned int pipe, int *max_error,
-- 
2.14.1

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

[PATCH 03/10] drm/i915: Handle 64-bit return from drm_crtc_vblank_count()

[Dhinakaran Pandiyan]

570e86963a51 ("drm: Widen vblank count to 64-bits [v3]") changed the
return type for drm_crtc_vblank_count() to u64, store all the bits
without truncating. There is no need to type cast this value down to
32-bits.

Cc: Keith Packard 
Cc: Paulo Zanoni 
Cc: Rodrigo Vivi 
Signed-off-by: Dhinakaran Pandiyan 
---
 drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
 drivers/gpu/drm/i915/i915_drv.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 3849ded354e3..4b9da04c1d4a 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1599,7 +1599,7 @@ static int i915_fbc_status(struct seq_file *m, void 
*unused)
seq_printf(m, "FBC disabled: %s\n", fbc->no_fbc_reason);
 
if (fbc->work.scheduled)
-   seq_printf(m, "FBC worker scheduled on vblank %u, now %llu\n",
+   seq_printf(m, "FBC worker scheduled on vblank %llu, now %llu\n",
   fbc->work.scheduled_vblank,
   drm_crtc_vblank_count(>crtc->base));
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c676269ed843..d22677494499 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -717,7 +717,7 @@ struct intel_fbc {
 
struct intel_fbc_work {
bool scheduled;
-   u32 scheduled_vblank;
+   u64 scheduled_vblank;
struct work_struct work;
} work;
 
-- 
2.14.1

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

[Bug 104888] DPMS issue w/ GFX8/Polaris10/Ellesmere/Rx-480-8GiB & agd5f's drm-next-4.17-wip

[bugzilla-daemon]

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

Robin Kauffman  changed:

   What|Removed |Added

 Status|RESOLVED|REOPENED
 Resolution|NOTABUG |---

--- Comment #6 from Robin Kauffman  ---
Reopening since the DPMS timeout (that *used to* work) still isn't 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

[Bug 104888] DPMS issue w/ GFX8/Polaris10/Ellesmere/Rx-480-8GiB & agd5f's drm-next-4.17-wip

[bugzilla-daemon]

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

--- Comment #5 from Robin Kauffman  ---
(In reply to Harry Wentland from comment #3)
> Curiously enough this is the first time I've heard of setterm. I'm not sure
> if it's really supported but I tried amdgpu, non-amdgpu (i.e. vbios), and an
> Intel platform and on neither I can get setterm --powersafe on/off to do
> anything, no matter whether in a VT or in X (seems like it's intended for
> VT).
> 
> We do support "xset -display :0.0 dpms force off" when you're in X.

Huh, interesting.  I can't (yet) test 'xset -display :0.0 dpms force off',
since in still having issues bringing X/Wayland up, but that's a separate bug.

No useful output from amdgpu.dc_log=1 and drm.debug=4 set yet, but I'm trying
agd5f's latest push to drm-next-4.17-wip, which has a commit that looks like it
may contain a fix, so we'll see.

-- 
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 5/5] drm/amdgpu: replace iova debugfs file with iomem

[StDenis, Tom]

I haven't tried the patch but just like to point out this breaks umr :-)  I'll 
have to craft something on Monday to support this and iova in parallel until 
the iova kernels are realistically EOL'ed.

On the other hand I support this idea since it eliminates the need for an fmem 
hack.  So much appreciated.

Cheers,
Tom


From: amd-gfx  on behalf of Christian 
König 
Sent: Friday, February 2, 2018 14:09
To: amd-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org
Subject: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

This allows access to pages allocated through the driver with optional
IOMMU mapping.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 57 -
 1 file changed, 35 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 648c449aaa79..795ceaeb82d5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1929,38 +1929,51 @@ static const struct file_operations amdgpu_ttm_gtt_fops 
= {

 #endif

-static ssize_t amdgpu_iova_to_phys_read(struct file *f, char __user *buf,
-  size_t size, loff_t *pos)
+static ssize_t amdgpu_iomem_read(struct file *f, char __user *buf,
+size_t size, loff_t *pos)
 {
struct amdgpu_device *adev = file_inode(f)->i_private;
-   int r;
-   uint64_t phys;
struct iommu_domain *dom;
+   ssize_t result = 0;
+   int r;

-   // always return 8 bytes
-   if (size != 8)
-   return -EINVAL;
+   dom = iommu_get_domain_for_dev(adev->dev);

-   // only accept page addresses
-   if (*pos & 0xFFF)
-   return -EINVAL;
+   while (size) {
+   phys_addr_t addr = *pos & PAGE_MASK;
+   loff_t off = *pos & ~PAGE_MASK;
+   size_t bytes = PAGE_SIZE - off;
+   unsigned long pfn;
+   struct page *p;
+   void *ptr;

-   dom = iommu_get_domain_for_dev(adev->dev);
-   if (dom)
-   phys = iommu_iova_to_phys(dom, *pos);
-   else
-   phys = *pos;
+   addr = dom ? iommu_iova_to_phys(dom, addr) : addr;

-   r = copy_to_user(buf, , 8);
-   if (r)
-   return -EFAULT;
+   pfn = addr >> PAGE_SHIFT;
+   if (!pfn_valid(pfn))
+   return -EPERM;
+
+   p = pfn_to_page(pfn);
+   if (p->mapping != adev->mman.bdev.dev_mapping)
+   return -EPERM;
+
+   ptr = kmap(p);
+   r = copy_to_user(buf, ptr, bytes);
+   kunmap(p);
+   if (r)
+   return -EFAULT;

-   return 8;
+   size -= bytes;
+   *pos += bytes;
+   result += bytes;
+   }
+
+   return result;
 }

-static const struct file_operations amdgpu_ttm_iova_fops = {
+static const struct file_operations amdgpu_ttm_iomem_fops = {
.owner = THIS_MODULE,
-   .read = amdgpu_iova_to_phys_read,
+   .read = amdgpu_iomem_read,
.llseek = default_llseek
 };

@@ -1973,7 +1986,7 @@ static const struct {
 #ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
{ "amdgpu_gtt", _ttm_gtt_fops, TTM_PL_TT },
 #endif
-   { "amdgpu_iova", _ttm_iova_fops, TTM_PL_SYSTEM },
+   { "amdgpu_iomem", _ttm_iomem_fops, TTM_PL_SYSTEM },
 };

 #endif
--
2.14.1

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

Re: [PATCH v4] Fix loading of module radeonfb on PowerMac

[kbuild test robot]

Hi Mathieu,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v4.15 next-20180202]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Mathieu-Malaterre/Fix-loading-of-module-radeonfb-on-PowerMac/20180203-085907
config: x86_64-randconfig-x009-201804 (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from drivers/video/fbdev/aty/radeon_base.c:91:0:
>> drivers/video/fbdev/aty/../edid.h:21:0: warning: "EDID_LENGTH" redefined
#define EDID_LENGTH0x80

   In file included from include/drm/drm_crtc.h:44:0,
from include/drm/drm_fb_helper.h:35,
from drivers/video/fbdev/aty/radeon_base.c:73:
   include/drm/drm_edid.h:32:0: note: this is the location of the previous 
definition
#define EDID_LENGTH 128

   Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:fls64
   Cyclomatic Complexity 1 include/linux/log2.h:__ilog2_u64
   Cyclomatic Complexity 1 include/asm-generic/getorder.h:__get_order
   Cyclomatic Complexity 1 include/linux/string.h:strnlen
   Cyclomatic Complexity 4 include/linux/string.h:strlen
   Cyclomatic Complexity 6 include/linux/string.h:strlcpy
   Cyclomatic Complexity 4 include/linux/string.h:memcpy
   Cyclomatic Complexity 1 
arch/x86/include/asm/paravirt.h:arch_local_irq_disable
   Cyclomatic Complexity 1 arch/x86/include/asm/paravirt.h:arch_local_irq_enable
   Cyclomatic Complexity 1 include/linux/spinlock.h:spinlock_check
   Cyclomatic Complexity 1 include/linux/spinlock.h:spin_unlock_irqrestore
   Cyclomatic Complexity 1 include/linux/jiffies.h:_msecs_to_jiffies
   Cyclomatic Complexity 3 include/linux/jiffies.h:msecs_to_jiffies
   Cyclomatic Complexity 1 arch/x86/include/asm/io.h:readb
   Cyclomatic Complexity 1 arch/x86/include/asm/io.h:readw
   Cyclomatic Complexity 1 arch/x86/include/asm/io.h:readl
   Cyclomatic Complexity 1 arch/x86/include/asm/io.h:writeb
   Cyclomatic Complexity 1 arch/x86/include/asm/io.h:writel
   Cyclomatic Complexity 1 arch/x86/include/asm/io.h:ioremap
   Cyclomatic Complexity 1 include/linux/kobject.h:kobject_name
   Cyclomatic Complexity 2 include/linux/device.h:dev_name
   Cyclomatic Complexity 1 include/linux/device.h:dev_get_drvdata
   Cyclomatic Complexity 1 include/linux/device.h:dev_set_drvdata
   Cyclomatic Complexity 1 include/linux/io.h:arch_phys_wc_add
   Cyclomatic Complexity 1 include/linux/io.h:arch_phys_wc_del
   Cyclomatic Complexity 68 include/linux/slab.h:kmalloc_large
   Cyclomatic Complexity 3 include/linux/slab.h:kmalloc
   Cyclomatic Complexity 1 include/linux/slab.h:kzalloc
   Cyclomatic Complexity 1 include/linux/pci.h:pci_get_drvdata
   Cyclomatic Complexity 1 include/linux/pci.h:pci_set_drvdata
   Cyclomatic Complexity 1 include/linux/pci.h:pci_name
   Cyclomatic Complexity 2 include/linux/fb.h:alloc_apertures
   Cyclomatic Complexity 2 
drivers/video/fbdev/aty/radeonfb.h:radeon_pll_errata_after_index
   Cyclomatic Complexity 2 
drivers/video/fbdev/aty/radeonfb.h:radeon_pll_errata_after_data
   Cyclomatic Complexity 1 drivers/video/fbdev/aty/radeonfb.h:round_div
   Cyclomatic Complexity 3 drivers/video/fbdev/aty/radeonfb.h:var_to_depth
   Cyclomatic Complexity 5 drivers/video/fbdev/aty/radeonfb.h:radeon_get_dstbpp
   Cyclomatic Complexity 1 drivers/video/fbdev/aty/radeonfb.h:radeonfb_bl_init
   Cyclomatic Complexity 1 drivers/video/fbdev/aty/radeonfb.h:radeonfb_bl_exit
   Cyclomatic Complexity 1 
include/drm/drm_fb_helper.h:drm_fb_helper_remove_conflicting_framebuffers
   Cyclomatic Complexity 21 
drivers/video/fbdev/aty/radeon_base.c:radeon_calc_pll_regs
   Cyclomatic Complexity 1 drivers/video/fbdev/aty/radeon_base.c:radeonfb_exit
   Cyclomatic Complexity 6 
drivers/video/fbdev/aty/radeon_base.c:radeon_find_mem_vbios
   Cyclomatic Complexity 4 
drivers/video/fbdev/aty/radeon_base.c:radeon_kick_out_firmware_fb
   Cyclomatic Complexity 5 
drivers/video/fbdev/aty/radeon_base.c:radeonfb_pci_unregister
   Cyclomatic Complexity 1 
drivers/video/fbdev/aty/radeon_base.c:radeon_show_one_edid
   Cyclomatic Complexity 3 
drivers/video/fbdev/aty/radeon_base.c:radeon_show_edid2
   Cyclomatic Complexity 3 
drivers/video/fbdev/aty/radeon_base.c:radeon_show_edid1
   Cyclomatic Complexity 2 
drivers/video/fbdev/aty/radeon_base.c:radeon_set_fbinfo
   Cyclomatic Complexity 18 
drivers/video/fbdev/aty/radeon_base.c:radeonfb_check_var
   Cyclomatic Complexity 2 
drivers/video/fbdev/aty/radeon_base.c:radeon_unmap_ROM
   Cyclomatic Complexity 7 drivers/video/fbdev/aty/radeon_base.c:radeon_map_ROM
   Cyclomatic Complexity 16 drivers/video/fbdev/aty/radeon_base.c:radeonfb_setup
   Cyclomatic Complexity 2 drivers/video/fbdev/aty/radeo

Re: [PATCH 1/5 v2] drm/pl111: Properly detect the ARM PL110 variants

[Eric Anholt]

Linus Walleij  writes:

> With a bit of refactoring we can contain the variant data for
> the strange PL110 versions that is feature-incomplete PL110 for
> the ARM Integrator/CP and somewhere inbetween PL110 and PL111
> for the ARM Versatile AB and Versatile PB.
>
> We also accomodate for the custom duct-taped RGB565/BGR565 support
> in the Versatile variant.
>
> Signed-off-by: Linus Walleij 
> ---
> ChangeLog v1->v2:
> - Push more logic into the pl111_versatile file and keep the
>   driver core neutral.
> - Pave the way better for the Integrator/CP variant as well.
> ---
>  drivers/gpu/drm/pl111/pl111_drm.h   |  3 ++
>  drivers/gpu/drm/pl111/pl111_drv.c   | 37 --
>  drivers/gpu/drm/pl111/pl111_versatile.c | 85 
> +
>  3 files changed, 79 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/gpu/drm/pl111/pl111_drm.h 
> b/drivers/gpu/drm/pl111/pl111_drm.h
> index 440f53ebee8c..c2f410f0b12e 100644
> --- a/drivers/gpu/drm/pl111/pl111_drm.h
> +++ b/drivers/gpu/drm/pl111/pl111_drm.h
> @@ -36,12 +36,15 @@ struct drm_minor;
>   * struct pl111_variant_data - encodes IP differences
>   * @name: the name of this variant
>   * @is_pl110: this is the early PL110 variant
> + * @external_bgr: this is the Versatile Pl110 variant with external
> + *   BGR/RGB routing
>   * @formats: array of supported pixel formats on this variant
>   * @nformats: the length of the array of supported pixel formats
>   */
>  struct pl111_variant_data {
>   const char *name;
>   bool is_pl110;
> + bool external_bgr;
>   const u32 *formats;
>   unsigned int nformats;
>  };
> diff --git a/drivers/gpu/drm/pl111/pl111_drv.c 
> b/drivers/gpu/drm/pl111/pl111_drv.c
> index 31a0c4268cc6..6967cd5428b2 100644
> --- a/drivers/gpu/drm/pl111/pl111_drv.c
> +++ b/drivers/gpu/drm/pl111/pl111_drv.c
> @@ -205,7 +205,7 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
>  {
>   struct device *dev = _dev->dev;
>   struct pl111_drm_dev_private *priv;
> - struct pl111_variant_data *variant = id->data;
> + const struct pl111_variant_data *variant = id->data;
>   struct drm_device *drm;
>   int ret;
>  
> @@ -221,27 +221,10 @@ static int pl111_amba_probe(struct amba_device 
> *amba_dev,
>   drm->dev_private = priv;
>   priv->variant = variant;
>  
> - /*
> -  * The PL110 and PL111 variants have two registers
> -  * swapped: interrupt enable and control. For this reason
> -  * we use offsets that we can change per variant.
> -  */
> + /* The two variants swap this register */
>   if (variant->is_pl110) {
> - /*
> -  * The ARM Versatile boards are even more special:
> -  * their PrimeCell ID say they are PL110 but the
> -  * control and interrupt enable registers are anyway
> -  * swapped to the PL111 order so they are not following
> -  * the PL110 datasheet.
> -  */
> - if (of_machine_is_compatible("arm,versatile-ab") ||
> - of_machine_is_compatible("arm,versatile-pb")) {
> - priv->ienb = CLCD_PL111_IENB;
> - priv->ctrl = CLCD_PL111_CNTL;
> - } else {
> - priv->ienb = CLCD_PL110_IENB;
> - priv->ctrl = CLCD_PL110_CNTL;
> - }
> + priv->ienb = CLCD_PL110_IENB;
> + priv->ctrl = CLCD_PL110_CNTL;
>   } else {
>   priv->ienb = CLCD_PL111_IENB;
>   priv->ctrl = CLCD_PL111_CNTL;
> @@ -253,6 +236,11 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
>   return PTR_ERR(priv->regs);
>   }
>  
> + /* This may override some variant settings */
> + ret = pl111_versatile_init(dev, priv);
> + if (ret)
> + goto dev_unref;
> +
>   /* turn off interrupts before requesting the irq */
>   writel(0, priv->regs + priv->ienb);
>  
> @@ -263,10 +251,6 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
>   return ret;
>   }
>  
> - ret = pl111_versatile_init(dev, priv);
> - if (ret)
> - goto dev_unref;
> -
>   ret = pl111_modeset_init(drm);
>   if (ret != 0)
>   goto dev_unref;
> @@ -299,8 +283,7 @@ static int pl111_amba_remove(struct amba_device *amba_dev)
>  }
>  
>  /*
> - * This variant exist in early versions like the ARM Integrator
> - * and this version lacks the 565 and 444 pixel formats.
> + * This early variant lacks the 565 and 444 pixel formats.
>   */
>  static const u32 pl110_pixel_formats[] = {
>   DRM_FORMAT_ABGR,
> diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c 
> b/drivers/gpu/drm/pl111/pl111_versatile.c
> index 97d4af6925a3..893d527fb42f 100644
> --- a/drivers/gpu/drm/pl111/pl111_versatile.c
> +++ b/drivers/gpu/drm/pl111/pl111_versatile.c
> @@ -1,3 +1,4 @@
> +#include 
>  

[Bug 104920] Broken hardware video encoding with vaapi/ffmpeg

[bugzilla-daemon]

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

--- Comment #1 from Andy Furniss  ---
You need -profile:v 578.

x11grab on my old CPU at least is not the best way to go.
ffmpeg is slow doing the BGR0 -> nv12 conversion, and by default it will be 601
rather than 709.
There are ways around that, and to be a bit faster with some convoluted
commands.

I don't know what versions of everything you need, but you can "now" get the
GPU to do the conversion and zero copy input, which is way faster and does BGR0
-> rec 709 CSC.

The downside is you need to be root or suid the ffmpeg binary.

1920x1080 is also slightly problematic but using mp4 or mkv will hide it (I
guess depending on player). mkv seems to be broken with current ffmpeg git,
bah. The issue being that eg. .ts would be seen as 1088 rather than 1080.

To get GPU accel using the newer/faster way would be something like -

ffmpeg -framerate 60 -device /dev/dri/card0 -f kmsgrab -i - -vsync 0
-init_hw_device vaapi=v:/dev/dri/renderD128 -filter_hw_device v -vf
'hwmap,scale_vaapi=format=nv12' -c:v h264_vaapi -profile:v 578 -bf 0 out.mp4

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

Re: [PATCH] drm/bridge/synopsys: dsi: use adjusted_mode in mode_set

[Philippe CORNU]

Hi Laurent & Daniel :-)

On 01/29/2018 11:40 AM, Laurent Pinchart wrote:
> Hi Philippe,
> 
> On Monday, 29 January 2018 12:17:37 EET Philippe CORNU wrote:
>> On 01/29/2018 10:46 AM, Laurent Pinchart wrote:
>>> On Thursday, 25 January 2018 17:55:04 EET Philippe Cornu wrote:
>>>
 The "adjusted_mode" clock value (ie the real pixel clock) is more
 accurate than "mode" clock value (ie the panel/bridge requested
 clock value). It offers a better preciseness for timing
 computations and allows to reduce the extra dsi bandwidth in
 burst mode (from ~20% to ~10-12%, hw platform dependant).

 Signed-off-by: Philippe Cornu 
>>>
>>> The adjusted mode is documented as
>>>
>>>   /**
>>>* @adjusted_mode:
>>>*
>>>* Internal display timings which can be used by the driver to handle
>>>* differences between the mode requested by userspace in @mode and what
>>>* is actually programmed into the hardware. It is purely driver
>>>* implementation defined what exactly this adjusted mode means. Usually
>>>* it is used to store the hardware display timings used between the
>>>* CRTC and encoder blocks.
>>>*/
>>>
>>> This is easy to handle when the CRTC and encoder are controlled by the
>>> same driver, as the field is "implementation defined" by a single driver
>>> . However, when using bridges, there are two drivers involved, and they
>>> must both agree to meaningfully use the adjusted mode. I can't see how to
>>> do so without standardizing the meaning of the adjusted mode field.
>>
>> This is exactly the reason why my first implementation used the dsi
>> bridge "optional pixel clock" instead of the adjusted_mode (see [1])
>>
>> But after digging more into the drm source code, I think using
>> adjusted_mode instead of the pixel clock here brings more advantages
>> because:
>> * adjusted_mode is an argument of bridge mode_set() probably for being
>> used in any manner, maybe like this :)
>> * if the bridge "user" (crtc or a master bridge drivers) does not need
>> to modify its adjusted_mode then mode & adjusted_mode mode_set()
>> arguments will have the same values so "no consequence" for the bridge.
>> * if the bridge "user" (crtc or master bridge drivers) needs to adjust
>> any value of the mode then this adjustment is available for the bridge.
> 
> Remember that there can be multiple chained bridges, and a single adjusted
> mode field.
> 
>> * rockchip crtc updates a part of the mode (the clock), stm is doing the
>> same (see [2]) but any future "user" of the dw_mipi_dsi bridge can
>> adjust something else (blankings...) and the dw_mipi_dsi bridge will be
>> then aware of...
>>
>> But maybe it is a wrong usage of the "adjusted_mode offer"...
> 
> I don't disagree that there's a need for using adjusted values, but I believe
> we need to create a clear API to do so. Using the adjust_mode field as-is when
> it's clearly documented as being implementation-defined is asking for trouble.
> 

Laurent, do you think we can use "adjusted mode" here in this small 
patch as the actual 2 "users" of this bridge (rockchip & stm) use both 
"adjusted mode" in their crtc?

>> Many thanks,
>> Philippe :-)
>> [1] https://patchwork.freedesktop.org/patch/200240/
>> [2] https://patchwork.freedesktop.org/patch/200720/
>>
>>> Daniel, what's your opinion on this ?
>>>

Daniel, any opinion on the adjusted_mode usage?

Many thanks to both of you,
Philippe :-)

 ---
 Note: This patch replaces "drm/bridge/synopsys: dsi: add optional pixel
 clock"

drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)

 diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
 b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c index
 ed8af32f8e52..b926b62e9e33 100644
 --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
 +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
 @@ -707,20 +707,20 @@ static void dw_mipi_dsi_bridge_mode_set(struct
 drm_bridge *bridge,

clk_prepare_enable(dsi->pclk);

 -  ret = phy_ops->get_lane_mbps(priv_data, mode, dsi->mode_flags,
 +  ret = phy_ops->get_lane_mbps(priv_data, adjusted_mode,
 dsi->mode_flags,
 dsi->lanes, dsi->format, 
 >lane_mbps);
if (ret)
DRM_DEBUG_DRIVER("Phy get_lane_mbps() failed\n");

pm_runtime_get_sync(dsi->dev);
dw_mipi_dsi_init(dsi);

 -  dw_mipi_dsi_dpi_config(dsi, mode);
 +  dw_mipi_dsi_dpi_config(dsi, adjusted_mode);
dw_mipi_dsi_packet_handler_config(dsi);
dw_mipi_dsi_video_mode_config(dsi);
 -  dw_mipi_dsi_video_packet_config(dsi, mode);
 +  dw_mipi_dsi_video_packet_config(dsi, adjusted_mode);
dw_mipi_dsi_command_mode_config(dsi);
 -  dw_mipi_dsi_line_timer_config(dsi, mode);
 -  dw_mipi_dsi_vertical_timing_config(dsi, mode);
 +  

Re: [PATCH] drm/bridge/synopsys: dsi: Add 1.31 version support

[Philippe CORNU]

Hi Archit, Andrzej, Laurent & Brian,

What is your opinion regarding this patch? Do you have any advice for 
handling hw versions?

Do not hesitate to comment.

Many thanks,
Philippe :-)


On 01/22/2018 04:08 PM, Philippe Cornu wrote:
> From: Philippe CORNU 
> 
> Add support for the Synopsys DesignWare MIPI DSI version 1.31
> Two registers need to be updated/added for supporting 1.31:
> * PHY_TMR_CFG 0x9c (updated)
>1.30 [31:24] phy_hs2lp_time
> [23:16] phy_lp2hs_time
> [14: 0] max_rd_time
> 
>1.31 [25:16] phy_hs2lp_time
> [ 9: 0] phy_lp2hs_time
> 
> * PHY_TMR_RD_CFG 0xf4 (new)
>1.31 [14: 0] max_rd_time
> 
> Signed-off-by: Philippe Cornu 
> ---
>   drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 52 
> +++
>   1 file changed, 46 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c 
> b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index 735f38429c06..20a2ca14a7ad 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -25,7 +25,13 @@
>   #include 
>   #include 
>   
> +#define HWVER_1300x31333000  /* IP version 1.30 */
> +#define HWVER_1310x31333100  /* IP version 1.31 */
> +#define HWVER_OLDEST HWVER_130
> +#define HWVER_NEWEST HWVER_131
> +
>   #define DSI_VERSION 0x00
> +#define VERSION  GENMASK(31, 8)
>   
>   #define DSI_PWR_UP  0x04
>   #define RESET   0
> @@ -161,11 +167,12 @@
>   #define PHY_CLKHS2LP_TIME(lbcc) (((lbcc) & 0x3ff) << 16)
>   #define PHY_CLKLP2HS_TIME(lbcc) ((lbcc) & 0x3ff)
>   
> -/* TODO Next register is slightly different between 1.30 & 1.31 IP version */
>   #define DSI_PHY_TMR_CFG 0x9c
> -#define PHY_HS2LP_TIME(lbcc) (((lbcc) & 0xff) << 24)
> -#define PHY_LP2HS_TIME(lbcc) (((lbcc) & 0xff) << 16)
> -#define MAX_RD_TIME(lbcc)((lbcc) & 0x7fff)
> +#define PHY_HS2LP_TIME_V130(lbcc)(((lbcc) & 0xff) << 24)
> +#define PHY_LP2HS_TIME_V130(lbcc)(((lbcc) & 0xff) << 16)
> +#define MAX_RD_TIME_V130(lbcc)   ((lbcc) & 0x7fff)
> +#define PHY_HS2LP_TIME_V131(lbcc)(((lbcc) & 0x3ff) << 16)
> +#define PHY_LP2HS_TIME_V131(lbcc)((lbcc) & 0x3ff)
>   
>   #define DSI_PHY_RSTZ0xa0
>   #define PHY_DISFORCEPLL 0
> @@ -204,7 +211,9 @@
>   #define DSI_INT_ST1 0xc0
>   #define DSI_INT_MSK00xc4
>   #define DSI_INT_MSK10xc8
> +
>   #define DSI_PHY_TMR_RD_CFG  0xf4
> +#define MAX_RD_TIME_V131(lbcc)   ((lbcc) & 0x7fff)
>   
>   #define PHY_STATUS_TIMEOUT_US   1
>   #define CMD_PKT_STATUS_TIMEOUT_US   2
> @@ -215,6 +224,7 @@ struct dw_mipi_dsi {
>   struct drm_bridge *panel_bridge;
>   struct device *dev;
>   void __iomem *base;
> + u32 hw_version;
>   
>   struct clk *pclk;
>   struct clk *px_clk;
> @@ -616,8 +626,14 @@ static void dw_mipi_dsi_dphy_timing_config(struct 
> dw_mipi_dsi *dsi)
>* note: DSI_PHY_TMR_CFG.MAX_RD_TIME should be in line with
>* DSI_CMD_MODE_CFG.MAX_RD_PKT_SIZE_LP (see CMD_MODE_ALL_LP)
>*/
> - dsi_write(dsi, DSI_PHY_TMR_CFG, PHY_HS2LP_TIME(0x40)
> -   | PHY_LP2HS_TIME(0x40) | MAX_RD_TIME(1));
> + if (dsi->hw_version == HWVER_131) {
> + dsi_write(dsi, DSI_PHY_TMR_CFG, PHY_HS2LP_TIME_V131(0x40) |
> +   PHY_LP2HS_TIME_V131(0x40));
> + dsi_write(dsi, DSI_PHY_TMR_RD_CFG, MAX_RD_TIME_V131(1));
> + } else {
> + dsi_write(dsi, DSI_PHY_TMR_CFG, PHY_HS2LP_TIME_V130(0x40) |
> +   PHY_LP2HS_TIME_V130(0x40) | MAX_RD_TIME_V130(1));
> + }
>   
>   dsi_write(dsi, DSI_PHY_TMR_LPCLK_CFG, PHY_CLKHS2LP_TIME(0x40)
> | PHY_CLKLP2HS_TIME(0x40));
> @@ -791,6 +807,28 @@ static const struct drm_bridge_funcs 
> dw_mipi_dsi_bridge_funcs = {
>   .attach   = dw_mipi_dsi_bridge_attach,
>   };
>   
> +static void dsi_get_version(struct dw_mipi_dsi *dsi)
> +{
> + u32 hw_version;
> +
> + clk_prepare_enable(dsi->pclk);
> + hw_version = dsi_read(dsi, DSI_VERSION) & VERSION;
> + clk_disable_unprepare(dsi->pclk);
> +
> + if (hw_version > HWVER_NEWEST) {
> + DRM_DEBUG("hw version: use 0x%08x for this recent 0x%08x\n",
> +   HWVER_NEWEST, hw_version);
> + hw_version = HWVER_NEWEST;
> +
> + } else if (hw_version < HWVER_OLDEST) {
> + DRM_DEBUG("hw version: use 0x%08x for this old 0x%08x\n",
> +   HWVER_OLDEST, hw_version);
> + hw_version = HWVER_OLDEST;
> + }
> +
> + dsi->hw_version = hw_version;
> +}
> +
>  

Re: [PATCH 2/2] drm/amdgpu/display: successful spelling fix

[Harry Wentland]

On 2018-02-02 04:37 PM, db...@chromium.org wrote:
> From: Dominik Behr 
> 
> Replace SUCESSFULL with SUCCESSFUL.
> 
> Signed-off-by: Dominik Behr 

Series is
Reviewed-by: Harry Wentland 

Will pull it into amd-stg over the weekend or on Monday.

Harry

> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 +-
>  drivers/gpu/drm/amd/display/dc/core/dc_link.c   | 2 +-
>  drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c   | 4 ++--
>  drivers/gpu/drm/amd/display/include/ddc_service_types.h | 2 +-
>  4 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> index 1e8a21b67df7..3b05900d 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> @@ -130,7 +130,7 @@ static ssize_t dm_dp_aux_transfer(struct drm_dp_aux *aux,
>msg->address,
>msg->buffer,
>msg->size,
> -  r == DDC_RESULT_SUCESSFULL);
> +  r == DDC_RESULT_SUCCESSFUL);
>  #endif
>  
>   return msg->size;
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c 
> b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> index 0023754e034b..3abd0f1a287f 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> @@ -1176,7 +1176,7 @@ static void dpcd_configure_panel_mode(
>   _config_set.raw,
>   sizeof(edp_config_set.raw));
>  
> - ASSERT(result == DDC_RESULT_SUCESSFULL);
> + ASSERT(result == DDC_RESULT_SUCCESSFUL);
>   }
>   }
>   dm_logger_write(link->ctx->logger, LOG_DETECTION_DP_CAPS,
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c 
> b/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
> index d5294798b0a5..6b69b339dba2 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
> @@ -661,7 +661,7 @@ enum ddc_result dal_ddc_service_read_dpcd_data(
>   ddc->ctx->i2caux,
>   ddc->ddc_pin,
>   ))
> - return DDC_RESULT_SUCESSFULL;
> + return DDC_RESULT_SUCCESSFUL;
>  
>   return DDC_RESULT_FAILED_OPERATION;
>  }
> @@ -698,7 +698,7 @@ enum ddc_result dal_ddc_service_write_dpcd_data(
>   ddc->ctx->i2caux,
>   ddc->ddc_pin,
>   ))
> - return DDC_RESULT_SUCESSFULL;
> + return DDC_RESULT_SUCCESSFUL;
>  
>   return DDC_RESULT_FAILED_OPERATION;
>  }
> diff --git a/drivers/gpu/drm/amd/display/include/ddc_service_types.h 
> b/drivers/gpu/drm/amd/display/include/ddc_service_types.h
> index 019e7a095ea1..f3bf749b3636 100644
> --- a/drivers/gpu/drm/amd/display/include/ddc_service_types.h
> +++ b/drivers/gpu/drm/amd/display/include/ddc_service_types.h
> @@ -32,7 +32,7 @@
>  
>  enum ddc_result {
>   DDC_RESULT_UNKNOWN = 0,
> - DDC_RESULT_SUCESSFULL,
> + DDC_RESULT_SUCCESSFUL,
>   DDC_RESULT_FAILED_CHANNEL_BUSY,
>   DDC_RESULT_FAILED_TIMEOUT,
>   DDC_RESULT_FAILED_PROTOCOL_ERROR,
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] i915 PSR test results and cursor lag

[Chris Wilson]

Quoting Andy Lutomirski (2018-02-02 19:23:33)
> On Fri, Feb 2, 2018 at 7:18 PM, Andy Lutomirski  wrote:
> > On Fri, Feb 2, 2018 at 1:24 AM, Andy Lutomirski  wrote:
> >> Anyway, this is all on a 4.14 kernel.  I should update to 4.16 and see
> >> what happens.
> >
> > I updated to 4.15, and the situation is much worse.  With
> > enable_psr=1, the system survives for several seconds and then the
> > screen stops updating entirely.  If I boot with i915.enable_psr=1, I
> > get to the Fedora login screen and then the system dies.  If I set
> > enable_psr=1 using sysfs, it does a bit after the next resume.  It
> > seems like it also sometimes hangs even worse a bit after the screen
> > stops updating, but it's hard to tell.
> >
> > I see this in my logs:
> >
> > [drm:drm_atomic_helper_wait_for_flip_done [drm_kms_helper]] *ERROR*
> > [CRTC:37:pipe A] flip_done timed out
> >
> > Sometimes I see this a bit later:
> >
> > [drm:drm_atomic_helper_wait_for_dependencies [drm_kms_helper]] *ERROR*
> > [CRTC:37:pipe A] flip_done timed out
> 
> I filed:
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=104918

Thank you. To be frank the only PSR system we have in CI isn't currently
doing PSR (due to the test not taking panel restrictions into account).
Not that I think we have any test looking for lag, but it should at
least have told us that PSR was snafu.
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 104919] R9285 4.17-wip locks/vmfaults since drm/amdgpu: revert "drm/amdgpu: use AMDGPU_GEM_CREATE_VRAM_CLEARED for VM PD/PTs" v2

[bugzilla-daemon]

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

--- Comment #3 from Andy Furniss  ---
(In reply to Christian König from comment #2)
> At least I now knew that the PASID handling is working fine.
> 
> Does it work if you disable the new clear method? E.g. just add a "return
> 0;" to the beginning of amdgpu_vm_clear_bo().

Seems good with that.
Just a quick test as got to be AFK, will try more later.

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

[PATCH 2/2] drm/amdgpu/display: successful spelling fix

[dbehr]

From: Dominik Behr 

Replace SUCESSFULL with SUCCESSFUL.

Signed-off-by: Dominik Behr 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 +-
 drivers/gpu/drm/amd/display/dc/core/dc_link.c   | 2 +-
 drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c   | 4 ++--
 drivers/gpu/drm/amd/display/include/ddc_service_types.h | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 1e8a21b67df7..3b05900d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -130,7 +130,7 @@ static ssize_t dm_dp_aux_transfer(struct drm_dp_aux *aux,
 msg->address,
 msg->buffer,
 msg->size,
-r == DDC_RESULT_SUCESSFULL);
+r == DDC_RESULT_SUCCESSFUL);
 #endif
 
return msg->size;
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
index 0023754e034b..3abd0f1a287f 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -1176,7 +1176,7 @@ static void dpcd_configure_panel_mode(
_config_set.raw,
sizeof(edp_config_set.raw));
 
-   ASSERT(result == DDC_RESULT_SUCESSFULL);
+   ASSERT(result == DDC_RESULT_SUCCESSFUL);
}
}
dm_logger_write(link->ctx->logger, LOG_DETECTION_DP_CAPS,
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
index d5294798b0a5..6b69b339dba2 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
@@ -661,7 +661,7 @@ enum ddc_result dal_ddc_service_read_dpcd_data(
ddc->ctx->i2caux,
ddc->ddc_pin,
))
-   return DDC_RESULT_SUCESSFULL;
+   return DDC_RESULT_SUCCESSFUL;
 
return DDC_RESULT_FAILED_OPERATION;
 }
@@ -698,7 +698,7 @@ enum ddc_result dal_ddc_service_write_dpcd_data(
ddc->ctx->i2caux,
ddc->ddc_pin,
))
-   return DDC_RESULT_SUCESSFULL;
+   return DDC_RESULT_SUCCESSFUL;
 
return DDC_RESULT_FAILED_OPERATION;
 }
diff --git a/drivers/gpu/drm/amd/display/include/ddc_service_types.h 
b/drivers/gpu/drm/amd/display/include/ddc_service_types.h
index 019e7a095ea1..f3bf749b3636 100644
--- a/drivers/gpu/drm/amd/display/include/ddc_service_types.h
+++ b/drivers/gpu/drm/amd/display/include/ddc_service_types.h
@@ -32,7 +32,7 @@
 
 enum ddc_result {
DDC_RESULT_UNKNOWN = 0,
-   DDC_RESULT_SUCESSFULL,
+   DDC_RESULT_SUCCESSFUL,
DDC_RESULT_FAILED_CHANNEL_BUSY,
DDC_RESULT_FAILED_TIMEOUT,
DDC_RESULT_FAILED_PROTOCOL_ERROR,
-- 
2.16.0.rc1.238.g530d649a79-goog

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

[PATCH 1/2] drm/amdgpu/display: fix wrong enum initializer

[dbehr]

From: Dominik Behr 

Signed-off-by: Dominik Behr 
---
 drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
index 33d91e4474ea..a83f7b83ce0f 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
@@ -1926,7 +1926,7 @@ bool dc_link_handle_hpd_rx_irq(struct dc_link *link, 
union hpd_irq_data *out_hpd
 {
union hpd_irq_data hpd_irq_dpcd_data = 0;
union device_service_irq device_service_clear = { { 0 } };
-   enum dc_status result = DDC_RESULT_UNKNOWN;
+   enum dc_status result = DC_OK;
bool status = false;
/* For use cases related to down stream connection status change,
 * PSR and device auto test, refer to function handle_sst_hpd_irq
-- 
2.16.0.rc1.238.g530d649a79-goog

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

Re: [PATCH] drm/amdgpu/display: fix wrong enum type for ddc_result

[Dominik Behr]

Aww, you are right. Let me redo it and do the spelling fix separately
because I cannot unsee it now.

On Fri, Feb 2, 2018 at 7:48 AM, Harry Wentland  wrote:
> On 2018-02-01 08:55 PM, db...@chromium.org wrote:
>> From: Dominik Behr 
>>
>> v2: now with fixed result comparison and spelling fixes
>>
>> Signed-off-by: Dominik Behr 
>> ---
>>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 +-
>>  drivers/gpu/drm/amd/display/dc/core/dc_link.c   | 2 +-
>>  drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c   | 4 ++--
>>  drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c| 4 ++--
>>  drivers/gpu/drm/amd/display/include/ddc_service_types.h | 2 +-
>>  5 files changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c 
>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
>> index 1e8a21b67df7..3b05900d 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
>> @@ -130,7 +130,7 @@ static ssize_t dm_dp_aux_transfer(struct drm_dp_aux *aux,
>>msg->address,
>>msg->buffer,
>>msg->size,
>> -  r == DDC_RESULT_SUCESSFULL);
>> +  r == DDC_RESULT_SUCCESSFUL);
>>  #endif
>>
>>   return msg->size;
>> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c 
>> b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
>> index 0023754e034b..3abd0f1a287f 100644
>> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
>> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
>> @@ -1176,7 +1176,7 @@ static void dpcd_configure_panel_mode(
>>   _config_set.raw,
>>   sizeof(edp_config_set.raw));
>>
>> - ASSERT(result == DDC_RESULT_SUCESSFULL);
>> + ASSERT(result == DDC_RESULT_SUCCESSFUL);
>>   }
>>   }
>>   dm_logger_write(link->ctx->logger, LOG_DETECTION_DP_CAPS,
>> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c 
>> b/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
>> index d5294798b0a5..6b69b339dba2 100644
>> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
>> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
>> @@ -661,7 +661,7 @@ enum ddc_result dal_ddc_service_read_dpcd_data(
>>   ddc->ctx->i2caux,
>>   ddc->ddc_pin,
>>   ))
>> - return DDC_RESULT_SUCESSFULL;
>> + return DDC_RESULT_SUCCESSFUL;
>>
>>   return DDC_RESULT_FAILED_OPERATION;
>>  }
>> @@ -698,7 +698,7 @@ enum ddc_result dal_ddc_service_write_dpcd_data(
>>   ddc->ctx->i2caux,
>>   ddc->ddc_pin,
>>   ))
>> - return DDC_RESULT_SUCESSFULL;
>> + return DDC_RESULT_SUCCESSFUL;
>>
>>   return DDC_RESULT_FAILED_OPERATION;
>>  }
>> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c 
>> b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
>> index 33d91e4474ea..cc067d04505d 100644
>> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
>> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
>> @@ -1926,7 +1926,7 @@ bool dc_link_handle_hpd_rx_irq(struct dc_link *link, 
>> union hpd_irq_data *out_hpd
>>  {
>>   union hpd_irq_data hpd_irq_dpcd_data = 0;
>>   union device_service_irq device_service_clear = { { 0 } };
>> - enum dc_status result = DDC_RESULT_UNKNOWN;
>> + enum ddc_result result = DDC_RESULT_UNKNOWN;
>
> Result gets the return value from read_hpd_rx_irq_data which is dc_status. 
> This line should be
>
> enum dc_status result = DC_OK;
>
>>   bool status = false;
>>   /* For use cases related to down stream connection status change,
>>* PSR and device auto test, refer to function handle_sst_hpd_irq
>> @@ -1946,7 +1946,7 @@ bool dc_link_handle_hpd_rx_irq(struct dc_link *link, 
>> union hpd_irq_data *out_hpd
>>   if (out_hpd_irq_dpcd_data)
>>   *out_hpd_irq_dpcd_data = hpd_irq_dpcd_data;
>>
>> - if (result != DC_OK) {
>> + if (result != DDC_RESULT_SUCCESSFUL) {
>
> We should keep the result != DC_OK check here as read_hpd_rx_irq_data returns 
> dc_status.
>
> Harry
>
>>   dm_logger_write(link->ctx->logger, LOG_HW_HPD_IRQ,
>>   "%s: DPCD read failed to obtain irq data\n",
>>   __func__);
>> diff --git a/drivers/gpu/drm/amd/display/include/ddc_service_types.h 
>> b/drivers/gpu/drm/amd/display/include/ddc_service_types.h
>> index 019e7a095ea1..f3bf749b3636 100644
>> --- a/drivers/gpu/drm/amd/display/include/ddc_service_types.h
>> +++ b/drivers/gpu/drm/amd/display/include/ddc_service_types.h
>> @@ -32,7 +32,7 @@
>>
>>  enum ddc_result {
>>   DDC_RESULT_UNKNOWN = 0,
>> - DDC_RESULT_SUCESSFULL,
>> + DDC_RESULT_SUCCESSFUL,
>>   

[Bug 104920] Broken hardware video encoding with vaapi/ffmpeg

[bugzilla-daemon]

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

Alex Deucher  changed:

   What|Removed |Added

Product|DRI |Mesa
  Component|DRM/AMDgpu  |Drivers/Gallium/radeonsi
 QA Contact||dri-devel@lists.freedesktop
   ||.org
Version|XOrg git|17.3

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

[Bug 104920] Broken hardware video encoding with vaapi/ffmpeg

[bugzilla-daemon]

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

Bug ID: 104920
   Summary: Broken hardware video encoding with vaapi/ffmpeg
   Product: DRI
   Version: XOrg git
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/AMDgpu
  Assignee: dri-devel@lists.freedesktop.org
  Reporter: s...@whiz.se

Hi,

I'm trying to encode video using ffmpeg 3.4.1 with the following command:

ffmpeg -vaapi_device /dev/dri/renderD128 -f x11grab -video_size 1920x1080 -i :0
-vf 'format=nv12,hwupload' -c:v h264_vaapi -bf 0 out.mp4

This seems to work, ffmpeg doesn't complain, and I can tell the GPU is active
as the fans turn on.

But the video recorded is corrupt and ffplay reports the following errors a
bunch of times: 

[h264 @ 0x7f4ae4003060] decode_slice_header error
[h264 @ 0x7f4ae4003060] no frame!
[h264 @ 0x7f4ae4003060] non-existing PPS 0 referenced

and

[mov,mp4,m4a,3gp,3g2,mj2 @ 0x56377e46] decoding for stream 0 failed
[mov,mp4,m4a,3gp,3g2,mj2 @ 0x56377e46] Could not find codec parameters for
stream 0 (Video: h264 (avc1 / 0x31637661), none, 1920x1080, 1159 kb/s):
unspecified pixel format


System environment:
-- system architecture: 64-bit
-- Linux distribution: Debian unstable
-- GPU: TONGA
-- Model: Asus Strix R9 285 2GB
-- Display connector: DVI
-- xf86-video-amdgpu: 1.4.0
-- xserver: 1.19.0
-- mesa: 17.3.3
-- drm: 2.4.89
-- kernel: 4.14
-- dri3

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

[Bug 104919] R9285 4.17-wip locks/vmfaults since drm/amdgpu: revert "drm/amdgpu: use AMDGPU_GEM_CREATE_VRAM_CLEARED for VM PD/PTs" v2

[bugzilla-daemon]

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

--- Comment #2 from Christian König  ---
At least I now knew that the PASID handling is working fine.

Does it work if you disable the new clear method? E.g. just add a "return 0;"
to the beginning of amdgpu_vm_clear_bo().

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

[Bug 104919] R9285 4.17-wip locks/vmfaults since drm/amdgpu: revert "drm/amdgpu: use AMDGPU_GEM_CREATE_VRAM_CLEARED for VM PD/PTs" v2

[bugzilla-daemon]

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

--- Comment #1 from Andy Furniss  ---
Created attachment 137138
  --> https://bugs.freedesktop.org/attachment.cgi?id=137138=edit
error logging from corruption/locks

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

[Bug 104919] R9285 4.17-wip locks/vmfaults since drm/amdgpu: revert "drm/amdgpu: use AMDGPU_GEM_CREATE_VRAM_CLEARED for VM PD/PTs" v2

[bugzilla-daemon]

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

Bug ID: 104919
   Summary: R9285 4.17-wip locks/vmfaults since drm/amdgpu: revert
"drm/amdgpu: use AMDGPU_GEM_CREATE_VRAM_CLEARED for VM
PD/PTs" v2
   Product: DRI
   Version: DRI git
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/AMDgpu
  Assignee: dri-devel@lists.freedesktop.org
  Reporter: adf.li...@gmail.com

last couple of agd5f 4.17-wips have locked with unreal tournament alpha for me
on R9 285.

Seems to be

first bad commit: [d712b817ceb9311cffad47867da26311c06a812b] drm/amdgpu: revert
"drm/amdgpu: use AMDGPU_GEM_CREATE_VRAM_CLEARED for VM PD/PTs" v2

Though it takes a while to lock, and sometimes only after restarting the game,
so slight chance of a false good.

This game requests slightly more than the 2 gig vram I have - maybe relevant if
others have more and can't reproduce.

Attached examples of logging retrieved after a lock. Usually sysrq will do,
once needed hard reset, first chunk got before a lock by quickly quitting the
game after seeing some new artifacts.

-- 
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: [Intel-gfx] i915 PSR test results and cursor lag

[Andy Lutomirski]

On Fri, Feb 2, 2018 at 7:18 PM, Andy Lutomirski  wrote:
> On Fri, Feb 2, 2018 at 1:24 AM, Andy Lutomirski  wrote:
>> On Thu, Feb 1, 2018 at 9:20 PM, Chris Wilson  
>> wrote:
>>> Quoting Andy Lutomirski (2018-02-01 21:04:30)
 I got this after a recent suspend/resume:

 Feb 01 09:44:34 laptop systemd-logind[2412]: Lid closed.
 Feb 01 09:44:34 laptop systemd-logind[2412]: device-enumerator: scan all 
 dirs
 Feb 01 09:44:34 laptop systemd-logind[2412]:   device-enumerator:
 scanning /sys/bus
 Feb 01 09:44:34 laptop systemd-logind[2412]:   device-enumerator:
 scanning /sys/class
 Feb 01 09:44:34 laptop systemd-logind[2412]: Failed to open
 configuration file '/etc/systemd/sleep.conf': No such file or
 directory
 Feb 01 09:44:34 laptop systemd-logind[2412]: Suspending...
 Feb 01 09:44:34 laptop systemd-logind[2412]: Sent message type=signal
 sender=n/a destination=n/a object=/org/freedesktop/login1
 interface=org.freedesktop.login1.Manager member=PrepareForSleep
 cookie=570 reply
 Feb 01 09:44:34 laptop systemd-logind[2412]: Got message
 type=method_call sender=:1.46 destination=:1.1
 object=/org/freedesktop/login1/session/_32
 interface=org.freedesktop.login1.Session member=ReleaseDevice
 Feb 01 09:44:34 laptop systemd-logind[2412]: Sent message type=signal
 sender=n/a destination=:1.46
 object=/org/freedesktop/login1/session/_32
 interface=org.freedesktop.login1.Session member=PauseDevice cookie
 Feb 01 09:44:34 laptop gnome-shell[2630]: Failed to apply DRM plane
 transform 0: Permission denied
 Feb 01 09:44:34 laptop gnome-shell[2630]: drmModeSetCursor2 failed
 with (Permission denied), drawing cursor with OpenGL from now on

 But I don't see the word "cursor" in my system logs before the first
 suspend.  What am I looking for?  This is Fedora 27 running a Gnome
 Wayland session, but it hasn't been reinstalled in some time, so it's
 possible that there are some weird settings sitting around.  But I did
 check and I have no weird i915 parameters.
>>>
>>> You are using gnome-shell as the display server. From that it appears to
>>> have started off with a HW cursor and switched to a SW cursor after
>>> suspend. Did you notice a change in behaviour? After rebooting or just
>>> restarting gnome-shell?
>>
>> I think it's less consistently bad after a reboot before suspending.
>>
>>>
 Also, are these things potentially related:

 [ 3067.702527] [drm:intel_pipe_update_start [i915]] *ERROR* Potential
 atomic update failure on pipe A
>>>
>>> They are just "missed the immediate vblank for the screen update"
>>> messages. Should not be related to PSR, but may cause jitter by delaying
>>> the odd screen update.
>>
>> I just got this one, and the timestamp is at least reasonably close to
>> a giant latency spike:
>>
>> [  288.799654] [drm:intel_pipe_update_end [i915]] *ERROR* Atomic
>> update failure on pipe A (start=31 end=32) time 15 us, min 1073, max
>> 1079, scanline start 1087, end 1088
>>
>>>
 As I'm typing this, I've seen a couple instances of what seems like a
 full *second* of cursor latency, but I've only gotten the potential
 atomic update failure once.

 And is there any straightforward tracing to do to distinguish between
 PSR exit latency and other potential sources of latency?
>>>
>>> It looks plausible that we could at least report how long it takes the
>>> registers to reflect the change in state (but we don't). The best source
>>> of information atm is /sys/kernel/debug/dri/0/i915_edp_psr_status.
>>
>> Hmm.
>>
>> I went and looked at the code, and I noticed what could be bugs or
>> could (more likely) be my confusion since I don't know this code at
>> all:
>>
>> intel_single_frame_update() does something inscrutable to me, but I
>> imagine it does something that causes the next page flip to get
>> noticed by the panel even with PSR on.  But how does the code that
>> calls it know that anything happened?  (Looking at the commit history,
>> maybe this is something special that's only needed on some platforms
>> but doesn't replace the normal PSR exit sequence.)
>>
>> Perhaps more interestingly, intel_psr_flush() does this:
>>
>> /* By definition flush = invalidate + flush */
>> if (frontbuffer_bits)
>> intel_psr_exit(dev_priv);
>>
>> if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits)
>> if (!work_busy(_priv->psr.work.work))
>> schedule_delayed_work(_priv->psr.work,
>>   msecs_to_jiffies(100));
>>
>> I'm guessing that the idea is that we're turning off PSR because we
>> want the panel to update and we expect that, in 100ms, the update will
>> have hit the panel and we'll have been idle long enough for it to make
>> sense to re-enter PSR.  IOW, the code wants PSR to be off for at least
>> 100ms 

Re: [Intel-gfx] i915 PSR test results and cursor lag

[Andy Lutomirski]

On Fri, Feb 2, 2018 at 1:24 AM, Andy Lutomirski  wrote:
> On Thu, Feb 1, 2018 at 9:20 PM, Chris Wilson  wrote:
>> Quoting Andy Lutomirski (2018-02-01 21:04:30)
>>> I got this after a recent suspend/resume:
>>>
>>> Feb 01 09:44:34 laptop systemd-logind[2412]: Lid closed.
>>> Feb 01 09:44:34 laptop systemd-logind[2412]: device-enumerator: scan all 
>>> dirs
>>> Feb 01 09:44:34 laptop systemd-logind[2412]:   device-enumerator:
>>> scanning /sys/bus
>>> Feb 01 09:44:34 laptop systemd-logind[2412]:   device-enumerator:
>>> scanning /sys/class
>>> Feb 01 09:44:34 laptop systemd-logind[2412]: Failed to open
>>> configuration file '/etc/systemd/sleep.conf': No such file or
>>> directory
>>> Feb 01 09:44:34 laptop systemd-logind[2412]: Suspending...
>>> Feb 01 09:44:34 laptop systemd-logind[2412]: Sent message type=signal
>>> sender=n/a destination=n/a object=/org/freedesktop/login1
>>> interface=org.freedesktop.login1.Manager member=PrepareForSleep
>>> cookie=570 reply
>>> Feb 01 09:44:34 laptop systemd-logind[2412]: Got message
>>> type=method_call sender=:1.46 destination=:1.1
>>> object=/org/freedesktop/login1/session/_32
>>> interface=org.freedesktop.login1.Session member=ReleaseDevice
>>> Feb 01 09:44:34 laptop systemd-logind[2412]: Sent message type=signal
>>> sender=n/a destination=:1.46
>>> object=/org/freedesktop/login1/session/_32
>>> interface=org.freedesktop.login1.Session member=PauseDevice cookie
>>> Feb 01 09:44:34 laptop gnome-shell[2630]: Failed to apply DRM plane
>>> transform 0: Permission denied
>>> Feb 01 09:44:34 laptop gnome-shell[2630]: drmModeSetCursor2 failed
>>> with (Permission denied), drawing cursor with OpenGL from now on
>>>
>>> But I don't see the word "cursor" in my system logs before the first
>>> suspend.  What am I looking for?  This is Fedora 27 running a Gnome
>>> Wayland session, but it hasn't been reinstalled in some time, so it's
>>> possible that there are some weird settings sitting around.  But I did
>>> check and I have no weird i915 parameters.
>>
>> You are using gnome-shell as the display server. From that it appears to
>> have started off with a HW cursor and switched to a SW cursor after
>> suspend. Did you notice a change in behaviour? After rebooting or just
>> restarting gnome-shell?
>
> I think it's less consistently bad after a reboot before suspending.
>
>>
>>> Also, are these things potentially related:
>>>
>>> [ 3067.702527] [drm:intel_pipe_update_start [i915]] *ERROR* Potential
>>> atomic update failure on pipe A
>>
>> They are just "missed the immediate vblank for the screen update"
>> messages. Should not be related to PSR, but may cause jitter by delaying
>> the odd screen update.
>
> I just got this one, and the timestamp is at least reasonably close to
> a giant latency spike:
>
> [  288.799654] [drm:intel_pipe_update_end [i915]] *ERROR* Atomic
> update failure on pipe A (start=31 end=32) time 15 us, min 1073, max
> 1079, scanline start 1087, end 1088
>
>>
>>> As I'm typing this, I've seen a couple instances of what seems like a
>>> full *second* of cursor latency, but I've only gotten the potential
>>> atomic update failure once.
>>>
>>> And is there any straightforward tracing to do to distinguish between
>>> PSR exit latency and other potential sources of latency?
>>
>> It looks plausible that we could at least report how long it takes the
>> registers to reflect the change in state (but we don't). The best source
>> of information atm is /sys/kernel/debug/dri/0/i915_edp_psr_status.
>
> Hmm.
>
> I went and looked at the code, and I noticed what could be bugs or
> could (more likely) be my confusion since I don't know this code at
> all:
>
> intel_single_frame_update() does something inscrutable to me, but I
> imagine it does something that causes the next page flip to get
> noticed by the panel even with PSR on.  But how does the code that
> calls it know that anything happened?  (Looking at the commit history,
> maybe this is something special that's only needed on some platforms
> but doesn't replace the normal PSR exit sequence.)
>
> Perhaps more interestingly, intel_psr_flush() does this:
>
> /* By definition flush = invalidate + flush */
> if (frontbuffer_bits)
> intel_psr_exit(dev_priv);
>
> if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits)
> if (!work_busy(_priv->psr.work.work))
> schedule_delayed_work(_priv->psr.work,
>   msecs_to_jiffies(100));
>
> I'm guessing that the idea is that we're turning off PSR because we
> want the panel to update and we expect that, in 100ms, the update will
> have hit the panel and we'll have been idle long enough for it to make
> sense to re-enter PSR.  IOW, the code wants PSR to be off for at least
> 100ms and then to turn back on.  But this code actually says "turn PSR
> back on in at *most* 100ms".  What happens if there are two screen
> updates 99ms apart?  The first one should 

[PATCH 2/5] drm/amdgpu: remove extra TT unpopulated check

[Christian König]

The subsystem chould check that, not the driver.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 95f990140f2a..648c449aaa79 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -997,9 +997,6 @@ static int amdgpu_ttm_tt_populate(struct ttm_tt *ttm,
struct amdgpu_ttm_tt *gtt = (void *)ttm;
bool slave = !!(ttm->page_flags & TTM_PAGE_FLAG_SG);
 
-   if (ttm->state != tt_unpopulated)
-   return 0;
-
if (gtt && gtt->userptr) {
ttm->sg = kzalloc(sizeof(struct sg_table), GFP_KERNEL);
if (!ttm->sg)
-- 
2.14.1

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

[PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

[Christian König]

This allows access to pages allocated through the driver with optional
IOMMU mapping.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 57 -
 1 file changed, 35 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 648c449aaa79..795ceaeb82d5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1929,38 +1929,51 @@ static const struct file_operations amdgpu_ttm_gtt_fops 
= {
 
 #endif
 
-static ssize_t amdgpu_iova_to_phys_read(struct file *f, char __user *buf,
-  size_t size, loff_t *pos)
+static ssize_t amdgpu_iomem_read(struct file *f, char __user *buf,
+size_t size, loff_t *pos)
 {
struct amdgpu_device *adev = file_inode(f)->i_private;
-   int r;
-   uint64_t phys;
struct iommu_domain *dom;
+   ssize_t result = 0;
+   int r;
 
-   // always return 8 bytes
-   if (size != 8)
-   return -EINVAL;
+   dom = iommu_get_domain_for_dev(adev->dev);
 
-   // only accept page addresses
-   if (*pos & 0xFFF)
-   return -EINVAL;
+   while (size) {
+   phys_addr_t addr = *pos & PAGE_MASK;
+   loff_t off = *pos & ~PAGE_MASK;
+   size_t bytes = PAGE_SIZE - off;
+   unsigned long pfn;
+   struct page *p;
+   void *ptr;
 
-   dom = iommu_get_domain_for_dev(adev->dev);
-   if (dom)
-   phys = iommu_iova_to_phys(dom, *pos);
-   else
-   phys = *pos;
+   addr = dom ? iommu_iova_to_phys(dom, addr) : addr;
 
-   r = copy_to_user(buf, , 8);
-   if (r)
-   return -EFAULT;
+   pfn = addr >> PAGE_SHIFT;
+   if (!pfn_valid(pfn))
+   return -EPERM;
+
+   p = pfn_to_page(pfn);
+   if (p->mapping != adev->mman.bdev.dev_mapping)
+   return -EPERM;
+
+   ptr = kmap(p);
+   r = copy_to_user(buf, ptr, bytes);
+   kunmap(p);
+   if (r)
+   return -EFAULT;
 
-   return 8;
+   size -= bytes;
+   *pos += bytes;
+   result += bytes;
+   }
+
+   return result;
 }
 
-static const struct file_operations amdgpu_ttm_iova_fops = {
+static const struct file_operations amdgpu_ttm_iomem_fops = {
.owner = THIS_MODULE,
-   .read = amdgpu_iova_to_phys_read,
+   .read = amdgpu_iomem_read,
.llseek = default_llseek
 };
 
@@ -1973,7 +1986,7 @@ static const struct {
 #ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
{ "amdgpu_gtt", _ttm_gtt_fops, TTM_PL_TT },
 #endif
-   { "amdgpu_iova", _ttm_iova_fops, TTM_PL_SYSTEM },
+   { "amdgpu_iomem", _ttm_iomem_fops, TTM_PL_SYSTEM },
 };
 
 #endif
-- 
2.14.1

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

[PATCH 1/5] drm/ttm: add ttm_tt_populate wrapper

[Christian König]

Stop calling the driver callback directly.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/ttm/ttm_bo_util.c | 12 +---
 drivers/gpu/drm/ttm/ttm_bo_vm.c   |  2 +-
 drivers/gpu/drm/ttm/ttm_tt.c  | 10 +-
 include/drm/ttm/ttm_bo_driver.h   |  9 +
 4 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 33ffe286f3a5..38da6903cae9 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -375,8 +375,8 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
/*
 * TTM might be null for moves within the same region.
 */
-   if (ttm && ttm->state == tt_unpopulated) {
-   ret = ttm->bdev->driver->ttm_tt_populate(ttm, ctx);
+   if (ttm) {
+   ret = ttm_tt_populate(ttm, ctx);
if (ret)
goto out1;
}
@@ -557,11 +557,9 @@ static int ttm_bo_kmap_ttm(struct ttm_buffer_object *bo,
 
BUG_ON(!ttm);
 
-   if (ttm->state == tt_unpopulated) {
-   ret = ttm->bdev->driver->ttm_tt_populate(ttm, );
-   if (ret)
-   return ret;
-   }
+   ret = ttm_tt_populate(ttm, );
+   if (ret)
+   return ret;
 
if (num_pages == 1 && (mem->placement & TTM_PL_FLAG_CACHED)) {
/*
diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 716e724ac710..610d6714042a 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -234,7 +234,7 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
cvma.vm_page_prot);
 
/* Allocate all page at once, most common usage */
-   if (ttm->bdev->driver->ttm_tt_populate(ttm, )) {
+   if (ttm_tt_populate(ttm, )) {
ret = VM_FAULT_OOM;
goto out_io_unlock;
}
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 95a77dab8cc9..39c44e301c72 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -276,7 +276,7 @@ int ttm_tt_bind(struct ttm_tt *ttm, struct ttm_mem_reg 
*bo_mem,
if (ttm->state == tt_bound)
return 0;
 
-   ret = ttm->bdev->driver->ttm_tt_populate(ttm, ctx);
+   ret = ttm_tt_populate(ttm, ctx);
if (ret)
return ret;
 
@@ -392,6 +392,14 @@ int ttm_tt_swapout(struct ttm_tt *ttm, struct file 
*persistent_swap_storage)
return ret;
 }
 
+int ttm_tt_populate(struct ttm_tt *ttm, struct ttm_operation_ctx *ctx)
+{
+   if (ttm->state != tt_unpopulated)
+   return 0;
+
+   return ttm->bdev->driver->ttm_tt_populate(ttm, ctx);
+}
+
 static void ttm_tt_clear_mapping(struct ttm_tt *ttm)
 {
pgoff_t i;
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 9b417eb2df20..2bac25a6cf90 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -700,6 +700,15 @@ int ttm_tt_swapin(struct ttm_tt *ttm);
 int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t placement);
 int ttm_tt_swapout(struct ttm_tt *ttm, struct file *persistent_swap_storage);
 
+/**
+ * ttm_tt_populate - allocate pages for a ttm
+ *
+ * @ttm: Pointer to the ttm_tt structure
+ *
+ * Calls the driver method to allocate pages for a ttm
+ */
+int ttm_tt_populate(struct ttm_tt *ttm, struct ttm_operation_ctx *ctx);
+
 /**
  * ttm_tt_unpopulate - free pages from a ttm
  *
-- 
2.14.1

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

[PATCH 4/5] drm/ttm: set page mapping during allocation

[Christian König]

To aid debugging set the page mapping during allocation instead of
during VM faults.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/ttm/ttm_bo_vm.c |  1 -
 drivers/gpu/drm/ttm/ttm_tt.c| 18 +-
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 610d6714042a..121f017ac7ca 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -257,7 +257,6 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
} else if (unlikely(!page)) {
break;
}
-   page->mapping = vma->vm_file->f_mapping;
page->index = drm_vma_node_start(>vma_node) +
page_offset;
pfn = page_to_pfn(page);
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 39c44e301c72..9fd7115a013a 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -392,12 +392,28 @@ int ttm_tt_swapout(struct ttm_tt *ttm, struct file 
*persistent_swap_storage)
return ret;
 }
 
+static void ttm_tt_add_mapping(struct ttm_tt *ttm)
+{
+   pgoff_t i;
+
+   if (ttm->page_flags & TTM_PAGE_FLAG_SG)
+   return;
+
+   for (i = 0; i < ttm->num_pages; ++i)
+   ttm->pages[i]->mapping = ttm->bdev->dev_mapping;
+}
+
 int ttm_tt_populate(struct ttm_tt *ttm, struct ttm_operation_ctx *ctx)
 {
+   int ret;
+
if (ttm->state != tt_unpopulated)
return 0;
 
-   return ttm->bdev->driver->ttm_tt_populate(ttm, ctx);
+   ret = ttm->bdev->driver->ttm_tt_populate(ttm, ctx);
+   if (!ret)
+   ttm_tt_add_mapping(ttm);
+   return ret;
 }
 
 static void ttm_tt_clear_mapping(struct ttm_tt *ttm)
-- 
2.14.1

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

[PATCH 3/5] drm/radeon: remove extra TT unpopulated check

[Christian König]

The subsystem chould check that, not the driver.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/radeon/radeon_ttm.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
b/drivers/gpu/drm/radeon/radeon_ttm.c
index a0a839bc39bf..42e3ee81a96e 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -728,9 +728,6 @@ static int radeon_ttm_tt_populate(struct ttm_tt *ttm,
struct radeon_device *rdev;
bool slave = !!(ttm->page_flags & TTM_PAGE_FLAG_SG);
 
-   if (ttm->state != tt_unpopulated)
-   return 0;
-
if (gtt && gtt->userptr) {
ttm->sg = kzalloc(sizeof(struct sg_table), GFP_KERNEL);
if (!ttm->sg)
-- 
2.14.1

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

Re: [PATCH] drm/msm/adreno/a5xx_debugfs: fix potential NULL pointer dereference

[Rob Clark]

On Fri, Feb 2, 2018 at 11:30 AM, Jordan Crouse  wrote:
> On Fri, Feb 02, 2018 at 06:32:23AM -0600, Gustavo A. R. Silva wrote:
>> _minor_ is being dereferenced before it is null checked, hence there
>> is a potential null pointer dereference. Fix this by moving the pointer
>> dereference after _minor_ has been null checked.
>>
>> Fixes: 024ad8df763f ("drm/msm: add a5xx specific debugfs")
>> Signed-off-by: Gustavo A. R. Silva 
>> ---
>>
>> I wonder if a better solution for this would be to WARN_ON in case _minor_
>> happens to be NULL and return -EINVAL, instead of just returning zero.
>>
>> Something like:
>>
>> struct drm_device *dev;
>>
>> if (WARN_ON(!minor)
>>   return -EINVAL;
>>
>> dev = minor->dev;
>>
>> What do you think?
>
> In my opinion everything in debugfs is optional. I'm not sure if it is even
> possible for dev->primary, dev->render or dev->control to be NULL from the DRM
> core but if so I think the failure should be silent.
>

Don't have code in front of me atm, but I think this is one of those
things you can hit both before and after we have the minor, depending
on whether filesystem and fw are present when the driver loads or
not.. so I don't think it should be a WARN_ON().

BR,
-R


> Jordan
>>
>>  drivers/gpu/drm/msm/adreno/a5xx_debugfs.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c 
>> b/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c
>> index 6b27941..059ec7d 100644
>> --- a/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c
>> +++ b/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c
>> @@ -159,13 +159,15 @@ DEFINE_SIMPLE_ATTRIBUTE(reset_fops, NULL, reset_set, 
>> "%llx\n");
>>
>>  int a5xx_debugfs_init(struct msm_gpu *gpu, struct drm_minor *minor)
>>  {
>> - struct drm_device *dev = minor->dev;
>> + struct drm_device *dev;
>>   struct dentry *ent;
>>   int ret;
>>
>>   if (!minor)
>>   return 0;
>>
>> + dev = minor->dev;
>> +
>>   ret = drm_debugfs_create_files(a5xx_debugfs_list,
>>   ARRAY_SIZE(a5xx_debugfs_list),
>>   minor->debugfs_root, minor);
>> --
>> 2.7.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence v3

[Jani Nikula]

On Fri, 02 Feb 2018, Ville Syrjälä  wrote:
> On Mon, Jan 29, 2018 at 03:47:35PM +0100, Hans de Goede wrote:
>> So far models of the Dell Venue 8 Pro, with a panel with MIPI panel
>> index = 3, one of which has been kindly provided to me by Jan Brummer,
>> where not working with the i915 driver, giving a black screen on the
>> first modeset.
>> 
>> The problem with at least these Dells is that their VBT defines a MIPI
>> ASSERT sequence, but not a DEASSERT sequence. Instead they DEASSERT the
>> reset in their INIT_OTP sequence, but the deassert must be done before
>> calling intel_dsi_device_ready(), so that is too late.
>> 
>> Simply doing the INIT_OTP sequence earlier is not enough to fix this,
>> because the INIT_OTP sequence also sends various MIPI packets to the
>> panel, which can only happen after calling intel_dsi_device_ready().
>> 
>> This commit fixes this by splitting the INIT_OTP sequence into everything
>> before the first DSI packet and everything else, including the first DSI
>> packet. The first part (everything before the first DSI packet) is then
>> used as deassert sequence.
>> 
>> Changed in v2:
>> -Split the init OTP sequence into a deassert reset and the actual init
>>  OTP sequence, instead of calling it earlier and then having the first
>>  mipi_exec_send_packet() call call intel_dsi_device_ready().
>> 
>> Changes in v3:
>> -Move the whole shebang to intel_bios.c
>> 
>> BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=82880
>
> Bugzilla:
>
>> Related: https://bugs.freedesktop.org/show_bug.cgi?id=101205
>
> References:
>
>> Cc: Jan-Michael Brummer 
>> Reported-by: Jan-Michael Brummer 
>> Tested-by: Hans de Goede 
>> Signed-off-by: Hans de Goede 
>
> This one seems good to me, and Jani hasn't complained about anything so
> Reviewed-by: Ville Syrjälä 

Thanks for hiding this in one place.

Acked-by: Jani Nikula 


>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h   |  1 +
>>  drivers/gpu/drm/i915/intel_bios.c | 83 
>> +++
>>  2 files changed, 84 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h 
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index 081190da0818..1f346266956b 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1349,6 +1349,7 @@ struct intel_vbt_data {
>>  u32 size;
>>  u8 *data;
>>  const u8 *sequence[MIPI_SEQ_MAX];
>> +u8 *deassert_seq; /* Used by fixup_mipi_sequences() */
>>  } dsi;
>>  
>>  int crt_ddc_pin;
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c 
>> b/drivers/gpu/drm/i915/intel_bios.c
>> index 64a0d55df28e..cca620f8deb6 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -947,6 +947,86 @@ static int goto_next_sequence_v3(const u8 *data, int 
>> index, int total)
>>  return 0;
>>  }
>>  
>> +/*
>> + * Get len of pre-fixed deassert fragment from a v1 init OTP sequence,
>> + * skip all delay + gpio operands and stop at the first DSI packet op.
>> + */
>> +static int get_init_otp_deassert_fragment_len(struct drm_i915_private 
>> *dev_priv)
>> +{
>> +const u8 *data = dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP];
>> +int index, len;
>> +
>> +if (WARN_ON(!data || dev_priv->vbt.dsi.seq_version != 1))
>> +return 0;
>> +
>> +/* index = 1 to skip sequence byte */
>> +for (index = 1; data[index] != MIPI_SEQ_ELEM_END; index += len) {
>> +switch (data[index]) {
>> +case MIPI_SEQ_ELEM_SEND_PKT:
>> +return index == 1 ? 0 : index;
>> +case MIPI_SEQ_ELEM_DELAY:
>> +len = 5; /* 1 byte for operand + uint32 */
>> +break;
>> +case MIPI_SEQ_ELEM_GPIO:
>> +len = 3; /* 1 byte for op, 1 for gpio_nr, 1 for value */
>> +break;
>> +default:
>> +return 0;
>> +}
>> +}
>> +
>> +return 0;
>> +}
>> +
>> +/*
>> + * Some v1 VBT MIPI sequences do the deassert in the init OTP sequence.
>> + * The deassert must be done before calling intel_dsi_device_ready, so for
>> + * these devices we split the init OTP sequence into a deassert sequence and
>> + * the actual init OTP part.
>> + */
>> +static void fixup_mipi_sequences(struct drm_i915_private *dev_priv)
>> +{
>> +u8 *init_otp;
>> +int len;
>> +
>> +/* Limit this to VLV for now. */
>> +if (!IS_VALLEYVIEW(dev_priv))
>> +return;
>> +
>> +/* Limit this to v1 vid-mode sequences */
>> +if (dev_priv->vbt.dsi.config->is_cmd_mode ||
>> +dev_priv->vbt.dsi.seq_version != 1)
>> +return;
>> +
>> +/* Only do this if there are otp and assert seqs and no deassert seq */
>> +if 

Re: [AMDGPU] module does not link without DEBUG_FS configuration option

[Michel Dänzer]

On 2018-02-02 05:29 PM, sylvain.bertr...@gmail.com wrote:
> Hi,
> 
> I did not look into details, but on amd-staging-drm-next
> (495e9e174feaec6e5aeb6f8224f0d3bda4c96114), linking the amdgpu module fails if
> DEBUG_FS is not enabled (probably some weird things happen in the code with
> the CONFIG_DEBUG_FS macro).
> 
> Saw passing something about an amd-gfx mailing list. Is this list still valid
> for amdgpu related work?

Yes, moving there.


-- 
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: [PATCH] drm/msm/adreno/a5xx_debugfs: fix potential NULL pointer dereference

[Jordan Crouse]

On Fri, Feb 02, 2018 at 06:32:23AM -0600, Gustavo A. R. Silva wrote:
> _minor_ is being dereferenced before it is null checked, hence there
> is a potential null pointer dereference. Fix this by moving the pointer
> dereference after _minor_ has been null checked.
> 
> Fixes: 024ad8df763f ("drm/msm: add a5xx specific debugfs")
> Signed-off-by: Gustavo A. R. Silva 
> ---
> 
> I wonder if a better solution for this would be to WARN_ON in case _minor_
> happens to be NULL and return -EINVAL, instead of just returning zero.
> 
> Something like:
> 
> struct drm_device *dev;
> 
> if (WARN_ON(!minor)
>   return -EINVAL;
> 
> dev = minor->dev;
> 
> What do you think?

In my opinion everything in debugfs is optional. I'm not sure if it is even
possible for dev->primary, dev->render or dev->control to be NULL from the DRM
core but if so I think the failure should be silent.

Jordan
> 
>  drivers/gpu/drm/msm/adreno/a5xx_debugfs.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c 
> b/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c
> index 6b27941..059ec7d 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c
> @@ -159,13 +159,15 @@ DEFINE_SIMPLE_ATTRIBUTE(reset_fops, NULL, reset_set, 
> "%llx\n");
>  
>  int a5xx_debugfs_init(struct msm_gpu *gpu, struct drm_minor *minor)
>  {
> - struct drm_device *dev = minor->dev;
> + struct drm_device *dev;
>   struct dentry *ent;
>   int ret;
>  
>   if (!minor)
>   return 0;
>  
> + dev = minor->dev;
> +
>   ret = drm_debugfs_create_files(a5xx_debugfs_list,
>   ARRAY_SIZE(a5xx_debugfs_list),
>   minor->debugfs_root, minor);
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[AMDGPU] module does not link without DEBUG_FS configuration option

[sylvain . bertrand]

Hi,

I did not look into details, but on amd-staging-drm-next
(495e9e174feaec6e5aeb6f8224f0d3bda4c96114), linking the amdgpu module fails if
DEBUG_FS is not enabled (probably some weird things happen in the code with
the CONFIG_DEBUG_FS macro).

Saw passing something about an amd-gfx mailing list. Is this list still valid
for amdgpu related work?

regards,

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

Re: [PATCH 2/2] drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence v3

[Ville Syrjälä]

On Mon, Jan 29, 2018 at 03:47:35PM +0100, Hans de Goede wrote:
> So far models of the Dell Venue 8 Pro, with a panel with MIPI panel
> index = 3, one of which has been kindly provided to me by Jan Brummer,
> where not working with the i915 driver, giving a black screen on the
> first modeset.
> 
> The problem with at least these Dells is that their VBT defines a MIPI
> ASSERT sequence, but not a DEASSERT sequence. Instead they DEASSERT the
> reset in their INIT_OTP sequence, but the deassert must be done before
> calling intel_dsi_device_ready(), so that is too late.
> 
> Simply doing the INIT_OTP sequence earlier is not enough to fix this,
> because the INIT_OTP sequence also sends various MIPI packets to the
> panel, which can only happen after calling intel_dsi_device_ready().
> 
> This commit fixes this by splitting the INIT_OTP sequence into everything
> before the first DSI packet and everything else, including the first DSI
> packet. The first part (everything before the first DSI packet) is then
> used as deassert sequence.
> 
> Changed in v2:
> -Split the init OTP sequence into a deassert reset and the actual init
>  OTP sequence, instead of calling it earlier and then having the first
>  mipi_exec_send_packet() call call intel_dsi_device_ready().
> 
> Changes in v3:
> -Move the whole shebang to intel_bios.c
> 
> BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=82880

Bugzilla:

> Related: https://bugs.freedesktop.org/show_bug.cgi?id=101205

References:

> Cc: Jan-Michael Brummer 
> Reported-by: Jan-Michael Brummer 
> Tested-by: Hans de Goede 
> Signed-off-by: Hans de Goede 

This one seems good to me, and Jani hasn't complained about anything so
Reviewed-by: Ville Syrjälä 

> ---
>  drivers/gpu/drm/i915/i915_drv.h   |  1 +
>  drivers/gpu/drm/i915/intel_bios.c | 83 
> +++
>  2 files changed, 84 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 081190da0818..1f346266956b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1349,6 +1349,7 @@ struct intel_vbt_data {
>   u32 size;
>   u8 *data;
>   const u8 *sequence[MIPI_SEQ_MAX];
> + u8 *deassert_seq; /* Used by fixup_mipi_sequences() */
>   } dsi;
>  
>   int crt_ddc_pin;
> diff --git a/drivers/gpu/drm/i915/intel_bios.c 
> b/drivers/gpu/drm/i915/intel_bios.c
> index 64a0d55df28e..cca620f8deb6 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -947,6 +947,86 @@ static int goto_next_sequence_v3(const u8 *data, int 
> index, int total)
>   return 0;
>  }
>  
> +/*
> + * Get len of pre-fixed deassert fragment from a v1 init OTP sequence,
> + * skip all delay + gpio operands and stop at the first DSI packet op.
> + */
> +static int get_init_otp_deassert_fragment_len(struct drm_i915_private 
> *dev_priv)
> +{
> + const u8 *data = dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP];
> + int index, len;
> +
> + if (WARN_ON(!data || dev_priv->vbt.dsi.seq_version != 1))
> + return 0;
> +
> + /* index = 1 to skip sequence byte */
> + for (index = 1; data[index] != MIPI_SEQ_ELEM_END; index += len) {
> + switch (data[index]) {
> + case MIPI_SEQ_ELEM_SEND_PKT:
> + return index == 1 ? 0 : index;
> + case MIPI_SEQ_ELEM_DELAY:
> + len = 5; /* 1 byte for operand + uint32 */
> + break;
> + case MIPI_SEQ_ELEM_GPIO:
> + len = 3; /* 1 byte for op, 1 for gpio_nr, 1 for value */
> + break;
> + default:
> + return 0;
> + }
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * Some v1 VBT MIPI sequences do the deassert in the init OTP sequence.
> + * The deassert must be done before calling intel_dsi_device_ready, so for
> + * these devices we split the init OTP sequence into a deassert sequence and
> + * the actual init OTP part.
> + */
> +static void fixup_mipi_sequences(struct drm_i915_private *dev_priv)
> +{
> + u8 *init_otp;
> + int len;
> +
> + /* Limit this to VLV for now. */
> + if (!IS_VALLEYVIEW(dev_priv))
> + return;
> +
> + /* Limit this to v1 vid-mode sequences */
> + if (dev_priv->vbt.dsi.config->is_cmd_mode ||
> + dev_priv->vbt.dsi.seq_version != 1)
> + return;
> +
> + /* Only do this if there are otp and assert seqs and no deassert seq */
> + if (!dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP] ||
> + !dev_priv->vbt.dsi.sequence[MIPI_SEQ_ASSERT_RESET] ||
> + dev_priv->vbt.dsi.sequence[MIPI_SEQ_DEASSERT_RESET])
> + return;
> +
> + /* The deassert-sequence ends at the first DSI packet */
> + len 

[PATCH v2] drm/ast: Add option to initialize palette on driver load

[Timothy Pearson]

Non-x86 systems, such as OpenPOWER and ARM machines, do not execute the ASPEED-
provided option ROM on system start.  As a result, the VGA palette registers
remain uninitialized, leading to odd colors and generally hard to read output
on the VGA port.

Add a new module option, ast_resetpalette, to enable loading a linear greyscale
palette into the VGA RAMDAC.  This option is intended for use by the first Linux
kernel to load after initial power on, such as the skiroot kernel on OpenPOWER
systems.
---
 drivers/gpu/drm/ast/ast_drv.c  |  4 
 drivers/gpu/drm/ast/ast_drv.h  | 14 ++
 drivers/gpu/drm/ast/ast_main.c |  8 
 drivers/gpu/drm/ast/ast_mode.c | 14 --
 4 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
index 69dab82a3771..8124eaa92ed3 100644
--- a/drivers/gpu/drm/ast/ast_drv.c
+++ b/drivers/gpu/drm/ast/ast_drv.c
@@ -1,5 +1,6 @@
 /*
  * Copyright 2012 Red Hat Inc.
+ * Copyright 2018 Raptor Engineering, LLC.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the
@@ -34,9 +35,12 @@
 #include "ast_drv.h"
 
 int ast_modeset = -1;
+int ast_resetpalette = -1;
 
 MODULE_PARM_DESC(modeset, "Disable/Enable modesetting");
+MODULE_PARM_DESC(resetpalette, "Disable/Enable palette reset on load");
 module_param_named(modeset, ast_modeset, int, 0400);
+module_param_named(resetpalette, ast_resetpalette, int, 0400);
 
 #define PCI_VENDOR_ASPEED 0x1a03
 
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index e6c4cd3dc50e..5e834e466b65 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -383,6 +383,20 @@ static inline int ast_bo_reserve(struct ast_bo *bo, bool 
no_wait)
return 0;
 }
 
+static inline void ast_load_palette_index(struct ast_private *ast,
+u8 index, u8 red, u8 green,
+u8 blue)
+{
+   ast_io_write8(ast, AST_IO_DAC_INDEX_WRITE, index);
+   ast_io_read8(ast, AST_IO_SEQ_PORT);
+   ast_io_write8(ast, AST_IO_DAC_DATA, red);
+   ast_io_read8(ast, AST_IO_SEQ_PORT);
+   ast_io_write8(ast, AST_IO_DAC_DATA, green);
+   ast_io_read8(ast, AST_IO_SEQ_PORT);
+   ast_io_write8(ast, AST_IO_DAC_DATA, blue);
+   ast_io_read8(ast, AST_IO_SEQ_PORT);
+}
+
 static inline void ast_bo_unreserve(struct ast_bo *bo)
 {
ttm_bo_unreserve(>bo);
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index dac355812adc..f13329b9a14d 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -1,5 +1,6 @@
 /*
  * Copyright 2012 Red Hat Inc.
+ * Copyright 2018 Raptor Engineering, LLC.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the
@@ -32,6 +33,8 @@
 #include 
 #include 
 
+extern int ast_resetpalette;
+
 void ast_set_index_reg_mask(struct ast_private *ast,
uint32_t base, uint8_t index,
uint8_t mask, uint8_t val)
@@ -483,6 +486,7 @@ int ast_driver_load(struct drm_device *dev, unsigned long 
flags)
struct ast_private *ast;
bool need_post;
int ret = 0;
+   int index = 0;
 
ast = kzalloc(sizeof(struct ast_private), GFP_KERNEL);
if (!ast)
@@ -565,6 +569,10 @@ int ast_driver_load(struct drm_device *dev, unsigned long 
flags)
if (ret)
goto out_free;
 
+   if (ast_resetpalette == 1)
+   for (index = 0x00; index < 0x100; index++)
+   ast_load_palette_index(ast, index, index, index, index);
+
return 0;
 out_free:
kfree(ast);
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 9555a3542022..9afc4d53bd60 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -46,20 +46,6 @@ static int ast_cursor_set(struct drm_crtc *crtc,
 static int ast_cursor_move(struct drm_crtc *crtc,
   int x, int y);
 
-static inline void ast_load_palette_index(struct ast_private *ast,
-u8 index, u8 red, u8 green,
-u8 blue)
-{
-   ast_io_write8(ast, AST_IO_DAC_INDEX_WRITE, index);
-   ast_io_read8(ast, AST_IO_SEQ_PORT);
-   ast_io_write8(ast, AST_IO_DAC_DATA, red);
-   ast_io_read8(ast, AST_IO_SEQ_PORT);
-   ast_io_write8(ast, AST_IO_DAC_DATA, green);
-   ast_io_read8(ast, AST_IO_SEQ_PORT);
-   ast_io_write8(ast, AST_IO_DAC_DATA, blue);
-   ast_io_read8(ast, AST_IO_SEQ_PORT);
-}
-
 static void ast_crtc_load_lut(struct drm_crtc *crtc)
 {
struct ast_private *ast = crtc->dev->dev_private;
-- 
2.15.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org

Re: clang warning: implicit conversion in intel_ddi.c:1481

[Lukas Bulwahn]

On Fri, 2 Feb 2018, Jani Nikula wrote:

> Being brutally honest, please write shorter reports and shorter emails
> to the lists.
> 
> The static analysis reports are welcome, but only when 1) we didn't
> already fix it in linux-next, or 2) it reveals an actual bug, not just a
> warning, warranting a backport.

That will be our policy.

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

Re: [PATCH v6 4/6] iommu/arm-smmu: Add the device_link between masters and smmu

[Sricharan R]

Hi Robin,

On 2/2/2018 5:01 PM, Robin Murphy wrote:
> On 02/02/18 05:40, Sricharan R wrote:
>> Hi Robin/Vivek,
>>
>> On 2/1/2018 2:23 PM, Vivek Gautam wrote:
>>> Hi,
>>>
>>>
>>> On 1/31/2018 6:39 PM, Robin Murphy wrote:
 On 19/01/18 11:43, Vivek Gautam wrote:
> From: Sricharan R 
>
> Finally add the device link between the master device and
> smmu, so that the smmu gets runtime enabled/disabled only when the
> master needs it. This is done from add_device callback which gets
> called once when the master is added to the smmu.

 Don't we need to balance this with a device_link_del() in .remove_device 
 (like exynos-iommu does)?
>>>
>>> Right. Will add device_link_del() call. Thanks for pointing out.
>>
>>   The reason for not adding device_link_del from .remove_device was, the 
>> core device_del
>>   which calls the .remove_device from notifier, calls device_links_purge 
>> before that.
>>   That does the same thing as device_link_del. So by the time .remove_device 
>> is called,
>>   device_links for that device is already cleaned up. Vivek, you may want to 
>> check once that
>>   calling device_link_del from .remove_device has no effect, just to confirm 
>> once more.
> 
> There is at least one path in which .remove_device is not called via the 
> notifier from device_del(), which is in the cleanup path of iommu_bus_init(). 
> AFAICS any links created by .add_device during that process would be left 
> dangling, because the device(s) would be live but otherwise disassociated 
> from the IOMMU afterwards.
> 
> From a maintenance perspective it's easier to have the call in its logical 
> place even if it does nothing 99% of the time; that way we shouldn't have to 
> keep an eye out for subtle changes in the power management code or driver 
> core that might invalidate the device_del() reasoning above, and the power 
> management guys shouldn't have to comprehend the internals of the IOMMU API 
> to make sense of the unbalanced call if they ever want to change their API.

 Ha, for a moment was thinking that with probe deferral add/remove_iommu_group 
in iommu_bus_init is dummy.
 But that may not be true for all Archs.
 Surely agree for the maintainability reason as well. Thanks.

Regards,
 Sricharan

-- 
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of 
Code Aurora Forum, hosted by The Linux Foundation
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: clang warning: implicit conversion in intel_ddi.c:1481

[Ruben Safir]

> 
> What is the goal of these types of emails?
> 

even more so on this mailing list.  It almost feels like guerilla
advertising for Clang.


> thanks,
> 
> greg k-h
> 
> ___
> Kernelnewbies mailing list
> kernelnewb...@kernelnewbies.org
> https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

-- 
So many immigrant groups have swept through our town
that Brooklyn, like Atlantis, reaches mythological
proportions in the mind of the world - RI Safir 1998
http://www.mrbrklyn.com 

DRM is THEFT - We are the STAKEHOLDERS - RI Safir 2002
http://www.nylxs.com - Leadership Development in Free Software
http://www2.mrbrklyn.com/resources - Unpublished Archive 
http://www.coinhangout.com - coins!
http://www.brooklyn-living.com 

Being so tracked is for FARM ANIMALS and and extermination camps, 
but incompatible with living as a free human being. -RI Safir 2013

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

[PATCH v2] drm/ast: Add option to initialize palette on driver load

[Timothy Pearson]

Non-x86 systems, such as OpenPOWER and ARM machines, do not execute the ASPEED-
provided option ROM on system start.  As a result, the VGA palette registers
remain uninitialized, leading to odd colors and generally hard to read output
on the VGA port.

Add a new module option, ast_resetpalette, to enable loading a linear greyscale
palette into the VGA RAMDAC.  This option is intended for use by the first Linux
kernel to load after initial power on, such as the skiroot kernel on OpenPOWER
systems.

Signed-off-by: Timothy Pearson 
---
 drivers/gpu/drm/ast/ast_drv.c  |  4 
 drivers/gpu/drm/ast/ast_drv.h  | 14 ++
 drivers/gpu/drm/ast/ast_main.c |  8 
 drivers/gpu/drm/ast/ast_mode.c | 14 --
 4 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
index 69dab82a3771..8124eaa92ed3 100644
--- a/drivers/gpu/drm/ast/ast_drv.c
+++ b/drivers/gpu/drm/ast/ast_drv.c
@@ -1,5 +1,6 @@
 /*
  * Copyright 2012 Red Hat Inc.
+ * Copyright 2018 Raptor Engineering, LLC.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the
@@ -34,9 +35,12 @@
 #include "ast_drv.h"
 
 int ast_modeset = -1;
+int ast_resetpalette = -1;
 
 MODULE_PARM_DESC(modeset, "Disable/Enable modesetting");
+MODULE_PARM_DESC(resetpalette, "Disable/Enable palette reset on load");
 module_param_named(modeset, ast_modeset, int, 0400);
+module_param_named(resetpalette, ast_resetpalette, int, 0400);
 
 #define PCI_VENDOR_ASPEED 0x1a03
 
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index e6c4cd3dc50e..5e834e466b65 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -383,6 +383,20 @@ static inline int ast_bo_reserve(struct ast_bo *bo, bool 
no_wait)
return 0;
 }
 
+static inline void ast_load_palette_index(struct ast_private *ast,
+u8 index, u8 red, u8 green,
+u8 blue)
+{
+   ast_io_write8(ast, AST_IO_DAC_INDEX_WRITE, index);
+   ast_io_read8(ast, AST_IO_SEQ_PORT);
+   ast_io_write8(ast, AST_IO_DAC_DATA, red);
+   ast_io_read8(ast, AST_IO_SEQ_PORT);
+   ast_io_write8(ast, AST_IO_DAC_DATA, green);
+   ast_io_read8(ast, AST_IO_SEQ_PORT);
+   ast_io_write8(ast, AST_IO_DAC_DATA, blue);
+   ast_io_read8(ast, AST_IO_SEQ_PORT);
+}
+
 static inline void ast_bo_unreserve(struct ast_bo *bo)
 {
ttm_bo_unreserve(>bo);
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index dac355812adc..f13329b9a14d 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -1,5 +1,6 @@
 /*
  * Copyright 2012 Red Hat Inc.
+ * Copyright 2018 Raptor Engineering, LLC.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the
@@ -32,6 +33,8 @@
 #include 
 #include 
 
+extern int ast_resetpalette;
+
 void ast_set_index_reg_mask(struct ast_private *ast,
uint32_t base, uint8_t index,
uint8_t mask, uint8_t val)
@@ -483,6 +486,7 @@ int ast_driver_load(struct drm_device *dev, unsigned long 
flags)
struct ast_private *ast;
bool need_post;
int ret = 0;
+   int index = 0;
 
ast = kzalloc(sizeof(struct ast_private), GFP_KERNEL);
if (!ast)
@@ -565,6 +569,10 @@ int ast_driver_load(struct drm_device *dev, unsigned long 
flags)
if (ret)
goto out_free;
 
+   if (ast_resetpalette == 1)
+   for (index = 0x00; index < 0x100; index++)
+   ast_load_palette_index(ast, index, index, index, index);
+
return 0;
 out_free:
kfree(ast);
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 9555a3542022..9afc4d53bd60 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -46,20 +46,6 @@ static int ast_cursor_set(struct drm_crtc *crtc,
 static int ast_cursor_move(struct drm_crtc *crtc,
   int x, int y);
 
-static inline void ast_load_palette_index(struct ast_private *ast,
-u8 index, u8 red, u8 green,
-u8 blue)
-{
-   ast_io_write8(ast, AST_IO_DAC_INDEX_WRITE, index);
-   ast_io_read8(ast, AST_IO_SEQ_PORT);
-   ast_io_write8(ast, AST_IO_DAC_DATA, red);
-   ast_io_read8(ast, AST_IO_SEQ_PORT);
-   ast_io_write8(ast, AST_IO_DAC_DATA, green);
-   ast_io_read8(ast, AST_IO_SEQ_PORT);
-   ast_io_write8(ast, AST_IO_DAC_DATA, blue);
-   ast_io_read8(ast, AST_IO_SEQ_PORT);
-}
-
 static void ast_crtc_load_lut(struct drm_crtc *crtc)
 {
struct ast_private *ast = crtc->dev->dev_private;
-- 
2.15.1

[PATCH] drm/ast: Add option to initialize palette on driver load

[Timothy Pearson]

Non-x86, such as OpenPOWER and ARM machines, do not execute the ASPEED-provided
option ROM on system start.  As a result, the VGA palette registers remain
uninitialized, leading to odd colors and generally hard to read output on the
VGA port.

Add a new module option, ast_resetpalette, to enable loading a linear greyscale
palette into the VGA RAMDAC.  This option is intended for use by the first Linux
kernel to load after initial power on, such as the skiroot kernel on OpenPOWER
systems.

Signed-off-by: Timothy Pearson 
---
 drivers/gpu/drm/ast/ast_drv.c  |  4 
 drivers/gpu/drm/ast/ast_drv.h  | 14 ++
 drivers/gpu/drm/ast/ast_main.c |  8 
 drivers/gpu/drm/ast/ast_mode.c | 14 --
 4 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
index 69dab82a3771..8124eaa92ed3 100644
--- a/drivers/gpu/drm/ast/ast_drv.c
+++ b/drivers/gpu/drm/ast/ast_drv.c
@@ -1,5 +1,6 @@
 /*
  * Copyright 2012 Red Hat Inc.
+ * Copyright 2018 Raptor Engineering, LLC.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the
@@ -34,9 +35,12 @@
 #include "ast_drv.h"
 
 int ast_modeset = -1;
+int ast_resetpalette = -1;
 
 MODULE_PARM_DESC(modeset, "Disable/Enable modesetting");
+MODULE_PARM_DESC(resetpalette, "Disable/Enable palette reset on load");
 module_param_named(modeset, ast_modeset, int, 0400);
+module_param_named(resetpalette, ast_resetpalette, int, 0400);
 
 #define PCI_VENDOR_ASPEED 0x1a03
 
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index e6c4cd3dc50e..5e834e466b65 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -383,6 +383,20 @@ static inline int ast_bo_reserve(struct ast_bo *bo, bool 
no_wait)
return 0;
 }
 
+static inline void ast_load_palette_index(struct ast_private *ast,
+u8 index, u8 red, u8 green,
+u8 blue)
+{
+   ast_io_write8(ast, AST_IO_DAC_INDEX_WRITE, index);
+   ast_io_read8(ast, AST_IO_SEQ_PORT);
+   ast_io_write8(ast, AST_IO_DAC_DATA, red);
+   ast_io_read8(ast, AST_IO_SEQ_PORT);
+   ast_io_write8(ast, AST_IO_DAC_DATA, green);
+   ast_io_read8(ast, AST_IO_SEQ_PORT);
+   ast_io_write8(ast, AST_IO_DAC_DATA, blue);
+   ast_io_read8(ast, AST_IO_SEQ_PORT);
+}
+
 static inline void ast_bo_unreserve(struct ast_bo *bo)
 {
ttm_bo_unreserve(>bo);
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index dac355812adc..c15c55f69e38 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -1,5 +1,6 @@
 /*
  * Copyright 2012 Red Hat Inc.
+ * Copyright 2018 Raptor Engineering, LLC.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the
@@ -32,6 +33,8 @@
 #include 
 #include 
 
+extern int ast_resetpalette;
+
 void ast_set_index_reg_mask(struct ast_private *ast,
uint32_t base, uint8_t index,
uint8_t mask, uint8_t val)
@@ -483,6 +486,7 @@ int ast_driver_load(struct drm_device *dev, unsigned long 
flags)
struct ast_private *ast;
bool need_post;
int ret = 0;
+   int index = 0;
 
ast = kzalloc(sizeof(struct ast_private), GFP_KERNEL);
if (!ast)
@@ -516,6 +520,10 @@ int ast_driver_load(struct drm_device *dev, unsigned long 
flags)
}
}
 
+   if (ast_resetpalette == 1)
+   for (index = 0x00; index < 0x100; index++)
+   ast_load_palette_index(ast, index, index, index, index);
+
ast_detect_chip(dev, _post);
 
if (need_post)
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 9555a3542022..9afc4d53bd60 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -46,20 +46,6 @@ static int ast_cursor_set(struct drm_crtc *crtc,
 static int ast_cursor_move(struct drm_crtc *crtc,
   int x, int y);
 
-static inline void ast_load_palette_index(struct ast_private *ast,
-u8 index, u8 red, u8 green,
-u8 blue)
-{
-   ast_io_write8(ast, AST_IO_DAC_INDEX_WRITE, index);
-   ast_io_read8(ast, AST_IO_SEQ_PORT);
-   ast_io_write8(ast, AST_IO_DAC_DATA, red);
-   ast_io_read8(ast, AST_IO_SEQ_PORT);
-   ast_io_write8(ast, AST_IO_DAC_DATA, green);
-   ast_io_read8(ast, AST_IO_SEQ_PORT);
-   ast_io_write8(ast, AST_IO_DAC_DATA, blue);
-   ast_io_read8(ast, AST_IO_SEQ_PORT);
-}
-
 static void ast_crtc_load_lut(struct drm_crtc *crtc)
 {
struct ast_private *ast = crtc->dev->dev_private;
-- 
2.15.1

___
dri-devel 

[PATCH] drm/msm/adreno/a5xx_debugfs: fix potential NULL pointer dereference

[Gustavo A. R. Silva]

_minor_ is being dereferenced before it is null checked, hence there
is a potential null pointer dereference. Fix this by moving the pointer
dereference after _minor_ has been null checked.

Fixes: 024ad8df763f ("drm/msm: add a5xx specific debugfs")
Signed-off-by: Gustavo A. R. Silva 
---

I wonder if a better solution for this would be to WARN_ON in case _minor_
happens to be NULL and return -EINVAL, instead of just returning zero.

Something like:

struct drm_device *dev;

if (WARN_ON(!minor)
return -EINVAL;

dev = minor->dev;

What do you think?

Thanks


 drivers/gpu/drm/msm/adreno/a5xx_debugfs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c 
b/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c
index 6b27941..059ec7d 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c
@@ -159,13 +159,15 @@ DEFINE_SIMPLE_ATTRIBUTE(reset_fops, NULL, reset_set, 
"%llx\n");
 
 int a5xx_debugfs_init(struct msm_gpu *gpu, struct drm_minor *minor)
 {
-   struct drm_device *dev = minor->dev;
+   struct drm_device *dev;
struct dentry *ent;
int ret;
 
if (!minor)
return 0;
 
+   dev = minor->dev;
+
ret = drm_debugfs_create_files(a5xx_debugfs_list,
ARRAY_SIZE(a5xx_debugfs_list),
minor->debugfs_root, minor);
-- 
2.7.4

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

[PATCH] drm/msm/dsi: Fix potential NULL pointer dereference in msm_dsi_modeset_init

[Gustavo A. R. Silva]

_dev_ is being dereferenced before it is null checked, hence there
is a potential null pointer dereference.

Fix this by moving the pointer dereference after _dev_ has been
null checked.

Fixes: d4e7f38d70ef ("drm/msm/dsi: check msm_dsi and dsi pointers before use")
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/gpu/drm/msm/dsi/dsi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
index ee7e090..b744bcc 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.c
+++ b/drivers/gpu/drm/msm/dsi/dsi.c
@@ -192,13 +192,14 @@ void __exit msm_dsi_unregister(void)
 int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev,
 struct drm_encoder *encoder)
 {
-   struct msm_drm_private *priv = dev->dev_private;
+   struct msm_drm_private *priv;
struct drm_bridge *ext_bridge;
int ret;
 
if (WARN_ON(!encoder) || WARN_ON(!msm_dsi) || WARN_ON(!dev))
return -EINVAL;
 
+   priv = dev->dev_private;
msm_dsi->dev = dev;
 
ret = msm_dsi_host_modeset_init(msm_dsi->host, dev);
-- 
2.7.4

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

Re: clang warning: implicit conversion in intel_ddi.c:1481

[Knut Omang]

On Fri, 2018-02-02 at 12:44 +0200, Jani Nikula wrote:
> +Knut, Fengguang
> 
> On Fri, 02 Feb 2018, Greg KH  wrote:
> > - If clang now builds the kernel "cleanly", yes, I want to take
> >   warning fixes in the stable tree.  And even better yet, if you
> >   keep working to ensure the tree is "clean", that would be
> >   wonderful.
> 
> So we can run sparse using 'make C=1' and friends, or other static
> analysis tools using 'make CHECK=foo C=1', as long as the passed command
> line params work. There was work by Knut to extend this make checker
> stuff [1]. Since mixing different HOSTCC's in a single workdir seems
> like a bad idea, I wonder how hard it would be to make clang work like
> this:
> 
> $ make CHECK=clang C=1
> 
> Or using Knut's wrapper. Feels like that could increase the use of clang
> for static analysis of patches.

Yes, definitely a natural addition to the set of tools supported by
runchecks to also support using alternate compiler(s) as "checkers" - I guess
the same would apply for people compiling with clang - that they don't 
accidentally
generate warnings with gcc..

Thanks,
Knut

> BR,
> Jani.
> 
> 
> [1] 
> http://mid.mail-archive.com/cover.5b56d020b8e826a7da33b1823c059acd0c123f8b.151507278
> 2.git-series.knut.om...@oracle.com
> 
> 
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/i915: Free memdup-ed bios data structures on driver_unload

[Ville Syrjälä]

On Mon, Jan 29, 2018 at 03:47:34PM +0100, Hans de Goede wrote:
> Add a new intel_bios_cleanup function to free memdup-ed bios data
> structures and call it from i915_driver_unload().
> 
> Signed-off-by: Hans de Goede 
> ---
>  drivers/gpu/drm/i915/i915_drv.c   |  2 ++
>  drivers/gpu/drm/i915/i915_drv.h   |  1 +
>  drivers/gpu/drm/i915/intel_bios.c | 11 +++
>  3 files changed, 14 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 1ec12add34b2..4ecf41724183 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1437,6 +1437,8 @@ void i915_driver_unload(struct drm_device *dev)
>  
>   intel_modeset_cleanup(dev);
>  
> + intel_bios_cleanup(dev_priv);
> +
>   /*
>* free the memory space allocated for the child device
>* config parsed from VBT

Looks like there's already a bunch of VBT related cleanup just below.
Maybe that should be sucked into the new cleanup function as well?

> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 454d8f937fae..081190da0818 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3663,6 +3663,7 @@ extern void intel_i2c_reset(struct drm_i915_private 
> *dev_priv);
>  
>  /* intel_bios.c */
>  void intel_bios_init(struct drm_i915_private *dev_priv);
> +void intel_bios_cleanup(struct drm_i915_private *dev_priv);
>  bool intel_bios_is_valid_vbt(const void *buf, size_t size);
>  bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
>  bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 
> *i2c_pin);
> diff --git a/drivers/gpu/drm/i915/intel_bios.c 
> b/drivers/gpu/drm/i915/intel_bios.c
> index 95f0b310d656..64a0d55df28e 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1588,6 +1588,17 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
>   pci_unmap_rom(pdev, bios);
>  }
>  
> +/**
> + * intel_bios_cleanup - Free any resources allocated by intel_bios_init()
> + * @dev_priv: i915 device instance
> + */
> +void intel_bios_cleanup(struct drm_i915_private *dev_priv)
> +{
> + kfree(dev_priv->vbt.dsi.data);
> + kfree(dev_priv->vbt.dsi.pps);
> + kfree(dev_priv->vbt.dsi.config);
> +}
> +
>  /**
>   * intel_bios_is_tv_present - is integrated TV present in VBT
>   * @dev_priv:i915 device instance
> -- 
> 2.14.3

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

Re: [PATCH] drm/amd/display: fix incompatible structure layouts

[Harry Wentland]

On 2018-02-02 11:02 AM, Arnd Bergmann wrote:
> On Fri, Feb 2, 2018 at 4:39 PM, Harry Wentland  wrote:
>> On 2018-02-02 07:31 AM, Arnd Bergmann wrote:
>>> Building the amd display driver with link-time optimizations revealed a bug
>>
>> Curious how I'd go about building with link-time optimizations.
> 
> I got the idea from last week's LWN article on the topic, see
> https://lwn.net/Articles/744507/. I needed the latest gcc version to
> avoid some compiler bugs, and a few dozen kernel patches on top
> to get a clean build in random configurations (posted them all today).
> 
> Most normal configurations probably work out of the box, but I have
> not actually tried running any ;-)
> 

Thanks. Learn something new every day. I like this bit from the article:

> So it is basically just like if it concatenated all source files into a 
> single one and made everything static.

Probably not a bad idea to think that way when doing kernel development. :)

Harry

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

Re: [PATCH] drm/amd/display: fix incompatible structure layouts

[Arnd Bergmann]

On Fri, Feb 2, 2018 at 4:39 PM, Harry Wentland  wrote:
> On 2018-02-02 07:31 AM, Arnd Bergmann wrote:
>> Building the amd display driver with link-time optimizations revealed a bug
>
> Curious how I'd go about building with link-time optimizations.

I got the idea from last week's LWN article on the topic, see
https://lwn.net/Articles/744507/. I needed the latest gcc version to
avoid some compiler bugs, and a few dozen kernel patches on top
to get a clean build in random configurations (posted them all today).

Most normal configurations probably work out of the box, but I have
not actually tried running any ;-)

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

Re: clang warning: implicit conversion in intel_ddi.c:1481

[Greg KH]

On Fri, Feb 02, 2018 at 04:37:55PM +0200, Jani Nikula wrote:
> On Fri, 02 Feb 2018, Greg KH  wrote:
> > On Fri, Feb 02, 2018 at 12:44:38PM +0200, Jani Nikula wrote:
> >> 
> >> +Knut, Fengguang
> >> 
> >> On Fri, 02 Feb 2018, Greg KH  wrote:
> >> >  - If clang now builds the kernel "cleanly", yes, I want to take
> >> >warning fixes in the stable tree.  And even better yet, if you
> >> >keep working to ensure the tree is "clean", that would be
> >> >wonderful.
> >> 
> >> So we can run sparse using 'make C=1' and friends, or other static
> >> analysis tools using 'make CHECK=foo C=1', as long as the passed command
> >> line params work. There was work by Knut to extend this make checker
> >> stuff [1]. Since mixing different HOSTCC's in a single workdir seems
> >> like a bad idea, I wonder how hard it would be to make clang work like
> >> this:
> >> 
> >> $ make CHECK=clang C=1
> >> 
> >> Or using Knut's wrapper. Feels like that could increase the use of clang
> >> for static analysis of patches.
> >
> > Why not just build with clang itself:
> > make CC=clang
> 
> Same as HOSTCC, mixing different CC's in a single build dir seems like a
> bad idea. Sure, everyone can setup a separate build dir for clang, but
> IMHO having 'make CHECK=clang C=1' work has least resistance. YMMV.

"O=some_output_dir" is your friend.  If you aren't doing that already
for your test builds, you don't know what you are missing :)

thanks,

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

Re: [PATCH] drm/amdgpu/display: fix wrong enum type for ddc_result

[Harry Wentland]

On 2018-02-01 08:55 PM, db...@chromium.org wrote:
> From: Dominik Behr 
> 
> v2: now with fixed result comparison and spelling fixes
> 
> Signed-off-by: Dominik Behr 
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 +-
>  drivers/gpu/drm/amd/display/dc/core/dc_link.c   | 2 +-
>  drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c   | 4 ++--
>  drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c| 4 ++--
>  drivers/gpu/drm/amd/display/include/ddc_service_types.h | 2 +-
>  5 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> index 1e8a21b67df7..3b05900d 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> @@ -130,7 +130,7 @@ static ssize_t dm_dp_aux_transfer(struct drm_dp_aux *aux,
>msg->address,
>msg->buffer,
>msg->size,
> -  r == DDC_RESULT_SUCESSFULL);
> +  r == DDC_RESULT_SUCCESSFUL);
>  #endif
>  
>   return msg->size;
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c 
> b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> index 0023754e034b..3abd0f1a287f 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> @@ -1176,7 +1176,7 @@ static void dpcd_configure_panel_mode(
>   _config_set.raw,
>   sizeof(edp_config_set.raw));
>  
> - ASSERT(result == DDC_RESULT_SUCESSFULL);
> + ASSERT(result == DDC_RESULT_SUCCESSFUL);
>   }
>   }
>   dm_logger_write(link->ctx->logger, LOG_DETECTION_DP_CAPS,
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c 
> b/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
> index d5294798b0a5..6b69b339dba2 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
> @@ -661,7 +661,7 @@ enum ddc_result dal_ddc_service_read_dpcd_data(
>   ddc->ctx->i2caux,
>   ddc->ddc_pin,
>   ))
> - return DDC_RESULT_SUCESSFULL;
> + return DDC_RESULT_SUCCESSFUL;
>  
>   return DDC_RESULT_FAILED_OPERATION;
>  }
> @@ -698,7 +698,7 @@ enum ddc_result dal_ddc_service_write_dpcd_data(
>   ddc->ctx->i2caux,
>   ddc->ddc_pin,
>   ))
> - return DDC_RESULT_SUCESSFULL;
> + return DDC_RESULT_SUCCESSFUL;
>  
>   return DDC_RESULT_FAILED_OPERATION;
>  }
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c 
> b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> index 33d91e4474ea..cc067d04505d 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> @@ -1926,7 +1926,7 @@ bool dc_link_handle_hpd_rx_irq(struct dc_link *link, 
> union hpd_irq_data *out_hpd
>  {
>   union hpd_irq_data hpd_irq_dpcd_data = 0;
>   union device_service_irq device_service_clear = { { 0 } };
> - enum dc_status result = DDC_RESULT_UNKNOWN;
> + enum ddc_result result = DDC_RESULT_UNKNOWN;

Result gets the return value from read_hpd_rx_irq_data which is dc_status. This 
line should be

enum dc_status result = DC_OK;

>   bool status = false;
>   /* For use cases related to down stream connection status change,
>* PSR and device auto test, refer to function handle_sst_hpd_irq
> @@ -1946,7 +1946,7 @@ bool dc_link_handle_hpd_rx_irq(struct dc_link *link, 
> union hpd_irq_data *out_hpd
>   if (out_hpd_irq_dpcd_data)
>   *out_hpd_irq_dpcd_data = hpd_irq_dpcd_data;
>  
> - if (result != DC_OK) {
> + if (result != DDC_RESULT_SUCCESSFUL) {

We should keep the result != DC_OK check here as read_hpd_rx_irq_data returns 
dc_status.

Harry

>   dm_logger_write(link->ctx->logger, LOG_HW_HPD_IRQ,
>   "%s: DPCD read failed to obtain irq data\n",
>   __func__);
> diff --git a/drivers/gpu/drm/amd/display/include/ddc_service_types.h 
> b/drivers/gpu/drm/amd/display/include/ddc_service_types.h
> index 019e7a095ea1..f3bf749b3636 100644
> --- a/drivers/gpu/drm/amd/display/include/ddc_service_types.h
> +++ b/drivers/gpu/drm/amd/display/include/ddc_service_types.h
> @@ -32,7 +32,7 @@
>  
>  enum ddc_result {
>   DDC_RESULT_UNKNOWN = 0,
> - DDC_RESULT_SUCESSFULL,
> + DDC_RESULT_SUCCESSFUL,
>   DDC_RESULT_FAILED_CHANNEL_BUSY,
>   DDC_RESULT_FAILED_TIMEOUT,
>   DDC_RESULT_FAILED_PROTOCOL_ERROR,
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/amd/display: fix incompatible structure layouts

[Harry Wentland]

On 2018-02-02 07:31 AM, Arnd Bergmann wrote:
> Building the amd display driver with link-time optimizations revealed a bug

Curious how I'd go about building with link-time optimizations.

> that caused dal_cmd_tbl_helper_dce80_get_table() and
> dal_cmd_tbl_helper_dce110_get_table() get called with an incompatible
> return type between the two callers in command_table_helper.c and
> command_table_helper2.c:
> 
> drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce80/command_table_helper_dce80.h:31:
>  error: type of 'dal_cmd_tbl_helper_dce80_get_table' does not match original 
> declaration [-Werror=lto-type-mismatch]
>  const struct command_table_helper *dal_cmd_tbl_helper_dce80_get_table(void);
> 
> drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce80/command_table_helper_dce80.c:351:
>  note: 'dal_cmd_tbl_helper_dce80_get_table' was previously declared here
>  const struct command_table_helper *dal_cmd_tbl_helper_dce80_get_table(void)
> 
> drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce110/command_table_helper_dce110.h:32:
>  error: type of 'dal_cmd_tbl_helper_dce110_get_table' does not match original 
> declaration [-Werror=lto-type-mismatch]
>  const struct command_table_helper *dal_cmd_tbl_helper_dce110_get_table(void);
> 
> drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce110/command_table_helper_dce110.c:361:
>  note: 'dal_cmd_tbl_helper_dce110_get_table' was previously declared here
>  const struct command_table_helper *dal_cmd_tbl_helper_dce110_get_table(void)
> 
> The two versions of the structure are obviously derived from the same
> one, but have diverged over time, before they got added to the kernel.
> 
> This moves the structure to a new shared header file and uses the superset
> of the members, to ensure the interfaces are all compatible.
> 
> Fixes: ae79c310b1a6 ("drm/amd/display: Add DCE12 bios parser support")
> Signed-off-by: Arnd Bergmann 

Thanks for the fix.

Reviewed-by: Harry Wentland 

Harry

> ---
>  .../drm/amd/display/dc/bios/command_table_helper.h | 33 +--
>  .../amd/display/dc/bios/command_table_helper2.h| 30 +-
>  .../display/dc/bios/command_table_helper_struct.h  | 66 
> ++
>  3 files changed, 68 insertions(+), 61 deletions(-)
>  create mode 100644 
> drivers/gpu/drm/amd/display/dc/bios/command_table_helper_struct.h
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table_helper.h 
> b/drivers/gpu/drm/amd/display/dc/bios/command_table_helper.h
> index 1fab634b66be..4c3789df253d 100644
> --- a/drivers/gpu/drm/amd/display/dc/bios/command_table_helper.h
> +++ b/drivers/gpu/drm/amd/display/dc/bios/command_table_helper.h
> @@ -29,38 +29,7 @@
>  #include "dce80/command_table_helper_dce80.h"
>  #include "dce110/command_table_helper_dce110.h"
>  #include "dce112/command_table_helper_dce112.h"
> -
> -struct command_table_helper {
> - bool (*controller_id_to_atom)(enum controller_id id, uint8_t *atom_id);
> - uint8_t (*encoder_action_to_atom)(
> - enum bp_encoder_control_action action);
> - uint32_t (*encoder_mode_bp_to_atom)(enum signal_type s,
> - bool enable_dp_audio);
> - bool (*engine_bp_to_atom)(enum engine_id engine_id,
> - uint32_t *atom_engine_id);
> - void (*assign_control_parameter)(
> - const struct command_table_helper *h,
> - struct bp_encoder_control *control,
> - DIG_ENCODER_CONTROL_PARAMETERS_V2 *ctrl_param);
> - bool (*clock_source_id_to_atom)(enum clock_source_id id,
> - uint32_t *atom_pll_id);
> - bool (*clock_source_id_to_ref_clk_src)(
> - enum clock_source_id id,
> - uint32_t *ref_clk_src_id);
> - uint8_t (*transmitter_bp_to_atom)(enum transmitter t);
> - uint8_t (*encoder_id_to_atom)(enum encoder_id id);
> - uint8_t (*clock_source_id_to_atom_phy_clk_src_id)(
> - enum clock_source_id id);
> - uint8_t (*signal_type_to_atom_dig_mode)(enum signal_type s);
> - uint8_t (*hpd_sel_to_atom)(enum hpd_source_id id);
> - uint8_t (*dig_encoder_sel_to_atom)(enum engine_id engine_id);
> - uint8_t (*phy_id_to_atom)(enum transmitter t);
> - uint8_t (*disp_power_gating_action_to_atom)(
> - enum bp_pipe_control_action action);
> - bool (*dc_clock_type_to_atom)(enum bp_dce_clock_type id,
> - uint32_t *atom_clock_type);
> -uint8_t (*transmitter_color_depth_to_atom)(enum transmitter_color_depth 
> id);
> -};
> +#include "command_table_helper_struct.h"
>  
>  bool dal_bios_parser_init_cmd_tbl_helper(const struct command_table_helper 
> **h,
>   enum dce_version dce);
> diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table_helper2.h 
> b/drivers/gpu/drm/amd/display/dc/bios/command_table_helper2.h
> index 9f587c91d843..785fcb20a1b9 100644
> --- 

[PATCH] drm: nouveau: use larger buffer in nvif_vmm_map

[Arnd Bergmann]

gcc points out a buffer that is clearly too small to be used
in a meaningful way, as the 'sizeof(*args) + argc > sizeof(stack)'
will always fail:

In function 'memcpy',
inlined from 'nvif_vmm_map' at drivers/gpu/drm/nouveau/nvif/vmm.c:55:2:
include/linux/string.h:353:9: error: '__builtin_memcpy' offset 40 is out of the 
bounds [0, 16] of object 'stack' with type 'u8[16]' {aka 'unsigned char[16]'} 
[-Werror=array-bounds]
  return __builtin_memcpy(p, q, size);
 ^~~~
drivers/gpu/drm/nouveau/nvif/vmm.c: In function 'nvif_vmm_map':
drivers/gpu/drm/nouveau/nvif/vmm.c:40:5: note: 'stack' declared here

This makes the buffer large enough so it should serve the purpose
that the author presumably had in mind. Alternatively we could
just get rid of it completely and simplify the code at the cost
of always doing the kmalloc (as we do in the current version).

Fixes: 920d2b5ef215 ("drm/nouveau/mmu: define user interfaces to mmu vmm 
opertaions")
Signed-off-by: Arnd Bergmann 
---
Cc: Martin Sebor 

Martin: this one is interesting, I think it qualifies as a
false-positive warning that gcc should not print because there
is no overflow, but the code is still wrong because we never
copy into the fixed-size buffer that was intended as a
micro-optimization
---
 drivers/gpu/drm/nouveau/nvif/vmm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvif/vmm.c 
b/drivers/gpu/drm/nouveau/nvif/vmm.c
index 31cdb2d2e1ff..191832be6c65 100644
--- a/drivers/gpu/drm/nouveau/nvif/vmm.c
+++ b/drivers/gpu/drm/nouveau/nvif/vmm.c
@@ -37,7 +37,7 @@ nvif_vmm_map(struct nvif_vmm *vmm, u64 addr, u64 size, void 
*argv, u32 argc,
 struct nvif_mem *mem, u64 offset)
 {
struct nvif_vmm_map_v0 *args;
-   u8 stack[16];
+   u8 stack[48];
int ret;
 
if (sizeof(*args) + argc > sizeof(stack)) {
-- 
2.9.0

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

[Bug 104902] unpackHalf2x16 causes LLVM Error on GCN 1.2+ cards (and possibly others)

[bugzilla-daemon]

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

Michel Dänzer  changed:

   What|Removed |Added

 CC||rtf...@gmail.com

--- Comment #2 from Michel Dänzer  ---
*** Bug 104374 has been marked as a duplicate of this bug. ***

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

[PATCH 0/2] drm/exynos/mixer: fixes for interlaced mode

[Andrzej Hajda]

Hi Inki, Tobias,

This is reviewed/updated/tested Tobias's patch addressing page-faults
in Video Processor in interlaced mode. Plus preliminary patch fixing Mixer
interlaced mode.

Regards
Andrzej


Andrzej Hajda (1):
  drm/exynos/mixer: fix synchronization check in interlaced mode

Tobias Jakobi (1):
  drm/exynos: mixer: avoid Oops in vp_video_buffer()

 drivers/gpu/drm/exynos/exynos_mixer.c | 22 +-
 drivers/gpu/drm/exynos/regs-mixer.h   |  1 +
 2 files changed, 18 insertions(+), 5 deletions(-)

-- 
2.16.1

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

[PATCH 2/2] drm/exynos: mixer: avoid Oops in vp_video_buffer()

[Andrzej Hajda]

From: Tobias Jakobi 

If an interlaced video mode is selected, a IOMMU pagefault is
triggered by vp_video_buffer().

Fix the most apparent bugs:
- pitch value for chroma plane
- divide by two of height and vpos of source and destination

Signed-off-by: Tobias Jakobi 
[ a.hajda: Halved also destination height and vpos, updated commit message ]
Signed-off-by: Andrzej Hajda 
---
 drivers/gpu/drm/exynos/exynos_mixer.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
b/drivers/gpu/drm/exynos/exynos_mixer.c
index ff7d088c922a..66b7cc2128e7 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -485,7 +485,7 @@ static void vp_video_buffer(struct mixer_context *ctx,
chroma_addr[1] = chroma_addr[0] + 0x40;
} else {
luma_addr[1] = luma_addr[0] + fb->pitches[0];
-   chroma_addr[1] = chroma_addr[0] + fb->pitches[0];
+   chroma_addr[1] = chroma_addr[0] + fb->pitches[1];
}
} else {
luma_addr[1] = 0;
@@ -508,21 +508,23 @@ static void vp_video_buffer(struct mixer_context *ctx,
vp_reg_write(ctx, VP_IMG_SIZE_Y, VP_IMG_HSIZE(fb->pitches[0]) |
VP_IMG_VSIZE(fb->height));
/* chroma plane for NV12/NV21 is half the height of the luma plane */
-   vp_reg_write(ctx, VP_IMG_SIZE_C, VP_IMG_HSIZE(fb->pitches[0]) |
+   vp_reg_write(ctx, VP_IMG_SIZE_C, VP_IMG_HSIZE(fb->pitches[1]) |
VP_IMG_VSIZE(fb->height / 2));
 
vp_reg_write(ctx, VP_SRC_WIDTH, state->src.w);
-   vp_reg_write(ctx, VP_SRC_HEIGHT, state->src.h);
vp_reg_write(ctx, VP_SRC_H_POSITION,
VP_SRC_H_POSITION_VAL(state->src.x));
-   vp_reg_write(ctx, VP_SRC_V_POSITION, state->src.y);
-
vp_reg_write(ctx, VP_DST_WIDTH, state->crtc.w);
vp_reg_write(ctx, VP_DST_H_POSITION, state->crtc.x);
+
if (test_bit(MXR_BIT_INTERLACE, >flags)) {
+   vp_reg_write(ctx, VP_SRC_HEIGHT, state->src.h / 2);
+   vp_reg_write(ctx, VP_SRC_V_POSITION, state->src.y / 2);
vp_reg_write(ctx, VP_DST_HEIGHT, state->crtc.h / 2);
vp_reg_write(ctx, VP_DST_V_POSITION, state->crtc.y / 2);
} else {
+   vp_reg_write(ctx, VP_SRC_HEIGHT, state->src.h);
+   vp_reg_write(ctx, VP_SRC_V_POSITION, state->src.y);
vp_reg_write(ctx, VP_DST_HEIGHT, state->crtc.h);
vp_reg_write(ctx, VP_DST_V_POSITION, state->crtc.y);
}
-- 
2.16.1

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

[PATCH 1/2] drm/exynos/mixer: fix synchronization check in interlaced mode

[Andrzej Hajda]

In case of interlace mode video processor registers and mixer config
register must be check to ensure internal state is in sync with shadow
registers.
This patch fixes page-faults in interlaced mode.

Signed-off-by: Andrzej Hajda 
---
 drivers/gpu/drm/exynos/exynos_mixer.c | 10 ++
 drivers/gpu/drm/exynos/regs-mixer.h   |  1 +
 2 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
b/drivers/gpu/drm/exynos/exynos_mixer.c
index dc5d79465f9b..ff7d088c922a 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -494,6 +494,7 @@ static void vp_video_buffer(struct mixer_context *ctx,
 
spin_lock_irqsave(>reg_slock, flags);
 
+   vp_reg_write(ctx, VP_SHADOW_UPDATE, 1);
/* interlace or progressive scan mode */
val = (test_bit(MXR_BIT_INTERLACE, >flags) ? ~0 : 0);
vp_reg_writemask(ctx, VP_MODE, val, VP_MODE_LINE_SKIP);
@@ -711,6 +712,15 @@ static irqreturn_t mixer_irq_handler(int irq, void *arg)
 
/* interlace scan need to check shadow register */
if (test_bit(MXR_BIT_INTERLACE, >flags)) {
+   if (test_bit(MXR_BIT_VP_ENABLED, >flags) &&
+   vp_reg_read(ctx, VP_SHADOW_UPDATE))
+   goto out;
+
+   base = mixer_reg_read(ctx, MXR_CFG);
+   shadow = mixer_reg_read(ctx, MXR_CFG_S);
+   if (base != shadow)
+   goto out;
+
base = mixer_reg_read(ctx, MXR_GRAPHIC_BASE(0));
shadow = mixer_reg_read(ctx, MXR_GRAPHIC_BASE_S(0));
if (base != shadow)
diff --git a/drivers/gpu/drm/exynos/regs-mixer.h 
b/drivers/gpu/drm/exynos/regs-mixer.h
index c311f571bdf9..189cfa2470a8 100644
--- a/drivers/gpu/drm/exynos/regs-mixer.h
+++ b/drivers/gpu/drm/exynos/regs-mixer.h
@@ -47,6 +47,7 @@
 #define MXR_MO 0x0304
 #define MXR_RESOLUTION 0x0310
 
+#define MXR_CFG_S  0x2004
 #define MXR_GRAPHIC0_BASE_S0x2024
 #define MXR_GRAPHIC1_BASE_S0x2044
 
-- 
2.16.1

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

Re: [PATCH 3/5] drm/armada: Construct a temporary crtc state for plane checks

[Ville Syrjälä]

On Fri, Feb 02, 2018 at 04:10:39PM +0200, Ville Syrjälä wrote:
> On Tue, Jan 23, 2018 at 09:02:35PM +0200, Ville Syrjälä wrote:
> > On Tue, Jan 23, 2018 at 06:42:00PM +, Russell King - ARM Linux wrote:
> > > On Tue, Jan 23, 2018 at 07:08:55PM +0200, Ville Syrjala wrote:
> > > > From: Ville Syrjälä 
> > > > 
> > > > As armada isn't an atomic driver trying to pass a non-populated
> > > > crtc->state to drm_atomic_helper_check_plane_state() will end in tears.
> > > > Construct a temporary crtc state a la drm_plane_helper_check_update()
> > > > and pass that instead. For now we just really need crtc_state->enable
> > > > to be there.
> > > 
> > > Would it be possible to solve this by having the atomic state setup
> > > for non-atomic drivers instead, so we're not unwinding some of the
> > > work that's already been done to try and convert drivers /to/
> > > atomic modeset?
> > 
> > Dunno. Feels like a wasted effort adding more code that'll just get
> > ripped out as soon as the atomic conversion happens. And I'd rather
> > not have to worry about potentially stale states hanging around, in
> > case you forgot to update something somewhere.
> > 
> > In any case, I don't think this is unwinding anything. Once you have
> > the atomic conversion done sufficiently you can just drop these
> > temporary states. We already have the temp state for the plane here
> > anyway, and pairing that with a crtc state seems rather logical.
> 
> So yea or nay on these armada patches?

Also cc:ing Lucas since apparently armada is somehow related to
etnaviv...

I have my doubts about the current code working at all (due to
the conflict resolution between my refactoring and rmk's work).

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

Re: [PATCH 2/5] drm/sun4i: Use drm_mode_get_hv_timing() to populate plane clip rectangle

[Ville Syrjälä]

On Tue, Jan 23, 2018 at 07:08:54PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> Use drm_mode_get_hv_timing() to fill out the plane clip rectangle.
> 
> Note that this replaces crtc_state->adjusted_mode usage with
> crtc_state->mode. The latter is the correct choice since that's the
> mode the user provided and it matches the plane crtc coordinates
> the user also provided.
> 
> Once everyone agrees on this we can move the clip handling into
> drm_atomic_helper_check_plane_state().
> 
> Cc: Maxime Ripard 
> Signed-off-by: Ville Syrjälä 

Pushed to drm-misc-next with Maximes's irc r-b:
16:13 < vsyrjala> mripard: got time to look at 
https://patchwork.freedesktop.org/patch/200244/ ?
...
16:25 < mripard> vsyrjala: it looks good to me, you can add my Reviewed-by

Also pushed 1/5. Thanks for the reviews.

> ---
>  drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 9 -
>  drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 9 -
>  2 files changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c 
> b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> index 28d7c48d50fe..2f0ccd50b54d 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> @@ -211,7 +211,7 @@ static int sun8i_ui_layer_atomic_check(struct drm_plane 
> *plane,
>   struct drm_crtc *crtc = state->crtc;
>   struct drm_crtc_state *crtc_state;
>   int min_scale, max_scale;
> - struct drm_rect clip;
> + struct drm_rect clip = {};
>  
>   if (!crtc)
>   return 0;
> @@ -220,10 +220,9 @@ static int sun8i_ui_layer_atomic_check(struct drm_plane 
> *plane,
>   if (WARN_ON(!crtc_state))
>   return -EINVAL;
>  
> - clip.x1 = 0;
> - clip.y1 = 0;
> - clip.x2 = crtc_state->adjusted_mode.hdisplay;
> - clip.y2 = crtc_state->adjusted_mode.vdisplay;
> + if (crtc_state->enable)
> + drm_mode_get_hv_timing(_state->mode,
> +, );
>  
>   min_scale = DRM_PLANE_HELPER_NO_SCALING;
>   max_scale = DRM_PLANE_HELPER_NO_SCALING;
> diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c 
> b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> index 40c3b303068a..eb3bf2d7291a 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> @@ -239,7 +239,7 @@ static int sun8i_vi_layer_atomic_check(struct drm_plane 
> *plane,
>   struct drm_crtc *crtc = state->crtc;
>   struct drm_crtc_state *crtc_state;
>   int min_scale, max_scale;
> - struct drm_rect clip;
> + struct drm_rect clip = {};
>  
>   if (!crtc)
>   return 0;
> @@ -248,10 +248,9 @@ static int sun8i_vi_layer_atomic_check(struct drm_plane 
> *plane,
>   if (WARN_ON(!crtc_state))
>   return -EINVAL;
>  
> - clip.x1 = 0;
> - clip.y1 = 0;
> - clip.x2 = crtc_state->adjusted_mode.hdisplay;
> - clip.y2 = crtc_state->adjusted_mode.vdisplay;
> + if (crtc_state->enable)
> + drm_mode_get_hv_timing(_state->mode,
> +, );
>  
>   min_scale = DRM_PLANE_HELPER_NO_SCALING;
>   max_scale = DRM_PLANE_HELPER_NO_SCALING;
> -- 
> 2.13.6

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

[PATCH] video: fbdev: sis: avoid mismatched prototypes

[Arnd Bergmann]

Building with LTO enabled reveals some functions whose prototypes
in the header are different from the definition:

drivers/video/fbdev/sis/sis_main.h:765:0: error: type of 'SiS_SetCH70xxANDOR' 
does not match original declaration [-Werror=lto-type-mismatch]
 extern void  SiS_SetCH70xxANDOR(struct SiS_Private *SiS_Pr, unsigned short reg,

drivers/video/fbdev/sis/init301.c:8937:0: note: type mismatch in parameter 4
 SiS_SetCH70xxANDOR(struct SiS_Private *SiS_Pr, unsigned short reg,

drivers/video/fbdev/sis/init301.c:8937:0: note: type 'short unsigned int' 
should match type 'unsigned char'
drivers/video/fbdev/sis/init301.c:8937:0: note: 'SiS_SetCH70xxANDOR' was 
previously declared here

The root cause appears to be the way that header files are used in this
driver, where they contain both static variable and declarations for
symbols in other files.

To clean that up, I'm changing all mixed headers to only contain
declarations the way we normally do in C, or contain only static
variables, and move the rest to a more appropriate place.  Once that
is done, the headers can be included in the other files as well, and
guarantee that the prototypes match.

There are a few headers that now only contain static variables, and
I'm leaving those alone here as the patch is already too big. These
could be trivially moved into the respective .c files.

Signed-off-by: Arnd Bergmann 
---
 drivers/video/fbdev/sis/init.h |  76 -
 drivers/video/fbdev/sis/init301.c  | 326 +
 drivers/video/fbdev/sis/init301.h  | 320 
 drivers/video/fbdev/sis/sis.h  | 131 +++
 drivers/video/fbdev/sis/sis_main.c |  51 ++
 drivers/video/fbdev/sis/sis_main.h | 117 -
 6 files changed, 508 insertions(+), 513 deletions(-)

diff --git a/drivers/video/fbdev/sis/init.h b/drivers/video/fbdev/sis/init.h
index 85d6738b6c64..400b0e5681b2 100644
--- a/drivers/video/fbdev/sis/init.h
+++ b/drivers/video/fbdev/sis/init.h
@@ -1461,81 +1461,5 @@ static const struct SiS_LVDSCRT1Data 
SiS_LVDSCRT1640x480_1_H[] =
0x00}}
 };
 
-bool   SiSInitPtr(struct SiS_Private *SiS_Pr);
-unsigned short SiS_GetModeID_LCD(int VGAEngine, unsigned int VBFlags, int 
HDisplay,
-   int VDisplay, int Depth, bool FSTN,
-   unsigned short CustomT, int LCDwith, int 
LCDheight,
-   unsigned int VBFlags2);
-unsigned short SiS_GetModeID_TV(int VGAEngine, unsigned int VBFlags, int 
HDisplay,
-   int VDisplay, int Depth, unsigned int VBFlags2);
-unsigned short SiS_GetModeID_VGA2(int VGAEngine, unsigned int VBFlags, int 
HDisplay,
-   int VDisplay, int Depth, unsigned int VBFlags2);
-
-void   SiS_DisplayOn(struct SiS_Private *SiS_Pr);
-void   SiS_DisplayOff(struct SiS_Private *SiS_Pr);
-void   SiSRegInit(struct SiS_Private *SiS_Pr, SISIOADDRESS BaseAddr);
-void   SiS_SetEnableDstn(struct SiS_Private *SiS_Pr, int enable);
-void   SiS_SetEnableFstn(struct SiS_Private *SiS_Pr, int enable);
-unsigned short SiS_GetModeFlag(struct SiS_Private *SiS_Pr, unsigned short 
ModeNo,
-   unsigned short ModeIdIndex);
-bool   SiSDetermineROMLayout661(struct SiS_Private *SiS_Pr);
-
-bool   SiS_SearchModeID(struct SiS_Private *SiS_Pr, unsigned short 
*ModeNo,
-   unsigned short *ModeIdIndex);
-unsigned short SiS_GetModePtr(struct SiS_Private *SiS_Pr, unsigned short 
ModeNo,
-   unsigned short ModeIdIndex);
-unsigned short  SiS_GetRefCRTVCLK(struct SiS_Private *SiS_Pr, unsigned short 
Index, int UseWide);
-unsigned short  SiS_GetRefCRT1CRTC(struct SiS_Private *SiS_Pr, unsigned short 
Index, int UseWide);
-unsigned short SiS_GetColorDepth(struct SiS_Private *SiS_Pr, unsigned short 
ModeNo,
-   unsigned short ModeIdIndex);
-unsigned short SiS_GetOffset(struct SiS_Private *SiS_Pr,unsigned short ModeNo,
-   unsigned short ModeIdIndex, unsigned short 
RRTI);
-#ifdef CONFIG_FB_SIS_300
-void   SiS_GetFIFOThresholdIndex300(struct SiS_Private *SiS_Pr, 
unsigned short *idx1,
-   unsigned short *idx2);
-unsigned short SiS_GetFIFOThresholdB300(unsigned short idx1, unsigned short 
idx2);
-unsigned short SiS_GetLatencyFactor630(struct SiS_Private *SiS_Pr, unsigned 
short index);
-#endif
-void   SiS_LoadDAC(struct SiS_Private *SiS_Pr, unsigned short ModeNo, 
unsigned short ModeIdIndex);
-bool   SiSSetMode(struct SiS_Private *SiS_Pr, unsigned short ModeNo);
-void   SiS_CalcCRRegisters(struct SiS_Private *SiS_Pr, int depth);
-void   SiS_CalcLCDACRT1Timing(struct SiS_Private *SiS_Pr, unsigned 
short ModeNo,
-   unsigned short ModeIdIndex);
-void   

Re: [Intel-gfx] [PATCH] drm/crc: Add support for polling on the data fd.

[Ville Syrjälä]

On Fri, Feb 02, 2018 at 03:27:43PM +0100, Maarten Lankhorst wrote:
> This will make it possible for userspace to know whether reading
> will block, without blocking on the fd. This makes it possible to
> drain all queued CRC's in blocking mode, without having to reopen
> the fd.
> 
> Signed-off-by: Maarten Lankhorst 
> ---
>  drivers/gpu/drm/drm_debugfs_crc.c | 19 +++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_debugfs_crc.c 
> b/drivers/gpu/drm/drm_debugfs_crc.c
> index 9dd879589a2c..8af1a74ec64d 100644
> --- a/drivers/gpu/drm/drm_debugfs_crc.c
> +++ b/drivers/gpu/drm/drm_debugfs_crc.c
> @@ -307,10 +307,29 @@ static ssize_t crtc_crc_read(struct file *filep, char 
> __user *user_buf,
>   return LINE_LEN(crc->values_cnt);
>  }
>  
> +static unsigned int crtc_crc_poll(struct file *file, poll_table *wait)
> +{
> + struct drm_crtc *crtc = file->f_inode->i_private;
> + struct drm_crtc_crc *crc = >crc;
> + unsigned ret;
> +
> + poll_wait(file, >wq, wait);
> +
> + spin_lock_irq(>lock);
> + if (crc->source && crtc_crc_data_count(crc))
> + ret = POLLIN;

Most places seem to also set POLLRDNORM. Maybe we should do it as well?

Apart from that this seems good to me.
Reviewed-by: Ville Syrjälä 

Could replace the usleep() loop in igt read_one_crc() with
poll/select() I suppose? Either that or we should switch between
blocking and nonblocking dynamically.

> + else
> + ret = 0;
> + spin_unlock_irq(>lock);
> +
> + return ret;
> +}
> +
>  static const struct file_operations drm_crtc_crc_data_fops = {
>   .owner = THIS_MODULE,
>   .open = crtc_crc_open,
>   .read = crtc_crc_read,
> + .poll = crtc_crc_poll,
>   .release = crtc_crc_release,
>  };
>  
> -- 
> 2.15.1
> 
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: clang warning: implicit conversion in intel_ddi.c:1481

[Jani Nikula]

On Fri, 02 Feb 2018, Greg KH  wrote:
> On Fri, Feb 02, 2018 at 12:44:38PM +0200, Jani Nikula wrote:
>> 
>> +Knut, Fengguang
>> 
>> On Fri, 02 Feb 2018, Greg KH  wrote:
>> >- If clang now builds the kernel "cleanly", yes, I want to take
>> >  warning fixes in the stable tree.  And even better yet, if you
>> >  keep working to ensure the tree is "clean", that would be
>> >  wonderful.
>> 
>> So we can run sparse using 'make C=1' and friends, or other static
>> analysis tools using 'make CHECK=foo C=1', as long as the passed command
>> line params work. There was work by Knut to extend this make checker
>> stuff [1]. Since mixing different HOSTCC's in a single workdir seems
>> like a bad idea, I wonder how hard it would be to make clang work like
>> this:
>> 
>> $ make CHECK=clang C=1
>> 
>> Or using Knut's wrapper. Feels like that could increase the use of clang
>> for static analysis of patches.
>
> Why not just build with clang itself:
>   make CC=clang

Same as HOSTCC, mixing different CC's in a single build dir seems like a
bad idea. Sure, everyone can setup a separate build dir for clang, but
IMHO having 'make CHECK=clang C=1' work has least resistance. YMMV.

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

[PATCH] drm/crc: Add support for polling on the data fd.

[Maarten Lankhorst]

This will make it possible for userspace to know whether reading
will block, without blocking on the fd. This makes it possible to
drain all queued CRC's in blocking mode, without having to reopen
the fd.

Signed-off-by: Maarten Lankhorst 
---
 drivers/gpu/drm/drm_debugfs_crc.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/drm_debugfs_crc.c 
b/drivers/gpu/drm/drm_debugfs_crc.c
index 9dd879589a2c..8af1a74ec64d 100644
--- a/drivers/gpu/drm/drm_debugfs_crc.c
+++ b/drivers/gpu/drm/drm_debugfs_crc.c
@@ -307,10 +307,29 @@ static ssize_t crtc_crc_read(struct file *filep, char 
__user *user_buf,
return LINE_LEN(crc->values_cnt);
 }
 
+static unsigned int crtc_crc_poll(struct file *file, poll_table *wait)
+{
+   struct drm_crtc *crtc = file->f_inode->i_private;
+   struct drm_crtc_crc *crc = >crc;
+   unsigned ret;
+
+   poll_wait(file, >wq, wait);
+
+   spin_lock_irq(>lock);
+   if (crc->source && crtc_crc_data_count(crc))
+   ret = POLLIN;
+   else
+   ret = 0;
+   spin_unlock_irq(>lock);
+
+   return ret;
+}
+
 static const struct file_operations drm_crtc_crc_data_fops = {
.owner = THIS_MODULE,
.open = crtc_crc_open,
.read = crtc_crc_read,
+   .poll = crtc_crc_poll,
.release = crtc_crc_release,
 };
 
-- 
2.15.1

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

Re: [PATCH 3/5] drm/armada: Construct a temporary crtc state for plane checks

[Ville Syrjälä]

On Tue, Jan 23, 2018 at 09:02:35PM +0200, Ville Syrjälä wrote:
> On Tue, Jan 23, 2018 at 06:42:00PM +, Russell King - ARM Linux wrote:
> > On Tue, Jan 23, 2018 at 07:08:55PM +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä 
> > > 
> > > As armada isn't an atomic driver trying to pass a non-populated
> > > crtc->state to drm_atomic_helper_check_plane_state() will end in tears.
> > > Construct a temporary crtc state a la drm_plane_helper_check_update()
> > > and pass that instead. For now we just really need crtc_state->enable
> > > to be there.
> > 
> > Would it be possible to solve this by having the atomic state setup
> > for non-atomic drivers instead, so we're not unwinding some of the
> > work that's already been done to try and convert drivers /to/
> > atomic modeset?
> 
> Dunno. Feels like a wasted effort adding more code that'll just get
> ripped out as soon as the atomic conversion happens. And I'd rather
> not have to worry about potentially stale states hanging around, in
> case you forgot to update something somewhere.
> 
> In any case, I don't think this is unwinding anything. Once you have
> the atomic conversion done sufficiently you can just drop these
> temporary states. We already have the temp state for the plane here
> anyway, and pairing that with a crtc state seems rather logical.

So yea or nay on these armada patches?

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

Re: [PATCH] drm/fb-helper: Scale back depth to supported maximum

[Ville Syrjälä]

On Fri, Feb 02, 2018 at 02:56:30PM +0100, Linus Walleij wrote:
> On Thu, Feb 1, 2018 at 2:19 PM, Ville Syrjälä
>  wrote:
> > [Me]
> >> + /*
> >> +  * If we run into a situation where, for example, the primary plane
> >> +  * supports RGBA5551 (16 bpp, depth 15) but not RGB565 (16 bpp, depth
> >> +  * 16) we need to scale down the depth of the sizes we request.
> >> +  */
> >> + drm_for_each_plane(plane, fb_helper->dev) {
> >> + /* Only check the primary plane */
> >> + if (plane->type != DRM_PLANE_TYPE_PRIMARY)
> >> + continue;
> >
> > I think this should look at crtc->primary for each of the crtcs managed
> > by the fb_helper.
> >
> > Also this probably shouldn't look at YUV formats at all?
> 
> I guess I can look into doing it this way, sorry for not knowing how to
> properly inspect DRM objects, I'm lost sometimes...
> 
> > I do wonder if instead we should just have the driver specify the
> > pixel format explicitly instead of trying to guess based on bpp?
> 
> That makes a lot more sense to me actually. It would
> give a better sense of control so the driver feel it knows
> what is actually going on.
> 
> So I would just update
> drm_fb_cma_fbdev_init() and drm_fb_helper_initial_config()
> to pass a reasonable pixel format instead and refactor all the
> way down?

Yeah, something along those lines would seem like the better approach
to me. But it's been a while since I've looked at this code so I might
be totally wrong :)

> 
> It does hit a lot of code on the way, but if everyone thinks this
> is a good idea I can very well take a stab at it.
> 
> Yours,
> Linus Walleij

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

Re: [PATCH] drm/fb-helper: Scale back depth to supported maximum

[Linus Walleij]

On Thu, Feb 1, 2018 at 2:19 PM, Ville Syrjälä
 wrote:
> [Me]
>> + /*
>> +  * If we run into a situation where, for example, the primary plane
>> +  * supports RGBA5551 (16 bpp, depth 15) but not RGB565 (16 bpp, depth
>> +  * 16) we need to scale down the depth of the sizes we request.
>> +  */
>> + drm_for_each_plane(plane, fb_helper->dev) {
>> + /* Only check the primary plane */
>> + if (plane->type != DRM_PLANE_TYPE_PRIMARY)
>> + continue;
>
> I think this should look at crtc->primary for each of the crtcs managed
> by the fb_helper.
>
> Also this probably shouldn't look at YUV formats at all?

I guess I can look into doing it this way, sorry for not knowing how to
properly inspect DRM objects, I'm lost sometimes...

> I do wonder if instead we should just have the driver specify the
> pixel format explicitly instead of trying to guess based on bpp?

That makes a lot more sense to me actually. It would
give a better sense of control so the driver feel it knows
what is actually going on.

So I would just update
drm_fb_cma_fbdev_init() and drm_fb_helper_initial_config()
to pass a reasonable pixel format instead and refactor all the
way down?

It does hit a lot of code on the way, but if everyone thinks this
is a good idea I can very well take a stab at it.

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

[Bug 100069] Dirt: Showdown bad performance with enabled advanced lightning

[bugzilla-daemon]

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

--- Comment #4 from Gregor Münch  ---
Created attachment 137129
  --> https://bugs.freedesktop.org/attachment.cgi?id=137129=edit
Ingame

Looks like this is one of the last games which have pretty garbled graphics
with mesa.
What is needed to fix this?

-- 
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: clang warning: implicit conversion in intel_ddi.c:1481

[Greg KH]

On Fri, Feb 02, 2018 at 12:44:38PM +0200, Jani Nikula wrote:
> 
> +Knut, Fengguang
> 
> On Fri, 02 Feb 2018, Greg KH  wrote:
> > - If clang now builds the kernel "cleanly", yes, I want to take
> >   warning fixes in the stable tree.  And even better yet, if you
> >   keep working to ensure the tree is "clean", that would be
> >   wonderful.
> 
> So we can run sparse using 'make C=1' and friends, or other static
> analysis tools using 'make CHECK=foo C=1', as long as the passed command
> line params work. There was work by Knut to extend this make checker
> stuff [1]. Since mixing different HOSTCC's in a single workdir seems
> like a bad idea, I wonder how hard it would be to make clang work like
> this:
> 
> $ make CHECK=clang C=1
> 
> Or using Knut's wrapper. Feels like that could increase the use of clang
> for static analysis of patches.

Why not just build with clang itself:
make CC=clang

thanks,

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

[PATCH] drm/amd/display: fix incompatible structure layouts

[Arnd Bergmann]

Building the amd display driver with link-time optimizations revealed a bug
that caused dal_cmd_tbl_helper_dce80_get_table() and
dal_cmd_tbl_helper_dce110_get_table() get called with an incompatible
return type between the two callers in command_table_helper.c and
command_table_helper2.c:

drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce80/command_table_helper_dce80.h:31:
 error: type of 'dal_cmd_tbl_helper_dce80_get_table' does not match original 
declaration [-Werror=lto-type-mismatch]
 const struct command_table_helper *dal_cmd_tbl_helper_dce80_get_table(void);

drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce80/command_table_helper_dce80.c:351:
 note: 'dal_cmd_tbl_helper_dce80_get_table' was previously declared here
 const struct command_table_helper *dal_cmd_tbl_helper_dce80_get_table(void)

drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce110/command_table_helper_dce110.h:32:
 error: type of 'dal_cmd_tbl_helper_dce110_get_table' does not match original 
declaration [-Werror=lto-type-mismatch]
 const struct command_table_helper *dal_cmd_tbl_helper_dce110_get_table(void);

drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce110/command_table_helper_dce110.c:361:
 note: 'dal_cmd_tbl_helper_dce110_get_table' was previously declared here
 const struct command_table_helper *dal_cmd_tbl_helper_dce110_get_table(void)

The two versions of the structure are obviously derived from the same
one, but have diverged over time, before they got added to the kernel.

This moves the structure to a new shared header file and uses the superset
of the members, to ensure the interfaces are all compatible.

Fixes: ae79c310b1a6 ("drm/amd/display: Add DCE12 bios parser support")
Signed-off-by: Arnd Bergmann 
---
 .../drm/amd/display/dc/bios/command_table_helper.h | 33 +--
 .../amd/display/dc/bios/command_table_helper2.h| 30 +-
 .../display/dc/bios/command_table_helper_struct.h  | 66 ++
 3 files changed, 68 insertions(+), 61 deletions(-)
 create mode 100644 
drivers/gpu/drm/amd/display/dc/bios/command_table_helper_struct.h

diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table_helper.h 
b/drivers/gpu/drm/amd/display/dc/bios/command_table_helper.h
index 1fab634b66be..4c3789df253d 100644
--- a/drivers/gpu/drm/amd/display/dc/bios/command_table_helper.h
+++ b/drivers/gpu/drm/amd/display/dc/bios/command_table_helper.h
@@ -29,38 +29,7 @@
 #include "dce80/command_table_helper_dce80.h"
 #include "dce110/command_table_helper_dce110.h"
 #include "dce112/command_table_helper_dce112.h"
-
-struct command_table_helper {
-   bool (*controller_id_to_atom)(enum controller_id id, uint8_t *atom_id);
-   uint8_t (*encoder_action_to_atom)(
-   enum bp_encoder_control_action action);
-   uint32_t (*encoder_mode_bp_to_atom)(enum signal_type s,
-   bool enable_dp_audio);
-   bool (*engine_bp_to_atom)(enum engine_id engine_id,
-   uint32_t *atom_engine_id);
-   void (*assign_control_parameter)(
-   const struct command_table_helper *h,
-   struct bp_encoder_control *control,
-   DIG_ENCODER_CONTROL_PARAMETERS_V2 *ctrl_param);
-   bool (*clock_source_id_to_atom)(enum clock_source_id id,
-   uint32_t *atom_pll_id);
-   bool (*clock_source_id_to_ref_clk_src)(
-   enum clock_source_id id,
-   uint32_t *ref_clk_src_id);
-   uint8_t (*transmitter_bp_to_atom)(enum transmitter t);
-   uint8_t (*encoder_id_to_atom)(enum encoder_id id);
-   uint8_t (*clock_source_id_to_atom_phy_clk_src_id)(
-   enum clock_source_id id);
-   uint8_t (*signal_type_to_atom_dig_mode)(enum signal_type s);
-   uint8_t (*hpd_sel_to_atom)(enum hpd_source_id id);
-   uint8_t (*dig_encoder_sel_to_atom)(enum engine_id engine_id);
-   uint8_t (*phy_id_to_atom)(enum transmitter t);
-   uint8_t (*disp_power_gating_action_to_atom)(
-   enum bp_pipe_control_action action);
-   bool (*dc_clock_type_to_atom)(enum bp_dce_clock_type id,
-   uint32_t *atom_clock_type);
-uint8_t (*transmitter_color_depth_to_atom)(enum transmitter_color_depth 
id);
-};
+#include "command_table_helper_struct.h"
 
 bool dal_bios_parser_init_cmd_tbl_helper(const struct command_table_helper **h,
enum dce_version dce);
diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table_helper2.h 
b/drivers/gpu/drm/amd/display/dc/bios/command_table_helper2.h
index 9f587c91d843..785fcb20a1b9 100644
--- a/drivers/gpu/drm/amd/display/dc/bios/command_table_helper2.h
+++ b/drivers/gpu/drm/amd/display/dc/bios/command_table_helper2.h
@@ -29,35 +29,7 @@
 #include "dce80/command_table_helper_dce80.h"
 #include "dce110/command_table_helper_dce110.h"
 #include "dce112/command_table_helper2_dce112.h"
-
-struct command_table_helper {
-   bool 

Re: [PATCH 1/2] fbdev: don't select I2C directly

[Arnd Bergmann]

On Fri, Feb 2, 2018 at 1:21 AM, Randy Dunlap  wrote:
> On 02/01/2018 08:14 AM, Bartlomiej Zolnierkiewicz wrote:
>> On Monday, January 15, 2018 05:14:04 PM Arnd Bergmann wrote:
>>> Using a Kconfig 'select' statement for a user-visible symbol that other
>>> drivers depend on often causes circular dependencies. A new one showed
>>> up when I wanted to add an NVMEM dependency to the DRM_MSM driver:
>>>
>>> drivers/i2c/Kconfig:7:error: recursive dependency detected!
>>> drivers/i2c/Kconfig:7:   symbol I2C is selected by FB_DDC
>>> drivers/video/fbdev/Kconfig:63:  symbol FB_DDC is selected by 
>>> FB_CYBER2000_DDC
>>> drivers/video/fbdev/Kconfig:390: symbol FB_CYBER2000_DDC depends on 
>>> FB_CYBER2000
>>> drivers/video/fbdev/Kconfig:378: symbol FB_CYBER2000 depends on FB
>>> drivers/video/fbdev/Kconfig:5:   symbol FB is selected by 
>>> DRM_KMS_FB_HELPER
>>> drivers/gpu/drm/Kconfig:77:  symbol DRM_KMS_FB_HELPER depends on 
>>> DRM_KMS_HELPER
>>> drivers/gpu/drm/Kconfig:71:  symbol DRM_KMS_HELPER is selected by DRM_MSM
>>> drivers/gpu/drm/msm/Kconfig:2:   symbol DRM_MSM depends on NVMEM
>>> drivers/nvmem/Kconfig:1: symbol NVMEM is selected by EEPROM_AT24
>>> drivers/misc/eeprom/Kconfig:3:   symbol EEPROM_AT24 depends on I2C
>>>
>>> Here, the problem is that many fbdev drivers have an i2c based interface
>>> and just 'select i2c' for that, while we also select the framebuffer
>>> subsystem indirectly from a DRM driver that now depends on i2c.
>>>
>>> This does away with the 'select' statement and instead uses 'depends on',
>>> like almost all I2C users.
>>
>> I worry that this change may cause various driver options to no longer
>> be visible to people configuring their kernels and not having I2C already
>> selected.
>>
>> DRM somehow manages to select I2C and I would prefer to be it the same
>> way for fbdev (also looking at the current next tree there are still
>> some drivers that 'select I2C')..
>
> a. Linus has stated that a driver should not enable an entire subsystem,
>so depends on would be better than select.
>If I had the email/patch, I would be glad to Ack it.
>
> b. DRM configuration is a mess. You shouldn't want to follow their model. :)

Right, that should also be fixed, so DRM no longer includes I2C ;-)

At the moment, DRM is the most common cause for circular dependencies
because it has a number of 'select' statements for symbols that otherwise
are used with 'depends on'. We should probably address the 'select I2C'
portion in there, but also some of the others like:

drivers/gpu/drm/Kconfig:select POWER_SUPPLY
drivers/gpu/drm/Kconfig:select HWMON
drivers/gpu/drm/Kconfig:select FB
drivers/gpu/drm/udl/Kconfig: select USB
drivers/gpu/drm/sti/Kconfig: select OF
drivers/gpu/drm/sti/Kconfig: select RESET_CONTROLLER
drivers/gpu/drm/etnaviv/Kconfig:select CMA if HAVE_DMA_CONTIGUOUS
drivers/gpu/drm/etnaviv/Kconfig:select TMPFS
drivers/gpu/drm/i915/Kconfig.debug:select DEBUG_FS
drivers/gpu/drm/i915/Kconfig.debug:select PREEMPT_COUNT
drivers/gpu/drm/i915/Kconfig.debug: select TRACING
drivers/gpu/drm/i915/Kconfig.debug: select FAULT_INJECTION
drivers/gpu/drm/mediatek/Kconfig:   select MEMORY
drivers/gpu/drm/mediatek/Kconfig:   select GENERIC_PHY
drivers/gpu/drm/msm/Kconfig:select REGULATOR

> c. If I2C is not enabled in the FB menu, someone could just add something 
> like this:
>
> comment "Enable I2C to see more driver choices"
> depends on !I2C

I don't think that would address the issue of 'defconfig' builds losing
I2C support when it's no longer automatically selection. On x86, this
is not an issue, as X86 always enables I2C. For the rest, we could
do a hack like this:

--- a/drivers/i2c/Kconfig
+++ b/drivers/i2c/Kconfig
@@ -8,6 +8,7 @@ config I2C
tristate "I2C support"
select RT_MUTEXES
select IRQ_DOMAIN
+   default DRM || FB
---help---
  I2C (pronounce: I-squared-C) is a slow serial bus protocol used in
  many micro controller applications and developed by Philips.  SMBus,

which would let all 'defconfig' versions keep working. It's a bit ugly, but
at least wouldn't cause other circular dependencies.

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

Re: [PATCH v6 4/6] iommu/arm-smmu: Add the device_link between masters and smmu

[Robin Murphy]

On 02/02/18 05:40, Sricharan R wrote:

Hi Robin/Vivek,

On 2/1/2018 2:23 PM, Vivek Gautam wrote:

Hi,


On 1/31/2018 6:39 PM, Robin Murphy wrote:

On 19/01/18 11:43, Vivek Gautam wrote:

From: Sricharan R 

Finally add the device link between the master device and
smmu, so that the smmu gets runtime enabled/disabled only when the
master needs it. This is done from add_device callback which gets
called once when the master is added to the smmu.


Don't we need to balance this with a device_link_del() in .remove_device (like 
exynos-iommu does)?


Right. Will add device_link_del() call. Thanks for pointing out.


  The reason for not adding device_link_del from .remove_device was, the core 
device_del
  which calls the .remove_device from notifier, calls device_links_purge before 
that.
  That does the same thing as device_link_del. So by the time .remove_device is 
called,
  device_links for that device is already cleaned up. Vivek, you may want to 
check once that
  calling device_link_del from .remove_device has no effect, just to confirm 
once more.


There is at least one path in which .remove_device is not called via the 
notifier from device_del(), which is in the cleanup path of 
iommu_bus_init(). AFAICS any links created by .add_device during that 
process would be left dangling, because the device(s) would be live but 
otherwise disassociated from the IOMMU afterwards.


From a maintenance perspective it's easier to have the call in its 
logical place even if it does nothing 99% of the time; that way we 
shouldn't have to keep an eye out for subtle changes in the power 
management code or driver core that might invalidate the device_del() 
reasoning above, and the power management guys shouldn't have to 
comprehend the internals of the IOMMU API to make sense of the 
unbalanced call if they ever want to change their API.


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

Re: clang warning: implicit conversion in intel_ddi.c:1481

[Jani Nikula]

+Knut, Fengguang

On Fri, 02 Feb 2018, Greg KH  wrote:
>   - If clang now builds the kernel "cleanly", yes, I want to take
> warning fixes in the stable tree.  And even better yet, if you
> keep working to ensure the tree is "clean", that would be
> wonderful.

So we can run sparse using 'make C=1' and friends, or other static
analysis tools using 'make CHECK=foo C=1', as long as the passed command
line params work. There was work by Knut to extend this make checker
stuff [1]. Since mixing different HOSTCC's in a single workdir seems
like a bad idea, I wonder how hard it would be to make clang work like
this:

$ make CHECK=clang C=1

Or using Knut's wrapper. Feels like that could increase the use of clang
for static analysis of patches.


BR,
Jani.


[1] 
http://mid.mail-archive.com/cover.5b56d020b8e826a7da33b1823c059acd0c123f8b.1515072782.git-series.knut.omang@oracle.com



-- 
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 v2] drm/bridge/sii8620: fix display modes validation

[Maciej Purski]

Current implementation of mode_valid() and mode_fixup() callbacks
handle packed pixel modes improperly.

Fix it by using proper maximum clock values from the documentation.

Signed-off-by: Maciej Purski 

---
Changes in v2:
- simplify is_packing_required() function
- fix uninitialized variable detected by kbuild robot in mode_valid()
---
 drivers/gpu/drm/bridge/sil-sii8620.c | 81 +++-
 1 file changed, 42 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c 
b/drivers/gpu/drm/bridge/sil-sii8620.c
index 5168783..912d8c2 100644
--- a/drivers/gpu/drm/bridge/sil-sii8620.c
+++ b/drivers/gpu/drm/bridge/sil-sii8620.c
@@ -34,8 +34,11 @@
 
 #define SII8620_BURST_BUF_LEN 288
 #define VAL_RX_HDMI_CTRL2_DEFVAL VAL_RX_HDMI_CTRL2_IDLE_CNT(3)
-#define MHL1_MAX_LCLK 225000
-#define MHL3_MAX_LCLK 60
+
+#define MHL1_MAX_PCLK 75000
+#define MHL1_MAX_PCLK_PP_MODE 15
+#define MHL3_MAX_PCLK 20
+#define MHL3_MAX_PCLK_PP_MODE 30
 
 enum sii8620_mode {
CM_DISCONNECTED,
@@ -2123,61 +2126,61 @@ static void sii8620_detach(struct drm_bridge *bridge)
rc_unregister_device(ctx->rc_dev);
 }
 
+static int sii8620_is_packing_required(struct sii8620 *ctx,
+const struct drm_display_mode *mode)
+{
+   int max_pclk, max_pclk_pp_mode;
+
+   if (sii8620_is_mhl3(ctx)) {
+   max_pclk = MHL3_MAX_PCLK;
+   max_pclk_pp_mode = MHL3_MAX_PCLK_PP_MODE;
+   } else {
+   max_pclk = MHL1_MAX_PCLK;
+   max_pclk_pp_mode = MHL1_MAX_PCLK_PP_MODE;
+   }
+
+   if (mode->clock < max_pclk)
+   return 0;
+   else if (mode->clock < max_pclk_pp_mode)
+   return 1;
+   else
+   return -1;
+}
+
 static enum drm_mode_status sii8620_mode_valid(struct drm_bridge *bridge,
 const struct drm_display_mode *mode)
 {
struct sii8620 *ctx = bridge_to_sii8620(bridge);
+   int pack_required = sii8620_is_packing_required(ctx, mode);
bool can_pack = ctx->devcap[MHL_DCAP_VID_LINK_MODE] &
MHL_DCAP_VID_LINK_PPIXEL;
-   unsigned int max_pclk = sii8620_is_mhl3(ctx) ? MHL3_MAX_LCLK :
-  MHL1_MAX_LCLK;
-   max_pclk /= can_pack ? 2 : 3;
 
-   return (mode->clock > max_pclk) ? MODE_CLOCK_HIGH : MODE_OK;
+   switch (pack_required) {
+   case 0:
+   return MODE_OK;
+   case 1:
+   return (can_pack) ? MODE_OK : MODE_CLOCK_HIGH;
+   default:
+   return MODE_CLOCK_HIGH;
+   }
 }
 
+
 static bool sii8620_mode_fixup(struct drm_bridge *bridge,
   const struct drm_display_mode *mode,
   struct drm_display_mode *adjusted_mode)
 {
struct sii8620 *ctx = bridge_to_sii8620(bridge);
-   int max_lclk;
-   bool ret = true;
 
mutex_lock(>lock);
 
-   max_lclk = sii8620_is_mhl3(ctx) ? MHL3_MAX_LCLK : MHL1_MAX_LCLK;
-   if (max_lclk > 3 * adjusted_mode->clock) {
-   ctx->use_packed_pixel = 0;
-   goto end;
-   }
-   if ((ctx->devcap[MHL_DCAP_VID_LINK_MODE] & MHL_DCAP_VID_LINK_PPIXEL) &&
-   max_lclk > 2 * adjusted_mode->clock) {
-   ctx->use_packed_pixel = 1;
-   goto end;
-   }
-   ret = false;
-end:
-   if (ret) {
-   u8 vic = drm_match_cea_mode(adjusted_mode);
-
-   if (!vic) {
-   union hdmi_infoframe frm;
-   u8 mhl_vic[] = { 0, 95, 94, 93, 98 };
-
-   /* FIXME: We need the connector here */
-   drm_hdmi_vendor_infoframe_from_display_mode(
-   , NULL, adjusted_mode);
-   vic = frm.vendor.hdmi.vic;
-   if (vic >= ARRAY_SIZE(mhl_vic))
-   vic = 0;
-   vic = mhl_vic[vic];
-   }
-   ctx->video_code = vic;
-   ctx->pixel_clock = adjusted_mode->clock;
-   }
+   ctx->use_packed_pixel = sii8620_is_packing_required(ctx, adjusted_mode);
+   ctx->video_code = drm_match_cea_mode(adjusted_mode);
+   ctx->pixel_clock = adjusted_mode->clock;
+
mutex_unlock(>lock);
-   return ret;
+
+   return true;
 }
 
 static const struct drm_bridge_funcs sii8620_bridge_funcs = {
-- 
2.7.4

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

Re: clang warning: implicit conversion in intel_ddi.c:1481

[Greg KH]

On Fri, Feb 02, 2018 at 10:56:36AM +0100, Lukas Bulwahn wrote:
> On Fri, 2 Feb 2018, Jani Nikula wrote:
> 
> > Being brutally honest, please write shorter reports and shorter emails
> > to the lists.
> > 
> > The static analysis reports are welcome, but only when 1) we didn't
> > already fix it in linux-next, or 2) it reveals an actual bug, not just a
> > warning, warranting a backport.
> 
> That will be our policy.

Great!

Also a few other things to be aware of when working with the Linux
kernel community, and to try to answer some of your longer original
email querstions:

- don't scatter emails to tons of lists at the same time.  If
  you use scripts/get_maintainer.pl on a file, it will tell you
  exactly who and what list to notify of an issue found.

- when finding an issue, again, always check linux-next as that
  contains up to the past 3 months of work.  Don't duplicate
  stuff that others have already done, as that doesn't help
  anyone out.

- for stable kernel patches, please read:

https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
  as to how to report a patch to be included in a stable kernel
  release (hint, just send the git hash to the list when it is
  in Linus's tree and ask for it to be included, as well as what
  trees you think it should be included in.)

- If clang now builds the kernel "cleanly", yes, I want to take
  warning fixes in the stable tree.  And even better yet, if you
  keep working to ensure the tree is "clean", that would be
  wonderful.

Hope this helps!

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

Re: Enabling i915 Panel Self Refresh by default on some devices ?

[Hans de Goede]

Hi,

On 01-02-18 13:31, Hans de Goede wrote:

Hi All,

As you may have heard I've recently been working on improving
Linux laptop battery life, specifically the OOTB experience
without tweaking any options such as e.g. powertop --auto-tune
would do, see:

https://fedoraproject.org/wiki/Changes/ImprovedLaptopBatteryLife

So far this is going quite nicely, it looks like Fedora 28
will have SATA ALPM (big win), autosuspend of USB Bluetooth HCIs
and snd_intel_hda powersaving all enabled OOTB.

Looking for more savings I've run some quick tests with
i915.enable_psr=1, this seems to be another nice win (for an idle
system) of aprox. 0.5W.

So as with the other 3 items I just mentioned I'm now looking into
somehow enabling this be default, at least one some models.

Currently I'm thinking doing a whitelist or blacklist (*) for this,
but first I think we need some more data about on how much models
this just works and where it is causing issues, as such I've done
a blog post to gather this data:

https://hansdegoede.livejournal.com/18653.html

So I will revisit this eventually, once people have had some time
to respond to this blog-post.

In the mean time I wonder if anyone can explain why this options
is currently disabled by default. E.g. are there any known specific
models laptops / panels which are causing issues, are the bugzillas
for this? Etc. ?

Also does anyone know if any problems are mainly panel or laptop
model specific ? I would expect this to mostly be panel specific
and not depend on the model laptop (given then certain models
ship with different panels over their production lifetime).

Regards,

Hans

p.s.

If anyone on this list can make 10 minutes to run the tests
described in my blogpost and gather the data mentioned there, then
that would be great.


*) I know that maintaining such a white/blacklist in the kernel
is going to be a pain, so my current thinking on this is to make
this runtime configurable through a sysfs attribute and then
use a udev rule + udev hwdb entries for this.


So a quick update on this. The response has been quite overwhelming,
with well over 50 test-reports received sofar. The results are all
over the place, some people see no changes, some people report the
aprox. 0.5W saving my own test show and many people also report
display problems, sometimes combined with a significant increase in
power-consumption.

I need to take a closer look at all the results, but right now I
believe that the best way forward with this is (unfortunately) a
whitelist matching on a combination of panel-id (from edid) and
dmi data, so that we can at least enable this on popular models
(any model with atleast one user willing to contribute).

So intel-gfx-team folks any input from your side, any feedback
on the plan to use a udev rule + udev hwdb entries to build a
whitelist for this?

Regards,

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

Re: [PATCH] drm/panel: lvds: tidyup header explanation

[Laurent Pinchart]

Hi Morimoto-san,

Thank you for your patch.

On Thursday, 1 February 2018 09:45:36 EET Kuninori Morimoto wrote:
> From: Kuninori Morimoto 
> 
> panel-lvds.c is for LVDS Panel Driver,
> not R-Car Display Unit CRTCs
> 
> Reported-by: Koji Matsuoka 
> Signed-off-by: Kuninori Morimoto 

A similar patch has been posted already:

https://patchwork.freedesktop.org/series/36953/

> ---
>  drivers/gpu/drm/panel/panel-lvds.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-lvds.c
> b/drivers/gpu/drm/panel/panel-lvds.c index b5e3994..c8075a8 100644
> --- a/drivers/gpu/drm/panel/panel-lvds.c
> +++ b/drivers/gpu/drm/panel/panel-lvds.c
> @@ -1,5 +1,5 @@
>  /*
> - * rcar_du_crtc.c  --  R-Car Display Unit CRTCs
> + * panel-lvds.c  --  LVDS Panel Driver
>   *
>   * Copyright (C) 2016 Laurent Pinchart
>   * Copyright (C) 2016 Renesas Electronics Corporation


-- 
Regards,

Laurent Pinchart

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

Re: [PATCH] drm: panels: lvds: fix driver description heading

[Laurent Pinchart]

Hi Baruch,

Thank you for the patch.

On Monday, 22 January 2018 18:01:42 EET Baruch Siach wrote:

A commit message is usually required. This patch is so simple that I'm fine 
with it as-is though.

> Cc: Laurent Pinchart 
> Signed-off-by: Baruch Siach 

Reviewed-by: Laurent Pinchart 

Thierry, could you take this in your tree ?

> ---
>  drivers/gpu/drm/panel/panel-lvds.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-lvds.c
> b/drivers/gpu/drm/panel/panel-lvds.c index e2d57c01200b..e651bf07df9a
> 100644
> --- a/drivers/gpu/drm/panel/panel-lvds.c
> +++ b/drivers/gpu/drm/panel/panel-lvds.c
> @@ -1,5 +1,5 @@
>  /*
> - * rcar_du_crtc.c  --  R-Car Display Unit CRTCs
> + * Generic LVDS panel driver
>   *
>   * Copyright (C) 2016 Laurent Pinchart
>   * Copyright (C) 2016 Renesas Electronics Corporation

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH] drm/amdgpu/display: fix wrong enum type for ddc_result

[Michel Dänzer]

Adding the amd-gfx list, please always send amdgpu patches there.


On 2018-02-02 02:55 AM, db...@chromium.org wrote:
> From: Dominik Behr 
> 
> v2: now with fixed result comparison and spelling fixes
> 
> Signed-off-by: Dominik Behr 
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 +-
>  drivers/gpu/drm/amd/display/dc/core/dc_link.c   | 2 +-
>  drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c   | 4 ++--
>  drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c| 4 ++--
>  drivers/gpu/drm/amd/display/include/ddc_service_types.h | 2 +-
>  5 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> index 1e8a21b67df7..3b05900d 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> @@ -130,7 +130,7 @@ static ssize_t dm_dp_aux_transfer(struct drm_dp_aux *aux,
>msg->address,
>msg->buffer,
>msg->size,
> -  r == DDC_RESULT_SUCESSFULL);
> +  r == DDC_RESULT_SUCCESSFUL);
>  #endif
>  
>   return msg->size;
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c 
> b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> index 0023754e034b..3abd0f1a287f 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> @@ -1176,7 +1176,7 @@ static void dpcd_configure_panel_mode(
>   _config_set.raw,
>   sizeof(edp_config_set.raw));
>  
> - ASSERT(result == DDC_RESULT_SUCESSFULL);
> + ASSERT(result == DDC_RESULT_SUCCESSFUL);
>   }
>   }
>   dm_logger_write(link->ctx->logger, LOG_DETECTION_DP_CAPS,
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c 
> b/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
> index d5294798b0a5..6b69b339dba2 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
> @@ -661,7 +661,7 @@ enum ddc_result dal_ddc_service_read_dpcd_data(
>   ddc->ctx->i2caux,
>   ddc->ddc_pin,
>   ))
> - return DDC_RESULT_SUCESSFULL;
> + return DDC_RESULT_SUCCESSFUL;
>  
>   return DDC_RESULT_FAILED_OPERATION;
>  }
> @@ -698,7 +698,7 @@ enum ddc_result dal_ddc_service_write_dpcd_data(
>   ddc->ctx->i2caux,
>   ddc->ddc_pin,
>   ))
> - return DDC_RESULT_SUCESSFULL;
> + return DDC_RESULT_SUCCESSFUL;
>  
>   return DDC_RESULT_FAILED_OPERATION;
>  }
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c 
> b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> index 33d91e4474ea..cc067d04505d 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> @@ -1926,7 +1926,7 @@ bool dc_link_handle_hpd_rx_irq(struct dc_link *link, 
> union hpd_irq_data *out_hpd
>  {
>   union hpd_irq_data hpd_irq_dpcd_data = 0;
>   union device_service_irq device_service_clear = { { 0 } };
> - enum dc_status result = DDC_RESULT_UNKNOWN;
> + enum ddc_result result = DDC_RESULT_UNKNOWN;
>   bool status = false;
>   /* For use cases related to down stream connection status change,
>* PSR and device auto test, refer to function handle_sst_hpd_irq
> @@ -1946,7 +1946,7 @@ bool dc_link_handle_hpd_rx_irq(struct dc_link *link, 
> union hpd_irq_data *out_hpd
>   if (out_hpd_irq_dpcd_data)
>   *out_hpd_irq_dpcd_data = hpd_irq_dpcd_data;
>  
> - if (result != DC_OK) {
> + if (result != DDC_RESULT_SUCCESSFUL) {
>   dm_logger_write(link->ctx->logger, LOG_HW_HPD_IRQ,
>   "%s: DPCD read failed to obtain irq data\n",
>   __func__);
> diff --git a/drivers/gpu/drm/amd/display/include/ddc_service_types.h 
> b/drivers/gpu/drm/amd/display/include/ddc_service_types.h
> index 019e7a095ea1..f3bf749b3636 100644
> --- a/drivers/gpu/drm/amd/display/include/ddc_service_types.h
> +++ b/drivers/gpu/drm/amd/display/include/ddc_service_types.h
> @@ -32,7 +32,7 @@
>  
>  enum ddc_result {
>   DDC_RESULT_UNKNOWN = 0,
> - DDC_RESULT_SUCESSFULL,
> + DDC_RESULT_SUCCESSFUL,
>   DDC_RESULT_FAILED_CHANNEL_BUSY,
>   DDC_RESULT_FAILED_TIMEOUT,
>   DDC_RESULT_FAILED_PROTOCOL_ERROR,
> 


-- 
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: [PATCH] mm/swap: add function get_total_swap_pages to expose total_swap_pages

[He, Roger]

Probably it is necessary to summarize, and I have done below experiments:

A: use a fixed limit to stopping swapping out as below:
 int ttm_bo_swapout(struct ttm_bo_global *glob, struct ttm_operation_ctx 
*ctx)   {
if get_nr_swap_pages() < 256MB  return no memory;

 }
 Set the value as 256MB not work on my platform which has 8GB system memory 
& 8GB swap disk.
 Set it as 4GB can pass, but 4GB not work on test machine with 16GB system 
memory & 8GB swap disk.
 So set the limit as fixed value doesn't work always.

B. use the fixed limit as scaling with system memory, something as below:
 int ttm_bo_swapout(struct ttm_bo_global *glob, struct ttm_operation_ctx 
*ctx)   {
if get_nr_swap_pages() < 1/2 * system memory size;

 }
 So far, it can work on all test machine.
 But it has an obvious defect.
 For example,  if the platform has 32G system memory & 8G swap disk.
 1/2 * ram = 16G which is bigger than swap disk, so TTM swap for TTM is 
disallowed even when swap disk is empty at the start.
 So seems this approach is not reasonable. 

C. Then we work out new patches as in mailist today.


Thanks
Roger(Hongbo.He)
-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of 
He, Roger
Sent: Friday, February 02, 2018 3:54 PM
To: Koenig, Christian ; Zhou, David(ChunMing) 
; dri-devel@lists.freedesktop.org
Cc: linux...@kvack.org; linux-ker...@vger.kernel.org
Subject: RE: [PATCH] mm/swap: add function get_total_swap_pages to expose 
total_swap_pages

Can you try to use a fixed limit like I suggested once more?
E.g. just stop swapping if get_nr_swap_pages() < 256MB.

Maybe you missed previous mail. I explain again here.
Set the value as 256MB not work on my platform.  My machine has 8GB system 
memory, and 8GB swap disk.
On my machine, set it as 4G can work.
But 4G also not work on test machine with 16GB system memory & 8GB swap disk.


Thanks
Roger(Hongbo.He)

-Original Message-
From: Koenig, Christian
Sent: Friday, February 02, 2018 3:46 PM
To: He, Roger ; Zhou, David(ChunMing) ; 
dri-devel@lists.freedesktop.org
Cc: linux...@kvack.org; linux-ker...@vger.kernel.org
Subject: Re: [PATCH] mm/swap: add function get_total_swap_pages to expose 
total_swap_pages

Can you try to use a fixed limit like I suggested once more?

E.g. just stop swapping if get_nr_swap_pages() < 256MB.

Regards,
Christian.

Am 02.02.2018 um 07:57 schrieb He, Roger:
>   Use the limit as total ram*1/2 seems work very well.
>   No OOM although swap disk reaches full at peak for piglit test.
>
> But for this approach, David noticed that has an obvious defect.
> For example,  if the platform has 32G system memory, 8G swap disk.
> 1/2 * ram = 16G which is bigger than swap disk, so no swap for TTM is allowed 
> at all.
> For now we work out an improved version based on get_nr_swap_pages().
> Going to send out later.
>
> Thanks
> Roger(Hongbo.He)
> -Original Message-
> From: He, Roger
> Sent: Thursday, February 01, 2018 4:03 PM
> To: Koenig, Christian ; Zhou,
> David(ChunMing) ; dri-devel@lists.freedesktop.org
> Cc: linux...@kvack.org; linux-ker...@vger.kernel.org; 'He, Roger' 
> 
> Subject: RE: [PATCH] mm/swap: add function get_total_swap_pages to 
> expose total_swap_pages
>
> Just now, I tried with fixed limit.  But not work always.
> For example: set the limit as 4GB on my platform with 8GB system memory, it 
> can pass.
> But when run with platform with 16GB system memory, it failed since OOM.
>
> And I guess it also depends on app's behavior.
> I mean some apps  make OS to use more swap space as well.
>
> Thanks
> Roger(Hongbo.He)
> -Original Message-
> From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On 
> Behalf Of He, Roger
> Sent: Thursday, February 01, 2018 1:48 PM
> To: Koenig, Christian ; Zhou,
> David(ChunMing) ; dri-devel@lists.freedesktop.org
> Cc: linux...@kvack.org; linux-ker...@vger.kernel.org
> Subject: RE: [PATCH] mm/swap: add function get_total_swap_pages to 
> expose total_swap_pages
>
>   But what we could do is to rely on a fixed limit like the Intel driver 
> does and I suggested before.
>   E.g. don't copy anything into a shmemfile when there is only x MB of 
> swap space left.
>
> Here I think we can do it further, let the limit value scaling with total 
> system memory.
> For example: total system memory * 1/2.
> If that it will match the platform configuration better.
>
>   Roger can you test that approach once more with your fix for the OOM 
> issues in the page fault handler?
>
> Sure. Use the limit as total ram*1/2 seems work very well.
> No OOM although swap disk reaches full at peak for piglit test.
> I speculate this 

Re: [drm_hwcomposer] [PATCH] Take Connection state into account. (v2)

[Daniel Vetter]

On Mon, Jan 8, 2018 at 9:46 PM, Sean Paul  wrote:
> On Mon, Jan 08, 2018 at 03:41:49PM -0500, Sean Paul wrote:
>> On Sat, Jan 06, 2018 at 12:59:58AM +0100, Mauro Rossi wrote:
>> > Porting of original commit 76fb87e675 of Jim Bish in android-ia master to 
>> > fdo
>> >
>> > Original commit message:
>> > "There are various places where we should be really taking connection
>> > state into account before querying the properties or assuming it
>> > as primary. This patch fixes them."
>> >
>> > (v2) checks on connection state are applied for both internal and external
>> > connectors, in order to select the correct primary, as opposed to setting,
>> > independently from its state, the first connector as primary
>> >
>> > This is essential to avoid following logcat errors on integrated and 
>> > dedicated GPUs:
>> >
>> > ... 2245  2245 E hwc-drm-resources: Could not find a suitable encoder/crtc 
>> > for display 2
>> > ... 2245  2245 E hwc-drm-resources: Failed CreateDisplayPipe 56 with -19
>> > ... 2245  2245 E hwcomposer-drm: Can't initialize Drm object -19
>> >
>> > Tested with i965 on Sandybridge and nouveau on GT120, GT610
>> > ---
>> >  drmresources.cpp | 9 +++--
>> >  1 file changed, 7 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drmresources.cpp b/drmresources.cpp
>> > index 32dd376..d582cfe 100644
>> > --- a/drmresources.cpp
>> > +++ b/drmresources.cpp
>> > @@ -159,7 +159,7 @@ int DrmResources::Init() {
>> >
>> >// First look for primary amongst internal connectors
>> >for (auto  : connectors_) {
>> > -if (conn->internal() && !found_primary) {
>> > +if (conn->state() == DRM_MODE_CONNECTED && conn->internal() && 
>> > !found_primary) {
>
> One more thing. How do you know this is the right thing to do? What if the
> internal panel is not connected initially, but becomes connected in the 
> future?
> IIUC, you'll end up numbering it incorrectly.

I'm not sure how consistent it all is, but the internal panel can
report as disconnected when e.g. the lid is closed, on at least some
drivers. So sounds like a real scenario to me.

If the panel isn't even there, then the driver shouldn't even register
the connector (or it's a driver bug). So maybe just don't filter the
internal connectors?
-Daniel

>
> Sean
>
>> >conn->set_display(0);
>> >found_primary = true;
>> >  } else {
>> > @@ -170,7 +170,7 @@ int DrmResources::Init() {
>> >
>> >// Then look for primary amongst external connectors
>> >for (auto  : connectors_) {
>> > -if (conn->external() && !found_primary) {
>> > +if (conn->state() == DRM_MODE_CONNECTED && conn->external() && 
>> > !found_primary) {
>>
>> These 2 lines exceed the max character limit. Did you run clang-format?
>>
>> Anyways, I think it'd be nice to add a connected() helper to the connector
>> object which would look cleaner and solve the long lines.

>> Sean
>>
>> >conn->set_display(0);
>> >found_primary = true;
>> >  }
>> > @@ -288,6 +288,11 @@ int DrmResources::TryEncoderForDisplay(int display, 
>> > DrmEncoder *enc) {
>> >
>> >  int DrmResources::CreateDisplayPipe(DrmConnector *connector) {
>> >int display = connector->display();
>> > +
>> > +  // skip not connected
>> > +  if (connector->state() == DRM_MODE_DISCONNECTED)
>> > +return 0;
>> > +
>> >/* Try to use current setup first */
>> >if (connector->encoder()) {
>> >  int ret = TryEncoderForDisplay(display, connector->encoder());
>> > --
>> > 2.14.1
>> >
>> > ___
>> > dri-devel mailing list
>> > dri-devel@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>> --
>> Sean Paul, Software Engineer, Google / Chromium OS
>
> --
> Sean Paul, Software Engineer, Google / Chromium OS
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: clang warning: implicit conversion in intel_ddi.c:1481

[Jani Nikula]

On Thu, 01 Feb 2018, Lukas Bulwahn  wrote:
> Hi Greg,
>
> On Thu, 1 Feb 2018, Greg KH wrote:
>
>> On Thu, Feb 01, 2018 at 06:33:30PM +0100, Ozan Alpay wrote:
>> > Dear Rodrigo Vivi, Ville Syrjälä,
>> > 
>> > My name is Ozan Alpay, and I am a student mentored by Lukas Bulwahn. We 
>> > intend to use static analysis tools on the kernel source to identify, 
>> > analyze and report issues. As a very first step, we are looking into 
>> > clang compiler warnings and will then move to more sophisticated tools. 
>> >
>> > [...]
>> >
>> > Linux 4.15 is shipped with this clang warning, but we don't see the 
>> > crucial need to provide a backport commit to the stable branch for 4.15. 
>> > We just wanted to inform you about our analysis of this clang warning. 
>> > Ultimately the final call if you would like to address this clang warning 
>> > in 4.15 is yours.
>> 
>> Note, I have not taken "clang warning fixes" for stable kernel updates
>> in the past, and I doubt I will in the future, unless the tree "builds
>> clean" with clang.  If it eventually gets there, then yes, I will do
>> that.
>> 
>> Note, if you are going to email this out to everyone who fixes a warning
>> message, you might want to reconsider it.  That's going to be a lot of
>> work, and for people who have already fixed an issue, it's kind of
>> pointless to just remind them of work they have done in the past, right?
>> 
>> What is the goal of these types of emails?
>> 
>
> We are interested in providing useful information on potential bugs or bug 
> patterns that we get from static analysis tools, after we pre-assess them 
> and manually select them to send to the review process of the patch 
> submission.
>
> For me, the clang warnings were an easy starting point for a student to 
> set up and look at, compared to much more sophisticated tools, such as 
> coccinelle, sparse or new tools for the kernel development, such as CMBC 
> or facebook's Infer.
>
> Once we really understand which tools are useful and which information can 
> be quickly pre-assessed and sent out in a useful way without just creating 
> more noise in the discussion, I would have contacted the 0-day 
> infrastructure team or the kernelci.org team to continue the discussion 
> how to include our first setup into a proper semi-automated service.
>
> Using the clang warnings, I wanted to explore how this would even 
> potentially work.
>
> Considering clang, of course, currently, we cannot compile the whole 
> kernel with all possible kernel configurations with clang, but I believe 
> Nick Desaulniers, Matthias Kaehlcke and others are already working on 
> that and are getting close to this goal. Hence, assuming they will be 
> successful soon, I wanted to explore the next step of using static 
> analysis tools around the clang/LLVM compiler.
>
> Actually, v4.15 builds almost "cleanly" with clang: For defconfig, there 
> are only two clang compiler warnings and the one that we looked into 
> deeper here is already resolved in linux-next, so chances are actually 
> high that we might get to this "builds clean" soon-ish, at least for 
> defconfig.
>
> Concerning clang warnings and how to progress towards that goal of 
> building cleanly, my strategy is to identify when new clang compiler
> warnings are introduced and just point these warnings out as code smells 
> during the review discussion before they are merged into the 
> first maintainer tree. If we manually inspect these clang warnings
> to make sure that they are genuine code smells of some "imprecise 
> implementation" before we send them to the mailing list, I would hope that 
> they are of some value for the developer in the submission process and 
> he/she could address the warning in a patch v2 while he/she is reacting to 
> the other review comments from the human reviewers.
>
> At best, I always considered them as useful information during the review 
> process, but I certainly DO NOT want to start patching the kernel due to 
> clang warnings. The chances/risk that we just break more due to naively 
> fixing warnings without proper understanding is much higher than the  
> benefit of actually improving the situation. If I recall correctly, I 
> think this is also one of the lessons learned from motivating newcomers 
> to address warnings in previous kernel newbies activities.
>
> Greg, do you think it is worthwhile to invest some effort to move 
> towards the goal "kernel builds cleanly with clang"?
> Would you agree that providing information during the patch review is a 
> good way to move forward to this goal if we find a suitable manner to 
> provide this feedback quickly and cleanly at this very early stage of 
> development?
>
> If not, we will immediately stop to move in this direction and this is the 
> first and last email that you have seen of this kind, and we will have to 
> come up with better/other ideas around work on the Linux kernel.
>
> If so, we will continue in the 

[Bug 104902] unpackHalf2x16 causes LLVM Error on GCN 1.2+ cards (and possibly others)

[bugzilla-daemon]

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

João Henrique  changed:

   What|Removed |Added

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

--- Comment #1 from João Henrique  ---
Issue has been fixed as of
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20180129/521298.html
so it wasn't really a Mesa bug.

-- 
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: clang warning: implicit conversion in intel_ddi.c:1481

[Ozgur]

01.02.2018, 21:03, "Greg KH" :
> On Thu, Feb 01, 2018 at 06:33:30PM +0100, Ozan Alpay wrote:
>>  Dear Rodrigo Vivi, Ville Syrjälä,
>>
>>  My name is Ozan Alpay, and I am a student mentored by Lukas Bulwahn. We

Hi Ozan,

why did you send e-mail to kernel development e-mail list?

>>  intend to use static analysis tools on the kernel source to identify,
>>  analyze and report issues. As a very first step, we are looking into
>>  clang compiler warnings and will then move to more sophisticated tools.
>>
>>  When compiling Linux 4.15 with clang, we have discovered that your commit
>>  2952cd6fb4cc ("drm/i915: Let's use more enum intel_dpll_id pll_id.")
>>  introduced the following warning:
>>
>>  drivers/gpu/drm/i915/intel_ddi.c:1481:30: warning: implicit conversion from 
>> enumeration type 'enum port' to different enumeration type 'enum 
>> intel_dpll_id' [-Wenum-conversion]
>>  enum intel_dpll_id pll_id = port;
>>
>>  To reproduce it, you can compile Linux 4.15 with clang with this command:
>>
>>  make HOSTCC=clang-5.0 defconfig && make -j32 HOSTCC=clang-5.0 CC=clang-5.0
>>
>>  If you don't have clang installed in your system, you can use this simple
>>  docker setup to compile the kernel with clang:
>>
>>  wget 
>> https://raw.githubusercontent.com/bulwahn/linux-kernel-analysis/master/docker/kernel-clang/Dockerfile
>>  && \
>>  docker build -t kernel-clang . && \
>>  docker run -v :/linux/ kernel-clang /bin/sh 
>> -c "cd linux && make CC=clang-5.0 clean && make HOSTCC=clang-5.0 defconfig 
>> && make -j32 HOSTCC=clang-5.0 CC=clang-5.0"
>>
>>  While we were doing our analysis on 4.15, we noticed that you already
>>  resolved this warning on linux-next with your work in commit bb911536f07e
>>  ("drm/i915: Eliminate pll->state usage from bxt_calc_pll_link()"). So,
>>  since it is resolved on linux-next and we expect that this commit will be
>>  merged in the merge window for 4.16, there is probably nothing further to
>>  do.
>>
>>  Linux 4.15 is shipped with this clang warning, but we don't see the
>>  crucial need to provide a backport commit to the stable branch for 4.15.
>>  We just wanted to inform you about our analysis of this clang warning.
>>  Ultimately the final call if you would like to address this clang warning
>>  in 4.15 is yours.
>
> Note, I have not taken "clang warning fixes" for stable kernel updates
> in the past, and I doubt I will in the future, unless the tree "builds
> clean" with clang. If it eventually gets there, then yes, I will do
> that.
>
> Note, if you are going to email this out to everyone who fixes a warning
> message, you might want to reconsider it. That's going to be a lot of
> work, and for people who have already fixed an issue, it's kind of
> pointless to just remind them of work they have done in the past, right?
>
> What is the goal of these types of emails?
>
> thanks,
>
> greg k-h

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

Re: clang warning: implicit conversion in intel_ddi.c:1481

[Lukas Bulwahn]

Hi Greg,

On Thu, 1 Feb 2018, Greg KH wrote:

> On Thu, Feb 01, 2018 at 06:33:30PM +0100, Ozan Alpay wrote:
> > Dear Rodrigo Vivi, Ville Syrjälä,
> > 
> > My name is Ozan Alpay, and I am a student mentored by Lukas Bulwahn. We 
> > intend to use static analysis tools on the kernel source to identify, 
> > analyze and report issues. As a very first step, we are looking into 
> > clang compiler warnings and will then move to more sophisticated tools. 
> >
> > [...]
> >
> > Linux 4.15 is shipped with this clang warning, but we don't see the 
> > crucial need to provide a backport commit to the stable branch for 4.15. 
> > We just wanted to inform you about our analysis of this clang warning. 
> > Ultimately the final call if you would like to address this clang warning 
> > in 4.15 is yours.
> 
> Note, I have not taken "clang warning fixes" for stable kernel updates
> in the past, and I doubt I will in the future, unless the tree "builds
> clean" with clang.  If it eventually gets there, then yes, I will do
> that.
> 
> Note, if you are going to email this out to everyone who fixes a warning
> message, you might want to reconsider it.  That's going to be a lot of
> work, and for people who have already fixed an issue, it's kind of
> pointless to just remind them of work they have done in the past, right?
> 
> What is the goal of these types of emails?
> 

We are interested in providing useful information on potential bugs or bug 
patterns that we get from static analysis tools, after we pre-assess them 
and manually select them to send to the review process of the patch 
submission.

For me, the clang warnings were an easy starting point for a student to 
set up and look at, compared to much more sophisticated tools, such as 
coccinelle, sparse or new tools for the kernel development, such as CMBC 
or facebook's Infer.

Once we really understand which tools are useful and which information can 
be quickly pre-assessed and sent out in a useful way without just creating 
more noise in the discussion, I would have contacted the 0-day 
infrastructure team or the kernelci.org team to continue the discussion 
how to include our first setup into a proper semi-automated service.

Using the clang warnings, I wanted to explore how this would even 
potentially work.

Considering clang, of course, currently, we cannot compile the whole 
kernel with all possible kernel configurations with clang, but I believe 
Nick Desaulniers, Matthias Kaehlcke and others are already working on 
that and are getting close to this goal. Hence, assuming they will be 
successful soon, I wanted to explore the next step of using static 
analysis tools around the clang/LLVM compiler.

Actually, v4.15 builds almost "cleanly" with clang: For defconfig, there 
are only two clang compiler warnings and the one that we looked into 
deeper here is already resolved in linux-next, so chances are actually 
high that we might get to this "builds clean" soon-ish, at least for 
defconfig.

Concerning clang warnings and how to progress towards that goal of 
building cleanly, my strategy is to identify when new clang compiler
warnings are introduced and just point these warnings out as code smells 
during the review discussion before they are merged into the 
first maintainer tree. If we manually inspect these clang warnings
to make sure that they are genuine code smells of some "imprecise 
implementation" before we send them to the mailing list, I would hope that 
they are of some value for the developer in the submission process and 
he/she could address the warning in a patch v2 while he/she is reacting to 
the other review comments from the human reviewers.

At best, I always considered them as useful information during the review 
process, but I certainly DO NOT want to start patching the kernel due to 
clang warnings. The chances/risk that we just break more due to naively 
fixing warnings without proper understanding is much higher than the  
benefit of actually improving the situation. If I recall correctly, I 
think this is also one of the lessons learned from motivating newcomers 
to address warnings in previous kernel newbies activities.

Greg, do you think it is worthwhile to invest some effort to move 
towards the goal "kernel builds cleanly with clang"?
Would you agree that providing information during the patch review is a 
good way to move forward to this goal if we find a suitable manner to 
provide this feedback quickly and cleanly at this very early stage of 
development?

If not, we will immediately stop to move in this direction and this is the 
first and last email that you have seen of this kind, and we will have to 
come up with better/other ideas around work on the Linux kernel.

If so, we will continue in the direction sketched above, and I think I 
just have to point out and apologize for the two obvious things that we 
did wrong in this specific case here:

  - We noticed that there were further changes in linux-next, but we

Re: [PATCH v6 4/6] iommu/arm-smmu: Add the device_link between masters and smmu

[Sricharan R]

Hi Robin/Vivek,

On 2/1/2018 2:23 PM, Vivek Gautam wrote:
> Hi,
> 
> 
> On 1/31/2018 6:39 PM, Robin Murphy wrote:
>> On 19/01/18 11:43, Vivek Gautam wrote:
>>> From: Sricharan R 
>>>
>>> Finally add the device link between the master device and
>>> smmu, so that the smmu gets runtime enabled/disabled only when the
>>> master needs it. This is done from add_device callback which gets
>>> called once when the master is added to the smmu.
>>
>> Don't we need to balance this with a device_link_del() in .remove_device 
>> (like exynos-iommu does)?
> 
> Right. Will add device_link_del() call. Thanks for pointing out.

 The reason for not adding device_link_del from .remove_device was, the core 
device_del 
 which calls the .remove_device from notifier, calls device_links_purge before 
that.
 That does the same thing as device_link_del. So by the time .remove_device is 
called,
 device_links for that device is already cleaned up. Vivek, you may want to 
check once that
 calling device_link_del from .remove_device has no effect, just to confirm 
once more.

Regards,
 Sricharan
 
> 
> regards
> Vivek
> 
>>
>> Robin.
>>
>>> Signed-off-by: Sricharan R 
>>> ---
>>>   drivers/iommu/arm-smmu.c | 11 +++
>>>   1 file changed, 11 insertions(+)
>>>
>>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>>> index 95478bfb182c..33bbcfedb896 100644
>>> --- a/drivers/iommu/arm-smmu.c
>>> +++ b/drivers/iommu/arm-smmu.c
>>> @@ -1367,6 +1367,7 @@ static int arm_smmu_add_device(struct device *dev)
>>>   struct arm_smmu_device *smmu;
>>>   struct arm_smmu_master_cfg *cfg;
>>>   struct iommu_fwspec *fwspec = dev->iommu_fwspec;
>>> +    struct device_link *link;
>>>   int i, ret;
>>>     if (using_legacy_binding) {
>>> @@ -1428,6 +1429,16 @@ static int arm_smmu_add_device(struct device *dev)
>>>     pm_runtime_put_sync(smmu->dev);
>>>   +    /*
>>> + * Establish the link between smmu and master, so that the
>>> + * smmu gets runtime enabled/disabled as per the master's
>>> + * needs.
>>> + */
>>> +    link = device_link_add(dev, smmu->dev, DL_FLAG_PM_RUNTIME);
>>> +    if (!link)
>>> +    dev_warn(smmu->dev, "Unable to create device link between %s and 
>>> %s\n",
>>> + dev_name(smmu->dev), dev_name(dev));
>>> +
>>>   return 0;
>>>     out_cfg_free:
>>>
> 

-- 
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of 
Code Aurora Forum, hosted by The Linux Foundation
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] fbdev: don't select I2C directly

[Randy Dunlap]

On 02/01/2018 08:14 AM, Bartlomiej Zolnierkiewicz wrote:
> On Monday, January 15, 2018 05:14:04 PM Arnd Bergmann wrote:
>> Using a Kconfig 'select' statement for a user-visible symbol that other
>> drivers depend on often causes circular dependencies. A new one showed
>> up when I wanted to add an NVMEM dependency to the DRM_MSM driver:
>>
>> drivers/i2c/Kconfig:7:error: recursive dependency detected!
>> drivers/i2c/Kconfig:7:   symbol I2C is selected by FB_DDC
>> drivers/video/fbdev/Kconfig:63:  symbol FB_DDC is selected by 
>> FB_CYBER2000_DDC
>> drivers/video/fbdev/Kconfig:390: symbol FB_CYBER2000_DDC depends on 
>> FB_CYBER2000
>> drivers/video/fbdev/Kconfig:378: symbol FB_CYBER2000 depends on FB
>> drivers/video/fbdev/Kconfig:5:   symbol FB is selected by 
>> DRM_KMS_FB_HELPER
>> drivers/gpu/drm/Kconfig:77:  symbol DRM_KMS_FB_HELPER depends on 
>> DRM_KMS_HELPER
>> drivers/gpu/drm/Kconfig:71:  symbol DRM_KMS_HELPER is selected by DRM_MSM
>> drivers/gpu/drm/msm/Kconfig:2:   symbol DRM_MSM depends on NVMEM
>> drivers/nvmem/Kconfig:1: symbol NVMEM is selected by EEPROM_AT24
>> drivers/misc/eeprom/Kconfig:3:   symbol EEPROM_AT24 depends on I2C
>>
>> Here, the problem is that many fbdev drivers have an i2c based interface
>> and just 'select i2c' for that, while we also select the framebuffer
>> subsystem indirectly from a DRM driver that now depends on i2c.
>>
>> This does away with the 'select' statement and instead uses 'depends on',
>> like almost all I2C users.
> 
> I worry that this change may cause various driver options to no longer
> be visible to people configuring their kernels and not having I2C already
> selected.
> 
> DRM somehow manages to select I2C and I would prefer to be it the same
> way for fbdev (also looking at the current next tree there are still
> some drivers that 'select I2C')..

a. Linus has stated that a driver should not enable an entire subsystem,
   so depends on would be better than select.
   If I had the email/patch, I would be glad to Ack it.

b. DRM configuration is a mess. You shouldn't want to follow their model. :)

c. If I2C is not enabled in the FB menu, someone could just add something like 
this:

comment "Enable I2C to see more driver choices"
depends on !I2C


>> Signed-off-by: Arnd Bergmann 
>> ---
>>  drivers/video/fbdev/Kconfig | 19 +++
>>  1 file changed, 15 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
>> index 6962b4583fd7..892eb1863100 100644
>> --- a/drivers/video/fbdev/Kconfig
>> +++ b/drivers/video/fbdev/Kconfig
>> @@ -64,7 +64,6 @@ config FB_DDC
>> tristate
>> depends on FB
>> select I2C_ALGOBIT
>> -   select I2C
>> default n
>>  
>>  config FB_BOOT_VESA_SUPPORT
>> @@ -390,6 +389,7 @@ config FB_CYBER2000
>>  config FB_CYBER2000_DDC
>>  bool "DDC for CyberPro support"
>>  depends on FB_CYBER2000
>> +depends on I2C=y || I2C=FB_CYBER2000
>>  select FB_DDC
>>  default y
>>  help
>> @@ -634,7 +634,7 @@ config FB_BF537_LQ035
>>  config FB_BFIN_7393
>>  tristate "Blackfin ADV7393 Video encoder"
>>  depends on FB && BLACKFIN
>> -select I2C
>> +depends on I2C
>>  select FB_CFB_FILLRECT
>>  select FB_CFB_COPYAREA
>>  select FB_CFB_IMAGEBLIT
>> @@ -1026,6 +1026,7 @@ config FB_NVIDIA
>>  config FB_NVIDIA_I2C
>> bool "Enable DDC Support"
>> depends on FB_NVIDIA
>> +   depends on I2C=y || I2C=FB_NVIDIA
>> select FB_DDC
>> help
>>This enables I2C support for nVidia Chipsets.  This is used
>> @@ -1073,6 +1074,7 @@ config FB_RIVA
>>  config FB_RIVA_I2C
>> bool "Enable DDC Support"
>> depends on FB_RIVA
>> +   depends on I2C=y || I2C=FB_RIVA
>> select FB_DDC
>> help
>>This enables I2C support for nVidia Chipsets.  This is used
>> @@ -1102,6 +1104,7 @@ config FB_RIVA_BACKLIGHT
>>  config FB_I740
>>  tristate "Intel740 support"
>>  depends on FB && PCI
>> +depends on I2C
>>  select FB_MODE_HELPERS
>>  select FB_CFB_FILLRECT
>>  select FB_CFB_COPYAREA
>> @@ -1155,6 +1158,7 @@ config FB_I810_GTF
>>  config FB_I810_I2C
>>  bool "Enable DDC Support"
>>  depends on FB_I810 && FB_I810_GTF
>> +depends on I2C=y || I2C=FB_I810
>>  select FB_DDC
>>  help
>>  
>> @@ -1206,6 +1210,7 @@ config FB_INTEL_DEBUG
>>  config FB_INTEL_I2C
>>  bool "DDC/I2C for Intel framebuffer support"
>>  depends on FB_INTEL
>> +depends on I2C=y || I2C=FB_INTEL
>>  select FB_DDC
>>  default y
>>  help
>> @@ -1285,6 +1290,7 @@ config FB_MATROX_G
>>  config FB_MATROX_I2C
>>  tristate "Matrox I2C support"
>>  depends on FB_MATROX
>> +depends on I2C
>>  select FB_DDC
>>  ---help---
>>This drivers creates I2C buses which are needed for accessing the
>> @@ -1350,6 +1356,7 @@ config FB_RADEON
>>  config 

RE: [PATCH 1/5] drm/ttm: add max_swap_mem in ttm_mem_global

[He, Roger]

Need call si_swapinfo to fill those valules .
void si_swapinfo(struct sysinfo *val)

But that function is not exported as well.

Thanks
Roger(Hongbo.He)
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of 
Chunming Zhou
Sent: Friday, February 02, 2018 3:38 PM
To: He, Roger ; dri-devel@lists.freedesktop.org
Cc: Koenig, Christian 
Subject: Re: [PATCH 1/5] drm/ttm: add max_swap_mem in ttm_mem_global




On 2018年02月02日 15:34, Chunming Zhou wrote:


On 2018年02月02日 15:22, Roger He wrote:

set its initial value as 1/2 * free swap cache size when module initial.
and adjust this value when allocate TTM memory

Signed-off-by: Roger He 
---
  drivers/gpu/drm/ttm/ttm_memory.c | 10 --
  include/drm/ttm/ttm_memory.h |  2 ++
  2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c
index aa0c381..b48931d 100644
--- a/drivers/gpu/drm/ttm/ttm_memory.c
+++ b/drivers/gpu/drm/ttm/ttm_memory.c
@@ -36,6 +36,7 @@
  #include 
  #include 
  #include 
+#include 
#define TTM_MEMORY_ALLOC_RETRIES 4
  @@ -372,9 +373,9 @@ int ttm_mem_global_init(struct ttm_mem_global *glob)
  kobject_put(>kobj);
  return ret;
  }
-
+/* set it as 1/2 * swap free space we can get at that time */
+glob->max_swap_mem = get_nr_swap_pages() << (PAGE_SHIFT - 1);
  si_meminfo();
Hi Roger,

I just find si_meminfo can get total swap size, see struct sysinfo definition:
 struct sysinfo {
__kernel_long_t uptime; /* Seconds since boot */
__kernel_ulong_t loads[3];  /* 1, 5, and 15 minute load averages */
__kernel_ulong_t totalram;  /* Total usable main memory size */
__kernel_ulong_t freeram;   /* Available memory size */
__kernel_ulong_t sharedram; /* Amount of shared memory */
__kernel_ulong_t bufferram; /* Memory used by buffers */
__kernel_ulong_t totalswap; /* Total swap space size */
__kernel_ulong_t freeswap;  /* swap space still available */
__u16 procs;/* Number of current processes */
...

can sysinfo.totalswap be used for your change?

Regards,
David Zhou

-
  ret = ttm_mem_init_kernel_zone(glob, );
  if (unlikely(ret != 0))
  goto out_no_zone;
@@ -473,12 +474,17 @@ static int ttm_mem_global_reserve(struct ttm_mem_global 
*glob,
struct ttm_mem_zone *single_zone,
uint64_t amount, bool reserve)
  {
+uint64_t free_swap_mem = get_nr_swap_pages() << (PAGE_SHIFT - 1);
  uint64_t limit;
  int ret = -ENOMEM;
  unsigned int i;
  struct ttm_mem_zone *zone;
spin_lock(>lock);
+/* adjust the max_swap_mem to cover the new inserted swap space */
+if (glob->max_swap_mem < free_swap_mem)
+glob->max_swap_mem = free_swap_mem;
Seems using max() for exchange is more obvious, otherwise looks ok to me.

Regards,
David Zhou

+
  for (i = 0; i < glob->num_zones; ++i) {
  zone = glob->zones[i];
  if (single_zone && zone != single_zone)
diff --git a/include/drm/ttm/ttm_memory.h b/include/drm/ttm/ttm_memory.h
index 8936285..ad5a557 100644
--- a/include/drm/ttm/ttm_memory.h
+++ b/include/drm/ttm/ttm_memory.h
@@ -49,6 +49,7 @@
   * @work: The workqueue callback for the shrink queue.
   * @lock: Lock to protect the @shrink - and the memory accounting members,
   * that is, essentially the whole structure with some exceptions.
+ * @max_swap_mem: upper limit of swap space TTM can use
   * @zones: Array of pointers to accounting zones.
   * @num_zones: Number of populated entries in the @zones array.
   * @zone_kernel: Pointer to the kernel zone.
@@ -67,6 +68,7 @@ struct ttm_mem_global {
  struct workqueue_struct *swap_queue;
  struct work_struct work;
  spinlock_t lock;
+uint64_t max_swap_mem;
  struct ttm_mem_zone *zones[TTM_MEM_MAX_ZONES];
  unsigned int num_zones;
  struct ttm_mem_zone *zone_kernel;


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

[Bug 104895] [CI] igt@tools_test@tools_test - fail - Test assertion failure function __real_main62 - Failed assertion: igt_system_quiet("./intel_reg dump") == 0

[bugzilla-daemon]

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

Marta Löfstedt  changed:

   What|Removed |Added

   Assignee|dri-devel@lists.freedesktop |mika.kah...@intel.com
   |.org|
 Status|NEW |NEEDINFO

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