Re: [PATCH 2/2] drm/atomic: Create and use __drm_atomic_helper_crtc_reset() everywhere

2018-11-19 Thread CK Hu
On Mon, 2018-11-12 at 16:01 +0100, Maarten Lankhorst wrote:
> We already have __drm_atomic_helper_connector_reset() and
> __drm_atomic_helper_plane_reset(), extend this to crtc as well.
> 
> Most drivers already have a gpu reset hook, correct it.
> Nouveau already implemented its own __drm_atomic_helper_crtc_reset(),
> convert it to the common one.
> 
> Signed-off-by: Maarten Lankhorst 
> Cc: Harry Wentland 
> Cc: Leo Li 
> Cc: Alex Deucher 
> Cc: "Christian König" 
> Cc: "David (ChunMing) Zhou" 
> Cc: David Airlie 
> Cc: Liviu Dudau 
> Cc: Brian Starkey 
> Cc: Mali DP Maintainers 
> Cc: Boris Brezillon 
> Cc: Nicolas Ferre 
> Cc: Alexandre Belloni 
> Cc: Ludovic Desroches 
> Cc: Maarten Lankhorst 
> Cc: Maxime Ripard 
> Cc: Sean Paul 
> Cc: Jani Nikula 
> Cc: Joonas Lahtinen 
> Cc: Rodrigo Vivi 
> Cc: Philipp Zabel 
> Cc: CK Hu 
> Cc: Matthias Brugger 
> Cc: Rob Clark 
> Cc: Ben Skeggs 
> Cc: Tomi Valkeinen 
> Cc: Laurent Pinchart 
> Cc: Kieran Bingham 
> Cc: Sandy Huang 
> Cc: "Heiko Stübner" 
> Cc: Thierry Reding 
> Cc: Jonathan Hunter 
> Cc: Eric Anholt 
> Cc: VMware Graphics 
> Cc: Sinclair Yeh 
> Cc: Thomas Hellstrom 
> Cc: Tony Cheng 
> Cc: Shirish S 
> Cc: Mikita Lipski 
> Cc: Bhawanpreet Lakha 
> Cc: David Francis 
> Cc: Anthony Koo 
> Cc: Jeykumar Sankaran 
> Cc: Jordan Crouse 
> Cc: Bruce Wang 
> Cc: Sravanthi Kollukuduru 
> Cc: Archit Taneja 
> Cc: Steve Kowalik 
> Cc: Carsten Behling 
> Cc: Haneen Mohammed 
> Cc: Daniel Vetter 
> Cc: Rodrigo Siqueira 
> Cc: Mahesh Kumar 
> Cc: amd-...@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-ker...@vger.kernel.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: intel-...@lists.freedesktop.org
> Cc: linux-media...@lists.infradead.org
> Cc: linux-arm-...@vger.kernel.org
> Cc: freedr...@lists.freedesktop.org
> Cc: nouv...@lists.freedesktop.org
> Cc: linux-renesas-...@vger.kernel.org
> Cc: linux-rockc...@lists.infradead.org
> Cc: linux-te...@vger.kernel.org
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  4 +--
>  drivers/gpu/drm/arm/malidp_crtc.c |  5 +--
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c|  5 +--
>  drivers/gpu/drm/drm_atomic_state_helper.c | 31 ---
>  drivers/gpu/drm/i915/intel_display.c  |  2 +-
>  drivers/gpu/drm/imx/ipuv3-crtc.c  |  5 +--
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c   |  5 +--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  | 12 ++-
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c |  6 +---
>  drivers/gpu/drm/nouveau/dispnv50/head.c   | 13 ++--
>  drivers/gpu/drm/omapdrm/omap_crtc.c   |  7 ++---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c|  4 +--
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |  7 +++--
>  drivers/gpu/drm/tegra/dc.c|  5 +--
>  drivers/gpu/drm/vc4/vc4_crtc.c|  8 ++---
>  drivers/gpu/drm/vkms/vkms_crtc.c  |  7 +
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c   |  9 +-
>  include/drm/drm_atomic_state_helper.h |  2 ++
>  18 files changed, 56 insertions(+), 81 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 5064768642f3..770a71726cd1 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2802,9 +2802,7 @@ static void dm_crtc_reset_state(struct drm_crtc *crtc)
>   if (WARN_ON(!state))
>   return;
>  
> - crtc->state = &state->base;
> - crtc->state->crtc = crtc;
> -
> + __drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/arm/malidp_crtc.c 
> b/drivers/gpu/drm/arm/malidp_crtc.c
> index e1b72782848c..9a924ff27148 100644
> --- a/drivers/gpu/drm/arm/malidp_crtc.c
> +++ b/drivers/gpu/drm/arm/malidp_crtc.c
> @@ -474,10 +474,7 @@ static void malidp_crtc_reset(struct drm_crtc *crtc)
>  
>   kfree(state);
>   state = kzalloc(sizeof(*state), GFP_KERNEL);
> - if (state) {
> - crtc->state = &state->base;
> - crtc->state->crtc = crtc;
> - }
> + __drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static void malidp_crtc_destroy_state(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index 96f4082671fe..8084d549c7d1 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -412,10 +412,7 @@ static void atmel_hlcdc_crtc_reset(struct drm_crtc *crtc)
>   }
>  
>   state = kzalloc(sizeof(*state), GFP_KERNEL);
> - if (state) {
> - crtc->state = &state->base;
> - crtc->state->crtc = crtc;
> - }
> + __drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/drm_atomic_sta

Re: [PATCH v3] drm/rockchip: update cursors asynchronously through atomic.

2018-11-19 Thread Tomasz Figa
Hi Helen,

On Tue, Nov 20, 2018 at 4:08 AM Helen Koike  wrote:
>
> From: Enric Balletbo i Serra 
>
> Add support to async updates of cursors by using the new atomic
> interface for that.
>
> Signed-off-by: Enric Balletbo i Serra 
> [updated for upstream]
> Signed-off-by: Helen Koike 
>
> ---
> Hello,
>
> This is the third version of the async-plane update suport to the
> Rockchip driver.
>

Thanks for a quick respin. Please see my comments inline. (I'll try to
be better at responding from now on...)

> I tested running igt kms_cursor_legacy and kms_atomic tests using a 96Boards 
> Ficus.
>
> Note that before the patch, the following igt tests failed:
>
> basic-flip-before-cursor-atomic
> basic-flip-before-cursor-legacy
> cursor-vs-flip-atomic
> cursor-vs-flip-legacy
> cursor-vs-flip-toggle
> flip-vs-cursor-atomic
> flip-vs-cursor-busy-crc-atomic
> flip-vs-cursor-busy-crc-legacy
> flip-vs-cursor-crc-atomic
> flip-vs-cursor-crc-legacy
> flip-vs-cursor-legacy
>
> Full log: https://people.collabora.com/~koike/results-4.20/html/
>
> Now with the patch applied the following were fixed:
> basic-flip-before-cursor-atomic
> basic-flip-before-cursor-legacy
> flip-vs-cursor-atomic
> flip-vs-cursor-legacy
>
> Full log: https://people.collabora.com/~koike/results-4.20-async/html/

Could you also test modetest, with the -C switch to test the legacy
cursor API? I remember it triggering crashes due to synchronization
issues easily.

