Re: [Intel-gfx] [PATCH libdrm] intel: drm_intel_bo_gem_create_from_* on platforms w/o HW tiling

[Eric Engestrom]

On Monday, 2020-01-20 18:43:43 +0200, Imre Deak wrote:
> Platforms without a HW detiler doesn't support the get_tiling IOCTL.
> Fix the drm_intel_bo_gem_create_from_* functions assuming the default
> no-tiling, no-swizzling setting for the GEM buffer in this case.
> 
> Signed-off-by: Imre Deak 
> ---
>  intel/intel_bufmgr_gem.c | 42 +---
>  1 file changed, 26 insertions(+), 16 deletions(-)
> 
> diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
> index fbf48730..fc249ef1 100644
> --- a/intel/intel_bufmgr_gem.c
> +++ b/intel/intel_bufmgr_gem.c
> @@ -1069,6 +1069,27 @@ check_bo_alloc_userptr(drm_intel_bufmgr *bufmgr,
> tiling_mode, stride, size, flags);
>  }
>  
> +static int get_tiling_mode(drm_intel_bufmgr_gem *bufmgr_gem,
> +uint32_t gem_handle,
> +uint32_t *tiling_mode,
> +uint32_t *swizzle_mode)
> +{
> + struct drm_i915_gem_get_tiling get_tiling;
> + int ret;
> +
> + memclear(get_tiling);
> + ret = drmIoctl(bufmgr_gem->fd,
> +DRM_IOCTL_I915_GEM_GET_TILING,
> +_tiling);

You're missing `get_tiling.handle = gem_handle;`

Or better yet, just initialise `get_tiling` and get rid of the memclear():
  struct drm_i915_gem_get_tiling get_tiling = {
.handle = gem_handle,
  };

With either fix:
Reviewed-by: Eric Engestrom 

FYI, I've posted the following MR for the equivalent Mesa changes:
https://gitlab.freedesktop.org/mesa/mesa/merge_requests/3497

> + if (ret != 0 && errno != EOPNOTSUPP)
> + return ret;
> +
> + *tiling_mode = get_tiling.tiling_mode;
> + *swizzle_mode = get_tiling.swizzle_mode;
> +
> + return 0;
> +}
> +
>  /**
>   * Returns a drm_intel_bo wrapping the given buffer object handle.
>   *
> @@ -1084,7 +1105,6 @@ drm_intel_bo_gem_create_from_name(drm_intel_bufmgr 
> *bufmgr,
>   drm_intel_bo_gem *bo_gem;
>   int ret;
>   struct drm_gem_open open_arg;
> - struct drm_i915_gem_get_tiling get_tiling;
>  
>   /* At the moment most applications only have a few named bo.
>* For instance, in a DRI client only the render buffers passed
> @@ -1146,16 +1166,11 @@ drm_intel_bo_gem_create_from_name(drm_intel_bufmgr 
> *bufmgr,
>   HASH_ADD(name_hh, bufmgr_gem->name_table,
>global_name, sizeof(bo_gem->global_name), bo_gem);
>  
> - memclear(get_tiling);
> - get_tiling.handle = bo_gem->gem_handle;
> - ret = drmIoctl(bufmgr_gem->fd,
> -DRM_IOCTL_I915_GEM_GET_TILING,
> -_tiling);
> + ret = get_tiling_mode(bufmgr_gem, bo_gem->gem_handle,
> +   _gem->tiling_mode, _gem->swizzle_mode);
>   if (ret != 0)
>   goto err_unref;
>  
> - bo_gem->tiling_mode = get_tiling.tiling_mode;
> - bo_gem->swizzle_mode = get_tiling.swizzle_mode;
>   /* XXX stride is unknown */
>   drm_intel_bo_gem_set_in_aperture_size(bufmgr_gem, bo_gem, 0);
>   DBG("bo_create_from_handle: %d (%s)\n", handle, bo_gem->name);
> @@ -2634,7 +2649,6 @@ drm_intel_bo_gem_create_from_prime(drm_intel_bufmgr 
> *bufmgr, int prime_fd, int s
>   int ret;
>   uint32_t handle;
>   drm_intel_bo_gem *bo_gem;
> - struct drm_i915_gem_get_tiling get_tiling;
>  
>   pthread_mutex_lock(_gem->lock);
>   ret = drmPrimeFDToHandle(bufmgr_gem->fd, prime_fd, );
> @@ -2688,15 +2702,11 @@ drm_intel_bo_gem_create_from_prime(drm_intel_bufmgr 
> *bufmgr, int prime_fd, int s
>   bo_gem->has_error = false;
>   bo_gem->reusable = false;
>  
> - memclear(get_tiling);
> - get_tiling.handle = bo_gem->gem_handle;
> - if (drmIoctl(bufmgr_gem->fd,
> -  DRM_IOCTL_I915_GEM_GET_TILING,
> -  _tiling))
> + ret = get_tiling_mode(bufmgr_gem, handle,
> +   _gem->tiling_mode, _gem->swizzle_mode);
> + if (ret)
>   goto err;
>  
> - bo_gem->tiling_mode = get_tiling.tiling_mode;
> - bo_gem->swizzle_mode = get_tiling.swizzle_mode;
>   /* XXX stride is unknown */
>   drm_intel_bo_gem_set_in_aperture_size(bufmgr_gem, bo_gem, 0);
>  
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3] drm/print: add drm_debug_enabled()

[Eric Engestrom]

On Tuesday, 2019-10-01 17:06:14 +0300, Jani Nikula wrote:
> Add helper to check if a drm debug category is enabled. Convert drm core
> to use it. No functional changes.
> 
> v2: Move unlikely() to drm_debug_enabled() (Eric)
> 
> v3: Keep unlikely() when combined with other conditions (Eric)
> 
> Cc: Eric Engestrom 

Reviewed-by: Eric Engestrom 

> Acked-by: Alex Deucher 
> Signed-off-by: Jani Nikula 
> ---
>  drivers/gpu/drm/drm_atomic_uapi.c | 2 +-
>  drivers/gpu/drm/drm_dp_mst_topology.c | 6 +++---
>  drivers/gpu/drm/drm_edid.c| 2 +-
>  drivers/gpu/drm/drm_edid_load.c   | 2 +-
>  drivers/gpu/drm/drm_mipi_dbi.c| 4 ++--
>  drivers/gpu/drm/drm_print.c   | 4 ++--
>  drivers/gpu/drm/drm_vblank.c  | 6 +++---
>  include/drm/drm_print.h   | 5 +
>  8 files changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index 7a26bfb5329c..0d466d3b0809 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -1405,7 +1405,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>   } else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) {
>   ret = drm_atomic_nonblocking_commit(state);
>   } else {
> - if (unlikely(drm_debug & DRM_UT_STATE))
> + if (drm_debug_enabled(DRM_UT_STATE))
>   drm_atomic_print_state(state);
>  
>   ret = drm_atomic_commit(state);
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index e6801db54d0f..6b14b63b8d62 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1179,7 +1179,7 @@ static int drm_dp_mst_wait_tx_reply(struct 
> drm_dp_mst_branch *mstb,
>   }
>   }
>  out:
> - if (unlikely(ret == -EIO && drm_debug & DRM_UT_DP)) {
> + if (unlikely(ret == -EIO) && drm_debug_enabled(DRM_UT_DP)) {
>   struct drm_printer p = drm_debug_printer(DBG_PREFIX);
>  
>   drm_dp_mst_dump_sideband_msg_tx(, txmsg);
> @@ -2322,7 +2322,7 @@ static int process_single_tx_qlock(struct 
> drm_dp_mst_topology_mgr *mgr,
>   idx += tosend + 1;
>  
>   ret = drm_dp_send_sideband_msg(mgr, up, chunk, idx);
> - if (unlikely(ret && drm_debug & DRM_UT_DP)) {
> + if (unlikely(ret) && drm_debug_enabled(DRM_UT_DP)) {
>   struct drm_printer p = drm_debug_printer(DBG_PREFIX);
>  
>   drm_printf(, "sideband msg failed to send\n");
> @@ -2389,7 +2389,7 @@ static void drm_dp_queue_down_tx(struct 
> drm_dp_mst_topology_mgr *mgr,
>   mutex_lock(>qlock);
>   list_add_tail(>next, >tx_msg_downq);
>  
> - if (unlikely(drm_debug & DRM_UT_DP)) {
> + if (drm_debug_enabled(DRM_UT_DP)) {
>   struct drm_printer p = drm_debug_printer(DBG_PREFIX);
>  
>   drm_dp_mst_dump_sideband_msg_tx(, txmsg);
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 3c9703b08491..0552175313cb 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1651,7 +1651,7 @@ static void connector_bad_edid(struct drm_connector 
> *connector,
>  {
>   int i;
>  
> - if (connector->bad_edid_counter++ && !(drm_debug & DRM_UT_KMS))
> + if (connector->bad_edid_counter++ && !drm_debug_enabled(DRM_UT_KMS))
>   return;
>  
>   dev_warn(connector->dev->dev,
> diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
> index d38b3b255926..37d8ba3ddb46 100644
> --- a/drivers/gpu/drm/drm_edid_load.c
> +++ b/drivers/gpu/drm/drm_edid_load.c
> @@ -175,7 +175,7 @@ static void *edid_load(struct drm_connector *connector, 
> const char *name,
>   u8 *edid;
>   int fwsize, builtin;
>   int i, valid_extensions = 0;
> - bool print_bad_edid = !connector->bad_edid_counter || (drm_debug & 
> DRM_UT_KMS);
> + bool print_bad_edid = !connector->bad_edid_counter || 
> drm_debug_enabled(DRM_UT_KMS);
>  
>   builtin = match_string(generic_edid_name, GENERIC_EDIDS, name);
>   if (builtin >= 0) {
> diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
> index f8154316a3b0..ccfb5b33c5e3 100644
> --- a/drivers/gpu/drm/drm_mipi_dbi.c
> +++ b/drivers/gpu/drm/drm_mipi_dbi.c
> @@ -783,7 +783,7 @@ static int mipi_dbi_spi1e_transfer(struct mipi_dbi *dbi, 
> int dc,
>   int i, ret;
>   u8 *dst;
>  
> - if (drm_debug & DRM_UT_DRIVER)
> +

Re: [Intel-gfx] [PATCH v2 0/9] drm/print: add and use drm_debug_enabled()

[Eric Engestrom]

On Tuesday, 2019-10-01 14:03:55 +0300, Jani Nikula wrote:
> On Thu, 26 Sep 2019, Eric Engestrom  wrote:
> > On Tuesday, 2019-09-24 15:58:56 +0300, Jani Nikula wrote:
> >> Hi all, v2 of [1], a little refactoring around drm_debug access to
> >> abstract it better. There shouldn't be any functional changes.
> >> 
> >> I'd appreciate acks for merging the lot via drm-misc. If there are any
> >> objections to that, we'll need to postpone the last patch until
> >> everything has been merged and converted in drm-next.
> >> 
> >> BR,
> >> Jani.
> >> 
> >> Cc: Eric Engestrom 
> >> Cc: Alex Deucher 
> >> Cc: Christian König 
> >> Cc: David (ChunMing) Zhou 
> >> Cc: amd-...@lists.freedesktop.org
> >> Cc: Ben Skeggs 
> >> Cc: nouv...@lists.freedesktop.org
> >> Cc: Rob Clark 
> >> Cc: Sean Paul 
> >> Cc: linux-arm-...@vger.kernel.org
> >> Cc: freedr...@lists.freedesktop.org
> >> Cc: Francisco Jerez 
> >> Cc: Lucas Stach 
> >> Cc: Russell King 
> >> Cc: Christian Gmeiner 
> >> Cc: etna...@lists.freedesktop.org
> >> 
> >> 
> >> [1] cover.1568375189.git.jani.nikula@intel.com">http://mid.mail-archive.com/cover.1568375189.git.jani.nikula@intel.com
> >> 
> >> Jani Nikula (9):
> >>   drm/print: move drm_debug variable to drm_print.[ch]
> >>   drm/print: add drm_debug_enabled()
> >>   drm/i915: use drm_debug_enabled() to check for debug categories
> >>   drm/print: rename drm_debug to __drm_debug to discourage use
> >
> > The above four patches are:
> > Reviewed-by: Eric Engestrom 
> >
> > Did you check to make sure the `unlikely()` is propagated correctly
> > outside the `drm_debug_enabled()` call?
> 
> I did now.
> 
> Having drm_debug_enabled() as a macro vs. as an inline function does not
> seem to make a difference, so I think the inline is clearly preferrable.

Agreed :)

> 
> However, for example
> 
>   unlikely(foo && drm_debug & DRM_UT_DP)
> 
> does produce code different from
> 
>   (foo && drm_debug_enabled(DRM_UT_DP))
> 
> indicating that the unlikely() within drm_debug_enabled() does not
> propagate to the whole condition. It's possible to retain the same
> assembly output with
> 
>   (unlikely(foo) && drm_debug_enabled(DRM_UT_DP))
> 
> but it's unclear to me whether this is really worth it, either
> readability or performance wise.
> 
> Thoughts?

That kind of code only happens 2 times, both in
drivers/gpu/drm/drm_dp_mst_topology.c (in patch 2/9), right?

I think your suggestion is the right thing to do here:

-   if (unlikely(ret && drm_debug & DRM_UT_DP)) {
+   if (unlikely(ret) && drm_debug_enabled(DRM_UT_DP)) {

It doesn't really cost much in readability (especially compared to what
it was before), and whether it's important performance wise I couldn't
tell, but I think it's best to keep the code optimised as it was before
unless there's a reason to drop it.

Lyude might know more since she wrote 2f015ec6eab69301fdcf5, if you want
to ping her?

> 
> BR,
> Jani.
> 
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 0/9] drm/print: add and use drm_debug_enabled()

[Eric Engestrom]

On Tuesday, 2019-09-24 15:58:56 +0300, Jani Nikula wrote:
> Hi all, v2 of [1], a little refactoring around drm_debug access to
> abstract it better. There shouldn't be any functional changes.
> 
> I'd appreciate acks for merging the lot via drm-misc. If there are any
> objections to that, we'll need to postpone the last patch until
> everything has been merged and converted in drm-next.
> 
> BR,
> Jani.
> 
> Cc: Eric Engestrom 
> Cc: Alex Deucher 
> Cc: Christian König 
> Cc: David (ChunMing) Zhou 
> Cc: amd-...@lists.freedesktop.org
> Cc: Ben Skeggs 
> Cc: nouv...@lists.freedesktop.org
> Cc: Rob Clark 
> Cc: Sean Paul 
> Cc: linux-arm-...@vger.kernel.org
> Cc: freedr...@lists.freedesktop.org
> Cc: Francisco Jerez 
> Cc: Lucas Stach 
> Cc: Russell King 
> Cc: Christian Gmeiner 
> Cc: etna...@lists.freedesktop.org
> 
> 
> [1] cover.1568375189.git.jani.nikula@intel.com">http://mid.mail-archive.com/cover.1568375189.git.jani.nikula@intel.com
> 
> Jani Nikula (9):
>   drm/print: move drm_debug variable to drm_print.[ch]
>   drm/print: add drm_debug_enabled()
>   drm/i915: use drm_debug_enabled() to check for debug categories
>   drm/print: rename drm_debug to __drm_debug to discourage use

The above four patches are:
Reviewed-by: Eric Engestrom 

Did you check to make sure the `unlikely()` is propagated correctly
outside the `drm_debug_enabled()` call?
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/9] drm/print: add drm_debug_enabled()

[Eric Engestrom]

On Monday, 2019-09-16 16:23:13 +0300, Jani Nikula wrote:
> On Mon, 16 Sep 2019, Eric Engestrom  wrote:
> > On Monday, 2019-09-16 11:53:24 +0300, Jani Nikula wrote:
> >> On Fri, 13 Sep 2019, Eric Engestrom  wrote:
> >> > On Friday, 2019-09-13 14:51:39 +0300, Jani Nikula wrote:
> >> >> Add helper to check if a drm debug category is enabled. Convert drm core
> >> >> to use it. No functional changes.
> >> >> 
> >> >> Signed-off-by: Jani Nikula 
> >> >> ---
> >> >>  drivers/gpu/drm/drm_atomic_uapi.c | 2 +-
> >> >>  drivers/gpu/drm/drm_dp_mst_topology.c | 6 +++---
> >> >>  drivers/gpu/drm/drm_edid.c| 2 +-
> >> >>  drivers/gpu/drm/drm_edid_load.c   | 2 +-
> >> >>  drivers/gpu/drm/drm_mipi_dbi.c| 4 ++--
> >> >>  drivers/gpu/drm/drm_print.c   | 4 ++--
> >> >>  drivers/gpu/drm/drm_vblank.c  | 6 +++---
> >> >>  include/drm/drm_print.h   | 5 +
> >> >>  8 files changed, 18 insertions(+), 13 deletions(-)
> >> >> 
> >> >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
> >> >> b/drivers/gpu/drm/drm_atomic_uapi.c
> >> >> index 5a5b42db6f2a..6576cd997cbd 100644
> >> >> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> >> >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> >> >> @@ -1406,7 +1406,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> >> >> } else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) {
> >> >> ret = drm_atomic_nonblocking_commit(state);
> >> >> } else {
> >> >> -   if (unlikely(drm_debug & DRM_UT_STATE))
> >> >> +   if (unlikely(drm_debug_enabled(DRM_UT_STATE)))
> >> >> drm_atomic_print_state(state);
> >> >>  
> >> >> ret = drm_atomic_commit(state);
> >> >> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
> >> >> b/drivers/gpu/drm/drm_dp_mst_topology.c
> >> >> index 97216099a718..f47c5b6b51f7 100644
> >> >> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> >> >> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> >> >> @@ -1180,7 +1180,7 @@ static int drm_dp_mst_wait_tx_reply(struct 
> >> >> drm_dp_mst_branch *mstb,
> >> >> }
> >> >> }
> >> >>  out:
> >> >> -   if (unlikely(ret == -EIO && drm_debug & DRM_UT_DP)) {
> >> >> +   if (unlikely(ret == -EIO && drm_debug_enabled(DRM_UT_DP))) {
> >> >> struct drm_printer p = drm_debug_printer(DBG_PREFIX);
> >> >>  
> >> >> drm_dp_mst_dump_sideband_msg_tx(, txmsg);
> >> >> @@ -2321,7 +2321,7 @@ static int process_single_tx_qlock(struct 
> >> >> drm_dp_mst_topology_mgr *mgr,
> >> >> idx += tosend + 1;
> >> >>  
> >> >> ret = drm_dp_send_sideband_msg(mgr, up, chunk, idx);
> >> >> -   if (unlikely(ret && drm_debug & DRM_UT_DP)) {
> >> >> +   if (unlikely(ret && drm_debug_enabled(DRM_UT_DP))) {
> >> >> struct drm_printer p = drm_debug_printer(DBG_PREFIX);
> >> >>  
> >> >> drm_printf(, "sideband msg failed to send\n");
> >> >> @@ -2388,7 +2388,7 @@ static void drm_dp_queue_down_tx(struct 
> >> >> drm_dp_mst_topology_mgr *mgr,
> >> >> mutex_lock(>qlock);
> >> >> list_add_tail(>next, >tx_msg_downq);
> >> >>  
> >> >> -   if (unlikely(drm_debug & DRM_UT_DP)) {
> >> >> +   if (unlikely(drm_debug_enabled(DRM_UT_DP))) {
> >> >> struct drm_printer p = drm_debug_printer(DBG_PREFIX);
> >> >>  
> >> >> drm_dp_mst_dump_sideband_msg_tx(, txmsg);
> >> >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> >> >> index 12c783f4d956..58dad4d24cd4 100644
> >> >> --- a/drivers/gpu/drm/drm_edid.c
> >> >> +++ b/drivers/gpu/drm/drm_edid.c
> >> >> @@ -1551,7 +1551,7 @@ static void connector_bad_edid(struct 
> >> >> drm_connector *connector,
> >> >>  {
> >> >> int i;
> >> >>  

Re: [Intel-gfx] [PATCH 2/9] drm/print: add drm_debug_enabled()

[Eric Engestrom]

On Monday, 2019-09-16 11:53:24 +0300, Jani Nikula wrote:
> On Fri, 13 Sep 2019, Eric Engestrom  wrote:
> > On Friday, 2019-09-13 14:51:39 +0300, Jani Nikula wrote:
> >> Add helper to check if a drm debug category is enabled. Convert drm core
> >> to use it. No functional changes.
> >> 
> >> Signed-off-by: Jani Nikula 
> >> ---
> >>  drivers/gpu/drm/drm_atomic_uapi.c | 2 +-
> >>  drivers/gpu/drm/drm_dp_mst_topology.c | 6 +++---
> >>  drivers/gpu/drm/drm_edid.c| 2 +-
> >>  drivers/gpu/drm/drm_edid_load.c   | 2 +-
> >>  drivers/gpu/drm/drm_mipi_dbi.c| 4 ++--
> >>  drivers/gpu/drm/drm_print.c   | 4 ++--
> >>  drivers/gpu/drm/drm_vblank.c  | 6 +++---
> >>  include/drm/drm_print.h   | 5 +
> >>  8 files changed, 18 insertions(+), 13 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
> >> b/drivers/gpu/drm/drm_atomic_uapi.c
> >> index 5a5b42db6f2a..6576cd997cbd 100644
> >> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> >> @@ -1406,7 +1406,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> >>} else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) {
> >>ret = drm_atomic_nonblocking_commit(state);
> >>} else {
> >> -  if (unlikely(drm_debug & DRM_UT_STATE))
> >> +  if (unlikely(drm_debug_enabled(DRM_UT_STATE)))
> >>drm_atomic_print_state(state);
> >>  
> >>ret = drm_atomic_commit(state);
> >> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
> >> b/drivers/gpu/drm/drm_dp_mst_topology.c
> >> index 97216099a718..f47c5b6b51f7 100644
> >> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> >> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> >> @@ -1180,7 +1180,7 @@ static int drm_dp_mst_wait_tx_reply(struct 
> >> drm_dp_mst_branch *mstb,
> >>}
> >>}
> >>  out:
> >> -  if (unlikely(ret == -EIO && drm_debug & DRM_UT_DP)) {
> >> +  if (unlikely(ret == -EIO && drm_debug_enabled(DRM_UT_DP))) {
> >>struct drm_printer p = drm_debug_printer(DBG_PREFIX);
> >>  
> >>drm_dp_mst_dump_sideband_msg_tx(, txmsg);
> >> @@ -2321,7 +2321,7 @@ static int process_single_tx_qlock(struct 
> >> drm_dp_mst_topology_mgr *mgr,
> >>idx += tosend + 1;
> >>  
> >>ret = drm_dp_send_sideband_msg(mgr, up, chunk, idx);
> >> -  if (unlikely(ret && drm_debug & DRM_UT_DP)) {
> >> +  if (unlikely(ret && drm_debug_enabled(DRM_UT_DP))) {
> >>struct drm_printer p = drm_debug_printer(DBG_PREFIX);
> >>  
> >>drm_printf(, "sideband msg failed to send\n");
> >> @@ -2388,7 +2388,7 @@ static void drm_dp_queue_down_tx(struct 
> >> drm_dp_mst_topology_mgr *mgr,
> >>mutex_lock(>qlock);
> >>list_add_tail(>next, >tx_msg_downq);
> >>  
> >> -  if (unlikely(drm_debug & DRM_UT_DP)) {
> >> +  if (unlikely(drm_debug_enabled(DRM_UT_DP))) {
> >>struct drm_printer p = drm_debug_printer(DBG_PREFIX);
> >>  
> >>drm_dp_mst_dump_sideband_msg_tx(, txmsg);
> >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> >> index 12c783f4d956..58dad4d24cd4 100644
> >> --- a/drivers/gpu/drm/drm_edid.c
> >> +++ b/drivers/gpu/drm/drm_edid.c
> >> @@ -1551,7 +1551,7 @@ static void connector_bad_edid(struct drm_connector 
> >> *connector,
> >>  {
> >>int i;
> >>  
> >> -  if (connector->bad_edid_counter++ && !(drm_debug & DRM_UT_KMS))
> >> +  if (connector->bad_edid_counter++ && !drm_debug_enabled(DRM_UT_KMS))
> >>return;
> >>  
> >>dev_warn(connector->dev->dev,
> >> diff --git a/drivers/gpu/drm/drm_edid_load.c 
> >> b/drivers/gpu/drm/drm_edid_load.c
> >> index d38b3b255926..37d8ba3ddb46 100644
> >> --- a/drivers/gpu/drm/drm_edid_load.c
> >> +++ b/drivers/gpu/drm/drm_edid_load.c
> >> @@ -175,7 +175,7 @@ static void *edid_load(struct drm_connector 
> >> *connector, const char *name,
> >>u8 *edid;
> >>int fwsize, builtin;
> >>int i, valid_extensions = 0;
> >> -  bool print_bad_edid = !connector->bad_edid_counter || (drm_debug & 
> >> DRM_UT_KMS);
> &g

Re: [Intel-gfx] [PATCH 2/9] drm/print: add drm_debug_enabled()

[Eric Engestrom]

On Friday, 2019-09-13 14:51:39 +0300, Jani Nikula wrote:
> Add helper to check if a drm debug category is enabled. Convert drm core
> to use it. No functional changes.
> 
> Signed-off-by: Jani Nikula 
> ---
>  drivers/gpu/drm/drm_atomic_uapi.c | 2 +-
>  drivers/gpu/drm/drm_dp_mst_topology.c | 6 +++---
>  drivers/gpu/drm/drm_edid.c| 2 +-
>  drivers/gpu/drm/drm_edid_load.c   | 2 +-
>  drivers/gpu/drm/drm_mipi_dbi.c| 4 ++--
>  drivers/gpu/drm/drm_print.c   | 4 ++--
>  drivers/gpu/drm/drm_vblank.c  | 6 +++---
>  include/drm/drm_print.h   | 5 +
>  8 files changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index 5a5b42db6f2a..6576cd997cbd 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -1406,7 +1406,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>   } else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) {
>   ret = drm_atomic_nonblocking_commit(state);
>   } else {
> - if (unlikely(drm_debug & DRM_UT_STATE))
> + if (unlikely(drm_debug_enabled(DRM_UT_STATE)))
>   drm_atomic_print_state(state);
>  
>   ret = drm_atomic_commit(state);
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 97216099a718..f47c5b6b51f7 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1180,7 +1180,7 @@ static int drm_dp_mst_wait_tx_reply(struct 
> drm_dp_mst_branch *mstb,
>   }
>   }
>  out:
> - if (unlikely(ret == -EIO && drm_debug & DRM_UT_DP)) {
> + if (unlikely(ret == -EIO && drm_debug_enabled(DRM_UT_DP))) {
>   struct drm_printer p = drm_debug_printer(DBG_PREFIX);
>  
>   drm_dp_mst_dump_sideband_msg_tx(, txmsg);
> @@ -2321,7 +2321,7 @@ static int process_single_tx_qlock(struct 
> drm_dp_mst_topology_mgr *mgr,
>   idx += tosend + 1;
>  
>   ret = drm_dp_send_sideband_msg(mgr, up, chunk, idx);
> - if (unlikely(ret && drm_debug & DRM_UT_DP)) {
> + if (unlikely(ret && drm_debug_enabled(DRM_UT_DP))) {
>   struct drm_printer p = drm_debug_printer(DBG_PREFIX);
>  
>   drm_printf(, "sideband msg failed to send\n");
> @@ -2388,7 +2388,7 @@ static void drm_dp_queue_down_tx(struct 
> drm_dp_mst_topology_mgr *mgr,
>   mutex_lock(>qlock);
>   list_add_tail(>next, >tx_msg_downq);
>  
> - if (unlikely(drm_debug & DRM_UT_DP)) {
> + if (unlikely(drm_debug_enabled(DRM_UT_DP))) {
>   struct drm_printer p = drm_debug_printer(DBG_PREFIX);
>  
>   drm_dp_mst_dump_sideband_msg_tx(, txmsg);
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 12c783f4d956..58dad4d24cd4 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1551,7 +1551,7 @@ static void connector_bad_edid(struct drm_connector 
> *connector,
>  {
>   int i;
>  
> - if (connector->bad_edid_counter++ && !(drm_debug & DRM_UT_KMS))
> + if (connector->bad_edid_counter++ && !drm_debug_enabled(DRM_UT_KMS))
>   return;
>  
>   dev_warn(connector->dev->dev,
> diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
> index d38b3b255926..37d8ba3ddb46 100644
> --- a/drivers/gpu/drm/drm_edid_load.c
> +++ b/drivers/gpu/drm/drm_edid_load.c
> @@ -175,7 +175,7 @@ static void *edid_load(struct drm_connector *connector, 
> const char *name,
>   u8 *edid;
>   int fwsize, builtin;
>   int i, valid_extensions = 0;
> - bool print_bad_edid = !connector->bad_edid_counter || (drm_debug & 
> DRM_UT_KMS);
> + bool print_bad_edid = !connector->bad_edid_counter || 
> drm_debug_enabled(DRM_UT_KMS);
>  
>   builtin = match_string(generic_edid_name, GENERIC_EDIDS, name);
>   if (builtin >= 0) {
> diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
> index f8154316a3b0..ccfb5b33c5e3 100644
> --- a/drivers/gpu/drm/drm_mipi_dbi.c
> +++ b/drivers/gpu/drm/drm_mipi_dbi.c
> @@ -783,7 +783,7 @@ static int mipi_dbi_spi1e_transfer(struct mipi_dbi *dbi, 
> int dc,
>   int i, ret;
>   u8 *dst;
>  
> - if (drm_debug & DRM_UT_DRIVER)
> + if (drm_debug_enabled(DRM_UT_DRIVER))
>   pr_debug("[drm:%s] dc=%d, max_chunk=%zu, transfers:\n",
>__func__, dc, max_chunk);
>  
> @@ -907,7 +907,7 @@ static int mipi_dbi_spi1_transfer(struct mipi_dbi *dbi, 
> int dc,
>   max_chunk = dbi->tx_buf9_len;
>   dst16 = dbi->tx_buf9;
>  
> - if (drm_debug & DRM_UT_DRIVER)
> + if (drm_debug_enabled(DRM_UT_DRIVER))
>   pr_debug("[drm:%s] dc=%d, max_chunk=%zu, transfers:\n",
>__func__, dc, max_chunk);
>  
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index 

Re: [Intel-gfx] [PATCH] drm/todo: Update drm_gem_object_funcs todo even more

[Eric Engestrom]

On Tuesday, 2019-06-18 16:02:41 +0200, Daniel Vetter wrote:
> I rushed merging this a bit too much, and Noralf pointed out that
> we're a lot better already and have made great progress.
> 
> Let's try again.
> 
> Fixes: 42dbbb4b54a3 ("drm/todo: Add new debugfs todo")
> Cc: Greg Kroah-Hartman 
> Cc: Daniel Vetter 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: Maarten Lankhorst 
> Cc: Maxime Ripard 
> Cc: Sean Paul 
> Cc: Thomas Zimmermann 
> Cc: Gerd Hoffmann 
> Cc: Rob Herring 
> Cc: Noralf Trønnes 
> Cc: Eric Anholt 
> Cc: Gerd Hoffmann 
> Signed-off-by: Daniel Vetter 
> ---
>  Documentation/gpu/todo.rst | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 25878dd048fd..66c123737c3d 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -212,9 +212,11 @@ struct drm_gem_object_funcs
>  GEM objects can now have a function table instead of having the callbacks on 
> the
>  DRM driver struct. This is now the preferred way and drivers can be moved 
> over.
>  
> -Unfortunately some of the recently added GEM helpers are going in the wrong
> -direction by adding OPS macros that use the old, deprecated hooks. See
> -DRM_GEM_CMA_VMAP_DRIVER_OPS, DRM_GEM_SHMEM_DRIVER_OPS, and 
> DRM_GEM_VRAM_DRIVER_PRIME.
> +DRM_GEM_CMA_VMAP_DRIVER_OPS, DRM_GEM_SHMEM_DRIVER_OPS already support this, 
> but
> +DRM_GEM_VRAM_DRIVER_PRIME does not yet and needs to be aligend with the 
> previous

s/aligend/aligned/

> +two. We also need a 2nd version of the CMA define that doesn't require the
> +vmapping to be present (different hook for prime importing). Plus this needs 
> to
> +be rolled out to all drivers using their own implementations, too.
>  
>  Use DRM_MODESET_LOCK_ALL_* helpers instead of boilerplate
>  -
> -- 
> 2.20.1
> 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH libdrm] headers: Sync with drm-next

[Eric Engestrom]

On Wednesday, 2019-04-10 21:49:33 -0400, Rob Clark wrote:
> On Tue, Apr 9, 2019 at 8:27 AM Eric Engestrom  
> wrote:
> > > > diff --git a/include/drm/msm_drm.h b/include/drm/msm_drm.h
> > > > index c06d0a5..91a16b3 100644
> > > > --- a/include/drm/msm_drm.h
> > > > +++ b/include/drm/msm_drm.h
> > > > @@ -105,14 +105,24 @@ struct drm_msm_gem_new {
> > > > __u32 handle; /* out */
> > > >  };
> > > >
> > > > -#define MSM_INFO_IOVA  0x01
> > > > -
> > > > -#define MSM_INFO_FLAGS (MSM_INFO_IOVA)
> > > > +/* Get or set GEM buffer info.  The requested value can be passed
> > > > + * directly in 'value', or for data larger than 64b 'value' is a
> > > > + * pointer to userspace buffer, with 'len' specifying the number of
> > > > + * bytes copied into that buffer.  For info returned by pointer,
> > > > + * calling the GEM_INFO ioctl with null 'value' will return the
> > > > + * required buffer size in 'len'
> > > > + */
> > > > +#define MSM_INFO_GET_OFFSET0x00   /* get mmap() offset, 
> > > > returned by value */
> > > > +#define MSM_INFO_GET_IOVA  0x01   /* get iova, returned by value */
> > > > +#define MSM_INFO_SET_NAME  0x02   /* set the debug name (by pointer) */
> > > > +#define MSM_INFO_GET_NAME  0x03   /* get debug name, returned by 
> > > > pointer */
> > > >
> > > >  struct drm_msm_gem_info {
> > > > __u32 handle; /* in */
> > > > -   __u32 flags;  /* in - combination of MSM_INFO_* flags */
> > > > -   __u64 offset; /* out, mmap() offset or iova */
> > > > +   __u32 info;   /* in - one of MSM_INFO_* */
> > > > +   __u64 value;  /* in or out */
> > > > +   __u32 len;/* in or out */
> > > > +   __u32 pad;
> >
> > freedreno/msm/msm_bo.c needs to be updated to reflect those changes.
> 
> 
> I think you can just rename flags->info and offset->value, the rest of
> the struct should be zero-initialized.. if in doubt you can check
> $mesa/src/freedreno/drm/msm_bo.c
> 
> side-note:  the libdrm_freedreno code was folded into mesa in 19.0, so
> at *some* point we can probably disable libdrm_freedreno build by
> default.

Right now, freedreno's `auto` enables it by default on arm and disables it on
everything else.

I always enable everything to at least build-test it, but Ayan was using
the defaults which is why he didn't see this issue at first.

Btw, the GitLab CI builds everything, so it hopefully won't bitrot unnoticed.

> (I'd kinda still like to keep the code around for some misc
> standalone tools I have, but that is the sort of thing where I can fix
> libdrm if it gets broken).  When to switch to disabled by default I
> guess comes down to how long we want to support mesa 18.x with latest
> libdrm??  Maybe after 19.1, since (selfishly motivated) that gives me
> a long enough window back in case I find myself needing to bisect for
> some regression..
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH libdrm] headers: Sync with drm-next

[Eric Engestrom]

On Tuesday, 2019-04-09 12:59:13 +0100, Eric Engestrom wrote:
> On Tuesday, 2019-04-09 11:35:14 +, Ayan Halder wrote:
> > Generated using make headers_install from the drm-next
> > tree - git://anongit.freedesktop.org/drm/drm
> > branch - drm-next
> > commit - 14d2bd53a47a7e1cb3e03d00a6b952734cf90f3f
> > 
> > The changes were as follows :-
> > 
> > core: (drm.h, drm_fourcc.h, drm_mode.h)
> > - Added 'struct drm_syncobj_transfer', 'struct drm_syncobj_timeline_wait' 
> > and 'struct drm_syncobj_timeline_array'
> > - Added various DRM_IOCTL_SYNCOBJ_ ioctls
> > - Added some new RGB and YUV formats
> > - Added 'DRM_FORMAT_MOD_VENDOR_ALLWINNER'
> > - Added 'SAMSUNG' and Arm's 'AFBC' and 'ALLWINNER' format modifiers
> > - Added 'struct drm_mode_rect'
> > 
> > i915:
> > - Added struct 'struct i915_user_extension' and various 'struct 
> > drm_i915_gem_context_'
> > - Added different modes of per-process Graphics Translation Table
> > 
> > msm:
> > - Added various get or set GEM buffer info flags
> > - Added some MSM_SUBMIT_BO_ macros
> > - Modified 'struct drm_msm_gem_info'
> > 
> > Signed-off-by: Ayan Kumar halder 
> 
> This looks sane, and applies cleanly :)
> Acked-by: Eric Engestrom 

Actually, revoking that, as this patch breaks the build; see below.

> 
> > ---
> >  include/drm/drm.h|  36 +++
> >  include/drm/drm_fourcc.h | 136 +++
> >  include/drm/drm_mode.h   |  23 -
> >  include/drm/i915_drm.h   | 237 
> > ---
> >  include/drm/msm_drm.h|  25 +++--
> >  5 files changed, 415 insertions(+), 42 deletions(-)
> > 
[snip]
> > diff --git a/include/drm/msm_drm.h b/include/drm/msm_drm.h
> > index c06d0a5..91a16b3 100644
> > --- a/include/drm/msm_drm.h
> > +++ b/include/drm/msm_drm.h
> > @@ -105,14 +105,24 @@ struct drm_msm_gem_new {
> > __u32 handle; /* out */
> >  };
> >  
> > -#define MSM_INFO_IOVA  0x01
> > -
> > -#define MSM_INFO_FLAGS (MSM_INFO_IOVA)
> > +/* Get or set GEM buffer info.  The requested value can be passed
> > + * directly in 'value', or for data larger than 64b 'value' is a
> > + * pointer to userspace buffer, with 'len' specifying the number of
> > + * bytes copied into that buffer.  For info returned by pointer,
> > + * calling the GEM_INFO ioctl with null 'value' will return the
> > + * required buffer size in 'len'
> > + */
> > +#define MSM_INFO_GET_OFFSET0x00   /* get mmap() offset, returned 
> > by value */
> > +#define MSM_INFO_GET_IOVA  0x01   /* get iova, returned by value */
> > +#define MSM_INFO_SET_NAME  0x02   /* set the debug name (by pointer) */
> > +#define MSM_INFO_GET_NAME  0x03   /* get debug name, returned by pointer */
> >  
> >  struct drm_msm_gem_info {
> > __u32 handle; /* in */
> > -   __u32 flags;  /* in - combination of MSM_INFO_* flags */
> > -   __u64 offset; /* out, mmap() offset or iova */
> > +   __u32 info;   /* in - one of MSM_INFO_* */
> > +   __u64 value;  /* in or out */
> > +   __u32 len;/* in or out */
> > +   __u32 pad;

freedreno/msm/msm_bo.c needs to be updated to reflect those changes.

> >  };
> >  
> >  #define MSM_PREP_READ0x01
> > @@ -188,8 +198,11 @@ struct drm_msm_gem_submit_cmd {
> >   */
> >  #define MSM_SUBMIT_BO_READ 0x0001
> >  #define MSM_SUBMIT_BO_WRITE0x0002
> > +#define MSM_SUBMIT_BO_DUMP 0x0004
> >  
> > -#define MSM_SUBMIT_BO_FLAGS(MSM_SUBMIT_BO_READ | 
> > MSM_SUBMIT_BO_WRITE)
> > +#define MSM_SUBMIT_BO_FLAGS(MSM_SUBMIT_BO_READ | \
> > +   MSM_SUBMIT_BO_WRITE | \
> > +   MSM_SUBMIT_BO_DUMP)
> >  
> >  struct drm_msm_gem_submit_bo {
> > __u32 flags;  /* in, mask of MSM_SUBMIT_BO_x */
> > -- 
> > 2.7.4
> > 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH libdrm] headers: Sync with drm-next

[Eric Engestrom]

On Tuesday, 2019-04-09 11:35:14 +, Ayan Halder wrote:
> Generated using make headers_install from the drm-next
> tree - git://anongit.freedesktop.org/drm/drm
> branch - drm-next
> commit - 14d2bd53a47a7e1cb3e03d00a6b952734cf90f3f
> 
> The changes were as follows :-
> 
> core: (drm.h, drm_fourcc.h, drm_mode.h)
> - Added 'struct drm_syncobj_transfer', 'struct drm_syncobj_timeline_wait' and 
> 'struct drm_syncobj_timeline_array'
> - Added various DRM_IOCTL_SYNCOBJ_ ioctls
> - Added some new RGB and YUV formats
> - Added 'DRM_FORMAT_MOD_VENDOR_ALLWINNER'
> - Added 'SAMSUNG' and Arm's 'AFBC' and 'ALLWINNER' format modifiers
> - Added 'struct drm_mode_rect'
> 
> i915:
> - Added struct 'struct i915_user_extension' and various 'struct 
> drm_i915_gem_context_'
> - Added different modes of per-process Graphics Translation Table
> 
> msm:
> - Added various get or set GEM buffer info flags
> - Added some MSM_SUBMIT_BO_ macros
> - Modified 'struct drm_msm_gem_info'
> 
> Signed-off-by: Ayan Kumar halder 

This looks sane, and applies cleanly :)
Acked-by: Eric Engestrom 

> ---
>  include/drm/drm.h|  36 +++
>  include/drm/drm_fourcc.h | 136 +++
>  include/drm/drm_mode.h   |  23 -
>  include/drm/i915_drm.h   | 237 
> ---
>  include/drm/msm_drm.h|  25 +++--
>  5 files changed, 415 insertions(+), 42 deletions(-)
> 
> diff --git a/include/drm/drm.h b/include/drm/drm.h
> index 85c685a..c893f3b 100644
> --- a/include/drm/drm.h
> +++ b/include/drm/drm.h
> @@ -729,8 +729,18 @@ struct drm_syncobj_handle {
>   __u32 pad;
>  };
>  
> +struct drm_syncobj_transfer {
> + __u32 src_handle;
> + __u32 dst_handle;
> + __u64 src_point;
> + __u64 dst_point;
> + __u32 flags;
> + __u32 pad;
> +};
> +
>  #define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL (1 << 0)
>  #define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT (1 << 1)
> +#define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE (1 << 2) /* wait for time 
> point to become available */
>  struct drm_syncobj_wait {
>   __u64 handles;
>   /* absolute timeout */
> @@ -741,12 +751,33 @@ struct drm_syncobj_wait {
>   __u32 pad;
>  };
>  
> +struct drm_syncobj_timeline_wait {
> + __u64 handles;
> + /* wait on specific timeline point for every handles*/
> + __u64 points;
> + /* absolute timeout */
> + __s64 timeout_nsec;
> + __u32 count_handles;
> + __u32 flags;
> + __u32 first_signaled; /* only valid when not waiting all */
> + __u32 pad;
> +};
> +
> +
>  struct drm_syncobj_array {
>   __u64 handles;
>   __u32 count_handles;
>   __u32 pad;
>  };
>  
> +struct drm_syncobj_timeline_array {
> + __u64 handles;
> + __u64 points;
> + __u32 count_handles;
> + __u32 pad;
> +};
> +
> +
>  /* Query current scanout sequence number */
>  struct drm_crtc_get_sequence {
>   __u32 crtc_id;  /* requested crtc_id */
> @@ -903,6 +934,11 @@ extern "C" {
>  #define DRM_IOCTL_MODE_GET_LEASE DRM_IOWR(0xC8, struct 
> drm_mode_get_lease)
>  #define DRM_IOCTL_MODE_REVOKE_LEASE  DRM_IOWR(0xC9, struct 
> drm_mode_revoke_lease)
>  
> +#define DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT  DRM_IOWR(0xCA, struct 
> drm_syncobj_timeline_wait)
> +#define DRM_IOCTL_SYNCOBJ_QUERY  DRM_IOWR(0xCB, struct 
> drm_syncobj_timeline_array)
> +#define DRM_IOCTL_SYNCOBJ_TRANSFER   DRM_IOWR(0xCC, struct 
> drm_syncobj_transfer)
> +#define DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNALDRM_IOWR(0xCD, struct 
> drm_syncobj_timeline_array)
> +
>  /**
>   * Device specific ioctls should only be in their respective headers
>   * The device specific ioctl range is from 0x40 to 0x9f.
> diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> index 139632b..3feeaa3 100644
> --- a/include/drm/drm_fourcc.h
> +++ b/include/drm/drm_fourcc.h
> @@ -144,6 +144,17 @@ extern "C" {
>  #define DRM_FORMAT_RGBA1010102   fourcc_code('R', 'A', '3', '0') /* 
> [31:0] R:G:B:A 10:10:10:2 little endian */
>  #define DRM_FORMAT_BGRA1010102   fourcc_code('B', 'A', '3', '0') /* 
> [31:0] B:G:R:A 10:10:10:2 little endian */
>  
> +/*
> + * Floating point 64bpp RGB
> + * IEEE 754-2008 binary16 half-precision float
> + * [15:0] sign:exponent:mantissa 1:5:10
> + */
> +#define DRM_FORMAT_XRGB16161616F fourcc_code('X', 'R', '4', 'H') /* [63:0] 
> x:R:G:B 16:16:16:16 little endian */
> +#define DRM_FORMAT_XBGR16161616F fourcc_code('X', 'B', '4', 'H') /* [63:0] 
> x:B:G:R 16:16:16:16 little endian */
> +
> +#define DRM_FORMAT_ARGB16161616F fourcc_code

Re: [Intel-gfx] [PATCH libdrm] headers: Sync with drm-next

[Eric Engestrom]

On Monday, 2019-04-08 13:44:17 +, Ayan Halder wrote:
> Generated using make headers_install from the drm-next
> tree - git://anongit.freedesktop.org/drm/drm
> branch - drm-next
> commit - 14d2bd53a47a7e1cb3e03d00a6b952734cf90f3f
> 
> The changes were as follows :-
> 
> core: (drm.h, drm_fourcc.h, drm_mode.h)
> - Added 'struct drm_syncobj_transfer', 'struct drm_syncobj_timeline_wait' and 
> 'struct drm_syncobj_timeline_array'
> - Added various DRM_IOCTL_SYNCOBJ_ ioctls
> - Added some new RGB and YUV formats
> - Added 'DRM_FORMAT_MOD_VENDOR_ALLWINNER'
> - Added 'SAMSUNG' and Arm's 'AFBC' and 'ALLWINNER' format modifiers
> - Added 'struct drm_mode_rect'
> 
> i915:
> - Added struct 'struct i915_user_extension' and various 'struct 
> drm_i915_gem_context_'
> - Added different modes of per-process Graphics Translation Table
> 
> msm:
> - Added various get or set GEM buffer info flags
> - Added some MSM_SUBMIT_BO_ macros
> - Modified 'struct drm_msm_gem_info'
> 
> Signed-off-by: Ayan Kumar halder 
> ---
>  include/drm/drm.h|  36 +++
>  include/drm/drm_fourcc.h | 136 +++
>  include/drm/drm_mode.h   |  23 -
>  include/drm/i915_drm.h   | 237 
> ---
>  include/drm/msm_drm.h|  25 +++--
>  5 files changed, 415 insertions(+), 42 deletions(-)
> 
> diff --git a/include/drm/drm.h b/include/drm/drm.h
> index 85c685a..c893f3b 100644
> --- a/include/drm/drm.h
> +++ b/include/drm/drm.h
> @@ -729,8 +729,18 @@ struct drm_syncobj_handle {
>  __u32 pad;

All the tabs have been deleted, making this patch inapplicable and even
if it applied it would produce invalid code :)

Could you try again, maybe with a different MTA?

>  };
> 
> +struct drm_syncobj_transfer {
> +__u32 src_handle;
> +__u32 dst_handle;
> +__u64 src_point;
> +__u64 dst_point;
> +__u32 flags;
> +__u32 pad;
> +};
> +
>  #define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL (1 << 0)
>  #define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT (1 << 1)
> +#define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE (1 << 2) /* wait for time 
> point to become available */
>  struct drm_syncobj_wait {
>  __u64 handles;
>  /* absolute timeout */
> @@ -741,12 +751,33 @@ struct drm_syncobj_wait {
>  __u32 pad;
>  };
> 
> +struct drm_syncobj_timeline_wait {
> +__u64 handles;
> +/* wait on specific timeline point for every handles*/
> +__u64 points;
> +/* absolute timeout */
> +__s64 timeout_nsec;
> +__u32 count_handles;
> +__u32 flags;
> +__u32 first_signaled; /* only valid when not waiting all */
> +__u32 pad;
> +};
> +
> +
>  struct drm_syncobj_array {
>  __u64 handles;
>  __u32 count_handles;
>  __u32 pad;
>  };
> 
> +struct drm_syncobj_timeline_array {
> +__u64 handles;
> +__u64 points;
> +__u32 count_handles;
> +__u32 pad;
> +};
> +
> +
>  /* Query current scanout sequence number */
>  struct drm_crtc_get_sequence {
>  __u32 crtc_id;/* requested crtc_id */
> @@ -903,6 +934,11 @@ extern "C" {
>  #define DRM_IOCTL_MODE_GET_LEASEDRM_IOWR(0xC8, struct drm_mode_get_lease)
>  #define DRM_IOCTL_MODE_REVOKE_LEASEDRM_IOWR(0xC9, struct 
> drm_mode_revoke_lease)
> 
> +#define DRM_IOCTL_SYNCOBJ_TIMELINE_WAITDRM_IOWR(0xCA, struct 
> drm_syncobj_timeline_wait)
> +#define DRM_IOCTL_SYNCOBJ_QUERYDRM_IOWR(0xCB, struct 
> drm_syncobj_timeline_array)
> +#define DRM_IOCTL_SYNCOBJ_TRANSFERDRM_IOWR(0xCC, struct drm_syncobj_transfer)
> +#define DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNALDRM_IOWR(0xCD, struct 
> drm_syncobj_timeline_array)
> +
>  /**
>   * Device specific ioctls should only be in their respective headers
>   * The device specific ioctl range is from 0x40 to 0x9f.
> diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> index 139632b..3feeaa3 100644
> --- a/include/drm/drm_fourcc.h
> +++ b/include/drm/drm_fourcc.h
> @@ -144,6 +144,17 @@ extern "C" {
>  #define DRM_FORMAT_RGBA1010102fourcc_code('R', 'A', '3', '0') /* [31:0] 
> R:G:B:A 10:10:10:2 little endian */
>  #define DRM_FORMAT_BGRA1010102fourcc_code('B', 'A', '3', '0') /* [31:0] 
> B:G:R:A 10:10:10:2 little endian */
> 
> +/*
> + * Floating point 64bpp RGB
> + * IEEE 754-2008 binary16 half-precision float
> + * [15:0] sign:exponent:mantissa 1:5:10
> + */
> +#define DRM_FORMAT_XRGB16161616F fourcc_code('X', 'R', '4', 'H') /* [63:0] 
> x:R:G:B 16:16:16:16 little endian */
> +#define DRM_FORMAT_XBGR16161616F fourcc_code('X', 'B', '4', 'H') /* [63:0] 
> x:B:G:R 16:16:16:16 little endian */
> +
> +#define DRM_FORMAT_ARGB16161616F fourcc_code('A', 'R', '4', 'H') /* [63:0] 
> A:R:G:B 16:16:16:16 little endian */
> +#define DRM_FORMAT_ABGR16161616F fourcc_code('A', 'B', '4', 'H') /* [63:0] 
> A:B:G:R 16:16:16:16 little endian */
> +
>  /* packed YCbCr */
>  #define DRM_FORMAT_YUYVfourcc_code('Y', 'U', 'Y', 'V') /* [31:0] 
> Cr0:Y1:Cb0:Y0 8:8:8:8 little endian */
>  #define DRM_FORMAT_YVYUfourcc_code('Y', 'V', 'Y', 'U') /* [31:0] 
> Cb0:Y1:Cr0:Y0 8:8:8:8 little endian */
> @@ -151,6 +162,52 @@ extern "C" {
>  #define DRM_FORMAT_VYUYfourcc_code('V', 'Y', 'U', 'Y') /* [31:0] 

[Intel-gfx] [PATCH 2/2] i915: rename modifiers to follow the naming convention

[Eric Engestrom]

$ sed -i s/I915_FORMAT_MOD_/DRM_FORMAT_MOD_INTEL_/g $(git grep -l 
I915_FORMAT_MOD_)
$ git checkout include/uapi/drm/drm_fourcc.h

Signed-off-by: Eric Engestrom 
---
 drivers/gpu/drm/i915/intel_atomic_plane.c |  12 +-
 drivers/gpu/drm/i915/intel_display.c  | 128 +++---
 drivers/gpu/drm/i915/intel_pm.c   |  26 ++---
 drivers/gpu/drm/i915/intel_sprite.c   |  58 +-
 4 files changed, 112 insertions(+), 112 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c 
b/drivers/gpu/drm/i915/intel_atomic_plane.c
index fa7df5fe154bf06bdfc5..c26f0b25afa2dc8c3a3c 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -126,8 +126,8 @@ int intel_plane_atomic_check_with_state(const struct 
intel_crtc_state *old_crtc_
if (state->fb && drm_rotation_90_or_270(state->rotation)) {
struct drm_format_name_buf format_name;
 
-   if (state->fb->modifier != I915_FORMAT_MOD_Y_TILED &&
-   state->fb->modifier != I915_FORMAT_MOD_Yf_TILED) {
+   if (state->fb->modifier != DRM_FORMAT_MOD_INTEL_Y_TILED &&
+   state->fb->modifier != DRM_FORMAT_MOD_INTEL_Yf_TILED) {
DRM_DEBUG_KMS("Y/Yf tiling required for 90/270!\n");
return -EINVAL;
}
@@ -169,10 +169,10 @@ int intel_plane_atomic_check_with_state(const struct 
intel_crtc_state *old_crtc_
 */
if (state->fb && INTEL_GEN(dev_priv) >= 9 && crtc_state->base.enable &&
adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) {
-   if (state->fb->modifier == I915_FORMAT_MOD_Y_TILED ||
-   state->fb->modifier == I915_FORMAT_MOD_Yf_TILED ||
-   state->fb->modifier == I915_FORMAT_MOD_Y_TILED_CCS ||
-   state->fb->modifier == I915_FORMAT_MOD_Yf_TILED_CCS) {
+   if (state->fb->modifier == DRM_FORMAT_MOD_INTEL_Y_TILED ||
+   state->fb->modifier == DRM_FORMAT_MOD_INTEL_Yf_TILED ||
+   state->fb->modifier == DRM_FORMAT_MOD_INTEL_Y_TILED_CCS ||
+   state->fb->modifier == DRM_FORMAT_MOD_INTEL_Yf_TILED_CCS) {
DRM_DEBUG_KMS("Y/Yf tiling not supported in IF-ID 
mode\n");
return -EINVAL;
}
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index b2bab57cd113f2293850..087302d655f9a146846a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -69,7 +69,7 @@ static const uint32_t i965_primary_formats[] = {
 };
 
 static const uint64_t i9xx_format_modifiers[] = {
-   I915_FORMAT_MOD_X_TILED,
+   DRM_FORMAT_MOD_INTEL_X_TILED,
DRM_FORMAT_MOD_LINEAR,
DRM_FORMAT_MOD_INVALID
 };
@@ -106,19 +106,19 @@ static const uint32_t skl_pri_planar_formats[] = {
 };
 
 static const uint64_t skl_format_modifiers_noccs[] = {
-   I915_FORMAT_MOD_Yf_TILED,
-   I915_FORMAT_MOD_Y_TILED,
-   I915_FORMAT_MOD_X_TILED,
+   DRM_FORMAT_MOD_INTEL_Yf_TILED,
+   DRM_FORMAT_MOD_INTEL_Y_TILED,
+   DRM_FORMAT_MOD_INTEL_X_TILED,
DRM_FORMAT_MOD_LINEAR,
DRM_FORMAT_MOD_INVALID
 };
 
 static const uint64_t skl_format_modifiers_ccs[] = {
-   I915_FORMAT_MOD_Yf_TILED_CCS,
-   I915_FORMAT_MOD_Y_TILED_CCS,
-   I915_FORMAT_MOD_Yf_TILED,
-   I915_FORMAT_MOD_Y_TILED,
-   I915_FORMAT_MOD_X_TILED,
+   DRM_FORMAT_MOD_INTEL_Yf_TILED_CCS,
+   DRM_FORMAT_MOD_INTEL_Y_TILED_CCS,
+   DRM_FORMAT_MOD_INTEL_Yf_TILED,
+   DRM_FORMAT_MOD_INTEL_Y_TILED,
+   DRM_FORMAT_MOD_INTEL_X_TILED,
DRM_FORMAT_MOD_LINEAR,
DRM_FORMAT_MOD_INVALID
 };
@@ -1925,25 +1925,25 @@ intel_tile_width_bytes(const struct drm_framebuffer 
*fb, int plane)
switch (fb->modifier) {
case DRM_FORMAT_MOD_LINEAR:
return cpp;
-   case I915_FORMAT_MOD_X_TILED:
+   case DRM_FORMAT_MOD_INTEL_X_TILED:
if (IS_GEN2(dev_priv))
return 128;
else
return 512;
-   case I915_FORMAT_MOD_Y_TILED_CCS:
+   case DRM_FORMAT_MOD_INTEL_Y_TILED_CCS:
if (plane == 1)
return 128;
/* fall through */
-   case I915_FORMAT_MOD_Y_TILED:
+   case DRM_FORMAT_MOD_INTEL_Y_TILED:
if (IS_GEN2(dev_priv) || HAS_128_BYTE_Y_TILING(dev_priv))
return 128;
else
return 512;
-   case I915_FORMAT_MOD_Yf_TILED_CCS:
+   case DRM_FORMAT_MOD_INTEL_Yf_TILED_CCS:
if (plane == 1)
return 128;
/* fall through */
-   case I915_F

Re: [Intel-gfx] [Mesa-dev] [PATCH libdrm] Add basic CONTRIBUTING file

[Eric Engestrom]

On Tuesday, 2018-09-04 16:24:44 +1000, Dave Airlie wrote:
> On Mon, 3 Sep 2018 at 18:47, Daniel Vetter  wrote:
> >
> > I picked up a bunch of the pieces from wayland's version:
> >
> > https://gitlab.freedesktop.org/wayland/wayland/blob/master/CONTRIBUTING.md
> >
> > The weston one is fairly similar. Then I rather massively trimmed it
> > down since in reality libdrm is a bit a dumping ground with very few
> > real rules. The commit rights and CoC sections I've copied verbatim
> > from igt respectively drm-misc. Weston/Wayland only differ in their
> > pick of how many patches you need (10 instead of 5). I think for
> > libdrm this is supremely relevant, since most everyone will get their
> > commit rights by contributing already to the kernel or mesa and having
> > commit rights there already.
> >
> > Anyway, I figured this is good to get the rules documented, even if
> > there's mostly not many rules.
> >
> > Note: This references maintainers in a MAINTAINERS file, which needs
> > to be created first.
> >
> > Note: With the gitlab migration the entire commit rights process is
> > still a bit up in the air. But gitlab commit rights and roles are
> > hierarchical, so we can do libdrm-only maintainer/commiter roles
> > ("Owner" and "Developer" in gitlab-speak). This should avoid
> > conflating libdrm roles with mesa roles, useful for those pushing to
> > libdrm as primarily kernel contributors.
> 
> Fine with me,
> 
> Acked-by: Dave Airlie 
> 
> Dave.

I think this has gathered enough acks and rbs, you can just push it now
and if there's anything that should be adjusted we can do that as
a follow up :)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 04/12] cpufreq: use for_each_if

[Eric Engestrom]

On Monday, 2018-07-09 10:36:42 +0200, Daniel Vetter wrote:
> Avoids the inverted condition compared to the open coded version.
> 
> Signed-off-by: Daniel Vetter 
> Cc: "Rafael J. Wysocki" 
> Cc: Viresh Kumar 
> Cc: linux...@vger.kernel.org
> ---
>  include/linux/cpufreq.h | 8 ++--
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 882a9b9e34bc..f2028c229b96 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -649,9 +649,7 @@ static inline void dev_pm_opp_free_cpufreq_table(struct 
> device *dev,
>  
>  #define cpufreq_for_each_valid_entry(pos, table) \
>   for (pos = table; pos->frequency != CPUFREQ_TABLE_END; pos++)   \
> - if (pos->frequency == CPUFREQ_ENTRY_INVALID)\
> - continue;   \
> - else
> + for_each_if (pos->frequency != CPUFREQ_ENTRY_INVALID)
>  
>  /*
>   * cpufreq_for_each_valid_entry_idx - iterate with index over a cpufreq
> @@ -663,9 +661,7 @@ static inline void dev_pm_opp_free_cpufreq_table(struct 
> device *dev,
>  
>  #define cpufreq_for_each_valid_entry_idx(pos, table, idx)\
>   cpufreq_for_each_entry_idx(pos, table, idx) \
> - if (pos->frequency == CPUFREQ_ENTRY_INVALID)\
> - continue;   \
> - else
> + for_each_if (pos->frequency == CPUFREQ_ENTRY_INVALID)

Should be `!=` there ^

>  
>  
>  int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy,
> -- 
> 2.18.0
> 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [maintainer-tools PATCH 6/7] dim: split out 'is email cc'ed in latest commit' to a function

[Eric Engestrom]

Signed-off-by: Eric Engestrom <eric.engest...@imgtec.com>
---
 dim | 46 ++
 1 file changed, 26 insertions(+), 20 deletions(-)

diff --git a/dim b/dim
index 481a53e23035..2e22fefa8867 100755
--- a/dim
+++ b/dim
@@ -1970,6 +1970,31 @@ function email_get_name
fi
 }
 
+function email_cc_in_latest_commit
+{
+   email="$1"
+   name="$2"
+
+   git show -s | grep -i "^Cc:" | sed 's/^ *[Cc][Cc]: *//' | while 
read -r testcc; do
+   testemail="$(email_get_address "$testcc")"
+   testname="$(email_get_name "$testcc" | sed -e 
's/^[[:space:]]*//')"
+
+   if [ "$testemail" = "$email" ]; then
+   return 1
+   fi
+
+   if [ -z "$testname" ] || [ -z "$name" ]; then
+   continue
+   fi
+
+   if [ "$testname" = "$name" ]; then
+   return 1
+   fi
+   done || return 0
+
+   return 1
+}
+
 function dim_add_missing_cc
 {
if [ $(git cat-file -p HEAD | grep -cE ^parent) -ne 1 ]; then
@@ -1987,26 +2012,7 @@ function dim_add_missing_cc
continue
fi
 
-   # Variables from the while loop don't propagate,
-   # print out a 1 on success
-   matches=$(
-   git show -s | grep -i "^Cc:" | sed 's/^ *[Cc][Cc]: 
*//' | while read testcc; do
-   testemail="$(email_get_address "$testcc")"
-
-   if [ "$testemail" != "$email" ]; then
-   if [ -z "$name" ]; then continue; fi
-
-   testname="$(email_get_name "$testcc" | 
sed -e 's/^[[:space:]]*//')"
-
-   if [ "$testname" != "$name" ]; then 
continue; fi
-   fi
-
-   echo 1
-   break
-   done
-   )
-
-   if [ -z "$matches" ]; then
+   if ! email_cc_in_latest_commit "$email" "$name"; then
$DRY dim_commit_add_tag "Cc: ${cc}"
fi
done
-- 
Cheers,
  Eric

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

[Intel-gfx] [maintainer-tools PATCH 7/7] dim: protect against escaped chars when reading cc'ed emails

[Eric Engestrom]

Suggested by shellcheck (`make check`).

Signed-off-by: Eric Engestrom <eric.engest...@imgtec.com>
---
 dim | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dim b/dim
index 2e22fefa8867..75ce55188af4 100755
--- a/dim
+++ b/dim
@@ -2002,7 +2002,7 @@ function dim_add_missing_cc
return
fi
 
-   git show | scripts/get_maintainer.pl --email --norolestats 
--pattern-depth 1 | while read cc; do
+   git show | scripts/get_maintainer.pl --email --norolestats 
--pattern-depth 1 | while read -r cc; do
email="$(email_get_address "$cc")"
name="$(email_get_name "$cc")"
 
-- 
Cheers,
  Eric

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

[Intel-gfx] [maintainer-tools PATCH 3/7] dim: split out email parsing functions

[Eric Engestrom]

Signed-off-by: Eric Engestrom <eric.engest...@imgtec.com>
---
 dim | 18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/dim b/dim
index eaabcec43c8f..a656afa0d255 100755
--- a/dim
+++ b/dim
@@ -1958,6 +1958,16 @@ function dim_fixes
fi
 }
 
+function email_get_address
+{
+   sed -e 's/.*.*//' <<< "$1"
+}
+
+function email_get_name
+{
+   echo ${cc/<*/} | sed -e 's/[[:space:]]*$//' <<< "$1"
+}
+
 function dim_add_missing_cc
 {
if [ $(git cat-file -p HEAD | grep -cE ^parent) -ne 1 ]; then
@@ -1966,11 +1976,11 @@ function dim_add_missing_cc
fi
 
git show | scripts/get_maintainer.pl --email --norolestats 
--pattern-depth 1 | while read cc; do
-   email="$(echo "$cc" | sed -e 's/.*.*//')"
+   email="$(email_get_address "$cc")"
name=''
 
if echo "$cc" | grep -q '<'; then
-   name="$(echo ${cc/<*/} | sed -e 's/[[:space:]]*$//')";
+   name="$(email_get_name "$cc")";
fi
 
# Don't add main mailing lists
@@ -1983,12 +1993,12 @@ function dim_add_missing_cc
# print out a 1 on success
matches=$(
git show -s | grep -i "^Cc:" | sed 's/^ *[Cc][Cc]: 
*//' | while read testcc; do
-   testemail="$(echo "$testcc" | sed -e 's/.*.*//')"
+   testemail="$(email_get_address "$testcc")"
 
if [ "$testemail" != "$email" ]; then
if [ -z "$name" ]; then continue; fi
 
-   testname="$(echo ${testcc/<*/} | sed -e 
's/[[:space:]]*$//' -e 's/^[[:space:]]*//')"
+   testname="$(email_get_name "$testcc" | 
sed -e 's/^[[:space:]]*//')"
 
if [ "$testname" != "$name" ]; then 
continue; fi
fi
-- 
Cheers,
  Eric

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

[Intel-gfx] [maintainer-tools PATCH 4/7] dim: merge string substitutions

[Eric Engestrom]

Signed-off-by: Eric Engestrom <eric.engest...@imgtec.com>
---
 dim | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dim b/dim
index a656afa0d255..4ffac497c621 100755
--- a/dim
+++ b/dim
@@ -1965,7 +1965,7 @@ function email_get_address
 
 function email_get_name
 {
-   echo ${cc/<*/} | sed -e 's/[[:space:]]*$//' <<< "$1"
+   sed -e 's/[[:space:]]*<.*$//' <<< "$1"
 }
 
 function dim_add_missing_cc
-- 
Cheers,
  Eric

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

[Intel-gfx] [maintainer-tools PATCH 5/7] dim: move empty name logic to function

[Eric Engestrom]

Fair warning: this slightly changes the behaviour, as $testname would
previously contain the email if $testcc didn't contain a name.
Shouldn't affect anything though.

Signed-off-by: Eric Engestrom <eric.engest...@imgtec.com>
---
 dim | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/dim b/dim
index 4ffac497c621..481a53e23035 100755
--- a/dim
+++ b/dim
@@ -1965,7 +1965,9 @@ function email_get_address
 
 function email_get_name
 {
-   sed -e 's/[[:space:]]*<.*$//' <<< "$1"
+   if grep -q '<' <<< "$1"; then
+   sed -e 's/[[:space:]]*<.*$//' <<< "$1"
+   fi
 }
 
 function dim_add_missing_cc
@@ -1977,11 +1979,7 @@ function dim_add_missing_cc
 
git show | scripts/get_maintainer.pl --email --norolestats 
--pattern-depth 1 | while read cc; do
email="$(email_get_address "$cc")"
-   name=''
-
-   if echo "$cc" | grep -q '<'; then
-   name="$(email_get_name "$cc")";
-   fi
+   name="$(email_get_name "$cc")"
 
# Don't add main mailing lists
if [ "$email" = "dri-de...@lists.freedesktop.org" -o \
-- 
Cheers,
  Eric

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

[Intel-gfx] [maintainer-tools PATCH 2/7] dim: fix end-of-line in regex

[Eric Engestrom]

Signed-off-by: Eric Engestrom <eric.engest...@imgtec.com>
---
 dim | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/dim b/dim
index af1baa11c7b2..eaabcec43c8f 100755
--- a/dim
+++ b/dim
@@ -1970,7 +1970,7 @@ function dim_add_missing_cc
name=''
 
if echo "$cc" | grep -q '<'; then
-   name="$(echo ${cc/<*/} | sed -e 's/[[:space:]]*\$//')";
+   name="$(echo ${cc/<*/} | sed -e 's/[[:space:]]*$//')";
fi
 
# Don't add main mailing lists
@@ -1988,7 +1988,7 @@ function dim_add_missing_cc
if [ "$testemail" != "$email" ]; then
if [ -z "$name" ]; then continue; fi
 
-   testname="$(echo ${testcc/<*/} | sed -e 
's/[[:space:]]*\$//' -e 's/^[[:space:]]*//')"
+   testname="$(echo ${testcc/<*/} | sed -e 
's/[[:space:]]*$//' -e 's/^[[:space:]]*//')"
 
if [ "$testname" != "$name" ]; then 
continue; fi
fi
-- 
Cheers,
  Eric

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

[Intel-gfx] [maintainer-tools PATCH 1/7] dim: don't run add-missing-cc on merge commits

[Eric Engestrom]

get_maintainer.pl needs a diff, so this script can't run on a merge
commit.

Signed-off-by: Eric Engestrom <eric.engest...@imgtec.com>
---
 dim | 5 +
 1 file changed, 5 insertions(+)

diff --git a/dim b/dim
index 619d855b321b..af1baa11c7b2 100755
--- a/dim
+++ b/dim
@@ -1960,6 +1960,11 @@ function dim_fixes
 
 function dim_add_missing_cc
 {
+   if [ $(git cat-file -p HEAD | grep -cE ^parent) -ne 1 ]; then
+   echoerr "This script doesn't work on merge commits"
+   return
+   fi
+
git show | scripts/get_maintainer.pl --email --norolestats 
--pattern-depth 1 | while read cc; do
email="$(echo "$cc" | sed -e 's/.*.*//')"
name=''
-- 
Cheers,
  Eric

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

[Intel-gfx] [PATCH maintainer-tools 1/3] make: run check on current `dim`, not the installed one

[Eric Engestrom]

Signed-off-by: Eric Engestrom <eric.engest...@imgtec.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index b4cea98..4291049 100644
--- a/Makefile
+++ b/Makefile
@@ -33,7 +33,7 @@ shellcheck:
shellcheck $(SC_EXCLUDE) dim bash_completion
 
 mancheck:
-   @for cmd in $$(dim list-commands); do \
+   @for cmd in $$(./dim list-commands); do \
if ! grep -q "^$$cmd" dim.rst; then \
echo "$@: $$cmd not documented"; \
fi \
-- 
Cheers,
  Eric

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

[Intel-gfx] [PATCH maintainer-tools 3/3] dim.rst: update CONTRIBUTING instructions

[Eric Engestrom]

Suggest using git-config instead of a flag on format-patch.
While at it, use the more common "PATCH foo" subject prefix.

Signed-off-by: Eric Engestrom <eric.engest...@imgtec.com>
---
 dim.rst | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/dim.rst b/dim.rst
index b99248e..c650317 100644
--- a/dim.rst
+++ b/dim.rst
@@ -476,10 +476,11 @@ CONTRIBUTING
 
 
 Submit patches for any of the maintainer tools to the
-intel-gfx@lists.freedesktop.org mailing list with [maintainer-tools PATCH]
+intel-gfx@lists.freedesktop.org mailing list with [PATCH maintainer-tools]
 prefix. Use::
 
-  $ git format-patch --subject-prefix="maintainer-tools PATCH"
+  $ git config sendemail.to intel-gfx@lists.freedesktop.org
+  $ git config format.subjectprefix 'PATCH maintainer-tools'
 
 for that. Please make sure your patches pass the build and self tests by
 running::
-- 
Cheers,
  Eric

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

[Intel-gfx] [PATCH maintainer-tools 2/3] dim: send 'not configured' error to stderr

[Eric Engestrom]

This fixes `make check` when dim is not configured.

Signed-off-by: Eric Engestrom <eric.engest...@imgtec.com>
---
 dim | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dim b/dim
index 588e859..b373901 100755
--- a/dim
+++ b/dim
@@ -197,7 +197,7 @@ export __dim_running=1
 if [ "$subcommand" != "setup" ] && [ "$subcommand" != "help" ] && [ 
"$subcommand" != "usage" ]; then
for d in $DIM_PREFIX $DIM_PREFIX/$DIM_DRM_INTEL $DIM_PREFIX/drm-rerere 
$DIM_PREFIX/drm-tip; do
if [ ! -d $d ]; then
-   echo "$d is missing, please check your configuration 
and/or run dim setup"
+   echoerr "$d is missing, please check your configuration 
and/or run dim setup"
exit 1
fi
done
-- 
Cheers,
  Eric

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

Re: [Intel-gfx] [PATCH 1/3] drm: Document maintainer duties

[Eric Engestrom]

On Monday, 2017-03-27 10:45:44 +0200, Daniel Vetter wrote:
> I wanted to get Sean Paul to run the drm-misc show for a bit, for
> training reasons and to increase the bus factor. And then realized
> there's no docs about what maintainers are doing.
> 
> Fix that.
> 
> v2: Add backmerges and taking the blame.
> 
> Signed-off-by: Daniel Vetter 
> ---
>  drm-misc.rst | 36 
>  1 file changed, 36 insertions(+)
> 
> diff --git a/drm-misc.rst b/drm-misc.rst
> index 139d45e92edf..b6d01f2c7c2b 100644
> --- a/drm-misc.rst
> +++ b/drm-misc.rst
> @@ -142,6 +142,42 @@ Slightly different rules apply:
>more involved rework in follow-up work. This way lengthy review cycles get
>avoided, which are a drag for both reviewer and author.
>  
> +Maintainer's Duties
> +===
> +
> +Maintainers mostly provide services to keep drm-misc running smoothly:
> +
> +* Coordinate cross-subsystem depencies and handle topic branches, sending out

s/depencies/dependencies/

> +  pull request and merging topic pull requests from other subsystems.
> +
> +* At least once per week check for pending bugfixes (using ``dim status``) 
> and
> +  if there are any (either in `-fixes` or `-next-fixes`), send out the pull
> +  request.
> +
> +* Fast-forward (when possible) `-fixes` to each released -rc kernel tag, to
> +  keep it current. We try to avoid backmerges for bugfix branches, and 
> rebasing
> +  isn't an option with multiple committers.
> +
> +* During the merge-windo blackout, i.e. from -rc6 on until the merge window

s/windo/window/

> +  closes with the release of -rc1, try to track `drm-next` with the
> +  `-next-fixes` branch. Do not advance past -rc1, otherwise the automagic in
> +  the scripts will push the wrong patches to the linux-next tree.
> +
> +* Between -rc1 and -rc6 send pull requests for the `-next` branch every 1-2
> +  weeks, depending upon how much is queued up.
> +
> +* Backmerge `drm-next` into the `-next` branch when needed, properly 
> recording
> +  that reason in the merge commit message. Do a backmerge at least once per
> +  month to avoid conflict chaos, and specifically merge in the main drm 
> feature
> +  pull request, to resync with all the late driver submissions during the 
> merge
> +  window.
> +
> +* Last resort fallback for applying patches, in case all area expert 
> committers
> +  are somehow unavailable.
> +
> +* Take the blame when something goes wrong. Maintainers interface and 
> represent
> +  the entire group of committers to the wider kernel community.

:)

> +
>  Tooling
>  ===
>  
> -- 
> 2.11.0
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t v1] lib/drmtest: Add comment explaining DRIVER_ANY excluding DRIVER_VGEM

[Eric Engestrom]

On Monday, 2017-01-30 10:12:28 -0500, Robert Foss wrote:
> Signed-off-by: Robert Foss <robert.f...@collabora.com>
> ---
>  lib/drmtest.h | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/lib/drmtest.h b/lib/drmtest.h
> index 19d4bd19..c9c019c0 100644
> --- a/lib/drmtest.h
> +++ b/lib/drmtest.h
> @@ -42,8 +42,15 @@
>  #define DRIVER_VC4   (1 << 1)
>  #define DRIVER_VGEM  (1 << 2)
>  #define DRIVER_VIRTIO(1 << 3)
> +/*
> + * Exclude DRVER_VGEM from DRIVER_ANY since if you run on a system
> + * with vgem as well as a supported driver, you can end up with a
> + * near-100% skip rate if you don't explicitly specify the device,
> + * depending on device-load ordering.
> + */
>  #define DRIVER_ANY   ~(DRIVER_VGEM)

Brilliant, cheers :)
Reviewed-by: Eric Engestrom <eric.engest...@imgtec.com>

>  
> +
>  #ifdef ANDROID
>  #if (!(defined HAVE_MMAP64)) && (!(defined __x86_64__))
>  extern void*  __mmap2(void *, size_t, int, int, int, off_t);
> -- 
> 2.11.0.453.g787f75f05
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t] lib/drmtest: make DRIVER_ANY match any driver

[Eric Engestrom]

On Monday, 2017-01-30 11:50:52 +, Daniel Stone wrote:
> Hi,
> 
> On 30 January 2017 at 11:46, Petri Latvala  wrote:
> > NAK.
> >
> > DRIVER_VGEM is omitted from DRIVER_ANY intentionally. Vgem is unable
> > to modeset, unable to render, practically it only supports the
> > vgem-specific tests. See also: lib/drmtest.c, __drm_open_driver().
> 
> Yeah, I agree with this. It's mostly just there as an auxiliary
> helper. Opening DRIVER_ANY to vgem means that, if you run on a system
> with vgem as well as a supported driver, you can end up with a
> near-100% skip rate if you don't explicitly specify the device,
> depending on device-load ordering.
> 
> Cheers,
> Daniel

OK. This should probably be documented then (a simple comment next to
the #define would go a long way), so that the next guy who see this
doesn't make the same mistake I did :)

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

Re: [Intel-gfx] [PATCH 15/15] drm: Update kerneldoc for drm_crtc.[hc]

[Eric Engestrom]

On Wednesday, 2017-01-25 07:26:57 +0100, Daniel Vetter wrote:
> After going through all the trouble of splitting out parts from
> drm_crtc.[hc] and then properly documenting each I've entirely
> forgotten to show the same TLC for CRTCs themselves!
> 
> Let's make amends asap.
> 
> Signed-off-by: Daniel Vetter <daniel.vet...@intel.com>
> ---
>  Documentation/gpu/drm-kms.rst |  6 ++
>  drivers/gpu/drm/drm_crtc.c| 21 +
>  include/drm/drm_crtc.h| 25 +++--
>  3 files changed, 46 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index 35c41cf84a1b..979cee853bb1 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -284,6 +284,12 @@ Atomic Mode Setting Function Reference
>  CRTC Abstraction
>  
>  
> +.. kernel-doc:: drivers/gpu/drm/drm_crtc.c
> +   :doc: overview
> +
> +CRTC Functions Reference
> +
> +
>  .. kernel-doc:: include/drm/drm_crtc.h
> :internal:
>  
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 5f28e3a5a3e0..6915f897bd8e 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -47,6 +47,27 @@
>  #include "drm_internal.h"
>  
>  /**
> + * DOC: overview
> + *
> + * A CRTC represents the overall display pipeline. It receives pixel data 
> from
> + * _plane and blends them together. The _display_mode is also 
> attached
> + * to the CRTC, specifying display timings. On the output side the data is 
> fed
> + * to one or more _encoder, which are then each connected to one
> + * _connector.
> + *
> + * To create a CRTC, a KMS drivers allocates and zeroes an instances of
> + *  drm_crtc (possibly as part of a larger structure) and registers it
> + * with a call to drm_crtc_init_with_planes().
> + *
> + * The CRTC is also the entry point for legacy modeset operations, see
> + * _crtc_funcs.set_config, legacy plane operations, see
> + * _crtc_funcs.page_flip and _crtc_funcs.cursor_set2, and other 
> legacy
> + * operations like _crtc_funcs.gamma_set. For atomic drivers all these
> + * features are controlled through _property and
> + * _mode_config_funcs.atomic_check and 
> _mode_config_funcs.atomic_check.
> + */
> +
> +/**
>   * drm_crtc_from_index - find the registered CRTC at an index
>   * @dev: DRM device
>   * @idx: index of registered CRTC to find for
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 816edab054e6..d788c624f67a 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -641,7 +641,7 @@ struct drm_crtc {
>*
>* This provides a read lock for the overall crtc state (mode, dpms
>* state, ...) and a write lock for everything which can be update
> -  * without a full modeset (fb, cursor data, crtc properties ...). Full
> +  * without a full modeset (fb, cursor data, crtc properties ...). A full
>* modeset also need to grab _mode_config.connection_mutex.
>*/
>   struct drm_modeset_lock mutex;
> @@ -774,10 +774,8 @@ struct drm_crtc {
>   * @connectors: array of connectors to drive with this CRTC if possible
>   * @num_connectors: size of @connectors array
>   *
> - * Represents a single crtc the connectors that it drives with what mode
> - * and from which framebuffer it scans out from.
> - *
> - * This is used to set modes.
> + * This represents a modeset configuration for the legacy SETCRTC ioctl and 
> is
> + * also used internally. Atomic drivers instead use _atomic_state.
>   */
>  struct drm_mode_set {
>   struct drm_framebuffer *fb;
> @@ -832,7 +830,15 @@ int drm_crtc_force_disable_all(struct drm_device *dev);
>  int drm_mode_set_config_internal(struct drm_mode_set *set);
>  struct drm_crtc *drm_crtc_from_index(struct drm_device *dev, int idx);
>  
> -/* Helpers */
> +/**
> + * drm_crtc_find - look up a CRTC object from its ID
> + * @dev: DRM device
> + * @id: _mode_object ID
> + *
> + * This can be used to look up a CRTC from its userspace ID. Only used by
> + * drivers for legacy IOCTLs and interface, nowadays extensions to the KMS
> + * userspace interface should be done using _property.
> + */
>  static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev,
>   uint32_t id)
>  {
> @@ -841,6 +847,13 @@ static inline struct drm_crtc *drm_crtc_find(struct 
> drm_device *dev,
>   return mo ? obj_to_crtc(mo) : NULL;
>  }
>  
> +/**
> + * drm_for_each_crtc - iterate over all encoders

s/encoder/crtc/ :)

Other than that, this and #3 (btw I know you didn't intro

Re: [Intel-gfx] [PATCH 03/15] drm/kms-core: Use recommened kerneldoc for struct member refs

[Eric Engestrom]

On Wednesday, 2017-01-25 07:26:45 +0100, Daniel Vetter wrote:
> I just learned that _name.member_name works and looks pretty
> even. It doesn't (yet) link to the member directly though, which would
> be really good for big structures or vfunc tables (where the
> per-member kerneldoc tends to be long).
> 
> Also some minor drive-by polish where it makes sense, I read a lot
> of docs ...
> 
> Cc: Jani Nikula 
> Cc: Chris Wilson 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_atomic.c| 71 
> ++---
>  drivers/gpu/drm/drm_blend.c | 11 +++---
>  drivers/gpu/drm/drm_connector.c | 12 +++
>  drivers/gpu/drm/drm_crtc.c  |  7 ++--
>  drivers/gpu/drm/drm_dumb_buffers.c  |  4 +--
>  drivers/gpu/drm/drm_encoder.c   |  2 +-
>  drivers/gpu/drm/drm_encoder_slave.c |  2 +-
>  drivers/gpu/drm/drm_framebuffer.c   | 10 +++---
>  drivers/gpu/drm/drm_modeset_lock.c  | 10 +++---
>  drivers/gpu/drm/drm_plane.c |  2 +-
>  drivers/gpu/drm/drm_property.c  |  2 +-
>  include/drm/drm_atomic.h|  6 ++--
>  include/drm/drm_color_mgmt.h|  2 +-
>  include/drm/drm_connector.h | 40 ++---
>  include/drm/drm_crtc.h  | 29 +++
>  include/drm/drm_framebuffer.h   | 15 
>  include/drm/drm_mode_config.h   | 12 +++
>  include/drm/drm_mode_object.h   | 13 ---
>  include/drm/drm_modeset_lock.h  |  2 +-
>  include/drm/drm_plane.h | 18 +-
>  include/drm/drm_property.h  |  8 ++---
>  21 files changed, 141 insertions(+), 137 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 723392fc98c8..96c81a61a542 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -195,8 +195,8 @@ EXPORT_SYMBOL(drm_atomic_state_default_clear);
>   * all locks. So someone else could sneak in and change the current modeset
>   * configuration. Which means that all the state assembled in @state is no
>   * longer an atomic update to the current state, but to some arbitrary 
> earlier
> - * state. Which could break assumptions the driver's ->atomic_check likely
> - * relies on.
> + * state. Which could break assumptions the driver's
> + * _mode_config_funcs.atomic_check likely relies on.
>   *
>   * Hence we must clear all cached state and completely start over, using this
>   * function.
> @@ -456,11 +456,10 @@ drm_atomic_replace_property_blob_from_id(struct 
> drm_crtc *crtc,
>   * @property: the property to set
>   * @val: the new property value
>   *
> - * Use this instead of calling crtc->atomic_set_property directly.
> - * This function handles generic/core properties and calls out to
> - * driver's ->atomic_set_property() for driver properties.  To ensure
> - * consistent behavior you must call this function rather than the
> - * driver hook directly.
> + * This function handles generic/core properties and calls out to driver's
> + * _crtc_funcs.atomic_set_property for driver properties. To ensure
> + * consistent behavior you must call this function rather than the driver 
> hook
> + * directly.
>   *
>   * RETURNS:
>   * Zero on success, error code on failure
> @@ -532,10 +531,10 @@ EXPORT_SYMBOL(drm_atomic_crtc_set_property);
>   * @property: the property to set
>   * @val: return location for the property value
>   *
> - * This function handles generic/core properties and calls out to
> - * driver's ->atomic_get_property() for driver properties.  To ensure
> - * consistent behavior you must call this function rather than the
> - * driver hook directly.
> + * This function handles generic/core properties and calls out to driver's
> + * _crtc_funcs.atomic_get_property for driver properties. To ensure
> + * consistent behavior you must call this function rather than the driver 
> hook
> + * directly.
>   *
>   * RETURNS:
>   * Zero on success, error code on failure
> @@ -716,11 +715,10 @@ EXPORT_SYMBOL(drm_atomic_get_plane_state);
>   * @property: the property to set
>   * @val: the new property value
>   *
> - * Use this instead of calling plane->atomic_set_property directly.
> - * This function handles generic/core properties and calls out to
> - * driver's ->atomic_set_property() for driver properties.  To ensure
> - * consistent behavior you must call this function rather than the
> - * driver hook directly.
> + * This function handles generic/core properties and calls out to driver's
> + * _plane_funcs.atomic_set_property for driver properties.  To ensure
> + * consistent behavior you must call this function rather than the driver 
> hook
> + * directly.
>   *
>   * RETURNS:
>   * Zero on success, error code on failure
> @@ -791,10 +789,10 @@ EXPORT_SYMBOL(drm_atomic_plane_set_property);
>   * @property: the property to set
>   * @val: return location for the property value
>   *
> - * This function 

[Intel-gfx] [PATCH i-g-t] lib/drmtest: make DRIVER_ANY match any driver

[Eric Engestrom]

Signed-off-by: Eric Engestrom <eric.engest...@imgtec.com>
---
Not tested or anything, I just happened to notice this code and it
looked wrong, but maybe I misunderstood what it was meant to do.

An alternative would be to just set the bits the the drivers that are
defined already, but that would require an update everytime a new
DRIVER_* is added (error-prone):
#define DRIVER_ANY  ((1 << 4) - 1)
---
 lib/drmtest.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/drmtest.h b/lib/drmtest.h
index 19d4bd19..1d41df93 100644
--- a/lib/drmtest.h
+++ b/lib/drmtest.h
@@ -42,7 +42,7 @@
 #define DRIVER_VC4 (1 << 1)
 #define DRIVER_VGEM(1 << 2)
 #define DRIVER_VIRTIO  (1 << 3)
-#define DRIVER_ANY ~(DRIVER_VGEM)
+#define DRIVER_ANY (~0)
 
 #ifdef ANDROID
 #if (!(defined HAVE_MMAP64)) && (!(defined __x86_64__))
-- 
Cheers,
  Eric

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

Re: [Intel-gfx] [PATCH v4] drm: add fourcc codes for 16bit R and RG

[Eric Engestrom]

On Wednesday, 2017-01-04 14:50:02 +0100, Rainer Hochecker wrote:
> From: Rainer Hochecker 
> 
> Signed-off-by: Rainer Hochecker 
> ---
>  include/uapi/drm/drm_fourcc.h | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index a5890bf..4d65fb6 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -41,10 +41,17 @@ extern "C" {
>  /* 8 bpp Red */
>  #define DRM_FORMAT_R8fourcc_code('R', '8', ' ', ' ') /* 
> [7:0] R */
>  
> +/* 16 bpp Red */
> +#define DRM_FORMAT_R16   fourcc_code('R', '1', '6', ' ') /* 
> [15:0] R */
> +
>  /* 16 bpp RG */
>  #define DRM_FORMAT_RG88  fourcc_code('R', 'G', '8', '8') /* 
> [15:0] R:G 8:8 little endian */
>  #define DRM_FORMAT_GR88  fourcc_code('G', 'R', '8', '8') /* 
> [15:0] G:R 8:8 little endian */
>  
> +/* 32 bpp RG */
> +#define DRM_FORMAT_RG1616fourcc_code('R', 'G', '3', '2') /* [31:0] G:R 
> 16:16 little endian */

Comment above is still wrong: s/G:R/R:G/ :)

> +#define DRM_FORMAT_GR1616fourcc_code('G', 'R', '3', '2') /* [31:0] G:R 
> 16:16 little endian */
> +
>  /* 8 bpp RGB */
>  #define DRM_FORMAT_RGB332fourcc_code('R', 'G', 'B', '8') /* [7:0] R:G:B 
> 3:3:2 */
>  #define DRM_FORMAT_BGR233fourcc_code('B', 'G', 'R', '8') /* [7:0] B:G:R 
> 2:3:3 */
> -- 
> 2.9.3
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] drm: add fourcc codes for 16bit R and GR

[Eric Engestrom]

On Wednesday, 2017-01-04 11:06:09 +0200, Jani Nikula wrote:
> On Wed, 04 Jan 2017, Daniel Vetter  wrote:
> > On Tue, Jan 03, 2017 at 08:02:07PM +0100, Rainer Hochecker wrote:
> >> From: Rainer Hochecker 
> >> 
> >> Now sent with git send-email:
> >> 
> >> Signed-off-by: Rainer Hochecker 
> >> ---
> >>  include/uapi/drm/drm_fourcc.h | 7 +++
> >>  1 file changed, 7 insertions(+)
> >> 
> >> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> >> index a5890bf..f1ef9cb 100644
> >> --- a/include/uapi/drm/drm_fourcc.h
> >> +++ b/include/uapi/drm/drm_fourcc.h
> >> @@ -41,10 +41,17 @@ extern "C" {
> >>  /* 8 bpp Red */
> >>  #define DRM_FORMAT_R8 fourcc_code('R', '8', ' ', ' ') /* 
> >> [7:0] R */
> >>  
> >> +/* 16 bpp Red */
> >> +#define DRM_FORMAT_R16fourcc_code('R', '1', '6', ' ') /* 
> >> [15:0] R */
> >> +
> >>  /* 16 bpp RG */
> >>  #define DRM_FORMAT_RG88   fourcc_code('R', 'G', '8', '8') /* 
> >> [15:0] R:G 8:8 little endian */
> >>  #define DRM_FORMAT_GR88   fourcc_code('G', 'R', '8', '8') /* 
> >> [15:0] G:R 8:8 little endian */
> >>  
> >> +/* 32 bpp GR */
> >> +#define DRM_FORMAT_RG32   fourcc_code('R', 'G', '3', '2') /* 
> >> [31:0] G:R 16:16 little endian */

Also, s/G:R/R:G/ in the comment above :)

> >> +#define DRM_FORMAT_GR32   fourcc_code('G', 'R', '3', '2') /* 
> >> [31:0] G:R 16:16 little endian */
> >
> > Now the define's name is inconsistent, since that would suggest a 5 bpp
> > format with 3 bits for R and 2 bits for G. I think what we want here for
> > consistency is _RG16_16 and _GR16_16, along the lines of what Ville
> > suggested.
> >
> > Sorry that this is such a bikeshed heaven ;-)
> 
> If there's going to be another version, please fix the commit message
> while at it. "Now sent with git send-email" is useless for posterity.
> 
> BR,
> Jani.
> 
> 
> > -Daniel
> >
> >> +
> >>  /* 8 bpp RGB */
> >>  #define DRM_FORMAT_RGB332 fourcc_code('R', 'G', 'B', '8') /* [7:0] R:G:B 
> >> 3:3:2 */
> >>  #define DRM_FORMAT_BGR233 fourcc_code('B', 'G', 'R', '8') /* [7:0] B:G:R 
> >> 2:3:3 */
> >> -- 
> >> 2.9.3
> >> 
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm: add fourcc codes for 16bit R and GR

[Eric Engestrom]

On Tuesday, 2017-01-03 17:56:10 +0100, Rainer Hochecker wrote:
> On Mon, Jan 2, 2017 at 3:31 PM, Rainer Hochecker  wrote:
> >
> > I chose GR16 because that matches with Mesa texture formats. Unfortunately
> > RG16 is already taken by DRM_FORMAT_RGB565
> > So GR32 / RG32 might be better. All other codes in fourcc.h seem to sum up
> > all planes.
> >
> > (sorry, gmail included some html links on last attempt)
> >
> > On Mon, Jan 2, 2017 at 3:05 PM, Ville Syrjälä 
> >  wrote:
> >>
> >> On Mon, Jan 02, 2017 at 01:23:23PM +0100, David Herrmann wrote:
> >> > Hi
> >> >
> >> > On Mon, Jan 2, 2017 at 11:41 AM, Rainer Hochecker  
> >> > wrote:
> >> > > From: Rainer Hochecker 
> >> > >
> >> > > Add fourcc codes for 16bit planes. Required by mesa for
> >> > > eglCreateImageKHR to access P010 surfaces created by vaapi.
> >> > >
> >> > > Signed-off-by: Rainer Hochecker 
> >> > > ---
> >> > >  include/uapi/drm/drm_fourcc.h | 6 ++
> >> > >  1 file changed, 6 insertions(+)
> >> > >
> >> > > diff --git a/include/uapi/drm/drm_fourcc.h 
> >> > > b/include/uapi/drm/drm_fourcc.h
> >> > > index a5890bf..e6ab638 100644
> >> > > --- a/include/uapi/drm/drm_fourcc.h
> >> > > +++ b/include/uapi/drm/drm_fourcc.h
> >> > > @@ -41,10 +41,16 @@ extern "C" {
> >> > >  /* 8 bpp Red */
> >> > >  #define DRM_FORMAT_R8  fourcc_code('R', '8', ' ', ' ') /* 
> >> > > [7:0] R */
> >> > >
> >> > > +/* 16 bpp Red */
> >> > > +#define DRM_FORMAT_R16 fourcc_code('R', '1', '6', ' ') /* 
> >> > > [15:0] R */
> >> > > +
> >> > >  /* 16 bpp RG */
> >> > >  #define DRM_FORMAT_RG88fourcc_code('R', 'G', '8', 
> >> > > '8') /* [15:0] R:G 8:8 little endian */
> >> > >  #define DRM_FORMAT_GR88fourcc_code('G', 'R', '8', 
> >> > > '8') /* [15:0] G:R 8:8 little endian */
> >> > >
> >> > > +/* 32 bpp GR */
> >> > > +#define DRM_FORMAT_GR16fourcc_code('G', 'R', '1', 
> >> > > '6') /* [31:0] G:R 16:16 little endian */
> >> > > +
> >> >
> >> > Shouldn't it be 'G', 'R', '3', '2'?
> >>
> >> The name should be _GR1616. Using GR16 for the fourcc seems OK to me
> >> since we can't fit in the full GR1616 in there. Althogh GR32 could work
> >> too I suppose.
> >>
> >> And what about RG16?
> >>
> >> >
> >> > Also, please put dri-devel on CC.
> >> >
> >> > Thanks
> >> > David
> >> >
> >> > >  /* 8 bpp RGB */
> >> > >  #define DRM_FORMAT_RGB332  fourcc_code('R', 'G', 'B', '8') /* 
> >> > > [7:0] R:G:B 3:3:2 */
> >> > >  #define DRM_FORMAT_BGR233  fourcc_code('B', 'G', 'R', '8') /* 
> >> > > [7:0] B:G:R 2:3:3 */
> >> > > --
> >> > > 2.9.3
> >> > >
> >> > ___
> >> > dri-devel mailing list
> >> > dri-de...@lists.freedesktop.org
> >> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >>
> >> --
> >> Ville Syrjälä
> >> Intel OTC
> >
> >
> 
> Updated patch as suggested by Ville Syrjälä
> 

You shouldn't send patches using Gmail's web interface: it completely
disregards formatting, breaking any machine-readable text. This patch is
unusable :(

It is usually recommended to send patches using `git send-email`, eg.:
  git send-email \
--in-reply-to 
CAH0Sn6HhaJmFBz5nsfUD7t0xca8=42+5+ia+qg6oqzevx_n...@mail.gmail.com \
0001-drm-add-fourcc-codes-for-16bit-R-and-GR.patch

`--in-reply-to` keeps the threads together. You can find the ID of the
message you want to reply to in the "Message-ID:" header

You might also want to use `-v2` when formatting the patches
(`git format-patch -vX`); this lets reviewers follow your revisions by
adjusting the subject of the mail :)

Cheers,
  Eric


> 
> From 29e74ff96e0b7c7a11d1b4131891b83adde621c1 Mon Sep 17 00:00:00 2001
> 
> From: Rainer Hochecker 
> 
> Date: Mon, 2 Jan 2017 11:25:18 +0100
> 
> Subject: [PATCH] drm: add fourcc codes for 16bit R and GR
> 
> 
> Signed-off-by: Rainer Hochecker 
> 
> ---
> 
>  include/uapi/drm/drm_fourcc.h | 7 +++
> 
>  1 file changed, 7 insertions(+)
> 
> 
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> 
> index a5890bf..f1ef9cb 100644
> 
> --- a/include/uapi/drm/drm_fourcc.h
> 
> +++ b/include/uapi/drm/drm_fourcc.h
> 
> @@ -41,10 +41,17 @@ extern "C" {
> 
>  /* 8 bpp Red */
> 
>  #define DRM_FORMAT_R8 fourcc_code('R', '8', ' ', ' ') /* [7:0] R */
> 
> 
> 
> +/* 16 bpp Red */
> 
> +#define DRM_FORMAT_R16 fourcc_code('R', '1', '6', ' ') /* [15:0] R */
> 
> +
> 
>  /* 16 bpp RG */
> 
>  #define DRM_FORMAT_RG88 fourcc_code('R', 'G', '8', '8') /* [15:0] R:G
> 8:8 little endian */
> 
>  #define DRM_FORMAT_GR88 fourcc_code('G', 'R', '8', '8') /* [15:0] G:R
> 8:8 little endian */
> 
> 
> 
> +/* 32 bpp GR */
> 
> +#define DRM_FORMAT_RG32 fourcc_code('R', 'G', '3', '2') /* [31:0] G:R
> 16:16 little endian */
> 
> +#define DRM_FORMAT_GR32 fourcc_code('G', 'R', '3', '2') /* [31:0] G:R
> 16:16 little 

Re: [Intel-gfx] [PATCH] drm: Don't block the kworker waiting for mode_config.lock in output_poll()

[Eric Engestrom]

On Tuesday, 2016-12-06 11:37:15 +, Chris Wilson wrote:
> If we cannot acquire the mode_config.lock immediately, just back off and

s/mode_config.lock/mode_config.mutex lock/ ?

> queue a new attempt after the poll interval. This is mostly to stop the
> hung task spam when the system is deadlocked, but it will also lessen
> the load (in such extreme cases).
> 
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>

Reviewed-by: Eric Engestrom <eric.engest...@imgtec.com>

> ---
>  drivers/gpu/drm/drm_probe_helper.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_probe_helper.c 
> b/drivers/gpu/drm/drm_probe_helper.c
> index ff9ba6f35248..0bf629354297 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -394,7 +394,11 @@ static void output_poll_execute(struct work_struct *work)
>   if (!drm_kms_helper_poll)
>   goto out;
>  
> - mutex_lock(>mode_config.mutex);
> + if (!mutex_trylock(>mode_config.mutex)) {
> + repoll = true;
> + goto out;
> + }
> +
>   drm_for_each_connector(connector, dev) {
>  
>   /* Ignore forced connectors. */
> -- 
> 2.11.0
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915/gvt: fix deadlock in dispatch_workload()'s error path

[Eric Engestrom]

90d27a1 moved the lock before this error path but forgot to add an
unlock here.

Fixes: 90d27a1b180e51ef0713 ("drm/i915/gvt: fix deadlock in workload_thread")
Cc: Pei Zhang <pei.zh...@intel.com>
Cc: Zhenyu Wang <zhen...@linux.intel.com>
Signed-off-by: Eric Engestrom <e...@engestrom.ch>
---
 drivers/gpu/drm/i915/gvt/scheduler.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c 
b/drivers/gpu/drm/i915/gvt/scheduler.c
index f898df3..cd13c4b 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -177,6 +177,7 @@ static int dispatch_workload(struct intel_vgpu_workload 
*workload)
rq = i915_gem_request_alloc(dev_priv->engine[ring_id], shadow_ctx);
if (IS_ERR(rq)) {
gvt_err("fail to allocate gem request\n");
+   mutex_unlock(_priv->drm.struct_mutex);
workload->status = PTR_ERR(rq);
return workload->status;
}
-- 
Cheers,
  Eric

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

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

[Eric Engestrom]

The function's behaviour was changed in 90844f00049e, without changing
its signature, causing people to keep using it the old way without
realising they were now leaking memory.
Rob Clark also noticed it was also allocating GFP_KERNEL memory in
atomic contexts, breaking them.

Instead of having to allocate GFP_ATOMIC memory and fixing the callers
to make them cleanup the memory afterwards, let's change the function's
signature by having the caller take care of the memory and passing it to
the function.
The new parameter is a single-field struct in order to enforce the size
of its buffer and help callers to correctly manage their memory.

Fixes: 90844f00049e ("drm: make drm_get_format_name thread-safe")
Cc: Rob Clark <robdcl...@gmail.com>
Cc: Christian König <christian.koe...@amd.com>
Acked-by: Christian König <christian.koe...@amd.com>
Acked-by: Rob Clark <robdcl...@gmail.com>
Acked-by: Sinclair Yeh <s...@vmware.com> (vmwgfx)
Reviewed-by: Jani Nikula <jani.nik...@intel.com>
Suggested-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
Signed-off-by: Eric Engestrom <e...@engestrom.ch>
---
v4 - rebase on drm-next (d8c1abd968f1c880ad8c)
   - add missed `const` to the return value

v3 - fix "Fixes" tag, replace it with an actual commit message
   - collect ack & r-b

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| 10 +++---
 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, 82 insertions(+), 87 deletions(-)

diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
index dc0aafa..fcc08da 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;
+const 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..90d2cc8 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)
+const 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/dr

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

[Eric Engestrom]

On Wednesday, 2016-11-09 14:13:40 +0100, Daniel Vetter wrote:
> On Wed, Nov 9, 2016 at 12:42 PM, Eric Engestrom
> <eric.engest...@imgtec.com> wrote:
> >> Well, had to drop it again since it didn't compile:
> >>
> >>
> >>   CC [M]  drivers/gpu/drm/drm_blend.o
> >> drivers/gpu/drm/drm_atomic.c: In function ‘drm_atomic_plane_print_state’:
> >> drivers/gpu/drm/drm_atomic.c:920:5: error: too few arguments to function 
> >> ‘drm_get_format_name’
> >>  drm_get_format_name(fb->pixel_format));
> >>  ^~~
> >> In file included from ./include/drm/drmP.h:71:0,
> >>  from drivers/gpu/drm/drm_atomic.c:29:
> >> ./include/drm/drm_fourcc.h:65:7: note: declared here
> >>  char *drm_get_format_name(uint32_t format, struct drm_format_name_buf 
> >> *buf);
> >>^~~
> >>
> >> Can you pls rebase onto drm-misc or linux-next or something?
> >
> > That was based on airlied/drm-next (last fetched on Sunday I think),
> > I can rebase it on drm-misc if it helps, but it seems older than
> > drm-next.
> > Should I just rebase on top of current head of drm-next?
> 
> It needs to be drm-misc (linux-next doesn't have it yet) due to the
> new atomic debug work that we just landed. I'm working on drm-tip as a
> drm local integration tree to ease pains like these a bit, but that
> doesn't really exist yet.

I'm confused as to how the different trees and branches merge back to
Torvalds' tree (I'm interested in particular in drm), and I'm not sure
which branch you want me to rebase on in the drm-misc tree [1],
especially since all of them are older than drm-next [2].

I'll try to rebase on drm-misc-fixes (currently at 4da5caa6a6f82cda3193)
as it sounds about right, but it doesn't apply at all, so it'll take
a little while.

Could you give me a quick explanation or point me to a doc/page that
explains how the various trees and branches get merged?
I googled a bit and found this doc [4] by Jani, but it doesn't mention
drm-misc for instance, so I'm not sure how up-to-date and
non-intel-specific it is.

Looking at this page, something just occurred to me: did you mean
drm-fixes [3], instead of one of the branches on drm-misc?

Cheers,
  Eric

[1] git://anongit.freedesktop.org/drm/drm-misc
[2] git://people.freedesktop.org/~airlied/linux drm-next
[2] git://people.freedesktop.org/~airlied/linux drm-fixes
[3] https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-intel.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Eric Engestrom]

On Wednesday, 2016-11-09 02:13:25 +0100, Daniel Vetter wrote:
> On Wed, Nov 09, 2016 at 02:09:16AM +0100, Daniel Vetter wrote:
> > On Wed, Nov 09, 2016 at 12:17:52AM +0000, Eric Engestrom wrote:
> > > The function's behaviour was changed in 90844f00049e, without changing
> > > its signature, causing people to keep using it the old way without
> > > realising they were now leaking memory.
> > > Rob Clark also noticed it was also allocating GFP_KERNEL memory in
> > > atomic contexts, breaking them.
> > > 
> > > Instead of having to allocate GFP_ATOMIC memory and fixing the callers
> > > to make them cleanup the memory afterwards, let's change the function's
> > > signature by having the caller take care of the memory and passing it to
> > > the function.
> > > The new parameter is a single-field struct in order to enforce the size
> > > of its buffer and help callers to correctly manage their memory.
> > > 
> > > Fixes: 90844f00049e ("drm: make drm_get_format_name thread-safe")
> > > Cc: Rob Clark <robdcl...@gmail.com>
> > > Cc: Christian König <christian.koe...@amd.com>
> > > Acked-by: Christian König <christian.koe...@amd.com>
> > > Acked-by: Rob Clark <robdcl...@gmail.com>
> > > Acked-by: Sinclair Yeh <s...@vmware.com> (vmwgfx)
> > > Reviewed-by: Jani Nikula <jani.nik...@intel.com>
> > > Suggested-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > > Signed-off-by: Eric Engestrom <e...@engestrom.ch>
> > > ---
> > > v3 - fix "Fixes" tag, replace it with an actual commit message
> > >- collect ack & r-b
> > > 
> > > v2 - use single-field struct instead of typedef to let the compiler
> > >  enforce the type (Christian König)
> > 
> > Applied to drm-misc, thanks.
> 
> Well, had to drop it again since it didn't compile:
> 
> 
>   CC [M]  drivers/gpu/drm/drm_blend.o
> drivers/gpu/drm/drm_atomic.c: In function ‘drm_atomic_plane_print_state’:
> drivers/gpu/drm/drm_atomic.c:920:5: error: too few arguments to function 
> ‘drm_get_format_name’
>  drm_get_format_name(fb->pixel_format));
>  ^~~
> In file included from ./include/drm/drmP.h:71:0,
>  from drivers/gpu/drm/drm_atomic.c:29:
> ./include/drm/drm_fourcc.h:65:7: note: declared here
>  char *drm_get_format_name(uint32_t format, struct drm_format_name_buf *buf);
>^~~
> 
> Can you pls rebase onto drm-misc or linux-next or something?

That was based on airlied/drm-next (last fetched on Sunday I think),
I can rebase it on drm-misc if it helps, but it seems older than
drm-next.
Should I just rebase on top of current head of drm-next?
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Eric Engestrom]

The function's behaviour was changed in 90844f00049e, without changing
its signature, causing people to keep using it the old way without
realising they were now leaking memory.
Rob Clark also noticed it was also allocating GFP_KERNEL memory in
atomic contexts, breaking them.

Instead of having to allocate GFP_ATOMIC memory and fixing the callers
to make them cleanup the memory afterwards, let's change the function's
signature by having the caller take care of the memory and passing it to
the function.
The new parameter is a single-field struct in order to enforce the size
of its buffer and help callers to correctly manage their memory.

Fixes: 90844f00049e ("drm: make drm_get_format_name thread-safe")
Cc: Rob Clark <robdcl...@gmail.com>
Cc: Christian König <christian.koe...@amd.com>
Acked-by: Christian König <christian.koe...@amd.com>
Acked-by: Rob Clark <robdcl...@gmail.com>
Acked-by: Sinclair Yeh <s...@vmware.com> (vmwgfx)
Reviewed-by: Jani Nikula <jani.nik...@intel.com>
Suggested-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
Signed-off-by: Eric Engestrom <e...@engestrom.ch>
---
v3 - fix "Fixes" tag, replace it with an actual commit message
   - collect ack & r-b

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 v2] drm: move allocation out of drm_get_format_name()

[Eric Engestrom]

On Monday, 2016-11-07 10:10:13 +0200, Jani Nikula wrote:
> On Mon, 07 Nov 2016, Eric Engestrom <e...@engestrom.ch> 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>
> 
> The Fixes: format is:
> 
> Fixes: 90844f00049e ("drm: make drm_get_format_name thread-safe")
> 
> But is this a fix, really, or just an improvement? What exactly is the
> bug being fixed? The commit message is not sufficient.

"The function's behaviour was changed in 90844f00049e, without changing
its signature, causing people to keep using it the old way without
realising they were now leaking memory.
Rob Clark also noticed it was also allocating GFP_KERNEL memory in
atomic contexts, breaking them.

Instead of having to allocate GFP_ATOMIC memory and fixing the callers
to make them cleanup the memory afterwards, let's change the function's
signature by having the caller take care of the memory and passing it to
the function.
The new parameter is a single-field struct in order to enforce the size
of its buffer and help callers to correctly manage their memory."

Does this sound good?

> > @@ -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);
> 
> I wonder if it would be better to make that return "const char *". If
> the user really wants to look under the hood, there's buf->str. *shrug*

Good idea, I'll do that in v3 with the proper commit msg and tags. It'll
have to wait another day though, -ENOTIME and all.

> 
> With the commit message improved,
> 
> Reviewed-by: Jani Nikula <jani.nik...@intel.com>

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

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

[Eric Engestrom]

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_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
@@ -2032,7 +2032,7 @@ static int dce_v10_0_crtc_do_set_base(struct drm_crtc 
*crtc,
u32 tmp, viewport_w, viewport_h;
int r;
bool bypass_lut = false;
-   char *format_name;
+   struct drm_format_name_buf format_name;
 
/* no fb bound */
if (!atomic && !crtc->primary->fb) {
@@ -2144,9 +2144,8 @@ static int dce_v10_0_crtc_do_set_base(struct drm_crtc 
*crtc,
bypass_lut = true;
break;
default:
-   format_name = drm_get_format_name(target_fb->pixel_format);
-   DRM_ERROR("Unsupported screen format %s\n", format_name);
+   D

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

[Eric Engestrom]

On Sunday, 2016-11-06 08:03:47 -0500, Rob Clark wrote:
> On Sun, Nov 6, 2016 at 4:47 AM, Christian König
> <christian.koe...@amd.com> wrote:
> > Am 05.11.2016 um 17:49 schrieb Rob Clark:
> >>
> >> On Sat, Nov 5, 2016 at 12:38 PM, Eric Engestrom <e...@engestrom.ch> wrote:
> >>>
> >>> On Saturday, 2016-11-05 13:11:36 +0100, Christian König wrote:
> >>>>
> >>>> Am 05.11.2016 um 02:33 schrieb Eric Engestrom:
> >>>>>
> >>>>> +typedef char drm_format_name_buf[32];
> >>>>
> >>>> Please don't use a typedef for this, just define the maximum size of
> >>>> characters the function might write somewhere.
> >>>>
> >>>> See the kernel coding style as well:
> >>>>>
> >>>>> In general, a pointer, or a struct that has elements that can
> >>>>> reasonably
> >>>>> be directly accessed should **never** be a typedef.
> >>>
> >>> I would normally agree as I tend to hate typedefs ($DAYJOB {ab,mis}uses
> >>> them way too much), and your way was what I wrote at first, but Rob
> >>> Clark's
> >>> typedef idea makes it much harder for someone to allocate a buffer of
> >>> the wrong size, which IMO is good thing here.
> >>
> >> IMHO I would make a small test program to verify this actually helps
> >> the compiler catch problems.  And if it does, I would stick with it.
> >> The coding-style should be guidelines, not something that supersedes
> >> common sense / practicality.
> >
> >
> > Well completely agree that we should be able to question the coding style
> > rules, but when we do it we discuss this on a the mailing list first and
> > then start to use it in code. Not the other way around.
> 
> if I'm not mistaken, that is what we are doing ;-)
> 
> >>
> >> That is my $0.02 anyways.. if others vehemently disagree and want to
> >> dogmatically stick to the coding-style guidelines, ok then.  OTOH, if
> >> this approach doesn't help the compiler catch issues, then it isn't
> >> worth it.
> >
> >
> > Yeah, exactly that's the point. If I'm not completely mistaken the compiler
> > won't issue a warning here if you pass an array with the wrong size.
> >
> > I think you need something like "struct drm_format_name_buf { char str[32];
> > };" to trigger this.
> 
> hmm, actually the struct is a nice idea then if the compiler wouldn't
> catch the wrong-size-array

Sending the patch in a minute.

> 
> > apart from that is this function really called so often that using
> > kasprintf() is a problem here? or is there another motivation behind the
> > change?
> 
> two things trouble me about the kasprintf approach.. (ignoring the
> fact that atm it is not gfp_atomic)
> 1) you can't do drm_debug("format: %s\n", drm_get_format_name(..)) so
> it pulls the memory allocation and sprintf outside of the drm_debug
> check
> 2) seems awfully easy to forget the kfree...

I actually found a couple of these memory leaks while doing this patch,
look for files where i don't remove kfree :)
(eg. vmwgfx at the end of the patch)

> i wouldn't have even
> known that now you need to free the result (with some patches i'm
> working on) if it weren't for the fact that lockdep alerted me to the
> gfp_kernel allocation in atomic ctx ;-)
> 
> br,
> -r
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Eric Engestrom]

On Saturday, 2016-11-05 13:11:36 +0100, Christian König wrote:
> Am 05.11.2016 um 02:33 schrieb Eric Engestrom:
> > +typedef char drm_format_name_buf[32];
> 
> Please don't use a typedef for this, just define the maximum size of
> characters the function might write somewhere.
> 
> See the kernel coding style as well:
> > In general, a pointer, or a struct that has elements that can reasonably
> > be directly accessed should **never** be a typedef.
> 

I would normally agree as I tend to hate typedefs ($DAYJOB {ab,mis}uses
them way too much), and your way was what I wrote at first, but Rob Clark's
typedef idea makes it much harder for someone to allocate a buffer of
the wrong size, which IMO is good thing here.

I can rewrite the typedef out if you think it's better.

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

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

[Eric Engestrom]

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>

Suggested-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
Signed-off-by: Eric Engestrom <e...@engestrom.ch>
---
 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_fourcc.c| 12 +++-
 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 |  8 ++---
 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 +-
 include/drm/drm_fourcc.h|  3 +-
 17 files changed, 71 insertions(+), 84 deletions(-)

diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
index dc0aafa..5a8cb4b 100644
--- a/include/drm/drm_fourcc.h
+++ b/include/drm/drm_fourcc.h
@@ -54,6 +54,7 @@ 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;
+typedef char drm_format_name_buf[32];
+char *drm_get_format_name(uint32_t format, 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..34ed520 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, drm_format_name_buf buf)
 {
-   char *buf = kmalloc(32, GFP_KERNEL);
-
-   snprintf(buf, 32,
+   snprintf(buf, sizeof(drm_format_name_buf),
 "%c%c%c%c %s-endian (0x%08x)",
 printable_char(format & 0xff),
 printable_char((format >> 8) & 0xff),
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
index 199d3f7..cefa3d8 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
@@ -2032,7 +2032,7 @@ static int dce_v10_0_crtc_do_set_base(struct drm_crtc 
*crtc,
u32 tmp, viewport_w, viewport_h;
int r;
bool bypass_lut = false;
-   char *format_name;
+   drm_format_name_buf format_name;
 
/* no fb bound */
if (!atomic && !crtc->primary->fb) {
@@ -2144,9 +2144,8 @@ static int dce_v10_0_crtc_do_set_base(struct drm_crtc 
*crtc,
bypass_lut = true;
break;
default:
-   format_name = drm_get_format_name(target_fb->pixel_format);
-   DRM_ERROR("Unsupported screen format %s\n", format_name);
-   kfree(format_name);
+   DRM_ERROR("Unsupported screen format %s\n",
+ drm_get_format_name(target_fb->pixel_format, 
format_name));
return -EINVAL;
}
 
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c 
b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
index ecd000e..462abb8 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
@@ -2013,7 +2013,7 @@ static int dce_v11_0_crtc_do_set_base(struct drm_crtc 
*crtc,
u32 tmp, viewport_w, viewport_h;
int r;
bool bypass_lut = false;
-   char *format_name;
+   drm_format_name_buf format_name;
 
/* no fb bound */
if (!atomic && !crtc->primary->fb) {
@@ -2125,9 +2125,8 @@ static int dce_v11_0_crtc_do_set_base(struct drm_crtc 
*crtc,
bypass_lut = true;
   

Re: [Intel-gfx] [PATCH] i915: don't call drm_atomic_state_put on invalid pointer

[Eric Engestrom]

On Tuesday, 2016-10-18 17:16:23 +0200, Arnd Bergmann wrote:
> The introduction of reference counting on the state structures caused
> sanitize_watermarks() in i915 to break in the error handling case,
> as pointed out by gcc -Wmaybe-uninitialized
> 
> drivers/gpu/drm/i915/intel_display.c: In function ‘intel_modeset_init’:
> include/drm/drm_atomic.h:224:2: error: ‘state’ may be used uninitialized in 
> this function [-Werror=maybe-uninitialized]
> 
> This changes the function back to only drop the reference count
> when it was successfully allocated first.
> 
> Fixes: 0853695c3ba4 ("drm: Add reference counting to drm_atomic_state")
> Cc: Chris Wilson <ch...@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vet...@ffwll.ch>
> Signed-off-by: Arnd Bergmann <a...@arndb.de>

Reviewed-by: Eric Engestrom <eric.engest...@imgtec.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 6d168685bbda..6a26da143aa6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -16314,7 +16314,7 @@ static void sanitize_watermarks(struct drm_device 
> *dev)
>* BIOS-programmed watermarks untouched and hope for the best.
>*/
>   WARN(true, "Could not determine valid watermarks for inherited 
> state\n");
> - goto fail;
> + goto put_state;
>   }
>  
>   /* Write calculated watermark values back */
> @@ -16325,8 +16325,9 @@ static void sanitize_watermarks(struct drm_device 
> *dev)
>   dev_priv->display.optimize_watermarks(cs);
>   }
>  
> -fail:
> +put_state:
>   drm_atomic_state_put(state);
> +fail:
>   drm_modeset_drop_locks();
>   drm_modeset_acquire_fini();
>  }
> -- 
> 2.9.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [patch] drm/i915: fix a read size argument

[Eric Engestrom]

On Thu, Oct 13, 2016 at 11:55:08AM +0300, Dan Carpenter wrote:
> We want to read 3 bytes here, but because the parenthesis are in the
> wrong place we instead read:
> 
>   sizeof(intel_dp->edp_dpcd) == sizeof(intel_dp->edp_dpcd)
> 
> which is one byte.
> 
> Fixes: fe5a66f91c88 ("drm/i915: Read PSR caps/intermediate freqs/etc. only 
> once on eDP")
> Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>

Good catch!  What tool did you use to find it, or did you find it by
inspection?
Reviewed-by: Eric Engestrom <eric.engest...@imgtec.com>

(btw, there's a missing `---` here, between the commit msg and the diff)

> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 14a3cf0..ee8aa95 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3551,8 +3551,8 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp)
>   /* Read the eDP Display control capabilities registers */
>   if ((intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP] & 
> DP_DPCD_DISPLAY_CONTROL_CAPABLE) &&
>   drm_dp_dpcd_read(_dp->aux, DP_EDP_DPCD_REV,
> -  intel_dp->edp_dpcd, sizeof(intel_dp->edp_dpcd) ==
> -  sizeof(intel_dp->edp_dpcd)))
> +  intel_dp->edp_dpcd, sizeof(intel_dp->edp_dpcd)) ==
> +  sizeof(intel_dp->edp_dpcd))
>   DRM_DEBUG_KMS("EDP DPCD : %*ph\n", (int) 
> sizeof(intel_dp->edp_dpcd),
> intel_dp->edp_dpcd);
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v5 1/4] drm: two more (drm_)printk() wrapper macros

[Eric Engestrom]

On Fri, Aug 26, 2016 at 06:50:56PM +0100, Dave Gordon wrote:
> We had only DRM_INFO() and DRM_ERROR(), whereas the underlying printk()
> provides several other useful intermediate levels such as NOTICE and
> WARNING. So this patch fills out the set by providing simple macros for
> the additional levels. We don't provide _DEV_ or _ONCE or RATELIMITED
> versions yet as it seems unlikely that they'll be as useful.
> 
> v2:
> Fix whitespace, missing ## (Eric Engestrom)
> v5:
> Much simplified after underlying functions were reworked.
> 
> Signed-off-by: Dave Gordon <david.s.gor...@intel.com>
> Previously-Reviewed-by: Eric Engestrom <eric.engest...@imgtec.com> (v2)

My r-b still stands on the v5 (and I really like the simplification!)
Thanks for CC'ing me on the updates :)

Cheers,
  Eric

> Cc: Eric Engestrom <eric.engest...@imgtec.com>
> Cc: dri-de...@lists.freedesktop.org
> ---
>  include/drm/drmP.h | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 94eb138..cd52624 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -168,6 +168,13 @@ void drm_printk(const char *level, unsigned int category,
>  /** \name Macros to make printk easier */
>  /*@{*/
>  
> +#define DRM_INFO(fmt, ...)   \
> + drm_printk(KERN_INFO, DRM_UT_NONE, __func__, "", fmt, ##__VA_ARGS__)
> +#define DRM_NOTE(fmt, ...)   \
> + drm_printk(KERN_NOTICE, DRM_UT_NONE, __func__, "", fmt, ##__VA_ARGS__)
> +#define DRM_WARN(fmt, ...)   \
> + drm_printk(KERN_WARNING, DRM_UT_NONE, __func__, "", fmt, ##__VA_ARGS__)
> +
>  /**
>   * Error output.
>   *
> @@ -202,8 +209,6 @@ void drm_printk(const char *level, unsigned int category,
>  #define DRM_DEV_INFO(dev, fmt, ...)  \
>   drm_dev_printk(dev, KERN_INFO, DRM_UT_NONE, __func__, "", fmt,  \
>  ##__VA_ARGS__)
> -#define DRM_INFO(fmt, ...)   \
> - drm_printk(KERN_INFO, DRM_UT_NONE, __func__, "", fmt, ##__VA_ARGS__)
>  
>  #define DRM_DEV_INFO_ONCE(dev, fmt, ...) \
>  ({   \
> -- 
> 1.9.1
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] drm: Add reference counting to drm_atomic_state

[Eric Engestrom]

On Wed, Aug 24, 2016 at 07:10:15AM +0100, Chris Wilson wrote:
> drm_atomic_state has a complicated single owner model that tracks the
> single reference from allocation through to destruction on another
> thread - or perhaps on a local error path. We can simplify this tracking
> by using reference counting (at a cost of a few more atomics). This is
> even more beneficial when the lifetime of the state becomes more
> convoluted than being passed to a single worker thread for the commit.
> 
> v2: Double check !intel atomic_commit functions for missing gets
> 
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vet...@ffwll.ch>
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c |  3 +-
>  drivers/gpu/drm/drm_atomic.c | 23 +++
>  drivers/gpu/drm/drm_atomic_helper.c  | 98 
> +++-
>  drivers/gpu/drm/drm_fb_helper.c  |  9 +--
>  drivers/gpu/drm/exynos/exynos_drm_drv.c  |  3 +-
>  drivers/gpu/drm/i915/i915_debugfs.c  |  3 +-
>  drivers/gpu/drm/i915/intel_display.c | 32 -
>  drivers/gpu/drm/i915/intel_sprite.c  |  4 +-
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c   |  3 +-
>  drivers/gpu/drm/msm/msm_atomic.c |  3 +-
>  drivers/gpu/drm/omapdrm/omap_drv.c   |  3 +-
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c|  3 +-
>  drivers/gpu/drm/sti/sti_drv.c|  3 +-
>  drivers/gpu/drm/tegra/drm.c  |  3 +-
>  drivers/gpu/drm/tilcdc/tilcdc_drv.c  |  2 -
>  drivers/gpu/drm/vc4/vc4_kms.c|  3 +-
>  include/drm/drm_atomic.h | 12 +++-
>  include/drm/drm_crtc.h   |  3 +
>  18 files changed, 85 insertions(+), 128 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> index d4a3d61b7b06..15a9f9d3ef9a 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> @@ -464,7 +464,7 @@ atmel_hlcdc_dc_atomic_complete(struct 
> atmel_hlcdc_dc_commit *commit)
>  
>   drm_atomic_helper_cleanup_planes(dev, old_state);
>  
> - drm_atomic_state_free(old_state);
> + drm_atomic_state_put(old_state);
>  
>   /* Complete the commit, wake up any waiter. */
>   spin_lock(>commit.wait.lock);
> @@ -521,6 +521,7 @@ static int atmel_hlcdc_dc_atomic_commit(struct drm_device 
> *dev,
>   /* Swap the state, this is the point of no return. */
>   drm_atomic_helper_swap_state(state, true);
>  
> + drm_atomic_state_get(state);
>   if (async)
>   queue_work(dc->wq, >work);
>   else
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 5cb2e22d5d55..349c2f0de900 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -74,6 +74,8 @@ EXPORT_SYMBOL(drm_atomic_state_default_release);
>  int
>  drm_atomic_state_init(struct drm_device *dev, struct drm_atomic_state *state)
>  {
> + kref_init(>ref);
> +
>   /* TODO legacy paths should maybe do a better job about
>* setting this appropriately?
>*/
> @@ -215,22 +217,16 @@ void drm_atomic_state_clear(struct drm_atomic_state 
> *state)
>  EXPORT_SYMBOL(drm_atomic_state_clear);
>  
>  /**
> - * drm_atomic_state_free - free all memory for an atomic state
> + * __drm_atomic_state_free - free all memory for an atomic state
>   * @state: atomic state to deallocate

Doc line needs updating as well.
Other than that, this looks good to me:
Reviewed-by: Eric Engestrom <eric.engest...@imgtec.com>

>   *
>   * This frees all memory associated with an atomic state, including all the
>   * per-object state for planes, crtcs and connectors.
>   */
> -void drm_atomic_state_free(struct drm_atomic_state *state)
> +void __drm_atomic_state_free(struct kref *ref)
>  {

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

Re: [Intel-gfx] [Mesa-dev] [PATCH v1 00/13] Implement sw_sync test

[Eric Engestrom]

On Tue, Aug 23, 2016 at 01:56:02PM -0400, robert.f...@collabora.com wrote:
> From: Robert Foss <robert.f...@collabora.com>
> 
> This series implements the sw_sync test and the lib/sw_sync helper functions
> for said test.
> 
> Gustavo Padovans sw_sync series was just de-staged in
> gregkh-staging/staging-next [1], and this test is targeted at verifying the
> functionality implemented in that series.
> 
> The sw_sync subtests range from very basic tests of the sw_sync functionality,
> to stress testing and randomized tests.
> 
> [1] http://git.kernel.org/cgit/linux/kernel/git/gregkh/staging.git/
> 
> Robert Foss (13):
>   lib/sw_sync: Add helper functions for managing synchronization
> primitives
>   tests/sw_sync: Add sw_sync test
>   tests/sw_sync: Add subtest test_alloc_fence
>   tests/sw_sync: Add subtest test_alloc_fence_invalid_timeline
>   tests/sw_sync: Add subtest test_alloc_merge_fence
>   tests/sw_sync: Add subtest test_sync_wait
>   tests/sw_sync: Add subtest test_sync_merge
>   tests/sw_sync: Add subtest test_sync_merge_same
>   tests/sw_sync: Add subtest test_sync_multi_consumer
>   tests/sw_sync: Add subtest test_sync_multi_consumer_producer
>   tests/sw_sync: Add subtest test_sync_random_merge
>   tests/sw_sync: Add subtest test_sync_multi_timeline_wait
>   tests/sw_sync: Add subtest test_sync_multi_producer_single_consumer
> 
>  lib/Makefile.sources   |   2 +
>  lib/sw_sync.c  | 237 +
>  lib/sw_sync.h  |  49 
>  tests/Makefile.sources |   1 +
>  tests/sw_sync.c| 693 
> +
>  5 files changed, 982 insertions(+)
>  create mode 100644 lib/sw_sync.c
>  create mode 100644 lib/sw_sync.h
>  create mode 100644 tests/sw_sync.c
> 
> -- 
> 2.7.4

Thanks for your work!

I sent some specific comments directly to a few patches, but everything
else looks good to me.

With the issues raised in patches 1 & 2 fixed (and with or without my
suggestions), the whole series is:
Reviewed-by: Eric Engestrom <e...@engestrom.ch>

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

Re: [Intel-gfx] [Mesa-dev] [PATCH v1 11/13] tests/sw_sync: Add subtest test_sync_random_merge

[Eric Engestrom]

On Tue, Aug 23, 2016 at 01:56:13PM -0400, robert.f...@collabora.com wrote:
> From: Robert Foss 
> 
> This subtest verifies that creating many timelines and merging random fences
> from each timeline with eachother results in merged fences that are fully
> functional.
> 
> Signed-off-by: Robert Foss 
> ---
>  tests/sw_sync.c | 73 
> +
>  1 file changed, 73 insertions(+)
> 
> diff --git a/tests/sw_sync.c b/tests/sw_sync.c
> index 0e67ad5..8e5a7c9 100644
> --- a/tests/sw_sync.c
> +++ b/tests/sw_sync.c
> @@ -383,6 +383,76 @@ static void test_sync_multi_consumer_producer(void)
>   igt_assert_f(thread_ret == 0, "A sync thread reported failure.\n");
>  }
>  
> +static void test_sync_random_merge(void)
> +{
> + int i, size, ret;
> + const int nbr_timeline = 32;
> + const int nbr_merge = 1024;
> + int fence_map[nbr_timeline];
> + int timeline_arr[nbr_timeline];
> + int fence, tmpfence, merged;
> + int timeline, timeline_offset, sync_pt;
> +
> + srand(time(NULL));
> +
> + for (i = 0; i < nbr_timeline; i++)
> + timeline_arr[i] = sw_sync_timeline_create();
> +
> + memset(fence_map, -1, sizeof(fence_map));

This sets each byte to -1, which happens to be the same as an int -1,
but I don't really like it: this is only true for two special values, -1
and 0, and would become a bug if it was ever changed.

I'd much prefer if you simply set:
fence_map[i] = -1;
in the loop you already have just above.

BTW, seeing how many times you set and test for -1 as the "invalid fd
sentinel", how about a:
#define INVALID_FD (-1)
and use that token throughout the code? I think it would improve
readability as well

> +
> + sync_pt = rand();
> + fence = sw_sync_fence_create(timeline_arr[0], sync_pt);
> +
> + fence_map[0] = sync_pt;
> +
> + /* Randomly create syncpoints out of a fixed set of timelines,
> +  * and merge them together.
> +  */
> + for (i = 0; i < nbr_merge; i++) {
> + /* Generate syncpoint. */
> + timeline_offset = rand() % nbr_timeline;
> + timeline = timeline_arr[timeline_offset];
> + sync_pt = rand();
> +
> + /* Keep track of the latest sync_pt in each timeline. */
> + if (fence_map[timeline_offset] == -1)
> + fence_map[timeline_offset] = sync_pt;
> + else if (fence_map[timeline_offset] < sync_pt)
> + fence_map[timeline_offset] = sync_pt;
> +
> + /* Merge. */
> + tmpfence = sw_sync_fence_create(timeline, sync_pt);
> + merged = sw_sync_merge(tmpfence, fence);
> + sw_sync_fence_destroy(tmpfence);
> + sw_sync_fence_destroy(fence);
> + fence = merged;
> + }
> +
> + size = 0;
> + for (i = 0; i < nbr_timeline; i++)
> + if (fence_map[i] != -1)
> + size++;
> +
> + /* Trigger the merged fence. */
> + for (i = 0; i < nbr_timeline; i++) {
> + if (fence_map[i] != -1) {
> + ret = sw_sync_wait(fence, 0);
> + igt_assert_f(ret == 0,
> + "Failure waiting on fence until timeout\n");
> + /* Increment the timeline to the last sync_pt */
> + sw_sync_timeline_inc(timeline_arr[i], fence_map[i]);
> + }
> + }
> +
> + /* Check that the fence is triggered. */
> + ret = sw_sync_wait(fence, 0);
> + igt_assert_f(ret > 0, "Failure triggering fence\n");
> +
> + sw_sync_fence_destroy(fence);
> + for (i = 0; i < nbr_timeline; i++)
> + sw_sync_timeline_destroy(timeline_arr[i]);
> +}
> +
>  igt_main
>  {
>   igt_subtest("alloc_timeline")
> @@ -411,5 +481,8 @@ igt_main
>  
>   igt_subtest("sync_multi_consumer_producer")
>   test_sync_multi_consumer_producer();
> +
> + igt_subtest("sync_random_merge")
> + test_sync_random_merge();
>  }
>  
> -- 
> 2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [Mesa-dev] [PATCH v1 02/13] tests/sw_sync: Add sw_sync test

[Eric Engestrom]

On Tue, Aug 23, 2016 at 01:56:04PM -0400, robert.f...@collabora.com wrote:
> From: Robert Foss 
> 
> Add initial tests for sw_sync.
> 
> Signed-off-by: Gustavo Padovan 
> Signed-off-by: Robert Foss 
> ---
>  lib/sw_sync.c  | 13 ++---
>  lib/sw_sync.h  |  2 +-
>  tests/Makefile.sources |  1 +
>  tests/sw_sync.c| 51 
> ++
>  4 files changed, 59 insertions(+), 8 deletions(-)
>  create mode 100644 tests/sw_sync.c
> 
> diff --git a/lib/sw_sync.c b/lib/sw_sync.c
> index c4e7d07..e3d5f85 100644
> --- a/lib/sw_sync.c
> +++ b/lib/sw_sync.c
> @@ -150,8 +150,6 @@ int sw_sync_wait(int fence, int timeout)
>  
>   ret = poll(, 1, timeout);
>  
> - sw_sync_fd_close(fence);
> -
>   return ret;
>  }
>  
> @@ -179,9 +177,10 @@ static struct sync_file_info *sync_file_info(int fd)
>   info->num_fences = num_fences;
>  
>   fence_info = calloc(num_fences, sizeof(struct sync_fence_info));
> - if (!fence_info)
> + if (!fence_info) {
>   free(info);
>   return NULL;
> + }

Oh, I see you fixed it here.
I would fold all these lib/ hunks from this patch into the previous
patch though, they don't belong in "Add sw_sync test" IMO, and having
them as a separate patch between the two would mean knowingly
introducing bugs in one commit and fixing them in the next.

>  
>   info->sync_fence_info = (uint64_t)(unsigned long) (fence_info);
>  
> @@ -217,18 +216,18 @@ int sw_sync_fence_size(int fd)
>   return count;
>  }
>  
> -int sw_sync_fence_count_with_status(int fd, int status)
> +int sw_sync_fence_count_status(int fd, int status)
>  {
>   int i, count = 0;
> - struct sync_fence_info *fenceInfo = NULL;
> + struct sync_fence_info *fence_info = NULL;
>   struct sync_file_info *info = sync_file_info(fd);
>  
>   if (!info)
>   return -1;
>  
> - fenceInfo = (struct sync_fence_info *)(uintptr_t)info->sync_fence_info;
> + fence_info = (struct sync_fence_info *)(uintptr_t)info->sync_fence_info;
>   for (i = 0 ; i < info->num_fences ; i++) {
> - if (fenceInfo[i].status == status)
> + if (fence_info[i].status == status)
>   count++;
>   }
>  
> diff --git a/lib/sw_sync.h b/lib/sw_sync.h
> index b179adf..1092608 100644
> --- a/lib/sw_sync.h
> +++ b/lib/sw_sync.h
> @@ -43,7 +43,7 @@ void sw_sync_timeline_inc(int fd, uint32_t count);
>  int sw_sync_merge(int fd1, int fd2);
>  int sw_sync_wait(int fence, int timeout);
>  int sw_sync_fence_size(int fd);
> -int sw_sync_fence_count_with_status(int fd, int status);
> +int sw_sync_fence_count_status(int fd, int status);
>  
>  #endif
>  
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 72a58ad..0ba769f 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -125,6 +125,7 @@ TESTS_progs_M = \
>   prime_mmap_kms \
>   prime_self_import \
>   prime_vgem \
> + sw_sync \
>   template \
>   vgem_basic \
>   vgem_slow \
> diff --git a/tests/sw_sync.c b/tests/sw_sync.c
> new file mode 100644
> index 000..d2d4c42
> --- /dev/null
> +++ b/tests/sw_sync.c
> @@ -0,0 +1,51 @@
> +/*
> + * Copyright 2012 Google, Inc
> + * Copyright © 2016 Collabora, Ltd.
> + *
> + * Based on the implementation from the Android Open Source Project
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
> DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + *Robert Foss 
> + */
> +
> +#include 
> +
> +#include "sw_sync.h"
> +#include "igt.h"
> +#include "igt_aux.h"
> +
> +IGT_TEST_DESCRIPTION("Test SW Sync Framework");
> +
> +static void test_alloc_timeline(void)
> +{
> + int timeline;
> +

Re: [Intel-gfx] [Mesa-dev] [PATCH v1 01/13] lib/sw_sync: Add helper functions for managing synchronization primitives

[Eric Engestrom]

On Tue, Aug 23, 2016 at 01:56:03PM -0400, robert.f...@collabora.com wrote:
> From: Robert Foss 
> 
> Base functions to help testing the Sync File Framework (explicit fencing
> mechanism ported from Android).
> These functions allow you to create, use and destroy timelines and fences.
> 
> Signed-off-by: Gustavo Padovan 
> Signed-off-by: Robert Foss 
> ---
>  lib/Makefile.sources |   2 +
>  lib/sw_sync.c| 238 
> +++
>  lib/sw_sync.h|  49 +++
>  3 files changed, 289 insertions(+)
>  create mode 100644 lib/sw_sync.c
>  create mode 100644 lib/sw_sync.h
> 

[snip]

> +int sw_sync_fd_is_valid(int fd)
> +{
> + int status;
> +
> + if (fd == -1)

`-1` seems too specific. While open() will return -1 on error, any
negative fd is invalid, so I'd test for `<0` here instead.

> + return 0;
> +
> + status = fcntl(fd, F_GETFD, 0);
> + return status >= 0;
> +}
> +
> +static
> +void sw_sync_fd_close(int fd)
> +{
> + if (fd == -1)
> + return;
> +
> + if (fcntl(fd, F_GETFD, 0) < 0)
> + return;

Why not replace these two tests with a simple
if (sw_sync_fd_is_valid(fd))

> +
> + close(fd);
> +}
> +
> +int sw_sync_timeline_create(void)
> +{
> + int fd = open("/dev/sw_sync", O_RDWR);
> +
> + if (!sw_sync_fd_is_valid(fd))
> + fd = open("/sys/kernel/debug/sync/sw_sync", O_RDWR);

I tend to prefer for hard-coded paths to be in a #define at the top, but
I don't know what the policy for that is in IGT.

> +
> + igt_assert(sw_sync_fd_is_valid(fd));
> +
> + return fd;
> +}
> +

[snip]

> +static struct sync_file_info *sync_file_info(int fd)
> +{
> + struct sync_file_info *info;
> + struct sync_fence_info *fence_info;
> + int err, num_fences;
> +
> + info = malloc(sizeof(*info));
> + if (info == NULL)
> + return NULL;
> +
> + memset(info, 0, sizeof(*info));

You could replace malloc() + memset(0) with calloc(), as you're doing
a few lines below.

> + err = ioctl(fd, SYNC_IOC_FILE_INFO, info);
> + if (err < 0) {
> + free(info);
> + return NULL;
> + }
> +
> + num_fences = info->num_fences;
> +
> + if (num_fences) {
> + info->flags = 0;
> + info->num_fences = num_fences;
> +
> + fence_info = calloc(num_fences, sizeof(struct sync_fence_info));

sizeof(*fence_info)

> + if (!fence_info)
> + free(info);
> + return NULL;

Missing braces

> +
> + info->sync_fence_info = (uint64_t)(unsigned long) (fence_info);
> +
> + err = ioctl(fd, SYNC_IOC_FILE_INFO, info);
> + if (err < 0) {
> + free(fence_info);
> + free(info);
> + return NULL;
> + }
> + }
> +
> + return info;
> +}

[snip]

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

[Intel-gfx] [FIXUP] drm: remove `const` attribute to hint at caller that they now own the memory

[Eric Engestrom]

Signed-off-by: Eric Engestrom <eric.engest...@imgtec.com>
---
 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c  | 2 +-
 drivers/gpu/drm/amd/amdgpu/dce_v11_0.c  | 2 +-
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c   | 2 +-
 drivers/gpu/drm/drm_atomic.c| 2 +-
 drivers/gpu/drm/drm_crtc.c  | 8 
 drivers/gpu/drm/drm_fourcc.c| 4 ++--
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 2 +-
 drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
 drivers/gpu/drm/i915/intel_atomic_plane.c   | 2 +-
 drivers/gpu/drm/i915/intel_display.c| 4 ++--
 drivers/gpu/drm/radeon/atombios_crtc.c  | 4 ++--
 include/drm/drm_fourcc.h| 2 +-
 12 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
index 0bf8959..741da36 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
@@ -2071,7 +2071,7 @@ static int dce_v10_0_crtc_do_set_base(struct drm_crtc 
*crtc,
u32 tmp, viewport_w, viewport_h;
int r;
bool bypass_lut = false;
-   const char *format_name;
+   char *format_name;
 
/* no fb bound */
if (!atomic && !crtc->primary->fb) {
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c 
b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
index 1558a97..2282eb6 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
@@ -2046,7 +2046,7 @@ static int dce_v11_0_crtc_do_set_base(struct drm_crtc 
*crtc,
u32 tmp, viewport_w, viewport_h;
int r;
bool bypass_lut = false;
-   const char *format_name;
+   char *format_name;
 
/* no fb bound */
if (!atomic && !crtc->primary->fb) {
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c 
b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
index 71a0375..8b7ad34 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
@@ -1952,7 +1952,7 @@ static int dce_v8_0_crtc_do_set_base(struct drm_crtc 
*crtc,
u32 viewport_w, viewport_h;
int r;
bool bypass_lut = false;
-   const char *format_name;
+   char *format_name;
 
/* no fb bound */
if (!atomic && !crtc->primary->fb) {
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 087391f..5cb2e22 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -837,7 +837,7 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
/* Check whether this plane supports the fb pixel format. */
ret = drm_plane_check_pixel_format(plane, state->fb->pixel_format);
if (ret) {
-   const char *format_name = 
drm_get_format_name(state->fb->pixel_format);
+   char *format_name = 
drm_get_format_name(state->fb->pixel_format);
DRM_DEBUG_ATOMIC("Invalid pixel format %s\n", format_name);
kfree(format_name);
return ret;
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 7da5d33..30ab28b 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2592,7 +2592,7 @@ static int __setplane_internal(struct drm_plane *plane,
/* Check whether this plane supports the fb pixel format. */
ret = drm_plane_check_pixel_format(plane, fb->pixel_format);
if (ret) {
-   const char *format_name = drm_get_format_name(fb->pixel_format);
+   char *format_name = drm_get_format_name(fb->pixel_format);
DRM_DEBUG_KMS("Invalid pixel format %s\n", format_name);
kfree(format_name);
goto out;
@@ -2903,7 +2903,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
ret = drm_plane_check_pixel_format(crtc->primary,
   fb->pixel_format);
if (ret) {
-   const char *format_name = 
drm_get_format_name(fb->pixel_format);
+   char *format_name = 
drm_get_format_name(fb->pixel_format);
DRM_DEBUG_KMS("Invalid pixel format %s\n", 
format_name);
kfree(format_name);
goto out;
@@ -3281,7 +3281,7 @@ int drm_mode_addfb(struct drm_device *dev,
 static int format_check(const struct drm_mode_fb_cmd2 *r)
 {
uint32_t format = r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN;
-   const char *format_name;
+   char *format_name;
 
switch (format) {
case DRM_FORMAT_C8:
@@ -3359,7 +3359,7 @@ static int framebuffer_check(const struct 
drm_mode_fb_cmd2 *r)
 
ret = format_check(r);
if (ret) {
-

Re: [Intel-gfx] [PATCH] drm: make drm_get_format_name thread-safe

[Eric Engestrom]

On Mon, Aug 15, 2016 at 03:52:07PM +0200, Daniel Vetter wrote:
> On Mon, Aug 15, 2016 at 04:13:54PM +0300, Jani Nikula wrote:
> > On Mon, 15 Aug 2016, Eric Engestrom <eric.engest...@imgtec.com> wrote:
> > > On Mon, Aug 15, 2016 at 12:54:01PM +0300, Jani Nikula wrote:
> > >> On Mon, 15 Aug 2016, Eric Engestrom <e...@engestrom.ch> wrote:
> > >> > Signed-off-by: Eric Engestrom <e...@engestrom.ch>
> > >> > ---
> > >> >
> > >> > I moved the main bits to be the first diffs, shouldn't affect anything
> > >> > when applying the patch, but I wanted to ask:
> > >> > I don't like the hard-coded `32` the appears in both kmalloc() and
> > >> > snprintf(), what do you think? If you don't like it either, what would
> > >> > you suggest? Should I #define it?
> > >> >
> > >> > Second question is about the patch mail itself: should I send this kind
> > >> > of patch separated by module, with a note requesting them to be 
> > >> > squashed
> > >> > when applying? It has to land as a single patch, but for review it 
> > >> > might
> > >> > be easier if people only see the bits they each care about, as well as
> > >> > to collect ack's/r-b's.
> > >> >
> > >> > Cheers,
> > >> >   Eric
> > >> >
> > >> > ---
> > >> >  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c  |  6 ++--
> > >> >  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c  |  6 ++--
> > >> >  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c   |  6 ++--
> > >> >  drivers/gpu/drm/drm_atomic.c|  5 ++--
> > >> >  drivers/gpu/drm/drm_crtc.c  | 21 -
> > >> >  drivers/gpu/drm/drm_fourcc.c| 17 ++-
> > >> >  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c |  6 ++--
> > >> >  drivers/gpu/drm/i915/i915_debugfs.c | 11 ++-
> > >> >  drivers/gpu/drm/i915/intel_atomic_plane.c   |  6 ++--
> > >> >  drivers/gpu/drm/i915/intel_display.c| 39 
> > >> > -
> > >> >  drivers/gpu/drm/radeon/atombios_crtc.c  | 12 +---
> > >> >  include/drm/drm_fourcc.h|  2 +-
> > >> >  12 files changed, 89 insertions(+), 48 deletions(-)
> > >> >
> > >> > diff --git a/drivers/gpu/drm/drm_fourcc.c 
> > >> > b/drivers/gpu/drm/drm_fourcc.c
> > >> > index 0645c85..38216a1 100644
> > >> > --- a/drivers/gpu/drm/drm_fourcc.c
> > >> > +++ b/drivers/gpu/drm/drm_fourcc.c
> > >> > @@ -39,16 +39,14 @@ static char printable_char(int c)
> > >> >   * drm_get_format_name - return a string for drm fourcc format
> > >> >   * @format: format to compute name of
> > >> >   *
> > >> > - * Note that the buffer used by this function is globally shared and 
> > >> > owned by
> > >> > - * the function itself.
> > >> > - *
> > >> > - * FIXME: This isn't really multithreading safe.
> > >> > + * Note that the buffer returned by this function is owned by the 
> > >> > caller
> > >> > + * and will need to be freed.
> > >> >   */
> > >> >  const char *drm_get_format_name(uint32_t format)
> > >> 
> > >> I find it surprising that a function that allocates a buffer returns a
> > >> const pointer. Some userspace libraries have conventions about the
> > >> ownership based on constness.
> > >> 
> > >> (I also find it suprising that kfree() takes a const pointer; arguably
> > >> that call changes the memory.)
> > >> 
> > >> Is there precedent for this?
> > >> 
> > >> BR,
> > >> Jani.
> > >
> > > It's not a const pointer, it's a normal pointer to a const char, i.e.
> > > you can do as you want with the pointer but you shouldn't change the
> > > chars it points to.
> > 
> > Ermh, that's what I meant even if I was sloppy in my reply. And arguably
> > freeing the bytes the pointer points at changes them, albeit subtly. And
> > having a function return a pointer to const data is often an indication
> > that the ownership of the data isn't transfered, i.e. you're not
> > supposed to free it yourself.
> 
> I already applied the patch, but yes dropping the const would be a good
> hint to callers that they now own that block of memory. Eric, can you pls
> follow up with a fix up patch - drm-misc is non-rebasing?
> -Daniel

I didn't know about that convention. I'll send a fixup patch, but it'll
have to wait until tomorrow night. I hope that's not an issue :(

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

Re: [Intel-gfx] [PATCH] drm: make drm_get_format_name thread-safe

[Eric Engestrom]

On Mon, Aug 15, 2016 at 12:54:01PM +0300, Jani Nikula wrote:
> On Mon, 15 Aug 2016, Eric Engestrom <e...@engestrom.ch> wrote:
> > Signed-off-by: Eric Engestrom <e...@engestrom.ch>
> > ---
> >
> > I moved the main bits to be the first diffs, shouldn't affect anything
> > when applying the patch, but I wanted to ask:
> > I don't like the hard-coded `32` the appears in both kmalloc() and
> > snprintf(), what do you think? If you don't like it either, what would
> > you suggest? Should I #define it?
> >
> > Second question is about the patch mail itself: should I send this kind
> > of patch separated by module, with a note requesting them to be squashed
> > when applying? It has to land as a single patch, but for review it might
> > be easier if people only see the bits they each care about, as well as
> > to collect ack's/r-b's.
> >
> > Cheers,
> >   Eric
> >
> > ---
> >  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c  |  6 ++--
> >  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c  |  6 ++--
> >  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c   |  6 ++--
> >  drivers/gpu/drm/drm_atomic.c|  5 ++--
> >  drivers/gpu/drm/drm_crtc.c  | 21 -
> >  drivers/gpu/drm/drm_fourcc.c| 17 ++-
> >  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c |  6 ++--
> >  drivers/gpu/drm/i915/i915_debugfs.c | 11 ++-
> >  drivers/gpu/drm/i915/intel_atomic_plane.c   |  6 ++--
> >  drivers/gpu/drm/i915/intel_display.c| 39 
> > -
> >  drivers/gpu/drm/radeon/atombios_crtc.c  | 12 +---
> >  include/drm/drm_fourcc.h|  2 +-
> >  12 files changed, 89 insertions(+), 48 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> > index 0645c85..38216a1 100644
> > --- a/drivers/gpu/drm/drm_fourcc.c
> > +++ b/drivers/gpu/drm/drm_fourcc.c
> > @@ -39,16 +39,14 @@ static char printable_char(int c)
> >   * drm_get_format_name - return a string for drm fourcc format
> >   * @format: format to compute name of
> >   *
> > - * Note that the buffer used by this function is globally shared and owned 
> > by
> > - * the function itself.
> > - *
> > - * FIXME: This isn't really multithreading safe.
> > + * Note that the buffer returned by this function is owned by the caller
> > + * and will need to be freed.
> >   */
> >  const char *drm_get_format_name(uint32_t format)
> 
> I find it surprising that a function that allocates a buffer returns a
> const pointer. Some userspace libraries have conventions about the
> ownership based on constness.
> 
> (I also find it suprising that kfree() takes a const pointer; arguably
> that call changes the memory.)
> 
> Is there precedent for this?
> 
> BR,
> Jani.

It's not a const pointer, it's a normal pointer to a const char, i.e.
you can do as you want with the pointer but you shouldn't change the
chars it points to.

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

[Intel-gfx] [PATCH] drm: make drm_get_format_name thread-safe

[Eric Engestrom]

Signed-off-by: Eric Engestrom <e...@engestrom.ch>
---

I moved the main bits to be the first diffs, shouldn't affect anything
when applying the patch, but I wanted to ask:
I don't like the hard-coded `32` the appears in both kmalloc() and
snprintf(), what do you think? If you don't like it either, what would
you suggest? Should I #define it?

Second question is about the patch mail itself: should I send this kind
of patch separated by module, with a note requesting them to be squashed
when applying? It has to land as a single patch, but for review it might
be easier if people only see the bits they each care about, as well as
to collect ack's/r-b's.

Cheers,
  Eric

---
 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c  |  6 ++--
 drivers/gpu/drm/amd/amdgpu/dce_v11_0.c  |  6 ++--
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c   |  6 ++--
 drivers/gpu/drm/drm_atomic.c|  5 ++--
 drivers/gpu/drm/drm_crtc.c  | 21 -
 drivers/gpu/drm/drm_fourcc.c| 17 ++-
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c |  6 ++--
 drivers/gpu/drm/i915/i915_debugfs.c | 11 ++-
 drivers/gpu/drm/i915/intel_atomic_plane.c   |  6 ++--
 drivers/gpu/drm/i915/intel_display.c| 39 -
 drivers/gpu/drm/radeon/atombios_crtc.c  | 12 +---
 include/drm/drm_fourcc.h|  2 +-
 12 files changed, 89 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 0645c85..38216a1 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -39,16 +39,14 @@ static char printable_char(int c)
  * drm_get_format_name - return a string for drm fourcc format
  * @format: format to compute name of
  *
- * Note that the buffer used by this function is globally shared and owned by
- * the function itself.
- *
- * FIXME: This isn't really multithreading safe.
+ * Note that the buffer returned by this function is owned by the caller
+ * and will need to be freed.
  */
 const char *drm_get_format_name(uint32_t format)
 {
-   static char buf[32];
+   char *buf = kmalloc(32, GFP_KERNEL);
 
-   snprintf(buf, sizeof(buf),
+   snprintf(buf, 32,
 "%c%c%c%c %s-endian (0x%08x)",
 printable_char(format & 0xff),
 printable_char((format >> 8) & 0xff),
@@ -73,6 +71,8 @@ EXPORT_SYMBOL(drm_get_format_name);
 void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
  int *bpp)
 {
+   const char *format_name;
+
switch (format) {
case DRM_FORMAT_C8:
case DRM_FORMAT_RGB332:
@@ -127,8 +127,9 @@ void drm_fb_get_bpp_depth(uint32_t format, unsigned int 
*depth,
*bpp = 32;
break;
default:
-   DRM_DEBUG_KMS("unsupported pixel format %s\n",
- drm_get_format_name(format));
+   format_name = drm_get_format_name(format);
+   DRM_DEBUG_KMS("unsupported pixel format %s\n", format_name);
+   kfree(format_name);
*depth = 0;
*bpp = 0;
break;
diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
index 7f90a39..030d22d 100644
--- a/include/drm/drm_fourcc.h
+++ b/include/drm/drm_fourcc.h
@@ -32,6 +32,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);
-const char *drm_get_format_name(uint32_t format);
+const char *drm_get_format_name(uint32_t format) __malloc;
 
 #endif /* __DRM_FOURCC_H__ */
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
index c1b04e9..0bf8959 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
@@ -2071,6 +2071,7 @@ static int dce_v10_0_crtc_do_set_base(struct drm_crtc 
*crtc,
u32 tmp, viewport_w, viewport_h;
int r;
bool bypass_lut = false;
+   const char *format_name;
 
/* no fb bound */
if (!atomic && !crtc->primary->fb) {
@@ -2182,8 +2183,9 @@ static int dce_v10_0_crtc_do_set_base(struct drm_crtc 
*crtc,
bypass_lut = true;
break;
default:
-   DRM_ERROR("Unsupported screen format %s\n",
-   drm_get_format_name(target_fb->pixel_format));
+   format_name = drm_get_format_name(target_fb->pixel_format);
+   DRM_ERROR("Unsupported screen format %s\n", format_name);
+   kfree(format_name);
return -EINVAL;
}
 
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c 
b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.

Re: [Intel-gfx] [PATCH 1/3] drm: extra printk() wrapper macros

[Eric Engestrom]

On Mon, Jul 11, 2016 at 07:01:27PM +0100, Dave Gordon wrote:
> We had only DRM_INFO() and DRM_ERROR(), whereas the underlying printk()
> provides several other useful intermediate levels such as NOTICE and
> WARNING. So this patch fills out the set by providing both regular and
> once-only macros for each of the levels INFO, NOTICE, and WARNING, using
> a common underlying macro that does all the token-pasting.
> 
> DRM_ERROR is unchanged, as it's not just a printk wrapper.
> 
> Signed-off-by: Dave Gordon <david.s.gor...@intel.com>
> ---
>  include/drm/drmP.h | 26 --
>  1 file changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index cf918e3e..82648b1 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -162,6 +162,26 @@ void drm_err(const char *format, ...);
>  /** \name Macros to make printk easier */
>  /*@{*/
>  
> +#define  _DRM_PRINTK(once, level, fmt, ...)  
> \

Tab after `#define`?

> + do {\
> + printk##once(KERN_##level "[" DRM_NAME "] " fmt,\
> +  ##__VA_ARGS__);\
> + } while (0)
> +
> +#define DRM_INFO(fmt, ...)   \
> + _DRM_PRINTK(, INFO, fmt, ##__VA_ARGS__)
> +#define DRM_NOTE(fmt, ...)   \
> + _DRM_PRINTK(, NOTICE, fmt, ##__VA_ARGS__)
> +#define DRM_WARN(fmt, ...)   \
> + _DRM_PRINTK(, WARNING, fmt, ##__VA_ARGS__)
> +
> +#define DRM_INFO_ONCE(fmt, ...)  
> \
> + _DRM_PRINTK(_once, INFO, fmt, __VA_ARGS__)

Missing ## here; should be: ##__VA_ARGS__
The rest looks good.

Reviewed-by: Eric Engestrom <eric.engest...@imgtec.com>

> +#define DRM_NOTE_ONCE(fmt, ...)  \
> + _DRM_PRINTK(_once, NOTICE, fmt, ##__VA_ARGS__)
> +#define DRM_WARN_ONCE(fmt, ...)  
> \
> + _DRM_PRINTK(_once, WARNING, fmt, ##__VA_ARGS__)
> +
>  /**
>   * Error output.
>   *
> @@ -187,12 +207,6 @@ void drm_err(const char *format, ...);
>   drm_err(fmt, ##__VA_ARGS__);\
>  })
>  
> -#define DRM_INFO(fmt, ...)   \
> - printk(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__)
> -
> -#define DRM_INFO_ONCE(fmt, ...)  \
> - printk_once(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__)
> -
>  /**
>   * Debug output.
>   *
> -- 
> 1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm: Prevent NULL deref in drm_name_info()

[Eric Engestrom]

On Mon, Jun 20, 2016 at 07:53:33PM +0100, Chris Wilson wrote:
> If a driver does not have a parent, or never sets the unique name for
> itself, then we may proceed to chase a NULL dereference through
> debugfs/.../name.
> 
> Testcase: igt/vgem_basic/debugfs
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vet...@ffwll.ch>

Reviewed-by: Eric Engestrom <eric.engest...@imgtec.com>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/atomic: Add __drm_atomic_get_current_plane_state

[Eric Engestrom]

On Thu, Jun 02, 2016 at 04:21:44PM +0200, Daniel Vetter wrote:
> ... and use it in msm Again just want to encapsulate
> drm_atomic_state internals a bit.
> 
> The const threading is a bit awkward in vc4 since C sucks, but I still
> think it's worth to enforce this. Eventually I want to make all the
> obj->state pointers const too, but that's a lot more work ...
> 
> v2: Provide safe macro to wrap up the unsafe helper better, suggested
> by Maarten.
> 
> Cc: Eric Anholt 
> Cc: Rob Clark 
> Cc: Maarten Lankhorst 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 10 ++---
>  drivers/gpu/drm/vc4/vc4_crtc.c   | 13 ++--
>  drivers/gpu/drm/vc4/vc4_drv.h|  2 +-
>  drivers/gpu/drm/vc4/vc4_plane.c  |  5 +++--
>  include/drm/drm_atomic.h | 36 
> 
>  include/drm/drm_atomic_helper.h  | 24 +++--
>  6 files changed, 66 insertions(+), 24 deletions(-)

[...]

> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index 92c84e9ab09a..4e97186293be 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -109,6 +109,42 @@ drm_atomic_get_existing_connector_state(struct 
> drm_atomic_state *state,
>   return state->connector_states[index];
>  }
>  
> +/**
> + * __drm_atomic_get_current_plane_state - get current plane state
> + * @state: global atomic state object
> + * @plane: plane to grab
> + *
> + * This function returns the plane state for the given plane, either from
> + * @state, or if the plane isn't part of the atomic state update, from 
> @plane.
> + * This is useful in atomic check callbacks, when drivers need to peek at, 
> but
> + * not change, state of other planes, since it avoids threading an error code
> + * back up the call chain.
> + *
> + * WARNING:
> + *
> + * Note that this function is in general unsafe since it doesn't check for 
> the
> + * required locking for access state structures. Drivers must ensure that it 
> is
> + * save to access the returned state structure through other means. One 
> commone

s/save/safe/
s/commone/common/

> + * example is when planes are fixed to a single CRTC, and the driver knows 
> that
> + * the CRTC locks is held already. In that case holding the CRTC locks gives 
> a
> + * read-lock on all planes connected to that CRTC. But if planes can be
> + * reassigned things get more tricky. In that case it's better to use
> + * drm_atomic_get_plane_state and wire up full error handling.
> + *
> + * Returns:
> + *
> + * Read-only pointer to the current plane state.
> + */
> +static inline const struct drm_plane_state *
> +__drm_atomic_get_current_plane_state(struct drm_atomic_state *state,
> +  struct drm_plane *plane)
> +{
> + if (state->plane_states[drm_plane_index(plane)])
> + return state->plane_states[drm_plane_index(plane)];
> +
> + return plane->state;
> +}
> +
>  int __must_check
>  drm_atomic_set_mode_for_crtc(struct drm_crtc_state *state,
>struct drm_display_mode *mode);
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [Mesa-dev] [PATCH v2 mesa] vk/intel: use negative VK_NO_PROTOTYPES scheme

[Eric Engestrom]

On Sun, May 01, 2016 at 09:39:58PM -0700, Jason Ekstrand wrote:
> On May 1, 2016 6:04 PM, "Kenneth Graunke"  wrote:
> > On Sunday, May 1, 2016 9:51:00 AM PDT Emil Velikov wrote:
> > > Adding the anv authors.
> > >
> > > Jason, Chad, is there a canonical place where changes to
> > > vulkan_intel.h should land first ? Eric has a nice fix which we want
> > > in mesa.
> > >
> > > Thanks
> > > Emil
> >
> > I'm pretty sure Mesa is the only repository where this lives.
> 
> Yup

Ping?

I'm not sure why this didn't get at least an ACK from Intel, but I don't
see any reason not to apply this fix.
I don't have commit access; Emil, can you do it?

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

Re: [Intel-gfx] [Mesa-dev] [PATCH v2 mesa] vk/intel: use negative VK_NO_PROTOTYPES scheme

[Eric Engestrom]

On Mon, Apr 25, 2016 at 05:08:18PM +0100, Emil Velikov wrote:
> On 21 April 2016 at 11:24, Eric Engestrom <eric.engest...@imgtec.com> wrote:
> > Commit 3d0fac7aca237bbe8ed8e2a362d3b42d0ef8c46c changed all the
> > VK_PROTOTYPES to VK_NO_PROTOTYPES
> > This brings the Intel header in line with the rest of the Vulkan code.
> >
> > Signed-off-by: Eric Engestrom <eric.engest...@imgtec.com>
> Reviewed-by: Emil Velikov <emil.veli...@collabora.com>
> 
> > ---
> >
> > This might break code that was using the old guard scheme (not sure how
> > that could've worked anyway).
> > What the policy on this?
> >
> > v2: rebase on top of 3caf2e89aa1711e80db80d2056e0a44663d9c7d2
> > ("anv: fix build without Wayland platform"). Should've done that the
> > first time around, sorry :]
> >
> >  include/vulkan/vulkan_intel.h  | 2 +-
> >  src/intel/vulkan/anv_private.h | 1 -
> >  2 files changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/include/vulkan/vulkan_intel.h b/include/vulkan/vulkan_intel.h
> > index 1f77128..8ede61b 100644
> > --- a/include/vulkan/vulkan_intel.h
> > +++ b/include/vulkan/vulkan_intel.h
> > @@ -44,7 +44,7 @@ typedef struct VkDmaBufImageCreateInfo_
> >
> >  typedef VkResult (VKAPI_PTR *PFN_vkCreateDmaBufImageINTEL)(VkDevice 
> > device, const VkDmaBufImageCreateInfo* pCreateInfo, const 
> > VkAllocationCallbacks* pAllocator, VkDeviceMemory* pMem, VkImage* pImage);
> >
> > -#ifdef VK_PROTOTYPES
> > +#ifndef VK_NO_PROTOTYPES
> >
> Would be great to hear from the Intel guys, if there is a another
> Cannonical repo where this change should land first ?

CC'ing intel-gfx even though this isn't X-related, because I don't know
of any other way to contact you guys :]

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

[Intel-gfx] [PATCH i-g-t 4/5] tests: fix spelling mistakes

[Eric Engestrom]

Signed-off-by: Eric Engestrom <e...@engestrom.ch>
---
 tests/gem_concurrent_all.c | 2 +-
 tests/gem_cpu_reloc.c  | 2 +-
 tests/gem_flink_race.c | 2 +-
 tests/gem_seqno_wrap.c | 2 +-
 tests/gem_tiled_wb.c   | 2 +-
 tests/prime_nv_api.c   | 2 +-
 tests/prime_nv_pcopy.c | 2 +-
 tests/prime_self_import.c  | 4 ++--
 8 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/tests/gem_concurrent_all.c b/tests/gem_concurrent_all.c
index 10e5357..688eecb 100644
--- a/tests/gem_concurrent_all.c
+++ b/tests/gem_concurrent_all.c
@@ -1530,7 +1530,7 @@ run_mode(const char *prefix,
  p->copy, h->hang);
}
 
-   /* and finally try to trick the kernel into loosing the 
pending write */
+   /* and finally try to trick the kernel into losing the 
pending write */
igt_subtest_f("%s-%s-%s-gpu-read-after-write%s%s", 
prefix, mode->name, p->prefix, suffix, h->suffix) {
buffers_create();
run_wrap_func(,
diff --git a/tests/gem_cpu_reloc.c b/tests/gem_cpu_reloc.c
index 520030a..7324c4c 100644
--- a/tests/gem_cpu_reloc.c
+++ b/tests/gem_cpu_reloc.c
@@ -232,7 +232,7 @@ static void run_test(int fd, int count)
igt_progress("gem_cpu_reloc: ", 2*count+i, 3*count);
}
 
-   igt_info("Subtest suceeded, cleanup up - this might take a while.\n");
+   igt_info("Subtest succeeded, cleanup up - this might take a while.\n");
for (i = 0; i < count; i++) {
gem_close(fd, handles[i]);
}
diff --git a/tests/gem_flink_race.c b/tests/gem_flink_race.c
index 30e33f6..5937bc0 100644
--- a/tests/gem_flink_race.c
+++ b/tests/gem_flink_race.c
@@ -143,7 +143,7 @@ static void test_flink_close(void)
void *status;
int fake;
 
-   /* Allocate exit handler fds in here so that we dont screw
+   /* Allocate exit handler fds in here so that we don't screw
 * up the counts */
fake = drm_open_driver(DRIVER_INTEL);
 
diff --git a/tests/gem_seqno_wrap.c b/tests/gem_seqno_wrap.c
index f6320f4..e4a6277 100644
--- a/tests/gem_seqno_wrap.c
+++ b/tests/gem_seqno_wrap.c
@@ -485,7 +485,7 @@ int main(int argc, char **argv)
"  -t --timeout=sec  set timeout to wait for testrun to sec 
seconds\n"
"  -d --dontwrap don't wrap just run the test\n"
"  -p --prewrap=nset seqno to WRAP - n for each 
testrun\n"
-   "  -r --norandom dont randomize prewrap space\n"
+   "  -r --norandom don't randomize prewrap space\n"
"  -i --buffers  number of buffers to copy\n";
 
options.rounds = SLOW_QUICK(50, 2);
diff --git a/tests/gem_tiled_wb.c b/tests/gem_tiled_wb.c
index 67d54bd..2919736 100644
--- a/tests/gem_tiled_wb.c
+++ b/tests/gem_tiled_wb.c
@@ -68,7 +68,7 @@ create_bo(int fd)
handle = gem_create(fd, SIZE);
gem_set_tiling(fd, handle, I915_TILING_X, WIDTH * sizeof(uint32_t));
 
-   /* Write throught the fence to tiled the data.
+   /* Write through the fence to tile the data.
 * We then manually detile on reading back through the mmap(wc).
 */
data = gem_mmap__gtt(fd, handle, SIZE, PROT_READ | PROT_WRITE);
diff --git a/tests/prime_nv_api.c b/tests/prime_nv_api.c
index 054a1ec..6bf891a 100644
--- a/tests/prime_nv_api.c
+++ b/tests/prime_nv_api.c
@@ -1,4 +1,4 @@
-/* wierd use of API tests */
+/* weird use of API tests */
 
 /* test1- export buffer from intel, import same fd twice into nouveau,
check handles match
diff --git a/tests/prime_nv_pcopy.c b/tests/prime_nv_pcopy.c
index b5ceabf..99eaeea 100644
--- a/tests/prime_nv_pcopy.c
+++ b/tests/prime_nv_pcopy.c
@@ -673,7 +673,7 @@ static void check3(const uint32_t *p, uint32_t pitch, 
uint32_t lines,
}
 }
 
-/* copy from nvidia bo to intel bo and copy to a linear bo to check if tiling 
went succesful */
+/* copy from nvidia bo to intel bo and copy to a linear bo to check if tiling 
went successful */
 static void test3_base(int tile_src, int tile_dst)
 {
struct nouveau_bo *bo_intel = NULL, *bo_nvidia = NULL, *bo_linear = 
NULL;
diff --git a/tests/prime_self_import.c b/tests/prime_self_import.c
index 992334d..5ec0baa 100644
--- a/tests/prime_self_import.c
+++ b/tests/prime_self_import.c
@@ -239,7 +239,7 @@ static void test_reimport_close_race(void)
uint32_t handle;
int fake;
 
-   /* Allocate exit handler fds in here so that we dont screw
+   /* Allocate exit handler fds in here so that we don't screw
 * up the counts */
fake = drm_open_driver(DRIVER_INTEL);
 
@@ -329,7 +329,7 @@ static void test_export_close_race(void)
 
threads = calloc(num_threads, s

[Intel-gfx] [PATCH i-g-t 2/5] assembler: fix spelling mistakes

[Eric Engestrom]

Signed-off-by: Eric Engestrom <e...@engestrom.ch>
---
 assembler/brw_defines.h| 2 +-
 assembler/brw_eu_compact.c | 2 +-
 assembler/brw_eu_emit.c| 4 ++--
 assembler/gen4asm.h| 4 ++--
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/assembler/brw_defines.h b/assembler/brw_defines.h
index 24e5e30..637b716 100644
--- a/assembler/brw_defines.h
+++ b/assembler/brw_defines.h
@@ -1239,7 +1239,7 @@ enum brw_message_target {
 # define GEN6_CLIP_XY_TEST (1 << 28)
 # define GEN6_CLIP_Z_TEST  (1 << 27)
 # define GEN6_CLIP_GB_TEST (1 << 26)
-/** 8-bit field of which user clip distances to clip aganist. */
+/** 8-bit field of which user clip distances to clip against. */
 # define GEN6_USER_CLIP_CLIP_DISTANCES_SHIFT   16
 # define GEN6_CLIP_MODE_NORMAL (0 << 13)
 # define GEN6_CLIP_MODE_REJECT_ALL (3 << 13)
diff --git a/assembler/brw_eu_compact.c b/assembler/brw_eu_compact.c
index d362ed3..c747e0f 100644
--- a/assembler/brw_eu_compact.c
+++ b/assembler/brw_eu_compact.c
@@ -727,7 +727,7 @@ brw_compact_instructions(struct brw_compile *p)
 dst = store + offset;
  }
 
- /* If we didn't compact this intruction, we need to move it down into
+ /* If we didn't compact this instruction, we need to move it down into
   * place.
   */
  if (offset != src_offset) {
diff --git a/assembler/brw_eu_emit.c b/assembler/brw_eu_emit.c
index 23f0da5..1f51037 100644
--- a/assembler/brw_eu_emit.c
+++ b/assembler/brw_eu_emit.c
@@ -1164,7 +1164,7 @@ get_inner_do_insn(struct brw_compile *p)
  *
  * When the matching 'else' instruction is reached (presumably by
  * countdown of the instruction count patched in by our ELSE/ENDIF
- * functions), the relevent flags are inverted.
+ * functions), the relevant flags are inverted.
  *
  * When the matching 'endif' instruction is reached, the flags are
  * popped off.  If the stack is now empty, normal execution resumes.
@@ -1431,7 +1431,7 @@ brw_ENDIF(struct brw_compile *p)
   emit_endif = false;
 
/*
-* A single next_insn() may change the base adress of instruction store
+* A single next_insn() may change the base address of instruction store
 * memory(p->store), so call it first before referencing the instruction
 * store pointer from an index
 */
diff --git a/assembler/gen4asm.h b/assembler/gen4asm.h
index 6b957e2..1e75126 100644
--- a/assembler/gen4asm.h
+++ b/assembler/gen4asm.h
@@ -173,9 +173,9 @@ static inline char *label_name(struct 
brw_program_instruction *i)
 return i->insn.label.name;
 }
 
-static inline bool is_relocatable(struct brw_program_instruction *intruction)
+static inline bool is_relocatable(struct brw_program_instruction *instruction)
 {
-return intruction->type == GEN4ASM_INSTRUCTION_GEN_RELOCATABLE;
+return instruction->type == GEN4ASM_INSTRUCTION_GEN_RELOCATABLE;
 }
 
 /**
-- 
2.8.0

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

[Intel-gfx] [PATCH i-g-t 5/5] tools: fix spelling mistakes

[Eric Engestrom]

Signed-off-by: Eric Engestrom <e...@engestrom.ch>
---
 tools/intel_audio_dump.c  | 14 +++---
 tools/intel_bios.h|  6 +++---
 tools/intel_display_poller.c  |  2 +-
 tools/intel_dump_decode.c |  2 +-
 tools/intel_opregion_decode.c |  2 +-
 tools/null_state_gen/intel_renderstate_gen7.c |  2 +-
 tools/null_state_gen/intel_renderstate_gen8.c |  2 +-
 tools/null_state_gen/intel_renderstate_gen9.c |  2 +-
 8 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/tools/intel_audio_dump.c b/tools/intel_audio_dump.c
index 8c24230..be6a1ea 100644
--- a/tools/intel_audio_dump.c
+++ b/tools/intel_audio_dump.c
@@ -318,8 +318,8 @@ static const char * const dis_eld_valid_pulse_trans[] = {
 };
 
 static const char * const dis_pd_pulse_trans[] = {
-   [0] = "Enable Presense Detect pulse transition when unsol is disabled",
-   [1] = "Disable Presense Detect pulse transition when unsol is disabled",
+   [0] = "Enable Presence Detect pulse transition when unsol is disabled",
+   [1] = "Disable Presence Detect pulse transition when unsol is disabled",
 };
 
 static const char * const dis_ts_delta_err[] = {
@@ -2154,11 +2154,11 @@ static void dump_hsw_plus(void)
dump_reg(DISPLAY_HOTPLUG_CTL, "display hotplug control");
 
/* HSW DDI Buffer */
-   dump_reg(DDI_BUF_CTL_A,"DDI Buffer Controler A");
-   dump_reg(DDI_BUF_CTL_B,"DDI Buffer Controler B");
-   dump_reg(DDI_BUF_CTL_C,"DDI Buffer Controler C");
-   dump_reg(DDI_BUF_CTL_D,"DDI Buffer Controler D");
-   dump_reg(DDI_BUF_CTL_E,"DDI Buffer Controler E");
+   dump_reg(DDI_BUF_CTL_A,"DDI Buffer Controller A");
+   dump_reg(DDI_BUF_CTL_B,"DDI Buffer Controller B");
+   dump_reg(DDI_BUF_CTL_C,"DDI Buffer Controller C");
+   dump_reg(DDI_BUF_CTL_D,"DDI Buffer Controller D");
+   dump_reg(DDI_BUF_CTL_E,"DDI Buffer Controller E");
 
/* HSW Pipe Function */
dump_reg(PIPE_CONF_A,  "PIPE Configuration A");
diff --git a/tools/intel_bios.h b/tools/intel_bios.h
index b7ebd48..288cb56 100644
--- a/tools/intel_bios.h
+++ b/tools/intel_bios.h
@@ -796,16 +796,16 @@ enum mipi_seq_element {
  * GR18 & SWF*.
  *
  * The VBIOS/firmware will signal to the gfx driver through the ASLE interrupt
- * (visible in the interupt regs at bit 0) when it wants something done.
+ * (visible in the interrupt regs at bit 0) when it wants something done.
  *
  * Pre-965:
  * The gfx driver can make calls to the VBIOS/firmware through an SMI request,
  * generated by writing to offset 0xe0 of the device's config space (see the
- * publically available 915 PRM for details).
+ * publicly available 915 PRM for details).
  *
  * 965 and above:
  * IGD OpRegion requests to the VBIOS/firmware are made using SWSCI, which can
- * be triggered by writing to offset 0xe4 (see the publically available
+ * be triggered by writing to offset 0xe4 (see the publicly available
  * 965 graphics PRM for details).
  */
 
diff --git a/tools/intel_display_poller.c b/tools/intel_display_poller.c
index eab17c5..9460358 100644
--- a/tools/intel_display_poller.c
+++ b/tools/intel_display_poller.c
@@ -1051,7 +1051,7 @@ int main(int argc, char *argv[])
 
/*
 * check if the requires registers are
-* avilable on the current platform.
+* available on the current platform.
 */
if (IS_GEN2(devid)) {
if (pipe > 1)
diff --git a/tools/intel_dump_decode.c b/tools/intel_dump_decode.c
index 0341aad..c20f5b6 100644
--- a/tools/intel_dump_decode.c
+++ b/tools/intel_dump_decode.c
@@ -185,7 +185,7 @@ main (int argc, char *argv[])
binary = 0;
break;
default:
-   printf("unkown command options\n");
+   printf("unknown command options\n");
break;
}
}
diff --git a/tools/intel_opregion_decode.c b/tools/intel_opregion_decode.c
index c65828a..d228d30 100644
--- a/tools/intel_opregion_decode.c
+++ b/tools/intel_opregion_decode.c
@@ -404,7 +404,7 @@ int main(int argc, char *argv[])
filename = optarg;
break;
default:
-   fprintf(stderr, "unkown command options\n");
+   fprintf(stderr, "unknown command options\n");
return 1;
}
}
diff --git a/tools/null_state_gen/intel_renderstate_gen7.c 
b/tools/null_state_gen/intel_renders

[Intel-gfx] [PATCH i-g-t 3/5] lib: fix spelling mistakes

[Eric Engestrom]

Signed-off-by: Eric Engestrom <e...@engestrom.ch>
---
 lib/gpgpu_fill.c | 4 ++--
 lib/igt_aux.h| 2 +-
 lib/igt_core.c   | 6 +++---
 lib/igt_core.h   | 4 ++--
 lib/intel_reg.h  | 2 +-
 lib/ioctl_wrappers.c | 2 +-
 lib/media_fill_gen8.c| 2 +-
 lib/media_fill_gen8lp.c  | 2 +-
 lib/media_fill_gen9.c| 2 +-
 lib/media_spin.c | 4 ++--
 lib/rendercopy_gen8.c| 2 +-
 lib/rendercopy_gen9.c| 4 ++--
 lib/tests/igt_segfault.c | 4 ++--
 13 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/lib/gpgpu_fill.c b/lib/gpgpu_fill.c
index 4d98643..70d7ea3 100644
--- a/lib/gpgpu_fill.c
+++ b/lib/gpgpu_fill.c
@@ -398,7 +398,7 @@ gen8_emit_state_base_address(struct intel_batchbuffer 
*batch)
OUT_BATCH(1 << 12 | 1);
/* indirect object buffer size */
OUT_BATCH(0xf000 | 1);
-   /* intruction buffer size, must set modify enable bit, otherwise it may 
result in GPU hang */
+   /* instruction buffer size, must set modify enable bit, otherwise it 
may result in GPU hang */
OUT_BATCH(1 << 12 | 1);
 }
 
@@ -434,7 +434,7 @@ gen9_emit_state_base_address(struct intel_batchbuffer 
*batch)
OUT_BATCH(1 << 12 | 1);
/* indirect object buffer size */
OUT_BATCH(0xf000 | 1);
-   /* intruction buffer size, must set modify enable bit, otherwise it may 
result in GPU hang */
+   /* instruction buffer size, must set modify enable bit, otherwise it 
may result in GPU hang */
OUT_BATCH(1 << 12 | 1);
 
/* Bindless surface state base address */
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index cdaed29..d4a4c74 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -71,7 +71,7 @@ void igt_cleanup_aperture_trashers(void);
 void igt_system_suspend_autoresume(void);
 void igt_system_hibernate_autoresume(void);
 
-/* dropping priviledges */
+/* dropping privileges */
 void igt_drop_root(void);
 
 void igt_debug_wait_for_keypress(const char *var);
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 832361b..1123633 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -449,7 +449,7 @@ void __igt_fixture_end(void)
  * running on Android they are sometimes killed by the Android low memory 
killer.
  * This seems to be due to some incompatibility between the kswapd free memory
  * targets and the way the lowmemorykiller assesses free memory.
- * The low memory killer really isn't usefull in this context and has no
+ * The low memory killer really isn't useful in this context and has no
  * interaction with the gpu driver that we are testing, so the following
  * function is used to disable it by modifying one of its module parameters.
  * We still have the normal linux oom killer to protect the kernel.
@@ -490,7 +490,7 @@ static void low_mem_killer_disable(bool disable)
igt_assert(adj_scores_len > 0);
 
/* writing  to this module parameter effectively diables the
-* low memory killer. This is not a real file, so we dont need 
to
+* low memory killer. This is not a real file, so we don't need 
to
 * seek to the start or truncate it */
igt_assert_eq(write(fd, no_lowmem_killer, 
sizeof(no_lowmem_killer)),
  sizeof(no_lowmem_killer));
@@ -1645,7 +1645,7 @@ void igt_install_exit_handler(igt_exit_handler_t fn)
igt_assert_f(0, "failed to install the signal handler\n");
 }
 
-/* simulation enviroment support */
+/* simulation environment support */
 
 /**
  * igt_run_in_simulation:
diff --git a/lib/igt_core.h b/lib/igt_core.h
index b3fa735..78dc74f 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -768,7 +768,7 @@ extern enum igt_log_level igt_log_level;
  */
 #define igt_warn_on(condition) do {\
if (condition) \
-   igt_warn("Warning on condition %s in fucntion %s, file 
%s:%i\n", \
+   igt_warn("Warning on condition %s in function %s, file 
%s:%i\n", \
 #condition, __func__, __FILE__, __LINE__); \
} while (0)
 
@@ -790,7 +790,7 @@ extern enum igt_log_level igt_log_level;
  */
 #define igt_warn_on_f(condition, f...) do {\
if (condition) {\
-   igt_warn("Warning on condition %s in fucntion %s, file 
%s:%i\n", \
+   igt_warn("Warning on condition %s in function %s, file 
%s:%i\n", \
 #condition, __func__, __FILE__, __LINE__); \
igt_warn(f); \
} \
diff --git a/lib/intel_reg.h b/lib/intel_reg.h
index 0ffa803..6104623 100644
--- a/lib/intel_reg.h
+++ b/lib/intel_reg.h
@@ -246,7 +246,7 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  * not sure they refer to local (graphics) memory.
  *
  * These details are for the local memory control registers,
- * (pp301-310

[Intel-gfx] [PATCH i-g-t 1/5] README: fix spelling mistakes

[Eric Engestrom]

Signed-off-by: Eric Engestrom <e...@engestrom.ch>
---
 README | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/README b/README
index d302af3..cfb6ab2 100644
--- a/README
+++ b/README
@@ -29,7 +29,7 @@ tests/
changes. Many of the tests have subtests, which can be listed by using
the --list-subtests command line option and then run using the
--run-subtest option. If --run-subtest is not used, all subtests will
-   be run. Some tests have futher options and these are detailed by using
+   be run. Some tests have further options and these are detailed by using
the --help option.
 
The test suite can be run using the run-tests.sh script available in
-- 
2.8.0

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

[Intel-gfx] [PATCH 1/2] drm/i915: remove left over dead code

[Eric Engestrom]

ae80152ddad252f33893b92dd69f00cc53c5949f ("drm/i915: Rewrite VLV/CHV
watermark code") removed everything that would have used those vars.

Signed-off-by: Eric Engestrom <eric.engest...@imgtec.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 14 --
 1 file changed, 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b28c29f..73c290a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -485,20 +485,6 @@ static const struct intel_watermark_params 
g4x_cursor_wm_info = {
.guard_size = 2,
.cacheline_size = G4X_FIFO_LINE_SIZE,
 };
-static const struct intel_watermark_params valleyview_wm_info = {
-   .fifo_size = VALLEYVIEW_FIFO_SIZE,
-   .max_wm = VALLEYVIEW_MAX_WM,
-   .default_wm = VALLEYVIEW_MAX_WM,
-   .guard_size = 2,
-   .cacheline_size = G4X_FIFO_LINE_SIZE,
-};
-static const struct intel_watermark_params valleyview_cursor_wm_info = {
-   .fifo_size = I965_CURSOR_FIFO,
-   .max_wm = VALLEYVIEW_CURSOR_MAX_WM,
-   .default_wm = I965_CURSOR_DFT_WM,
-   .guard_size = 2,
-   .cacheline_size = G4X_FIFO_LINE_SIZE,
-};
 static const struct intel_watermark_params i965_cursor_wm_info = {
.fifo_size = I965_CURSOR_FIFO,
.max_wm = I965_CURSOR_MAX_WM,
-- 
2.7.1

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

[Intel-gfx] [PATCH 2/2] drm/i915: remove dead code

[Eric Engestrom]

79e539453b34e35f39299a899d263b0a1f1670bd ("DRM: i915: add mode setting
support") added those variables but never used them.

Signed-off-by: Eric Engestrom <eric.engest...@imgtec.com>
---
 drivers/gpu/drm/i915/intel_tv.c | 12 
 1 file changed, 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 948cbff..14f0b41 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -326,24 +326,12 @@ static const struct color_conversion sdtv_csc_yprpb = {
.rv = 0x0100, .gv = 0x03ad, .bv = 0x074d, .av = 0x0200,
 };
 
-static const struct color_conversion sdtv_csc_rgb = {
-   .ry = 0x, .gy = 0x0f00, .by = 0x, .ay = 0x0166,
-   .ru = 0x, .gu = 0x, .bu = 0x0f00, .au = 0x0166,
-   .rv = 0x0f00, .gv = 0x, .bv = 0x, .av = 0x0166,
-};
-
 static const struct color_conversion hdtv_csc_yprpb = {
.ry = 0x05b3, .gy = 0x016e, .by = 0x0728, .ay = 0x0145,
.ru = 0x07d5, .gu = 0x038b, .bu = 0x0100, .au = 0x0200,
.rv = 0x0100, .gv = 0x03d1, .bv = 0x06bc, .av = 0x0200,
 };
 
-static const struct color_conversion hdtv_csc_rgb = {
-   .ry = 0x, .gy = 0x0f00, .by = 0x, .ay = 0x0166,
-   .ru = 0x, .gu = 0x, .bu = 0x0f00, .au = 0x0166,
-   .rv = 0x0f00, .gv = 0x, .bv = 0x, .av = 0x0166,
-};
-
 static const struct video_levels component_levels = {
.blank = 279, .black = 279, .burst = 0,
 };
-- 
2.7.1

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

Re: [Intel-gfx] [PATCH] drm/i915: fix itnull.cocci warnings (fwd)

[Eric Engestrom]

I expect this is the script she mentions:
https://github.com/coccinelle/coccinellery/blob/master/itnull/itnull.cocci

Julia is one of the authors of Coccinelle, and the author of that script :)


On 18/01/16 17:20, Daniel Vetter wrote:
> On Mon, Jan 18, 2016 at 04:49:06PM +0100, Julia Lawall wrote:
>> List_for_each entry binds its first argument to an offset from the list
>> pointer, so this should not be NULL.
>>
>> Generated by: scripts/coccinelle/iterators/itnull.cocci
>>
>> Signed-off-by: Fengguang Wu 
>> ---
>>
>> Please take the patch only if it's a positive warning. Thanks!
> 
> Against which tree is this? I can't find this anywhere like that ...
> -Daniel
> 
>>
>>  intel_display.c |2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -16498,7 +16498,7 @@ void intel_modeset_preclose(struct drm_d
>>  struct intel_flip_work *work;
>>
>>  list_for_each_entry(work, >flip_work, head) {
>> -if (work && work->event &&
>> +if (work->event &&
>>  work->event->base.file_priv == file) {
>>  kfree(work->event);
>>  work->event = NULL;
>> ___
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx