Re: [PATCH] drm/nouveau: Fix out-of-bounds access when deferencing MMU type

2020-11-11 Thread Ben Skeggs 
> >>>  [   18.778890]  __kasan_kmalloc.constprop.0+0xbf/0xd0
> >>>  [   18.785646]  __kmalloc_track_caller+0x1be/0x390
> >>>  [   18.792165]  kstrdup_const+0x46/0x70
> >>>  [   18.797686]  kobject_set_name_vargs+0x2f/0xb0
> >>>  [   18.803992]  kobject_init_and_add+0x9d/0xf0
> >>>  [   18.810117]  ttm_mem_global_init+0x12c/0x210 [ttm]
> >>>  [   18.816853]  ttm_bo_global_init+0x4a/0x160 [ttm]
> >>>  [   18.823420]  ttm_bo_device_init+0x39/0x220 [ttm]
> >>>  [   18.830046]  nouveau_ttm_init+0x2c3/0x830 [nouveau]
> >>>  [   18.836929]  nouveau_drm_device_init+0x1b4/0x3f0 [nouveau]
> >>>  <...>
> >>>  [   19.105336]
> >>>
> >===
> >>> ===
> >>>
> >>> Fix this error, by not using type_vram as an index if it's negative.
> >>> Assume default values instead.
> >>>
> >>> The error was seen on Nvidia G72 hardware.
> >>>
> >>> Signed-off-by: Thomas Zimmermann 
> >>> Fixes: 1cf65c45183a ("drm/ttm: add caching state to ttm_bus_placement")
> >>> Cc: Christian König 
> >>> Cc: Michael J. Ruhl 
> >>> Cc: Maarten Lankhorst 
> >>> Cc: Maxime Ripard 
> >>> Cc: Thomas Zimmermann 
> >>> Cc: David Airlie 
> >>> Cc: Daniel Vetter 
> >>> Cc: Ben Skeggs 
> >>> Cc: Dave Airlie 
> >>> Cc: Gerd Hoffmann 
> >>> Cc: Alex Deucher 
> >>> Cc: "Christian König" 
> >>> Cc: VMware Graphics 
> >>> Cc: Roland Scheidegger 
> >>> Cc: Huang Rui 
> >>> Cc: Felix Kuehling 
> >>> Cc: Hawking Zhang 
> >>> Cc: Jason Gunthorpe 
> >>> Cc: Likun Gao 
> >>> Cc: dri-de...@lists.freedesktop.org
> >>> Cc: nouv...@lists.freedesktop.org
> >>> Cc: virtualizat...@lists.linux-foundation.org
> >>> Cc: spice-de...@lists.freedesktop.org
> >>> Cc: amd-gfx@lists.freedesktop.org
> >>> ---
> >>> drivers/gpu/drm/nouveau/nouveau_bo.c | 5 -
> >>> 1 file changed, 4 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
> >>> b/drivers/gpu/drm/nouveau/nouveau_bo.c
> >>> index 8133377d865d..fe15299d417e 100644
> >>> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> >>> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> >>> @@ -1142,9 +1142,12 @@ nouveau_ttm_io_mem_reserve(struct
> >>> ttm_bo_device *bdev, struct ttm_resource *reg)
> >>> struct nvkm_device *device = nvxx_device(>client.device);
> >>> struct nouveau_mem *mem = nouveau_mem(reg);
> >>> struct nvif_mmu *mmu = >client.mmu;
> >>> -   const u8 type = mmu->type[drm->ttm.type_vram].type;
> >>> +   u8 type = 0;
> >>> int ret;
> >>>
> >>> +   if (drm->ttm.type_vram >= 0)
> >>> +   type = mmu->type[drm->ttm.type_vram].type;
> >>> +
> >>> mutex_lock(>ttm.io_reserve_mutex);
> >>> retry:
> >>> switch (reg->mem_type) {
> >>> --
> >>> 2.29.2
> >>
> >
> >--
> >Thomas Zimmermann
> >Graphics Driver Developer
> >SUSE Software Solutions Germany GmbH
> >Maxfeldstr. 5, 90409 Nürnberg, Germany
> >(HRB 36809, AG Nürnberg)
> >Geschäftsführer: Felix Imendörffer
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v4 09/22] drm/nouveau: Convert to CRTC VBLANK callbacks

2020-01-29 Thread Ben Skeggs 
On Fri, 24 Jan 2020 at 00:00, Thomas Zimmermann  wrote:
>
> VBLANK callbacks in struct drm_driver are deprecated in favor of
> their equivalents in struct drm_crtc_funcs. Convert nouvean over.
>
> v4:
> * add argument names in function declaration
>
> Signed-off-by: Thomas Zimmermann 
Reviewed-by: Ben Skeggs 