>
> Tomasz, as you mentined in v2 about waiting the hardware before updating
> the framebuffer, now I call the loop you pointed out in the async path,
> was that what you had in mind? Or do you think I would make sense to
> call the vop_crtc_atomic_flush() instead of just exposing that loop?
>
> Thanks
> Helen
>
> Changes in v3:
> - Rebased on top of drm-misc
> - Fix missing include in rockchip_drm_vop.c
> - New function vop_crtc_atomic_commit_flush
>
> Changes in v2:
> - v2: https://patchwork.freedesktop.org/patch/254180/
> - Change the framebuffer as well to cover jumpy cursor when hovering
>   text boxes or hyperlink. (Tomasz)
> - Use the PSR inhibit mechanism when accessing VOP hardware instead of
>   PSR flushing (Tomasz)
>
> Changes in v1:
> - Rebased on top of drm-misc
> - In async_check call drm_atomic_helper_check_plane_state to check that
>   the desired plane is valid and update various bits of derived state
>   (clipped coordinates etc.)
> - In async_check allow to configure new scaling in the fast path.
> - In async_update force to flush all registered PSR encoders.
> - In async_update call atomic_update directly.
> - In async_update call vop_cfg_done needed to set the vop registers and take 
> effect.
>
>  drivers/gpu/drm/rockchip/rockchip_drm_fb.c  |  36 ---
>  drivers/gpu/drm/rockchip/rockchip_drm_psr.c |  37 +++
>  drivers/gpu/drm/rockchip/rockchip_drm_psr.h |   3 +
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 108 +---
>  4 files changed, 131 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c 
> b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> index ea18cb2a76c0..08bec50d9c5d 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> @@ -127,42 +127,6 @@ rockchip_user_fb_create(struct drm_device *dev, struct 
> drm_file *file_priv,
> return ERR_PTR(ret);
>  }
>
> -static void
> -rockchip_drm_psr_inhibit_get_state(struct drm_atomic_state *state)
> -{
> -   struct drm_crtc *crtc;
> -   struct drm_crtc_state *crtc_state;
> -   struct drm_encoder *encoder;
> -   u32 encoder_mask = 0;
> -   int i;
> -
> -   for_each_old_crtc_in_state(state, crtc, crtc_state, i) {
> -   encoder_mask |= crtc_state->encoder_mask;
> -   encoder_mask |= crtc->state->encoder_mask;
> -   }
> -
> -   drm_for_each_encoder_mask(encoder, state->dev, encoder_mask)
> -   rockchip_drm_psr_inhibit_get(encoder);
> -}
> -
> -static void
> -rockchip_drm_psr_inhibit_put_state(struct drm_atomic_state *state)
> -{
> -   struct drm_crtc *crtc;
> -   struct drm_crtc_state *crtc_state;
> -   struct drm_encoder *encoder;
> -   u32 encoder_mask = 0;
> -   int i;
> -
> -   for_each_old_crtc_in_state(state, crtc, crtc_state, i) {
> -   encoder_mask |= crtc_state->encoder_mask;
> -   encoder_mask |= crtc->state->encoder_mask;
> -   }
> -
> -   drm_for_each_encoder_mask(encoder, state->dev, encoder_mask)
> -   rockchip_drm_psr_inhibit_put(encoder);
> -}
> -
>  static void
>  rockchip_atomic_helper_commit_tail_rpm(struct drm_atomic_state *old_state)
>  {
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c 
> b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> index 01ff3c858875..22a70ab6e214 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
>

Re: [PATCH 4/6] drm/vc4: Rework the async update logic

2018-11-19 Thread Eric Anholt
Boris Brezillon  writes:

> On Thu, 15 Nov 2018 12:49:11 -0800
> Eric Anholt  wrote:
>
>> Boris Brezillon  writes:
>> 
>> > vc4_plane_atomic_async_check() was only based on the
>> > state->{crtc,src}_{w,h} which was fine since scaling was not allowed on
>> > the cursor plane.
>> >
>> > We are about to change that to properly support underscan, and, in order
>> > to make the async check more reliable, we call vc4_plane_mode_set()
>> > from there and check that only the pos0, pos2 and ptr0 entries in the
>> > dlist have changed.
>> >
>> > In vc4_plane_atomic_async_update(), we no longer call
>> > vc4_plane_atomic_check() since vc4_plane_mode_set() has already been
>> > called in vc4_plane_atomic_async_check(), and we don't need to allocate
>> > a new LBM region (we reuse the one from the current state).
>> >
>> > Note that we now have to manually update each field of the current
>> > plane state since it's no longer updated in place (not sure we have
>> > to sync all of them, but it's harmless if we do).
>> > We also drop the vc4_plane_async_set_fb() call (ptr0 dlist entry has
>> > been properly updated in vc4_plane_mode_set())
>> >
>> > Signed-off-by: Boris Brezillon 
>> > ---
>> >  drivers/gpu/drm/vc4/vc4_plane.c | 87 +
>> >  1 file changed, 66 insertions(+), 21 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/vc4/vc4_plane.c 
>> > b/drivers/gpu/drm/vc4/vc4_plane.c
>> > index 09c7478b095b..31c7b63dd723 100644
>> > --- a/drivers/gpu/drm/vc4/vc4_plane.c
>> > +++ b/drivers/gpu/drm/vc4/vc4_plane.c
>> > @@ -895,30 +895,50 @@ static void vc4_plane_atomic_async_update(struct 
>> > drm_plane *plane,
>> >  {
>> >struct vc4_plane_state *vc4_state, *new_vc4_state;
>> >  
>> > -  if (plane->state->fb != state->fb) {
>> > -  vc4_plane_async_set_fb(plane, state->fb);
>> > -  drm_atomic_set_fb_for_plane(plane->state, state->fb);
>> > -  }
>> > -
>> > -  /* Set the cursor's position on the screen.  This is the
>> > -   * expected change from the drm_mode_cursor_universal()
>> > -   * helper.
>> > -   */
>> > +  drm_atomic_set_fb_for_plane(plane->state, state->fb);
>> >plane->state->crtc_x = state->crtc_x;
>> >plane->state->crtc_y = state->crtc_y;
>> > -
>> > -  /* Allow changing the start position within the cursor BO, if
>> > -   * that matters.
>> > -   */
>> > +  plane->state->crtc_w = state->crtc_w;
>> > +  plane->state->crtc_h = state->crtc_h;
>> >plane->state->src_x = state->src_x;
>> >plane->state->src_y = state->src_y;
>> > -
>> > -  /* Update the display list based on the new crtc_x/y. */
>> > -  vc4_plane_atomic_check(plane, state);
>> > +  plane->state->src_w = state->src_w;
>> > +  plane->state->src_h = state->src_h;
>> > +  plane->state->src_h = state->src_h;
>> > +  plane->state->alpha = state->alpha;
>> > +  plane->state->pixel_blend_mode = state->pixel_blend_mode;
>> > +  plane->state->rotation = state->rotation;
>> > +  plane->state->zpos = state->zpos;
>> > +  plane->state->normalized_zpos = state->normalized_zpos;
>> > +  plane->state->color_encoding = state->color_encoding;
>> > +  plane->state->color_range = state->color_range;
>> > +  plane->state->src = state->src;
>> > +  plane->state->dst = state->dst;
>> > +  plane->state->visible = state->visible;
>> >  
>> >new_vc4_state = to_vc4_plane_state(state);
>> >vc4_state = to_vc4_plane_state(plane->state);
>> >  
>> > +  vc4_state->crtc_x = new_vc4_state->crtc_x;
>> > +  vc4_state->crtc_y = new_vc4_state->crtc_y;
>> > +  vc4_state->crtc_h = new_vc4_state->crtc_h;
>> > +  vc4_state->crtc_w = new_vc4_state->crtc_w;
>> > +  vc4_state->src_x = new_vc4_state->src_x;
>> > +  vc4_state->src_y = new_vc4_state->src_y;
>> > +  memcpy(vc4_state->src_w, new_vc4_state->src_w,
>> > + sizeof(vc4_state->src_w));
>> > +  memcpy(vc4_state->src_h, new_vc4_state->src_h,
>> > + sizeof(vc4_state->src_h));
>> > +  memcpy(vc4_state->x_scaling, new_vc4_state->x_scaling,
>> > + sizeof(vc4_state->x_scaling));
>> > +  memcpy(vc4_state->y_scaling, new_vc4_state->y_scaling,
>> > + sizeof(vc4_state->y_scaling));
>> > +  vc4_state->is_unity = new_vc4_state->is_unity;
>> > +  vc4_state->is_yuv = new_vc4_state->is_yuv;
>> > +  memcpy(vc4_state->offsets, new_vc4_state->offsets,
>> > + sizeof(vc4_state->offsets));
>> > +  vc4_state->needs_bg_fill = new_vc4_state->needs_bg_fill;  
>> 
>> This copying feels like a maintenance nightmare to me -- nothing's going
>> to be testing async updates of each random bit of state, so if something
>> new could change while passing atomic_async_check(), we're going to get
>> it wrong.
>
> Yeah, I don't like it either. I'd definitely prefer if states could be
> swapped somehow, but then you have the problem of migrating some of the
> resources from the old state to the new one (the most obvious one being
> the LBM drm_mm_node object which is already inserted in a list, but
> I'm pretty we have the same issue with other fields too).
>
>> 

Re: [PATCH 2/6] drm/vc4: Move LBM creation out of vc4_plane_mode_set()

2018-11-19 Thread Eric Anholt
Boris Brezillon  writes:

> On Thu, 15 Nov 2018 12:39:42 -0800
> Eric Anholt  wrote:
>
>> Boris Brezillon  writes:
>> 
>> > We are about to use vc4_plane_mode_set() in the async check path, and
>> > async updates require that LBM size stay the same since they reuse the
>> > LBM from the previous state. So we definitely don't want to allocate a
>> > new LBM region that we know for sure will be free right away.
>> >
>> > Move the LBM allocation out of vc4_plane_mode_set() and call the new
>> > function (vc4_plane_update_lbm()) from vc4_plane_atomic_check().
>> >
>> > Signed-off-by: Boris Brezillon 
>> > ---
>> >  drivers/gpu/drm/vc4/vc4_drv.h   |  1 +
>> >  drivers/gpu/drm/vc4/vc4_plane.c | 82 ++---
>> >  2 files changed, 55 insertions(+), 28 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
>> > index bd6ef1f31822..9ed05fb61eb6 100644
>> > --- a/drivers/gpu/drm/vc4/vc4_drv.h
>> > +++ b/drivers/gpu/drm/vc4/vc4_drv.h
>> > @@ -338,6 +338,7 @@ struct vc4_plane_state {
>> >u32 pos0_offset;
>> >u32 pos2_offset;
>> >u32 ptr0_offset;
>> > +  u32 lbm_offset;
>> >  
>> >/* Offset where the plane's dlist was last stored in the
>> > * hardware at vc4_crtc_atomic_flush() time.
>> > diff --git a/drivers/gpu/drm/vc4/vc4_plane.c 
>> > b/drivers/gpu/drm/vc4/vc4_plane.c
>> > index f6e3e8d33115..392a51f2bf8f 100644
>> > --- a/drivers/gpu/drm/vc4/vc4_plane.c
>> > +++ b/drivers/gpu/drm/vc4/vc4_plane.c
>> > @@ -452,6 +452,43 @@ static void vc4_write_scaling_parameters(struct 
>> > drm_plane_state *state,
>> >}
>> >  }
>> >  
>> > +static int vc4_plane_update_lbm(struct drm_plane_state *state)
>> > +{
>> > +  struct vc4_dev *vc4 = to_vc4_dev(state->plane->dev);
>> > +  struct vc4_plane_state *vc4_state = to_vc4_plane_state(state);
>> > +  unsigned long irqflags;
>> > +  u32 lbm_size;
>> > +
>> > +  lbm_size = vc4_lbm_size(state);
>> > +  if (!lbm_size)
>> > +  return 0;
>> > +
>> > +  if (WARN_ON(!vc4_state->lbm_offset))
>> > +  return -EINVAL;
>> > +
>> > +  /* Allocate the LBM memory that the HVS will use for temporary
>> > +   * storage due to our scaling/format conversion.
>> > +   */
>> > +  if (!vc4_state->lbm.allocated) {
>> > +  int ret;
>> > +
>> > +  spin_lock_irqsave(&vc4->hvs->mm_lock, irqflags);
>> > +  ret = drm_mm_insert_node_generic(&vc4->hvs->lbm_mm,
>> > +   &vc4_state->lbm,
>> > +   lbm_size, 32, 0, 0);
>> > +  spin_unlock_irqrestore(&vc4->hvs->mm_lock, irqflags);
>> > +
>> > +  if (ret)
>> > +  return ret;
>> > +  } else {
>> > +  WARN_ON_ONCE(lbm_size != vc4_state->lbm.size);
>> > +  }
>> > +
>> > +  vc4_state->dlist[vc4_state->lbm_offset] = vc4_state->lbm.start;
>> > +
>> > +  return 0;
>> > +}
>> > +
>> >  /* Writes out a full display list for an active plane to the plane's
>> >   * private dlist state.
>> >   */
>> > @@ -469,31 +506,12 @@ static int vc4_plane_mode_set(struct drm_plane 
>> > *plane,
>> >bool mix_plane_alpha;
>> >bool covers_screen;
>> >u32 scl0, scl1, pitch0;
>> > -  u32 lbm_size, tiling;
>> > -  unsigned long irqflags;
>> > +  u32 tiling;
>> >u32 hvs_format = format->hvs;
>> >int ret, i;
>> >  
>> > -  ret = vc4_plane_setup_clipping_and_scaling(state);
>> > -  if (ret)
>> > -  return ret;
>> > -
>> > -  /* Allocate the LBM memory that the HVS will use for temporary
>> > -   * storage due to our scaling/format conversion.
>> > -   */
>> > -  lbm_size = vc4_lbm_size(state);
>> > -  if (lbm_size) {
>> > -  if (!vc4_state->lbm.allocated) {
>> > -  spin_lock_irqsave(&vc4->hvs->mm_lock, irqflags);
>> > -  ret = drm_mm_insert_node_generic(&vc4->hvs->lbm_mm,
>> > -   &vc4_state->lbm,
>> > -   lbm_size, 32, 0, 0);
>> > -  spin_unlock_irqrestore(&vc4->hvs->mm_lock, irqflags);
>> > -  } else {
>> > -  WARN_ON_ONCE(lbm_size != vc4_state->lbm.size);
>> > -  }
>> > -  }
>> >  
>> > +  ret = vc4_plane_setup_clipping_and_scaling(state);
>> >if (ret)
>> >return ret;
>> >  
>> > @@ -717,15 +735,18 @@ static int vc4_plane_mode_set(struct drm_plane 
>> > *plane,
>> >vc4_dlist_write(vc4_state, SCALER_CSC2_ITR_R_601_5);
>> >}
>> >  
>> > +  vc4_state->lbm_offset = 0;
>> > +
>> >if (vc4_state->x_scaling[0] != VC4_SCALING_NONE ||
>> >vc4_state->x_scaling[1] != VC4_SCALING_NONE ||
>> >vc4_state->y_scaling[0] != VC4_SCALING_NONE ||
>> >vc4_state->y_scaling[1] != VC4_SCALING_NONE) {
>> > -  /* LBM Base Address. */
>> > +  /* Reserve a slot for the LBM Base Address. The real value will
>> > +   * be set when calling vc4_plane_update_lbm().
>> > +   */
>> >

Re: [PATCH 3/6] drm/vc4: Don't check plane state more than once

2018-11-19 Thread Eric Anholt
Boris Brezillon  writes:

> On Thu, 15 Nov 2018 12:41:36 -0800
> Eric Anholt  wrote:
>
>> Boris Brezillon  writes:
>> 
>> > We are about to use vc4_plane_mode_set() in the async check path, but
>> > async check can decide that async update is not possible and force the
>> > driver to fallback to a sync update.
>> >
>> > All the checks that have been done on the plane state during async check
>> > stay valid, and checking it again is not necessary. Add a ->checked
>> > field to vc4_plane_state, and use it to track the status of the state
>> > (checked or not).
>> >
>> > Signed-off-by: Boris Brezillon 
>> > ---
>> >  drivers/gpu/drm/vc4/vc4_drv.h   |  5 +
>> >  drivers/gpu/drm/vc4/vc4_plane.c | 10 ++
>> >  2 files changed, 15 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
>> > index 9ed05fb61eb6..d1000c4805c2 100644
>> > --- a/drivers/gpu/drm/vc4/vc4_drv.h
>> > +++ b/drivers/gpu/drm/vc4/vc4_drv.h
>> > @@ -370,6 +370,11 @@ struct vc4_plane_state {
>> > * to enable background color fill.
>> > */
>> >bool needs_bg_fill;
>> > +
>> > +  /* Mark the state as checked. Useful to avoid checking it twice when
>> > +   * async update is not possible.
>> > +   */
>> > +  bool checked;
>> >  };  
>> 
>> Since this doesn't cover the whole atomic_check process, which won't
>> have been called from async update, maybe rename to dlist_initialized or
>> something?
>
> Do you mean that vc4_plane_mode_set() should be run again from the sync
> check path when async check failed, or is it just the name you don't
> like?

Just the name, s/checked/dlist_initialized/ (or something)


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


[PATCH libdrm] xf86drm: Make drmNodeIsDRM() public.

2018-11-19 Thread Christopher James Halse Rogers
I have wanted this code in Mir, so it's plausibly useful elsewhere,
particularly if the DRM device major number is going to become
dynamic.

Signed-off-by: Christopher James Halse Rogers 

---
 xf86drm.c | 2 +-
 xf86drm.h | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/xf86drm.c b/xf86drm.c
index 10df682b..f32cb1bb 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -2767,7 +2767,7 @@ drm_public char *drmGetDeviceNameFromFd(int fd)
 return strdup(name);
 }
 
-static bool drmNodeIsDRM(int maj, int min)
+drm_public bool drmNodeIsDRM(int maj, int min)
 {
 #ifdef __linux__
 char path[64];
diff --git a/xf86drm.h b/xf86drm.h
index 7773d71a..6dc92180 100644
--- a/xf86drm.h
+++ b/xf86drm.h
@@ -37,6 +37,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #if defined(__cplusplus)
@@ -783,6 +784,8 @@ extern int drmPrimeFDToHandle(int fd, int prime_fd, 
uint32_t *handle);
 extern char *drmGetPrimaryDeviceNameFromFd(int fd);
 extern char *drmGetRenderDeviceNameFromFd(int fd);
 
+extern bool drmNodeIsDRM(int major, int minor);
+
 #define DRM_BUS_PCI   0
 #define DRM_BUS_USB   1
 #define DRM_BUS_PLATFORM  2
-- 
2.19.1

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


Re: [PATCH v5 05/12] drm: mediatek: Omit warning on probe defers

2018-11-19 Thread CK Hu
Hi,

On Tue, 2018-11-20 at 12:05 +0800, CK Hu wrote:
> Hi, Matthias:
> 
> On Mon, 2018-11-19 at 10:26 +0100, Matthias Brugger wrote:
> > 
> > On 19/11/2018 06:38, CK Hu wrote:
> > > Hi, Matthias:
> > > 
> > > On Fri, 2018-11-16 at 13:54 +0100, matthias@kernel.org wrote:
> > >> From: Matthias Brugger 
> > >>
> > >> It can happen that the mmsys clock drivers aren't probed before the
> > >> platform driver gets invoked. The platform driver used to print a warning
> > >> that the driver failed to get the clocks. Omit this error on
> > >> the defered probe path.
> > > 
> > > This patch looks good to me, but you have not modified the sub driver in
> > > HDMI path. We could let HDMI path print the warning and someone send
> > > another patch later, or you modify for HDMI path in this patch.
> > 
> > Sure, I'll add this in v6. After inspecting the code, I think we will need 
> > to
> > also check for not initialized clocks in mtk_mdp_comp_init, as the driver 
> > for
> > now does not even check if the clocks are present. What do you think?
> 
> Yes, we do really need to consider mdp driver because mmsys clock
> include mdp clock. You remind me that mmsys control 4 major function:
> drm routing, drm clock, mdp routing, and mdp clock. Your design let the
> mmsys device as drm device (control drm routing) and create a sub device
> as clock device (control drm clock, mdp clock). If one day mdp device
> (may need control drm routing) need to control the register of mdp

Sorry for typo: 'mdp device (may need control mdp routing)'

Regards,
CK

> routing, would mdp device be a sub device? Or we need not to consider
> this because it need not to access mmsys register now?
> 
> Regards,
> CK
> 
> > 
> > I'll address the coding style issue you metioned below as well.
> > 
> > Regards,
> > Matthias
> > 
> > >>
> > >> Signed-off-by: Matthias Brugger 
> > >> ---
> > >>  drivers/gpu/drm/mediatek/mtk_disp_color.c | 4 +++-
> > >>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c   | 4 +++-
> > >>  drivers/gpu/drm/mediatek/mtk_disp_rdma.c  | 4 +++-
> > >>  drivers/gpu/drm/mediatek/mtk_drm_ddp.c| 3 ++-
> > >>  drivers/gpu/drm/mediatek/mtk_dsi.c| 6 --
> > >>  5 files changed, 15 insertions(+), 6 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_color.c 
> > >> b/drivers/gpu/drm/mediatek/mtk_disp_color.c
> > >> index f609b62b8be6..1ea3178d4c18 100644
> > >> --- a/drivers/gpu/drm/mediatek/mtk_disp_color.c
> > >> +++ b/drivers/gpu/drm/mediatek/mtk_disp_color.c
> > >> @@ -126,7 +126,9 @@ static int mtk_disp_color_probe(struct 
> > >> platform_device *pdev)
> > >>  ret = mtk_ddp_comp_init(dev, dev->of_node, &priv->ddp_comp, 
> > >> comp_id,
> > >>  &mtk_disp_color_funcs);
> > >>  if (ret) {
> > >> -dev_err(dev, "Failed to initialize component: %d\n", 
> > >> ret);
> > >> +if (ret != -EPROBE_DEFER)
> > >> +dev_err(dev, "Failed to initialize component: 
> > >> %d\n",
> > >> +ret);
> > > 
> > > I would like one more blank line here.
> > > 
> > >>  return ret;
> > >>  }
> > >>  
> > >> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c 
> > >> b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > >> index 28d191192945..5ebbcaa4e70e 100644
> > >> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > >> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > >> @@ -293,7 +293,9 @@ static int mtk_disp_ovl_probe(struct platform_device 
> > >> *pdev)
> > >>  ret = mtk_ddp_comp_init(dev, dev->of_node, &priv->ddp_comp, 
> > >> comp_id,
> > >>  &mtk_disp_ovl_funcs);
> > >>  if (ret) {
> > >> -dev_err(dev, "Failed to initialize component: %d\n", 
> > >> ret);
> > >> +if (ret != -EPROBE_DEFER)
> > >> +dev_err(dev, "Failed to initialize component: 
> > >> %d\n",
> > >> +ret);
> > > 
> > > I would like to align to the right of '('.
> > > 
> > > Regards,
> > > CK
> > > 
> > >>  return ret;
> > >>  }
> > >>  
> > >> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c 
> > >> b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> > >> index b0a5cffe345a..59a08ed5fea5 100644
> > >> --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> > >> +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> > >> @@ -295,7 +295,9 @@ static int mtk_disp_rdma_probe(struct 
> > >> platform_device *pdev)
> > >>  ret = mtk_ddp_comp_init(dev, dev->of_node, &priv->ddp_comp, 
> > >> comp_id,
> > >>  &mtk_disp_rdma_funcs);
> > >>  if (ret) {
> > >> -dev_err(dev, "Failed to initialize component: %d\n", 
> > >> ret);
> > >> +if (ret != -EPROBE_DEFER)
> > >> +dev_err(dev, "Failed to initialize component: 
> > >> %d\n",
> > >> + 

Re: [PATCH v5 05/12] drm: mediatek: Omit warning on probe defers

2018-11-19 Thread CK Hu
Hi, Matthias:

On Mon, 2018-11-19 at 10:26 +0100, Matthias Brugger wrote:
> 
> On 19/11/2018 06:38, CK Hu wrote:
> > Hi, Matthias:
> > 
> > On Fri, 2018-11-16 at 13:54 +0100, matthias@kernel.org wrote:
> >> From: Matthias Brugger 
> >>
> >> It can happen that the mmsys clock drivers aren't probed before the
> >> platform driver gets invoked. The platform driver used to print a warning
> >> that the driver failed to get the clocks. Omit this error on
> >> the defered probe path.
> > 
> > This patch looks good to me, but you have not modified the sub driver in
> > HDMI path. We could let HDMI path print the warning and someone send
> > another patch later, or you modify for HDMI path in this patch.
> 
> Sure, I'll add this in v6. After inspecting the code, I think we will need to
> also check for not initialized clocks in mtk_mdp_comp_init, as the driver for
> now does not even check if the clocks are present. What do you think?

Yes, we do really need to consider mdp driver because mmsys clock
include mdp clock. You remind me that mmsys control 4 major function:
drm routing, drm clock, mdp routing, and mdp clock. Your design let the
mmsys device as drm device (control drm routing) and create a sub device
as clock device (control drm clock, mdp clock). If one day mdp device
(may need control drm routing) need to control the register of mdp
routing, would mdp device be a sub device? Or we need not to consider
this because it need not to access mmsys register now?

Regards,
CK

> 
> I'll address the coding style issue you metioned below as well.
> 
> Regards,
> Matthias
> 
> >>
> >> Signed-off-by: Matthias Brugger 
> >> ---
> >>  drivers/gpu/drm/mediatek/mtk_disp_color.c | 4 +++-
> >>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c   | 4 +++-
> >>  drivers/gpu/drm/mediatek/mtk_disp_rdma.c  | 4 +++-
> >>  drivers/gpu/drm/mediatek/mtk_drm_ddp.c| 3 ++-
> >>  drivers/gpu/drm/mediatek/mtk_dsi.c| 6 --
> >>  5 files changed, 15 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_color.c 
> >> b/drivers/gpu/drm/mediatek/mtk_disp_color.c
> >> index f609b62b8be6..1ea3178d4c18 100644
> >> --- a/drivers/gpu/drm/mediatek/mtk_disp_color.c
> >> +++ b/drivers/gpu/drm/mediatek/mtk_disp_color.c
> >> @@ -126,7 +126,9 @@ static int mtk_disp_color_probe(struct platform_device 
> >> *pdev)
> >>ret = mtk_ddp_comp_init(dev, dev->of_node, &priv->ddp_comp, comp_id,
> >>&mtk_disp_color_funcs);
> >>if (ret) {
> >> -  dev_err(dev, "Failed to initialize component: %d\n", ret);
> >> +  if (ret != -EPROBE_DEFER)
> >> +  dev_err(dev, "Failed to initialize component: %d\n",
> >> +  ret);
> > 
> > I would like one more blank line here.
> > 
> >>return ret;
> >>}
> >>  
> >> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c 
> >> b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> >> index 28d191192945..5ebbcaa4e70e 100644
> >> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> >> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> >> @@ -293,7 +293,9 @@ static int mtk_disp_ovl_probe(struct platform_device 
> >> *pdev)
> >>ret = mtk_ddp_comp_init(dev, dev->of_node, &priv->ddp_comp, comp_id,
> >>&mtk_disp_ovl_funcs);
> >>if (ret) {
> >> -  dev_err(dev, "Failed to initialize component: %d\n", ret);
> >> +  if (ret != -EPROBE_DEFER)
> >> +  dev_err(dev, "Failed to initialize component: %d\n",
> >> +  ret);
> > 
> > I would like to align to the right of '('.
> > 
> > Regards,
> > CK
> > 
> >>return ret;
> >>}
> >>  
> >> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c 
> >> b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> >> index b0a5cffe345a..59a08ed5fea5 100644
> >> --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> >> +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> >> @@ -295,7 +295,9 @@ static int mtk_disp_rdma_probe(struct platform_device 
> >> *pdev)
> >>ret = mtk_ddp_comp_init(dev, dev->of_node, &priv->ddp_comp, comp_id,
> >>&mtk_disp_rdma_funcs);
> >>if (ret) {
> >> -  dev_err(dev, "Failed to initialize component: %d\n", ret);
> >> +  if (ret != -EPROBE_DEFER)
> >> +  dev_err(dev, "Failed to initialize component: %d\n",
> >> +  ret);
> >>return ret;
> >>}
> >>  
> >> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c 
> >> b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> >> index b06cd9d4b525..b76a2d071a97 100644
> >> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> >> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> >> @@ -566,7 +566,8 @@ static int mtk_ddp_probe(struct platform_device *pdev)
> >>  
> >>ddp->clk = devm_clk_get(dev, NULL);
> >>if (IS_ERR(ddp->clk)) {
> >> -  dev_err(dev, "Failed to get clock\n");
> >> +  if (PTR_ERR(ddp->clk) != -E

[Bug 108806] AMDGPU Failed to read EDID from display

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108806

Michael Lindman  changed:

   What|Removed |Added

   Hardware|Other   |x86-64 (AMD64)
 OS|All |Linux (All)

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


[Bug 108806] AMDGPU Failed to read EDID from display

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108806

Bug ID: 108806
   Summary: AMDGPU Failed to read EDID from display
   Product: DRI
   Version: unspecified
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/AMDgpu
  Assignee: dri-devel@lists.freedesktop.org
  Reporter: michael.lind...@gmail.com

Created attachment 142526
  --> https://bugs.freedesktop.org/attachment.cgi?id=142526&action=edit
boot log showing EDID failure

When connecting one of my monitors to the system it defaults to 1024x768 and
not the native resolution and grepping through the kernel logs shows that
AMDGPU fails to read the EDID of the display.

[5.054779] [drm:dc_link_detect [amdgpu]] *ERROR* No EDID read.

The display was working fine as of kernel 4.18.18 with the issues arising upon
4.19.2.

I am using a POLARIS 10 GPU and both displays are connected to DisplayPort with
the Asus PG2780 having the issues with failing to grab the EDID.

DisplayPort-0 LG 25UM65-P
DisplayPort-1 ASUS PG278Q

-- 
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 libdrm] xf86drm: Add drmIsMaster()

2018-11-19 Thread Christopher James Halse Rogers
We can't use drmSetMaster to query whether or not a drm fd is master
because it requires CAP_SYS_ADMIN, even if the fd *is* a master fd.

Pick DRM_IOCTL_MODE_ATTACHMODE as a long-deprecated ioctl that is
DRM_MASTER but not DRM_ROOT_ONLY as the probe by which we can detect
whether or not the fd is master.

Signed-off-by: Christopher James Halse Rogers 

---
 xf86drm.c | 20 
 xf86drm.h |  2 ++
 2 files changed, 22 insertions(+)

diff --git a/xf86drm.c b/xf86drm.c
index 10df682b..bdb0439d 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -2741,6 +2741,26 @@ drm_public int drmDropMaster(int fd)
 return drmIoctl(fd, DRM_IOCTL_DROP_MASTER, NULL);
 }
 
+drm_public bool drmIsMaster(int fd)
+{
+struct drm_mode_mode_cmd cmd;
+
+memclear(cmd);
+/* Set an invalid connector_id to ensure that ATTACHMODE errors with
+ * EINVAL in the unlikely event someone feels like calling this on a
+ * kernel prior to 3.9. */
+cmd.connector_id = -1;
+
+if (drmIoctl(fd, DRM_IOCTL_MODE_ATTACHMODE, &cmd) != -1)
+{
+/* On 3.9 ATTACHMODE was changed to drm_noop, and so will succeed
+* iff we've got a master fd */
+return true;
+}
+
+return errno == EINVAL;
+}
+
 drm_public char *drmGetDeviceNameFromFd(int fd)
 {
 char name[128];
diff --git a/xf86drm.h b/xf86drm.h
index 7773d71a..9e920db9 100644
--- a/xf86drm.h
+++ b/xf86drm.h
@@ -37,6 +37,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #if defined(__cplusplus)
@@ -733,6 +734,7 @@ extern void drmMsg(const char *format, ...) 
DRM_PRINTFLIKE(1, 2);
 
 extern int drmSetMaster(int fd);
 extern int drmDropMaster(int fd);
+extern bool drmIsMaster(int fd);
 
 #define DRM_EVENT_CONTEXT_VERSION 4
 
-- 
2.19.1

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


[PATCH] drm: rcar-du: Reject modes that fail CRTC timing requirements

2018-11-19 Thread Laurent Pinchart
The hardware requires the HDSR and VDSR registers to be set to 1 or
higher. This translates to a minimum combined horizontal sync and back
porch of 20 pixels and a minimum vertical back porch of 3 lines. Reject
modes that fail those requirements.

Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c 
b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 79021d7aa3ce..9d425894e000 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -739,6 +739,15 @@ enum drm_mode_status rcar_du_crtc_mode_valid(struct 
drm_crtc *crtc,
if (interlaced && !rcar_du_has(rcdu, RCAR_DU_FEATURE_INTERLACED))
return MODE_NO_INTERLACE;
 
+   /*
+* The hardware requires a minimum combined horizontal sync and back
+* porch of 20 pixels and a minimum vertical back porch of 3 lines.
+*/
+   if (mode->htotal - mode->hsync_start < 20)
+   return MODE_HBLANK_NARROW;
+   if (mode->crtc_vtotal - mode->crtc_vsync_end < 3)
+   return MODE_VBLANK_NARROW;
+
return MODE_OK;
 }
 
-- 
Regards,

Laurent Pinchart

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


[Bug 108781] 4.19 Regression - Hawaii (R9 390) boot failure - Invalid PCC GPIO / invalid powerlevel state / Fatal error during GPU init

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108781

--- Comment #6 from jamespharve...@gmail.com ---
Alex Deucher, unfortunately the patch on bug 108704 has no effect.

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


linux-next: manual merge of the staging tree with the drm tree

2018-11-19 Thread Stephen Rothwell
Hi Greg,

Today's linux-next merge of the staging tree got a conflict in:

  drivers/staging/vboxvideo/vbox_ttm.c

between commits:

  a64f784bb14a ("drm/ttm: initialize globals during device init (v2)")

from the drm tree and commit:

  cd76c287a52f ("staging: vboxvideo: Cleanup the comments")

from the staging tree.

I fixed it up (the former removed the comments updated by the latter) and
can carry the fix as necessary. This is now fixed as far as linux-next
is concerned, but any non trivial conflicts should be mentioned to your
upstream maintainer when your tree is submitted for merging.  You may
also want to consider cooperating with the maintainer of the conflicting
tree to minimise any particularly complex conflicts.



-- 
Cheers,
Stephen Rothwell


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


Re: [PATCH v9 22/24] drm/i915/fec: Set FEC_READY in FEC_CONFIGURATION

2018-11-19 Thread Manasi Navare
On Mon, Nov 19, 2018 at 10:19:42PM +0200, Ville Syrjälä wrote:
> On Tue, Nov 13, 2018 at 05:52:30PM -0800, Manasi Navare wrote:
> > From: Anusha Srivatsa 
> > 
> > If the panel supports FEC, the driver has to
> > set the FEC_READY bit in the dpcd register:
> > FEC_CONFIGURATION.
> > 
> > This has to happen before link training.
> > 
> > v2: s/intel_dp_set_fec_ready/intel_dp_sink_set_fec_ready
> >- change commit message. (Gaurav)
> > 
> > v3: rebased. (r-b Manasi)
> > 
> > v4: Use fec crtc state, before setting FEC_READY
> > bit. (Anusha)
> > 
> > v5: Move to intel_ddi.c
> > - Make the function static (Anusha)
> > 
> > v6: Dont pass state as a separate argument (Ville)
> > 
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: Gaurav K Singh 
> > Cc: Jani Nikula 
> > Cc: Ville Syrjala 
> > Cc: Manasi Navare 
> > Signed-off-by: Anusha Srivatsa 
> > Reviewed-by: Manasi Navare 
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c | 11 +++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index 0638fd2febfb..2d15520f13c5 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -3102,6 +3102,16 @@ static void icl_program_mg_dp_mode(struct 
> > intel_digital_port *intel_dig_port)
> > I915_WRITE(MG_DP_MODE(port, 1), ln1);
> >  }
> >  
> > +static void intel_dp_sink_set_fec_ready(struct intel_dp *intel_dp,
> > +   const struct intel_crtc_state 
> > *crtc_state)
> > +{
> > +   if (!crtc_state->fec_enable)
> > +   return;
> > +
> > +   if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_FEC_CONFIGURATION, 
> > DP_FEC_READY) <= 0)
> > +   DRM_DEBUG_KMS("Failed to get FEC enabled in sink\n");
> 
> The debug message is still bonkers.

Havent we always had a debug print on drm_dp_dpcd_write() calls?

Manasi

> 
> > +}
> > +
> >  static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
> > const struct intel_crtc_state *crtc_state,
> > const struct drm_connector_state 
> > *conn_state)
> > @@ -3142,6 +3152,7 @@ static void intel_ddi_pre_enable_dp(struct 
> > intel_encoder *encoder,
> > intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
> > intel_dp_sink_set_decompression_state(intel_dp, crtc_state,
> >   true);
> > +   intel_dp_sink_set_fec_ready(intel_dp, crtc_state);
> > intel_dp_start_link_train(intel_dp);
> > if (port != PORT_A || INTEL_GEN(dev_priv) >= 9)
> > intel_dp_stop_link_train(intel_dp);
> > -- 
> > 2.19.1
> 
> -- 
> Ville Syrjälä
> Intel
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 106175] amdgpu.dc=1 shows performance issues with Xorg compositors when moving windows

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=106175

--- Comment #53 from bmil...@gmail.com ---
(In reply to rropid from comment #45)
> (In reply to bmilreu from comment #43)
> > If devs want an easy test case, use these links for reproducing it in
> > chromium:
> > 
> > https://www.vsynctester.com/
> > https://www.testufo.com/photo
> > https://www.slither.io
> > 
> > move the cursor around, move/resize some windows. you will notice it
> > 
> > the vsync/cursor stutters and frame-skips are pretty noticeable with dc=1 on
> > all three links
> > 
> > KWin, compton, TearFree, mutter, xfwm4 all have the same problems.
> 
> I just tried dc=1 and I only seem to have a problem if I use TearFree.
> Things are totally fine without TearFree.
> 
> To be clear about what I'm doing here right now:
> 
> I made sure DC is enabled:
> 
>   $ systool -vm amdgpu | grep dc
>   dc  = "1"
>   $ dmesg | grep -i display
>   [1.014297] [drm] Display Core initialized with v3.1.59!
> 
> I removed TearFree from my X config:
> 
>   $ cat /etc/X11/xorg.conf.d/20-amdgpu.conf 
>   Section "OutputClass"
>   Identifier "my amdgpu settings"
>   MatchDriver "amdgpu"
>   Option "DRI" "3"
>   EndSection
> 
> And I started Compton like this to make sure it's a clean config:
> 
>   $ compton --config /dev/null --backend glx --vsync opengl
> 
> With this setup, I don't seem to have any stutter. I visited the websites
> you mention in a Chromium window, then opened another window and tried
> moving things around and resizing. It behaves fine, same as what I know from
> normally using dc=0.
> 
> Kernel is 4.19.2, Mesa 18.2.4, Xorg 1.20.3, the GPU is a RX480, monitor is
> 60 Hz.
> 
> After I had typed this, I have now added TearFree to the X config and
> restarted X:
> 
>   $ cat /etc/X11/xorg.conf.d/20-amdgpu.conf 
>   Section "OutputClass"
>   Identifier "my amdgpu settings"
>   MatchDriver "amdgpu"
>   Option "TearFree" "true"
>   Option "DRI" "3"
>   EndSection
> 
> Now, with TearFree enabled, things are super terrible. Trying to move a
> window around has extreme stutter, it seems to drop frames. If I restart
> Compton with "GALLIUM_HUD=fps" and then try moving a window around in
> circles, I can see it stays below 40 fps instead of hitting the 60 fps that
> it should be running at.

"compton --vsync opengl" is a case less/not affected by this in my setup, try
--vsync opengl-swc, --vsync opengl-oml or --vsync opengl-mswc

Also try other compositors. Kwin, mutter, xfwm4

-- 
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] [PATCH 1/2] drm/edid: Add and export function to parse manufacturer id

2018-11-19 Thread Souza, Jose
On Tue, 2018-11-13 at 22:07 +0200, Jani Nikula wrote:
> On Thu, 08 Nov 2018, Daniel Vetter  wrote:
> > On Thu, Nov 08, 2018 at 08:42:52PM +, Souza, Jose wrote:
> > > On Thu, 2018-11-08 at 09:31 +0100, Daniel Vetter wrote:
> > > > On Wed, Nov 07, 2018 at 04:23:52PM -0800, José Roberto de Souza
> > > > wrote:
> > > > > This function will be helpful to drivers that wants to add
> > > > > its own
> > > > > quirks to sinks.
> > > > 
> > > > Why would you want to do that? The point of a shared edid
> > > > parsing
> > > > code is
> > > > that we can share all these quirks ...
> > > > 
> > > > For these kind of patches, always include the driver code that
> > > > makes
> > > > use
> > > > of your new code too. That makes it much easier to answer these
> > > > questions.
> > > 
> > > This will be used to disable or enable with quirks PSR in some
> > > panels
> > > that do not behave like eDP spec states.
> > > As this would be specifc to i915, I guess is better keep the list
> > > only
> > > in i915.
> > > 
> > > What is your opinion about that?
> > 
> > For anything dp, shouldn't we use the OUI instead? Or is that more
> > garbage
> > than the EDID serial?
> 
> The OUI isn't always present, but we can use it when it is. And we
> already have DPCD quirk support for that in drm_dp_helper.c.

I was not aware about OUI, thanks.

OUI is a requirement in DP 1.2+ and PSR was added in DP 1.3, so I can
safely use it.

> 
> > And yes, psr quirking for i915 seems like a reasonable thing to do,
> > using
> > either approach. But definitely include the full picture, including
> > i915
> > patches, in your next round.
> 
> I think all of the quirk matching should be in drm core or helpers.
> For
> most quirks, it's up to the drivers to actually do something with
> that
> information anyway, so it'll still remain i915 specific.

The edid quirks are returned from drm_add_display_info() and it is used
only to updated some connector parameters in drm_add_edid_modes().

Also the use that we have right now is blacklist all panels from a
vendor that have its own proprietary PSR-like implementation, so that
use would not fit in the current drm quirk list.

So for now I can use the OUI to check if the panel vendor match and if
so not enable PSR.

> 
> BR,
> Jani.
> 
> 
> 
> 
> > -Daniel
> > 
> > > > Thanks, Daniel
> > > > 
> > > > 
> > > > > Signed-off-by: José Roberto de Souza 
> > > > > ---
> > > > >  drivers/gpu/drm/drm_edid.c | 20 
> > > > >  include/drm/drm_edid.h |  1 +
> > > > >  2 files changed, 17 insertions(+), 4 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/drm_edid.c
> > > > > b/drivers/gpu/drm/drm_edid.c
> > > > > index b506e3622b08..1a0ddf3d326b 100644
> > > > > --- a/drivers/gpu/drm/drm_edid.c
> > > > > +++ b/drivers/gpu/drm/drm_edid.c
> > > > > @@ -1755,6 +1755,21 @@ EXPORT_SYMBOL(drm_edid_duplicate);
> > > > >  
> > > > >  /*** EDID parsing ***/
> > > > >  
> > > > > +/**
> > > > > + * drm_edid_manufacturer_parse - parse the EDID manufacturer
> > > > > id to
> > > > > readable
> > > > > + * characters and set into manufacturer parameter.
> > > > > + * @edid: EDID to get the manufacturer
> > > > > + * @manufacturer: the char buffer to store the id
> > > > > + */
> > > > > +void drm_edid_manufacturer_parse(const struct edid *edid,
> > > > > char
> > > > > manufacturer[3])
> > > > > +{
> > > > > + manufacturer[0] = ((edid->mfg_id[0] & 0x7c) >> 2) +
> > > > > '@';
> > > > > + manufacturer[1] = (((edid->mfg_id[0] & 0x3) << 3) |
> > > > > +   ((edid->mfg_id[1] & 0xe0) >> 5)) +
> > > > > '@';
> > > > > + manufacturer[2] = (edid->mfg_id[1] & 0x1f) + '@';
> > > > > +}
> > > > > +EXPORT_SYMBOL(drm_edid_manufacturer_parse);
> > > > > +
> > > > >  /**
> > > > >   * edid_vendor - match a string against EDID's obfuscated
> > > > > vendor
> > > > > field
> > > > >   * @edid: EDID to match
> > > > > @@ -1766,10 +1781,7 @@ static bool edid_vendor(const struct
> > > > > edid
> > > > > *edid, const char *vendor)
> > > > >  {
> > > > >   char edid_vendor[3];
> > > > >  
> > > > > - edid_vendor[0] = ((edid->mfg_id[0] & 0x7c) >> 2) + '@';
> > > > > - edid_vendor[1] = (((edid->mfg_id[0] & 0x3) << 3) |
> > > > > -   ((edid->mfg_id[1] & 0xe0) >> 5)) +
> > > > > '@';
> > > > > - edid_vendor[2] = (edid->mfg_id[1] & 0x1f) + '@';
> > > > > + drm_edid_manufacturer_parse(edid, edid_vendor);
> > > > >  
> > > > >   return !strncmp(edid_vendor, vendor, 3);
> > > > >  }
> > > > > diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> > > > > index e3c404833115..e4f3f7f34d6a 100644
> > > > > --- a/include/drm/drm_edid.h
> > > > > +++ b/include/drm/drm_edid.h
> > > > > @@ -466,6 +466,7 @@ struct edid
> > > > > *drm_get_edid_switcheroo(struct
> > > > > drm_connector *connector,
> > > > >struct i2c_adapter
> > > > > *adapter);
> > > > >  struct edid *drm_edid_duplicate(const struct edid *edid);

[Bug 106175] amdgpu.dc=1 shows performance issues with Xorg compositors when moving windows

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=106175

--- Comment #52 from Brandon Wright  ---
Ok, I think I understand what's going on. Forgive me if this sounds stupid, I'm
looking at the DRM code for the first time.

The old KMS interface uses what's flagged as "legacy" cursor updates. These are
"asynchronous" in that they're handled and passed to the hardware as they come
in. On the vertical retrace interrupt, it uses whatever the last data passed in
was. 

My theory is the DC interface isn't passing these on to the hardware
immediately. It's aggregating them until the next sync, when they're all
handled at once. And that is what's causing the disturbance at page-flip time.
High-report-rate mice might exacerbate it.

Intel's driver hasn't merged that async code yet. It's still using legacy
cursor updates and working around this.

The DC code seems to have a TODO comment in amdgpu_dm.c that suggests something
about the legacy_cursor_update flag, but it doesn't do anything with it.

-- 
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 4.20 regression fix] drm/i915: Revert "Fix assert_plane() warning on bootup with external display"

2018-11-19 Thread Hans de Goede
Starting with 4.20-rc1 I'm seeing the LCD screen briefly turn mostly purple
on devices with a DSI panel (seen on 2 different devices with a DSI panel).

This happens both with and without fastboot=1. This is caused by
commit 516a49cc1946 ("drm/i915: Fix assert_plane() warning on bootup with
external display").

And a user commenting on fdo bug 108225 has reported a flicker caused by
the screen briefly turning blank when booting with fastboot=1, which goes
away when reverting this commit.

I believe a revert of the offending commit is the best solution because
the commit introduces a drm_atomic_commit() call in the drivers' probe
path, causing writes to the hardware during probe.
I believe this goes agains the design of the driver which is to only
read-back state on probe and only write to the hardware on the first
commit from the fbcon or userspace.

Instead the assert_plane() problem the commit tried to fix should be fixed
by fixing the state read-back code.

Related: https://bugs.freedesktop.org/show_bug.cgi?id=108225
Cc: Azhar Shaikh 
Cc: Ville Syrjälä 
Signed-off-by: Hans de Goede 
---
 drivers/gpu/drm/i915/intel_display.c | 61 +---
 1 file changed, 2 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 2107de6da692..03dec1c05652 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15193,61 +15193,12 @@ static void intel_update_fdi_pll_freq(struct 
drm_i915_private *dev_priv)
DRM_DEBUG_DRIVER("FDI PLL freq=%d\n", dev_priv->fdi_pll_freq);
 }
 
-static int intel_initial_commit(struct drm_device *dev)
-{
-   struct drm_atomic_state *state = NULL;
-   struct drm_modeset_acquire_ctx ctx;
-   struct drm_crtc *crtc;
-   struct drm_crtc_state *crtc_state;
-   int ret = 0;
-
-   state = drm_atomic_state_alloc(dev);
-   if (!state)
-   return -ENOMEM;
-
-   drm_modeset_acquire_init(&ctx, 0);
-
-retry:
-   state->acquire_ctx = &ctx;
-
-   drm_for_each_crtc(crtc, dev) {
-   crtc_state = drm_atomic_get_crtc_state(state, crtc);
-   if (IS_ERR(crtc_state)) {
-   ret = PTR_ERR(crtc_state);
-   goto out;
-   }
-
-   if (crtc_state->active) {
-   ret = drm_atomic_add_affected_planes(state, crtc);
-   if (ret)
-   goto out;
-   }
-   }
-
-   ret = drm_atomic_commit(state);
-
-out:
-   if (ret == -EDEADLK) {
-   drm_atomic_state_clear(state);
-   drm_modeset_backoff(&ctx);
-   goto retry;
-   }
-
-   drm_atomic_state_put(state);
-
-   drm_modeset_drop_locks(&ctx);
-   drm_modeset_acquire_fini(&ctx);
-
-   return ret;
-}
-
 int intel_modeset_init(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = to_i915(dev);
struct i915_ggtt *ggtt = &dev_priv->ggtt;
enum pipe pipe;
struct intel_crtc *crtc;
-   int ret;
 
dev_priv->modeset_wq = alloc_ordered_workqueue("i915_modeset", 0);
 
@@ -15319,6 +15270,8 @@ int intel_modeset_init(struct drm_device *dev)
  INTEL_INFO(dev_priv)->num_pipes > 1 ? "s" : "");
 
for_each_pipe(dev_priv, pipe) {
+   int ret;
+
ret = intel_crtc_init(dev_priv, pipe);
if (ret) {
drm_mode_config_cleanup(dev);
@@ -15374,16 +15327,6 @@ int intel_modeset_init(struct drm_device *dev)
if (!HAS_GMCH_DISPLAY(dev_priv))
sanitize_watermarks(dev);
 
-   /*
-* Force all active planes to recompute their states. So that on
-* mode_setcrtc after probe, all the intel_plane_state variables
-* are already calculated and there is no assert_plane warnings
-* during bootup.
-*/
-   ret = intel_initial_commit(dev);
-   if (ret)
-   DRM_DEBUG_KMS("Initial commit in probe failed.\n");
-
return 0;
 }
 
-- 
2.19.1

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


Re: [PATCH] drm/i915: change i915_sw_fence license to MIT

2018-11-19 Thread Jonathan Gray
On Mon, Nov 19, 2018 at 10:09:33AM -0800, Rodrigo Vivi wrote:
> On Sun, Nov 18, 2018 at 08:44:30PM +1100, Jonathan Gray wrote:
> > On Wed, Oct 31, 2018 at 08:43:03AM +, Chris Wilson wrote:
> > > Quoting Jonathan Gray (2018-10-31 00:56:12)
> > > > Chris Wilson said Intel is willing to change the license of these files
> > > > to MIT.
> > > > 
> > > > Signed-off-by: Jonathan Gray 
> > > Reviewed-by: Chris Wilson 
> > > -Chris
> > 
> > Still looking to get this merged/reviewed by one of the i915 maintainers.
> 
> I believe this patch should have a better commit message.

It isn't clear what should be changed, what do you have in mind?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v9 20/24] drm/i915/dsc: Add Per connector debugfs node for DSC support/enable

2018-11-19 Thread Manasi Navare
On Mon, Nov 19, 2018 at 10:27:44PM +0200, Ville Syrjälä wrote:
> On Thu, Nov 15, 2018 at 05:39:42PM -0800, Manasi Navare wrote:
> > On Tue, Nov 13, 2018 at 05:52:28PM -0800, Manasi Navare wrote:
> > > DSC can be supported per DP connector. This patch adds a per connector
> > > debugfs node to expose DSC support capability by the kernel.
> > > The same node can be used from userspace to force DSC enable.
> > > 
> > > force_dsc_en written through this debugfs node is used to force
> > > DSC even for lower resolutions.
> > > 
> > > v4:
> > > * Add missed connector_status check (Manasi)
> > > * Create i915_dsc_support node only for Gen >=10 (manasi)
> > > * Access intel_dp->dsc_dpcd only if its not NULL (Manasi)
> > > v3:
> > > * Combine Force_dsc_en with this patch (Ville)
> > > v2:
> > > * Use kstrtobool_from_user to avoid explicit error checking (Lyude)
> > > * Rebase on drm-tip (Manasi)
> > > 
> > > Cc: Rodrigo Vivi 
> > > Cc: Ville Syrjala 
> > > Cc: Anusha Srivatsa 
> > > Cc: Lyude Paul 
> > > Signed-off-by: Manasi Navare 
> > > Reviewed-by: Lyude Paul 
> > > ---
> > >  drivers/gpu/drm/i915/i915_debugfs.c | 77 +
> > >  drivers/gpu/drm/i915/intel_dp.c |  3 +-
> > >  drivers/gpu/drm/i915/intel_drv.h|  3 ++
> > >  3 files changed, 82 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
> > > b/drivers/gpu/drm/i915/i915_debugfs.c
> > > index 670db5073d70..3c112bb225d6 100644
> > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > @@ -5074,6 +5074,76 @@ static int i915_hdcp_sink_capability_show(struct 
> > > seq_file *m, void *data)
> > >  }
> > >  DEFINE_SHOW_ATTRIBUTE(i915_hdcp_sink_capability);
> > >  
> > > +static int i915_dsc_support_show(struct seq_file *m, void *data)
> > > +{
> > > + struct drm_connector *connector = m->private;
> > > + struct intel_encoder *encoder = intel_attached_encoder(connector);
> > > + struct intel_dp *intel_dp =
> > > + enc_to_intel_dp(&encoder->base);
> > > + struct intel_crtc *crtc;
> > > + struct intel_crtc_state *crtc_state;
> > > +
> > > + if (connector->status != connector_status_connected)
> > > + return -ENODEV;
> > > +
> > > + crtc = to_intel_crtc(encoder->base.crtc);
> > > + crtc_state = to_intel_crtc_state(crtc->base.state);
> 
> This stuff is broken. If you want to root around in the atomic states
> you need to grab some locks first.

I thought using the drm_modeset_lock before grabbing the crtc_state value should
be enough.

Are you referring to using the locks like:
drm_modeset_lock_single_interruptible()?

> 
> > > + drm_modeset_lock(&crtc->base.mutex, NULL);
> > > + seq_printf(m, "Enabled: %s\n",
> > > +yesno(crtc_state->dsc_params.compression_enable));
> > > + if (intel_dp->dsc_dpcd)
> > > + seq_printf(m, "Supported: %s\n",
> > > +yesno(drm_dp_sink_supports_dsc(intel_dp->dsc_dpcd)));
> > 
> > I think this supported should also use the intel_dp_supports_dsc() helper so
> > it checks for sink support and platform support as well.
> > 
> > Ville, what do you think?
> 
> Possibly. We may want to report per-crtc whether DSC is possible or not
> (due to transcoder A not being capable of DSC). And to do that we
> actually need to change intel_dp_source_supports_dsc() & co. to not take
> the crtc state, and rather we just need to pass in the transcoder. That
> would mean we have to determine the correct transcoder here. Perhaps
> a better alternative would be to pass in pipe/crtc + port and let the
> function derive the transcoder itself. That would keep the
> pipe+port->transcoder logic in one place. Actually we have that logic
> in two places at least already (intel_ddi_compute_config() and
> intel_ddi_connector_get_hw_state()). We could extract that bit into
> a small helper that gets called by all three places.

So there would be Sink_Supported and Enabled separately and then for 
for_each_crtc loop and report out Source_supported for every crtc
by passing port and pipe/crtc and determining transcoder from that find
the intel_dp_source_supports_dsc for that transcoder ?

Is this the correct way?
Also should this be kept separately in a follow on series and get the simpler
version of intel_dp_supports_dsc() merged first?

Manasi

> 
> > 
> > Manasi
> > 
> > > + drm_modeset_unlock(&crtc->base.mutex);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static ssize_t i915_dsc_support_write(struct file *file,
> > > +   const char __user *ubuf,
> > > +   size_t len, loff_t *offp)
> > > +{
> > > + bool dsc_enable = false;
> > > + int ret;
> > > + struct drm_connector *connector =
> > > + ((struct seq_file *)file->private_data)->private;
> > > + struct intel_encoder *encoder = intel_attached_encoder(connector);
> > > + struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> > > +
> > > + if (len == 0)
> > > + return 0;

Re: [PATCH v9 01/24] drm/dsc: Modify DRM helper to return complete DSC color depth capabilities

2018-11-19 Thread Manasi Navare
On Mon, Nov 19, 2018 at 10:33:37PM +0200, Ville Syrjälä wrote:
> On Mon, Nov 19, 2018 at 12:10:47PM -0800, Manasi Navare wrote:
> > On Mon, Nov 19, 2018 at 09:43:38PM +0200, Ville Syrjälä wrote:
> > > On Tue, Nov 13, 2018 at 05:52:09PM -0800, Manasi Navare wrote:
> > > > DSC DPCD color depth register advertises its color depth capabilities
> > > > by setting each of the bits that corresponding to a specific color
> > > > depth. This patch defines those specific color depths and adds
> > > > a helper to return an array of color depth capabilities.
> > > > 
> > > > Signed-off-by: Manasi Navare 
> > > > Cc: Ville Syrjala 
> > > > ---
> > > >  drivers/gpu/drm/drm_dp_helper.c | 29 +++--
> > > >  include/drm/drm_dp_helper.h |  9 +
> > > >  2 files changed, 24 insertions(+), 14 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/drm_dp_helper.c 
> > > > b/drivers/gpu/drm/drm_dp_helper.c
> > > > index 6d483487f2b4..286567063960 100644
> > > > --- a/drivers/gpu/drm/drm_dp_helper.c
> > > > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > > > @@ -1428,17 +1428,26 @@ u8 drm_dp_dsc_sink_line_buf_depth(const u8 
> > > > dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
> > > >  }
> > > >  EXPORT_SYMBOL(drm_dp_dsc_sink_line_buf_depth);
> > > >  
> > > > -u8 drm_dp_dsc_sink_max_color_depth(const u8 
> > > > dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
> > > > +void drm_dp_dsc_sink_color_depth_cap(const u8 
> > > > dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
> > > > +u8 *dsc_sink_color_depth_cap)
> > > >  {
> > > > +   int i, cnt = 0;
> > > > u8 color_depth = dsc_dpcd[DP_DSC_DEC_COLOR_DEPTH_CAP - 
> > > > DP_DSC_SUPPORT];
> > > >  
> > > > -   if (color_depth & DP_DSC_12_BPC)
> > > > -   return 12;
> > > > -   if (color_depth & DP_DSC_10_BPC)
> > > > -   return 10;
> > > > -   if (color_depth & DP_DSC_8_BPC)
> > > > -   return 8;
> > > > -
> > > > -   return 0;
> > > > +   for (i = 1; i <= 3; i++) {
> > > > +   if (!(color_depth & BIT(i)))
> > > > +   continue;
> > > > +   switch (i) {
> > > > +   case 1:
> > > > +   dsc_sink_color_depth_cap[cnt++] = DP_DSC_8_BPC;
> > > > +   break;
> > > > +   case 2:
> > > > +   dsc_sink_color_depth_cap[cnt++] = DP_DSC_10_BPC;
> > > > +   break;
> > > > +   case 3:
> > > > +   dsc_sink_color_depth_cap[cnt++] = DP_DSC_12_BPC;
> > > > +   break;
> > > > +   }
> > > > +   }
> > > >  }
> > > > -EXPORT_SYMBOL(drm_dp_dsc_sink_max_color_depth);
> > > > +EXPORT_SYMBOL(drm_dp_dsc_sink_color_depth_cap);
> > > > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> > > > index 3314e91f6eb3..ea3233b0a790 100644
> > > > --- a/include/drm/drm_dp_helper.h
> > > > +++ b/include/drm/drm_dp_helper.h
> > > > @@ -242,9 +242,9 @@
> > > >  # define DP_DSC_YCbCr420_Native (1 << 4)
> > > >  
> > > >  #define DP_DSC_DEC_COLOR_DEPTH_CAP  0x06A
> > > > -# define DP_DSC_8_BPC   (1 << 1)
> > > > -# define DP_DSC_10_BPC  (1 << 2)
> > > > -# define DP_DSC_12_BPC  (1 << 3)
> > > > +# define DP_DSC_8_BPC   8
> > > > +# define DP_DSC_10_BPC  10
> > > > +# define DP_DSC_12_BPC  12
> > > 
> > > 
> > > I'd suggest something simpler like:
> > > 
> > > int foo(u8 bpc[3])
> > 
> > Is passing a full array recommended method vs passing the pointer to the 
> > array?
> 
> It's the same thing in C. The compiler will treat both as a pointer
> (eg. sadly you can't use ARRAY_SIZE() on this because it will just use
> the size of the pointer rather than the size of the array in the
> calculation).
> 
> Despite the language shortcomings I like to use the array notation
> as a means to document what is the expected size of the passed in
> array.
>

Ok will change it to use array notation
 
> > 
> > > {
> > >   int num_bpc = 0;
> > > 
> > >   if (color_depth & DP_DSC_12_BPC)
> > >   bpc[num_bpc++] = 12;
> > >   if (color_depth & DP_DSC_10_BPC)
> > >   bpc[num_bpc++] = 10;
> > >   if (color_depth & DP_DSC_8_BPC)
> > >   bpc[num_bpc++] = 8;
> > > 
> > >   return num_bpc;
> > > }
> > 
> > Yes I could modify like above except start from lowest bpc in bpc[0] going 
> > all the way to highest.
> 
> Highest to lowest seems more sensible to me since we want to pick the
> max. So when we walk the array we can bail as soon as we find the
> highest suitable value.

Yes so bail as soon as the value is less than or equal to the max determined 
from the user requested bpc, right?

Manasi

> 
> > Also as of now its only 3 bpcs so its safe to just have an array for 3 bpcs 
> > for now right?
> 
> Yes. Adding more would require changing the function anyway,

Re: [PATCH v9 08/24] drm/i915/dp: Compute DSC pipe config in atomic check

2018-11-19 Thread Manasi Navare
Thanks for the comments, please some my answers below:

On Mon, Nov 19, 2018 at 10:11:04PM +0200, Ville Syrjälä wrote:
> On Tue, Nov 13, 2018 at 05:52:16PM -0800, Manasi Navare wrote:
> > DSC params like the enable, compressed bpp, slice count and
> > dsc_split are added to the intel_crtc_state. These parameters
> > are set based on the requested mode and available link parameters
> > during the pipe configuration in atomic check phase.
> > These values are then later used to populate the remaining DSC
> > and RC parameters before enbaling DSC in atomic commit.
> > 
> > v13:
> > * Compute DSC bpc only when DSC is req to be enabled (Ville)
> > v12:
> > * Override bpp with dsc dpcd color depth (Manasi)
> > v11:
> > * Const crtc_state, reject DSC on DP without FEC (Ville)
> > * Dont set dsc_split to false (Ville)
> > v10:
> > * Add a helper for dp_dsc support (Ville)
> > * Set pipe_config to max bpp, link params for DSC for now (Ville)
> > * Compute bpp - use dp dsc support helper (Ville)
> > v9:
> > * Rebase on top of drm-tip that now uses fast_narrow config
> > for edp (Manasi)
> > v8:
> > * Check for DSC bpc not 0 (manasi)
> > 
> > v7:
> > * Fix indentation in compute_m_n (Manasi)
> > 
> > v6 (From Gaurav):
> > * Remove function call of intel_dp_compute_dsc_params() and
> > invoke intel_dp_compute_dsc_params() in the patch where
> > it is defined to fix compilation warning (Gaurav)
> > 
> > v5:
> > Add drm_dsc_cfg in intel_crtc_state (Manasi)
> > 
> > v4:
> > * Rebase on refactoring of intel_dp_compute_config on tip (Manasi)
> > * Add a comment why we need to check PSR while enabling DSC (Gaurav)
> > 
> > v3:
> > * Check PPR > max_cdclock to use 2 VDSC instances (Ville)
> > 
> > v2:
> > * Add if-else for eDP/DP (Gaurav)
> > 
> > Cc: Jani Nikula 
> > Cc: Ville Syrjala 
> > Cc: Anusha Srivatsa 
> > Cc: Gaurav K Singh 
> > Signed-off-by: Manasi Navare 
> > Reviewed-by: Anusha Srivatsa 
> > Acked-by: Jani Nikula 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c |  20 ++-
> >  drivers/gpu/drm/i915/intel_display.h |   3 +-
> >  drivers/gpu/drm/i915/intel_dp.c  | 187 ---
> >  drivers/gpu/drm/i915/intel_dp_mst.c  |   2 +-
> >  4 files changed, 184 insertions(+), 28 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 132e978227fb..390c4b3970db 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -6483,7 +6483,7 @@ static int ironlake_fdi_compute_config(struct 
> > intel_crtc *intel_crtc,
> >  
> > pipe_config->fdi_lanes = lane;
> >  
> > -   intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
> > +   intel_link_compute_m_n(pipe_config->pipe_bpp, 0, lane, fdi_dotclock,
> >link_bw, &pipe_config->fdi_m_n, false);
> >  
> > ret = ironlake_check_fdi_lanes(dev, intel_crtc->pipe, pipe_config);
> > @@ -6723,17 +6723,25 @@ static void compute_m_n(unsigned int m, unsigned 
> > int n,
> >  }
> >  
> >  void
> > -intel_link_compute_m_n(int bits_per_pixel, int nlanes,
> > +intel_link_compute_m_n(int bits_per_pixel, uint16_t compressed_bpp,
> > +  int nlanes,
> >int pixel_clock, int link_clock,
> >struct intel_link_m_n *m_n,
> >bool constant_n)
> >  {
> > m_n->tu = 64;
> >  
> > -   compute_m_n(bits_per_pixel * pixel_clock,
> > -   link_clock * nlanes * 8,
> > -   &m_n->gmch_m, &m_n->gmch_n,
> > -   constant_n);
> > +   /* For DSC, Data M/N calculation uses compressed BPP */
> > +   if (compressed_bpp)
> 
> The extra parameter seems to be pointless. Just pass
> compressed_bpp instead of pipe_bpp and it'll do what
> you want.

So if compressed_bpp then just pass compressed_bpp instead of bits_per_pixel,
is that what you are suggesting? Because we dont want to call compressed_bpp 
otherwise.

> 
> > +   compute_m_n(compressed_bpp * pixel_clock,
> > +   link_clock * nlanes * 8,
> > +   &m_n->gmch_m, &m_n->gmch_n,
> > +   constant_n);
> > +   else
> > +   compute_m_n(bits_per_pixel * pixel_clock,
> > +   link_clock * nlanes * 8,
> > +   &m_n->gmch_m, &m_n->gmch_n,
> > +   constant_n);
> >  
> > compute_m_n(pixel_clock, link_clock,
> > &m_n->link_m, &m_n->link_n,
> > diff --git a/drivers/gpu/drm/i915/intel_display.h 
> > b/drivers/gpu/drm/i915/intel_display.h
> > index 5d50decbcbb5..b0b23e1e9392 100644
> > --- a/drivers/gpu/drm/i915/intel_display.h
> > +++ b/drivers/gpu/drm/i915/intel_display.h
> > @@ -407,7 +407,8 @@ struct intel_link_m_n {
> >  (__i)++) \
> > for_each_if(plane)
> >  
> > -void intel_link_compute_m_n(int bpp, int nlanes,
> > +void intel_link_compute_m_n(int bpp, uint16_t compressed_bpp,
> > +   int nlanes,

[Bug 106175] amdgpu.dc=1 shows performance issues with Xorg compositors when moving windows

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=106175

--- Comment #51 from Alex Deucher  ---
DC uses the atomic KMS interface, the old code uses the legacy KMS interface.

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


Re: [PATCH v2 8/9] phy: Add Cadence D-PHY support

2018-11-19 Thread Sakari Ailus
Hi Maxime,

On Tue, Nov 06, 2018 at 03:54:20PM +0100, Maxime Ripard wrote:
> Cadence has designed a D-PHY that can be used by the, currently in tree,
> DSI bridge (DRM), CSI Transceiver and CSI Receiver (v4l2) drivers.
> 
> Only the DSI driver has an ad-hoc driver for that phy at the moment, while
> the v4l2 drivers are completely missing any phy support. In order to make
> that phy support available to all these drivers, without having to
> duplicate that code three times, let's create a generic phy framework
> driver.
> 
> Signed-off-by: Maxime Ripard 
> ---
>  Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt |  21 +-
>  Documentation/devicetree/bindings/phy/cdns,dphy.txt   |  20 +-

Coould you split out the DT binding changes into a separate patch?

>  drivers/phy/cadence/Kconfig   |  13 +-
>  drivers/phy/cadence/Makefile  |   1 +-
>  drivers/phy/cadence/cdns-dphy.c   | 459 +++-
>  5 files changed, 492 insertions(+), 22 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/phy/cdns,dphy.txt
>  create mode 100644 drivers/phy/cadence/cdns-dphy.c
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt 
> b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
> index f5725bb6c61c..525a4bfd8634 100644
> --- a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
> @@ -31,28 +31,7 @@ Required subnodes:
>  - one subnode per DSI device connected on the DSI bus. Each DSI device should
>contain a reg property encoding its virtual channel.
>  
> -Cadence DPHY
> -
> -
> -Cadence DPHY block.
> -
> -Required properties:
> -- compatible: should be set to "cdns,dphy".
> -- reg: physical base address and length of the DPHY registers.
> -- clocks: DPHY reference clocks.
> -- clock-names: must contain "psm" and "pll_ref".
> -- #phy-cells: must be set to 0.
> -
> -
>  Example:
> - dphy0: dphy@fd0e{
> - compatible = "cdns,dphy";
> - reg = <0x0 0xfd0e 0x0 0x1000>;
> - clocks = <&psm_clk>, <&pll_ref_clk>;
> - clock-names = "psm", "pll_ref";
> - #phy-cells = <0>;
> - };
> -
>   dsi0: dsi@fd0c {
>   compatible = "cdns,dsi";
>   reg = <0x0 0xfd0c 0x0 0x1000>;
> diff --git a/Documentation/devicetree/bindings/phy/cdns,dphy.txt 
> b/Documentation/devicetree/bindings/phy/cdns,dphy.txt
> new file mode 100644
> index ..1095bc4e72d9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/cdns,dphy.txt
> @@ -0,0 +1,20 @@
> +Cadence DPHY
> +
> +
> +Cadence DPHY block.
> +
> +Required properties:
> +- compatible: should be set to "cdns,dphy".
> +- reg: physical base address and length of the DPHY registers.
> +- clocks: DPHY reference clocks.
> +- clock-names: must contain "psm" and "pll_ref".
> +- #phy-cells: must be set to 0.
> +
> +Example:
> + dphy0: dphy@fd0e{
> + compatible = "cdns,dphy";
> + reg = <0x0 0xfd0e 0x0 0x1000>;
> + clocks = <&psm_clk>, <&pll_ref_clk>;
> + clock-names = "psm", "pll_ref";
> + #phy-cells = <0>;
> + };
> diff --git a/drivers/phy/cadence/Kconfig b/drivers/phy/cadence/Kconfig
> index 57fff7de4031..240effa81046 100644
> --- a/drivers/phy/cadence/Kconfig
> +++ b/drivers/phy/cadence/Kconfig
> @@ -1,6 +1,7 @@
>  #
> -# Phy driver for Cadence MHDP DisplayPort controller
> +# Phy drivers for Cadence IPs
>  #
> +
>  config PHY_CADENCE_DP
>   tristate "Cadence MHDP DisplayPort PHY driver"
>   depends on OF
> @@ -8,3 +9,13 @@ config PHY_CADENCE_DP
>   select GENERIC_PHY
>   help
> Support for Cadence MHDP DisplayPort PHY.
> +
> +config PHY_CADENCE_DPHY
> + tristate "Cadence D-PHY Support"
> + depends on HAS_IOMEM && OF
> + select GENERIC_PHY
> + select GENERIC_PHY_MIPI_DPHY
> + help
> +   Choose this option if you have a Cadence D-PHY in your
> +   system. If M is selected, the module will be called
> +   cdns-csi.
> diff --git a/drivers/phy/cadence/Makefile b/drivers/phy/cadence/Makefile
> index e5b0a11cf28a..64cb9ea66ceb 100644
> --- a/drivers/phy/cadence/Makefile
> +++ b/drivers/phy/cadence/Makefile
> @@ -1 +1,2 @@
>  obj-$(CONFIG_PHY_CADENCE_DP) += phy-cadence-dp.o
> +obj-$(CONFIG_PHY_CADENCE_DPHY)   += cdns-dphy.o
> diff --git a/drivers/phy/cadence/cdns-dphy.c b/drivers/phy/cadence/cdns-dphy.c
> new file mode 100644
> index ..a398b401e5d3
> --- /dev/null
> +++ b/drivers/phy/cadence/cdns-dphy.c
> @@ -0,0 +1,459 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright: 2017-2018 Cadence Design Systems, Inc.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +
> +#define REG_W

[Bug 106175] amdgpu.dc=1 shows performance issues with Xorg compositors when moving windows

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=106175

--- Comment #50 from Brandon Wright  ---
> I have this issue too, disabling page flipping fixes it for me on my vega10. 
> It started with 4.16rc1 IIRC

Negative. I checked back as far as the DC/DAL was integrated (4.15) and it's
been there from the start. 

It's in the kernel somewhere, in the DC DRM layer above the device specific
stuff. I looked in and couldn't see anything that's grossly problematic. I
suspect Michel's suggestion for async cursor updates might be the fix, but I
can't help wondering why the legacy DRM code is unaffected.

-- 
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 v9 01/24] drm/dsc: Modify DRM helper to return complete DSC color depth capabilities

2018-11-19 Thread Ville Syrjälä
On Mon, Nov 19, 2018 at 12:10:47PM -0800, Manasi Navare wrote:
> On Mon, Nov 19, 2018 at 09:43:38PM +0200, Ville Syrjälä wrote:
> > On Tue, Nov 13, 2018 at 05:52:09PM -0800, Manasi Navare wrote:
> > > DSC DPCD color depth register advertises its color depth capabilities
> > > by setting each of the bits that corresponding to a specific color
> > > depth. This patch defines those specific color depths and adds
> > > a helper to return an array of color depth capabilities.
> > > 
> > > Signed-off-by: Manasi Navare 
> > > Cc: Ville Syrjala 
> > > ---
> > >  drivers/gpu/drm/drm_dp_helper.c | 29 +++--
> > >  include/drm/drm_dp_helper.h |  9 +
> > >  2 files changed, 24 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_dp_helper.c 
> > > b/drivers/gpu/drm/drm_dp_helper.c
> > > index 6d483487f2b4..286567063960 100644
> > > --- a/drivers/gpu/drm/drm_dp_helper.c
> > > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > > @@ -1428,17 +1428,26 @@ u8 drm_dp_dsc_sink_line_buf_depth(const u8 
> > > dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
> > >  }
> > >  EXPORT_SYMBOL(drm_dp_dsc_sink_line_buf_depth);
> > >  
> > > -u8 drm_dp_dsc_sink_max_color_depth(const u8 
> > > dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
> > > +void drm_dp_dsc_sink_color_depth_cap(const u8 
> > > dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
> > > +  u8 *dsc_sink_color_depth_cap)
> > >  {
> > > + int i, cnt = 0;
> > >   u8 color_depth = dsc_dpcd[DP_DSC_DEC_COLOR_DEPTH_CAP - DP_DSC_SUPPORT];
> > >  
> > > - if (color_depth & DP_DSC_12_BPC)
> > > - return 12;
> > > - if (color_depth & DP_DSC_10_BPC)
> > > - return 10;
> > > - if (color_depth & DP_DSC_8_BPC)
> > > - return 8;
> > > -
> > > - return 0;
> > > + for (i = 1; i <= 3; i++) {
> > > + if (!(color_depth & BIT(i)))
> > > + continue;
> > > + switch (i) {
> > > + case 1:
> > > + dsc_sink_color_depth_cap[cnt++] = DP_DSC_8_BPC;
> > > + break;
> > > + case 2:
> > > + dsc_sink_color_depth_cap[cnt++] = DP_DSC_10_BPC;
> > > + break;
> > > + case 3:
> > > + dsc_sink_color_depth_cap[cnt++] = DP_DSC_12_BPC;
> > > + break;
> > > + }
> > > + }
> > >  }
> > > -EXPORT_SYMBOL(drm_dp_dsc_sink_max_color_depth);
> > > +EXPORT_SYMBOL(drm_dp_dsc_sink_color_depth_cap);
> > > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> > > index 3314e91f6eb3..ea3233b0a790 100644
> > > --- a/include/drm/drm_dp_helper.h
> > > +++ b/include/drm/drm_dp_helper.h
> > > @@ -242,9 +242,9 @@
> > >  # define DP_DSC_YCbCr420_Native (1 << 4)
> > >  
> > >  #define DP_DSC_DEC_COLOR_DEPTH_CAP  0x06A
> > > -# define DP_DSC_8_BPC   (1 << 1)
> > > -# define DP_DSC_10_BPC  (1 << 2)
> > > -# define DP_DSC_12_BPC  (1 << 3)
> > > +# define DP_DSC_8_BPC   8
> > > +# define DP_DSC_10_BPC  10
> > > +# define DP_DSC_12_BPC  12
> > 
> > 
> > I'd suggest something simpler like:
> > 
> > int foo(u8 bpc[3])
> 
> Is passing a full array recommended method vs passing the pointer to the 
> array?

It's the same thing in C. The compiler will treat both as a pointer
(eg. sadly you can't use ARRAY_SIZE() on this because it will just use
the size of the pointer rather than the size of the array in the
calculation).

Despite the language shortcomings I like to use the array notation
as a means to document what is the expected size of the passed in
array.

> 
> > {
> > int num_bpc = 0;
> > 
> > if (color_depth & DP_DSC_12_BPC)
> > bpc[num_bpc++] = 12;
> > if (color_depth & DP_DSC_10_BPC)
> > bpc[num_bpc++] = 10;
> > if (color_depth & DP_DSC_8_BPC)
> > bpc[num_bpc++] = 8;
> > 
> > return num_bpc;
> > }
> 
> Yes I could modify like above except start from lowest bpc in bpc[0] going 
> all the way to highest.

Highest to lowest seems more sensible to me since we want to pick the
max. So when we walk the array we can bail as soon as we find the
highest suitable value.

> Also as of now its only 3 bpcs so its safe to just have an array for 3 bpcs 
> for now right?

Yes. Adding more would require changing the function anyway,
and so the callers can be updated at the same time.

> 
> Manasi
> 
> > 
> > >  
> > >  #define DP_DSC_PEAK_THROUGHPUT  0x06B
> > >  # define DP_DSC_THROUGHPUT_MODE_0_MASK  (0xf << 0)
> > > @@ -1123,7 +1123,8 @@ drm_dp_is_branch(const u8 
> > > dpcd[DP_RECEIVER_CAP_SIZE])
> > >  u8 drm_dp_dsc_sink_max_slice_count(const u8 
> > > dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
> > >  bool is_edp);
> > >  u8 drm_dp_dsc_sink_line_buf_depth(const u8 
> > > dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE]);
> > > -u8 drm_dp_dsc_sink_max_color_depth(const u8 
> > > dsc_dpc[DP_DSC_RECEIV

Re: [PATCH v9 20/24] drm/i915/dsc: Add Per connector debugfs node for DSC support/enable

2018-11-19 Thread Ville Syrjälä
On Thu, Nov 15, 2018 at 05:39:42PM -0800, Manasi Navare wrote:
> On Tue, Nov 13, 2018 at 05:52:28PM -0800, Manasi Navare wrote:
> > DSC can be supported per DP connector. This patch adds a per connector
> > debugfs node to expose DSC support capability by the kernel.
> > The same node can be used from userspace to force DSC enable.
> > 
> > force_dsc_en written through this debugfs node is used to force
> > DSC even for lower resolutions.
> > 
> > v4:
> > * Add missed connector_status check (Manasi)
> > * Create i915_dsc_support node only for Gen >=10 (manasi)
> > * Access intel_dp->dsc_dpcd only if its not NULL (Manasi)
> > v3:
> > * Combine Force_dsc_en with this patch (Ville)
> > v2:
> > * Use kstrtobool_from_user to avoid explicit error checking (Lyude)
> > * Rebase on drm-tip (Manasi)
> > 
> > Cc: Rodrigo Vivi 
> > Cc: Ville Syrjala 
> > Cc: Anusha Srivatsa 
> > Cc: Lyude Paul 
> > Signed-off-by: Manasi Navare 
> > Reviewed-by: Lyude Paul 
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 77 +
> >  drivers/gpu/drm/i915/intel_dp.c |  3 +-
> >  drivers/gpu/drm/i915/intel_drv.h|  3 ++
> >  3 files changed, 82 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 670db5073d70..3c112bb225d6 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -5074,6 +5074,76 @@ static int i915_hdcp_sink_capability_show(struct 
> > seq_file *m, void *data)
> >  }
> >  DEFINE_SHOW_ATTRIBUTE(i915_hdcp_sink_capability);
> >  
> > +static int i915_dsc_support_show(struct seq_file *m, void *data)
> > +{
> > +   struct drm_connector *connector = m->private;
> > +   struct intel_encoder *encoder = intel_attached_encoder(connector);
> > +   struct intel_dp *intel_dp =
> > +   enc_to_intel_dp(&encoder->base);
> > +   struct intel_crtc *crtc;
> > +   struct intel_crtc_state *crtc_state;
> > +
> > +   if (connector->status != connector_status_connected)
> > +   return -ENODEV;
> > +
> > +   crtc = to_intel_crtc(encoder->base.crtc);
> > +   crtc_state = to_intel_crtc_state(crtc->base.state);

This stuff is broken. If you want to root around in the atomic states
you need to grab some locks first.

> > +   drm_modeset_lock(&crtc->base.mutex, NULL);
> > +   seq_printf(m, "Enabled: %s\n",
> > +  yesno(crtc_state->dsc_params.compression_enable));
> > +   if (intel_dp->dsc_dpcd)
> > +   seq_printf(m, "Supported: %s\n",
> > +  yesno(drm_dp_sink_supports_dsc(intel_dp->dsc_dpcd)));
> 
> I think this supported should also use the intel_dp_supports_dsc() helper so
> it checks for sink support and platform support as well.
> 
> Ville, what do you think?

Possibly. We may want to report per-crtc whether DSC is possible or not
(due to transcoder A not being capable of DSC). And to do that we
actually need to change intel_dp_source_supports_dsc() & co. to not take
the crtc state, and rather we just need to pass in the transcoder. That
would mean we have to determine the correct transcoder here. Perhaps
a better alternative would be to pass in pipe/crtc + port and let the
function derive the transcoder itself. That would keep the
pipe+port->transcoder logic in one place. Actually we have that logic
in two places at least already (intel_ddi_compute_config() and
intel_ddi_connector_get_hw_state()). We could extract that bit into
a small helper that gets called by all three places.

> 
> Manasi
> 
> > +   drm_modeset_unlock(&crtc->base.mutex);
> > +
> > +   return 0;
> > +}
> > +
> > +static ssize_t i915_dsc_support_write(struct file *file,
> > + const char __user *ubuf,
> > + size_t len, loff_t *offp)
> > +{
> > +   bool dsc_enable = false;
> > +   int ret;
> > +   struct drm_connector *connector =
> > +   ((struct seq_file *)file->private_data)->private;
> > +   struct intel_encoder *encoder = intel_attached_encoder(connector);
> > +   struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> > +
> > +   if (len == 0)
> > +   return 0;
> > +
> > +   DRM_DEBUG_DRIVER("Copied %d bytes from user to force DSC\n",
> > +(unsigned int)len);
> > +
> > +   ret = kstrtobool_from_user(ubuf, len, &dsc_enable);
> > +   if (ret < 0)
> > +   return ret;
> > +
> > +   DRM_DEBUG_DRIVER("Got %s for DSC Enable\n",
> > +(dsc_enable) ? "true" : "false");
> > +   intel_dp->force_dsc_en = dsc_enable;
> > +
> > +   *offp += len;
> > +   return len;
> > +}
> > +
> > +static int i915_dsc_support_open(struct inode *inode,
> > +struct file *file)
> > +{
> > +   return single_open(file, i915_dsc_support_show,
> > +  inode->i_private);
> > +}
> > +
> > +static const struct file_operations i915_dsc_support_fops = {
> > +   .owner = THIS_MODULE,
> > +   .open = i915_dsc_support

Re: [PATCH v9 22/24] drm/i915/fec: Set FEC_READY in FEC_CONFIGURATION

2018-11-19 Thread Ville Syrjälä
On Tue, Nov 13, 2018 at 05:52:30PM -0800, Manasi Navare wrote:
> From: Anusha Srivatsa 
> 
> If the panel supports FEC, the driver has to
> set the FEC_READY bit in the dpcd register:
> FEC_CONFIGURATION.
> 
> This has to happen before link training.
> 
> v2: s/intel_dp_set_fec_ready/intel_dp_sink_set_fec_ready
>- change commit message. (Gaurav)
> 
> v3: rebased. (r-b Manasi)
> 
> v4: Use fec crtc state, before setting FEC_READY
> bit. (Anusha)
> 
> v5: Move to intel_ddi.c
> - Make the function static (Anusha)
> 
> v6: Dont pass state as a separate argument (Ville)
> 
> Cc: dri-devel@lists.freedesktop.org
> Cc: Gaurav K Singh 
> Cc: Jani Nikula 
> Cc: Ville Syrjala 
> Cc: Manasi Navare 
> Signed-off-by: Anusha Srivatsa 
> Reviewed-by: Manasi Navare 
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 0638fd2febfb..2d15520f13c5 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3102,6 +3102,16 @@ static void icl_program_mg_dp_mode(struct 
> intel_digital_port *intel_dig_port)
>   I915_WRITE(MG_DP_MODE(port, 1), ln1);
>  }
>  
> +static void intel_dp_sink_set_fec_ready(struct intel_dp *intel_dp,
> + const struct intel_crtc_state 
> *crtc_state)
> +{
> + if (!crtc_state->fec_enable)
> + return;
> +
> + if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_FEC_CONFIGURATION, 
> DP_FEC_READY) <= 0)
> + DRM_DEBUG_KMS("Failed to get FEC enabled in sink\n");

The debug message is still bonkers.

> +}
> +
>  static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
>   const struct intel_crtc_state *crtc_state,
>   const struct drm_connector_state 
> *conn_state)
> @@ -3142,6 +3152,7 @@ static void intel_ddi_pre_enable_dp(struct 
> intel_encoder *encoder,
>   intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
>   intel_dp_sink_set_decompression_state(intel_dp, crtc_state,
> true);
> + intel_dp_sink_set_fec_ready(intel_dp, crtc_state);
>   intel_dp_start_link_train(intel_dp);
>   if (port != PORT_A || INTEL_GEN(dev_priv) >= 9)
>   intel_dp_stop_link_train(intel_dp);
> -- 
> 2.19.1

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


Re: [PATCH v9 14/24] drm/i915/dp: Configure i915 Picture parameter Set registers during DSC enabling

2018-11-19 Thread Ville Syrjälä
On Tue, Nov 13, 2018 at 05:52:22PM -0800, Manasi Navare wrote:
> After encoder->pre_enable() hook, after link training sequence is
> completed, PPS registers for DSC encoder are configured using the
> DSC state parameters in intel_crtc_state as part of DSC enabling
> routine in the source. DSC enabling routine is called after
> encoder->pre_enable() before enbaling the pipe and after
> compression is enabled on the sink.
> 
> v6:
> intel_dsc_enable to be part of pre_enable hook (Ville)
> v5:
> * make crtc_state const (Ville)
> v4:
> * Use cpu_transcoder instead of encoder->type for using EDP transcoder
> DSC registers(Ville)
> * Keep all PSS regs together (Anusha)
> 
> v3:
> * Configure Pic_width/2 for each VDSC engine when two VDSC engines per pipe
> are used (Manasi)
> * Add DSC slice_row_per_frame in PPS16 (Manasi)
> 
> v2:
> * Enable PG2 power well for VDSC on eDP
> 
> Cc: Jani Nikula 
> Cc: Ville Syrjala 
> Cc: Anusha Srivatsa 
> Signed-off-by: Manasi Navare 
> Reviewed-by: Anusha Srivatsa 
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |   2 +
>  drivers/gpu/drm/i915/intel_ddi.c |   6 +
>  drivers/gpu/drm/i915/intel_display.c |   1 +
>  drivers/gpu/drm/i915/intel_vdsc.c| 419 +++
>  4 files changed, 428 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8fbf45cd3eb8..dbabe54b0ae2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3490,6 +3490,8 @@ extern void intel_rps_mark_interactive(struct 
> drm_i915_private *i915,
>  bool interactive);
>  extern bool intel_set_memory_cxsr(struct drm_i915_private *dev_priv,
> bool enable);
> +extern void intel_dsc_enable(struct intel_encoder *encoder,
> +  const struct intel_crtc_state *crtc_state);
>  
>  int i915_reg_read_ioctl(struct drm_device *dev, void *data,
>   struct drm_file *file);
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> b/drivers/gpu/drm/i915/intel_ddi.c
> index c7d417e6262f..f6279e54f15b 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3144,6 +3144,12 @@ static void intel_ddi_pre_enable_dp(struct 
> intel_encoder *encoder,
>  
>   if (!is_mst)
>   intel_ddi_enable_pipe_clock(crtc_state);
> +
> + /*
> +  * Enable and Configure Display Stream Compression in the source
> +  * if enabled in intel_crtc_state.
> +  */

Does the comment provide some useful extra information that isn't
obvious?

> + intel_dsc_enable(encoder, crtc_state);
>  }
>  
>  static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 390c4b3970db..210da9e9d31e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5458,6 +5458,7 @@ static void intel_encoders_pre_enable(struct drm_crtc 
> *crtc,
>  
>   if (encoder->pre_enable)
>   encoder->pre_enable(encoder, crtc_state, conn_state);
> +

leftovers

>   }
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_vdsc.c 
> b/drivers/gpu/drm/i915/intel_vdsc.c
> index b644f69f1c93..94f346b97c10 100644
> --- a/drivers/gpu/drm/i915/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/intel_vdsc.c
> @@ -577,3 +577,422 @@ int intel_dp_compute_dsc_params(struct intel_dp 
> *intel_dp,
>  
>   return intel_compute_rc_parameters(vdsc_cfg);
>  }
> +
> +static void intel_configure_pps_for_dsc_encoder(struct intel_encoder 
> *encoder,
> + const struct intel_crtc_state 
> *crtc_state)
> +{
> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + const struct drm_dsc_config *vdsc_cfg = &crtc_state->dp_dsc_cfg;
> + enum pipe pipe = crtc->pipe;
> + enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> + u32 pps_val = 0;
> + u32 rc_buf_thresh_dword[4];
> + u32 rc_range_params_dword[8];
> + u8 num_vdsc_instances = (crtc_state->dsc_params.dsc_split) ? 2 : 1;
> + int i = 0;
> +
> + /* Populate PICTURE_PARAMETER_SET_0 registers */
> + pps_val = DSC_VER_MAJ | vdsc_cfg->dsc_version_minor <<
> + DSC_VER_MIN_SHIFT |
> + vdsc_cfg->bits_per_component << DSC_BPC_SHIFT |
> + vdsc_cfg->line_buf_depth << DSC_LINE_BUF_DEPTH_SHIFT;
> + if (vdsc_cfg->block_pred_enable)
> + pps_val |= DSC_BLOCK_PREDICTION;
> + else
> + pps_val &= ~DSC_BLOCK_PREDICTION;

All these &=~ things seem redundant.

> + if (vdsc_cfg->convert_rgb)
> + pps_val |= DSC_COLOR_SPACE_CONVERSION;
> + else
> + pps_val &= ~DSC_COLOR_SPACE_CONVERSION;
> + if (vdsc_cfg->enable422)
> + pps_val |= DSC_422_ENABLE

Re: [PATCH] drm/rockchip: support hwc layer

2018-11-19 Thread kbuild test robot
Hi ayaka,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on rockchip/for-next]
[also build test ERROR on v4.20-rc3 next-20181119]
[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/Randy-Li/drm-rockchip-support-hwc-layer/20181119-173536
base:   
https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git 
for-next
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

   drivers/gpu//drm/rockchip/rockchip_drm_vop.c: In function 'is_yuv_10bit':
>> drivers/gpu//drm/rockchip/rockchip_drm_vop.c:229:7: error: 
>> 'DRM_FORMAT_NV12_10LE40' undeclared (first use in this function); did you 
>> mean 'DRM_FORMAT_NV12'?
 case DRM_FORMAT_NV12_10LE40:
  ^~
  DRM_FORMAT_NV12
   drivers/gpu//drm/rockchip/rockchip_drm_vop.c:229:7: note: each undeclared 
identifier is reported only once for each function it appears in
   At top level:
   drivers/gpu//drm/rockchip/rockchip_drm_vop.c:226:13: warning: 'is_yuv_10bit' 
defined but not used [-Wunused-function]
static bool is_yuv_10bit (uint32_t format)
^~~~

vim +229 drivers/gpu//drm/rockchip/rockchip_drm_vop.c

   225  
   226  static bool is_yuv_10bit (uint32_t format)
   227  {
   228  switch (format) {
 > 229  case DRM_FORMAT_NV12_10LE40:
   230  return true;
   231  default:
   232  return false;
   233  };
   234  }
   235  

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


.config.gz
Description: application/gzip
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2 24/24] drm/msm: dpu: Remove crtc_lock

2018-11-19 Thread Jeykumar Sankaran

On 2018-11-16 10:42, Sean Paul wrote:

From: Sean Paul 

Each time it's called we're holding the crtc modeset lock, so it's
redundant.

Changes in v2:
- None

Signed-off-by: Sean Paul 


Reviewed-by: Jeykumar Sankaran 


---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 11 ---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h |  3 ---
 2 files changed, 14 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index f15cba2584a0..70b5104d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -69,7 +69,6 @@ static void dpu_crtc_destroy(struct drm_crtc *crtc)
return;

drm_crtc_cleanup(crtc);
-   mutex_destroy(&dpu_crtc->crtc_lock);
kfree(dpu_crtc);
 }

@@ -806,8 +805,6 @@ static void dpu_crtc_disable(struct drm_crtc *crtc,
  old_crtc_state->encoder_mask)
dpu_encoder_assign_crtc(encoder, NULL);

-   mutex_lock(&dpu_crtc->crtc_lock);
-
/* wait for frame_event_done completion */
if (_dpu_crtc_wait_for_frame_done(crtc))
DPU_ERROR("crtc%d wait for frame done
failed;frame_pending%d\n",
@@ -836,8 +833,6 @@ static void dpu_crtc_disable(struct drm_crtc *crtc,
cstate->bw_control = false;
cstate->bw_split_vote = false;

-   mutex_unlock(&dpu_crtc->crtc_lock);
-
if (crtc->state->event && !crtc->state->active) {
spin_lock_irqsave(&crtc->dev->event_lock, flags);
drm_crtc_send_vblank_event(crtc, crtc->state->event);
@@ -870,12 +865,9 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,
dpu_encoder_register_frame_event_callback(encoder,
dpu_crtc_frame_event_cb, (void *)crtc);

-   mutex_lock(&dpu_crtc->crtc_lock);
trace_dpu_crtc_enable(DRMID(crtc), true, dpu_crtc);
dpu_crtc->enabled = true;

-   mutex_unlock(&dpu_crtc->crtc_lock);
-
drm_for_each_encoder_mask(encoder, crtc->dev,
crtc->state->encoder_mask)
dpu_encoder_assign_crtc(encoder, crtc);

@@ -1177,7 +1169,6 @@ static int _dpu_debugfs_status_show(struct 
seq_file

*s, void *data)
drm_modeset_lock_all(crtc->dev);
cstate = to_dpu_crtc_state(crtc->state);

-   mutex_lock(&dpu_crtc->crtc_lock);
mode = &crtc->state->adjusted_mode;
out_width = _dpu_crtc_get_mixer_width(cstate, mode);

@@ -1264,7 +1255,6 @@ static int _dpu_debugfs_status_show(struct 
seq_file

*s, void *data)
dpu_crtc->vblank_cb_time = ktime_set(0, 0);
}

-   mutex_unlock(&dpu_crtc->crtc_lock);
drm_modeset_unlock_all(crtc->dev);

return 0;
@@ -1414,7 +1404,6 @@ struct drm_crtc *dpu_crtc_init(struct drm_device
*dev, struct drm_plane *plane,
crtc = &dpu_crtc->base;
crtc->dev = dev;

-   mutex_init(&dpu_crtc->crtc_lock);
spin_lock_init(&dpu_crtc->spin_lock);
atomic_set(&dpu_crtc->frame_pending, 0);

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index 2b358546af49..34f0c4d4d774 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -140,7 +140,6 @@ struct dpu_crtc_frame_event {
  * @dirty_list: list of color processing features are dirty
  * @ad_dirty: list containing ad properties that are dirty
  * @ad_active: list containing ad properties that are active
- * @crtc_lock : crtc lock around create, destroy and access.
  * @frame_pending : Whether or not an update is pending
  * @frame_events  : static allocation of in-flight frame events
  * @frame_event_list : available frame event list
@@ -173,8 +172,6 @@ struct dpu_crtc {
struct list_head ad_dirty;
struct list_head ad_active;

-   struct mutex crtc_lock;
-
atomic_t frame_pending;
struct dpu_crtc_frame_event
frame_events[DPU_CRTC_FRAME_EVENT_SIZE];
struct list_head frame_event_list;


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


Re: [PATCH v2 23/24] drm/msm: dpu: Remove vblank_requested flag from dpu_crtc

2018-11-19 Thread Jeykumar Sankaran

On 2018-11-16 10:42, Sean Paul wrote:

From: Sean Paul 

It's just for debugfs output, we don't need it

Changes in v2:
- None

Cc: Jeykumar Sankaran 
Signed-off-by: Sean Paul 


Reviewed-by: Jeykumar Sankaran 


---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  |  6 --
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h  |  2 --
 drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 14 --
 3 files changed, 4 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 725e15178cdb..f15cba2584a0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1149,10 +1149,6 @@ int dpu_crtc_vblank(struct drm_crtc *crtc, bool 
en)

dpu_encoder_toggle_vblank_for_crtc(enc, crtc, en);
}

-   mutex_lock(&dpu_crtc->crtc_lock);
-   dpu_crtc->vblank_requested = en;
-   mutex_unlock(&dpu_crtc->crtc_lock);
-
return 0;
 }

@@ -1268,8 +1264,6 @@ static int _dpu_debugfs_status_show(struct 
seq_file

*s, void *data)
dpu_crtc->vblank_cb_time = ktime_set(0, 0);
}

-   seq_printf(s, "vblank_enable:%d\n", dpu_crtc->vblank_requested);
-
mutex_unlock(&dpu_crtc->crtc_lock);
drm_modeset_unlock_all(crtc->dev);

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index 54595cc29be5..2b358546af49 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -132,7 +132,6 @@ struct dpu_crtc_frame_event {
  * @vblank_cb_count : count of vblank callback since last reset
  * @play_count: frame count between crtc enable and disable
  * @vblank_cb_time  : ktime at vblank count reset
- * @vblank_requested : whether the user has requested vblank events
  * @enabled   : whether the DPU CRTC is currently enabled. updated 
in

the
  *  commit-thread, not state-swap time which is 
earlier,

so
  *  safe to make decisions on during VBLANK on/off 
work

@@ -166,7 +165,6 @@ struct dpu_crtc {
u32 vblank_cb_count;
u64 play_count;
ktime_t vblank_cb_time;
-   bool vblank_requested;
bool enabled;

struct list_head feature_list;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
index 328df37d7580..c78b521ceda1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
@@ -728,20 +728,17 @@ TRACE_EVENT(dpu_crtc_vblank_enable,
__field(uint32_t,   enc_id  )
__field(bool,   enable  )
__field(bool,   enabled )
-   __field(bool,   vblank_requested )
),
TP_fast_assign(
__entry->drm_id = drm_id;
__entry->enc_id = enc_id;
__entry->enable = enable;
__entry->enabled = crtc->enabled;
-   __entry->vblank_requested = crtc->vblank_requested;
),
-   TP_printk("id:%u encoder:%u enable:%s state{enabled:%s
vblank_req:%s}",
+   TP_printk("id:%u encoder:%u enable:%s state{enabled:%s}",
  __entry->drm_id, __entry->enc_id,
  __entry->enable ? "true" : "false",
- __entry->enabled ? "true" : "false",
- __entry->vblank_requested ? "true" : "false")
+ __entry->enabled ? "true" : "false")
 );

 DECLARE_EVENT_CLASS(dpu_crtc_enable_template,
@@ -751,18 +748,15 @@ DECLARE_EVENT_CLASS(dpu_crtc_enable_template,
__field(uint32_t,   drm_id  )
__field(bool,   enable  )
__field(bool,   enabled )
-   __field(bool,   vblank_requested )
),
TP_fast_assign(
__entry->drm_id = drm_id;
__entry->enable = enable;
__entry->enabled = crtc->enabled;
-   __entry->vblank_requested = crtc->vblank_requested;
),
-   TP_printk("id:%u enable:%s state{enabled:%s vblank_req:%s}",
+   TP_printk("id:%u enable:%s state{enabled:%s}",
  __entry->drm_id, __entry->enable ? "true" : "false",
- __entry->enabled ? "true" : "false",
- __entry->vblank_requested ? "true" : "false")
+ __entry->enabled ? "true" : "false")
 );
 DEFINE_EVENT(dpu_crtc_enable_template, dpu_crtc_enable,
TP_PROTO(uint32_t drm_id, bool enable, struct dpu_crtc *crtc),


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


Re: [PATCH v2 22/24] drm/msm: dpu: Separate crtc assignment from vblank enable

2018-11-19 Thread Jeykumar Sankaran

On 2018-11-16 10:42, Sean Paul wrote:

From: Sean Paul 

Instead of assigning/clearing the crtc on vblank enable/disable, we can
just assign and clear the crtc on modeset. That allows us to just 
toggle

the encoder's vblank interrupts on vblank_enable.

So why is this important? Previously the driver was using the legacy
pointers to assign/clear the crtc. Legacy pointers are cleared _after_
disabling the hardware, so the legacy pointer was valid during
vblank_disable, but that's not something we should rely on.

Instead of relying on the core ordering the legacy pointer assignments
just so, we'll assign the crtc in dpu_crtc enable/disable. This is the
only place that mapping can change, so we're covered there.

We're also taking advantage of drm_crtc_vblank_on/off. By using this, 
we

ensure that vblank_enable/disable can never be called while the crtc is
off (which means the assigned crtc will always be valid). As such, we
don't need to use modeset locks or the crtc_lock in the
vblank_enable/disable routine to be sure state is consistent.

...I think.

Changes in v2:
- Changed crtc check in toggle_vblank to != (Jeykumar)

Cc: Jeykumar Sankaran 
Signed-off-by: Sean Paul 


Reviewed-by: Jeykumar Sankaran 


---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 77 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 27 +---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 10 +++
 3 files changed, 59 insertions(+), 55 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 54bb777b2d0c..725e15178cdb 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -749,43 +749,6 @@ void dpu_crtc_commit_kickoff(struct drm_crtc 
*crtc)

DPU_ATRACE_END("crtc_commit");
 }

-/**
- * _dpu_crtc_vblank_enable_no_lock - update power resource and vblank
request
- * @dpu_crtc: Pointer to dpu crtc structure
- * @enable: Whether to enable/disable vblanks
- */
-static void _dpu_crtc_vblank_enable_no_lock(
-   struct dpu_crtc *dpu_crtc, bool enable)
-{
-   struct drm_crtc *crtc = &dpu_crtc->base;
-   struct drm_device *dev = crtc->dev;
-   struct drm_encoder *enc;
-
-   if (enable) {
-   list_for_each_entry(enc, &dev->mode_config.encoder_list,
head) {
-   if (enc->crtc != crtc)
-   continue;
-
-
trace_dpu_crtc_vblank_enable(DRMID(&dpu_crtc->base),
-DRMID(enc), enable,
-dpu_crtc);
-
-   dpu_encoder_assign_crtc(enc, crtc);
-   }
-   } else {
-   list_for_each_entry(enc, &dev->mode_config.encoder_list,
head) {
-   if (enc->crtc != crtc)
-   continue;
-
-
trace_dpu_crtc_vblank_enable(DRMID(&dpu_crtc->base),
-DRMID(enc), enable,
-dpu_crtc);
-
-   dpu_encoder_assign_crtc(enc, NULL);
-   }
-   }
-}
-
 /**
  * dpu_crtc_duplicate_state - state duplicate hook
  * @crtc: Pointer to drm crtc structure
@@ -839,6 +802,10 @@ static void dpu_crtc_disable(struct drm_crtc 
*crtc,

/* Disable/save vblank irq handling */
drm_crtc_vblank_off(crtc);

+   drm_for_each_encoder_mask(encoder, crtc->dev,
+ old_crtc_state->encoder_mask)
+   dpu_encoder_assign_crtc(encoder, NULL);
+
mutex_lock(&dpu_crtc->crtc_lock);

/* wait for frame_event_done completion */
@@ -848,9 +815,6 @@ static void dpu_crtc_disable(struct drm_crtc *crtc,
atomic_read(&dpu_crtc->frame_pending));

trace_dpu_crtc_disable(DRMID(crtc), false, dpu_crtc);
-   if (dpu_crtc->enabled && dpu_crtc->vblank_requested) {
-   _dpu_crtc_vblank_enable_no_lock(dpu_crtc, false);
-   }
dpu_crtc->enabled = false;

if (atomic_read(&dpu_crtc->frame_pending)) {
@@ -908,13 +872,13 @@ static void dpu_crtc_enable(struct drm_crtc 
*crtc,


mutex_lock(&dpu_crtc->crtc_lock);
trace_dpu_crtc_enable(DRMID(crtc), true, dpu_crtc);
-   if (!dpu_crtc->enabled && dpu_crtc->vblank_requested) {
-   _dpu_crtc_vblank_enable_no_lock(dpu_crtc, true);
-   }
dpu_crtc->enabled = true;

mutex_unlock(&dpu_crtc->crtc_lock);

+   drm_for_each_encoder_mask(encoder, crtc->dev,
crtc->state->encoder_mask)
+   dpu_encoder_assign_crtc(encoder, crtc);
+
/* Enable/restore vblank irq handling */
drm_crtc_vblank_on(crtc);
 }
@@ -1159,10 +1123,33 @@ static int dpu_crtc_atomic_check(struct 
drm_crtc

*crtc,
 int dpu_crtc_vblank(struct drm_crtc *crtc, bool en)
 {
struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
+   struct drm_encoder *enc;

-   mutex_

Re: [PATCH v2 21/24] drm/msm: dpu: Don't bother checking ->enabled in dpu_crtc_vblank

2018-11-19 Thread Jeykumar Sankaran

On 2018-11-16 10:42, Sean Paul wrote:

From: Sean Paul 

The drm_crtc_vblank_on/off calls in enable/disable guarantee that we
won't call this function when crtc is not enabled.

Changes in v2:
- None

Signed-off-by: Sean Paul 


Reviewed-by: Jeykumar Sankaran 


---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 9efb41c7973b..54bb777b2d0c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1162,9 +1162,7 @@ int dpu_crtc_vblank(struct drm_crtc *crtc, bool 
en)


mutex_lock(&dpu_crtc->crtc_lock);
trace_dpu_crtc_vblank(DRMID(&dpu_crtc->base), en, dpu_crtc);
-   if (dpu_crtc->enabled) {
-   _dpu_crtc_vblank_enable_no_lock(dpu_crtc, en);
-   }
+   _dpu_crtc_vblank_enable_no_lock(dpu_crtc, en);
dpu_crtc->vblank_requested = en;
mutex_unlock(&dpu_crtc->crtc_lock);


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


Re: [PATCH v2 20/24] drm/msm: dpu: Use atomic_disable for dpu_crtc_disable

2018-11-19 Thread Jeykumar Sankaran

On 2018-11-16 10:42, Sean Paul wrote:

From: Sean Paul 

Matches dpu_crtc_enable and we'll need the old state in a future patch

Changes in v2:
- None

Signed-off-by: Sean Paul 


Reviewed-by: Jeykumar Sankaran 


---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 4d7f9ff1e9f4..9efb41c7973b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -815,7 +815,8 @@ static struct drm_crtc_state
*dpu_crtc_duplicate_state(struct drm_crtc *crtc)
return &cstate->base;
 }

-static void dpu_crtc_disable(struct drm_crtc *crtc)
+static void dpu_crtc_disable(struct drm_crtc *crtc,
+struct drm_crtc_state *old_crtc_state)
 {
struct dpu_crtc *dpu_crtc;
struct dpu_crtc_state *cstate;
@@ -1407,7 +1408,7 @@ static const struct drm_crtc_funcs dpu_crtc_funcs 
=

{
 };

 static const struct drm_crtc_helper_funcs dpu_crtc_helper_funcs = {
-   .disable = dpu_crtc_disable,
+   .atomic_disable = dpu_crtc_disable,
.atomic_enable = dpu_crtc_enable,
.atomic_check = dpu_crtc_atomic_check,
.atomic_begin = dpu_crtc_atomic_begin,


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


Re: [PATCH v2 19/24] drm/msm: dpu: Remove vblank_callback from encoder

2018-11-19 Thread Jeykumar Sankaran

On 2018-11-16 10:42, Sean Paul wrote:

From: Sean Paul 

The indirection of registering a callback and opaque pointer isn't 
reall

useful when there's only one callsite. So instead of having the
vblank_cb registration, just give encoder a crtc and let it directly
call the vblank handler.

In a later patch, we'll make use of this further.

Changes in v2:
- None

Cc: Jeykumar Sankaran 
Signed-off-by: Sean Paul 


Reviewed-by: Jeykumar Sankaran 


---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c|  8 +++
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h|  6 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 25 +++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 10 -
 4 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 141ed1b0e90a..4d7f9ff1e9f4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -293,9 +293,8 @@ enum dpu_intf_mode dpu_crtc_get_intf_mode(struct
drm_crtc *crtc)
return INTF_MODE_NONE;
 }

-static void dpu_crtc_vblank_cb(void *data)
+void dpu_crtc_vblank_callback(struct drm_crtc *crtc)
 {
-   struct drm_crtc *crtc = (struct drm_crtc *)data;
struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);

/* keep statistics on vblank callback - with auto reset via
debugfs */
@@ -771,8 +770,7 @@ static void _dpu_crtc_vblank_enable_no_lock(
 DRMID(enc), enable,
 dpu_crtc);

-   dpu_encoder_register_vblank_callback(enc,
-   dpu_crtc_vblank_cb, (void *)crtc);
+   dpu_encoder_assign_crtc(enc, crtc);
}
} else {
list_for_each_entry(enc, &dev->mode_config.encoder_list,
head) {
@@ -783,7 +781,7 @@ static void _dpu_crtc_vblank_enable_no_lock(
 DRMID(enc), enable,
 dpu_crtc);

-   dpu_encoder_register_vblank_callback(enc, NULL,
NULL);
+   dpu_encoder_assign_crtc(enc, NULL);
}
}
 }
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index 93d21a61a040..54595cc29be5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -270,6 +270,12 @@ static inline int dpu_crtc_frame_pending(struct
drm_crtc *crtc)
  */
 int dpu_crtc_vblank(struct drm_crtc *crtc, bool en);

+/**
+ * dpu_crtc_vblank_callback - called on vblank irq, issues completion
events
+ * @crtc: Pointer to drm crtc object
+ */
+void dpu_crtc_vblank_callback(struct drm_crtc *crtc);
+
 /**
  * dpu_crtc_commit_kickoff - trigger kickoff of the commit for this 
crtc

  * @crtc: Pointer to drm crtc object
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index d89ac520f7e6..fd6514f681ae 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -142,9 +142,11 @@ enum dpu_enc_rc_states {
  * @intfs_swapped  Whether or not the phys_enc interfaces have been
swapped
  * for partial update right-only cases, such as
pingpong
  * split where virtual pingpong does not generate
IRQs
- * @crtc_vblank_cb:Callback into the upper layer / CRTC for
- * notification of the VBLANK
- * @crtc_vblank_cb_data:   Data from upper layer for VBLANK
notification
+ * @crtc:  Pointer to the currently assigned crtc. Normally
you
+ * would use crtc->state->encoder_mask to determine
the
+ * link between encoder/crtc. However in this case we
need
+ * to track crtc in the disable() hook which is
called
+ * _after_ encoder_mask is cleared.
  * @crtc_kickoff_cb:   Callback into CRTC that will flush & start
  * all CTL paths
  * @crtc_kickoff_cb_data:  Opaque user data given to crtc_kickoff_cb
@@ -186,8 +188,7 @@ struct dpu_encoder_virt {

bool intfs_swapped;

-   void (*crtc_vblank_cb)(void *);
-   void *crtc_vblank_cb_data;
+   struct drm_crtc *crtc;

struct dentry *debugfs_root;
struct mutex enc_lock;
@@ -1241,8 +1242,8 @@ static void dpu_encoder_vblank_callback(struct
drm_encoder *drm_enc,
dpu_enc = to_dpu_encoder_virt(drm_enc);

spin_lock_irqsave(&dpu_enc->enc_spinlock, lock_flags);
-   if (dpu_enc->crtc_vblank_cb)
-   dpu_enc->crtc_vblank_cb(dpu_enc->crtc_vblank_cb_data);
+   if (dpu_enc->crtc)
+   dpu_crtc_vblank_callback(dpu_enc->crtc);
spin_unlock_irqrestore(&dpu_enc->enc_spinlock, lock_flags);

atomic_inc(&phy_enc->vsync_

Re: [PATCH v9 08/24] drm/i915/dp: Compute DSC pipe config in atomic check

2018-11-19 Thread Ville Syrjälä
On Tue, Nov 13, 2018 at 05:52:16PM -0800, Manasi Navare wrote:
> DSC params like the enable, compressed bpp, slice count and
> dsc_split are added to the intel_crtc_state. These parameters
> are set based on the requested mode and available link parameters
> during the pipe configuration in atomic check phase.
> These values are then later used to populate the remaining DSC
> and RC parameters before enbaling DSC in atomic commit.
> 
> v13:
> * Compute DSC bpc only when DSC is req to be enabled (Ville)
> v12:
> * Override bpp with dsc dpcd color depth (Manasi)
> v11:
> * Const crtc_state, reject DSC on DP without FEC (Ville)
> * Dont set dsc_split to false (Ville)
> v10:
> * Add a helper for dp_dsc support (Ville)
> * Set pipe_config to max bpp, link params for DSC for now (Ville)
> * Compute bpp - use dp dsc support helper (Ville)
> v9:
> * Rebase on top of drm-tip that now uses fast_narrow config
> for edp (Manasi)
> v8:
> * Check for DSC bpc not 0 (manasi)
> 
> v7:
> * Fix indentation in compute_m_n (Manasi)
> 
> v6 (From Gaurav):
> * Remove function call of intel_dp_compute_dsc_params() and
> invoke intel_dp_compute_dsc_params() in the patch where
> it is defined to fix compilation warning (Gaurav)
> 
> v5:
> Add drm_dsc_cfg in intel_crtc_state (Manasi)
> 
> v4:
> * Rebase on refactoring of intel_dp_compute_config on tip (Manasi)
> * Add a comment why we need to check PSR while enabling DSC (Gaurav)
> 
> v3:
> * Check PPR > max_cdclock to use 2 VDSC instances (Ville)
> 
> v2:
> * Add if-else for eDP/DP (Gaurav)
> 
> Cc: Jani Nikula 
> Cc: Ville Syrjala 
> Cc: Anusha Srivatsa 
> Cc: Gaurav K Singh 
> Signed-off-by: Manasi Navare 
> Reviewed-by: Anusha Srivatsa 
> Acked-by: Jani Nikula 
> ---
>  drivers/gpu/drm/i915/intel_display.c |  20 ++-
>  drivers/gpu/drm/i915/intel_display.h |   3 +-
>  drivers/gpu/drm/i915/intel_dp.c  | 187 ---
>  drivers/gpu/drm/i915/intel_dp_mst.c  |   2 +-
>  4 files changed, 184 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 132e978227fb..390c4b3970db 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6483,7 +6483,7 @@ static int ironlake_fdi_compute_config(struct 
> intel_crtc *intel_crtc,
>  
>   pipe_config->fdi_lanes = lane;
>  
> - intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
> + intel_link_compute_m_n(pipe_config->pipe_bpp, 0, lane, fdi_dotclock,
>  link_bw, &pipe_config->fdi_m_n, false);
>  
>   ret = ironlake_check_fdi_lanes(dev, intel_crtc->pipe, pipe_config);
> @@ -6723,17 +6723,25 @@ static void compute_m_n(unsigned int m, unsigned int 
> n,
>  }
>  
>  void
> -intel_link_compute_m_n(int bits_per_pixel, int nlanes,
> +intel_link_compute_m_n(int bits_per_pixel, uint16_t compressed_bpp,
> +int nlanes,
>  int pixel_clock, int link_clock,
>  struct intel_link_m_n *m_n,
>  bool constant_n)
>  {
>   m_n->tu = 64;
>  
> - compute_m_n(bits_per_pixel * pixel_clock,
> - link_clock * nlanes * 8,
> - &m_n->gmch_m, &m_n->gmch_n,
> - constant_n);
> + /* For DSC, Data M/N calculation uses compressed BPP */
> + if (compressed_bpp)

The extra parameter seems to be pointless. Just pass
compressed_bpp instead of pipe_bpp and it'll do what
you want.

> + compute_m_n(compressed_bpp * pixel_clock,
> + link_clock * nlanes * 8,
> + &m_n->gmch_m, &m_n->gmch_n,
> + constant_n);
> + else
> + compute_m_n(bits_per_pixel * pixel_clock,
> + link_clock * nlanes * 8,
> + &m_n->gmch_m, &m_n->gmch_n,
> + constant_n);
>  
>   compute_m_n(pixel_clock, link_clock,
>   &m_n->link_m, &m_n->link_n,
> diff --git a/drivers/gpu/drm/i915/intel_display.h 
> b/drivers/gpu/drm/i915/intel_display.h
> index 5d50decbcbb5..b0b23e1e9392 100644
> --- a/drivers/gpu/drm/i915/intel_display.h
> +++ b/drivers/gpu/drm/i915/intel_display.h
> @@ -407,7 +407,8 @@ struct intel_link_m_n {
>(__i)++) \
>   for_each_if(plane)
>  
> -void intel_link_compute_m_n(int bpp, int nlanes,
> +void intel_link_compute_m_n(int bpp, uint16_t compressed_bpp,
> + int nlanes,
>   int pixel_clock, int link_clock,
>   struct intel_link_m_n *m_n,
>   bool constant_n);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 2b090609bee2..931c3c7e55c1 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -47,6 +47,8 @@
>  
>  /* DP DSC small joiner has 2 FIFOs each of 640 x 6 bytes */
>  #define DP_DSC_MAX_

Re: [PATCH v2 18/24] drm/msm: dpu: Remove crtc_lock from setup_mixers

2018-11-19 Thread Jeykumar Sankaran

On 2018-11-16 10:42, Sean Paul wrote:

From: Sean Paul 

I think the intention here was to protect the enc->crtc access, but
that's insufficient to avoid enc->crtc changing. Fortunately we're
already holding the modeset lock when this is called (from
atomic_check), so remove the crtc_lock and add a modeset lock check.

While we're at it, use the encoder mask from crtc state instead of
legacy pointer.

Changes in v2:
- None

Signed-off-by: Sean Paul 


Reviewed-by: Jeykumar Sankaran 


---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 12 +++-
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index c49aaf412b6e..141ed1b0e90a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -474,19 +474,13 @@ static void _dpu_crtc_setup_mixer_for_encoder(

 static void _dpu_crtc_setup_mixers(struct drm_crtc *crtc)
 {
-   struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
struct drm_encoder *enc;

-   mutex_lock(&dpu_crtc->crtc_lock);
-   /* Check for mixers on all encoders attached to this crtc */
-   list_for_each_entry(enc, &crtc->dev->mode_config.encoder_list,
head) {
-   if (enc->crtc != crtc)
-   continue;
+   WARN_ON(!drm_modeset_is_locked(&crtc->mutex));

+   /* Check for mixers on all encoders attached to this crtc */
+   drm_for_each_encoder_mask(enc, crtc->dev,
crtc->state->encoder_mask)
_dpu_crtc_setup_mixer_for_encoder(crtc, enc);
-   }
-
-   mutex_unlock(&dpu_crtc->crtc_lock);
 }

 static void _dpu_crtc_setup_lm_bounds(struct drm_crtc *crtc,


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


Re: [PATCH v2 17/24] drm/msm: dpu: Move pm_runtime_(get|put) from vblank_enable

2018-11-19 Thread Jeykumar Sankaran

On 2018-11-16 10:42, Sean Paul wrote:

From: Sean Paul 

There are 4 times that _dpu_crtc_vblank_enable_no_lock() is called:

1- crtc enable
2- crtc disable
3- crtc vblank enable
4- crtc vblank disable

When we enable or disable the crtc, we call drm_crtc_vblank_on and
drm_crtc_vblank_off respectively. That will gate vblank enables and
disables to only being called when the crtc is active. That means that
we can just enable/disable pm runtime in crtc enable/disable. This will
be beneficial in trying to eliminate blocking calls from the vblank 
call

chain.

Changes in v2:
- None

Signed-off-by: Sean Paul 


Reviewed-by: Jeykumar Sankaran 


---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index cd0a0bea4335..c49aaf412b6e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -769,8 +769,6 @@ static void _dpu_crtc_vblank_enable_no_lock(
struct drm_encoder *enc;

if (enable) {
-   pm_runtime_get_sync(dev->dev);
-
list_for_each_entry(enc, &dev->mode_config.encoder_list,
head) {
if (enc->crtc != crtc)
continue;
@@ -793,8 +791,6 @@ static void _dpu_crtc_vblank_enable_no_lock(

dpu_encoder_register_vblank_callback(enc, NULL,
NULL);
}
-
-   pm_runtime_put_sync(dev->dev);
}
 }

@@ -891,6 +887,8 @@ static void dpu_crtc_disable(struct drm_crtc *crtc)
crtc->state->event = NULL;
spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
}
+
+   pm_runtime_put_sync(crtc->dev->dev);
 }

 static void dpu_crtc_enable(struct drm_crtc *crtc,
@@ -906,6 +904,8 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,
}
priv = crtc->dev->dev_private;

+   pm_runtime_get_sync(crtc->dev->dev);
+
DRM_DEBUG_KMS("crtc%d\n", crtc->base.id);
dpu_crtc = to_dpu_crtc(crtc);


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


Re: [PATCH v9 01/24] drm/dsc: Modify DRM helper to return complete DSC color depth capabilities

2018-11-19 Thread Manasi Navare
On Mon, Nov 19, 2018 at 09:43:38PM +0200, Ville Syrjälä wrote:
> On Tue, Nov 13, 2018 at 05:52:09PM -0800, Manasi Navare wrote:
> > DSC DPCD color depth register advertises its color depth capabilities
> > by setting each of the bits that corresponding to a specific color
> > depth. This patch defines those specific color depths and adds
> > a helper to return an array of color depth capabilities.
> > 
> > Signed-off-by: Manasi Navare 
> > Cc: Ville Syrjala 
> > ---
> >  drivers/gpu/drm/drm_dp_helper.c | 29 +++--
> >  include/drm/drm_dp_helper.h |  9 +
> >  2 files changed, 24 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_helper.c 
> > b/drivers/gpu/drm/drm_dp_helper.c
> > index 6d483487f2b4..286567063960 100644
> > --- a/drivers/gpu/drm/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > @@ -1428,17 +1428,26 @@ u8 drm_dp_dsc_sink_line_buf_depth(const u8 
> > dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
> >  }
> >  EXPORT_SYMBOL(drm_dp_dsc_sink_line_buf_depth);
> >  
> > -u8 drm_dp_dsc_sink_max_color_depth(const u8 
> > dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
> > +void drm_dp_dsc_sink_color_depth_cap(const u8 
> > dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
> > +u8 *dsc_sink_color_depth_cap)
> >  {
> > +   int i, cnt = 0;
> > u8 color_depth = dsc_dpcd[DP_DSC_DEC_COLOR_DEPTH_CAP - DP_DSC_SUPPORT];
> >  
> > -   if (color_depth & DP_DSC_12_BPC)
> > -   return 12;
> > -   if (color_depth & DP_DSC_10_BPC)
> > -   return 10;
> > -   if (color_depth & DP_DSC_8_BPC)
> > -   return 8;
> > -
> > -   return 0;
> > +   for (i = 1; i <= 3; i++) {
> > +   if (!(color_depth & BIT(i)))
> > +   continue;
> > +   switch (i) {
> > +   case 1:
> > +   dsc_sink_color_depth_cap[cnt++] = DP_DSC_8_BPC;
> > +   break;
> > +   case 2:
> > +   dsc_sink_color_depth_cap[cnt++] = DP_DSC_10_BPC;
> > +   break;
> > +   case 3:
> > +   dsc_sink_color_depth_cap[cnt++] = DP_DSC_12_BPC;
> > +   break;
> > +   }
> > +   }
> >  }
> > -EXPORT_SYMBOL(drm_dp_dsc_sink_max_color_depth);
> > +EXPORT_SYMBOL(drm_dp_dsc_sink_color_depth_cap);
> > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> > index 3314e91f6eb3..ea3233b0a790 100644
> > --- a/include/drm/drm_dp_helper.h
> > +++ b/include/drm/drm_dp_helper.h
> > @@ -242,9 +242,9 @@
> >  # define DP_DSC_YCbCr420_Native (1 << 4)
> >  
> >  #define DP_DSC_DEC_COLOR_DEPTH_CAP  0x06A
> > -# define DP_DSC_8_BPC   (1 << 1)
> > -# define DP_DSC_10_BPC  (1 << 2)
> > -# define DP_DSC_12_BPC  (1 << 3)
> > +# define DP_DSC_8_BPC   8
> > +# define DP_DSC_10_BPC  10
> > +# define DP_DSC_12_BPC  12
> 
> 
> I'd suggest something simpler like:
> 
> int foo(u8 bpc[3])

Is passing a full array recommended method vs passing the pointer to the array?

> {
>   int num_bpc = 0;
> 
>   if (color_depth & DP_DSC_12_BPC)
>   bpc[num_bpc++] = 12;
>   if (color_depth & DP_DSC_10_BPC)
>   bpc[num_bpc++] = 10;
>   if (color_depth & DP_DSC_8_BPC)
>   bpc[num_bpc++] = 8;
> 
>   return num_bpc;
> }

Yes I could modify like above except start from lowest bpc in bpc[0] going all 
the way to highest.
Also as of now its only 3 bpcs so its safe to just have an array for 3 bpcs for 
now right?

Manasi

> 
> >  
> >  #define DP_DSC_PEAK_THROUGHPUT  0x06B
> >  # define DP_DSC_THROUGHPUT_MODE_0_MASK  (0xf << 0)
> > @@ -1123,7 +1123,8 @@ drm_dp_is_branch(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
> >  u8 drm_dp_dsc_sink_max_slice_count(const u8 
> > dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
> >bool is_edp);
> >  u8 drm_dp_dsc_sink_line_buf_depth(const u8 
> > dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE]);
> > -u8 drm_dp_dsc_sink_max_color_depth(const u8 
> > dsc_dpc[DP_DSC_RECEIVER_CAP_SIZE]);
> > +void drm_dp_dsc_sink_color_depth_cap(const u8 
> > dsc_dpc[DP_DSC_RECEIVER_CAP_SIZE],
> > +u8 *dsc_sink_color_depth_cap);
> >  
> >  static inline bool
> >  drm_dp_sink_supports_dsc(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
> > -- 
> > 2.19.1
> 
> -- 
> Ville Syrjälä
> Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2 15/24] drm/msm: dpu: Stop using encoder->crtc pointer

2018-11-19 Thread Jeykumar Sankaran

On 2018-11-16 13:14, Sean Paul wrote:

On Fri, Nov 16, 2018 at 12:05:09PM -0800, Jeykumar Sankaran wrote:

On 2018-11-16 10:42, Sean Paul wrote:
> From: Sean Paul 
>
> It's for legacy drivers, for atomic drivers crtc->state->encoder_mask
> should be used to map encoder to crtc.
>
> Changes in v2:
> - None
>
> Signed-off-by: Sean Paul 
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 46



>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c  | 19 +++---
>  2 files changed, 29 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 156f4c77ca44..a008a87a8113 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -284,9 +284,9 @@ enum dpu_intf_mode dpu_crtc_get_intf_mode(struct
> drm_crtc *crtc)
>return INTF_MODE_NONE;
>}
>
> -  drm_for_each_encoder(encoder, crtc->dev)
> -  if (encoder->crtc == crtc)
> -  return dpu_encoder_get_intf_mode(encoder);
> +  /* TODO: Returns the first INTF_MODE, could there be multiple
> values? */
> +  drm_for_each_encoder_mask(encoder, crtc->dev,
> crtc->state->encoder_mask)
> +  return dpu_encoder_get_intf_mode(encoder);
>
>return INTF_MODE_NONE;
>  }
> @@ -551,13 +551,9 @@ static void dpu_crtc_atomic_begin(struct drm_crtc
> *crtc,
>spin_unlock_irqrestore(&dev->event_lock, flags);
>}
>
> -  list_for_each_entry(encoder, &dev->mode_config.encoder_list, head)
> {
> -  if (encoder->crtc != crtc)
> -  continue;
> -
> -  /* encoder will trigger pending mask now */
> +  /* encoder will trigger pending mask now */
> +  drm_for_each_encoder_mask(encoder, crtc->dev,
> crtc->state->encoder_mask)
>dpu_encoder_trigger_kickoff_pending(encoder);
> -  }
>
>/*
> * If no mixers have been allocated in dpu_crtc_atomic_check(),
> @@ -704,7 +700,6 @@ static int _dpu_crtc_wait_for_frame_done(struct
> drm_crtc *crtc)
>  void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
>  {
>struct drm_encoder *encoder;
> -  struct drm_device *dev = crtc->dev;
>struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
>struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
>struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state);
> @@ -720,16 +715,13 @@ void dpu_crtc_commit_kickoff(struct drm_crtc
> *crtc)
>
>DPU_ATRACE_BEGIN("crtc_commit");
>
> -  list_for_each_entry(encoder, &dev->mode_config.encoder_list, head)
> {
> +  /*
> +   * Encoder will flush/start now, unless it has a tx pending. If
> so, it
> +   * may delay and flush at an irq event (e.g. ppdone)
> +   */
> +  drm_for_each_encoder_mask(encoder, crtc->dev,
> +crtc->state->encoder_mask) {
>struct dpu_encoder_kickoff_params params = { 0 };
> -
> -  if (encoder->crtc != crtc)
> -  continue;
> -
> -  /*
> -   * Encoder will flush/start now, unless it has a tx
> pending.
> -   * If so, it may delay and flush at an irq event (e.g.
> ppdone)
> -   */
>dpu_encoder_prepare_for_kickoff(encoder, ¶ms);
>}
>
> @@ -754,12 +746,8 @@ void dpu_crtc_commit_kickoff(struct drm_crtc

*crtc)

>
>dpu_vbif_clear_errors(dpu_kms);
>
> -  list_for_each_entry(encoder, &dev->mode_config.encoder_list, head)
> {
> -  if (encoder->crtc != crtc)
> -  continue;
> -
> +  drm_for_each_encoder_mask(encoder, crtc->dev,
> crtc->state->encoder_mask)
>dpu_encoder_kickoff(encoder);
> -  }
We wont be holding the modeset locks here (and in crtc_atomic_begin) 
in

the

display thread. Is
it safe to iterate over encoder_mask?


Hmm, I'm not sure I follow. AFAICT, there are 2 callsites for
dpu_crtc_commit_kickoff():

1- dpu_kms_encoder_enable() which is called via the 
encoder->funcs->enable

hook
2- dpu_kms_commit() which is called in the
mode_config->funcs->atomic_commit_tail


atomic_commit_tail will be called from the WQ. Do we hold
the modeset locks in the WQ? I believe userspace thread will drop
the locks at the end of drm_mode_atomic_ioctl.

Thanks


Both of these callsites will hold the modeset locks.

Am I missing something?

Thanks for your review,

Sean


>
>  end:
>reinit_completion(&dpu_crtc->frame_done_comp);
> @@ -883,11 +871,8 @@ static void dpu_crtc_disable(struct drm_crtc

*crtc)

>
>dpu_core_perf_crtc_update(crtc, 0, true);
>
> -  drm_for_each_encoder(encoder, crtc->dev) {
> -  if (encoder->crtc != crtc)
> -  continue;
> +  drm_for_each_encoder_mask(encoder, crtc->dev,
> crtc->state->encoder_mask)
>dpu_encoder_register_frame_event_callback(encoder, NULL,
> NULL);
> -  }
>
>memset(cstate->mixers, 0, sizeof(cstate->mixers));
>cstate->num_mixers = 0;
> @@ -922,12 +907,9 @@ static void dpu_crtc_enable(struct drm_crtc

*crtc,

>DRM_DEBUG_KMS("crtc%d\n", crtc->base.id);
>dpu_crtc = to_dpu_c

RE: [PATCH 2/2] drm/amd/dm: Understand why attaching path/tile properties are needed

2018-11-19 Thread Zuo, Jerry
-Original Message-
From: Lyude Paul  
Sent: November 16, 2018 6:25 PM
To: amd-...@lists.freedesktop.org
Cc: Wentland, Harry ; Li, Sun peng (Leo) 
; Deucher, Alexander ; Koenig, 
Christian ; Zhou, David(ChunMing) 
; David Airlie ; Zuo, Jerry 
; Li, Roman ; Cheng, Tony 
; Daniel Vetter ; S, Shirish 
; dri-devel@lists.freedesktop.org; 
linux-ker...@vger.kernel.org
Subject: [PATCH 2/2] drm/amd/dm: Understand why attaching path/tile properties 
are needed

Path property is used for userspace to know what MST connector goes to what 
actual DRM DisplayPort connector, the tiling property is for tiling 
configurations. Not sure what else there is to figure out.

Signed-off-by: Lyude Paul 
Reviewed-by: Jerry (Fangzhi) Zuo 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 3 ---
 1 file changed, 3 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 0cca1809fdcd..1b0d209d8367 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
@@ -345,9 +345,6 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
drm_connector_attach_encoder(&aconnector->base,
 &aconnector->mst_encoder->base);
 
-   /*
-* TODO: understand why this one is needed
-*/
drm_object_attach_property(
&connector->base,
dev->mode_config.path_property,
--
2.19.1

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


Re: [PATCH v2 13/24] drm/msm: dpu: Don't drop locks in crtc_vblank_enable

2018-11-19 Thread Jeykumar Sankaran

On 2018-11-16 10:42, Sean Paul wrote:

From: Sean Paul 

Now that runtime resume is handled in encoder, we don't need to worry
about crtc_lock recursion when calling pm_runtime_(get|put). So drop 
the

lock drops in _dpu_crtc_vblank_enable_no_lock().

Changes in v2:
- None

Signed-off-by: Sean Paul 


Reviewed-by: Jeykumar Sankaran 


---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 9be24907f8c1..80de5289ada3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -777,10 +777,7 @@ static void _dpu_crtc_vblank_enable_no_lock(
struct drm_encoder *enc;

if (enable) {
-   /* drop lock since power crtc cb may try to re-acquire
lock */
-   mutex_unlock(&dpu_crtc->crtc_lock);
pm_runtime_get_sync(dev->dev);
-   mutex_lock(&dpu_crtc->crtc_lock);

list_for_each_entry(enc, &dev->mode_config.encoder_list,
head) {
if (enc->crtc != crtc)
@@ -805,10 +802,7 @@ static void _dpu_crtc_vblank_enable_no_lock(
dpu_encoder_register_vblank_callback(enc, NULL,
NULL);
}

-   /* drop lock since power crtc cb may try to re-acquire
lock */
-   mutex_unlock(&dpu_crtc->crtc_lock);
pm_runtime_put_sync(dev->dev);
-   mutex_lock(&dpu_crtc->crtc_lock);
}
 }


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


Re: [PATCH v9 01/24] drm/dsc: Modify DRM helper to return complete DSC color depth capabilities

2018-11-19 Thread Ville Syrjälä
On Tue, Nov 13, 2018 at 05:52:09PM -0800, Manasi Navare wrote:
> DSC DPCD color depth register advertises its color depth capabilities
> by setting each of the bits that corresponding to a specific color
> depth. This patch defines those specific color depths and adds
> a helper to return an array of color depth capabilities.
> 
> Signed-off-by: Manasi Navare 
> Cc: Ville Syrjala 
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 29 +++--
>  include/drm/drm_dp_helper.h |  9 +
>  2 files changed, 24 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 6d483487f2b4..286567063960 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -1428,17 +1428,26 @@ u8 drm_dp_dsc_sink_line_buf_depth(const u8 
> dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
>  }
>  EXPORT_SYMBOL(drm_dp_dsc_sink_line_buf_depth);
>  
> -u8 drm_dp_dsc_sink_max_color_depth(const u8 
> dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
> +void drm_dp_dsc_sink_color_depth_cap(const u8 
> dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
> +  u8 *dsc_sink_color_depth_cap)
>  {
> + int i, cnt = 0;
>   u8 color_depth = dsc_dpcd[DP_DSC_DEC_COLOR_DEPTH_CAP - DP_DSC_SUPPORT];
>  
> - if (color_depth & DP_DSC_12_BPC)
> - return 12;
> - if (color_depth & DP_DSC_10_BPC)
> - return 10;
> - if (color_depth & DP_DSC_8_BPC)
> - return 8;
> -
> - return 0;
> + for (i = 1; i <= 3; i++) {
> + if (!(color_depth & BIT(i)))
> + continue;
> + switch (i) {
> + case 1:
> + dsc_sink_color_depth_cap[cnt++] = DP_DSC_8_BPC;
> + break;
> + case 2:
> + dsc_sink_color_depth_cap[cnt++] = DP_DSC_10_BPC;
> + break;
> + case 3:
> + dsc_sink_color_depth_cap[cnt++] = DP_DSC_12_BPC;
> + break;
> + }
> + }
>  }
> -EXPORT_SYMBOL(drm_dp_dsc_sink_max_color_depth);
> +EXPORT_SYMBOL(drm_dp_dsc_sink_color_depth_cap);
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 3314e91f6eb3..ea3233b0a790 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -242,9 +242,9 @@
>  # define DP_DSC_YCbCr420_Native (1 << 4)
>  
>  #define DP_DSC_DEC_COLOR_DEPTH_CAP  0x06A
> -# define DP_DSC_8_BPC   (1 << 1)
> -# define DP_DSC_10_BPC  (1 << 2)
> -# define DP_DSC_12_BPC  (1 << 3)
> +# define DP_DSC_8_BPC   8
> +# define DP_DSC_10_BPC  10
> +# define DP_DSC_12_BPC  12


I'd suggest something simpler like:

int foo(u8 bpc[3])
{
int num_bpc = 0;

if (color_depth & DP_DSC_12_BPC)
bpc[num_bpc++] = 12;
if (color_depth & DP_DSC_10_BPC)
bpc[num_bpc++] = 10;
if (color_depth & DP_DSC_8_BPC)
bpc[num_bpc++] = 8;

return num_bpc;
}

>  
>  #define DP_DSC_PEAK_THROUGHPUT  0x06B
>  # define DP_DSC_THROUGHPUT_MODE_0_MASK  (0xf << 0)
> @@ -1123,7 +1123,8 @@ drm_dp_is_branch(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
>  u8 drm_dp_dsc_sink_max_slice_count(const u8 
> dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
>  bool is_edp);
>  u8 drm_dp_dsc_sink_line_buf_depth(const u8 
> dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE]);
> -u8 drm_dp_dsc_sink_max_color_depth(const u8 
> dsc_dpc[DP_DSC_RECEIVER_CAP_SIZE]);
> +void drm_dp_dsc_sink_color_depth_cap(const u8 
> dsc_dpc[DP_DSC_RECEIVER_CAP_SIZE],
> +  u8 *dsc_sink_color_depth_cap);
>  
>  static inline bool
>  drm_dp_sink_supports_dsc(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
> -- 
> 2.19.1

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


Re: [PATCH v2 05/24] drm/msm: dpu: Handle crtc pm_runtime_resume() directly

2018-11-19 Thread Jeykumar Sankaran

On 2018-11-16 10:42, Sean Paul wrote:

From: Sean Paul 

Instead of registering through dpu_power_handle just to get a call on
runtime_resume, call the crtc function directly.

Changes in v2:
- None

Signed-off-by: Sean Paul 


Reviewed-by: Jeykumar Sankaran 


---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  | 23 ++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h  | 10 ++
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c   |  4 
 drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h |  8 
 4 files changed, 20 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index e09209d6c469..c55cb751e2b4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -33,7 +33,6 @@
 #include "dpu_plane.h"
 #include "dpu_encoder.h"
 #include "dpu_vbif.h"
-#include "dpu_power_handle.h"
 #include "dpu_core_perf.h"
 #include "dpu_trace.h"

@@ -69,8 +68,6 @@ static void dpu_crtc_destroy(struct drm_crtc *crtc)
if (!crtc)
return;

-   dpu_crtc->phandle = NULL;
-
drm_crtc_cleanup(crtc);
mutex_destroy(&dpu_crtc->crtc_lock);
kfree(dpu_crtc);
@@ -844,15 +841,17 @@ static struct drm_crtc_state
*dpu_crtc_duplicate_state(struct drm_crtc *crtc)
return &cstate->base;
 }

-static void dpu_crtc_handle_power_event(u32 event_type, void *arg)
+void dpu_crtc_runtime_resume(struct drm_crtc *crtc)
 {
-   struct drm_crtc *crtc = arg;
struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
struct drm_encoder *encoder;

mutex_lock(&dpu_crtc->crtc_lock);

-   trace_dpu_crtc_handle_power_event(DRMID(crtc), event_type);
+   if (!dpu_crtc->enabled)
+   goto end;
+
+   trace_dpu_crtc_runtime_resume(DRMID(crtc));

/* restore encoder; crtc will be programmed during commit */
drm_for_each_encoder(encoder, crtc->dev) {
@@ -862,6 +861,7 @@ static void dpu_crtc_handle_power_event(u32
event_type, void *arg)
dpu_encoder_virt_restore(encoder);
}

+end:
mutex_unlock(&dpu_crtc->crtc_lock);
 }

@@ -917,10 +917,6 @@ static void dpu_crtc_disable(struct drm_crtc 
*crtc)

dpu_encoder_register_frame_event_callback(encoder, NULL,
NULL);
}

-   if (dpu_crtc->power_event)
-   dpu_power_handle_unregister_event(dpu_crtc->phandle,
-   dpu_crtc->power_event);
-
memset(cstate->mixers, 0, sizeof(cstate->mixers));
cstate->num_mixers = 0;

@@ -972,11 +968,6 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,

/* Enable/restore vblank irq handling */
drm_crtc_vblank_on(crtc);
-
-   dpu_crtc->power_event = dpu_power_handle_register_event(
-   dpu_crtc->phandle, DPU_POWER_EVENT_ENABLE,
-   dpu_crtc_handle_power_event, crtc, dpu_crtc->name);
-
 }

 struct plane_state {
@@ -1522,8 +1513,6 @@ struct drm_crtc *dpu_crtc_init(struct drm_device
*dev, struct drm_plane *plane,
/* initialize event handling */
spin_lock_init(&dpu_crtc->event_lock);

-   dpu_crtc->phandle = &kms->phandle;
-
DPU_DEBUG("%s: successfully initialized crtc\n", dpu_crtc->name);
return crtc;
 }
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index 4822602402f9..1dca91d1210f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -151,7 +151,6 @@ struct dpu_crtc_frame_event {
  * @event_worker  : Event worker queue
  * @event_lock: Spinlock around event handling code
  * @phandle: Pointer to power handler
- * @power_event   : registered power event handle
  * @cur_perf  : current performance committed to clock/bandwidth
driver
  */
 struct dpu_crtc {
@@ -187,9 +186,6 @@ struct dpu_crtc {
/* for handling internal event thread */
spinlock_t event_lock;

-   struct dpu_power_handle *phandle;
-   struct dpu_power_event *power_event;
-
struct dpu_core_perf_params cur_perf;

struct dpu_crtc_smmu_state_data smmu_state;
@@ -333,4 +329,10 @@ static inline bool dpu_crtc_is_enabled(struct
drm_crtc *crtc)
return crtc ? crtc->enabled : false;
 }

+/**
+ * dpu_crtc_runtime_resume - called by the top-level on 
pm_runtime_resume

+ * @crtc: CRTC to resume
+ */
+void dpu_crtc_runtime_resume(struct drm_crtc *crtc);
+
 #endif /* _DPU_CRTC_H_ */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index ab8574ab8327..654ea5060e02 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1137,6 +1137,7 @@ static int __maybe_unused 
dpu_runtime_resume(struct

device *dev)
int rc = -1;
struct platform_device *pdev = to_platform_device(dev);
struct dpu_kms *dpu_kms = platform_get_drvdata(pdev);
+   struct drm_crtc *crtc;
struct d

[Bug 108671] Massive Screen Artifacting on linux 4.19+

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108671

--- Comment #11 from Alex Deucher  ---
(In reply to Alex Deucher from comment #10)
> Possibly a duplicate of bug 108704.  Does the patch there help?

Nevermind, you have a vega10.

-- 
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 108671] Massive Screen Artifacting on linux 4.19+

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108671

--- Comment #10 from Alex Deucher  ---
Possibly a duplicate of bug 108704.  Does the patch there help?

-- 
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/amdgpu: Add missing firmware entry for HAINAN

2018-11-19 Thread Alex Deucher
On Mon, Nov 19, 2018 at 7:03 AM Christian König
 wrote:
>
> Am 19.11.18 um 12:55 schrieb Takashi Iwai:
> > Due to lack of MODULE_FIRMWARE() with hainan_mc.bin, the driver
> > doesn't work properly in initrd.  Let's add it.
> >
> > Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1116239
> > Fixes: 8eaf2b1faaf4 ("drm/amdgpu: switch firmware path for SI parts")
> > Cc: 
> > Signed-off-by: Takashi Iwai 
>
> Reviewed-by: Christian König 
>

Applied.  thanks!

Alex

> > ---
> >   drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c 
> > b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> > index e1c2b4e9c7b2..73ad02aea2b2 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> > @@ -46,6 +46,7 @@ MODULE_FIRMWARE("amdgpu/tahiti_mc.bin");
> >   MODULE_FIRMWARE("amdgpu/pitcairn_mc.bin");
> >   MODULE_FIRMWARE("amdgpu/verde_mc.bin");
> >   MODULE_FIRMWARE("amdgpu/oland_mc.bin");
> > +MODULE_FIRMWARE("amdgpu/hainan_mc.bin");
> >   MODULE_FIRMWARE("amdgpu/si58_mc.bin");
> >
> >   #define MC_SEQ_MISC0__MT__MASK   0xf000
>
> ___
> 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] drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c: Remove duplicate header

2018-11-19 Thread Alex Deucher
On Mon, Nov 19, 2018 at 2:17 PM Brajeswar Ghosh
 wrote:
>
> Remove gca/gfx_8_0_enum.h which is included more than once
>
> Signed-off-by: Brajeswar Ghosh 

Applied.  thanks!

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index 3d0f277a6523..4ca1ecf1ffd7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -44,7 +44,6 @@
>  #include "gca/gfx_8_0_d.h"
>  #include "gca/gfx_8_0_enum.h"
>  #include "gca/gfx_8_0_sh_mask.h"
> -#include "gca/gfx_8_0_enum.h"
>
>  #include "dce/dce_10_0_d.h"
>  #include "dce/dce_10_0_sh_mask.h"
> --
> 2.17.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 v5 08/12] dt-bindings: mediatek: Change the binding for mmsys clocks

2018-11-19 Thread Rob Herring
On Sun, Nov 18, 2018 at 11:12 AM Matthias Brugger
 wrote:
>
>
>
> On 11/17/18 12:15 AM, Rob Herring wrote:
> > On Fri, Nov 16, 2018 at 01:54:45PM +0100, matthias@kernel.org wrote:
> >> From: Matthias Brugger 
> >>
> >> On SoCs with no publical available HW or no working graphic stack
> >> we change the devicetree binding for the mmsys clock part. This
> >> way we don't need to register a platform device explicitly in the
> >> drm driver. Instead we can create a mmsys child which invokes the
> >> clock driver.
> >>
> >> Signed-off-by: Matthias Brugger 
> >> ---
> >>  .../bindings/arm/mediatek/mediatek,mmsys.txt  | 21 ---
> >>  .../display/mediatek/mediatek,disp.txt|  4 
> >>  2 files changed, 18 insertions(+), 7 deletions(-)
> >>
> >> diff --git 
> >> a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt 
> >> b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
> >> index 4468345f8b1a..d4e205981363 100644
> >> --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
> >> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
> >> @@ -1,4 +1,4 @@
> >> -Mediatek mmsys controller
> >> +Mediatek mmsys clock controller
> >>  
> >>
> >>  The Mediatek mmsys controller provides various clocks to the system.
> >> @@ -6,18 +6,25 @@ The Mediatek mmsys controller provides various clocks to 
> >> the system.
> >>  Required Properties:
> >>
> >>  - compatible: Should be one of:
> >> -- "mediatek,mt2712-mmsys", "syscon"
> >> -- "mediatek,mt6797-mmsys", "syscon"
> >> +- "mediatek,mt2712-mmsys-clk", "syscon"
> >> +- "mediatek,mt6797-mmsys-clk", "syscon"
> >
> > Doesn't match the example.>
> >>  - #clock-cells: Must be 1
> >>
> >> -The mmsys controller uses the common clk binding from
> >> +The mmsys clock controller uses the common clk binding from
> >>  Documentation/devicetree/bindings/clock/clock-bindings.txt
> >>  The available clocks are defined in dt-bindings/clock/mt*-clk.h.
> >> +It is a child of the mmsys block, see binding at:
> >> +Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt
> >>
> >>  Example:
> >>
> >> -mmsys: clock-controller@1400 {
> >> -compatible = "mediatek,mt8173-mmsys", "syscon";
> >> +mmsys: syscon@1400 {
> >> +compatible = "mediatek,mt2712-mmsys", "syscon", "simple-mfd";
> >>  reg = <0 0x1400 0 0x1000>;
> >> -#clock-cells = <1>;
> >> +
> >> +mmsys_clk: clock-controller@1400 {
> >> +compatible = "mediatek,mt2712-mmsys-clk";
> >> +#clock-cells = <1>;
> >
> > This goes against the general direction of not defining separate nodes
> > for providers with no resources.
> >
> > Why do you need this and what does it buy if you have to continue to
> > support the existing chips?
> >
>
> It would show explicitly that the mmsys block is used to probe two
> drivers, one for the gpu and one for the clocks. Otherwise that is
> hidden in the drm driver code. I think it is cleaner to describe that in
> the device tree.

No, that's maybe cleaner for the driver implementation in the Linux
kernel. What about other OS's or when Linux drivers and subsystems
needs change? Cleaner for DT is design bindings that reflect the h/w.
Hardware is sometimes just messy.

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


[Bug 108778] [R9 390] amdgpu: Fatal error during GPU init 4.20-rc2

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108778

Alex Deucher  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |DUPLICATE

--- Comment #7 from Alex Deucher  ---


*** This bug has been marked as a duplicate of bug 108704 ***

-- 
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 108778] [R9 390] amdgpu: Fatal error during GPU init 4.20-rc2

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108778

--- Comment #6 from Alex Deucher  ---
(In reply to Garth Theisen from comment #5)
> Reviewing my 4.18.18 log, the dce110_link_encoder_construct is present, but
> not 'amdgpu: [powerplay] Failed to retrieve minimum clocks.'.

Those are harmless.

-- 
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 108704] 4.19 amdgpu Tonga powerplay regressions

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108704

Alex Deucher  changed:

   What|Removed |Added

 CC||garththei...@hotmail.com

--- Comment #4 from Alex Deucher  ---
*** Bug 108778 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


Re: [PATCH v3] drm/bridge/sii902x: Add missing dependency on I2C_MUX

2018-11-19 Thread Boris Brezillon
On Mon, 19 Nov 2018 18:59:04 +
Fabrizio Castro  wrote:

> Hello Boris,
> 
> > From: Boris Brezillon 
> > Sent: 19 November 2018 16:55
> > Subject: Re: [PATCH v3] drm/bridge/sii902x: Add missing dependency on 
> > I2C_MUX
> >
> > Hi Fabrizio,
> >
> > On Mon, 19 Nov 2018 13:26:18 +
> > Fabrizio Castro  wrote:
> >  
> > > kbuild test robot reports:
> > >  
> > > >> ERROR: "i2c_mux_add_adapter" [drivers/gpu/drm/bridge/sii902x.ko]  
> > > undefined!  
> > > >> ERROR: "i2c_mux_alloc" [drivers/gpu/drm/bridge/sii902x.ko]  
> > > undefined!  
> > > >> ERROR: "i2c_mux_del_adapters" [drivers/gpu/drm/bridge/sii902x.ko]  
> > > undefined!
> > >
> > > Quite obviously the driver depends on I2C_MUX, but adding a "depends on"
> > > introduces a recursive dependency, therefore this patch selects I2C_MUX
> > > instead.
> > >
> > > Fixes: 21d808405fe4 ("drm/bridge/sii902x: Fix EDID readback")
> > > Signed-off-by: Fabrizio Castro 
> > > Link: https://lists.01.org/pipermail/kbuild-all/2018-November/054924.html 
> > >  
> >
> > I don't see the patch on the dri-devel ML, and it does not appear in
> > patchwork either :-/.  
> 
> Mmm, my email address may have been blocked somehow by the dri-devel ML,
> I can't think of anything else that could get in the way, as the dri-devel ML 
> was
> in copy in every email. Maybe that's because I am not subscribed? I have 
> started
> the subscription process, once I am subscribed I'll try and repost.

That's weird. Your previous were visible on the dri-devel patchwork [1].

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


RE: [PATCH 1/2] drm/amd/dm: Don't forget to attach MST encoders

2018-11-19 Thread Zuo, Jerry
Sure, I'll do it.

-Original Message-
From: Lyude Paul  
Sent: November 19, 2018 1:52 PM
To: Zuo, Jerry ; amd-...@lists.freedesktop.org
Cc: Wentland, Harry ; Li, Sun peng (Leo) 
; Deucher, Alexander ; Koenig, 
Christian ; Zhou, David(ChunMing) 
; David Airlie ; Li, Roman 
; S, Shirish ; Daniel Vetter 
; dri-devel@lists.freedesktop.org; 
linux-ker...@vger.kernel.org
Subject: Re: [PATCH 1/2] drm/amd/dm: Don't forget to attach MST encoders

Cool! If it did actually fix those problems, would you mind making sure this 
gets Cc'd to stable when it gets pushed upstream?

On Mon, 2018-11-19 at 15:00 +, Zuo, Jerry wrote:
> Reviewed-by: Jerry (Fangzhi) Zuo 
> 
> The change fixed MST + SST daisy chain and S3 scenarios. The issue 
> shows huge delay in MST + SST daisy chain, and soft hang in S3 resume.
> 
> The aux sequence is changed by failed iteration search in 
> drm_connector_for_each_possible_encoder().
> The failure of searching for the best encoder for the connector due to 
> the miss of attached encoder in the process of adding MST connector. 
> The iteration search takes time to push 
> drm_dp_send_enum_path_resources() aux transaction after the mode 
> probe, and causes conflict to drm_dp_mst_i2c_xfer(), leading to the aux 
> transaction timeout.
> 
> -Original Message-
> From: Lyude Paul 
> Sent: November 16, 2018 6:25 PM
> To: amd-...@lists.freedesktop.org
> Cc: Zuo, Jerry ; Wentland, Harry 
>  ; Li, Sun peng (Leo) ; 
> Deucher, Alexander < alexander.deuc...@amd.com>; Koenig, Christian 
> ; Zhou, David(ChunMing) 
> ; David Airlie  ; Li, Roman 
> ; S, Shirish ; Daniel Vetter 
> ; dri-devel@lists.freedesktop.org; 
> linux-ker...@vger.kernel.org
> Subject: [PATCH 1/2] drm/amd/dm: Don't forget to attach MST encoders
> 
> Drive-by fix, this is bound to cause problems somewhere.
> 
> Signed-off-by: Lyude Paul 
> Cc: Jerry Zuo 
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> 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 d02c32a1039c..0cca1809fdcd 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
> @@ -342,6 +342,8 @@ dm_dp_add_mst_connector(struct 
> drm_dp_mst_topology_mgr *mgr,
>   master->connector_id);
>  
>   aconnector->mst_encoder = dm_dp_create_fake_mst_encoder(master);
> + drm_connector_attach_encoder(&aconnector->base,
> +  &aconnector->mst_encoder->base);
>  
>   /*
>* TODO: understand why this one is needed
--
Cheers,
Lyude Paul

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


Re: [PATCH 1/2] drm/amd/dm: Don't forget to attach MST encoders

2018-11-19 Thread Lyude Paul
Cool! If it did actually fix those problems, would you mind making sure this
gets Cc'd to stable when it gets pushed upstream?

On Mon, 2018-11-19 at 15:00 +, Zuo, Jerry wrote:
> Reviewed-by: Jerry (Fangzhi) Zuo 
> 
> The change fixed MST + SST daisy chain and S3 scenarios. The issue shows
> huge delay in MST + SST daisy chain, and soft hang in S3 resume.
> 
> The aux sequence is changed by failed iteration search in
> drm_connector_for_each_possible_encoder(). 
> The failure of searching for the best encoder for the connector due to the
> miss of attached encoder in the process of adding MST connector. The
> iteration search takes time to push drm_dp_send_enum_path_resources() aux
> transaction after the mode probe, and causes conflict to
> drm_dp_mst_i2c_xfer(), leading to the aux transaction timeout.
> 
> -Original Message-
> From: Lyude Paul  
> Sent: November 16, 2018 6:25 PM
> To: amd-...@lists.freedesktop.org
> Cc: Zuo, Jerry ; Wentland, Harry 
> ; Li, Sun peng (Leo) ; Deucher, Alexander <
> alexander.deuc...@amd.com>; Koenig, Christian ;
> Zhou, David(ChunMing) ; David Airlie 
> ; Li, Roman ; S, Shirish ; Daniel
> Vetter ; dri-devel@lists.freedesktop.org; 
> linux-ker...@vger.kernel.org
> Subject: [PATCH 1/2] drm/amd/dm: Don't forget to attach MST encoders
> 
> Drive-by fix, this is bound to cause problems somewhere.
> 
> Signed-off-by: Lyude Paul 
> Cc: Jerry Zuo 
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> 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 d02c32a1039c..0cca1809fdcd 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
> @@ -342,6 +342,8 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr
> *mgr,
>   master->connector_id);
>  
>   aconnector->mst_encoder = dm_dp_create_fake_mst_encoder(master);
> + drm_connector_attach_encoder(&aconnector->base,
> +  &aconnector->mst_encoder->base);
>  
>   /*
>* TODO: understand why this one is needed
-- 
Cheers,
Lyude Paul

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


Re: [PATCH 1/2] drm/msm/dpu: add dpu encoder uninit

2018-11-19 Thread Jeykumar Sankaran

On 2018-11-16 13:37, Sean Paul wrote:

On Fri, Nov 16, 2018 at 04:35:26PM -0500, Sean Paul wrote:

On Fri, Nov 16, 2018 at 11:22:21AM -0800, Jeykumar Sankaran wrote:
> Add encoder interface to release dpu encoder
> on mode_init failures in kms.
>
> Signed-off-by: Jeykumar Sankaran 
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 12 ++--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h |  6 ++
>  2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c

b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c

> index dd7ab85..b253165 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -422,6 +422,15 @@ void dpu_encoder_get_hw_resources(struct

drm_encoder *drm_enc,

>}
>  }
>
> +void dpu_encoder_uninit(struct drm_encoder *drm_enc)
> +{
> +  struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
> +
> +  drm_encoder_cleanup(drm_enc);
> +
> +  kfree(dpu_enc);
> +}
> +
>  static void dpu_encoder_destroy(struct drm_encoder *drm_enc)
>  {
>struct dpu_encoder_virt *dpu_enc = NULL;
> @@ -453,10 +462,9 @@ static void dpu_encoder_destroy(struct

drm_encoder *drm_enc)

>dpu_enc->num_phys_encs = 0;
>mutex_unlock(&dpu_enc->enc_lock);
>
> -  drm_encoder_cleanup(drm_enc);
>mutex_destroy(&dpu_enc->enc_lock);
>
> -  kfree(dpu_enc);
> +  dpu_encoder_uninit(drm_enc);
>  }
>
>  void dpu_encoder_helper_split_config(
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h

b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h

> index 9dbf38f..60b88bd 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> @@ -142,6 +142,12 @@ struct drm_encoder *dpu_encoder_init(
>int drm_enc_mode);
>
>  /**
> + * dpu_encoder_uninit - uninitialize virtual encoder object
> + * @drm_enc:  Pointer to drm encoder
> + */
> +void dpu_encoder_uninit(struct drm_encoder *drm_enc);

Just make it static?


I just saw YueHaibing's patch to remove the kfree entirely since 
dpu_enc

is
devm_* managed. IMO, that'd be a better solution than this.

Sean


I still need the unint here to call drm_encoder_cleanup
if the display mode_init fails.

I will rebase the patch on top of https://lkml.org/lkml/2018/11/17/87

Thanks,
Jeykumar S.




> +
> +/**
>   * dpu_encoder_setup - setup dpu_encoder for the display probed
>   * @dev:  Pointer to drm device structure
>   * @enc:  Pointer to the drm_encoder
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora

Forum,

> a Linux Foundation Collaborative Project
>

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


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


Re: [PATCH 1/2] drm/msm/dpu: add dpu encoder uninit

2018-11-19 Thread Jeykumar Sankaran

On 2018-11-16 13:35, Sean Paul wrote:

On Fri, Nov 16, 2018 at 11:22:21AM -0800, Jeykumar Sankaran wrote:

Add encoder interface to release dpu encoder
on mode_init failures in kms.

Signed-off-by: Jeykumar Sankaran 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 12 ++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h |  6 ++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c

b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c

index dd7ab85..b253165 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -422,6 +422,15 @@ void dpu_encoder_get_hw_resources(struct

drm_encoder *drm_enc,

}
 }

+void dpu_encoder_uninit(struct drm_encoder *drm_enc)
+{
+   struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
+
+   drm_encoder_cleanup(drm_enc);
+
+   kfree(dpu_enc);
+}
+
 static void dpu_encoder_destroy(struct drm_encoder *drm_enc)
 {
struct dpu_encoder_virt *dpu_enc = NULL;
@@ -453,10 +462,9 @@ static void dpu_encoder_destroy(struct 
drm_encoder

*drm_enc)

dpu_enc->num_phys_encs = 0;
mutex_unlock(&dpu_enc->enc_lock);

-   drm_encoder_cleanup(drm_enc);
mutex_destroy(&dpu_enc->enc_lock);

-   kfree(dpu_enc);
+   dpu_encoder_uninit(drm_enc);
 }

 void dpu_encoder_helper_split_config(
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h

b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h

index 9dbf38f..60b88bd 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
@@ -142,6 +142,12 @@ struct drm_encoder *dpu_encoder_init(
int drm_enc_mode);

 /**
+ * dpu_encoder_uninit - uninitialize virtual encoder object
+ * @drm_enc:  Pointer to drm encoder
+ */
+void dpu_encoder_uninit(struct drm_encoder *drm_enc);


Just make it static?

encoder_uninit will be called by dpu_kms in patch 2/2 in case
of mode_init failures.

Thanks and Regards,
Jeykumar S.



+
+/**
  * dpu_encoder_setup - setup dpu_encoder for the display probed
  * @dev:   Pointer to drm device structure
  * @enc:   Pointer to the drm_encoder
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora

Forum,

a Linux Foundation Collaborative Project



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


[Bug 108729] [Kaveri CIK 7400K] [regression, works on radeon] [drm:vce_v2_0_start [amdgpu]] *ERROR* VCE not responding, trying to reset the ECPU!!!

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108729

Vedran Miletić  changed:

   What|Removed |Added

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

--- Comment #5 from Vedran Miletić  ---
Everything is good with 4.19.2.

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


Re: [PATCH v2 3/3] drm/msm: dpu: Make legacy cursor updates asynchronous

2018-11-19 Thread Jeykumar Sankaran

On 2018-11-16 12:02, Sean Paul wrote:

On Thu, Nov 08, 2018 at 02:00:51PM -0800, Jeykumar Sankaran wrote:

On 2018-10-30 09:00, Sean Paul wrote:
> From: Sean Paul 
>
> This patch sprinkles a few async/legacy_cursor_update checks
> through commit to ensure that cursor updates aren't blocked on vsync.
> There are 2 main components to this, the first is that we don't want

to

> wait_for_commit_done in msm_atomic  before returning from
> atomic_complete.
> The second is that in dpu we don't want to wait for frame_done events
> when
> updating the cursor.
>
> Changes in v2:
> - None
>
> Signed-off-by: Sean Paul 
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 44

+++--

>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h|  3 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 22 +++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h |  6 ++-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c |  5 ++-
>  drivers/gpu/drm/msm/msm_atomic.c|  3 +-
>  6 files changed, 49 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index ed84cf44a222..1e3e57817b72 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -702,7 +702,7 @@ static int _dpu_crtc_wait_for_frame_done(struct
> drm_crtc *crtc)
>return rc;
>  }
>
> -void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
> +void dpu_crtc_commit_kickoff(struct drm_crtc *crtc, bool async)
>  {
>struct drm_encoder *encoder;
>struct drm_device *dev = crtc->dev;
> @@ -731,27 +731,30 @@ void dpu_crtc_commit_kickoff(struct drm_crtc
> *crtc)
> * Encoder will flush/start now, unless it has a tx
> pending.
> * If so, it may delay and flush at an irq event (e.g.
> ppdone)
> */
> -  dpu_encoder_prepare_for_kickoff(encoder, ¶ms);
> +  dpu_encoder_prepare_for_kickoff(encoder, ¶ms, async);
>}
>
> -  /* wait for frame_event_done completion */
> -  DPU_ATRACE_BEGIN("wait_for_frame_done_event");
> -  ret = _dpu_crtc_wait_for_frame_done(crtc);
> -  DPU_ATRACE_END("wait_for_frame_done_event");
> -  if (ret) {
> -  DPU_ERROR("crtc%d wait for frame done
> failed;frame_pending%d\n",
> -  crtc->base.id,
> -  atomic_read(&dpu_crtc->frame_pending));
> -  goto end;
> -  }
>
> -  if (atomic_inc_return(&dpu_crtc->frame_pending) == 1) {
> -  /* acquire bandwidth and other resources */
> -  DPU_DEBUG("crtc%d first commit\n", crtc->base.id);
> -  } else
> -  DPU_DEBUG("crtc%d commit\n", crtc->base.id);
> +  if (!async) {
> +  /* wait for frame_event_done completion */
> +  DPU_ATRACE_BEGIN("wait_for_frame_done_event");
> +  ret = _dpu_crtc_wait_for_frame_done(crtc);
> +  DPU_ATRACE_END("wait_for_frame_done_event");
> +  if (ret) {
> +  DPU_ERROR("crtc%d wait for frame done
> failed;frame_pending%d\n",
> +  crtc->base.id,
> +
> atomic_read(&dpu_crtc->frame_pending));
> +  goto end;
> +  }
> +
> +  if (atomic_inc_return(&dpu_crtc->frame_pending) == 1) {
> +  /* acquire bandwidth and other resources */
> +  DPU_DEBUG("crtc%d first commit\n", crtc->base.id);
> +  } else
> +  DPU_DEBUG("crtc%d commit\n", crtc->base.id);
>
> -  dpu_crtc->play_count++;
> +  dpu_crtc->play_count++;
> +  }
>
>dpu_vbif_clear_errors(dpu_kms);
>
> @@ -759,11 +762,12 @@ void dpu_crtc_commit_kickoff(struct drm_crtc
> *crtc)
>if (encoder->crtc != crtc)
>continue;
>
> -  dpu_encoder_kickoff(encoder);
> +  dpu_encoder_kickoff(encoder, async);
>}
>
>  end:
> -  reinit_completion(&dpu_crtc->frame_done_comp);
> +  if (!async)
> +  reinit_completion(&dpu_crtc->frame_done_comp);
>DPU_ATRACE_END("crtc_commit");
>  }
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> index 4822602402f9..ec633ce3ee6c 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> @@ -277,8 +277,9 @@ int dpu_crtc_vblank(struct drm_crtc *crtc, bool

en);

>  /**
>   * dpu_crtc_commit_kickoff - trigger kickoff of the commit for this
> crtc
>   * @crtc: Pointer to drm crtc object
> + * @async: true if the commit is asynchronous, false otherwise
>   */
> -void dpu_crtc_commit_kickoff(struct drm_crtc *crtc);
> +void dpu_crtc_commit_kickoff(struct drm_crtc *crtc, bool async);
>
>  /**
>   * dpu_crtc_complete_commit - callback signalling completion of

current

> commit
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 82c55efb500f..a8ba10ceaacf 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_

[Bug 108778] [R9 390] amdgpu: Fatal error during GPU init 4.20-rc2

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108778

--- Comment #5 from Garth Theisen  ---
Reviewing my 4.18.18 log, the dce110_link_encoder_construct is present, but not
'amdgpu: [powerplay] Failed to retrieve minimum clocks.'.

[3.314952] [drm] amdgpu: 8192M of VRAM memory ready
[3.314954] [drm] amdgpu: 8192M of GTT memory ready.
[3.314967] [drm] GART: num cpu pages 262144, num gpu pages 262144
[3.315973] [drm] PCIE GART of 1024M enabled (table at 0x00F4007E9000).
[3.317330] [drm] Internal thermal controller with fan control
[3.343843] random: alsactl: uninitialized urandom read (4 bytes read)
[3.343850] random: alsactl: uninitialized urandom read (4 bytes read)
[3.347055] [drm] Invalid PCC GPIO: 13!
[3.347057] [drm] amdgpu: dpm initialized
[3.356305] [drm] Found UVD firmware Version: 1.64 Family ID: 9
[3.357433] [drm] Found VCE firmware Version: 50.10 Binary ID: 2
[3.357629] [drm] PCIE gen 2 link speeds already enabled
[3.365448] [drm] dce110_link_encoder_construct: Failed to get
encoder_cap_info from VBIOS with error code 4!
[3.365462] [drm] dce110_link_encoder_construct: Failed to get
encoder_cap_info from VBIOS with error code 4!
[3.365475] [drm] dce110_link_encoder_construct: Failed to get
encoder_cap_info from VBIOS with error code 4!
[3.377133] [drm] Display Core initialized with v3.1.44!
[3.403803] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[3.403805] [drm] Driver supports precise vblank timestamp query.
[3.442064] [drm] UVD initialized successfully.
[3.570052] [drm] VCE initialized successfully.
[3.572268] [drm] fb mappable at 0xD0BD
[3.572269] [drm] vram apper at 0xD000
[3.572270] [drm] size 8294400
[3.572271] [drm] fb depth is 24
[3.572271] [drm]pitch is 7680
[3.572350] fbcon: amdgpudrmfb (fb0) is primary device
[3.576367] [drm] dce_get_required_clocks_state: clocks unsupported disp_clk
681000 pix_clk 148500
[3.593179] Console: switching to colour frame buffer device 240x67
[3.599360] amdgpu :01:00.0: fb0: amdgpudrmfb frame buffer device
[3.607152] [drm] Initialized amdgpu 3.26.0 20150101 for :01:00.0 on
minor 0

(In reply to Garth Theisen from comment #4)
> After applying the patch 4.19.2 boots without the Init failure.
> 
> There seems to be some additional issues present in the dmesg log after the
> patch ... but this might be occurring for the 4.18.x series also.
> 
> [3.705820] amdgpu: [powerplay] Failed to retrieve minimum clocks.
> [3.705821] amdgpu: [powerplay] Error in phm_get_clock_info 
> [3.705922] [drm] dce110_link_encoder_construct: Failed to get
> encoder_cap_info from VBIOS with error code 4!
> [3.705932] [drm] dce110_link_encoder_construct: Failed to get
> encoder_cap_info from VBIOS with error code 4!
> [3.705943] [drm] dce110_link_encoder_construct: Failed to get
> encoder_cap_info from VBIOS with error code 4!
> [3.720386] [drm] Display Core initialized with v3.1.59!
> [3.748551] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> [3.748552] [drm] Driver supports precise vblank timestamp query.
> [3.786502] [drm] UVD initialized successfully.
> [3.907523] [drm] VCE initialized successfully.
> [3.911228] [drm] fb mappable at 0xD0BD
> [3.911230] [drm] vram apper at 0xD000
> [3.911232] [drm] size 8294400
> [3.911233] [drm] fb depth is 24
> [3.911234] [drm]pitch is 7680
> [3.911401] fbcon: amdgpudrmfb (fb0) is primary device
> [3.937471] Console: switching to colour frame buffer device 240x67
> [3.942828] amdgpu :01:00.0: fb0: amdgpudrmfb frame buffer device
> [3.949554] [drm] Initialized amdgpu 3.27.0 20150101 for :01:00.0 on
> minor 0

-- 
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 108778] [R9 390] amdgpu: Fatal error during GPU init 4.20-rc2

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108778

--- Comment #4 from Garth Theisen  ---
After applying the patch 4.19.2 boots without the Init failure.

There seems to be some additional issues present in the dmesg log after the
patch ... but this might be occurring for the 4.18.x series also.

[3.705820] amdgpu: [powerplay] Failed to retrieve minimum clocks.
[3.705821] amdgpu: [powerplay] Error in phm_get_clock_info 
[3.705922] [drm] dce110_link_encoder_construct: Failed to get
encoder_cap_info from VBIOS with error code 4!
[3.705932] [drm] dce110_link_encoder_construct: Failed to get
encoder_cap_info from VBIOS with error code 4!
[3.705943] [drm] dce110_link_encoder_construct: Failed to get
encoder_cap_info from VBIOS with error code 4!
[3.720386] [drm] Display Core initialized with v3.1.59!
[3.748551] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[3.748552] [drm] Driver supports precise vblank timestamp query.
[3.786502] [drm] UVD initialized successfully.
[3.907523] [drm] VCE initialized successfully.
[3.911228] [drm] fb mappable at 0xD0BD
[3.911230] [drm] vram apper at 0xD000
[3.911232] [drm] size 8294400
[3.911233] [drm] fb depth is 24
[3.911234] [drm]pitch is 7680
[3.911401] fbcon: amdgpudrmfb (fb0) is primary device
[3.937471] Console: switching to colour frame buffer device 240x67
[3.942828] amdgpu :01:00.0: fb0: amdgpudrmfb frame buffer device
[3.949554] [drm] Initialized amdgpu 3.27.0 20150101 for :01:00.0 on
minor 0

-- 
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 200695] Blank screen on RX 580 with amdgpu.dc=1 enabled (no displays detected)

2018-11-19 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=200695

Claude Heiland-Allen (cla...@mathr.co.uk) changed:

   What|Removed |Added

 Kernel Version|4.17.19, 4.18.0-rc7,|4.17.19, 4.18.5 through
   |4.18.5, 4.18.6, 4.18.7, |4.18.19, 4.19-rc1 through
   |4.18.8, 4.18.9, 4.18.10,|4.19.2, 4.20-rc1 through
   |4.18.11, 4.18.12, 4.18.13,  |4.20-rc3
   |4.18.14, 4.18.15, 4.18.16,  |
   |4.18.17, 4.18.18, 4.19-rc1, |
   |4.19-rc2, 4.19-rc3, |
   |4.19-rc4, 4.19-rc5, |
   |4.19-rc6, 4.19-rc7, |
   |4.19-rc8, 4.19, 4.19.1, |
   |4.20-rc1, 4.20-rc2  |

--- Comment #20 from Claude Heiland-Allen (cla...@mathr.co.uk) ---
still an issue in 4.18.19
still an issue in 4.19.2
still an issue in 4.20-rc3

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


Re: [PATCH] drm/i915: change i915_sw_fence license to MIT

2018-11-19 Thread Rodrigo Vivi
On Sun, Nov 18, 2018 at 08:44:30PM +1100, Jonathan Gray wrote:
> On Wed, Oct 31, 2018 at 08:43:03AM +, Chris Wilson wrote:
> > Quoting Jonathan Gray (2018-10-31 00:56:12)
> > > Chris Wilson said Intel is willing to change the license of these files
> > > to MIT.
> > > 
> > > Signed-off-by: Jonathan Gray 
> > Reviewed-by: Chris Wilson 
> > -Chris
> 
> Still looking to get this merged/reviewed by one of the i915 maintainers.

I believe this patch should have a better commit message.

Also Jani was looking to this conversions.
Jani, any comment here?

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


[Bug 108781] 4.19 Regression - Hawaii (R9 390) boot failure - Invalid PCC GPIO / invalid powerlevel state / Fatal error during GPU init

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108781

--- Comment #5 from m...@mgoodwin.net ---
I can add that I also hit this on a R9 290 Reference card:

01:00.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI]
Hawaii PRO [Radeon R9 290/390] (prog-if 00 [VGA controller])
Subsystem: Advanced Micro Devices, Inc. [AMD/ATI] Device 0b00
Flags: fast devsel, IRQ 16
Memory at e000 (64-bit, prefetchable) [size=256M]
Memory at f000 (64-bit, prefetchable) [size=8M]
I/O ports at e000 [size=256]
Memory at f7e0 (32-bit, non-prefetchable) [size=256K]
Expansion ROM at 000c [disabled] [size=128K]
Capabilities: [48] Vendor Specific Information: Len=08 
Capabilities: [50] Power Management version 3
Capabilities: [58] Express Legacy Endpoint, MSI 00
Capabilities: [a0] MSI: Enable- Count=1/1 Maskable- 64bit+
Capabilities: [100] Vendor Specific Information: ID=0001 Rev=1 Len=010

Capabilities: [150] Advanced Error Reporting
Capabilities: [270] Secondary PCI Express 
Capabilities: [2b0] Address Translation Service (ATS)
Capabilities: [2c0] Page Request Interface (PRI)
Capabilities: [2d0] Process Address Space ID (PASID)
Kernel modules: radeon, amdgpu


with Fedora's:

kernel-4.19.2-300.fc29.x86_64


with options:

$ cat /etc/modprobe.d/amdgpu.conf
blacklist radeon
options amdgpu cik_support=1
options amdgpu dpm=1
options amdgpu dc=0
options amdgpu pcie_gen2=0


Dmesg:


kern  :err   : [Mon Nov 19 12:11:42 2018] [drm:amdgpu_vce_ring_test_ring
[amdgpu]] *ERROR* amdgpu: ring 12 test failed
kern  :err   : [Mon Nov 19 12:11:42 2018] [drm:amdgpu_device_init.cold.28
[amdgpu]] *ERROR* hw_init of IP block  failed -110
kern  :err   : [Mon Nov 19 12:11:42 2018] amdgpu :01:00.0:
amdgpu_device_ip_init failed
kern  :err   : [Mon Nov 19 12:11:42 2018] amdgpu :01:00.0: Fatal error
during GPU init
kern  :info  : [Mon Nov 19 12:11:42 2018] [drm] amdgpu: finishing device.
kern  :warn  : [Mon Nov 19 12:11:42 2018] [ cut here ]
kern  :warn  : [Mon Nov 19 12:11:42 2018] Memory manager not clean during
takedown.
kern  :warn  : [Mon Nov 19 12:11:42 2018] WARNING: CPU: 1 PID: 380 at
drivers/gpu/drm/drm_mm.c:950 drm_mm_takedown+0x1f/0x30 [drm]
kern  :warn  : [Mon Nov 19 12:11:42 2018] Modules linked in: btrfs libcrc32c
xor amdkfd zstd_decompress zstd_compress amd_iommu_v2 xxhash amdgpu(+) raid6_pq
chash gpu_sched i2c_algo_bit drm_kms_helper ttm crc32c_intel drm e1000e
serio_raw uas usb_storage bfq lz4 lz4_compress
kern  :warn  : [Mon Nov 19 12:11:42 2018] CPU: 1 PID: 380 Comm: systemd-udevd
Not tainted 4.19.2-300.fc29.x86_64 #1
kern  :warn  : [Mon Nov 19 12:11:42 2018] Hardware name: System manufacturer
System Product Name/P8P67 PRO REV 3.1, BIOS 3602 11/01/2012
kern  :warn  : [Mon Nov 19 12:11:42 2018] RIP: 0010:drm_mm_takedown+0x1f/0x30
[drm]
kern  :warn  : [Mon Nov 19 12:11:42 2018] Code: f6 c3 48 8d 41 c0 eb bb 0f 1f
00 66 66 66 66 90 48 8b 47 38 48 83 c7 38 48 39 c7 75 01 c3 48 c7 c7 a0 88 4e
c0 e8 6b 2d c0 fb <0f> 0b c3 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 66 66 66
66 90
kern  :warn  : [Mon Nov 19 12:11:42 2018] RSP: 0018:bced41e5f9e8 EFLAGS:
00010282
kern  :warn  : [Mon Nov 19 12:11:42 2018] RAX:  RBX:
948185a21d00 RCX: 0006
kern  :warn  : [Mon Nov 19 12:11:42 2018] RDX: 0007 RSI:
0086 RDI: 94818ea96860
kern  :warn  : [Mon Nov 19 12:11:42 2018] RBP: 9481836429a0 R08:
003c R09: 0003
kern  :warn  : [Mon Nov 19 12:11:42 2018] R10:  R11:
0001 R12: 948183642980
kern  :warn  : [Mon Nov 19 12:11:42 2018] R13:  R14:
0170 R15: 9481860b9e30
kern  :warn  : [Mon Nov 19 12:11:42 2018] FS:  7f283bcf8940()
GS:94818ea8() knlGS:
kern  :warn  : [Mon Nov 19 12:11:42 2018] CS:  0010 DS:  ES:  CR0:
80050033
kern  :warn  : [Mon Nov 19 12:11:42 2018] CR2: 559d1d60abd8 CR3:
000404f18004 CR4: 000606e0
kern  :warn  : [Mon Nov 19 12:11:42 2018] Call Trace:
kern  :warn  : [Mon Nov 19 12:11:42 2018]  amdgpu_vram_mgr_fini+0x22/0x40
[amdgpu]
kern  :warn  : [Mon Nov 19 12:11:42 2018]  ttm_bo_clean_mm+0xa2/0xb0 [ttm]
kern  :warn  : [Mon Nov 19 12:11:42 2018]  amdgpu_ttm_fini+0x71/0x100 [amdgpu]
kern  :warn  : [Mon Nov 19 12:11:42 2018]  amdgpu_bo_fini+0xe/0x30 [amdgpu]
kern  :warn  : [Mon Nov 19 12:11:42 2018]  gmc_v7_0_sw_fini+0x32/0x60 [amdgpu]
kern  :warn  : [Mon Nov 19 12:11:42 2018]  amdgpu_device_fini+0x2cc/0x487
[amdgpu]
kern  :warn  : [Mon Nov 19 12:11:42 2018]  amdgpu_driver_unload_kms+0x42/0x90
[amdgpu]
kern  :warn  : [Mon Nov 19 12:11:42 2018]  amdgpu_driver_load_kms+0x146/0x2c0
[amdgpu]
kern  :warn  : [Mon Nov 19 12:11:42 2018]  drm_dev_register+0x109/0x140 [drm]
kern  :warn  : [Mon Nov 19 12:11:42 2018]  amdgpu_pci_probe+0x13c/0x1c0
[am

Re: [PATCH v2 6/9] phy: Move Allwinner A31 D-PHY driver to drivers/phy/

2018-11-19 Thread Sakari Ailus
Hi Maxime,

On Tue, Nov 06, 2018 at 03:54:18PM +0100, Maxime Ripard wrote:
> Now that our MIPI D-PHY driver has been converted to the phy framework,
> let's move it into the drivers/phy directory.
> 
> Signed-off-by: Maxime Ripard 
> ---
>  drivers/gpu/drm/sun4i/Kconfig   |  10 +-
>  drivers/gpu/drm/sun4i/Makefile  |   1 +-
>  drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c | 318 +-
>  drivers/phy/allwinner/Kconfig   |  12 +-
>  drivers/phy/allwinner/Makefile  |   1 +-
>  drivers/phy/allwinner/phy-sun6i-mipi-dphy.c | 318 +-
>  6 files changed, 332 insertions(+), 328 deletions(-)
>  delete mode 100644 drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c
>  create mode 100644 drivers/phy/allwinner/phy-sun6i-mipi-dphy.c

Could you use git format-patch -M option on the next time, please?

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3] drm/bridge/sii902x: Add missing dependency on I2C_MUX

2018-11-19 Thread Boris Brezillon
Hi Fabrizio,

On Mon, 19 Nov 2018 13:26:18 +
Fabrizio Castro  wrote:

> kbuild test robot reports:
> 
> >> ERROR: "i2c_mux_add_adapter" [drivers/gpu/drm/bridge/sii902x.ko]  
> undefined!
> >> ERROR: "i2c_mux_alloc" [drivers/gpu/drm/bridge/sii902x.ko]  
> undefined!
> >> ERROR: "i2c_mux_del_adapters" [drivers/gpu/drm/bridge/sii902x.ko]  
> undefined!
> 
> Quite obviously the driver depends on I2C_MUX, but adding a "depends on"
> introduces a recursive dependency, therefore this patch selects I2C_MUX
> instead.
> 
> Fixes: 21d808405fe4 ("drm/bridge/sii902x: Fix EDID readback")
> Signed-off-by: Fabrizio Castro 
> Link: https://lists.01.org/pipermail/kbuild-all/2018-November/054924.html

I don't see the patch on the dri-devel ML, and it does not appear in
patchwork either :-/.

> ---
> v2->v3:
> * Changed the title
> 
> v1->v2:
> * Added "Fixes" tag
> 
>  drivers/gpu/drm/bridge/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index 9eeb8ef..2fee47b 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -95,6 +95,7 @@ config DRM_SII902X
>   depends on OF
>   select DRM_KMS_HELPER
>   select REGMAP_I2C
> + select I2C_MUX
>   ---help---
> Silicon Image sii902x bridge chip driver.
>  

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


[Bug 106175] amdgpu.dc=1 shows performance issues with Xorg compositors when moving windows

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=106175

--- Comment #49 from Brandon Wright  ---
I'm going to speculate that maybe the hardware cursor updates are triggering an
update to the vsync timestamp counter or msc that's incorrect and throwing off
the timing.

-- 
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] [PATCH v3 2/3] drm: Add CRTC background color property (v3)

2018-11-19 Thread Brian Starkey
Hi Matt,

On Thu, Nov 15, 2018 at 02:13:45PM -0800, Matt Roper wrote:
>Some display controllers can be programmed to present non-black colors
>for pixels not covered by any plane (or pixels covered by the
>transparent regions of higher planes).  Compositors that want a UI with
>a solid color background can potentially save memory bandwidth by
>setting the CRTC background property and using smaller planes to display
>the rest of the content.
>
>To avoid confusion between different ways of encoding RGB data, we
>define a standard 64-bit format that should be used for this property's
>value.  Helper functions and macros are provided to generate and dissect
>values in this standard format with varying component precision values.
>
>v2:
> - Swap internal representation's blue and red bits to make it easier
>   to read if printed out.  (Ville)
> - Document bgcolor property in drm_blend.c.  (Sean Paul)
> - s/background_color/bgcolor/ for consistency between property name and
>   value storage field.  (Sean Paul)
> - Add a convenience function to attach property to a given crtc.
>
>v3:
> - Restructure ARGB component extraction macros to be easier to
>   understand and enclose the parameters in () to avoid calculations
>   if expressions are passed.  (Sean Paul)
> - s/rgba/argb/ in helper function/macro names.  Even though the idea is
>   to not worry about the internal representation of the u64, it can
>   still be confusing to look at code that uses 'rgba' terminology, but
>   stores values with argb ordering.  (Ville)
>
>Cc: dri-devel@lists.freedesktop.org
>Cc: wei.c...@intel.com
>Cc: harish.krupo@intel.com
>Cc: Ville Syrjälä 
>Cc: Sean Paul 
>Signed-off-by: Matt Roper 
>Reviewed-by(v2): Sean Paul 
>---
> drivers/gpu/drm/drm_atomic_state_helper.c |  1 +
> drivers/gpu/drm/drm_atomic_uapi.c |  5 +
> drivers/gpu/drm/drm_blend.c   | 21 ++---
> drivers/gpu/drm/drm_mode_config.c |  6 ++
> include/drm/drm_blend.h   |  1 +
> include/drm/drm_crtc.h| 17 +
> include/drm/drm_mode_config.h |  5 +
> include/uapi/drm/drm_mode.h   | 28 
> 8 files changed, 81 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c 
>b/drivers/gpu/drm/drm_atomic_state_helper.c
>index 3ba996069d69..2f8c55668089 100644
>--- a/drivers/gpu/drm/drm_atomic_state_helper.c
>+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
>@@ -101,6 +101,7 @@ void __drm_atomic_helper_crtc_duplicate_state(struct 
>drm_crtc *crtc,
>   state->planes_changed = false;
>   state->connectors_changed = false;
>   state->color_mgmt_changed = false;
>+  state->bgcolor_changed = false;
>   state->zpos_changed = false;
>   state->commit = NULL;
>   state->event = NULL;
>diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
>b/drivers/gpu/drm/drm_atomic_uapi.c
>index 86ac33922b09..b95a55a778e2 100644
>--- a/drivers/gpu/drm/drm_atomic_uapi.c
>+++ b/drivers/gpu/drm/drm_atomic_uapi.c
>@@ -467,6 +467,9 @@ static int drm_atomic_crtc_set_property(struct drm_crtc 
>*crtc,
>   return -EFAULT;
>
>   set_out_fence_for_crtc(state->state, crtc, fence_ptr);
>+  } else if (property == config->bgcolor_property) {
>+  state->bgcolor = val;
>+  state->bgcolor_changed = true;
>   } else if (crtc->funcs->atomic_set_property) {
>   return crtc->funcs->atomic_set_property(crtc, state, property, 
> val);
>   } else {
>@@ -499,6 +502,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>   *val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>   else if (property == config->prop_out_fence_ptr)
>   *val = 0;
>+  else if (property == config->bgcolor_property)
>+  *val = state->bgcolor;

The docs say it's always opaque - so perhaps override alpha to all
'1's here, or reject values which aren't opaque?

>   else if (crtc->funcs->atomic_get_property)
>   return crtc->funcs->atomic_get_property(crtc, state, property, 
> val);
>   else
>diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
>index 0c78ca386cbe..7da28c0cb74d 100644
>--- a/drivers/gpu/drm/drm_blend.c
>+++ b/drivers/gpu/drm/drm_blend.c
>@@ -175,9 +175,16 @@
>  * plane does not expose the "alpha" property, then this is
>  * assumed to be 1.0
>  *
>- * Note that all the property extensions described here apply either to the
>- * plane or the CRTC (e.g. for the background color, which currently is not
>- * exposed and assumed to be black).
>+ * The property extensions described above all apply to the plane.  Drivers
>+ * may also expose the following crtc property extension:
>+ *
>+ * bgcolor:

That should be "BACKGROUND_COLOR" shouldn't it?

>+ *Background color is setup with drm_crtc_add_bgcolor_property().  It
>+ *controls the RGB color o

[Bug 106175] amdgpu.dc=1 shows performance issues with Xorg compositors when moving windows

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=106175

--- Comment #48 from tempel.jul...@gmail.com ---
With amdgpu.dc=0, TearFree works as expected for me (no tearing without
compositor, scrolling in Firefox windowed is free of stutter, no issues with
compositor vsync either).

I think we should leave TearFree out of this as it's entirely unrelated, apart
from the fact that it forces pageflipping.

Regarding the original issue with amdgpu.dc=1:
Still totally unchanged for me with latest stable versions and also 4.21-wip,
llvm-svn, mesa-git, libdrm-git, xorg-server 1.20.3 and modesetting /
xf86-video-amdgpu-git on Arch.

I'm getting an Asus Vega 56 Strix card tomorrow, which I will try instead of my
current MSI Aero RX 560 card. But since there were already reports for Vega,
I'm not hopeful.

-- 
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 108780] Missing "hainan_mc.bin" firmware reference.

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108780

--- Comment #1 from Alex Deucher  ---
Fixed here:
https://patchwork.freedesktop.org/patch/262616/

-- 
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 201727] Hardware Error reported on Ryzen 5 2500U

2018-11-19 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=201727

Alex Deucher (alexdeuc...@gmail.com) changed:

   What|Removed |Added

 CC||alexdeuc...@gmail.com

--- Comment #10 from Alex Deucher (alexdeuc...@gmail.com) ---
(In reply to Christian König from comment #6)
> 
> What is your BIOS setting for the stolen VRAM?

[2.665594] [drm] amdgpu: 256M of VRAM memory ready

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


[Bug 106175] amdgpu.dc=1 shows performance issues with Xorg compositors when moving windows

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=106175

--- Comment #47 from Michel Dänzer  ---
FWIW, note that TearFree can be toggled at runtime using the RandR output
property of the same name. At its default value "auto", TearFree is
automatically enabled for an output using rotation / scaling / other
transformations.

(In reply to bmilreu from comment #41)
> BTW, TearFree is slow and stuttery even with old dc for me.

Sounds like the issue you're seeing with TearFree might be different from the
one this report is about.

-- 
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 106175] amdgpu.dc=1 shows performance issues with Xorg compositors when moving windows

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=106175

--- Comment #46 from Brandon Wright  ---
I've never run TearFree, so that's not the case here. My Xorg config is similar
to yours, just amdgpu and DRI 3. I did have an extra section to use evdev
instead of libinput, but I tried removing that and there's still no change.

-- 
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 v4 0/8] drm/omap: Add virtual-planes support

2018-11-19 Thread Benoit Parrot
Ping.

Benoit

Benoit Parrot  wrote on Fri [2018-Oct-12 15:16:55 -0500]:
> This patch series adds virtual-plane support to omapdrm driver to allow the 
> use
> of display wider than 2048 pixels.
> 
> In order to do so we introduce the concept of hw_overlay which can then be
> dynamically allocated to a plane. When the requested output width exceed what
> be supported by one overlay a second is then allocated if possible to handle
> display wider then 2048.
> 
> This series replaces an earlier series which was DT based and using statically
> allocated resources. 
> 
> This implementation is inspired from the work done in msm/disp/mdp5
> driver.
> 
> Benoit Parrot (8):
>   drm/omap: Add ability to check if requested plane modes can be
> supported
>   drm/omap: Add ovl checking funcs to dispc_ops
>   drm/omap: introduce omap_hw_overlay
>   drm/omap: omap_plane: subclass drm_plane_state
>   drm/omap: Add global state as a private atomic object
>   drm/omap: dynamically assign hw overlays to planes
>   drm/omap: add plane_atomic_print_state support
>   drm/omap: Add a 'right overlay' to plane state
> 
>  drivers/gpu/drm/omapdrm/Makefile   |   1 +
>  drivers/gpu/drm/omapdrm/dss/dispc.c|  32 +++
>  drivers/gpu/drm/omapdrm/dss/omapdss.h  |   6 +
>  drivers/gpu/drm/omapdrm/omap_drv.c | 196 -
>  drivers/gpu/drm/omapdrm/omap_drv.h |  30 +++
>  drivers/gpu/drm/omapdrm/omap_fb.c  |  33 ++-
>  drivers/gpu/drm/omapdrm/omap_fb.h  |   4 +-
>  drivers/gpu/drm/omapdrm/omap_overlay.c | 257 ++
>  drivers/gpu/drm/omapdrm/omap_overlay.h |  41 
>  drivers/gpu/drm/omapdrm/omap_plane.c   | 374 
> -
>  drivers/gpu/drm/omapdrm/omap_plane.h   |   1 +
>  11 files changed, 922 insertions(+), 53 deletions(-)
>  create mode 100644 drivers/gpu/drm/omapdrm/omap_overlay.c
>  create mode 100644 drivers/gpu/drm/omapdrm/omap_overlay.h
> 
> -- 
> 2.9.0
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 108794] Inconsistent behavior with Separable Shaders (ARB_separate_shader_objects)

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108794

Bug ID: 108794
   Summary: Inconsistent behavior with Separable Shaders
(ARB_separate_shader_objects)
   Product: Mesa
   Version: unspecified
  Hardware: x86-64 (AMD64)
OS: Linux (All)
Status: NEW
  Severity: normal
  Priority: medium
 Component: Drivers/Gallium/radeonsi
  Assignee: dri-devel@lists.freedesktop.org
  Reporter: epigr...@yahoo.com
QA Contact: dri-devel@lists.freedesktop.org

When running Cemu, (an OpenGL application under wine), it appears there is
inconsistent behavior with their feature "Separable Shaders".

a) When BotW runs, and SS is off, the "tent" of some Stables may be invariably
culled off the scene (along with a few other textures).

b) In contrast, when SS is on, it causes the lighting in some Shrines to be
oversaturated.

Hardware: AMD R9 290, Intel Haswell
Software: Kernel 4.18.19, latest libdrm, current mesa repo source, llvm 8 from
their site's repo for debian sid.

tl;dr: In any case, there appears to be inconsistent behavior on results of
separable shaders being used or not when the same methods seem to work on
Windows. I do not have access to the source of the test software to investigate
further and I have not debugged mesa calls further (and I probably won't).

-- 
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 108781] 4.19 Regression - Hawaii (R9 390) boot failure - Invalid PCC GPIO / invalid powerlevel state / Fatal error during GPU init

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108781

--- Comment #4 from Alex Deucher  ---
Possibly the same issue as bug 108704.  Does the patch there help?

-- 
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 1/2] drm/amd/dm: Don't forget to attach MST encoders

2018-11-19 Thread Zuo, Jerry
Reviewed-by: Jerry (Fangzhi) Zuo 

The change fixed MST + SST daisy chain and S3 scenarios. The issue shows huge 
delay in MST + SST daisy chain, and soft hang in S3 resume.

The aux sequence is changed by failed iteration search in 
drm_connector_for_each_possible_encoder(). 
The failure of searching for the best encoder for the connector due to the miss 
of attached encoder in the process of adding MST connector. The iteration 
search takes time to push drm_dp_send_enum_path_resources() aux transaction 
after the mode probe, and causes conflict to drm_dp_mst_i2c_xfer(), leading to 
the aux transaction timeout.

-Original Message-
From: Lyude Paul  
Sent: November 16, 2018 6:25 PM
To: amd-...@lists.freedesktop.org
Cc: Zuo, Jerry ; Wentland, Harry ; 
Li, Sun peng (Leo) ; Deucher, Alexander 
; Koenig, Christian ; 
Zhou, David(ChunMing) ; David Airlie ; 
Li, Roman ; S, Shirish ; Daniel Vetter 
; dri-devel@lists.freedesktop.org; 
linux-ker...@vger.kernel.org
Subject: [PATCH 1/2] drm/amd/dm: Don't forget to attach MST encoders

Drive-by fix, this is bound to cause problems somewhere.

Signed-off-by: Lyude Paul 
Cc: Jerry Zuo 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 ++
 1 file changed, 2 insertions(+)

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 d02c32a1039c..0cca1809fdcd 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
@@ -342,6 +342,8 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
master->connector_id);
 
aconnector->mst_encoder = dm_dp_create_fake_mst_encoder(master);
+   drm_connector_attach_encoder(&aconnector->base,
+&aconnector->mst_encoder->base);
 
/*
 * TODO: understand why this one is needed
-- 
2.19.1

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


[Bug 108778] [R9 390] amdgpu: Fatal error during GPU init 4.20-rc2

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108778

--- Comment #3 from Alex Deucher  ---
Possibly the same issue as bug 108704.  Does the patch there help?

-- 
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] drm/sun4i: wait on implicit fence before display

2018-11-19 Thread Qiang Yu
Render like lima will attach a fence to the framebuffer
dma_buf, display like sun4i should wait it finish before
show the framebuffer. Otherwise tearing will be observed.

Signed-off-by: Qiang Yu 
---
 drivers/gpu/drm/sun4i/sun4i_layer.c| 2 ++
 drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 2 ++
 drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 2 ++
 3 files changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c 
b/drivers/gpu/drm/sun4i/sun4i_layer.c
index 750ad24de1d7..d68e663df9a0 100644
--- a/drivers/gpu/drm/sun4i/sun4i_layer.c
+++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
@@ -12,6 +12,7 @@
 
 #include 
 #include 
+#include 
 #include 
 
 #include "sun4i_backend.h"
@@ -114,6 +115,7 @@ static void sun4i_backend_layer_atomic_update(struct 
drm_plane *plane,
 }
 
 static const struct drm_plane_helper_funcs sun4i_backend_layer_helper_funcs = {
+   .prepare_fb = drm_gem_fb_prepare_fb,
.atomic_disable = sun4i_backend_layer_atomic_disable,
.atomic_update  = sun4i_backend_layer_atomic_update,
 };
diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c 
b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
index 28c15c6ef1ef..7bc2ca2bd0c3 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -287,6 +288,7 @@ static void sun8i_ui_layer_atomic_update(struct drm_plane 
*plane,
 }
 
 static struct drm_plane_helper_funcs sun8i_ui_layer_helper_funcs = {
+   .prepare_fb = drm_gem_fb_prepare_fb,
.atomic_check   = sun8i_ui_layer_atomic_check,
.atomic_disable = sun8i_ui_layer_atomic_disable,
.atomic_update  = sun8i_ui_layer_atomic_update,
diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c 
b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
index f4fe97813f94..815895795afd 100644
--- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -315,6 +316,7 @@ static void sun8i_vi_layer_atomic_update(struct drm_plane 
*plane,
 }
 
 static struct drm_plane_helper_funcs sun8i_vi_layer_helper_funcs = {
+   .prepare_fb = drm_gem_fb_prepare_fb,
.atomic_check   = sun8i_vi_layer_atomic_check,
.atomic_disable = sun8i_vi_layer_atomic_disable,
.atomic_update  = sun8i_vi_layer_atomic_update,
-- 
2.17.1

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


Re: [PATCH] drm/rockchip: fix for mailbox read size

2018-11-19 Thread Heiko Stuebner
Am Dienstag, 6. November 2018, 16:37:05 CET schrieb Damian Kos:
> Some of the functions (like cdn_dp_dpcd_read, cdn_dp_get_edid_block)
> allow to read 64KiB, but the cdn_dp_mailbox_read_receive, that is
> used by them, can read only up to 255 bytes at once. Normally, it's
> not a big issue as DPCD or EDID reads won't (hopefully) exceed that
> value.
> The real issue here is the revocation list read during the HDCP
> authentication process. (problematic use case:
> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.4/drivers/gpu/drm/rockchip/cdn-dp-reg.c#1152)
> The list can reach 127*5+4 bytes (num devs * 5 bytes per ID/Bksv +
> 4 bytes of an additional info).
> In other words - CTSes with HDCP Repeater won't pass without this
> fix. Oh, and the driver will most likely stop working (best case
> scenario).
> 
> Signed-off-by: Damian Kos 

applied to drm-misc-next

Thanks
Heiko


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


[Bug 105113] [hawaii, radeonsi, clover] Running Piglit cl/program/execute/{, tail-}calls{, -struct, -workitem-id}.cl cause GPU VM error and ring stalled GPU lockup

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=105113

--- Comment #8 from Maciej S. Szmigiero  ---
(In reply to Jan Vesely from comment #7)
> (In reply to Maciej S. Szmigiero from comment #6)
> > There are really two issues at play here:
> > 1) If the LLVM-generated code cannot be run properly then it should be 
> > simply
> > rejected by whatever is actually in charge of submitting it to the GPU (I
> > guess
> > this would be Mesa?).
> > This way an application will know it cannot use OpenCL for computation, at
> > least
> > not with this compute kernel.
> > 
> > Instead, it currently looks like many of these test run but give incorrect
> > results, which is obviously rather bad.
> 
> Do you have an example of this? clover should return OUT_OF_RESOURCES error
> when the compute state creation fails (like in the presence of code
> relocations).
> It does not change the content of the buffer, so it will return whatever was
> stored in the buffer on creation.

Aren't program@execute@calls-struct and program@execute@tail-calls tests
from comment 4 examples of this behavior?
These seem to run but return wrong results, or am I not parsing the piglit
test results correctly?

> > 2) Some (previous) Mesa + LLVM versions generate a command stream that
> > crashes the GPU and, as far as I can remember, sometimes even lockup the
> > whole machine.
> > 
> > It should not be possible to crash the GPU, regardless how incorrect a
> > command stream that userspace sends to it is - because otherwise it is
> > possible for
> > an unprivileged user with GPU access to DoS the machine.
> 
> This is a separate issue. GPU hangs are generally addressed via gpu reset
> which should be enabled for gfx8/9 GPUs in recent amdgpu.ko [0]
> 
> [0] https://patchwork.freedesktop.org/patch/257994/

This would explain why "amdgpu" seemed to not even attempt to reset the GPU
after a crash.

However, I think I've got at least one lockup when testing this issue half a
year ago on "radeon" driver ("amdgpu" is still marked as experimental for SI
parts).
If I am able to reproduce it in the future I will report it then.

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


Re: [PATCH v2 3/9] phy: Add MIPI D-PHY configuration options

2018-11-19 Thread Sakari Ailus
Hi Maxime,

On Tue, Nov 06, 2018 at 03:54:15PM +0100, Maxime Ripard wrote:
> Now that we have some infrastructure for it, allow the MIPI D-PHY phy's to
> be configured through the generic functions through a custom structure
> added to the generic union.
> 
> The parameters added here are the ones defined in the MIPI D-PHY spec, plus
> the number of lanes in use. The current set of parameters should cover all
> the potential users.
> 
> Signed-off-by: Maxime Ripard 
> ---
>  include/linux/phy/phy-mipi-dphy.h | 232 +++-
>  include/linux/phy/phy.h   |   6 +-
>  2 files changed, 238 insertions(+)
>  create mode 100644 include/linux/phy/phy-mipi-dphy.h
> 
> diff --git a/include/linux/phy/phy-mipi-dphy.h 
> b/include/linux/phy/phy-mipi-dphy.h
> new file mode 100644
> index ..0b05932916af
> --- /dev/null
> +++ b/include/linux/phy/phy-mipi-dphy.h
> @@ -0,0 +1,232 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2018 Cadence Design Systems Inc.
> + */
> +
> +#ifndef __PHY_MIPI_DPHY_H_
> +#define __PHY_MIPI_DPHY_H_
> +
> +#include 
> +
> +/**
> + * struct phy_configure_opts_mipi_dphy - MIPI D-PHY configuration set
> + *
> + * This structure is used to represent the configuration state of a
> + * MIPI D-PHY phy.
> + */
> +struct phy_configure_opts_mipi_dphy {
> + /**
> +  * @clk_miss:
> +  *
> +  * Timeout, in nanoseconds, for receiver to detect absence of
> +  * Clock transitions and disable the Clock Lane HS-RX.
> +  */
> + unsigned intclk_miss;
> +
> + /**
> +  * @clk_post:
> +  *
> +  * Time, in nanoseconds, that the transmitter continues to
> +  * send HS clock after the last associated Data Lane has
> +  * transitioned to LP Mode. Interval is defined as the period
> +  * from the end of @hs_trail to the beginning of @clk_trail.
> +  */
> + unsigned intclk_post;
> +
> + /**
> +  * @clk_pre:
> +  *
> +  * Time, in nanoseconds, that the HS clock shall be driven by
> +  * the transmitter prior to any associated Data Lane beginning
> +  * the transition from LP to HS mode.
> +  */
> + unsigned intclk_pre;

Is the unit of clk_pre intended to be UI or ns?

How about adding information on the limits of these values as well?

> +
> + /**
> +  * @clk_prepare:
> +  *
> +  * Time, in nanoseconds, that the transmitter drives the Clock
> +  * Lane LP-00 Line state immediately before the HS-0 Line
> +  * state starting the HS transmission.
> +  */
> + unsigned intclk_prepare;
> +
> + /**
> +  * @clk_settle:
> +  *
> +  * Time interval, in nanoseconds, during which the HS receiver
> +  * should ignore any Clock Lane HS transitions, starting from
> +  * the beginning of @clk_prepare.
> +  */
> + unsigned intclk_settle;
> +
> + /**
> +  * @clk_term_en:
> +  *
> +  * Time, in nanoseconds, for the Clock Lane receiver to enable
> +  * the HS line termination.
> +  */
> + unsigned intclk_term_en;
> +
> + /**
> +  * @clk_trail:
> +  *
> +  * Time, in nanoseconds, that the transmitter drives the HS-0
> +  * state after the last payload clock bit of a HS transmission
> +  * burst.
> +  */
> + unsigned intclk_trail;
> +
> + /**
> +  * @clk_zero:
> +  *
> +  * Time, in nanoseconds, that the transmitter drives the HS-0
> +  * state prior to starting the Clock.
> +  */
> + unsigned intclk_zero;
> +
> + /**
> +  * @d_term_en:
> +  *
> +  * Time, in nanoseconds, for the Data Lane receiver to enable
> +  * the HS line termination.
> +  */
> + unsigned intd_term_en;
> +
> + /**
> +  * @eot:
> +  *
> +  * Transmitted time interval, in nanoseconds, from the start
> +  * of @hs_trail or @clk_trail, to the start of the LP- 11
> +  * state following a HS burst.
> +  */
> + unsigned inteot;
> +
> + /**
> +  * @hs_exit:
> +  *
> +  * Time, in nanoseconds, that the transmitter drives LP-11
> +  * following a HS burst.
> +  */
> + unsigned inths_exit;
> +
> + /**
> +  * @hs_prepare:
> +  *
> +  * Time, in nanoseconds, that the transmitter drives the Data
> +  * Lane LP-00 Line state immediately before the HS-0 Line
> +  * state starting the HS transmission
> +  */
> + unsigned inths_prepare;
> +
> + /**
> +  * @hs_settle:
> +  *
> +  * Time interval, in nanoseconds, during which the HS receiver
> +  * shall ignore any Data Lane HS transitions, starting from
> +  * the beginning of @hs_prepare.
> +  */
> + unsigned inths_settle;
> +
> + /**
> +  * @hs_skip:
> +  *
> +  * Time interval, in nanoseconds, during which the HS-R

[Bug 108760] AMD GPU pro 18.40 doesn't work with Kabylake-g

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108760

--- Comment #4 from Alex Deucher  ---
upstream is the open stack.

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


Re: [PATCH v2 4/9] phy: dphy: Add configuration helpers

2018-11-19 Thread Sakari Ailus
Hi Maxime,

Apologies for the delayed review. Please see my comments below.

On Tue, Nov 06, 2018 at 03:54:16PM +0100, Maxime Ripard wrote:
> The MIPI D-PHY spec defines default values and boundaries for most of the
> parameters it defines. Introduce helpers to help drivers get meaningful
> values based on their current parameters, and validate the boundaries of
> these parameters if needed.
> 
> Signed-off-by: Maxime Ripard 
> ---
>  drivers/phy/Kconfig   |   8 ++-
>  drivers/phy/Makefile  |   1 +-
>  drivers/phy/phy-core-mipi-dphy.c  | 160 +++-
>  include/linux/phy/phy-mipi-dphy.h |   6 +-
>  4 files changed, 175 insertions(+)
>  create mode 100644 drivers/phy/phy-core-mipi-dphy.c
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 60f949e2a684..c87a7d49eaab 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -15,6 +15,14 @@ config GENERIC_PHY
> phy users can obtain reference to the PHY. All the users of this
> framework should select this config.
>  
> +config GENERIC_PHY_MIPI_DPHY
> + bool
> + help
> +   Generic MIPI D-PHY support.
> +
> +   Provides a number of helpers a core functions for MIPI D-PHY
> +   drivers to us.
> +
>  config PHY_LPC18XX_USB_OTG
>   tristate "NXP LPC18xx/43xx SoC USB OTG PHY driver"
>   depends on OF && (ARCH_LPC18XX || COMPILE_TEST)
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 0301e25d07c1..baec59cebbab 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -4,6 +4,7 @@
>  #
>  
>  obj-$(CONFIG_GENERIC_PHY)+= phy-core.o
> +obj-$(CONFIG_GENERIC_PHY_MIPI_DPHY)  += phy-core-mipi-dphy.o
>  obj-$(CONFIG_PHY_LPC18XX_USB_OTG)+= phy-lpc18xx-usb-otg.o
>  obj-$(CONFIG_PHY_XGENE)  += phy-xgene.o
>  obj-$(CONFIG_PHY_PISTACHIO_USB)  += phy-pistachio-usb.o
> diff --git a/drivers/phy/phy-core-mipi-dphy.c 
> b/drivers/phy/phy-core-mipi-dphy.c
> new file mode 100644
> index ..127ca6960084
> --- /dev/null
> +++ b/drivers/phy/phy-core-mipi-dphy.c
> @@ -0,0 +1,160 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2013 NVIDIA Corporation
> + * Copyright (C) 2018 Cadence Design Systems Inc.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +
> +/*
> + * Minimum D-PHY timings based on MIPI D-PHY specification. Derived
> + * from the valid ranges specified in Section 6.9, Table 14, Page 41
> + * of the D-PHY specification (v2.1).

I assume these values are compliant with the earlier spec releases.

> + */
> +int phy_mipi_dphy_get_default_config(unsigned long pixel_clock,

How about using the bus frequency instead of the pixel clock? Chances are
that the caller already has that information, instead of calculating it
here?

> +  unsigned int bpp,
> +  unsigned int lanes,
> +  struct phy_configure_opts_mipi_dphy *cfg)
> +{
> + unsigned long hs_clk_rate;
> + unsigned long ui;
> +
> + if (!cfg)
> + return -EINVAL;
> +
> + hs_clk_rate = pixel_clock * bpp / lanes;
> + ui = DIV_ROUND_UP(NSEC_PER_SEC, hs_clk_rate);

Nanoseconds may not be precise enough for practical computations on these
values. At 1 GHz, this ends up being precisely 1. At least Intel hardware
has some more precision, I presume others do, too. How about using
picoseconds instead?

> +
> + cfg->clk_miss = 0;
> + cfg->clk_post = 60 + 52 * ui;
> + cfg->clk_pre = 8;
> + cfg->clk_prepare = 38;
> + cfg->clk_settle = 95;
> + cfg->clk_term_en = 0;
> + cfg->clk_trail = 60;
> + cfg->clk_zero = 262;
> + cfg->d_term_en = 0;
> + cfg->eot = 0;
> + cfg->hs_exit = 100;
> + cfg->hs_prepare = 40 + 4 * ui;
> + cfg->hs_zero = 105 + 6 * ui;
> + cfg->hs_settle = 85 + 6 * ui;
> + cfg->hs_skip = 40;
> +
> + /*
> +  * The MIPI D-PHY specification (Section 6.9, v1.2, Table 14, Page 40)
> +  * contains this formula as:
> +  *
> +  * T_HS-TRAIL = max(n * 8 * ui, 60 + n * 4 * ui)
> +  *
> +  * where n = 1 for forward-direction HS mode and n = 4 for reverse-
> +  * direction HS mode. There's only one setting and this function does
> +  * not parameterize on anything other that ui, so this code will
> +  * assumes that reverse-direction HS mode is supported and uses n = 4.
> +  */
> + cfg->hs_trail = max(4 * 8 * ui, 60 + 4 * 4 * ui);
> +
> + cfg->init = 10;
> + cfg->lpx = 60;
> + cfg->ta_get = 5 * cfg->lpx;
> + cfg->ta_go = 4 * cfg->lpx;
> + cfg->ta_sure = 2 * cfg->lpx;
> + cfg->wakeup = 100;
> +
> + cfg->hs_clk_rate = hs_clk_rate;

How about the LP clock?

Frankly, I have worked with MIPI CSI-2 hardware soon a decade, and the very
few cases where software has needed to deal with these values has been in
form of d

[Bug 108625] AMDGPU - Can't even get Xorg to start - Kernel driver hangs with ring buffer timeout on ARM64

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108625

--- Comment #16 from Christian König  ---
(In reply to Carsten Haitzler from comment #15)
> Makes it work. Of course this isn't a brilliant patch, but indeed there is
> something up with the way write combined memory is handled on ARM here.

Well disabling WC is also a good way of reducing the performance in general.

E.g. what could be is that because you disabled WC the performance is reduced
and because of that the timing is changed

-- 
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: Fix 0-day build error

2018-11-19 Thread Boris Brezillon
On Mon, 19 Nov 2018 13:09:16 +
Fabrizio Castro  wrote:

> Hi Boris,
> 
> > From: Boris Brezillon 
> > Sent: 19 November 2018 12:51
> > Subject: Re: [PATCH] drm/bridge: Fix 0-day build error
> >
> > Hi Fabrizio,
> >
> > The prefix should be "drm/bridge/sii902x:" and I'd prefer a short
> > explanation of what is problematic in the subject rather than "Fix
> > 0-day build error". Maybe something like
> >
> > "drm/bridge/sii902x: Add missing dependency on I2C_MUX"
> >
> > On Mon, 19 Nov 2018 12:44:23 +
> > Fabrizio Castro  wrote:
> >  
> > > kbuild test robot reports:
> > >  
> > > >> ERROR: "i2c_mux_add_adapter" [drivers/gpu/drm/bridge/sii902x.ko]  
> > > undefined!  
> > > >> ERROR: "i2c_mux_alloc" [drivers/gpu/drm/bridge/sii902x.ko]  
> > > undefined!  
> > > >> ERROR: "i2c_mux_del_adapters" [drivers/gpu/drm/bridge/sii902x.ko]  
> > > undefined!
> > >
> > > Quite obviously the driver depends on I2C_MUX, but adding a "depends on"
> > > introduces a recursive dependency, therefore this patch selects I2C_MUX
> > > instead.
> > >  
> >
> > You need a fixes tag here:
> >
> > Fixes: 21d808405fe4 ("drm/bridge/sii902x: Fix EDID readback")  
> 
> Thank you for spotting this, I'll send a v2 right away.

Looks like you didn't change the subject in your v2.

> 
> Cheers,
> Fab
> 
> >
> > Thanks,
> >
> > Boris
> >  
> > > Signed-off-by: Fabrizio Castro 
> > > Link: https://lists.01.org/pipermail/kbuild-all/2018-November/054924.html
> > > ---
> > >  drivers/gpu/drm/bridge/Kconfig | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/gpu/drm/bridge/Kconfig 
> > > b/drivers/gpu/drm/bridge/Kconfig
> > > index 9eeb8ef..2fee47b 100644
> > > --- a/drivers/gpu/drm/bridge/Kconfig
> > > +++ b/drivers/gpu/drm/bridge/Kconfig
> > > @@ -95,6 +95,7 @@ config DRM_SII902X
> > >  depends on OF
> > >  select DRM_KMS_HELPER
> > >  select REGMAP_I2C
> > > +select I2C_MUX
> > >  ---help---
> > >Silicon Image sii902x bridge chip driver.
> > >  
> 
> 
> 
> 
> Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, 
> Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered 
> No. 04586709.

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


[Bug 108625] AMDGPU - Can't even get Xorg to start - Kernel driver hangs with ring buffer timeout on ARM64

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108625

--- Comment #15 from Carsten Haitzler  ---
And lo and behold:

--- ./include/drm/drm_cache.h~  2018-08-12 21:41:04.0 +0100
+++ ./include/drm/drm_cache.h   2018-11-16 11:06:16.976842816 +
@@ -48,7 +48,7 @@
 #elif defined(CONFIG_MIPS) && defined(CONFIG_CPU_LOONGSON3)
return false;
 #else
-   return true;
+   return false;
 #endif
 }

Makes it work. Of course this isn't a brilliant patch, but indeed there is
something up with the way write combined memory is handled on ARM here. but
disabling WC for all ARM DRM devices might be too much of a sledgehammer... I'm
going to look into a less sledge-hammer solution that might make this work more
universally. I'll get back to you on that.

-- 
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 108767] [CI][Runner] Abort on network down

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108767

Lakshmi  changed:

   What|Removed |Added

   Assignee|dri-devel@lists.freedesktop |petri.latv...@intel.com
   |.org|

-- 
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: Fix 0-day build error

2018-11-19 Thread Boris Brezillon
Hi Fabrizio,

The prefix should be "drm/bridge/sii902x:" and I'd prefer a short
explanation of what is problematic in the subject rather than "Fix
0-day build error". Maybe something like

"drm/bridge/sii902x: Add missing dependency on I2C_MUX"

On Mon, 19 Nov 2018 12:44:23 +
Fabrizio Castro  wrote:

> kbuild test robot reports:
> 
> >> ERROR: "i2c_mux_add_adapter" [drivers/gpu/drm/bridge/sii902x.ko]  
> undefined!
> >> ERROR: "i2c_mux_alloc" [drivers/gpu/drm/bridge/sii902x.ko]  
> undefined!
> >> ERROR: "i2c_mux_del_adapters" [drivers/gpu/drm/bridge/sii902x.ko]  
> undefined!
> 
> Quite obviously the driver depends on I2C_MUX, but adding a "depends on"
> introduces a recursive dependency, therefore this patch selects I2C_MUX
> instead.
> 

You need a fixes tag here:

Fixes: 21d808405fe4 ("drm/bridge/sii902x: Fix EDID readback")

Thanks,

Boris

> Signed-off-by: Fabrizio Castro 
> Link: https://lists.01.org/pipermail/kbuild-all/2018-November/054924.html
> ---
>  drivers/gpu/drm/bridge/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index 9eeb8ef..2fee47b 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -95,6 +95,7 @@ config DRM_SII902X
>   depends on OF
>   select DRM_KMS_HELPER
>   select REGMAP_I2C
> + select I2C_MUX
>   ---help---
> Silicon Image sii902x bridge chip driver.
>  

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


[Bug 201727] Hardware Error reported on Ryzen 5 2500U

2018-11-19 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=201727

--- Comment #9 from Michal Herko (misko.he...@gmail.com) ---
> What is your BIOS setting for the stolen VRAM?
I can't find any such settings in bios. I really do not have any options
regarding video.

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


Re: [PATCH 3/9] drm: replace "drm_dev_unref" function with "drm_dev_put"

2018-11-19 Thread Shawn Guo
On Thu, Nov 15, 2018 at 11:16:23PM +0100, Fernando Ramos wrote:
> This patch unifies the naming of DRM functions for reference counting as
> requested on Documentation/gpu/todo.rst
> 
> Signed-off-by: Fernando Ramos 
> ---
>  drivers/gpu/drm/arc/arcpgu_drv.c | 4 ++--
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 4 ++--
>  drivers/gpu/drm/etnaviv/etnaviv_drv.c| 4 ++--
>  drivers/gpu/drm/mxsfb/mxsfb_drv.c| 4 ++--
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c| 2 +-
>  drivers/gpu/drm/shmobile/shmob_drm_drv.c | 4 ++--
>  drivers/gpu/drm/tve200/tve200_drv.c  | 4 ++--
>  drivers/gpu/drm/zte/zx_drm_drv.c | 4 ++--

We have already queued a patch [1] from Thomas Zimmermann for
zx_drm_drv.

Shawn

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


Re: [PATCHv4 5/6] drm/omap: add framedone interrupt support

2018-11-19 Thread Pavel Machek
Hi!

> > > @@ -217,6 +239,9 @@ static irqreturn_t omap_irq_handler(int irq, void 
> > > *arg)
> > >  
> > >   if (irqstatus & 
> > > priv->dispc_ops->mgr_get_sync_lost_irq(priv->dispc, channel))
> > >   omap_crtc_error_irq(crtc, irqstatus);
> > > +
> > > + if (irqstatus & 
> > > priv->dispc_ops->mgr_get_framedone_irq(priv->dispc, channel))
> > > + omap_crtc_framedone_irq(crtc, irqstatus);
> > >   }
> > >  
> > >   omap_irq_ocp_error_handler(dev, irqstatus);
> > 
> > Will the mgr_get_framedone_irq(priv->dispc, channel) change from
> > interrupt to interrupt? Would it make sense to cache result as a
> > micro-organization?
> 
> Maybe. But this is the same for the the omap_crtc_error_* and the
> driver is currently being restructured by Laurent. I think this can
> wait for later.

Definitely can wait. Thanks!
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


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


Re: [PATCH] drm/amdgpu: Add missing firmware entry for HAINAN

2018-11-19 Thread Christian König

Am 19.11.18 um 12:55 schrieb Takashi Iwai:

Due to lack of MODULE_FIRMWARE() with hainan_mc.bin, the driver
doesn't work properly in initrd.  Let's add it.

Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1116239
Fixes: 8eaf2b1faaf4 ("drm/amdgpu: switch firmware path for SI parts")
Cc: 
Signed-off-by: Takashi Iwai 


Reviewed-by: Christian König 


---
  drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
index e1c2b4e9c7b2..73ad02aea2b2 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
@@ -46,6 +46,7 @@ MODULE_FIRMWARE("amdgpu/tahiti_mc.bin");
  MODULE_FIRMWARE("amdgpu/pitcairn_mc.bin");
  MODULE_FIRMWARE("amdgpu/verde_mc.bin");
  MODULE_FIRMWARE("amdgpu/oland_mc.bin");
+MODULE_FIRMWARE("amdgpu/hainan_mc.bin");
  MODULE_FIRMWARE("amdgpu/si58_mc.bin");
  
  #define MC_SEQ_MISC0__MT__MASK   0xf000


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


[PATCH] drm/amdgpu: Add missing firmware entry for HAINAN

2018-11-19 Thread Takashi Iwai
Due to lack of MODULE_FIRMWARE() with hainan_mc.bin, the driver
doesn't work properly in initrd.  Let's add it.

Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1116239
Fixes: 8eaf2b1faaf4 ("drm/amdgpu: switch firmware path for SI parts")
Cc: 
Signed-off-by: Takashi Iwai 
---
 drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
index e1c2b4e9c7b2..73ad02aea2b2 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
@@ -46,6 +46,7 @@ MODULE_FIRMWARE("amdgpu/tahiti_mc.bin");
 MODULE_FIRMWARE("amdgpu/pitcairn_mc.bin");
 MODULE_FIRMWARE("amdgpu/verde_mc.bin");
 MODULE_FIRMWARE("amdgpu/oland_mc.bin");
+MODULE_FIRMWARE("amdgpu/hainan_mc.bin");
 MODULE_FIRMWARE("amdgpu/si58_mc.bin");
 
 #define MC_SEQ_MISC0__MT__MASK   0xf000
-- 
2.19.1

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


[Bug 108760] AMD GPU pro 18.40 doesn't work with Kabylake-g

2018-11-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108760

--- Comment #3 from Chen Xi  ---
Alex, Thanks a lot for your quick response! 

Hai is taking leave. I will follow up the issue when he takes leave.

Could you please be more specific about the "upstream driver" mentioned in your
comment? so we could have a try on our side.

-- 
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 201727] Hardware Error reported on Ryzen 5 2500U

2018-11-19 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=201727

--- Comment #8 from Michal Herko (misko.he...@gmail.com) ---
Created attachment 279521
  --> https://bugzilla.kernel.org/attachment.cgi?id=279521&action=edit
revert of 284dec

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


[Bug 201727] Hardware Error reported on Ryzen 5 2500U

2018-11-19 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=201727

--- Comment #7 from Michal Herko (misko.he...@gmail.com) ---
Everything works after a revert. There was a conflict, i am attaching a diff of
the revert.

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


[Bug 201727] Hardware Error reported on Ryzen 5 2500U

2018-11-19 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=201727

--- Comment #6 from Christian König (christian.koe...@amd.com) ---
Currently GTT is only used for PD/PT as last resort when there is so few stolen
memory assigned to the APU that it won't work at all otherwise.

The faulting address looks suspicious like we miss to handle an error code
correctly somewhere and instead use the value as DMA address.

What is your BIOS setting for the stolen VRAM?

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


  1   2   >