Re: [Intel-gfx] [PATCH 10/10] drm/vmwgfx: Use drm_plane_mask() & co.

[Sinclair Yeh]

Reviewed-by: Sinclair Yeh 

I assume you'll upstream this as part of your series?

On Tue, Jun 26, 2018 at 10:47:16PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> Use drm_{plane,connector}_mask() where appropriate.
> 
> Cc: VMware Graphics 
> Cc: Sinclair Yeh 
> Cc: Thomas Hellstrom 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index ef96ba7432ad..17e01423ead1 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -535,9 +535,9 @@ int vmw_du_crtc_atomic_check(struct drm_crtc *crtc,
>struct drm_crtc_state *new_state)
>  {
>   struct vmw_display_unit *du = vmw_crtc_to_du(new_state->crtc);
> - int connector_mask = 1 << drm_connector_index(>connector);
> + int connector_mask = drm_connector_mask(>connector);
>   bool has_primary = new_state->plane_mask &
> -BIT(drm_plane_index(crtc->primary));
> +drm_plane_mask(crtc->primary);
>  
>   /* We always want to have an active plane with an active CRTC */
>   if (has_primary != new_state->enable)
> -- 
> 2.16.4
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 02/13] drm/vmwgfx: Stop using plane->fb in vmw_kms_helper_dirty()

[Sinclair Yeh]

On Wed, May 30, 2018 at 11:08:57PM +0300, Ville Syrjälä wrote:
> On Fri, May 25, 2018 at 09:50:34PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä 
> > 
> > Instead of plane->fb (which we're going to deprecate for atomic drivers)
> > we need to look at plane->state->fb. The maze of code leading to
> > vmw_kms_helper_dirty() wasn't particularly clear, but my analysis
> > concluded that the calls originating from 
> > vmw_*_primary_plane_atomic_update()
> > all pass in the crtc which means we'll never end up in this branch
> > of the function. All other callers use drm_modeset_lock_all() somewhere
> > higher up, which means accessing plane->state is safe. We'll toss in
> > a lockdep assert to catch wrongdoers.
> > 
> > v2: Drop the comment and make the code do what it did before (Thomas)
> > 
> > Cc: Deepak Rawat 
> > Cc: Thomas Hellstrom 
> > Cc: Sinclair Yeh 
> > Cc: VMware Graphics 
> > Cc: Daniel Vetter 
> > Signed-off-by: Ville Syrjälä 
> > ---
> >  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 9 ++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c 
> > b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > index 2e4c38bb846d..5417eb1b486e 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > @@ -2326,9 +2326,12 @@ int vmw_kms_helper_dirty(struct vmw_private 
> > *dev_priv,
> > } else {
> > list_for_each_entry(crtc, _priv->dev->mode_config.crtc_list,
> > head) {
> > -   if (crtc->primary->fb != >base)
> > -   continue;
> > -   units[num_units++] = vmw_crtc_to_du(crtc);
> > +   struct drm_plane *plane = crtc->primary;
> > +
> > +   lockdep_assert_held(>mutex);
> 
> kbuild test robot told me
> >> include/linux/lockdep.h:347:52: error: 'struct drm_modeset_lock' has
> >> no member named 'dep_map'  
> #define lockdep_is_held(lock)  lock_is_held(&(lock)->dep_map) 
>  
> 
> Maybe I'll just drop the asserts? Or do people really want them
> (in which case I gues I need to dig out the underlying mutex)?

Eitherway is fine with me.


> 
> > +
> > +   if (plane->state->fb == >base)
> > +   units[num_units++] = vmw_crtc_to_du(crtc);
> > }
> > }
> >  
> > -- 
> > 2.16.1
> 
> -- 
> Ville Syrjälä
> Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 00/13] drm: Eliminate plane->fb/crtc usage for atomic drivers

[Sinclair Yeh]

Thanks Ville.

This series: Reviewed-by: Sinclair Yeh 

On Fri, May 25, 2018 at 09:50:32PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> Here are again the last (?) bits of eliminating the plane->fb/crtc
> usage for atomic drivers. I've pushed everything else (thanks to
> everyone who reviewed them). 
> 
> Deepak said he'd tested the vmwgfx stuff, so I think it should be
> safe to land. Just missing a bit of review...
> 
> Cc: Alex Deucher 
> Cc: amd-...@lists.freedesktop.org
> Cc: "Christian König" 
> Cc: Daniel Vetter 
> Cc: David Airlie 
> Cc: "David (ChunMing) Zhou" 
> Cc: Deepak Rawat 
> Cc: Eric Anholt 
> Cc: freedr...@lists.freedesktop.org
> Cc: Gerd Hoffmann 
> Cc: Harry Wentland 
> Cc: Inki Dae 
> Cc: Joonyoung Shim 
> Cc: Kyungmin Park 
> Cc: linux-arm-...@vger.kernel.org
> Cc: Rob Clark 
> Cc: Seung-Woo Kim 
> Cc: Sinclair Yeh 
> Cc: Thomas Hellstrom 
> Cc: virtualizat...@lists.linux-foundation.org
> Cc: VMware Graphics 
> 
> Ville Syrjälä (13):
>   drm/vmwgfx: Stop using plane->fb in vmw_kms_atomic_check_modeset()
>   drm/vmwgfx: Stop using plane->fb in vmw_kms_helper_dirty()
>   drm/vmwgfx: Stop using plane->fb in vmw_kms_update_implicit_fb()
>   drm/vmwgfx: Stop updating plane->fb
>   drm/vmwgfx: Stop using plane->fb in atomic_enable()
>   drm/vmwgfx: Stop messing about with plane->fb/old_fb/crtc
>   drm/amdgpu/dc: Stop updating plane->fb
>   drm/i915: Stop updating plane->fb/crtc
>   drm/exynos: Stop updating plane->crtc
>   drm/msm: Stop updating plane->fb/crtc
>   drm/virtio: Stop updating plane->crtc
>   drm/vc4: Stop updating plane->fb/crtc
>   drm: Stop updating plane->crtc/fb/old_fb on atomic drivers
> 
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  2 -
>  drivers/gpu/drm/drm_atomic.c  | 55 
> +++
>  drivers/gpu/drm/drm_atomic_helper.c   | 15 +--
>  drivers/gpu/drm/drm_crtc.c|  8 +++-
>  drivers/gpu/drm/drm_fb_helper.c   |  7 ---
>  drivers/gpu/drm/drm_framebuffer.c |  5 ---
>  drivers/gpu/drm/drm_plane.c   | 14 +++---
>  drivers/gpu/drm/drm_plane_helper.c|  4 +-
>  drivers/gpu/drm/exynos/exynos_drm_plane.c |  2 -
>  drivers/gpu/drm/i915/intel_atomic_plane.c | 12 -
>  drivers/gpu/drm/i915/intel_display.c  |  7 ++-
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c |  1 -
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c|  2 -
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c |  1 -
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c|  2 -
>  drivers/gpu/drm/vc4/vc4_crtc.c|  3 --
>  drivers/gpu/drm/virtio/virtgpu_display.c  |  2 -
>  drivers/gpu/drm/vmwgfx/vmwgfx_fb.c| 24 --
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c   | 24 +++---
>  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c  |  2 -
>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c  |  5 +--
>  include/drm/drm_atomic.h  |  3 --
>  22 files changed, 46 insertions(+), 154 deletions(-)
> 
> -- 
> 2.16.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 13/15] drm/vmwgfx: Use drm_mode_get_hv_timing() to populate plane clip rectangle

[Sinclair Yeh]

This looks okay to me.

Reviewed-by: Sinclair Yeh <s...@vmware.com>

On Thu, Nov 23, 2017 at 09:05:00PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrj...@linux.intel.com>
> 
> Use drm_mode_get_hv_timing() to fill out the plane clip rectangle.
> 
> Note that this replaces crtc_state->adjusted_mode usage with
> crtc_state->mode. The latter is the correct choice since that's the
> mode the user provided and it matches the plane crtc coordinates
> the user also provided.
> 
> Once everyone agrees on this we can move the clip handling into
> drm_atomic_helper_check_plane_state().
> 
> Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> Cc: VMware Graphics <linux-graphics-maintai...@vmware.com>
> Cc: Sinclair Yeh <s...@vmware.com>
> Cc: Thomas Hellstrom <thellst...@vmware.com>
> Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index a2a93d7e2a04..25d96560180b 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -449,10 +449,9 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane 
> *plane,
>   if (state->crtc)
>   crtc_state = drm_atomic_get_new_crtc_state(state->state, 
> state->crtc);
>  
> - if (crtc_state && crtc_state->enable) {
> - clip.x2 = crtc_state->adjusted_mode.hdisplay;
> - clip.y2 = crtc_state->adjusted_mode.vdisplay;
> - }
> + if (crtc_state && crtc_state->enable)
> + drm_mode_get_hv_timing(_state->mode,
> +, );
>  
>   ret = drm_atomic_helper_check_plane_state(state, crtc_state, ,
> DRM_PLANE_HELPER_NO_SCALING,
> -- 
> 2.13.6
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff

[Sinclair Yeh]

On Mon, Nov 20, 2017 at 08:34:50AM +0100, Daniel Vetter wrote:
> On Fri, Nov 10, 2017 at 11:42:59PM +0200, Ville Syrjälä wrote:
> > On Fri, Nov 10, 2017 at 01:26:47PM -0800, Sinclair Yeh wrote:
> > > Sorry this took so long.
> > 
> > No worries.
> > 
> > > 
> > > The vmwgfx part:  Reviewed-by: Sinclair Yeh <s...@vmware.com>
> > > 
> > > I've done some testing and the vmwgfx part looks good.  Has Daniel
> > > already taken these or should I put them in my next request?
> > 
> > You can take them, or I can push them to drm-misc-next. Whatever
> > works best for you.
> > 
> > And I'll want to revisit this topic soonish and move the clip
> > handling into the helper as discussed with Daniel. But that can
> > wait a bit until we get this round merged somewhere.
> 
> Because we're still in the merge window I think it's probably best if we
> push the entire series in through drm-misc. Tree-coordination in the merge
> window is always a bit a pain.
> -Daniel

Ok, so I'll leave this series to you then.

thanks,

Sinclair

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff

[Sinclair Yeh]

Sorry this took so long.

The vmwgfx part:  Reviewed-by: Sinclair Yeh <s...@vmware.com>

I've done some testing and the vmwgfx part looks good.  Has Daniel
already taken these or should I put them in my next request?

Sinclair

On Wed, Nov 01, 2017 at 08:29:15PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrj...@linux.intel.com>
> 
> While trawling the tree I spotted some issues with the way vmwgfx
> uses drm_plane_helper_check_state(). Here's my attempt at fixing it.
> Do note that I haven't actually tested the resulting code at all,
> but it does build at least.
> 
> And while touching that general area I took up Daniel's suggestion from
> long ago that drm_plane_helper_check_state() should be renamed and
> relocated to better reflect its status.
> 
> Here's a branch with the entire series:
> git://github.com/vsyrjala/linux.git atomic_helper_plane_stuff
> 
> Cc: VMware Graphics <linux-graphics-maintai...@vmware.com>
> Cc: Sinclair Yeh <s...@vmware.com>
> Cc: Thomas Hellstrom <thellst...@vmware.com>
> Cc: Daniel Vetter <dan...@ffwll.ch>
> 
> Ville Syrjälä (5):
>   drm/vmwgfx: Remove bogus crtc coords vs fb size check
>   drm/vmwgfx: Use drm_plane_helper_check_state()
>   drm/vmwgfx: Try to fix plane clipping
>   drm: Check crtc_state->enable rather than crtc->enabled in
> drm_plane_helper_check_state()
>   drm: Move drm_plane_helper_check_state() into drm_atomic_helper.c
> 
>  drivers/gpu/drm/arm/hdlcd_crtc.c|   8 +-
>  drivers/gpu/drm/arm/malidp_planes.c |   3 +-
>  drivers/gpu/drm/drm_atomic_helper.c |  95 
>  drivers/gpu/drm/drm_plane_helper.c  | 111 
> +++-
>  drivers/gpu/drm/drm_simple_kms_helper.c |   9 ++-
>  drivers/gpu/drm/i915/intel_display.c|  20 ++---
>  drivers/gpu/drm/imx/ipuv3-plane.c   |   8 +-
>  drivers/gpu/drm/mediatek/mtk_drm_plane.c|   8 +-
>  drivers/gpu/drm/meson/meson_plane.c |   8 +-
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c   |   5 +-
>  drivers/gpu/drm/nouveau/nv50_display.c  |  18 +++--
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c |   6 +-
>  drivers/gpu/drm/tegra/dc.c  |   4 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c |  40 --
>  drivers/gpu/drm/zte/zx_plane.c  |  15 ++--
>  include/drm/drm_atomic_helper.h |   7 ++
>  include/drm/drm_plane_helper.h  |   5 --
>  17 files changed, 187 insertions(+), 183 deletions(-)
> 
> -- 
> 2.13.6
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 05/16] drm/vmwgfx: Make check_modeset() use the new atomic iterator macros.