> ---
>  drivers/gpu/drm/nouveau/dispnv04/crtc.c   |  3 +++
>  drivers/gpu/drm/nouveau/dispnv50/head.c   |  4 
>  drivers/gpu/drm/nouveau/nouveau_display.c | 14 ++
>  drivers/gpu/drm/nouveau/nouveau_display.h |  4 ++--
>  drivers/gpu/drm/nouveau/nouveau_drm.c |  4 
>  5 files changed, 11 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c 
> b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> index 17e9d1c078a0..1f08de4241e0 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> @@ -1248,6 +1248,9 @@ static const struct drm_crtc_funcs nv04_crtc_funcs = {
> .set_config = drm_crtc_helper_set_config,
> .page_flip = nv04_crtc_page_flip,
> .destroy = nv_crtc_destroy,
> +   .enable_vblank = nouveau_display_vblank_enable,
> +   .disable_vblank = nouveau_display_vblank_disable,
> +   .get_vblank_timestamp = drm_crtc_vblank_helper_get_vblank_timestamp,
>  };
>
>  static const struct drm_crtc_helper_funcs nv04_crtc_helper_funcs = {
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c 
> b/drivers/gpu/drm/nouveau/dispnv50/head.c
> index 41852dd8fdbd..8f6455697ba7 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/head.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
> @@ -29,6 +29,7 @@
>
>  #include 
>  #include 
> +#include 
>  #include "nouveau_connector.h"
>  void
>  nv50_head_flush_clr(struct nv50_head *head,
> @@ -482,6 +483,9 @@ nv50_head_func = {
> .page_flip = drm_atomic_helper_page_flip,
> .atomic_duplicate_state = nv50_head_atomic_duplicate_state,
> .atomic_destroy_state = nv50_head_atomic_destroy_state,
> +   .enable_vblank = nouveau_display_vblank_enable,
> +   .disable_vblank = nouveau_display_vblank_disable,
> +   .get_vblank_timestamp = drm_crtc_vblank_helper_get_vblank_timestamp,
>  };
>
>  struct nv50_head *
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c 
> b/drivers/gpu/drm/nouveau/nouveau_display.c
> index 86f99dc8fcef..700817dc4fa0 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
> @@ -54,15 +54,10 @@ nouveau_display_vblank_handler(struct nvif_notify *notify)
>  }
>
>  int
> -nouveau_display_vblank_enable(struct drm_device *dev, unsigned int pipe)
> +nouveau_display_vblank_enable(struct drm_crtc *crtc)
>  {
> -   struct drm_crtc *crtc;
> struct nouveau_crtc *nv_crtc;
>
> -   crtc = drm_crtc_from_index(dev, pipe);
> -   if (!crtc)
> -   return -EINVAL;
> -
> nv_crtc = nouveau_crtc(crtc);
> nvif_notify_get(_crtc->vblank);
>
> @@ -70,15 +65,10 @@ nouveau_display_vblank_enable(struct drm_device *dev, 
> unsigned int pipe)
>  }
>
>  void
> -nouveau_display_vblank_disable(struct drm_device *dev, unsigned int pipe)
> +nouveau_display_vblank_disable(struct drm_crtc *crtc)
>  {
> -   struct drm_crtc *crtc;
> struct nouveau_crtc *nv_crtc;
>
> -   crtc = drm_crtc_from_index(dev, pipe);
> -   if (!crtc)
> -   return;
> -
> nv_crtc = nouveau_crtc(crtc);
> nvif_notify_put(_crtc->vblank);
>  }
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h 
> b/drivers/gpu/drm/nouveau/nouveau_display.h
> index 26d34f1a77da..de004018ab5c 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.h
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.h
> @@ -61,8 +61,8 @@ int  nouveau_display_init(struct drm_device *dev, bool 
> resume, bool runtime);
>  void nouveau_display_fini(struct drm_device *dev, bool suspend, bool 
> runtime);
>  int  nouveau_display_suspend(struct drm_device *dev, bool runtime);
>  void nouveau_display_resume(struct drm_device *dev, bool runtime);
> -int  nouveau_display_vblank_enable(struct drm_device *, unsigned int);
> -void nouveau_display_vblank_disable(struct drm_device *, unsigned int);
> +int  nouveau_display_vblank_enable(struct drm_crtc *crtc);
> +void nouveau_display_vblank_disable(struct drm_crtc *crtc);
>  bool nouveau_display_scanoutpos(struct drm_crtc *crtc,
> bool in_vblank_irq, int *vpos, int *hpos,
> ktime_t *stime, ktime_t *etime,
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c 
> b/drivers/gpu/drm/nouveau/nouveau_drm

Re: [PATCH v4 08/22] drm/nouveau: Convert to struct drm_crtc_helper_funcs.get_scanout_position()

2020-01-29 Thread Ben Skeggs 
On Fri, 24 Jan 2020 at 00:00, Thomas Zimmermann  wrote:
>
> The callback struct drm_driver.get_scanout_position() is deprecated in
> favor of struct drm_crtc_helper_funcs.get_scanout_position(). Convert
> nouveau over.
>
> v4:
> * add argument names in function declaration
>
> Signed-off-by: Thomas Zimmermann 
Reviewed-by: Ben Skeggs 

> ---
>  drivers/gpu/drm/nouveau/dispnv04/crtc.c   |  1 +
>  drivers/gpu/drm/nouveau/dispnv50/head.c   |  1 +
>  drivers/gpu/drm/nouveau/nouveau_display.c | 14 +++---
>  drivers/gpu/drm/nouveau/nouveau_display.h |  7 ---
>  drivers/gpu/drm/nouveau/nouveau_drm.c |  1 -
>  5 files changed, 9 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c 
> b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> index 37c50ea8f847..17e9d1c078a0 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> @@ -1258,6 +1258,7 @@ static const struct drm_crtc_helper_funcs 
> nv04_crtc_helper_funcs = {
> .mode_set_base = nv04_crtc_mode_set_base,
> .mode_set_base_atomic = nv04_crtc_mode_set_base_atomic,
> .disable = nv_crtc_disable,
> +   .get_scanout_position = nouveau_display_scanoutpos,
>  };
>
>  static const uint32_t modeset_formats[] = {
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c 
> b/drivers/gpu/drm/nouveau/dispnv50/head.c
> index d9d64602947d..41852dd8fdbd 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/head.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
> @@ -413,6 +413,7 @@ nv50_head_atomic_check(struct drm_crtc *crtc, struct 
> drm_crtc_state *state)
>  static const struct drm_crtc_helper_funcs
>  nv50_head_help = {
> .atomic_check = nv50_head_atomic_check,
> +   .get_scanout_position = nouveau_display_scanoutpos,
>  };
>
>  static void
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c 
> b/drivers/gpu/drm/nouveau/nouveau_display.c
> index 53f9bceaf17a..86f99dc8fcef 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
> @@ -136,21 +136,13 @@ nouveau_display_scanoutpos_head(struct drm_crtc *crtc, 
> int *vpos, int *hpos,
>  }
>
>  bool
> -nouveau_display_scanoutpos(struct drm_device *dev, unsigned int pipe,
> +nouveau_display_scanoutpos(struct drm_crtc *crtc,
>bool in_vblank_irq, int *vpos, int *hpos,
>ktime_t *stime, ktime_t *etime,
>const struct drm_display_mode *mode)
>  {
> -   struct drm_crtc *crtc;
> -
> -   list_for_each_entry(crtc, >mode_config.crtc_list, head) {
> -   if (nouveau_crtc(crtc)->index == pipe) {
> -   return nouveau_display_scanoutpos_head(crtc, vpos, 
> hpos,
> -  stime, etime);
> -   }
> -   }
> -
> -   return false;
> +   return nouveau_display_scanoutpos_head(crtc, vpos, hpos,
> +  stime, etime);
>  }
>
>  static void
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h 
> b/drivers/gpu/drm/nouveau/nouveau_display.h
> index 6e8e66882e45..26d34f1a77da 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.h
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.h
> @@ -63,9 +63,10 @@ int  nouveau_display_suspend(struct drm_device *dev, bool 
> runtime);
>  void nouveau_display_resume(struct drm_device *dev, bool runtime);
>  int  nouveau_display_vblank_enable(struct drm_device *, unsigned int);
>  void nouveau_display_vblank_disable(struct drm_device *, unsigned int);
> -bool  nouveau_display_scanoutpos(struct drm_device *, unsigned int,
> -bool, int *, int *, ktime_t *,
> -ktime_t *, const struct drm_display_mode *);
> +bool nouveau_display_scanoutpos(struct drm_crtc *crtc,
> +   bool in_vblank_irq, int *vpos, int *hpos,
> +   ktime_t *stime, ktime_t *etime,
> +   const struct drm_display_mode *mode);
>
>  int  nouveau_display_dumb_create(struct drm_file *, struct drm_device *,
>  struct drm_mode_create_dumb *args);
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c 
> b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index b65ae817eabf..fcc036a08965 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -1122,7 +1122,6 @@ driver_stub = {
>
> .enable_vblank = nouveau_display_vblank_enable,
> .disable_vblank = nouveau_display_vblank_disable,
> -

Re: [Nouveau] [Intel-gfx] [PATCH v6 08/17] drm/ttm: use gem vma_node

2019-09-15 Thread Ben Skeggs 
On Wed, 11 Sep 2019 at 07:53, Thierry Reding  wrote:
>
> On Sat, Sep 07, 2019 at 09:58:46PM -0400, Ilia Mirkin wrote:
> > On Wed, Aug 21, 2019 at 7:55 AM Thierry Reding  
> > wrote:
> > >
> > > On Wed, Aug 21, 2019 at 04:33:58PM +1000, Ben Skeggs wrote:
> > > > On Wed, 14 Aug 2019 at 20:14, Gerd Hoffmann  wrote:
> > > > >
> > > > >   Hi,
> > > > >
> > > > > > > Changing the order doesn't look hard.  Patch attached (untested, 
> > > > > > > have no
> > > > > > > test hardware).  But maybe I missed some detail ...
> > > > > >
> > > > > > I came up with something very similar by splitting up 
> > > > > > nouveau_bo_new()
> > > > > > into allocation and initialization steps, so that when necessary 
> > > > > > the GEM
> > > > > > object can be initialized in between. I think that's slightly more
> > > > > > flexible and easier to understand than a boolean flag.
> > > > >
> > > > > Yes, that should work too.
> > > > >
> > > > > Acked-by: Gerd Hoffmann 
> > > > Acked-by: Ben Skeggs 
> > >
> > > Thanks guys, applied to drm-misc-next.
> >
> > Hi Thierry,
> >
> > Initial investigations suggest that this commit currently in drm-next
> >
> > commit 019cbd4a4feb3aa3a917d78e7110e3011bbff6d5
> > Author: Thierry Reding 
> > Date:   Wed Aug 14 11:00:48 2019 +0200
> >
> > drm/nouveau: Initialize GEM object before TTM object
> >
> > breaks nouveau userspace which tries to allocate GEM objects with a
> > non-page-aligned size. Previously nouveau_gem_new would just call
> > nouveau_bo_init which would call nouveau_bo_fixup_align before
> > initializing the GEM object. With this change, it is done after. What
> > do you think -- OK to just move that bit of logic into the new
> > nouveau_bo_alloc() (and make size/align be pointers so that they can
> > be fixed up?)
>
> Hi Ilia,
>
> sorry, got side-tracked earlier and forgot to send this out. I'll turn
> this into a proper patch, but if you manage to find the time to test
> this while I work out the userspace issues that are preventing me from
> testing this more thoroughly, that'd be great.
I can confirm both I can reproduce the bug, and that the fix here
appears to do the trick nicely.

Ben.

>
> Thierry
>
> --- >8 ---
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c 
> b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index e918b437af17..7d5ede756711 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -186,8 +186,8 @@ nouveau_bo_fixup_align(struct nouveau_bo *nvbo, u32 flags,
>  }
>
>  struct nouveau_bo *
> -nouveau_bo_alloc(struct nouveau_cli *cli, u64 size, u32 flags, u32 tile_mode,
> -u32 tile_flags)
> +nouveau_bo_alloc(struct nouveau_cli *cli, u64 *size, int *align, u32 flags,
> +u32 tile_mode, u32 tile_flags)
>  {
> struct nouveau_drm *drm = cli->drm;
> struct nouveau_bo *nvbo;
> @@ -195,8 +195,8 @@ nouveau_bo_alloc(struct nouveau_cli *cli, u64 size, u32 
> flags, u32 tile_mode,
> struct nvif_vmm *vmm = cli->svm.cli ? >svm.vmm : >vmm.vmm;
> int i, pi = -1;
>
> -   if (!size) {
> -   NV_WARN(drm, "skipped size %016llx\n", size);
> +   if (!*size) {
> +   NV_WARN(drm, "skipped size %016llx\n", *size);
> return ERR_PTR(-EINVAL);
> }
>
> @@ -266,7 +266,7 @@ nouveau_bo_alloc(struct nouveau_cli *cli, u64 size, u32 
> flags, u32 tile_mode,
> pi = i;
>
> /* Stop once the buffer is larger than the current page size. 
> */
> -   if (size >= 1ULL << vmm->page[i].shift)
> +   if (*size >= 1ULL << vmm->page[i].shift)
> break;
> }
>
> @@ -281,6 +281,8 @@ nouveau_bo_alloc(struct nouveau_cli *cli, u64 size, u32 
> flags, u32 tile_mode,
> }
> nvbo->page = vmm->page[pi].shift;
>
> +   nouveau_bo_fixup_align(nvbo, flags, align, size);
> +
> return nvbo;
>  }
>
> @@ -292,12 +294,11 @@ nouveau_bo_init(struct nouveau_bo *nvbo, u64 size, int 
> align, u32 flags,
> size_t acc_size;
> int ret;
>
> -   acc_size = ttm_bo_dma_acc_size(nvbo->bo.bdev, size, sizeof(*nvbo));
> -
> -   nouveau_bo_fixup_align(nvbo, flags, , );
> nvbo->bo.me

Re: [PATCH v7 1/9] drm_dp_cec: add connector info support.

2019-08-26 Thread Ben Skeggs 
On Fri, Aug 16, 2019 at 4:10 AM Lyude Paul  wrote:
>
> Reviewed-by: Lyude Paul 
Reviewed-by: Ben Skeggs 

>
> On Wed, 2019-08-14 at 12:44 +0200, Dariusz Marcinkiewicz wrote:
> > Pass the connector info to the CEC adapter. This makes it possible
> > to associate the CEC adapter with the corresponding drm connector.
> >
> > Signed-off-by: Dariusz Marcinkiewicz 
> > Signed-off-by: Hans Verkuil 
> > Tested-by: Hans Verkuil 
> > ---
> >  .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  2 +-
> >  drivers/gpu/drm/drm_dp_cec.c  | 25 ---
> >  drivers/gpu/drm/i915/display/intel_dp.c   |  4 +--
> >  drivers/gpu/drm/nouveau/nouveau_connector.c   |  3 +--
> >  include/drm/drm_dp_helper.h   | 17 ++---
> >  5 files changed, 27 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> > index 16218a202b591..5ec14efd4d8cb 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> > @@ -416,7 +416,7 @@ void amdgpu_dm_initialize_dp_connector(struct
> > amdgpu_display_manager *dm,
> >
> >   drm_dp_aux_register(>dm_dp_aux.aux);
> >   drm_dp_cec_register_connector(>dm_dp_aux.aux,
> > -   aconnector->base.name, dm->adev->dev);
> > +   >base);
> >   aconnector->mst_mgr.cbs = _mst_cbs;
> >   drm_dp_mst_topology_mgr_init(
> >   >mst_mgr,
> > diff --git a/drivers/gpu/drm/drm_dp_cec.c b/drivers/gpu/drm/drm_dp_cec.c
> > index b15cee85b702b..b457c16c3a8bb 100644
> > --- a/drivers/gpu/drm/drm_dp_cec.c
> > +++ b/drivers/gpu/drm/drm_dp_cec.c
> > @@ -8,7 +8,9 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> > +#include 
> >  #include 
> >
> >  /*
> > @@ -295,7 +297,10 @@ static void drm_dp_cec_unregister_work(struct 
> > work_struct
> > *work)
> >   */
> >  void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid)
> >  {
> > - u32 cec_caps = CEC_CAP_DEFAULTS | CEC_CAP_NEEDS_HPD;
> > + struct drm_connector *connector = aux->cec.connector;
> > + u32 cec_caps = CEC_CAP_DEFAULTS | CEC_CAP_NEEDS_HPD |
> > +CEC_CAP_CONNECTOR_INFO;
> > + struct cec_connector_info conn_info;
> >   unsigned int num_las = 1;
> >   u8 cap;
> >
> > @@ -344,13 +349,17 @@ void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const
> > struct edid *edid)
> >
> >   /* Create a new adapter */
> >   aux->cec.adap = cec_allocate_adapter(_dp_cec_adap_ops,
> > -  aux, aux->cec.name, cec_caps,
> > +  aux, connector->name, cec_caps,
> >num_las);
> >   if (IS_ERR(aux->cec.adap)) {
> >   aux->cec.adap = NULL;
> >   goto unlock;
> >   }
> > - if (cec_register_adapter(aux->cec.adap, aux->cec.parent)) {
> > +
> > + cec_fill_conn_info_from_drm(_info, connector);
> > + cec_s_conn_info(aux->cec.adap, _info);
> > +
> > + if (cec_register_adapter(aux->cec.adap, connector->dev->dev)) {
> >   cec_delete_adapter(aux->cec.adap);
> >   aux->cec.adap = NULL;
> >   } else {
> > @@ -406,22 +415,20 @@ EXPORT_SYMBOL(drm_dp_cec_unset_edid);
> >  /**
> >   * drm_dp_cec_register_connector() - register a new connector
> >   * @aux: DisplayPort AUX channel
> > - * @name: name of the CEC device
> > - * @parent: parent device
> > + * @connector: drm connector
> >   *
> >   * A new connector was registered with associated CEC adapter name and
> >   * CEC adapter parent device. After registering the name and parent
> >   * drm_dp_cec_set_edid() is called to check if the connector supports
> >   * CEC and to register a CEC adapter if that is the case.
> >   */
> > -void drm_dp_cec_register_connector(struct drm_dp_aux *aux, const char 
> > *name,
> > -struct device *parent)
> > +void drm_dp_cec_register_connector(struct drm_dp_aux *aux,
> > +struct drm_connector *connector)
> >  {
> >   WARN_ON(aux->cec.adap);
> >

Re: [Nouveau] [Intel-gfx] [PATCH v6 08/17] drm/ttm: use gem vma_node

2019-08-21 Thread Ben Skeggs 
On Wed, 14 Aug 2019 at 20:14, Gerd Hoffmann  wrote:
>
>   Hi,
>
> > > Changing the order doesn't look hard.  Patch attached (untested, have no
> > > test hardware).  But maybe I missed some detail ...
> >
> > I came up with something very similar by splitting up nouveau_bo_new()
> > into allocation and initialization steps, so that when necessary the GEM
> > object can be initialized in between. I think that's slightly more
> > flexible and easier to understand than a boolean flag.
>
> Yes, that should work too.
>
> Acked-by: Gerd Hoffmann 
Acked-by: Ben Skeggs 

>
> cheers,
>   Gerd
>
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [Nouveau] [PATCH v6 00/20] MST refcounting/atomic helpers cleanup

2019-01-10 Thread Ben Skeggs 
For the nouveau patches in the series:

Reviewed-By: Ben Skeggs 

On Fri, 11 Jan 2019 at 05:59, Lyude Paul  wrote:
>
> This is the series I've been working on for a while now to get all of
> the atomic DRM drivers in the tree to use the atomic MST helpers, and to
> make the atomic MST helpers actually idempotent. Turns out it's a lot
> more difficult to do that without also fixing how port and branch device
> refcounting works so that it actually makes sense, since the current
> upstream implementation requires a ton of magic in the atomic helpers to
> work around properly and in many situations just plain doesn't work as
> intended.
>
> There's still more cleanup that can be done, but I think this is a good
> place to start off for now :).
>
> Also available on gitlab:
>
> https://gitlab.freedesktop.org/lyudess/linux/commits/wip/mst-dual-kref-start-v6
>
> Lyude Paul (20):
>   drm/dp_mst: Fix some formatting in drm_dp_add_port()
>   drm/dp_mst: Fix some formatting in drm_dp_payload_send_msg()
>   drm/dp_mst: Fix some formatting in drm_dp_mst_allocate_vcpi()
>   drm/dp_mst: Fix some formatting in drm_dp_mst_deallocate_vcpi()
>   drm/dp_mst: Rename drm_dp_mst_get_validated_(port|mstb)_ref and
> friends
>   drm/dp_mst: Introduce new refcounting scheme for mstbs and ports
>   drm/dp_mst: Restart last_connected_port_and_mstb() if topology ref
> fails
>   drm/dp_mst: Stop releasing VCPI when removing ports from topology
>   drm/dp_mst: Fix payload deallocation on hotplugs using malloc refs
>   drm/i915: Keep malloc references to MST ports
>   drm/amdgpu/display: Keep malloc ref to MST port
>   drm/nouveau: Remove bogus cleanup in nv50_mstm_add_connector()
>   drm/nouveau: Remove unnecessary VCPI checks in nv50_msto_cleanup()
>   drm/nouveau: Keep malloc references to MST ports
>   drm/nouveau: Stop unsetting mstc->port, use malloc refs
>   drm/nouveau: Grab payload lock in nv50_msto_payload()
>   drm/dp_mst: Add some atomic state iterator macros
>   drm/dp_mst: Start tracking per-port VCPI allocations
>   drm/dp_mst: Check payload count in drm_dp_mst_atomic_check()
>   drm/nouveau: Use atomic VCPI helpers for MST
>
>  .../gpu/dp-mst/topology-figure-1.dot  |  52 +
>  .../gpu/dp-mst/topology-figure-2.dot  |  56 ++
>  .../gpu/dp-mst/topology-figure-3.dot  |  59 ++
>  Documentation/gpu/drm-kms-helpers.rst |  26 +-
>  .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  11 +-
>  drivers/gpu/drm/drm_dp_mst_topology.c | 946 ++
>  drivers/gpu/drm/i915/intel_connector.c|   4 +
>  drivers/gpu/drm/i915/intel_display.c  |   4 +
>  drivers/gpu/drm/i915/intel_dp_mst.c   |  55 +-
>  drivers/gpu/drm/nouveau/dispnv50/disp.c   |  96 +-
>  include/drm/drm_dp_mst_helper.h   | 151 ++-
>  11 files changed, 1204 insertions(+), 256 deletions(-)
>  create mode 100644 Documentation/gpu/dp-mst/topology-figure-1.dot
>  create mode 100644 Documentation/gpu/dp-mst/topology-figure-2.dot
>  create mode 100644 Documentation/gpu/dp-mst/topology-figure-3.dot
>
> --
> 2.20.1
>
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/3] drm/connector: Add generic underscan properties

2018-05-07 Thread Ben Skeggs 
On 8 May 2018 at 04:26, Harry Wentland  wrote:
>
>
> On 2018-05-07 12:19 PM, Boris Brezillon wrote:
>> On Mon, 7 May 2018 18:01:44 +0300
>> Ville Syrjälä  wrote:
>>
>>> On Mon, May 07, 2018 at 04:44:32PM +0200, Boris Brezillon wrote:
 We have 3 drivers defining the "underscan", "underscan hborder" and
 "underscan vborder" properties (radeon, amd and nouveau) and we are
 about to add the same kind of thing in VC4.

 Define generic underscan props and add new fields to the drm_connector
 state so that the property parsing logic can be shared by all DRM
 drivers.

 A driver can now attach underscan properties to its connector through
 the drm_connector_attach_underscan_properties() helper, and can
 check/apply the underscan setup based on the
 drm_connector_state->underscan fields.

 Signed-off-by: Boris Brezillon 
 ---
  drivers/gpu/drm/drm_atomic.c|  12 
  drivers/gpu/drm/drm_connector.c | 120 
 
  include/drm/drm_connector.h |  78 ++
  3 files changed, 210 insertions(+)

 diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
 index dc850b4b6e21..b7312bd172c9 100644
 --- a/drivers/gpu/drm/drm_atomic.c
 +++ b/drivers/gpu/drm/drm_atomic.c
 @@ -1278,6 +1278,12 @@ static int drm_atomic_connector_set_property(struct 
 drm_connector *connector,
 return -EINVAL;
 }
 state->content_protection = val;
 +   } else if (property == connector->underscan_mode_property) {
 +   state->underscan.mode = val;
 +   } else if (property == connector->underscan_hborder_property) {
 +   state->underscan.hborder = val;
 +   } else if (property == connector->underscan_vborder_property) {
 +   state->underscan.vborder = val;
 } else if (connector->funcs->atomic_set_property) {
 return connector->funcs->atomic_set_property(connector,
 state, property, val);
 @@ -1359,6 +1365,12 @@ drm_atomic_connector_get_property(struct 
 drm_connector *connector,
 *val = state->scaling_mode;
 } else if (property == connector->content_protection_property) {
 *val = state->content_protection;
 +   } else if (property == connector->underscan_mode_property) {
 +   *val = state->underscan.mode;
 +   } else if (property == connector->underscan_hborder_property) {
 +   *val = state->underscan.hborder;
 +   } else if (property == connector->underscan_vborder_property) {
 +   *val = state->underscan.vborder;
 } else if (connector->funcs->atomic_get_property) {
 return connector->funcs->atomic_get_property(connector,
 state, property, val);
 diff --git a/drivers/gpu/drm/drm_connector.c 
 b/drivers/gpu/drm/drm_connector.c
 index dfc8ca1e9413..9937390b8a25 100644
 --- a/drivers/gpu/drm/drm_connector.c
 +++ b/drivers/gpu/drm/drm_connector.c
 @@ -914,6 +914,31 @@ DRM_ENUM_NAME_FN(drm_get_content_protection_name, 
 drm_cp_enum_list)
   * can also expose this property to external outputs, in which case they
   * must support "None", which should be the default (since external 
 screens
   * have a built-in scaler).
 + *
 + * underscan:
 + * This properties defines whether underscan is activated or not, and when
 + * it is activated, how the horizontal and vertical borders are 
 calculated:
 + *
 + * off:
 + * Underscan is disabled. The output image shouldn't be scaled to
 + * take screen borders into account.
>>>
 + * on:
 + * Underscan is activated and horizontal and vertical borders are
 + * specified through the "underscan hborder" and
 + * "underscan vborder" properties.
>>>
>>> How is the output scaled?
>>
>> In HW. The formula is
>>
>>   hfactor = (hdisplay - hborder) / hdisplay
>>   vfactor = (vdisplay - vborder) / vdisplay
>>
>>> What does the user mode hdisplay/vdisplay mean
>>> in this case?
>>
>> The same as before this patch: the output resolution. You just add
>> black margins.
>>
>>> What if I want underscan without scaling?
>>
>> Then don't involve the DRM driver and do that from userspace: just
>> fill the visible portion of the framebuffer and leave the rest black.
>> There nothing the DRM driver can do to help with that, except maybe
>> exposing the information about the active area of the screen. It would
>> be nice to do that, but that means patching all userspace libs to take
>> this into account.
>>
>>>
 + * auto:
 + * Underscan is activated and horizontal and vertical borders are
 + *

Re: [PATCH 4/5] drm/ttm: add ttm_sg_tt_init

2018-03-05 Thread Ben Skeggs 
On Mon, Mar 5, 2018 at 10:06 PM, Christian König
 wrote:
> Ping?
On a quick look, it looks like both are used sometimes.  I believe
this had something to do with the addition of Tegra support, but I'll
need some time to look into the details of why/how these things are
used again.

Ben.

>
>
> Am 27.02.2018 um 13:07 schrieb Christian König:
>>
>> Hi guys,
>>
>> at least on amdgpu and radeon the page array allocated by ttm_dma_tt_init
>> is completely unused in the case of DMA-buf sharing. So I'm trying to get
>> rid of that by only allocating the DMA address array.
>>
>> Now the only other user of DMA-buf together with ttm_dma_tt_init is
>> Nouveau. So my question is are you guys using the page array anywhere in
>> your kernel driver in case of a DMA-buf sharing?
>>
>> If no then I could just make this the default behavior for all drivers and
>> save quite a bit of memory for everybody.
>>
>> Thanks,
>> Christian.
>>
>> Am 27.02.2018 um 12:49 schrieb Christian König:
>>>
>>> This allows drivers to only allocate dma addresses, but not a page
>>> array.
>>>
>>> Signed-off-by: Christian König 
>>> ---
>>>   drivers/gpu/drm/ttm/ttm_tt.c | 54
>>> 
>>>   include/drm/ttm/ttm_tt.h |  2 ++
>>>   2 files changed, 47 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
>>> index 8e0b525cda00..971133106ec2 100644
>>> --- a/drivers/gpu/drm/ttm/ttm_tt.c
>>> +++ b/drivers/gpu/drm/ttm/ttm_tt.c
>>> @@ -108,6 +108,16 @@ static int ttm_dma_tt_alloc_page_directory(struct
>>> ttm_dma_tt *ttm)
>>>   return 0;
>>>   }
>>>   +static int ttm_sg_tt_alloc_page_directory(struct ttm_dma_tt *ttm)
>>> +{
>>> +ttm->dma_address = kvmalloc_array(ttm->ttm.num_pages,
>>> +  sizeof(*ttm->dma_address),
>>> +  GFP_KERNEL | __GFP_ZERO);
>>> +if (!ttm->dma_address)
>>> +return -ENOMEM;
>>> +return 0;
>>> +}
>>> +
>>>   #ifdef CONFIG_X86
>>>   static inline int ttm_tt_set_page_caching(struct page *p,
>>> enum ttm_caching_state c_old,
>>> @@ -227,8 +237,8 @@ void ttm_tt_destroy(struct ttm_tt *ttm)
>>>   ttm->func->destroy(ttm);
>>>   }
>>>   -int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
>>> -unsigned long size, uint32_t page_flags)
>>> +void ttm_tt_init_fields(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
>>> +unsigned long size, uint32_t page_flags)
>>>   {
>>>   ttm->bdev = bdev;
>>>   ttm->num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
>>> @@ -236,6 +246,12 @@ int ttm_tt_init(struct ttm_tt *ttm, struct
>>> ttm_bo_device *bdev,
>>>   ttm->page_flags = page_flags;
>>>   ttm->state = tt_unpopulated;
>>>   ttm->swap_storage = NULL;
>>> +}
>>> +
>>> +int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device *bdev,
>>> +unsigned long size, uint32_t page_flags)
>>> +{
>>> +ttm_tt_init_fields(ttm, bdev, size, page_flags);
>>> if (ttm_tt_alloc_page_directory(ttm)) {
>>>   ttm_tt_destroy(ttm);
>>> @@ -258,12 +274,7 @@ int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma,
>>> struct ttm_bo_device *bdev,
>>>   {
>>>   struct ttm_tt *ttm = _dma->ttm;
>>>   -ttm->bdev = bdev;
>>> -ttm->num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
>>> -ttm->caching_state = tt_cached;
>>> -ttm->page_flags = page_flags;
>>> -ttm->state = tt_unpopulated;
>>> -ttm->swap_storage = NULL;
>>> +ttm_tt_init_fields(ttm, bdev, size, page_flags);
>>> INIT_LIST_HEAD(_dma->pages_list);
>>>   if (ttm_dma_tt_alloc_page_directory(ttm_dma)) {
>>> @@ -275,11 +286,36 @@ int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma,
>>> struct ttm_bo_device *bdev,
>>>   }
>>>   EXPORT_SYMBOL(ttm_dma_tt_init);
>>>   +int ttm_sg_tt_init(struct ttm_dma_tt *ttm_dma, struct ttm_bo_device
>>> *bdev,
>>> +   unsigned long size, uint32_t page_flags)
>>> +{
>>> +struct ttm_tt *ttm = _dma->ttm;
>>> +int ret;
>>> +
>>> +ttm_tt_init_fields(ttm, bdev, size, page_flags);
>>> +
>>> +INIT_LIST_HEAD(_dma->pages_list);
>>> +if (page_flags & TTM_PAGE_FLAG_SG)
>>> +ret = ttm_sg_tt_alloc_page_directory(ttm_dma);
>>> +else
>>> +ret = ttm_dma_tt_alloc_page_directory(ttm_dma);
>>> +if (ret) {
>>> +ttm_tt_destroy(ttm);
>>> +pr_err("Failed allocating page table\n");
>>> +return -ENOMEM;
>>> +}
>>> +return 0;
>>> +}
>>> +EXPORT_SYMBOL(ttm_sg_tt_init);
>>> +
>>>   void ttm_dma_tt_fini(struct ttm_dma_tt *ttm_dma)
>>>   {
>>>   struct ttm_tt *ttm = _dma->ttm;
>>>   -kvfree(ttm->pages);
>>> +if (ttm->pages)
>>> +kvfree(ttm->pages);
>>> +else
>>> +kvfree(ttm_dma->dma_address);
>>>   ttm->pages = NULL;
>>>   ttm_dma->dma_address = NULL;
>>>   }
>>> diff --git a/include/drm/ttm/ttm_tt.h b/include/drm/ttm/ttm_tt.h
>>> index