Re: [PATCH v2 1/2] drm/ttm: Change ttm_device_init to use a struct instead of multiple bools

2024-10-02 Thread Zack Rusin
On Wed, Oct 2, 2024 at 8:24 AM Thomas Hellström
 wrote:
>
> The ttm_device_init funcition uses multiple bool arguments. That means
> readability in the caller becomes poor, and all callers need to change if
> yet another bool is added.
>
> Instead use a struct with multiple single-bit flags. This addresses both
> problems. Prefer it over using defines or enums with explicit bit shifts,
> since converting to and from these bit values uses logical operations or
> tests which are implicit with the struct usage, and ofc type-checking.
>
> This is in preparation of adding yet another bool flag parameter to the
> function.
>
> Cc: Christian König 
> Cc: amd-...@lists.freedesktop.org
> Cc: intel-gfx@lists.freedesktop.org
> Cc: nouv...@lists.freedesktop.org
> Cc: spice-de...@lists.freedesktop.org
> Cc: dri-de...@lists.freedesktop.org
> Cc: Zack Rusin 
> Cc: 
> Cc: Sui Jingfeng 
> Cc: 
> Signed-off-by: Thomas Hellström 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c   |  6 --
>  drivers/gpu/drm/drm_gem_vram_helper.c |  7 ---
>  drivers/gpu/drm/i915/intel_region_ttm.c   |  3 ++-
>  drivers/gpu/drm/loongson/lsdc_ttm.c   |  5 -
>  drivers/gpu/drm/nouveau/nouveau_ttm.c |  7 +--
>  drivers/gpu/drm/qxl/qxl_ttm.c |  2 +-
>  drivers/gpu/drm/radeon/radeon_ttm.c   |  6 --
>  drivers/gpu/drm/ttm/tests/ttm_bo_test.c   | 16 +++
>  .../gpu/drm/ttm/tests/ttm_bo_validate_test.c  |  3 ++-
>  drivers/gpu/drm/ttm/tests/ttm_device_test.c   | 16 ---
>  drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c | 20 ---
>  drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h |  6 ++
>  drivers/gpu/drm/ttm/ttm_device.c  |  7 +++
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c   |  4 ++--
>  drivers/gpu/drm/xe/xe_device.c|  3 ++-
>  include/drm/ttm/ttm_device.h  | 12 ++-
>  16 files changed, 71 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 74adb983ab03..e43635ac54fd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1853,8 +1853,10 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
> r = ttm_device_init(&adev->mman.bdev, &amdgpu_bo_driver, adev->dev,
>adev_to_drm(adev)->anon_inode->i_mapping,
>adev_to_drm(adev)->vma_offset_manager,
> -  adev->need_swiotlb,
> -  dma_addressing_limited(adev->dev));
> +  (struct ttm_device_init_flags){
> +  .use_dma_alloc = adev->need_swiotlb,
> +  .use_dma32 = 
> dma_addressing_limited(adev->dev)
> +  });
> if (r) {
> DRM_ERROR("failed initializing buffer object driver(%d).\n", 
> r);
> return r;
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c 
> b/drivers/gpu/drm/drm_gem_vram_helper.c
> index 22b1fe9c03b8..7c3165b00378 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -931,9 +931,10 @@ static int drm_vram_mm_init(struct drm_vram_mm *vmm, 
> struct drm_device *dev,
> vmm->vram_size = vram_size;
>
> ret = ttm_device_init(&vmm->bdev, &bo_driver, dev->dev,
> -dev->anon_inode->i_mapping,
> -dev->vma_offset_manager,
> -false, true);
> + dev->anon_inode->i_mapping,
> + dev->vma_offset_manager,
> + (struct ttm_device_init_flags)
> + {.use_dma32 = true});
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/i915/intel_region_ttm.c 
> b/drivers/gpu/drm/i915/intel_region_ttm.c
> index 04525d92bec5..db34da63814c 100644
> --- a/drivers/gpu/drm/i915/intel_region_ttm.c
> +++ b/drivers/gpu/drm/i915/intel_region_ttm.c
> @@ -34,7 +34,8 @@ int intel_region_ttm_device_init(struct drm_i915_private 
> *dev_priv)
>
> return ttm_device_init(&dev_priv->bdev, i915_ttm_driver(),
>drm->dev, drm->anon_inode->i_mapping,
> -  drm->vma_offset_manager, false, false);
> +  drm->vma_offset_manager,
> +  (struct ttm_device_in

Re: [PATCH v4 71/80] drm/vmwgfx: Run DRM default client setup

2024-09-09 Thread Zack Rusin
On Mon, Sep 9, 2024 at 7:37 AM Thomas Zimmermann  wrote:
>
> Call drm_client_setup() to run the kernel's default client setup
> for DRM. Set fbdev_probe in struct drm_driver, so that the client
> setup can start the common fbdev client.
>
> Signed-off-by: Thomas Zimmermann 
> Cc: Zack Rusin 
> Cc: Broadcom internal kernel review list 
> 
> Acked-by: Javier Martinez Canillas 

Quick note: I love what you did with drm client and drm fbdev. Thanks
a lot for that work!

Reviewed-by: Zack Rusin 

z


Re: Rework TTMs busy handling

2024-01-16 Thread Zack Rusin
On Tue, Jan 16, 2024 at 4:57 AM Christian König
 wrote:
>
> Am 12.01.24 um 13:51 schrieb Christian König:
> > Hi guys,
>
> just a gentle ping on this.
>
> Zack any more comments for the VMWGFX parts?

The new vmwgfx code looks great, thanks a lot for implementing it! In
fact the entire series looks good to me. For the series:

Reviewed-by: Zack Rusin 

z


Re: [PATCH 3/5] drm/ttm: replace busy placement with flags v5

2024-01-09 Thread Zack Rusin
[i];
> struct ttm_resource_manager *man;
>
> +   if (place->flags & TTM_PL_FLAG_BUSY)
> +   continue;
> +
> man = ttm_manager_type(bdev, place->mem_type);
> if (!man || !ttm_resource_manager_used(man))
> continue;
> @@ -813,10 +815,13 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
> return 0;
> }
>
> -   for (i = 0; i < placement->num_busy_placement; ++i) {
> -   const struct ttm_place *place = &placement->busy_placement[i];
> +   for (i = 0; i < placement->num_placement; ++i) {
> +   const struct ttm_place *place = &placement->placement[i];
> struct ttm_resource_manager *man;
>
> +   if (place->flags & TTM_PL_FLAG_IDLE)
> +   continue;
> +
> man = ttm_manager_type(bdev, place->mem_type);
> if (!man || !ttm_resource_manager_used(man))
> continue;
> @@ -904,11 +909,11 @@ int ttm_bo_validate(struct ttm_buffer_object *bo,
> /*
>  * Remove the backing store if no placement is given.
>  */
> -   if (!placement->num_placement && !placement->num_busy_placement)
> +   if (!placement->num_placement)
> return ttm_bo_pipeline_gutting(bo);
>
> /* Check whether we need to move buffer. */
> -   if (bo->resource && ttm_resource_compat(bo->resource, placement))
> +   if (bo->resource && ttm_resource_compatible(bo->resource, placement))
> return 0;
>
> /* Moving of pinned BOs is forbidden */
> diff --git a/drivers/gpu/drm/ttm/ttm_resource.c 
> b/drivers/gpu/drm/ttm/ttm_resource.c
> index 02b96d23fdb9..fb14f7716cf8 100644
> --- a/drivers/gpu/drm/ttm/ttm_resource.c
> +++ b/drivers/gpu/drm/ttm/ttm_resource.c
> @@ -291,37 +291,15 @@ bool ttm_resource_intersects(struct ttm_device *bdev,
>  }
>
>  /**
> - * ttm_resource_compatible - test for compatibility
> + * ttm_resource_compatible - check if resource is compatible with placement
>   *
> - * @bdev: TTM device structure
> - * @res: The resource to test
> - * @place: The placement to test
> - * @size: How many bytes the new allocation needs.
> - *
> - * Test if @res compatible with @place and @size.
> + * @res: the resource to check
> + * @placement: the placement to check against
>   *
> - * Returns true if the res placement compatible with @place and @size.
> + * Returns true if the placement is compatible.
>   */
> -bool ttm_resource_compatible(struct ttm_device *bdev,
> -struct ttm_resource *res,
> -const struct ttm_place *place,
> -size_t size)
> -{
> -   struct ttm_resource_manager *man;
> -
> -   if (!res || !place)
> -   return false;
> -
> -   man = ttm_manager_type(bdev, res->mem_type);
> -   if (!man->func->compatible)
> -   return true;
> -
> -   return man->func->compatible(man, res, place, size);
> -}
> -
> -static bool ttm_resource_places_compat(struct ttm_resource *res,
> -  const struct ttm_place *places,
> -  unsigned num_placement)
> +bool ttm_resource_compatible(struct ttm_resource *res,
> +struct ttm_placement *placement)
>  {
> struct ttm_buffer_object *bo = res->bo;
> struct ttm_device *bdev = bo->bdev;
> @@ -330,44 +308,25 @@ static bool ttm_resource_places_compat(struct 
> ttm_resource *res,
> if (res->placement & TTM_PL_FLAG_TEMPORARY)
> return false;
>
> -   for (i = 0; i < num_placement; i++) {
> -   const struct ttm_place *heap = &places[i];
> +   for (i = 0; i < placement->num_placement; i++) {
> +   const struct ttm_place *place = &placement->placement[i];
> +   struct ttm_resource_manager *man;
>
> -   if (!ttm_resource_compatible(bdev, res, heap, bo->base.size))
> +   if (res->mem_type != place->mem_type)
> +   continue;
> +
> +   man = ttm_manager_type(bdev, res->mem_type);
> +   if (man->func->compatible &&
> +   !man->func->compatible(man, res, place, bo->base.size))
> continue;
>
> -   if ((res->mem_type == heap->mem_type) &&
> -   (!(heap->flags & TTM_PL_FLAG_CONTIGUOUS) ||
> +   if ((!(place->flags & TTM_PL_FLAG_CONTIGUOUS) ||
>  (res->placement & TTM_PL_FLAG_CONTIGUOUS)))
> return true;
> }
> return false;
>  }
>
> -/**
> - * ttm_resource_compat - check if resource is compatible with placement
> - *
> - * @res: the resource to check
> - * @placement: the placement to check against
> - *
> - * Returns true if the placement is compatible.
> - */
> -bool ttm_resource_compat(struct ttm_resource *res,
> -struct ttm_placement *placement)
> -{
> -   if (ttm_resource_places_compat(res, placement->placement,
> -  placement->num_placement))
> -   return true;
> -
> -   if ((placement->busy_placement != placement->placement ||
> -placement->num_busy_placement > placement->num_placement) &&
> -   ttm_resource_places_compat(res, placement->busy_placement,
> -  placement->num_busy_placement))
> -   return true;
> -
> -   return false;
> -}
> -
>  void ttm_resource_set_bo(struct ttm_resource *res,
>  struct ttm_buffer_object *bo)
>  {
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> index 2bfac3aad7b7..7d7b33fcb5cf 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> @@ -821,8 +821,6 @@ void vmw_bo_placement_set(struct vmw_bo *bo, u32 domain, 
> u32 busy_domain)
>  __func__, bo->tbo.resource->mem_type, 
> domain);
> }
>
> -   pl->busy_placement = bo->busy_places;
> -   pl->num_busy_placement = set_placement_list(bo->busy_places, 
> busy_domain);
>  }

Sorry, one last thing. Could you add the exact same code you've added
to nouveau_bo.c here or add a fixme mentioning that it should be done?

With that, for the series:
Reviewed-by: Zack Rusin 

z


Re: [PATCH 2/4] drm/ttm: replace busy placement with flags v4

2024-01-04 Thread Zack Rusin
On Thu, Jan 4, 2024 at 10:05 AM Christian König
 wrote:
>
> From: Somalapuram Amaranath 
>
> Instead of a list of separate busy placement add flags which indicate
> that a placement should only be used when there is room or if we need to
> evict.
>
> v2: add missing TTM_PL_FLAG_IDLE for i915
> v3: fix auto build test ERROR on drm-tip/drm-tip
> v4: fix some typos pointed out by checkpatch
>
> Signed-off-by: Christian König 
> Signed-off-by: Somalapuram Amaranath 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  6 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 11 +--
>  drivers/gpu/drm/drm_gem_vram_helper.c  |  2 -
>  drivers/gpu/drm/i915/gem/i915_gem_ttm.c| 37 
>  drivers/gpu/drm/loongson/lsdc_ttm.c|  2 -
>  drivers/gpu/drm/nouveau/nouveau_bo.c   | 59 +
>  drivers/gpu/drm/nouveau/nouveau_bo.h   |  1 -
>  drivers/gpu/drm/qxl/qxl_object.c   |  2 -
>  drivers/gpu/drm/qxl/qxl_ttm.c  |  2 -
>  drivers/gpu/drm/radeon/radeon_object.c |  2 -
>  drivers/gpu/drm/radeon/radeon_ttm.c|  8 +-
>  drivers/gpu/drm/radeon/radeon_uvd.c|  1 -
>  drivers/gpu/drm/ttm/ttm_bo.c   | 21 +++--
>  drivers/gpu/drm/ttm/ttm_resource.c | 73 
>  drivers/gpu/drm/vmwgfx/vmwgfx_bo.c |  2 -
>  drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 99 +-
>  include/drm/ttm/ttm_placement.h| 10 ++-
>  include/drm/ttm/ttm_resource.h |  8 +-
>  18 files changed, 159 insertions(+), 187 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index cef920a93924..aa0dd6dad068 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -220,9 +220,6 @@ void amdgpu_bo_placement_from_domain(struct amdgpu_bo 
> *abo, u32 domain)
>
> placement->num_placement = c;
> placement->placement = places;
> -
> -   placement->num_busy_placement = c;
> -   placement->busy_placement = places;
>  }
>
>  /**
> @@ -1406,8 +1403,7 @@ vm_fault_t amdgpu_bo_fault_reserve_notify(struct 
> ttm_buffer_object *bo)
> AMDGPU_GEM_DOMAIN_GTT);
>
> /* Avoid costly evictions; only set GTT as a busy placement */
> -   abo->placement.num_busy_placement = 1;
> -   abo->placement.busy_placement = &abo->placements[1];
> +   abo->placements[0].flags |= TTM_PL_FLAG_IDLE;
>
> r = ttm_bo_validate(bo, &abo->placement, &ctx);
> if (unlikely(r == -EBUSY || r == -ERESTARTSYS))
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 05991c5c8ddb..9a6a00b1af40 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -102,23 +102,19 @@ static void amdgpu_evict_flags(struct ttm_buffer_object 
> *bo,
> /* Don't handle scatter gather BOs */
> if (bo->type == ttm_bo_type_sg) {
> placement->num_placement = 0;
> -   placement->num_busy_placement = 0;
> return;
> }
>
> /* Object isn't an AMDGPU object so ignore */
> if (!amdgpu_bo_is_amdgpu_bo(bo)) {
> placement->placement = &placements;
> -   placement->busy_placement = &placements;
> placement->num_placement = 1;
> -   placement->num_busy_placement = 1;
> return;
> }
>
> abo = ttm_to_amdgpu_bo(bo);
> if (abo->flags & AMDGPU_GEM_CREATE_DISCARDABLE) {
> placement->num_placement = 0;
> -   placement->num_busy_placement = 0;
> return;
> }
>
> @@ -128,13 +124,13 @@ static void amdgpu_evict_flags(struct ttm_buffer_object 
> *bo,
> case AMDGPU_PL_OA:
> case AMDGPU_PL_DOORBELL:
> placement->num_placement = 0;
> -   placement->num_busy_placement = 0;
> return;
>
> case TTM_PL_VRAM:
> if (!adev->mman.buffer_funcs_enabled) {
> /* Move to system memory */
> amdgpu_bo_placement_from_domain(abo, 
> AMDGPU_GEM_DOMAIN_CPU);
> +
> } else if (!amdgpu_gmc_vram_full_visible(&adev->gmc) &&
>!(abo->flags & 
> AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) &&
>amdgpu_bo_in_cpu_visible_vram(abo)) {
> @@ -149,8 +145,7 @@ static void amdgpu_evict_flags(struct ttm_buffer_object 
> *bo,
> 
> AMDGPU_GEM_DOMAIN_CPU);
> abo->placements[0].fpfn = adev->gmc.visible_vram_size 
> >> PAGE_SHIFT;
> abo->placements[0].lpfn = 0;
> -   abo->placement.busy_placement = &abo->placements[1];
> -   abo->placement.num_busy_placement = 1;
> + 

Re: [Intel-gfx] [PATCH 8/9] drm/vmwgfx: Annotate struct vmw_surface_dirty with __counted_by

2023-09-22 Thread Zack Rusin
On Fri, 2023-09-22 at 10:32 -0700, Kees Cook wrote:
> Prepare for the coming implementation by GCC and Clang of the __counted_by
> attribute. Flexible array members annotated with __counted_by can have
> their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
> (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
> functions).
> 
> As found with Coccinelle[1], add __counted_by for struct vmw_surface_dirty.
> 
> [1]
> https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
> 
> Cc: Zack Rusin 
> Cc: VMware Graphics Reviewers 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: dri-de...@lists.freedesktop.org
> Signed-off-by: Kees Cook 
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> index 5db403ee8261..2d1d857f99ae 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> @@ -77,7 +77,7 @@ struct vmw_surface_offset {
>  struct vmw_surface_dirty {
> struct vmw_surface_cache cache;
> u32 num_subres;
> -   SVGA3dBox boxes[];
> +   SVGA3dBox boxes[] __counted_by(num_subres);
>  };
>  
>  static void vmw_user_surface_free(struct vmw_resource *res);

Thanks!

Reviewed-by: Zack Rusin 


Re: [Intel-gfx] [PATCH 4/5] drm/vmwgfx: Remove redundant framebuffer format check

2023-01-11 Thread Zack Rusin
On Mon, 2023-01-09 at 07:58 -0300, Maíra Canal wrote:
> Now that framebuffer_check() verifies that the format is properly
> supported, there is no need to check it again on vmwgfx's inside
> helpers.
> 
> Therefore, remove the redundant framebuffer format check from the
> vmw_kms_new_framebuffer_surface() and vmw_kms_new_framebuffer_bo()
> functions, letting framebuffer_check() perform the framebuffer
> validation.
> 
> Signed-off-by: Maíra Canal 
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 18 --
>  1 file changed, 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 257f090071f1..05b8d8f912bf 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -1317,15 +1317,6 @@ static int vmw_kms_new_framebuffer_surface(struct
> vmw_private *dev_priv,
>  * Sanity checks.
>  */
>  
> -   if (!drm_any_plane_has_format(&dev_priv->drm,
> - mode_cmd->pixel_format,
> - mode_cmd->modifier[0])) {
> -   drm_dbg(&dev_priv->drm,
> -   "unsupported pixel format %p4cc / modifier 0x%llx\n",
> -   &mode_cmd->pixel_format, mode_cmd->modifier[0]);
> -   return -EINVAL;
> -   }
> -
> /* Surface must be marked as a scanout. */
> if (unlikely(!surface->metadata.scanout))
> return -EINVAL;
> @@ -1648,15 +1639,6 @@ static int vmw_kms_new_framebuffer_bo(struct 
> vmw_private
> *dev_priv,
> return -EINVAL;
> }
>  
> -   if (!drm_any_plane_has_format(&dev_priv->drm,
> - mode_cmd->pixel_format,
> - mode_cmd->modifier[0])) {
> -   drm_dbg(&dev_priv->drm,
> -   "unsupported pixel format %p4cc / modifier 0x%llx\n",
> -   &mode_cmd->pixel_format, mode_cmd->modifier[0]);
> -   return -EINVAL;
> -   }
> -
> vfbd = kzalloc(sizeof(*vfbd), GFP_KERNEL);
> if (!vfbd) {
> ret = -ENOMEM;

That's a nice cleanup. Thanks.

Reviewed-by: Zack Rusin 


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

2022-10-20 Thread Zack Rusin
On Fri, 2022-10-21 at 11:02 +1100, 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/nouveau/nouveau_display.c: In function 
> 'nouveau_display_create':
> drivers/gpu/drm/nouveau/nouveau_display.c:662:29: error: unused variable 
> 'device' [-Werror=unused-variable]
>   662 | struct nvkm_device *device = nvxx_device(&drm->client.device);
>   | ^~
> cc1: all warnings being treated as errors
> 
> Introduced by commit
> 
>   7c99616e3fe7 ("drm: Remove drm_mode_config::fb_base")
> 
> I have used the drm-misc tree from next-20221020 for today.
> 

Hi, Stephen.

I've just sent out a trivial fix for this. I'm not sure how the bots and I 
missed
it. Thanks for letting me know!

z


Re: [Intel-gfx] [PATCH v2 1/4] drm/vmwgfx: Stop using 'TRUE'

2022-07-05 Thread Zack Rusin

On Jun 30, 2022, at 3:51 PM, Ville Syrjala 
mailto:ville.syrj...@linux.intel.com>> wrote:

From: Ville Syrjälä 
mailto:ville.syrj...@linux.intel.com>>

Stop using the 'TRUE' define. This ultimately gets defined by
acpi/actypes.h that gets included here via a convoluted chain of
other headers. drm_crtc.h is part of that chain, and I'm trying
to eliminate all unnecessary includes from it to avoid pointless
rebuilds.

v2: Split out from the bigger patch

Cc: Zack Rusin mailto:za...@vmware.com>>
Cc: VMware Graphics Reviewers 
mailto:linux-graphics-maintai...@vmware.com>>
Acked-by: Sam Ravnborg mailto:s...@ravnborg.org>>
Acked-by: Jani Nikula mailto:jani.nik...@intel.com>>
Signed-off-by: Ville Syrjälä 
mailto:ville.syrj...@linux.intel.com>>
---
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 693028c31b6b..ff2f735bbe7a 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -252,7 +252,7 @@ static void vmw_cursor_update_position(struct vmw_private 
*dev_priv,
vmw_write(dev_priv, SVGA_REG_CURSOR4_Y, y);
vmw_write(dev_priv, SVGA_REG_CURSOR4_SCREEN_ID, SVGA3D_INVALID_ID);
vmw_write(dev_priv, SVGA_REG_CURSOR4_ON, svga_cursor_on);
- vmw_write(dev_priv, SVGA_REG_CURSOR4_SUBMIT, TRUE);
+ vmw_write(dev_priv, SVGA_REG_CURSOR4_SUBMIT, 1);
} else if (vmw_is_cursor_bypass3_enabled(dev_priv)) {
vmw_fifo_mem_write(dev_priv, SVGA_FIFO_CURSOR_ON, svga_cursor_on);
vmw_fifo_mem_write(dev_priv, SVGA_FIFO_CURSOR_X, x);

Hi, Ville.

Sorry for the delay. Looks great. In case you haven’t pushed it to 
drm-misc-next yet:

Reviewed-by: Zack Rusin mailto:za...@vmware.com>>

z


Re: [Intel-gfx] [PATCH 03/15] dma-buf & drm/amdgpu: remove dma_resv workaround

2022-04-21 Thread Zack Rusin
On Thu, 2022-04-21 at 12:17 +0200, Christian König wrote:
> ⚠ External Email
> 
> Am 20.04.22 um 21:28 schrieb Zack Rusin:
> > [SNIP]
> > > To figure out what it is could you try the following code
> > > fragment:
> > > 
> > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_validation.c
> > > b/drivers/gpu/drm/vmwgfx/vmwgfx_validation.c
> > > index f46891012be3..a36f89d3f36d 100644
> > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_validation.c
> > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_validation.c
> > > @@ -288,7 +288,7 @@ int vmw_validation_add_bo(struct
> > > vmw_validation_context *ctx,
> > >   val_buf->bo = ttm_bo_get_unless_zero(&vbo-
> > > >base);
> > >   if (!val_buf->bo)
> > >   return -ESRCH;
> > > -   val_buf->num_shared = 0;
> > > +   val_buf->num_shared = 16;
> > >   list_add_tail(&val_buf->head, &ctx->bo_list);
> > >   bo_node->as_mob = as_mob;
> > >   bo_node->cpu_blit = cpu_blit;
> > Fails the same BUG_ON with num_fences and max_fences == 0.
> 
> Thanks for testing this.
> 
> So the buffer object is not reserved through
> vmw_validation_bo_reserve(), but comes from somewhere else.
> Unfortunately I absolutely can't find where that's coming from.
> 
> Do you have some documentation howto setup vmwgfx? E.g. sample VM
> which
> I can download somewhere etc..

I don't have an external machine to upload it to. Getting an external
machine to run Mesa CI on has been on our todo for a while, so I'll try
to setup something next week.

The issue here seems to be that vmwgfx always had some buffers that
didn't immediately go through vmw_validation_bo_reserve. What's
happening is that in vmwgfx_execbuf.c in vmw_execbuf_process we call
vmw_validation_bo_reserve and after it we call
vmw_validation_res_validate. Inside vmw_validation_res_validate (in
vmwgfx_validation.c) we call vmw_resource_validate, which calls
vmw_resource_do_validate . vmw_resource_do_validate has this code "ret
= func->create(res);" which is an issue for vmwgfx_cotable.c . The
func->create for cotable's is vmw_cotable_create which calls
vmw_cotable_resize which creates, reserves and validates a new bo.

In short a new bo is created in vmw_cotable_resize between
ttm_eu_reserve_buffers and ttm_eu_fence_buffer_objects calls.

z


Re: [Intel-gfx] [PATCH 03/15] dma-buf & drm/amdgpu: remove dma_resv workaround

2022-04-20 Thread Zack Rusin
On Wed, 2022-04-20 at 20:56 +0200, Christian König wrote:
> ⚠ External Email
> 
> Am 20.04.22 um 20:49 schrieb Christian König:
> > Am 20.04.22 um 20:41 schrieb Zack Rusin:
> > > On Wed, 2022-04-20 at 19:40 +0200, Christian König wrote:
> > > > Am 20.04.22 um 19:38 schrieb Zack Rusin:
> > > > > On Wed, 2022-04-20 at 09:37 +0200, Christian König wrote:
> > > > > > ⚠ External Email
> > > > > > 
> > > > > > Hi Zack,
> > > > > > 
> > > > > > Am 20.04.22 um 05:56 schrieb Zack Rusin:
> > > > > > > On Thu, 2022-04-07 at 10:59 +0200, Christian König wrote:
> > > > > > > > Rework the internals of the dma_resv object to allow
> > > > > > > > adding
> > > > > > > > more
> > > > > > > > than
> > > > > > > > one
> > > > > > > > write fence and remember for each fence what purpose it
> > > > > > > > had.
> > > > > > > > 
> > > > > > > > This allows removing the workaround from amdgpu which
> > > > > > > > used a
> > > > > > > > container
> > > > > > > > for
> > > > > > > > this instead.
> > > > > > > > 
> > > > > > > > Signed-off-by: Christian König
> > > > > > > > 
> > > > > > > > Reviewed-by: Daniel Vetter 
> > > > > > > > Cc: amd-...@lists.freedesktop.org
> > > > > > > afaict this change broke vmwgfx which now kernel oops
> > > > > > > right
> > > > > > > after
> > > > > > > boot.
> > > > > > > I haven't had the time to look into it yet, so I'm not
> > > > > > > sure
> > > > > > > what's
> > > > > > > the
> > > > > > > problem. I'll look at this tomorrow, but just in case you
> > > > > > > have
> > > > > > > some
> > > > > > > clues, the backtrace follows:
> > > > > > that's a known issue and should already be fixed with:
> > > > > > 
> > > > > > commit d72dcbe9fce505228dae43bef9da8f2b707d1b3d
> > > > > > Author: Christian König 
> > > > > > Date:   Mon Apr 11 15:21:59 2022 +0200
> > > > > Unfortunately that doesn't seem to be it. The backtrace is
> > > > > from the
> > > > > current (as of the time of sending of this email) drm-misc-
> > > > > next,
> > > > > which
> > > > > has this change, so it's something else.
> > > > Ok, that's strange. In this case I need to investigate further.
> > > > 
> > > > Maybe VMWGFX is adding more than one fence and we actually need
> > > > to
> > > > reserve multiple slots.
> > > This might be helper code issue with CONFIG_DEBUG_MUTEXES set. On
> > > that config
> > > dma_resv_reset_max_fences does:
> > >     fences->max_fences = fences->num_fences;
> > > For some objects num_fences is 0 and so after max_fences and
> > > num_fences are both 0.
> > > And then BUG_ON(num_fences >= max_fences) is triggered.
> > 
> > Yeah, but that's expected behavior.
> > 
> > What's not expected is that max_fences is still 0 (or equal to old
> > num_fences) when VMWGFX tries to add a new fence. The function
> > ttm_eu_reserve_buffers() should have reserved at least one fence
> > slot.
> > 
> > So the underlying problem is that either ttm_eu_reserve_buffers()
> > was
> > never called or VMWGFX tried to add more than one fence.
> 
> 
> To figure out what it is could you try the following code fragment:
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_validation.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_validation.c
> index f46891012be3..a36f89d3f36d 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_validation.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_validation.c
> @@ -288,7 +288,7 @@ int vmw_validation_add_bo(struct
> vmw_validation_context *ctx,
>  val_buf->bo = ttm_bo_get_unless_zero(&vbo->base);
>  if (!val_buf->bo)
>  return -ESRCH;
> -   val_buf->num_shared = 0;
> +   val_buf->num_shared = 16;
>  list_add_tail(&val_buf->head, &ctx->bo_list);
>  bo_node->as_mob = as_mob;
>  bo_node->cpu_blit = cpu_blit;

Fails the same BUG_ON with num_fences and max_fences == 0.

z


Re: [Intel-gfx] [PATCH 03/15] dma-buf & drm/amdgpu: remove dma_resv workaround

2022-04-20 Thread Zack Rusin
On Wed, 2022-04-20 at 19:40 +0200, Christian König wrote:
> 
> Am 20.04.22 um 19:38 schrieb Zack Rusin:
> > On Wed, 2022-04-20 at 09:37 +0200, Christian König wrote:
> > > ⚠ External Email
> > > 
> > > Hi Zack,
> > > 
> > > Am 20.04.22 um 05:56 schrieb Zack Rusin:
> > > > On Thu, 2022-04-07 at 10:59 +0200, Christian König wrote:
> > > > > Rework the internals of the dma_resv object to allow adding
> > > > > more
> > > > > than
> > > > > one
> > > > > write fence and remember for each fence what purpose it had.
> > > > > 
> > > > > This allows removing the workaround from amdgpu which used a
> > > > > container
> > > > > for
> > > > > this instead.
> > > > > 
> > > > > Signed-off-by: Christian König 
> > > > > Reviewed-by: Daniel Vetter 
> > > > > Cc: amd-...@lists.freedesktop.org
> > > > afaict this change broke vmwgfx which now kernel oops right
> > > > after
> > > > boot.
> > > > I haven't had the time to look into it yet, so I'm not sure
> > > > what's
> > > > the
> > > > problem. I'll look at this tomorrow, but just in case you have
> > > > some
> > > > clues, the backtrace follows:
> > > that's a known issue and should already be fixed with:
> > > 
> > > commit d72dcbe9fce505228dae43bef9da8f2b707d1b3d
> > > Author: Christian König 
> > > Date:   Mon Apr 11 15:21:59 2022 +0200
> > Unfortunately that doesn't seem to be it. The backtrace is from the
> > current (as of the time of sending of this email) drm-misc-next,
> > which
> > has this change, so it's something else.
> 
> Ok, that's strange. In this case I need to investigate further.
> 
> Maybe VMWGFX is adding more than one fence and we actually need to
> reserve multiple slots.

This might be helper code issue with CONFIG_DEBUG_MUTEXES set. On that config
dma_resv_reset_max_fences does: 
   fences->max_fences = fences->num_fences;
For some objects num_fences is 0 and so after max_fences and num_fences are 
both 0.
And then BUG_ON(num_fences >= max_fences) is triggered.

z



Re: [Intel-gfx] [PATCH 03/15] dma-buf & drm/amdgpu: remove dma_resv workaround

2022-04-20 Thread Zack Rusin
On Wed, 2022-04-20 at 09:37 +0200, Christian König wrote:
> ⚠ External Email
> 
> Hi Zack,
> 
> Am 20.04.22 um 05:56 schrieb Zack Rusin:
> > On Thu, 2022-04-07 at 10:59 +0200, Christian König wrote:
> > > Rework the internals of the dma_resv object to allow adding more
> > > than
> > > one
> > > write fence and remember for each fence what purpose it had.
> > > 
> > > This allows removing the workaround from amdgpu which used a
> > > container
> > > for
> > > this instead.
> > > 
> > > Signed-off-by: Christian König 
> > > Reviewed-by: Daniel Vetter 
> > > Cc: amd-...@lists.freedesktop.org
> > 
> > afaict this change broke vmwgfx which now kernel oops right after
> > boot.
> > I haven't had the time to look into it yet, so I'm not sure what's
> > the
> > problem. I'll look at this tomorrow, but just in case you have some
> > clues, the backtrace follows:
> 
> that's a known issue and should already be fixed with:
> 
> commit d72dcbe9fce505228dae43bef9da8f2b707d1b3d
> Author: Christian König 
> Date:   Mon Apr 11 15:21:59 2022 +0200

Unfortunately that doesn't seem to be it. The backtrace is from the
current (as of the time of sending of this email) drm-misc-next, which
has this change, so it's something else.

z


Re: [Intel-gfx] [PATCH 03/15] dma-buf & drm/amdgpu: remove dma_resv workaround

2022-04-19 Thread Zack Rusin
On Thu, 2022-04-07 at 10:59 +0200, Christian König wrote:
> Rework the internals of the dma_resv object to allow adding more than
> one
> write fence and remember for each fence what purpose it had.
> 
> This allows removing the workaround from amdgpu which used a container
> for
> this instead.
> 
> Signed-off-by: Christian König 
> Reviewed-by: Daniel Vetter 
> Cc: amd-...@lists.freedesktop.org


afaict this change broke vmwgfx which now kernel oops right after boot.
I haven't had the time to look into it yet, so I'm not sure what's the
problem. I'll look at this tomorrow, but just in case you have some
clues, the backtrace follows:

 [ cut here ]
 kernel BUG at drivers/dma-buf/dma-resv.c:306!
 invalid opcode:  [#1] PREEMPT SMP PTI
 CPU: 1 PID: 1608 Comm: gnome-shell Not tainted 5.18.0-rc1-vmwgfx #18
 Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop
Reference Platform, BIOS 6.00 11/12/2020
 RIP: 0010:dma_resv_add_fence+0x2ed/0x300
 Code: ff ff be 01 00 00 00 e8 31 7d d9 ff e9 80 fd ff ff be 03 00 00
00 e8 22 7d d9 ff e9 ee fe ff ff 0f 1f 44 00 00 e9 bc fe ff ff <0f> 0b
e8 4c cc 45 00 66 6>
 RSP: 0018:a1e6846c3ab0 EFLAGS: 00010246
 RAX:  RBX: 94c5c5507138 RCX: 902bc24e7b7c70ae
 RDX: 902bc24e7b7c70ae RSI: aaf7f437 RDI: aaffde66
 RBP: a1e6846c3b08 R08:  R09: 0001
 R10: 0004 R11:  R12: 94c5cba90578
 R13:  R14: 94c5cba8bc00 R15: 
 FS:  7f9a17c6e600() GS:94c6f9e4()
knlGS:
 CS:  0010 DS:  ES:  CR0: 80050033
 CR2: 7f9a14113000 CR3: 0144c003 CR4: 003706e0
 Call Trace:
  
  ttm_eu_fence_buffer_objects+0x54/0x110 [ttm]
  vmw_execbuf_process+0xcae/0x12a0 [vmwgfx]
  ? vmw_execbuf_release_pinned_bo+0x60/0x60 [vmwgfx]
  vmw_execbuf_ioctl+0xfb/0x160 [vmwgfx]
  ? vmw_execbuf_release_pinned_bo+0x60/0x60 [vmwgfx]
  drm_ioctl_kernel+0xba/0x150 [drm]
  ? __might_fault+0x77/0x80
  drm_ioctl+0x247/0x460 [drm]
  ? vmw_execbuf_release_pinned_bo+0x60/0x60 [vmwgfx]
  ? find_held_lock+0x31/0x90
  ? __fget_files+0xc5/0x190
  ? __this_cpu_preempt_check+0x13/0x20
  ? lock_release+0x142/0x2f0
  ? drm_ioctl_kernel+0x150/0x150 [drm]
  vmw_generic_ioctl+0xa3/0x110 [vmwgfx]
  vmw_unlocked_ioctl+0x15/0x20 [vmwgfx]
  __x64_sys_ioctl+0x91/0xc0
  do_syscall_64+0x3b/0x90
  entry_SYSCALL_64_after_hwframe+0x44/0xae
 RIP: 0033:0x7f9a1af1aaff
 Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24 10 00 00 00 48
89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00 0f 05 <41> 89
c0 3d 00 f0 ff ff 7>
 RSP: 002b:7ffd833696c0 EFLAGS: 0246 ORIG_RAX: 0010
 RAX: ffda RBX: 7ffd83369780 RCX: 7f9a1af1aaff
 RDX: 7ffd83369780 RSI: 4028644c RDI: 000d
 RBP: 4028644c R08: 1248 R09: 7ffd83369808
 R10: 0008 R11: 0246 R12: 7ffd83369808
 R13: 000d R14: 55719cb629c0 R15: 7ffd83369808
  
 Modules linked in: overlay snd_ens1371 intel_rapl_msr snd_ac97_codec
intel_rapl_common ac97_bus vsock_loopback
vmw_vsock_virtio_transport_common vmw_vsock_vmci>
 ---[ end trace  ]---

z


Re: [Intel-gfx] [PATCH 11/11] drm/vmwgfx: remove vmw_wait_dma_fence

2022-01-24 Thread Zack Rusin
On Mon, 2022-01-24 at 14:03 +0100, Christian König wrote:
> Decomposing fence containers don't seem to make any sense here.
> 
> So just remove the function entirely and call dma_fence_wait()
> directly.
> 
> Signed-off-by: Christian König 
> Cc: VMware Graphics 
> Cc: Zack Rusin 

Looks good. That's a great cleanup.

Reviewed-by: Zack Rusin 


Re: [Intel-gfx] [PATCH 3/3] drm/vmwgfx: fix potential UAF in vmwgfx_surface.c

2021-07-22 Thread Zack Rusin

On 7/22/21 5:29 AM, Desmond Cheong Zhi Xi wrote:

drm_file.master should be protected by either drm_device.master_mutex
or drm_file.master_lookup_lock when being dereferenced. However,
drm_master_get is called on unprotected file_priv->master pointers in
vmw_surface_define_ioctl and vmw_gb_surface_define_internal.

This is fixed by replacing drm_master_get with drm_file_get_master.

Signed-off-by: Desmond Cheong Zhi Xi 


Reviewed-by: Zack Rusin 

Thanks for taking the time to fix this. Apart from the clear logic error, do 
you happen to know under what circumstances would this be hit? We have someone 
looking at writing some vmwgfx specific igt tests and I was wondering if I 
could add this to the list.

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


Re: [Intel-gfx] [PATCH v4 25/27] drm/vmwgfx: Don't set struct drm_device.irq_enabled

2021-06-25 Thread Zack Rusin


> On Jun 25, 2021, at 04:22, Thomas Zimmermann  wrote:
> 
> The field drm_device.irq_enabled is only used by legacy drivers
> with userspace modesetting. Don't set it in vmxgfx. All usage of
> the field within vmwgfx can safely be removed.
> 
> Signed-off-by: Thomas Zimmermann 
> Reviewed-by: Laurent Pinchart 
> Acked-by: Daniel Vetter 

Looks good.

Reviewed-by: Zack Rusin 

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


Re: [Intel-gfx] [patch 2/7] drm/vmgfx: Replace kmap_atomic()

2021-03-04 Thread Zack Rusin


> On Mar 3, 2021, at 08:20, Thomas Gleixner  wrote:
> 
> From: Thomas Gleixner 
> 
> There is no reason to disable pagefaults and preemption as a side effect of
> kmap_atomic_prot().
> 
> Use kmap_local_page_prot() instead and document the reasoning for the
> mapping usage with the given pgprot.
> 
> Remove the NULL pointer check for the map. These functions return a valid
> address for valid pages and the return was bogus anyway as it would have
> left preemption and pagefaults disabled.
> 
> Signed-off-by: Thomas Gleixner 
> Cc: VMware Graphics 
> Cc: Roland Scheidegger 
> Cc: Zack Rusin 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: dri-de...@lists.freedesktop.org
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_blit.c |   30 --
> 1 file changed, 12 insertions(+), 18 deletions(-)
> 
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c
> @@ -375,12 +375,12 @@ static int vmw_bo_cpu_blit_line(struct v
>   copy_size = min_t(u32, copy_size, PAGE_SIZE - src_page_offset);
> 
>   if (unmap_src) {
> - kunmap_atomic(d->src_addr);
> + kunmap_local(d->src_addr);
>   d->src_addr = NULL;
>   }
> 
>   if (unmap_dst) {
> - kunmap_atomic(d->dst_addr);
> + kunmap_local(d->dst_addr);
>   d->dst_addr = NULL;
>   }
> 
> @@ -388,12 +388,8 @@ static int vmw_bo_cpu_blit_line(struct v
>   if (WARN_ON_ONCE(dst_page >= d->dst_num_pages))
>   return -EINVAL;
> 
> - d->dst_addr =
> - kmap_atomic_prot(d->dst_pages[dst_page],
> -  d->dst_prot);
> - if (!d->dst_addr)
> - return -ENOMEM;
> -
> + d->dst_addr = 
> kmap_local_page_prot(d->dst_pages[dst_page],
> +d->dst_prot);
>   d->mapped_dst = dst_page;
>   }
> 
> @@ -401,12 +397,8 @@ static int vmw_bo_cpu_blit_line(struct v
>   if (WARN_ON_ONCE(src_page >= d->src_num_pages))
>   return -EINVAL;
> 
> - d->src_addr =
> - kmap_atomic_prot(d->src_pages[src_page],
> -  d->src_prot);
> - if (!d->src_addr)
> - return -ENOMEM;
> -
> + d->src_addr = 
> kmap_local_page_prot(d->src_pages[src_page],
> +d->src_prot);
>   d->mapped_src = src_page;
>   }
>   diff->do_cpy(diff, d->dst_addr + dst_page_offset,
> @@ -436,8 +428,10 @@ static int vmw_bo_cpu_blit_line(struct v
>  *
>  * Performs a CPU blit from one buffer object to another avoiding a full
>  * bo vmap which may exhaust- or fragment vmalloc space.
> - * On supported architectures (x86), we're using kmap_atomic which avoids
> - * cross-processor TLB- and cache flushes and may, on non-HIGHMEM systems
> + *
> + * On supported architectures (x86), we're using kmap_local_prot() which
> + * avoids cross-processor TLB- and cache flushes. kmap_local_prot() will
> + * either map a highmem page with the proper pgprot on HIGHMEM=y systems or
>  * reference already set-up mappings.
>  *
>  * Neither of the buffer objects may be placed in PCI memory
> @@ -500,9 +494,9 @@ int vmw_bo_cpu_blit(struct ttm_buffer_ob
>   }
> out:
>   if (d.src_addr)
> - kunmap_atomic(d.src_addr);
> + kunmap_local(d.src_addr);
>   if (d.dst_addr)
> - kunmap_atomic(d.dst_addr);
> + kunmap_local(d.dst_addr);
> 
>   return ret;
> }


Looks good. Thanks.

Reviewed-by: Zack Rusin 

z

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


Re: [Intel-gfx] [PATCH 00/29] [Set 15] Finally rid W=1 warnings from GPU!

2021-01-19 Thread Zack Rusin



> On Jan 19, 2021, at 03:29, Lee Jones  wrote:
> 
> On Mon, 18 Jan 2021, Daniel Vetter wrote:
> 
>> On Mon, Jan 18, 2021 at 03:09:45PM +, Lee Jones wrote:
>>> On Mon, 18 Jan 2021, Daniel Vetter wrote:
>>> 
>>>> On Fri, Jan 15, 2021 at 06:27:15PM +, Zack Rusin wrote:
>>>>> 
>>>>>> On Jan 15, 2021, at 13:15, Lee Jones  wrote:
>>>>>> 
>>>>>> This set is part of a larger effort attempting to clean-up W=1
>>>>>> kernel builds, which are currently overwhelmingly riddled with
>>>>>> niggly little warnings.
>>>>>> 
>>>>>> Last set!  All clean after this for; Arm, Arm64, PPC, MIPS and x86.
>>>>> 
>>>>> Thanks! For all the vmwgfx bits:
>>>>> Reviewed-by: Zack Rusin 
>>>> 
>>>> Ok I merged everything except vmwgfx (that's for Zack) and i915/nouveau
>>>> (those generally go through other trees, pls holler if they're stuck).
>>> 
>>> Thanks Daniel, you're a superstar!
>>> 
>>> So Zack will take the vmwgfx parts?  Despite providing a R-b?
>> 
>> I only merge stuff that's defacto abandoned already. Everything else I try
>> to offload to whomever actually cares :-)
> 
> Understood.  Thanks for the explanation.
> 
> Hopefully Zack actually cares. :D

hah, I do. I just pushed all of the changes to drm-misc-next. Let me know if I 
missed anything. Thanks!

z


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


Re: [Intel-gfx] [PATCH v4 5/6] drm/vmwgfx: Remove reference to struct drm_device.pdev

2021-01-18 Thread Zack Rusin


> On Jan 18, 2021, at 08:14, Thomas Zimmermann  wrote:
> 
> Using struct drm_device.pdev is deprecated in favor of drm_device.dev.
> The reference to the field was reintroduced during a rebase.
> 
> Signed-off-by: Thomas Zimmermann 
> Fixes: 9703bb329206 ("drm/vmwgfx: Switch to a managed drm device")
> Cc: Zack Rusin 
> Cc: Martin Krastev 
> Cc: Roland Scheidegger 
> Cc: VMware Graphics 
> Cc: dri-de...@lists.freedesktop.org
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 1 -
> 1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index 8c3eb00e8b54..545b83e338fc 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -1524,7 +1524,6 @@ static int vmw_probe(struct pci_dev *pdev, const struct 
> pci_device_id *ent)
>   if (IS_ERR(vmw))
>   return PTR_ERR(vmw);
> 
> - vmw->drm.pdev = pdev;
>   pci_set_drvdata(pdev, &vmw->drm);
> 
>   ret = vmw_driver_load(vmw, ent->device);

Ah, sorry about that. Looks good.

Reviewed-by: Zack Rusin 

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


Re: [Intel-gfx] [PATCH 00/29] [Set 15] Finally rid W=1 warnings from GPU!

2021-01-15 Thread Zack Rusin


> On Jan 15, 2021, at 13:15, Lee Jones  wrote:
> 
> This set is part of a larger effort attempting to clean-up W=1
> kernel builds, which are currently overwhelmingly riddled with
> niggly little warnings.
> 
> Last set!  All clean after this for; Arm, Arm64, PPC, MIPS and x86.

Thanks! For all the vmwgfx bits:
Reviewed-by: Zack Rusin 

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


Re: [Intel-gfx] [PATCH 14/15] drm/vmwgfx: Remove references to struct drm_device.pdev

2020-12-03 Thread Zack Rusin


> On Dec 3, 2020, at 10:12, Daniel Vetter  wrote:
> 
> On Thu, Dec 03, 2020 at 03:06:20AM +0000, Zack Rusin wrote:
>> 
>> 
>>> On Dec 2, 2020, at 11:03, Daniel Vetter  wrote:
>>> 
>>> On Wed, Dec 2, 2020 at 4:37 PM Zack Rusin  wrote:
>>>> 
>>>> 
>>>> 
>>>>> On Dec 2, 2020, at 09:27, Thomas Zimmermann  wrote:
>>>>> 
>>>>> Hi
>>>>> 
>>>>> Am 02.12.20 um 09:01 schrieb Thomas Zimmermann:
>>>>>> Hi
>>>>>> Am 30.11.20 um 21:59 schrieb Zack Rusin:
>>>>>>> 
>>>>>>> 
>>>>>>>> On Nov 24, 2020, at 06:38, Thomas Zimmermann  
>>>>>>>> wrote:
>>>>>>>> 
>>>>>>>> Using struct drm_device.pdev is deprecated. Convert vmwgfx to struct
>>>>>>>> drm_device.dev. No functional changes.
>>>>>>>> 
>>>>>>>> Signed-off-by: Thomas Zimmermann 
>>>>>>>> Cc: Roland Scheidegger 
>>>>>>>> ---
>>>>>>>> drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c |  8 
>>>>>>>> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c| 27 +-
>>>>>>>> drivers/gpu/drm/vmwgfx/vmwgfx_fb.c |  2 +-
>>>>>>> 
>>>>>>> Reviewed-by: Zack Rusin 
>>>>>> Could you add this patch to the vmwgfx tree?
>>>>> 
>>>>> AMD devs indicated that they'd prefer to merge the patchset trough 
>>>>> drm-misc-next. If you're OK with that, I'd merge the vmwgfx patch through 
>>>>> drm-misc-next as well.
>>>> 
>>>> Sounds good. I’ll make sure to rebase our latest patch set on top of it 
>>>> when it’s in. Thanks!
>>> 
>>> btw if you want to avoid multi-tree coordination headaches, we can
>>> also manage vmwgfx in drm-misc and give you & Roland commit rights
>>> there. Up to you. There is some scripting involved for now (but I hope
>>> whenever we move to gitlab we could do the checks server-side):
>> 
>> I’d be happy to take you up on that. I would like to move a lot more of
>> our development into public repos and reducing the burden of maintaining
>> multiple trees would help. Is there a lot of changes to drm core that
>> doesn’t go through drm-misc? Or alternatively is drm-misc often pulling
>> from Linus’ master? I’m trying to figure out how much would our need to
>> rebase/merge be reduced if we just used drm-misc-next/drm-misc-fixes
>> because that would also allow me to point some internal testing
>> infrastructure at it as well.
> 
> I think nowadays pretty much all the cross-driver work lands through
> drm-misc. Exception is just new stuff that's only used by a single driver.
> 
> For testing there's also drm-tip, which tries to pull it all together (but
> you might still want to point your CI at branches).
> 
> Also note that drm-misc-next doesn't have a merge window freeze period
> (with have the drm-misc-next-fixes branch during that time for merge
> window fixes), so you can treat it like a main development branch like
> e.g. in mesa, with the -fixes branches as the release branches. Only
> difference is that we don't cherry pick patches from the main branch to
> the release branches (at least not as the normal flow).
> 
> If you do need a backmerge for patches from Linus to drm-misc-next because
> of some feature work (or conflicts with other stuff in general) drm-misc
> maintainers do that. Usually happens every few weeks.

Perfect, thanks a lot! This has been something I wanted our driver to do for a 
while, I’ll go ahead and switch our internal dev to drm-misc.

z

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


Re: [Intel-gfx] [PATCH 14/15] drm/vmwgfx: Remove references to struct drm_device.pdev

2020-12-02 Thread Zack Rusin


> On Dec 2, 2020, at 11:03, Daniel Vetter  wrote:
> 
> On Wed, Dec 2, 2020 at 4:37 PM Zack Rusin  wrote:
>> 
>> 
>> 
>>> On Dec 2, 2020, at 09:27, Thomas Zimmermann  wrote:
>>> 
>>> Hi
>>> 
>>> Am 02.12.20 um 09:01 schrieb Thomas Zimmermann:
>>>> Hi
>>>> Am 30.11.20 um 21:59 schrieb Zack Rusin:
>>>>> 
>>>>> 
>>>>>> On Nov 24, 2020, at 06:38, Thomas Zimmermann  wrote:
>>>>>> 
>>>>>> Using struct drm_device.pdev is deprecated. Convert vmwgfx to struct
>>>>>> drm_device.dev. No functional changes.
>>>>>> 
>>>>>> Signed-off-by: Thomas Zimmermann 
>>>>>> Cc: Roland Scheidegger 
>>>>>> ---
>>>>>> drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c |  8 
>>>>>> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c| 27 +-
>>>>>> drivers/gpu/drm/vmwgfx/vmwgfx_fb.c |  2 +-
>>>>> 
>>>>> Reviewed-by: Zack Rusin 
>>>> Could you add this patch to the vmwgfx tree?
>>> 
>>> AMD devs indicated that they'd prefer to merge the patchset trough 
>>> drm-misc-next. If you're OK with that, I'd merge the vmwgfx patch through 
>>> drm-misc-next as well.
>> 
>> Sounds good. I’ll make sure to rebase our latest patch set on top of it when 
>> it’s in. Thanks!
> 
> btw if you want to avoid multi-tree coordination headaches, we can
> also manage vmwgfx in drm-misc and give you & Roland commit rights
> there. Up to you. There is some scripting involved for now (but I hope
> whenever we move to gitlab we could do the checks server-side):

I’d be happy to take you up on that. I would like to move a lot more of our 
development into public repos and reducing the burden of maintaining multiple 
trees would help. Is there a lot of changes to drm core that doesn’t go through 
drm-misc? Or alternatively is drm-misc often pulling from Linus’ master? I’m 
trying to figure out how much would our need to rebase/merge be reduced if we 
just used drm-misc-next/drm-misc-fixes because that would also allow me to 
point some internal testing infrastructure at it as well.

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


Re: [Intel-gfx] [PATCH 14/15] drm/vmwgfx: Remove references to struct drm_device.pdev

2020-12-02 Thread Zack Rusin


> On Dec 2, 2020, at 09:27, Thomas Zimmermann  wrote:
> 
> Hi
> 
> Am 02.12.20 um 09:01 schrieb Thomas Zimmermann:
>> Hi
>> Am 30.11.20 um 21:59 schrieb Zack Rusin:
>>> 
>>> 
>>>> On Nov 24, 2020, at 06:38, Thomas Zimmermann  wrote:
>>>> 
>>>> Using struct drm_device.pdev is deprecated. Convert vmwgfx to struct
>>>> drm_device.dev. No functional changes.
>>>> 
>>>> Signed-off-by: Thomas Zimmermann 
>>>> Cc: Roland Scheidegger 
>>>> ---
>>>> drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c |  8 ----
>>>> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c| 27 +-
>>>> drivers/gpu/drm/vmwgfx/vmwgfx_fb.c |  2 +-
>>> 
>>> Reviewed-by: Zack Rusin 
>> Could you add this patch to the vmwgfx tree?
> 
> AMD devs indicated that they'd prefer to merge the patchset trough 
> drm-misc-next. If you're OK with that, I'd merge the vmwgfx patch through 
> drm-misc-next as well.

Sounds good. I’ll make sure to rebase our latest patch set on top of it when 
it’s in. Thanks!

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


Re: [Intel-gfx] [PATCH 14/15] drm/vmwgfx: Remove references to struct drm_device.pdev

2020-11-30 Thread Zack Rusin



> On Nov 24, 2020, at 06:38, Thomas Zimmermann  wrote:
> 
> Using struct drm_device.pdev is deprecated. Convert vmwgfx to struct
> drm_device.dev. No functional changes.
> 
> Signed-off-by: Thomas Zimmermann 
> Cc: Roland Scheidegger 
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c |  8 
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c| 27 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_fb.c |  2 +-

Reviewed-by: Zack Rusin 

z

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