[Sinclair Yeh]

Reviewed-by: Sinclair Yeh <s...@vmware.com>

On Wed, Jul 12, 2017 at 10:13:33AM +0200, Maarten Lankhorst wrote:
> I don't think the checking of resources in this function is very
> atomic-like, but it should definitely not use a macro that's about
> to be removed.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> Cc: VMware Graphics <linux-graphics-maintai...@vmware.com>
> Cc: Sinclair Yeh <s...@vmware.com>
> Cc: Thomas Hellstrom <thellst...@vmware.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 1cd67b10a0d9..64f66ff97fab 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -1536,8 +1536,7 @@ vmw_kms_atomic_check_modeset(struct drm_device *dev,
>   struct vmw_private *dev_priv = vmw_priv(dev);
>   int i;
>  
> -
> - for_each_crtc_in_state(state, crtc, crtc_state, i) {
> + for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
>   unsigned long requested_bb_mem = 0;
>  
>   if (dev_priv->active_display_unit == vmw_du_screen_target) {
> -- 
> 2.11.0
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 05/16] drm/vmwgfx: Make check_modeset() use the new atomic iterator macros.

[Sinclair Yeh]

On Wed, Jul 12, 2017 at 11:09:16AM +0200, Daniel Vetter wrote:
> On Wed, Jul 12, 2017 at 10:13:33AM +0200, Maarten Lankhorst wrote:
> > I don't think the checking of resources in this function is very
> > atomic-like, but it should definitely not use a macro that's about
> > to be removed.
> > 
> > Signed-off-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> > Cc: VMware Graphics <linux-graphics-maintai...@vmware.com>
> > Cc: Sinclair Yeh <s...@vmware.com>
> > Cc: Thomas Hellstrom <thellst...@vmware.com>
> > ---
> >  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c 
> > b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > index 1cd67b10a0d9..64f66ff97fab 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > @@ -1536,8 +1536,7 @@ vmw_kms_atomic_check_modeset(struct drm_device *dev,
> 
> Afaics vmw_kms_atomic_check_modeset should be static. Feel free to include
> or not include that bikeshed.
> 
> Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch>

Yeah, good point.

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/vmwgfx: Drop drm_vblank_cleanup

[Sinclair Yeh]

Thanks!

Minor typo "dsiplay", otherwise:  Reviewed-by: Sinclair Yeh <s...@vmware.com>

On Mon, Jun 26, 2017 at 06:19:48PM +0200, Daniel Vetter wrote:
> Again stopping the vblank before uninstalling the irq handler is kinda
> the wrong way round, but the fb_off stuff should take care of
> disabling the dsiplay at least in most cases. So drop the
> drm_vblank_cleanup code since it's not really doing anything, it looks
> all cargo-culted.
> 
> v2: Appease gcc better.
> 
> v3: Simplify code (Sean Paul)
> 
> Cc: Sinclair Yeh <s...@vmware.com>
> Cc: Thomas Hellstrom <thellst...@vmware.com>
> Reviewed-by: Sean Paul <seanp...@chromium.org>
> Reviewed-by: Thomas Hellstrom <thellst...@vmware.com>
> Signed-off-by: Daniel Vetter <daniel.vet...@intel.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  |  8 ++--
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.h  |  2 --
>  drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c  |  4 
>  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c |  9 -
>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 27 +--
>  5 files changed, 3 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 3d94ea67a825..1cd67b10a0d9 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -1658,7 +1658,7 @@ int vmw_kms_init(struct vmw_private *dev_priv)
>  
>  int vmw_kms_close(struct vmw_private *dev_priv)
>  {
> - int ret;
> + int ret = 0;
>  
>   /*
>* Docs says we should take the lock before calling this function
> @@ -1666,11 +1666,7 @@ int vmw_kms_close(struct vmw_private *dev_priv)
>* drm_encoder_cleanup which takes the lock we deadlock.
>*/
>   drm_mode_config_cleanup(dev_priv->dev);
> - if (dev_priv->active_display_unit == vmw_du_screen_object)
> - ret = vmw_kms_sou_close_display(dev_priv);
> - else if (dev_priv->active_display_unit == vmw_du_screen_target)
> - ret = vmw_kms_stdu_close_display(dev_priv);
> - else
> + if (dev_priv->active_display_unit == vmw_du_legacy)
>   ret = vmw_kms_ldu_close_display(dev_priv);
>  
>   return ret;
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> index 5f8d678ae675..ff9c8389ff21 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> @@ -390,7 +390,6 @@ int vmw_kms_update_proxy(struct vmw_resource *res,
>   * Screen Objects display functions - vmwgfx_scrn.c
>   */
>  int vmw_kms_sou_init_display(struct vmw_private *dev_priv);
> -int vmw_kms_sou_close_display(struct vmw_private *dev_priv);
>  int vmw_kms_sou_do_surface_dirty(struct vmw_private *dev_priv,
>struct vmw_framebuffer *framebuffer,
>struct drm_clip_rect *clips,
> @@ -418,7 +417,6 @@ int vmw_kms_sou_readback(struct vmw_private *dev_priv,
>   * Screen Target Display Unit functions - vmwgfx_stdu.c
>   */
>  int vmw_kms_stdu_init_display(struct vmw_private *dev_priv);
> -int vmw_kms_stdu_close_display(struct vmw_private *dev_priv);
>  int vmw_kms_stdu_surface_dirty(struct vmw_private *dev_priv,
>  struct vmw_framebuffer *framebuffer,
>  struct drm_clip_rect *clips,
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
> index d3987bcf53f8..449ed4fba0f2 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
> @@ -582,13 +582,9 @@ int vmw_kms_ldu_init_display(struct vmw_private 
> *dev_priv)
>  
>  int vmw_kms_ldu_close_display(struct vmw_private *dev_priv)
>  {
> - struct drm_device *dev = dev_priv->dev;
> -
>   if (!dev_priv->ldu_priv)
>   return -ENOSYS;
>  
> - drm_vblank_cleanup(dev);
> -
>   BUG_ON(!list_empty(_priv->ldu_priv->active));
>  
>   kfree(dev_priv->ldu_priv);
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> index 8d7dc9def7c2..3b917c9b0c21 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> @@ -746,15 +746,6 @@ int vmw_kms_sou_init_display(struct vmw_private 
> *dev_priv)
>   return 0;
>  }
>  
> -int vmw_kms_sou_close_display(struct vmw_private *dev_priv)
> -{
> - struct drm_device *dev = dev_priv->dev;
> -
> - drm_vblank_cleanup(dev);
> -
> - return 0;
> -}
> -
>  static int do_dmabuf_define_gmrfb(struct vmw_private *dev_priv,
>   

Re: [Intel-gfx] [PATCH 35/37] drm/vmwgfx: Drop drm_vblank_cleanup

[Sinclair Yeh]

Looks ok.  I won't get around to testing this until maybe mid next week though.

On Wed, May 24, 2017 at 04:52:10PM +0200, Daniel Vetter wrote:
> Again stopping the vblank before uninstalling the irq handler is kinda
> the wrong way round, but the fb_off stuff should take care of
> disabling the dsiplay at least in most cases. So drop the
typo ^

> drm_vblank_cleanup code since it's not really doing anything, it looks
> all cargo-culted.
> 
> v2: Appease gcc better.
> 
> Cc: Sinclair Yeh <s...@vmware.com>
> Cc: Thomas Hellstrom <thellst...@vmware.com>
> Signed-off-by: Daniel Vetter <daniel.vet...@intel.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  |  9 +++--
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.h  |  2 --
>  drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c  |  4 
>  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c |  9 -
>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 27 +--
>  5 files changed, 4 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index a8876b070168..d1f742478b03 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -1771,7 +1771,7 @@ int vmw_kms_init(struct vmw_private *dev_priv)
>  
>  int vmw_kms_close(struct vmw_private *dev_priv)
>  {
> - int ret;
> + int ret = 0;
>  
>   /*
>* Docs says we should take the lock before calling this function
> @@ -1779,11 +1779,8 @@ int vmw_kms_close(struct vmw_private *dev_priv)
>* drm_encoder_cleanup which takes the lock we deadlock.
>*/
>   drm_mode_config_cleanup(dev_priv->dev);
> - if (dev_priv->active_display_unit == vmw_du_screen_object)
> - ret = vmw_kms_sou_close_display(dev_priv);
> - else if (dev_priv->active_display_unit == vmw_du_screen_target)
> - ret = vmw_kms_stdu_close_display(dev_priv);
> - else
> + if (dev_priv->active_display_unit != vmw_du_screen_object &&
> + dev_priv->active_display_unit != vmw_du_screen_target)
>   ret = vmw_kms_ldu_close_display(dev_priv);
>  
>   return ret;
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> index 13f2f1d2818a..f94b4ca38ab2 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> @@ -405,7 +405,6 @@ int vmw_kms_update_proxy(struct vmw_resource *res,
>   * Screen Objects display functions - vmwgfx_scrn.c
>   */
>  int vmw_kms_sou_init_display(struct vmw_private *dev_priv);
> -int vmw_kms_sou_close_display(struct vmw_private *dev_priv);
>  int vmw_kms_sou_do_surface_dirty(struct vmw_private *dev_priv,
>struct vmw_framebuffer *framebuffer,
>struct drm_clip_rect *clips,
> @@ -433,7 +432,6 @@ int vmw_kms_sou_readback(struct vmw_private *dev_priv,
>   * Screen Target Display Unit functions - vmwgfx_stdu.c
>   */
>  int vmw_kms_stdu_init_display(struct vmw_private *dev_priv);
> -int vmw_kms_stdu_close_display(struct vmw_private *dev_priv);
>  int vmw_kms_stdu_surface_dirty(struct vmw_private *dev_priv,
>  struct vmw_framebuffer *framebuffer,
>  struct drm_clip_rect *clips,
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
> index d3987bcf53f8..449ed4fba0f2 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
> @@ -582,13 +582,9 @@ int vmw_kms_ldu_init_display(struct vmw_private 
> *dev_priv)
>  
>  int vmw_kms_ldu_close_display(struct vmw_private *dev_priv)
>  {
> - struct drm_device *dev = dev_priv->dev;
> -
>   if (!dev_priv->ldu_priv)
>   return -ENOSYS;
>  
> - drm_vblank_cleanup(dev);
> -
>   BUG_ON(!list_empty(_priv->ldu_priv->active));
>  
>   kfree(dev_priv->ldu_priv);
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> index 8d7dc9def7c2..3b917c9b0c21 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> @@ -746,15 +746,6 @@ int vmw_kms_sou_init_display(struct vmw_private 
> *dev_priv)
>   return 0;
>  }
>  
> -int vmw_kms_sou_close_display(struct vmw_private *dev_priv)
> -{
> - struct drm_device *dev = dev_priv->dev;
> -
> - drm_vblank_cleanup(dev);
> -
> - return 0;
> -}
> -
>  static int do_dmabuf_define_gmrfb(struct vmw_private *dev_priv,
> struct vmw_framebuffer *framebuffer)
>  {
> diff --git a/drivers/g

Re: [Intel-gfx] [PATCH v3] drm: Add DRM_MODE_ROTATE_ and DRM_MODE_REFLECT_ to UAPI

[Sinclair Yeh]

vmwgfx part: Reviewed-by: Sinclair Yeh <s...@vmware.com>

On Wed, May 17, 2017 at 09:39:11PM -0400, Robert Foss wrote:
> Add DRM_MODE_ROTATE_ and DRM_MODE_REFLECT_ defines to the UAPI
> as a convenience.
> 
> Ideally the DRM_ROTATE_ and DRM_REFLECT_ property ids are looked up
> through the atomic API, but realizing that userspace is likely to take
> shortcuts and assume that the enum values are what is sent over the
> wire.
> 
> As a result these defines are provided purely as a convenience to
> userspace applications.
> 
> Signed-off-by: Robert Foss <robert.f...@collabora.com>
> ---
> Changes since v2:
>  - Changed define prefix from DRM_MODE_PROP_ to DRM_MODE_
>  - Fix compilation errors
>  - Changed comment formatting
>  - Deduplicated comment lines
>  - Clarified DRM_MODE_PROP_REFLECT_ comment
> 
> Changes since v1:
>  - Moved defines from drm.h to drm_mode.h
>  - Changed define prefix from DRM_ to DRM_MODE_PROP_ 
>  - Updated uses of the defines to the new prefix
>  - Removed include from drm_rect.c
>  - Stopped using the BIT() macro 
> 
>  drivers/gpu/drm/arm/malidp_drv.h|  2 +-
>  drivers/gpu/drm/arm/malidp_planes.c | 18 -
>  drivers/gpu/drm/armada/armada_overlay.c |  2 +-
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 20 +-
>  drivers/gpu/drm/drm_atomic.c|  2 +-
>  drivers/gpu/drm/drm_atomic_helper.c |  2 +-
>  drivers/gpu/drm/drm_blend.c | 43 ++---
>  drivers/gpu/drm/drm_fb_helper.c |  4 +-
>  drivers/gpu/drm/drm_plane_helper.c  |  2 +-
>  drivers/gpu/drm/drm_rect.c  | 36 +-
>  drivers/gpu/drm/i915/i915_debugfs.c | 14 +++
>  drivers/gpu/drm/i915/intel_atomic_plane.c   |  6 +--
>  drivers/gpu/drm/i915/intel_display.c| 50 
> -
>  drivers/gpu/drm/i915/intel_fbc.c|  2 +-
>  drivers/gpu/drm/i915/intel_fbdev.c  |  2 +-
>  drivers/gpu/drm/i915/intel_sprite.c | 20 +-
>  drivers/gpu/drm/imx/ipuv3-plane.c   |  2 +-
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c   | 30 +++
>  drivers/gpu/drm/nouveau/nv50_display.c  |  2 +-
>  drivers/gpu/drm/omapdrm/omap_drv.c  |  4 +-
>  drivers/gpu/drm/omapdrm/omap_fb.c   | 18 -
>  drivers/gpu/drm/omapdrm/omap_plane.c| 16 
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c |  4 +-
>  include/drm/drm_blend.h | 21 +--
>  include/uapi/drm/drm_mode.h | 49 +++-
>  25 files changed, 201 insertions(+), 170 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/malidp_drv.h 
> b/drivers/gpu/drm/arm/malidp_drv.h
> index 040311ffcaec..2e2033140efc 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.h
> +++ b/drivers/gpu/drm/arm/malidp_drv.h
> @@ -65,6 +65,6 @@ void malidp_de_planes_destroy(struct drm_device *drm);
>  int malidp_crtc_init(struct drm_device *drm);
>  
>  /* often used combination of rotational bits */
> -#define MALIDP_ROTATED_MASK  (DRM_ROTATE_90 | DRM_ROTATE_270)
> +#define MALIDP_ROTATED_MASK  (DRM_MODE_ROTATE_90 | DRM_MODE_ROTATE_270)
>  
>  #endif  /* __MALIDP_DRV_H__ */
> diff --git a/drivers/gpu/drm/arm/malidp_planes.c 
> b/drivers/gpu/drm/arm/malidp_planes.c
> index 814fda23cead..063a8d2b0be3 100644
> --- a/drivers/gpu/drm/arm/malidp_planes.c
> +++ b/drivers/gpu/drm/arm/malidp_planes.c
> @@ -80,7 +80,7 @@ static void malidp_plane_reset(struct drm_plane *plane)
>   state = kzalloc(sizeof(*state), GFP_KERNEL);
>   if (state) {
>   state->base.plane = plane;
> - state->base.rotation = DRM_ROTATE_0;
> + state->base.rotation = DRM_MODE_ROTATE_0;
>   plane->state = >base;
>   }
>  }
> @@ -221,7 +221,7 @@ static int malidp_de_plane_check(struct drm_plane *plane,
>   return ret;
>  
>   /* packed RGB888 / BGR888 can't be rotated or flipped */
> - if (state->rotation != DRM_ROTATE_0 &&
> + if (state->rotation != DRM_MODE_ROTATE_0 &&
>   (fb->format->format == DRM_FORMAT_RGB888 ||
>fb->format->format == DRM_FORMAT_BGR888))
>   return -EINVAL;
> @@ -315,12 +315,12 @@ static void malidp_de_plane_update(struct drm_plane 
> *plane,
>   val &= ~LAYER_ROT_MASK;
>  
>   /* setup the rotation and axis flip bits */
> - if (plane->state->rotation & DRM_ROTATE_MASK)
> - val |= ilog2(plane->state->rotation & DRM_ROTATE_MASK)

Re: [Intel-gfx] [PATCH] dma-buf: Rename dma-ops to prevent conflict with kunmap_atomic macro

[Sinclair Yeh]

Minor nits, otherwise the vmwgfx part,
  Reviewed-by: Sinclair Yeh <s...@vmware.com>

On Tue, Apr 18, 2017 at 06:17:00PM -0600, Logan Gunthorpe wrote:
> Seeing the kunmap_atomic dma_buf_op shares the same name with a macro
s^

> in higmem.h, the former can be aliased if any dma-buf user includes
   h^
> that header.
> 
> I'm personally trying to include highmem.h inside scatterlist.h and this
> breaks the dma-buf code proper.
> 
> Christoph Hellwig suggested [1] renaming it and pushing this patch ASAP.
> 
> To maintain consistency I've renamed all four of kmap* and kunmap* to be
> map* and unmap*. (Even though only kmap_atomic presently conflicts.)
> 
> [1] 
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.spinics.net_lists_target-2Ddevel_msg15070.html=DwIBAg=uilaK90D4TOVoH58JNXRgQ=HaJ2a6NYExoV0cntAYcoqA=QP_jolGTC4ofBnHRnAs0tFIXHnVWaTT0AdMyCL9SM64=un2hxBL1283iOTtJeJnvyyvtAu1d5Imyh5Q7AzljrfQ=
>  
> 
> Signed-off-by: Logan Gunthorpe <log...@deltatee.com>
> ---
>  drivers/dma-buf/dma-buf.c  | 16 
>  drivers/gpu/drm/armada/armada_gem.c|  8 
>  drivers/gpu/drm/drm_prime.c|  8 
>  drivers/gpu/drm/i915/i915_gem_dmabuf.c |  8 
>  drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c  |  8 
>  drivers/gpu/drm/tegra/gem.c|  4 ++--
>  drivers/gpu/drm/udl/udl_dmabuf.c   |  8 
>  drivers/gpu/drm/vmwgfx/vmwgfx_prime.c  |  8 
>  drivers/media/v4l2-core/videobuf2-dma-contig.c |  4 ++--
>  drivers/media/v4l2-core/videobuf2-dma-sg.c |  4 ++--
>  drivers/media/v4l2-core/videobuf2-vmalloc.c|  4 ++--
>  drivers/staging/android/ion/ion.c  |  8 
>  include/linux/dma-buf.h| 22 +++---
>  13 files changed, 55 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 0007b79..7cc2bfe 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -405,8 +405,8 @@ struct dma_buf *dma_buf_export(const struct 
> dma_buf_export_info *exp_info)
> || !exp_info->ops->map_dma_buf
> || !exp_info->ops->unmap_dma_buf
> || !exp_info->ops->release
> -   || !exp_info->ops->kmap_atomic
> -   || !exp_info->ops->kmap
> +   || !exp_info->ops->map_atomic
> +   || !exp_info->ops->map
> || !exp_info->ops->mmap)) {
>   return ERR_PTR(-EINVAL);
>   }
> @@ -872,7 +872,7 @@ void *dma_buf_kmap_atomic(struct dma_buf *dmabuf, 
> unsigned long page_num)
>  {
>   WARN_ON(!dmabuf);
>  
> - return dmabuf->ops->kmap_atomic(dmabuf, page_num);
> + return dmabuf->ops->map_atomic(dmabuf, page_num);
>  }
>  EXPORT_SYMBOL_GPL(dma_buf_kmap_atomic);
>  
> @@ -889,8 +889,8 @@ void dma_buf_kunmap_atomic(struct dma_buf *dmabuf, 
> unsigned long page_num,
>  {
>   WARN_ON(!dmabuf);
>  
> - if (dmabuf->ops->kunmap_atomic)
> - dmabuf->ops->kunmap_atomic(dmabuf, page_num, vaddr);
> + if (dmabuf->ops->unmap_atomic)
> + dmabuf->ops->unmap_atomic(dmabuf, page_num, vaddr);
>  }
>  EXPORT_SYMBOL_GPL(dma_buf_kunmap_atomic);
>  
> @@ -907,7 +907,7 @@ void *dma_buf_kmap(struct dma_buf *dmabuf, unsigned long 
> page_num)
>  {
>   WARN_ON(!dmabuf);
>  
> - return dmabuf->ops->kmap(dmabuf, page_num);
> + return dmabuf->ops->map(dmabuf, page_num);
>  }
>  EXPORT_SYMBOL_GPL(dma_buf_kmap);
>  
> @@ -924,8 +924,8 @@ void dma_buf_kunmap(struct dma_buf *dmabuf, unsigned long 
> page_num,
>  {
>   WARN_ON(!dmabuf);
>  
> - if (dmabuf->ops->kunmap)
> - dmabuf->ops->kunmap(dmabuf, page_num, vaddr);
> + if (dmabuf->ops->unmap)
> + dmabuf->ops->unmap(dmabuf, page_num, vaddr);
>  }
>  EXPORT_SYMBOL_GPL(dma_buf_kunmap);
>  
> diff --git a/drivers/gpu/drm/armada/armada_gem.c 
> b/drivers/gpu/drm/armada/armada_gem.c
> index 1597458..d6c2a5d 100644
> --- a/drivers/gpu/drm/armada/armada_gem.c
> +++ b/drivers/gpu/drm/armada/armada_gem.c
> @@ -529,10 +529,10 @@ static const struct dma_buf_ops 
> armada_gem_prime_dmabuf_ops = {
>   .map_dma_buf= armada_gem_prime_map_dma_buf,
>   .unmap_dma_buf  = armada_gem_prime_unmap_dma_buf,
>   .release= drm_gem_dmabuf_release,
> - .kmap_atomic= armada_gem_dmabuf_no_kmap,
> - 

Re: [Intel-gfx] [PATCH 18/19] drm: Add acquire ctx parameter to ->set_config

[Sinclair Yeh]

I missed this one, and looks like it's already in.  So a belated:
Reviewed-by: Sinclair Yeh <s...@vmware.com>

for the vmwgfx part

On Wed, Mar 22, 2017 at 10:50:57PM +0100, Daniel Vetter wrote:
> Surprisingly a lot of legacy drivers roll their own, for
> runtime pm and because vmwgfx.
> 
> Also make nouveau's set_config static while at it.
> 
> Cc: Sinclair Yeh <s...@vmware.com>
> Cc: Thomas Hellstrom <thellst...@vmware.com>
> Cc: Ben Skeggs <bske...@redhat.com>
> Cc: Patrik Jakobsson <patrik.r.jakobs...@gmail.com>
> Cc: Alex Deucher <alexander.deuc...@amd.com>
> Cc: Christian König <christian.koe...@amd.com>
> 
> Signed-off-by: Daniel Vetter <daniel.vet...@ffwll.ch>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 5 +++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h| 3 ++-
>  drivers/gpu/drm/drm_atomic_helper.c | 4 +++-
>  drivers/gpu/drm/drm_crtc.c  | 2 +-
>  drivers/gpu/drm/drm_crtc_helper.c   | 4 +++-
>  drivers/gpu/drm/drm_plane_helper.c  | 2 +-
>  drivers/gpu/drm/gma500/gma_display.c| 7 ---
>  drivers/gpu/drm/gma500/gma_display.h| 3 ++-
>  drivers/gpu/drm/nouveau/dispnv04/crtc.c | 7 ---
>  drivers/gpu/drm/nouveau/nouveau_display.h   | 1 -
>  drivers/gpu/drm/radeon/radeon_display.c | 5 +++--
>  drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 3 ++-
>  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c| 3 ++-
>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c| 3 ++-
>  include/drm/drm_atomic_helper.h | 3 ++-
>  include/drm/drm_crtc.h  | 3 ++-
>  include/drm/drm_crtc_helper.h   | 3 ++-
>  17 files changed, 38 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> index 7b4fe91d3aec..ce15721cadda 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> @@ -333,7 +333,8 @@ int amdgpu_crtc_page_flip_target(struct drm_crtc *crtc,
>   return 0;
>  }
>  
> -int amdgpu_crtc_set_config(struct drm_mode_set *set)
> +int amdgpu_crtc_set_config(struct drm_mode_set *set,
> +struct drm_modeset_acquire_ctx *ctx)
>  {
>   struct drm_device *dev;
>   struct amdgpu_device *adev;
> @@ -350,7 +351,7 @@ int amdgpu_crtc_set_config(struct drm_mode_set *set)
>   if (ret < 0)
>   return ret;
>  
> - ret = drm_crtc_helper_set_config(set);
> + ret = drm_crtc_helper_set_config(set, ctx);
>  
>   list_for_each_entry(crtc, >mode_config.crtc_list, head)
>   if (crtc->enabled)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> index d19b803ba509..20d6522fd7b4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> @@ -593,7 +593,8 @@ int amdgpu_align_pitch(struct amdgpu_device *adev, int 
> width, int bpp, bool tile
>  /* amdgpu_display.c */
>  void amdgpu_print_display_setup(struct drm_device *dev);
>  int amdgpu_modeset_create_props(struct amdgpu_device *adev);
> -int amdgpu_crtc_set_config(struct drm_mode_set *set);
> +int amdgpu_crtc_set_config(struct drm_mode_set *set,
> +struct drm_modeset_acquire_ctx *ctx);
>  int amdgpu_crtc_page_flip_target(struct drm_crtc *crtc,
>struct drm_framebuffer *fb,
>struct drm_pending_vblank_event *event,
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 08d10abcece0..b502e2809ebd 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2266,6 +2266,7 @@ static int update_output_state(struct drm_atomic_state 
> *state,
>  /**
>   * drm_atomic_helper_set_config - set a new config from userspace
>   * @set: mode set configuration
> + * @ctx: lock acquisition context
>   *
>   * Provides a default crtc set_config handler using the atomic driver 
> interface.
>   *
> @@ -2278,7 +2279,8 @@ static int update_output_state(struct drm_atomic_state 
> *state,
>   * Returns:
>   * Returns 0 on success, negative errno numbers on failure.
>   */
> -int drm_atomic_helper_set_config(struct drm_mode_set *set)
> +int drm_atomic_helper_set_config(struct drm_mode_set *set,
> +  struct drm_modeset_acquire_ctx *ctx)
>  {
>   struct drm_atomic_state *state;
>   struct drm_crtc *crtc = set->crtc;
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index bfaa0e769ea6..3fe1ec23c87e 100644
> --- a/

Re: [Intel-gfx] [PATCH 12/15] drm: Add acquire ctx to ->gamma_set hook

[Sinclair Yeh]

vmwgfx part:  Reviewed-by: Sinclair Yeh <s...@vmware.com>

On Mon, Apr 03, 2017 at 10:33:01AM +0200, Daniel Vetter wrote:
> Atomic helpers really want this instead of the hacked-up legacy
> backoff trick, which unfortunately prevents drivers from using their
> own private drm_modeset_locks.
> 
> Aside: There's a few atomic drivers (nv50, vc4, soon vmwgfx) which
> don't yet use the new atomic color mgmt/gamma table stuff. Would be
> nice if they could switch over and just hook up
> drm_atomic_helper_legacy_gamma_set() instead.
> 
> Cc: Dave Airlie <airl...@redhat.com>
> Cc: Alex Deucher <alexander.deuc...@amd.com>
> Cc: Christian König <christian.koe...@amd.com>
> Cc: Gerd Hoffmann <kra...@redhat.com>
> Cc: Ben Skeggs <bske...@redhat.com>
> Cc: Sinclair Yeh <s...@vmware.com>
> Cc: Thomas Hellstrom <thellst...@vmware.com>
> Cc: Eric Anholt <e...@anholt.net>
> Signed-off-by: Daniel Vetter <daniel.vet...@intel.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c   | 3 ++-
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c   | 3 ++-
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c| 3 ++-
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c| 3 ++-
>  drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 3 ++-
>  drivers/gpu/drm/ast/ast_mode.c   | 3 ++-
>  drivers/gpu/drm/cirrus/cirrus_mode.c | 3 ++-
>  drivers/gpu/drm/drm_atomic_helper.c  | 4 +++-
>  drivers/gpu/drm/drm_color_mgmt.c | 3 ++-
>  drivers/gpu/drm/drm_fb_helper.c  | 3 ++-
>  drivers/gpu/drm/gma500/gma_display.c | 3 ++-
>  drivers/gpu/drm/gma500/gma_display.h | 3 ++-
>  drivers/gpu/drm/mgag200/mgag200_mode.c   | 3 ++-
>  drivers/gpu/drm/nouveau/dispnv04/crtc.c  | 3 ++-
>  drivers/gpu/drm/nouveau/nv50_display.c   | 3 ++-
>  drivers/gpu/drm/radeon/radeon_display.c  | 3 ++-
>  drivers/gpu/drm/vc4/vc4_crtc.c   | 3 ++-
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  | 3 ++-
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.h  | 3 ++-
>  include/drm/drm_atomic_helper.h  | 3 ++-
>  include/drm/drm_crtc.h   | 3 ++-
>  21 files changed, 43 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c 
> b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> index f525ae4e0576..daf003dd2351 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> @@ -2631,7 +2631,8 @@ static void dce_v10_0_cursor_reset(struct drm_crtc 
> *crtc)
>  }
>  
>  static int dce_v10_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 
> *green,
> - u16 *blue, uint32_t size)
> + u16 *blue, uint32_t size,
> + struct drm_modeset_acquire_ctx *ctx)
>  {
>   struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
>   int i;
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c 
> b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> index 3eac27f24d94..3a7296724457 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> @@ -2651,7 +2651,8 @@ static void dce_v11_0_cursor_reset(struct drm_crtc 
> *crtc)
>  }
>  
>  static int dce_v11_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 
> *green,
> - u16 *blue, uint32_t size)
> + u16 *blue, uint32_t size,
> + struct drm_modeset_acquire_ctx *ctx)
>  {
>   struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
>   int i;
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c 
> b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> index 838cf1a778f2..8ccada5d6f39 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> @@ -1998,7 +1998,8 @@ static void dce_v6_0_cursor_reset(struct drm_crtc *crtc)
>  }
>  
>  static int dce_v6_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 
> *green,
> -u16 *blue, uint32_t size)
> +u16 *blue, uint32_t size,
> +struct drm_modeset_acquire_ctx *ctx)
>  {
>   struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
>   int i;
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c 
> b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> index 1b0717b11efe..6943f2641c90 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> @@ -2482,7 +2482,8 @@ static void dce_v8_0_cursor_reset(struct drm_crtc *crtc)
>  }
>  
>  static int dce_v8_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 
> *green,
> -u16 *blue, uint32

Re: [Intel-gfx] linux-next: build failure after merge of the drm-misc tree

[Sinclair Yeh]

Thanks for this.   This and "drm/vmwgfx: merge fixup for set_config API change":

Reviewed-by: Sinclair Yeh <s...@vmware.com>

On Mon, Apr 03, 2017 at 01:31:29PM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the drm-misc tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
> 
> drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c: In function 'vmw_sou_crtc_page_flip':
> drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c:327:8: error: too few arguments to 
> function 'drm_atomic_helper_page_flip'
>   ret = drm_atomic_helper_page_flip(crtc, new_fb, NULL, flags);
> ^
> In file included from drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c:31:0:
> include/drm/drm_atomic_helper.h:126:5: note: declared here
>  int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
>  ^
> drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c: In function 'vmw_stdu_crtc_page_flip':
> drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c:508:8: error: too few arguments to 
> function 'drm_atomic_helper_page_flip'
>   ret = drm_atomic_helper_page_flip(crtc, new_fb, NULL, flags);
> ^
> In file included from drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c:32:0:
> include/drm/drm_atomic_helper.h:126:5: note: declared here
>  int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
>  ^
> 
> Caused by commit
> 
>   41292b1fa13a ("drm: Add acquire ctx parameter to ->page_flip(_target)")
> 
> interacting with commits
> 
>   904bb5e5817f ("drm/vmwgfx: Switch over to internal atomic API for STDU")
>   b0119cb9229d ("drm/vmwgfx: Switch over to internal atomic API for SOU and 
> LDU")
> 
> from the drm tree.
> 
> I added this merge fix patch for today:
> 
> From: Stephen Rothwell <s...@canb.auug.org.au>
> Date: Mon, 3 Apr 2017 13:25:55 +1000
> Subject: [PATCH] drm/vmwgfx: merge fixup for page_flip API change
> 
> Signed-off-by: Stephen Rothwell <s...@canb.auug.org.au>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> index 02b8f2541dca..8d7dc9def7c2 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> @@ -324,7 +324,7 @@ static int vmw_sou_crtc_page_flip(struct drm_crtc *crtc,
>   return -EINVAL;
>  
>   flags &= ~DRM_MODE_PAGE_FLIP_ASYNC;
> - ret = drm_atomic_helper_page_flip(crtc, new_fb, NULL, flags);
> + ret = drm_atomic_helper_page_flip(crtc, new_fb, NULL, flags, ctx);
>   if (ret) {
>   DRM_ERROR("Page flip error %d.\n", ret);
>   return ret;
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> index e59bbcd8b226..bad31bdf09b6 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> @@ -505,7 +505,7 @@ static int vmw_stdu_crtc_page_flip(struct drm_crtc *crtc,
>* don't hand it to the helper.
>*/
>   flags &= ~DRM_MODE_PAGE_FLIP_ASYNC;
> - ret = drm_atomic_helper_page_flip(crtc, new_fb, NULL, flags);
> + ret = drm_atomic_helper_page_flip(crtc, new_fb, NULL, flags, ctx);
>   if (ret) {
>   DRM_ERROR("Page flip error %d.\n", ret);
>   return ret;
> -- 
> 2.11.0
> 
> -- 
> Cheers,
> Stephen Rothwell
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/3] gpu: drm: drivers: Convert printk(KERN_ to pr_

[Sinclair Yeh]

For drm/vmwgfx:  Acked-by: Sinclair Yeh <s...@vmware.com>

On Tue, Feb 28, 2017 at 04:55:54AM -0800, Joe Perches wrote:
> Use a more common logging style.
> 
> Miscellanea:
> 
> o Coalesce formats and realign arguments
> o Neaten a few macros now using pr_
> 
> Signed-off-by: Joe Perches <j...@perches.com>
> ---
>  drivers/gpu/drm/gma500/cdv_intel_lvds.c   |  9 -
>  drivers/gpu/drm/gma500/oaktrail_lvds.c| 18 +-
>  drivers/gpu/drm/gma500/psb_drv.h  |  5 ++---
>  drivers/gpu/drm/gma500/psb_intel_lvds.c   |  7 +++
>  drivers/gpu/drm/i915/i915_sw_fence.c  |  8 
>  drivers/gpu/drm/mgag200/mgag200_mode.c|  2 +-
>  drivers/gpu/drm/msm/msm_drv.c |  2 +-
>  drivers/gpu/drm/nouveau/nouveau_acpi.c|  7 ---
>  drivers/gpu/drm/nouveau/nouveau_vga.c |  4 ++--
>  drivers/gpu/drm/nouveau/nv50_display.c| 22 +++---
>  drivers/gpu/drm/nouveau/nvkm/core/mm.c| 10 +-
>  drivers/gpu/drm/omapdrm/dss/dsi.c | 17 -
>  drivers/gpu/drm/omapdrm/dss/dss.c |  3 +--
>  drivers/gpu/drm/omapdrm/dss/dss.h | 15 ++-
>  drivers/gpu/drm/omapdrm/omap_gem.c|  5 ++---
>  drivers/gpu/drm/r128/r128_cce.c   |  7 +++
>  drivers/gpu/drm/ttm/ttm_bo.c  |  2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_fence.c |  6 ++
>  drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c |  3 +--
>  drivers/gpu/drm/vmwgfx/vmwgfx_resource.c  |  4 ++--
>  20 files changed, 72 insertions(+), 84 deletions(-)
> 
> diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c 
> b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
> index 5efdb7fbb7ee..e64960db3224 100644
> --- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
> +++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
> @@ -284,8 +284,7 @@ static bool cdv_intel_lvds_mode_fixup(struct drm_encoder 
> *encoder,
>   head) {
>   if (tmp_encoder != encoder
>   && tmp_encoder->crtc == encoder->crtc) {
> - printk(KERN_ERR "Can't enable LVDS and another "
> -"encoder on the same pipe\n");
> + pr_err("Can't enable LVDS and another encoder on the 
> same pipe\n");
>   return false;
>   }
>   }
> @@ -756,13 +755,13 @@ void cdv_intel_lvds_init(struct drm_device *dev,
>  
>  failed_find:
>   mutex_unlock(>mode_config.mutex);
> - printk(KERN_ERR "Failed find\n");
> + pr_err("Failed find\n");
>   psb_intel_i2c_destroy(gma_encoder->ddc_bus);
>  failed_ddc:
> - printk(KERN_ERR "Failed DDC\n");
> + pr_err("Failed DDC\n");
>   psb_intel_i2c_destroy(gma_encoder->i2c_bus);
>  failed_blc_i2c:
> - printk(KERN_ERR "Failed BLC\n");
> + pr_err("Failed BLC\n");
>   drm_encoder_cleanup(encoder);
>   drm_connector_cleanup(connector);
>   kfree(lvds_priv);
> diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c 
> b/drivers/gpu/drm/gma500/oaktrail_lvds.c
> index f7038f12ac76..e6943fef0611 100644
> --- a/drivers/gpu/drm/gma500/oaktrail_lvds.c
> +++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c
> @@ -255,15 +255,15 @@ static void oaktrail_lvds_get_configuration_mode(struct 
> drm_device *dev,
>   ((ti->vblank_hi << 8) | ti->vblank_lo);
>   mode->clock = ti->pixel_clock * 10;
>  #if 0
> - printk(KERN_INFO "hdisplay is %d\n", mode->hdisplay);
> - printk(KERN_INFO "vdisplay is %d\n", mode->vdisplay);
> - printk(KERN_INFO "HSS is %d\n", mode->hsync_start);
> - printk(KERN_INFO "HSE is %d\n", mode->hsync_end);
> - printk(KERN_INFO "htotal is %d\n", mode->htotal);
> - printk(KERN_INFO "VSS is %d\n", mode->vsync_start);
> - printk(KERN_INFO "VSE is %d\n", mode->vsync_end);
> - printk(KERN_INFO "vtotal is %d\n", mode->vtotal);
> - printk(KERN_INFO "clock is %d\n", mode->clock);
> + pr_info("hdisplay is %d\n", mode->hdisplay);
> + pr_info("vdisplay is %d\n", mode->vdisplay);
> + pr_info("HSS is %d\n", mode->hsync_start);
> + pr_info("HSE is %d\n", mode->hsync_end);
> + pr_info("htotal is %d\n", mode->htotal);
> + pr_inf

Re: [Intel-gfx] [PATCH v3 4/4] drm: Resurrect atomic rmfb code, v3

[Sinclair Yeh]

On Wed, Feb 15, 2017 at 03:56:09PM +0200, Jani Nikula wrote:
> On Wed, 25 Jan 2017, Maarten Lankhorst  
> wrote:
> > This was somehow lost between v3 and the merged version in Maarten's
> > patch merged as:
> >
> > commit f2d580b9a8149735cbc4b59c4a8df60173658140
> > Author: Maarten Lankhorst 
> > Date:   Wed May 4 14:38:26 2016 +0200
> >
> > drm/core: Do not preserve framebuffer on rmfb, v4.
> >
> > This introduces a slight behavioral change to rmfb. Instead of
> > disabling a crtc when the primary plane is disabled, we try to
> > preserve it.
> >
> > Apart from old versions of the vmwgfx xorg driver, there is
> > nothing depending on rmfb disabling a crtc. Since vmwgfx is
> > a legacy driver we can safely only disable the plane with atomic.
> >
> > If this commit is rejected by the driver then we will still fall
> > back to the old behavior and turn off the crtc.
> >
> > v2:
> > - Remove plane->fb assignment, done by drm_atomic_clean_old_fb.
> > - Add WARN_ON when atomic_remove_fb fails.
> > - Always call drm_atomic_state_put.
> > v3:
> > - Use drm_drv_uses_atomic_modeset
> > - Handle the case where the first plane-disable-only commit fails
> >   with -EINVAL. Some drivers do not support this, fall back to
> >   disabling all crtc's in this case.
> >
> > Signed-off-by: Daniel Vetter 
> > Signed-off-by: Daniel Vetter 
> > Signed-off-by: Maarten Lankhorst 
> 
> Pushed to drm-misc-next-fixes, and sent the pull request to Dave. Thanks
> for the patch.

I verified yesterday that this patch will cause a regression for vmwgfx
multi-mon.  Can we hold off on this?



> 
> BR,
> Jani.
> 
> 
> > ---
> >  drivers/gpu/drm/drm_atomic.c| 106 
> > 
> >  drivers/gpu/drm/drm_crtc_internal.h |   1 +
> >  drivers/gpu/drm/drm_framebuffer.c   |   7 +++
> >  3 files changed, 114 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 723392fc98c8..c79ab8048435 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -2058,6 +2058,112 @@ static void complete_crtc_signaling(struct 
> > drm_device *dev,
> > kfree(fence_state);
> >  }
> >  
> > +int drm_atomic_remove_fb(struct drm_framebuffer *fb)
> > +{
> > +   struct drm_modeset_acquire_ctx ctx;
> > +   struct drm_device *dev = fb->dev;
> > +   struct drm_atomic_state *state;
> > +   struct drm_plane *plane;
> > +   struct drm_connector *conn;
> > +   struct drm_connector_state *conn_state;
> > +   int i, ret = 0;
> > +   unsigned plane_mask, disable_crtcs = false;
> > +
> > +   state = drm_atomic_state_alloc(dev);
> > +   if (!state)
> > +   return -ENOMEM;
> > +
> > +   drm_modeset_acquire_init(, 0);
> > +   state->acquire_ctx = 
> > +
> > +retry:
> > +   plane_mask = 0;
> > +   ret = drm_modeset_lock_all_ctx(dev, );
> > +   if (ret)
> > +   goto unlock;
> > +
> > +   drm_for_each_plane(plane, dev) {
> > +   struct drm_plane_state *plane_state;
> > +
> > +   if (plane->state->fb != fb)
> > +   continue;
> > +
> > +   plane_state = drm_atomic_get_plane_state(state, plane);
> > +   if (IS_ERR(plane_state)) {
> > +   ret = PTR_ERR(plane_state);
> > +   goto unlock;
> > +   }
> > +
> > +   /*
> > +* Some drivers do not support keeping crtc active with the
> > +* primary plane disabled. If we fail to commit with -EINVAL
> > +* then we will try to perform the same commit but with all
> > +* crtc's disabled for primary planes as well.
> > +*/
> > +   if (disable_crtcs && plane_state->crtc->primary == plane) {
> > +   struct drm_crtc_state *crtc_state;
> > +
> > +   crtc_state = drm_atomic_get_existing_crtc_state(state, 
> > plane_state->crtc);
> > +
> > +   ret = drm_atomic_add_affected_connectors(state, 
> > plane_state->crtc);
> > +   if (ret)
> > +   goto unlock;
> > +
> > +   crtc_state->active = false;
> > +   ret = drm_atomic_set_mode_for_crtc(crtc_state, NULL);
> > +   if (ret)
> > +   goto unlock;
> > +   }
> > +
> > +   drm_atomic_set_fb_for_plane(plane_state, NULL);
> > +   ret = drm_atomic_set_crtc_for_plane(plane_state, NULL);
> > +   if (ret)
> > +   goto unlock;
> > +
> > +   plane_mask |= BIT(drm_plane_index(plane));
> > +
> > +   plane->old_fb = plane->fb;
> > +   }
> > +
> > +   /* This list is only not empty when disable_crtcs is set. */
> > +   for_each_connector_in_state(state, conn, conn_state, i) {
> > +   ret = drm_atomic_set_crtc_for_connector(conn_state, NULL);
> > +
> > 

Re: [Intel-gfx] [PATCH 4/4] drm: Resurrect atomic rmfb code, v2

[Sinclair Yeh]

I've verified that it doesn't break our existing code, but I'm in the process 
of rebasing my atomic enabling patch series onto drm-next along with this.  I 
should be able to get this done by tomorrow morning.

From: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
Sent: Thursday, February 9, 2017 4:29:49 AM
To: Sinclair Yeh
Cc: Thomas Hellstrom; Daniel Vetter; Matt Roper; Daniel Vetter; 
intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org; Daniel Vetter
Subject: Re: [PATCH 4/4] drm: Resurrect atomic rmfb code, v2

Op 26-01-17 om 19:39 schreef Sinclair Yeh:
> On Thu, Jan 26, 2017 at 10:55:51AM +0100, Maarten Lankhorst wrote:
>> Op 25-01-17 om 19:05 schreef Sinclair Yeh:
>>> On Wed, Jan 25, 2017 at 09:36:36AM +0100, Maarten Lankhorst wrote:
>>>> Op 25-01-17 om 09:09 schreef Thomas Hellstrom:
>>>>> On 01/25/2017 05:54 AM, Daniel Vetter wrote:
>>>>>> On Tue, Jan 24, 2017 at 01:44:54PM -0800, Matt Roper wrote:
>>>>>>> On Wed, Jan 11, 2017 at 05:15:47PM +0100, Daniel Vetter wrote:
>>>>>>>> On Thu, Dec 15, 2016 at 03:29:45PM +0100, Maarten Lankhorst wrote:
>>>>>>>>> From: Daniel Vetter <daniel.vet...@ffwll.ch>
>>>>>>>>>
>>>>>>>>> This was somehow lost between v3 and the merged version in Maarten's
>>>>>>>>> patch merged as:
>>>>>>>>>
>>>>>>>>> commit f2d580b9a8149735cbc4b59c4a8df60173658140
>>>>>>>>> Author: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
>>>>>>>>> Date:   Wed May 4 14:38:26 2016 +0200
>>>>>>>>>
>>>>>>>>> drm/core: Do not preserve framebuffer on rmfb, v4.
>>>>>>>>>
>>>>>>>>> Actual code copied from Maarten's patch, but with the slight change to
>>>>>>>>> just use dev->mode_config.funcs->atomic_commit to decide whether to
>>>>>>>>> use the atomic path or not.
>>>>>>>>>
>>>>>>>>> v2:
>>>>>>>>> - Remove plane->fb assignment, done by drm_atomic_clean_old_fb.
>>>>>>>>> - Add WARN_ON when atomic_remove_fb fails.
>>>>>>>>> - Always call drm_atomic_state_put.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Daniel Vetter <daniel.vet...@intel.com>
>>>>>>>>> Signed-off-by: Daniel Vetter <daniel.vet...@ffwll.ch>
>>>>>>>>> Signed-off-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
>>>>>>>> Would be great if someone else could r-b this, I've proven pretty well
>>>>>>>> that I don't understand the complexity here :(
>>>>>>>> -Daniel
>>>>>>> It looks like this will change the behavior slightly in that rmfb will
>>>>>>> cause primary planes to be disabled, but no longer cause the entire CRTC
>>>>>>> to be turned off.  You'll probably want to note that in the commit
>>>>>>> message, along with the justification on why this is okay ABI-wise.
>>>>>>>
>>>>>>> I know that 13803132818c ("drm/core: Preserve the framebuffer after
>>>>>>> removing it.") was initially trying to not only leave the CRTC on, but
>>>>>>> also preserve the framebuffer and leave the planes on; that wound up
>>>>>>> causing some kind of regression for vmwgfx, but I'm unclear on the
>>>>>>> details there.  I'd suggest getting an Ack from one of the vmware guys
>>>>>>> to ensure that the less drastic change in behavior here won't cause them
>>>>>>> any problems.
>>>>> The vmware Xorg driver is currently relying on rmfb to turn all attached
>>>>> crtcs off. Even if we were to fix that in the Xorg driver now, older
>>>>> Xorgs with newer kernels still would break.
>>>> Is it allowed for vmwgfx to keep the crtc enabled, but the primary plane 
>>>> disabled?
>>>>
>>>> If so, when vmwgfx is eventually converted to atomic then we need to 
>>>> special-case rmfb for them somehow.
>>> FYI, we are in the process of converting things to atomic.  This may happen
>>> around 4.12
>>>
>> Will the driver allow the crtc to be enabled without primary plane?
> Give me a few days to get back to you.  I'm reworking some patches right now.
>
>
Any update on this?

~Maarten

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v4 1/2] drm: Improve drm_mm search (and fix topdown allocation) with rbtrees

[Sinclair Yeh]

vmwgfx part:  Reviewed-by: Sinclair Yeh <s...@vmware.com>

On Thu, Feb 02, 2017 at 11:44:33AM +, Chris Wilson wrote:
> The drm_mm range manager claimed to support top-down insertion, but it
> was neither searching for the top-most hole that could fit the
> allocation request nor fitting the request to the hole correctly.
> 
> In order to search the range efficiently, we create a secondary index
> for the holes using either their size or their address. This index
> allows us to find the smallest hole or the hole at the bottom or top of
> the range efficiently, whilst keeping the hole stack to rapidly service
> evictions.
> 
> v2: Search for holes both high and low. Rename flags to mode.
> v3: Discover rb_entry_safe() and use it!
> v4: Kerneldoc for enum drm_mm_insert_mode.
> 
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> Reviewed-by: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
> Cc: Alex Deucher <alexander.deuc...@amd.com>
> Cc: "Christian König" <christian.koe...@amd.com>
> Cc: David Airlie <airl...@linux.ie>
> Cc: Russell King <rmk+ker...@armlinux.org.uk>
> Cc: Daniel Vetter <daniel.vet...@intel.com>
> Cc: Jani Nikula <jani.nik...@linux.intel.com>
> Cc: Sean Paul <seanp...@chromium.org>
> Cc: Lucas Stach <l.st...@pengutronix.de>
> Cc: Christian Gmeiner <christian.gmei...@gmail.com>
> Cc: Rob Clark <robdcl...@gmail.com>
> Cc: Thierry Reding <thierry.red...@gmail.com>
> Cc: Stephen Warren <swar...@wwwdotorg.org>
> Cc: Alexandre Courbot <gnu...@gmail.com>
> Cc: Eric Anholt <e...@anholt.net>
> Cc: Sinclair Yeh <s...@vmware.com>
> Cc: Thomas Hellstrom <thellst...@vmware.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c  |  16 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c |  20 +-
>  drivers/gpu/drm/armada/armada_gem.c  |   4 +-
>  drivers/gpu/drm/drm_mm.c | 488 
> +++
>  drivers/gpu/drm/drm_vma_manager.c|   3 +-
>  drivers/gpu/drm/etnaviv/etnaviv_mmu.c|  16 +-
>  drivers/gpu/drm/i915/i915_gem.c  |  10 +-
>  drivers/gpu/drm/i915/i915_gem_evict.c|   9 +-
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c   |   5 +-
>  drivers/gpu/drm/i915/i915_gem_gtt.c  |  39 +--
>  drivers/gpu/drm/i915/i915_gem_stolen.c   |   6 +-
>  drivers/gpu/drm/msm/msm_gem.c|   3 +-
>  drivers/gpu/drm/msm/msm_gem_vma.c|   3 +-
>  drivers/gpu/drm/selftests/test-drm_mm.c  |  58 ++--
>  drivers/gpu/drm/sis/sis_mm.c |   6 +-
>  drivers/gpu/drm/tegra/gem.c  |   4 +-
>  drivers/gpu/drm/ttm/ttm_bo_manager.c |  18 +-
>  drivers/gpu/drm/vc4/vc4_crtc.c   |   2 +-
>  drivers/gpu/drm/vc4/vc4_hvs.c|   3 +-
>  drivers/gpu/drm/vc4/vc4_plane.c  |   6 +-
>  drivers/gpu/drm/via/via_mm.c |   4 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c   |  10 +-
>  include/drm/drm_mm.h | 184 +-
>  23 files changed, 470 insertions(+), 447 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> index e4eb6dd3798a..0335c2f331e9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> @@ -97,8 +97,7 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
>  {
>   struct amdgpu_gtt_mgr *mgr = man->priv;
>   struct drm_mm_node *node = mem->mm_node;
> - enum drm_mm_search_flags sflags = DRM_MM_SEARCH_BEST;
> - enum drm_mm_allocator_flags aflags = DRM_MM_CREATE_DEFAULT;
> + enum drm_mm_insert_mode mode;
>   unsigned long fpfn, lpfn;
>   int r;
>  
> @@ -115,15 +114,14 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager 
> *man,
>   else
>   lpfn = man->size;
>  
> - if (place && place->flags & TTM_PL_FLAG_TOPDOWN) {
> - sflags = DRM_MM_SEARCH_BELOW;
> - aflags = DRM_MM_CREATE_TOP;
> - }
> + mode = DRM_MM_INSERT_BEST;
> + if (place && place->flags & TTM_PL_FLAG_TOPDOWN)
> + mode = DRM_MM_INSERT_HIGH;
>  
>   spin_lock(>lock);
> - r = drm_mm_insert_node_in_range_generic(>mm, node, mem->num_pages,
> - mem->page_alignment, 0,
> - fpfn, lpfn, sflags, aflags);
> + r = drm_mm_insert_node_in_range(>mm, node,
> + mem->num_pages, mem->page_alignment, 0,
> +

Re: [Intel-gfx] [PATCH 4/4] drm: Resurrect atomic rmfb code, v2

[Sinclair Yeh]

On Thu, Jan 26, 2017 at 10:55:51AM +0100, Maarten Lankhorst wrote:
> Op 25-01-17 om 19:05 schreef Sinclair Yeh:
> > On Wed, Jan 25, 2017 at 09:36:36AM +0100, Maarten Lankhorst wrote:
> >> Op 25-01-17 om 09:09 schreef Thomas Hellstrom:
> >>> On 01/25/2017 05:54 AM, Daniel Vetter wrote:
> >>>> On Tue, Jan 24, 2017 at 01:44:54PM -0800, Matt Roper wrote:
> >>>>> On Wed, Jan 11, 2017 at 05:15:47PM +0100, Daniel Vetter wrote:
> >>>>>> On Thu, Dec 15, 2016 at 03:29:45PM +0100, Maarten Lankhorst wrote:
> >>>>>>> From: Daniel Vetter <daniel.vet...@ffwll.ch>
> >>>>>>>
> >>>>>>> This was somehow lost between v3 and the merged version in Maarten's
> >>>>>>> patch merged as:
> >>>>>>>
> >>>>>>> commit f2d580b9a8149735cbc4b59c4a8df60173658140
> >>>>>>> Author: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> >>>>>>> Date:   Wed May 4 14:38:26 2016 +0200
> >>>>>>>
> >>>>>>> drm/core: Do not preserve framebuffer on rmfb, v4.
> >>>>>>>
> >>>>>>> Actual code copied from Maarten's patch, but with the slight change to
> >>>>>>> just use dev->mode_config.funcs->atomic_commit to decide whether to
> >>>>>>> use the atomic path or not.
> >>>>>>>
> >>>>>>> v2:
> >>>>>>> - Remove plane->fb assignment, done by drm_atomic_clean_old_fb.
> >>>>>>> - Add WARN_ON when atomic_remove_fb fails.
> >>>>>>> - Always call drm_atomic_state_put.
> >>>>>>>
> >>>>>>> Signed-off-by: Daniel Vetter <daniel.vet...@intel.com>
> >>>>>>> Signed-off-by: Daniel Vetter <daniel.vet...@ffwll.ch>
> >>>>>>> Signed-off-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> >>>>>> Would be great if someone else could r-b this, I've proven pretty well
> >>>>>> that I don't understand the complexity here :(
> >>>>>> -Daniel
> >>>>> It looks like this will change the behavior slightly in that rmfb will
> >>>>> cause primary planes to be disabled, but no longer cause the entire CRTC
> >>>>> to be turned off.  You'll probably want to note that in the commit
> >>>>> message, along with the justification on why this is okay ABI-wise.
> >>>>>
> >>>>> I know that 13803132818c ("drm/core: Preserve the framebuffer after
> >>>>> removing it.") was initially trying to not only leave the CRTC on, but
> >>>>> also preserve the framebuffer and leave the planes on; that wound up
> >>>>> causing some kind of regression for vmwgfx, but I'm unclear on the
> >>>>> details there.  I'd suggest getting an Ack from one of the vmware guys
> >>>>> to ensure that the less drastic change in behavior here won't cause them
> >>>>> any problems.
> >>> The vmware Xorg driver is currently relying on rmfb to turn all attached
> >>> crtcs off. Even if we were to fix that in the Xorg driver now, older
> >>> Xorgs with newer kernels still would break.
> >> Is it allowed for vmwgfx to keep the crtc enabled, but the primary plane 
> >> disabled?
> >>
> >> If so, when vmwgfx is eventually converted to atomic then we need to 
> >> special-case rmfb for them somehow.
> > FYI, we are in the process of converting things to atomic.  This may happen
> > around 4.12
> >
> Will the driver allow the crtc to be enabled without primary plane?

Give me a few days to get back to you.  I'm reworking some patches right now.


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/4] drm: Resurrect atomic rmfb code, v2

[Sinclair Yeh]

On Wed, Jan 25, 2017 at 09:36:36AM +0100, Maarten Lankhorst wrote:
> Op 25-01-17 om 09:09 schreef Thomas Hellstrom:
> > On 01/25/2017 05:54 AM, Daniel Vetter wrote:
> >> On Tue, Jan 24, 2017 at 01:44:54PM -0800, Matt Roper wrote:
> >>> On Wed, Jan 11, 2017 at 05:15:47PM +0100, Daniel Vetter wrote:
>  On Thu, Dec 15, 2016 at 03:29:45PM +0100, Maarten Lankhorst wrote:
> > From: Daniel Vetter 
> >
> > This was somehow lost between v3 and the merged version in Maarten's
> > patch merged as:
> >
> > commit f2d580b9a8149735cbc4b59c4a8df60173658140
> > Author: Maarten Lankhorst 
> > Date:   Wed May 4 14:38:26 2016 +0200
> >
> > drm/core: Do not preserve framebuffer on rmfb, v4.
> >
> > Actual code copied from Maarten's patch, but with the slight change to
> > just use dev->mode_config.funcs->atomic_commit to decide whether to
> > use the atomic path or not.
> >
> > v2:
> > - Remove plane->fb assignment, done by drm_atomic_clean_old_fb.
> > - Add WARN_ON when atomic_remove_fb fails.
> > - Always call drm_atomic_state_put.
> >
> > Signed-off-by: Daniel Vetter 
> > Signed-off-by: Daniel Vetter 
> > Signed-off-by: Maarten Lankhorst 
>  Would be great if someone else could r-b this, I've proven pretty well
>  that I don't understand the complexity here :(
>  -Daniel
> >>> It looks like this will change the behavior slightly in that rmfb will
> >>> cause primary planes to be disabled, but no longer cause the entire CRTC
> >>> to be turned off.  You'll probably want to note that in the commit
> >>> message, along with the justification on why this is okay ABI-wise.
> >>>
> >>> I know that 13803132818c ("drm/core: Preserve the framebuffer after
> >>> removing it.") was initially trying to not only leave the CRTC on, but
> >>> also preserve the framebuffer and leave the planes on; that wound up
> >>> causing some kind of regression for vmwgfx, but I'm unclear on the
> >>> details there.  I'd suggest getting an Ack from one of the vmware guys
> >>> to ensure that the less drastic change in behavior here won't cause them
> >>> any problems.
> > The vmware Xorg driver is currently relying on rmfb to turn all attached
> > crtcs off. Even if we were to fix that in the Xorg driver now, older
> > Xorgs with newer kernels still would break.
> Is it allowed for vmwgfx to keep the crtc enabled, but the primary plane 
> disabled?
> 
> If so, when vmwgfx is eventually converted to atomic then we need to 
> special-case rmfb for them somehow.

FYI, we are in the process of converting things to atomic.  This may happen
around 4.12

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] drm: move allocation out of drm_get_format_name()

[Sinclair Yeh]

Thomas has already acked the earlier version, but in case you need
it for this one, too.

vmwgfx portion:  Acked-by: Sinclair Yeh <s...@vmware.com>

On Mon, Nov 07, 2016 at 12:48:09AM +, Eric Engestrom wrote:
> Fixes: 90844f00049e9f42573fd31d7c32e8fd31d3fd07
> 
> drm: make drm_get_format_name thread-safe
> 
> Signed-off-by: Eric Engestrom <e...@engestrom.ch>
> [danvet: Clarify that the returned pointer must be freed with
> kfree().]
> Signed-off-by: Daniel Vetter <daniel.vet...@ffwll.ch>
> 
> Cc: Rob Clark <robdcl...@gmail.com>
> Cc: Christian König <christian.koe...@amd.com>
> Suggested-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> Signed-off-by: Eric Engestrom <e...@engestrom.ch>
> ---
> 
> v2: use single-field struct instead of typedef to let the compiler
> enforce the type (Christian König)
> 
> ---
>  include/drm/drm_fourcc.h| 10 +-
>  drivers/gpu/drm/drm_fourcc.c| 14 +++--
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c  |  7 ++---
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c  |  7 ++---
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c   |  3 +-
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c   |  7 ++---
>  drivers/gpu/drm/drm_atomic.c|  7 +++--
>  drivers/gpu/drm/drm_crtc.c  |  7 +++--
>  drivers/gpu/drm/drm_framebuffer.c   |  7 +++--
>  drivers/gpu/drm/drm_modeset_helper.c|  7 +++--
>  drivers/gpu/drm/drm_plane.c |  7 +++--
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c |  7 ++---
>  drivers/gpu/drm/i915/i915_debugfs.c | 10 +++---
>  drivers/gpu/drm/i915/intel_atomic_plane.c   |  8 ++---
>  drivers/gpu/drm/i915/intel_display.c| 41 
> ++---
>  drivers/gpu/drm/radeon/atombios_crtc.c  | 14 -
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c |  3 +-
>  17 files changed, 80 insertions(+), 86 deletions(-)
> 
> diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> index dc0aafa..4b03ca0 100644
> --- a/include/drm/drm_fourcc.h
> +++ b/include/drm/drm_fourcc.h
> @@ -45,6 +45,14 @@ struct drm_format_info {
>   u8 vsub;
>  };
>  
> +/**
> + * struct drm_format_name_buf - name of a DRM format
> + * @str: string buffer containing the format name
> + */
> +struct drm_format_name_buf {
> + char str[32];
> +};
> +
>  const struct drm_format_info *__drm_format_info(u32 format);
>  const struct drm_format_info *drm_format_info(u32 format);
>  uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth);
> @@ -54,6 +62,6 @@ int drm_format_horz_chroma_subsampling(uint32_t format);
>  int drm_format_vert_chroma_subsampling(uint32_t format);
>  int drm_format_plane_width(int width, uint32_t format, int plane);
>  int drm_format_plane_height(int height, uint32_t format, int plane);
> -char *drm_get_format_name(uint32_t format) __malloc;
> +char *drm_get_format_name(uint32_t format, struct drm_format_name_buf *buf);
>  
>  #endif /* __DRM_FOURCC_H__ */
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index cbb8b77..99b0b60 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -79,17 +79,13 @@ uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t 
> depth)
>  EXPORT_SYMBOL(drm_mode_legacy_fb_format);
>  
>  /**
> - * drm_get_format_name - return a string for drm fourcc format
> + * drm_get_format_name - fill a string with a drm fourcc format's name
>   * @format: format to compute name of
> + * @buf: caller-supplied buffer
> - *
> - * Note that the buffer returned by this function is owned by the caller
> - * and will need to be freed using kfree().
>   */
> -char *drm_get_format_name(uint32_t format)
> +char *drm_get_format_name(uint32_t format, struct drm_format_name_buf *buf)
>  {
> - char *buf = kmalloc(32, GFP_KERNEL);
> -
> - snprintf(buf, 32,
> + snprintf(buf->str, sizeof(buf->str),
>"%c%c%c%c %s-endian (0x%08x)",
>printable_char(format & 0xff),
>printable_char((format >> 8) & 0xff),
> @@ -98,7 +94,7 @@ char *drm_get_format_name(uint32_t format)
>format & DRM_FORMAT_BIG_ENDIAN ? "big" : "little",
>format);
>  
> - return buf;
> + return buf->str;
>  }
>  EXPORT_SYMBOL(drm_get_format_name);
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c 
> b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> index 199d3f7..2924cdd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce

Re: [Intel-gfx] [PATCH 05/11] drm/vmwgfx: Remove call to reservation_object_test_signaled_rcu before wait

[Sinclair Yeh]

Hi,

I'm preparing a fixes pull request, and I'll include this one if it
hasn't been applied by others already.

Sinclair


On Wed, Oct 05, 2016 at 09:41:22PM +0530, Sumit Semwal wrote:
> Hi Thomas, Sinclair,
> 
> On 23 September 2016 at 18:26, Daniel Vetter <dan...@ffwll.ch> wrote:
> > On Mon, Aug 29, 2016 at 08:08:28AM +0100, Chris Wilson wrote:
> >> Since fence_wait_timeout_reservation_object_wait_timeout_rcu() with a
> >> timeout of 0 becomes reservation_object_test_signaled_rcu(), we do not
> >> need to handle such conversion in the caller. The only challenge are
> >> those callers that wish to differentiate the error code between the
> >> nonblocking busy check and potentially blocking wait.
> >>
> >> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> >> Cc: Sinclair Yeh <s...@vmware.com>
> >> Cc: Thomas Hellstrom <thellst...@vmware.com>
> >> Reviewed-by: Sinclair Yeh <s...@vmware.com>
> >
> > Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch>
> >
> Could you please let me know if this patch is already queued up at
> your end, or should I just take it via drm-misc with Sinclair's r-b?
> 
> Thanks and Best,
> Sumit.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] drm/core: Change declaration for gamma_set.

[Sinclair Yeh]

Acked-by: Sinclair Yeh <s...@vmware.com>

On Tue, Jun 07, 2016 at 12:49:30PM +0200, Maarten Lankhorst wrote:
> Change return value to int to propagate errors from gamma_set,
> and remove start parameter. Updates always use the full size,
> and some drivers even ignore the start parameter altogether.
> 
> This is needed for atomic drivers, where an atomic commit can
> fail with -EINTR or -ENOMEM and should be restarted.
> 
> Changes since v1:
> - Fix compiler warning. (Emil)
> - Fix commit message (Daniel)
> 
> Cc: Alex Deucher <alexander.deuc...@amd.com>
> Acked-by: Alex Deucher <alexander.deuc...@amd.com>
> Cc: Christian König <christian.koe...@amd.com>
> Cc: David Airlie <airl...@linux.ie>
> Cc: Patrik Jakobsson <patrik.r.jakobs...@gmail.com>
> Cc: Ben Skeggs <bske...@redhat.com>
> Cc: Eric Anholt <e...@anholt.net>
> Cc: VMware Graphics <linux-graphics-maintai...@vmware.com>
> Cc: Mathieu Larouche <mathieu.larou...@matrox.com>
> Cc: Thierry Reding <tred...@nvidia.com>
> Signed-off-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c  | 10 ++
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c  | 10 ++
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c   | 10 ++
>  drivers/gpu/drm/ast/ast_mode.c  | 10 ++
>  drivers/gpu/drm/cirrus/cirrus_mode.c|  8 +---
>  drivers/gpu/drm/drm_atomic_helper.c | 13 ++---
>  drivers/gpu/drm/drm_crtc.c  |  2 +-
>  drivers/gpu/drm/drm_fb_helper.c |  2 +-
>  drivers/gpu/drm/gma500/gma_display.c|  9 +
>  drivers/gpu/drm/gma500/gma_display.h|  4 ++--
>  drivers/gpu/drm/mgag200/mgag200_mode.c  |  9 +
>  drivers/gpu/drm/nouveau/dispnv04/crtc.c | 12 +++-
>  drivers/gpu/drm/nouveau/nv50_display.c  |  9 +
>  drivers/gpu/drm/radeon/radeon_display.c | 11 +++
>  drivers/gpu/drm/vc4/vc4_crtc.c  |  8 +---
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c |  8 +---
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.h |  4 ++--
>  include/drm/drm_atomic_helper.h |  6 +++---
>  include/drm/drm_crtc.h  |  4 ++--
>  19 files changed, 85 insertions(+), 64 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c 
> b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> index 7e57447bf5f4..112e358f0f9b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> @@ -2667,19 +2667,21 @@ static void dce_v10_0_cursor_reset(struct drm_crtc 
> *crtc)
>   }
>  }
>  
> -static void dce_v10_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 
> *green,
> - u16 *blue, uint32_t start, uint32_t size)
> +static int dce_v10_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 
> *green,
> + u16 *blue, uint32_t size)
>  {
>   struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
> - int end = (start + size > 256) ? 256 : start + size, i;
> + int i;
>  
>   /* userspace palettes are always correct as is */
> - for (i = start; i < end; i++) {
> + for (i = 0; i < size; i++) {
>   amdgpu_crtc->lut_r[i] = red[i] >> 6;
>   amdgpu_crtc->lut_g[i] = green[i] >> 6;
>   amdgpu_crtc->lut_b[i] = blue[i] >> 6;
>   }
>   dce_v10_0_crtc_load_lut(crtc);
> +
> + return 0;
>  }
>  
>  static void dce_v10_0_crtc_destroy(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c 
> b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> index da101551960b..b522fa2435a8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> @@ -2678,19 +2678,21 @@ static void dce_v11_0_cursor_reset(struct drm_crtc 
> *crtc)
>   }
>  }
>  
> -static void dce_v11_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 
> *green,
> - u16 *blue, uint32_t start, uint32_t size)
> +static int dce_v11_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 
> *green,
> + u16 *blue, uint32_t size)
>  {
>   struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
> - int end = (start + size > 256) ? 256 : start + size, i;
> + int i;
>  
>   /* userspace palettes are always correct as is */
> - for (i = start; i < end; i++) {
> + for (i = 0; i < size; i++) {
>   amdgpu_crtc->lut_r[i] = red[i] >> 6;
>   amdgpu_crtc->lut_g[i] = green[i] >> 6;
>   amdgpu_crtc->lut_b[i] = blue[i]

[Intel-gfx] [PATCH] drm/i915: Revert workaround for disabling L3 cache aging on BYT

[Sinclair Yeh]

V2:  edit the commit message to contain more info
The W/A spreadsheet says this is still required, but the b-spec says
it's not for BYT-T.  So the documentation is not clear.  However,
our experience with the other SKUs of BYT-I/M on Android and Linux
suggests that setting this bit actually causes GPU hang for certain
OGL benchmark applications.

Removing this bit completely resolves the GPU hangs.
---
 drivers/gpu/drm/i915/intel_pm.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a6b877a..3ba037e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5004,9 +5004,6 @@ static void valleyview_init_clock_gating(struct 
drm_device *dev)
   _MASKED_BIT_ENABLE(GEN7_MAX_PS_THREAD_DEP |
  GEN7_PSD_SINGLE_PORT_DISPATCH_ENABLE));
 
-   /* WaDisableL3CacheAging:vlv */
-   I915_WRITE(GEN7_L3CNTLREG1, I915_READ(GEN7_L3CNTLREG1) | GEN7_L3AGDIS);
-
/* WaForceL3Serialization:vlv */
I915_WRITE(GEN7_L3SQCREG4, I915_READ(GEN7_L3SQCREG4) 
   ~L3SQ_URB_READ_CAM_MATCH_DISABLE);
-- 
1.8.1.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] Fix GPU Hang on BYT

[Sinclair Yeh]

On certain OpenGL benchmark applications, setting this bit results in
frequent GPU hangs.  On BYT-M/I, the hangs go away by not setting this
bit.
---
 drivers/gpu/drm/i915/intel_pm.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a6b877a..3ba037e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5004,9 +5004,6 @@ static void valleyview_init_clock_gating(struct 
drm_device *dev)
   _MASKED_BIT_ENABLE(GEN7_MAX_PS_THREAD_DEP |
  GEN7_PSD_SINGLE_PORT_DISPATCH_ENABLE));
 
-   /* WaDisableL3CacheAging:vlv */
-   I915_WRITE(GEN7_L3CNTLREG1, I915_READ(GEN7_L3CNTLREG1) | GEN7_L3AGDIS);
-
/* WaForceL3Serialization:vlv */
I915_WRITE(GEN7_L3SQCREG4, I915_READ(GEN7_L3SQCREG4) 
   ~L3SQ_URB_READ_CAM_MATCH_DISABLE);
-- 
1.8.1.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx