All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [CI] drm/i915: Isolate i915_getparam_ioctl()
Date: Wed, 7 Aug 2019 07:27:25 -0700	[thread overview]
Message-ID: <20190807142725.GB1761@intel.com> (raw)
In-Reply-To: <20190807142041.32699-1-chris@chris-wilson.co.uk>

On Wed, Aug 07, 2019 at 03:20:41PM +0100, Chris Wilson wrote:
> This giant switch has tendrils all other the struct and does not fit
> into the rest of the driver bring up and control of i915_drv.c. Push it
> to one side so that it can grow in peace.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Acked-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/i915/Makefile        |   1 +
>  drivers/gpu/drm/i915/i915_drv.c      | 167 --------------------------
>  drivers/gpu/drm/i915/i915_drv.h      |   3 +
>  drivers/gpu/drm/i915/i915_getparam.c | 168 +++++++++++++++++++++++++++
>  4 files changed, 172 insertions(+), 167 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/i915_getparam.c
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 8fe157f71617..d9f1b23bdf77 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -41,6 +41,7 @@ subdir-ccflags-y += -I$(srctree)/$(src)
>  # core driver code
>  i915-y += i915_drv.o \
>  	  i915_irq.o \
> +	  i915_getparam.o \
>  	  i915_params.o \
>  	  i915_pci.o \
>  	  i915_scatterlist.o \
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index efe904626bf5..3480db36b63f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -62,17 +62,12 @@
>  
>  #include "gem/i915_gem_context.h"
>  #include "gem/i915_gem_ioctls.h"
> -#include "gt/intel_engine_user.h"
>  #include "gt/intel_gt.h"
>  #include "gt/intel_gt_pm.h"
> -#include "gt/intel_reset.h"
> -#include "gt/intel_workarounds.h"
> -#include "gt/uc/intel_uc.h"
>  
>  #include "i915_debugfs.h"
>  #include "i915_drv.h"
>  #include "i915_irq.h"
> -#include "i915_pmu.h"
>  #include "i915_query.h"
>  #include "i915_trace.h"
>  #include "i915_vgpu.h"
> @@ -344,168 +339,6 @@ static void intel_detect_pch(struct drm_i915_private *dev_priv)
>  	pci_dev_put(pch);
>  }
>  
> -static int i915_getparam_ioctl(struct drm_device *dev, void *data,
> -			       struct drm_file *file_priv)
> -{
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> -	struct pci_dev *pdev = dev_priv->drm.pdev;
> -	const struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu;
> -	drm_i915_getparam_t *param = data;
> -	int value;
> -
> -	switch (param->param) {
> -	case I915_PARAM_IRQ_ACTIVE:
> -	case I915_PARAM_ALLOW_BATCHBUFFER:
> -	case I915_PARAM_LAST_DISPATCH:
> -	case I915_PARAM_HAS_EXEC_CONSTANTS:
> -		/* Reject all old ums/dri params. */
> -		return -ENODEV;
> -	case I915_PARAM_CHIPSET_ID:
> -		value = pdev->device;
> -		break;
> -	case I915_PARAM_REVISION:
> -		value = pdev->revision;
> -		break;
> -	case I915_PARAM_NUM_FENCES_AVAIL:
> -		value = dev_priv->ggtt.num_fences;
> -		break;
> -	case I915_PARAM_HAS_OVERLAY:
> -		value = dev_priv->overlay ? 1 : 0;
> -		break;
> -	case I915_PARAM_HAS_BSD:
> -		value = !!intel_engine_lookup_user(dev_priv,
> -						   I915_ENGINE_CLASS_VIDEO, 0);
> -		break;
> -	case I915_PARAM_HAS_BLT:
> -		value = !!intel_engine_lookup_user(dev_priv,
> -						   I915_ENGINE_CLASS_COPY, 0);
> -		break;
> -	case I915_PARAM_HAS_VEBOX:
> -		value = !!intel_engine_lookup_user(dev_priv,
> -						   I915_ENGINE_CLASS_VIDEO_ENHANCE, 0);
> -		break;
> -	case I915_PARAM_HAS_BSD2:
> -		value = !!intel_engine_lookup_user(dev_priv,
> -						   I915_ENGINE_CLASS_VIDEO, 1);
> -		break;
> -	case I915_PARAM_HAS_LLC:
> -		value = HAS_LLC(dev_priv);
> -		break;
> -	case I915_PARAM_HAS_WT:
> -		value = HAS_WT(dev_priv);
> -		break;
> -	case I915_PARAM_HAS_ALIASING_PPGTT:
> -		value = INTEL_PPGTT(dev_priv);
> -		break;
> -	case I915_PARAM_HAS_SEMAPHORES:
> -		value = !!(dev_priv->caps.scheduler & I915_SCHEDULER_CAP_SEMAPHORES);
> -		break;
> -	case I915_PARAM_HAS_SECURE_BATCHES:
> -		value = capable(CAP_SYS_ADMIN);
> -		break;
> -	case I915_PARAM_CMD_PARSER_VERSION:
> -		value = i915_cmd_parser_get_version(dev_priv);
> -		break;
> -	case I915_PARAM_SUBSLICE_TOTAL:
> -		value = intel_sseu_subslice_total(sseu);
> -		if (!value)
> -			return -ENODEV;
> -		break;
> -	case I915_PARAM_EU_TOTAL:
> -		value = sseu->eu_total;
> -		if (!value)
> -			return -ENODEV;
> -		break;
> -	case I915_PARAM_HAS_GPU_RESET:
> -		value = i915_modparams.enable_hangcheck &&
> -			intel_has_gpu_reset(dev_priv);
> -		if (value && intel_has_reset_engine(dev_priv))
> -			value = 2;
> -		break;
> -	case I915_PARAM_HAS_RESOURCE_STREAMER:
> -		value = 0;
> -		break;
> -	case I915_PARAM_HAS_POOLED_EU:
> -		value = HAS_POOLED_EU(dev_priv);
> -		break;
> -	case I915_PARAM_MIN_EU_IN_POOL:
> -		value = sseu->min_eu_in_pool;
> -		break;
> -	case I915_PARAM_HUC_STATUS:
> -		value = intel_huc_check_status(&dev_priv->gt.uc.huc);
> -		if (value < 0)
> -			return value;
> -		break;
> -	case I915_PARAM_MMAP_GTT_VERSION:
> -		/* Though we've started our numbering from 1, and so class all
> -		 * earlier versions as 0, in effect their value is undefined as
> -		 * the ioctl will report EINVAL for the unknown param!
> -		 */
> -		value = i915_gem_mmap_gtt_version();
> -		break;
> -	case I915_PARAM_HAS_SCHEDULER:
> -		value = dev_priv->caps.scheduler;
> -		break;
> -
> -	case I915_PARAM_MMAP_VERSION:
> -		/* Remember to bump this if the version changes! */
> -	case I915_PARAM_HAS_GEM:
> -	case I915_PARAM_HAS_PAGEFLIPPING:
> -	case I915_PARAM_HAS_EXECBUF2: /* depends on GEM */
> -	case I915_PARAM_HAS_RELAXED_FENCING:
> -	case I915_PARAM_HAS_COHERENT_RINGS:
> -	case I915_PARAM_HAS_RELAXED_DELTA:
> -	case I915_PARAM_HAS_GEN7_SOL_RESET:
> -	case I915_PARAM_HAS_WAIT_TIMEOUT:
> -	case I915_PARAM_HAS_PRIME_VMAP_FLUSH:
> -	case I915_PARAM_HAS_PINNED_BATCHES:
> -	case I915_PARAM_HAS_EXEC_NO_RELOC:
> -	case I915_PARAM_HAS_EXEC_HANDLE_LUT:
> -	case I915_PARAM_HAS_COHERENT_PHYS_GTT:
> -	case I915_PARAM_HAS_EXEC_SOFTPIN:
> -	case I915_PARAM_HAS_EXEC_ASYNC:
> -	case I915_PARAM_HAS_EXEC_FENCE:
> -	case I915_PARAM_HAS_EXEC_CAPTURE:
> -	case I915_PARAM_HAS_EXEC_BATCH_FIRST:
> -	case I915_PARAM_HAS_EXEC_FENCE_ARRAY:
> -	case I915_PARAM_HAS_EXEC_SUBMIT_FENCE:
> -		/* For the time being all of these are always true;
> -		 * if some supported hardware does not have one of these
> -		 * features this value needs to be provided from
> -		 * INTEL_INFO(), a feature macro, or similar.
> -		 */
> -		value = 1;
> -		break;
> -	case I915_PARAM_HAS_CONTEXT_ISOLATION:
> -		value = intel_engines_has_context_isolation(dev_priv);
> -		break;
> -	case I915_PARAM_SLICE_MASK:
> -		value = sseu->slice_mask;
> -		if (!value)
> -			return -ENODEV;
> -		break;
> -	case I915_PARAM_SUBSLICE_MASK:
> -		value = sseu->subslice_mask[0];
> -		if (!value)
> -			return -ENODEV;
> -		break;
> -	case I915_PARAM_CS_TIMESTAMP_FREQUENCY:
> -		value = 1000 * RUNTIME_INFO(dev_priv)->cs_timestamp_frequency_khz;
> -		break;
> -	case I915_PARAM_MMAP_GTT_COHERENT:
> -		value = INTEL_INFO(dev_priv)->has_coherent_ggtt;
> -		break;
> -	default:
> -		DRM_DEBUG("Unknown parameter %d\n", param->param);
> -		return -EINVAL;
> -	}
> -
> -	if (put_user(value, param->value))
> -		return -EFAULT;
> -
> -	return 0;
> -}
> -
>  static int i915_get_bridge_dev(struct drm_i915_private *dev_priv)
>  {
>  	int domain = pci_domain_nr(dev_priv->drm.pdev->bus);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7d424ddd3523..c9476f24f5c1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2397,6 +2397,9 @@ static inline bool intel_vgpu_active(struct drm_i915_private *dev_priv)
>  	return dev_priv->vgpu.active;
>  }
>  
> +int i915_getparam_ioctl(struct drm_device *dev, void *data,
> +			struct drm_file *file_priv);
> +
>  /* i915_gem.c */
>  int i915_gem_init_userptr(struct drm_i915_private *dev_priv);
>  void i915_gem_cleanup_userptr(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_getparam.c b/drivers/gpu/drm/i915/i915_getparam.c
> new file mode 100644
> index 000000000000..5d9101376a3d
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_getparam.c
> @@ -0,0 +1,168 @@
> +/*
> + * SPDX-License-Identifier: MIT
> + */
> +
> +#include "gt/intel_engine_user.h"
> +
> +#include "i915_drv.h"
> +
> +int i915_getparam_ioctl(struct drm_device *dev, void *data,
> +			struct drm_file *file_priv)
> +{
> +	struct drm_i915_private *i915 = to_i915(dev);
> +	const struct sseu_dev_info *sseu = &RUNTIME_INFO(i915)->sseu;
> +	drm_i915_getparam_t *param = data;
> +	int value;
> +
> +	switch (param->param) {
> +	case I915_PARAM_IRQ_ACTIVE:
> +	case I915_PARAM_ALLOW_BATCHBUFFER:
> +	case I915_PARAM_LAST_DISPATCH:
> +	case I915_PARAM_HAS_EXEC_CONSTANTS:
> +		/* Reject all old ums/dri params. */
> +		return -ENODEV;
> +	case I915_PARAM_CHIPSET_ID:
> +		value = i915->drm.pdev->device;
> +		break;
> +	case I915_PARAM_REVISION:
> +		value = i915->drm.pdev->revision;
> +		break;
> +	case I915_PARAM_NUM_FENCES_AVAIL:
> +		value = i915->ggtt.num_fences;
> +		break;
> +	case I915_PARAM_HAS_OVERLAY:
> +		value = !!i915->overlay;
> +		break;
> +	case I915_PARAM_HAS_BSD:
> +		value = !!intel_engine_lookup_user(i915,
> +						   I915_ENGINE_CLASS_VIDEO, 0);
> +		break;
> +	case I915_PARAM_HAS_BLT:
> +		value = !!intel_engine_lookup_user(i915,
> +						   I915_ENGINE_CLASS_COPY, 0);
> +		break;
> +	case I915_PARAM_HAS_VEBOX:
> +		value = !!intel_engine_lookup_user(i915,
> +						   I915_ENGINE_CLASS_VIDEO_ENHANCE, 0);
> +		break;
> +	case I915_PARAM_HAS_BSD2:
> +		value = !!intel_engine_lookup_user(i915,
> +						   I915_ENGINE_CLASS_VIDEO, 1);
> +		break;
> +	case I915_PARAM_HAS_LLC:
> +		value = HAS_LLC(i915);
> +		break;
> +	case I915_PARAM_HAS_WT:
> +		value = HAS_WT(i915);
> +		break;
> +	case I915_PARAM_HAS_ALIASING_PPGTT:
> +		value = INTEL_PPGTT(i915);
> +		break;
> +	case I915_PARAM_HAS_SEMAPHORES:
> +		value = !!(i915->caps.scheduler & I915_SCHEDULER_CAP_SEMAPHORES);
> +		break;
> +	case I915_PARAM_HAS_SECURE_BATCHES:
> +		value = capable(CAP_SYS_ADMIN);
> +		break;
> +	case I915_PARAM_CMD_PARSER_VERSION:
> +		value = i915_cmd_parser_get_version(i915);
> +		break;
> +	case I915_PARAM_SUBSLICE_TOTAL:
> +		value = intel_sseu_subslice_total(sseu);
> +		if (!value)
> +			return -ENODEV;
> +		break;
> +	case I915_PARAM_EU_TOTAL:
> +		value = sseu->eu_total;
> +		if (!value)
> +			return -ENODEV;
> +		break;
> +	case I915_PARAM_HAS_GPU_RESET:
> +		value = i915_modparams.enable_hangcheck &&
> +			intel_has_gpu_reset(i915);
> +		if (value && intel_has_reset_engine(i915))
> +			value = 2;
> +		break;
> +	case I915_PARAM_HAS_RESOURCE_STREAMER:
> +		value = 0;
> +		break;
> +	case I915_PARAM_HAS_POOLED_EU:
> +		value = HAS_POOLED_EU(i915);
> +		break;
> +	case I915_PARAM_MIN_EU_IN_POOL:
> +		value = sseu->min_eu_in_pool;
> +		break;
> +	case I915_PARAM_HUC_STATUS:
> +		value = intel_huc_check_status(&i915->gt.uc.huc);
> +		if (value < 0)
> +			return value;
> +		break;
> +	case I915_PARAM_MMAP_GTT_VERSION:
> +		/* Though we've started our numbering from 1, and so class all
> +		 * earlier versions as 0, in effect their value is undefined as
> +		 * the ioctl will report EINVAL for the unknown param!
> +		 */
> +		value = i915_gem_mmap_gtt_version();
> +		break;
> +	case I915_PARAM_HAS_SCHEDULER:
> +		value = i915->caps.scheduler;
> +		break;
> +
> +	case I915_PARAM_MMAP_VERSION:
> +		/* Remember to bump this if the version changes! */
> +	case I915_PARAM_HAS_GEM:
> +	case I915_PARAM_HAS_PAGEFLIPPING:
> +	case I915_PARAM_HAS_EXECBUF2: /* depends on GEM */
> +	case I915_PARAM_HAS_RELAXED_FENCING:
> +	case I915_PARAM_HAS_COHERENT_RINGS:
> +	case I915_PARAM_HAS_RELAXED_DELTA:
> +	case I915_PARAM_HAS_GEN7_SOL_RESET:
> +	case I915_PARAM_HAS_WAIT_TIMEOUT:
> +	case I915_PARAM_HAS_PRIME_VMAP_FLUSH:
> +	case I915_PARAM_HAS_PINNED_BATCHES:
> +	case I915_PARAM_HAS_EXEC_NO_RELOC:
> +	case I915_PARAM_HAS_EXEC_HANDLE_LUT:
> +	case I915_PARAM_HAS_COHERENT_PHYS_GTT:
> +	case I915_PARAM_HAS_EXEC_SOFTPIN:
> +	case I915_PARAM_HAS_EXEC_ASYNC:
> +	case I915_PARAM_HAS_EXEC_FENCE:
> +	case I915_PARAM_HAS_EXEC_CAPTURE:
> +	case I915_PARAM_HAS_EXEC_BATCH_FIRST:
> +	case I915_PARAM_HAS_EXEC_FENCE_ARRAY:
> +	case I915_PARAM_HAS_EXEC_SUBMIT_FENCE:
> +		/* For the time being all of these are always true;
> +		 * if some supported hardware does not have one of these
> +		 * features this value needs to be provided from
> +		 * INTEL_INFO(), a feature macro, or similar.
> +		 */
> +		value = 1;
> +		break;
> +	case I915_PARAM_HAS_CONTEXT_ISOLATION:
> +		value = intel_engines_has_context_isolation(i915);
> +		break;
> +	case I915_PARAM_SLICE_MASK:
> +		value = sseu->slice_mask;
> +		if (!value)
> +			return -ENODEV;
> +		break;
> +	case I915_PARAM_SUBSLICE_MASK:
> +		value = sseu->subslice_mask[0];
> +		if (!value)
> +			return -ENODEV;
> +		break;
> +	case I915_PARAM_CS_TIMESTAMP_FREQUENCY:
> +		value = 1000 * RUNTIME_INFO(i915)->cs_timestamp_frequency_khz;
> +		break;
> +	case I915_PARAM_MMAP_GTT_COHERENT:
> +		value = INTEL_INFO(i915)->has_coherent_ggtt;
> +		break;
> +	default:
> +		DRM_DEBUG("Unknown parameter %d\n", param->param);
> +		return -EINVAL;
> +	}
> +
> +	if (put_user(value, param->value))
> +		return -EFAULT;
> +
> +	return 0;
> +}
> -- 
> 2.23.0.rc1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2019-08-07 14:26 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-07 14:20 [CI] drm/i915: Isolate i915_getparam_ioctl() Chris Wilson
2019-08-07 14:27 ` Rodrigo Vivi [this message]
2019-08-07 14:55 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2019-08-07 15:46 ` ✓ Fi.CI.BAT: success " Patchwork
2019-08-08  0:25 ` ✗ Fi.CI.IGT: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190807142725.GB1761@intel.com \
    --to=rodrigo.vivi@intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.