Re: [PATCH v2 2/2] drm/i915: Move display device info capabilities to its own struct

[Lucas De Marchi <lucas.de.marchi@gmail.com>]

On Mon, Nov 26, 2018 at 04:07:59PM -0800, José Roberto de Souza wrote:
> This helps separate what capabilities are display capabilities.
> 
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Suggested-by: Jani Nikula <jani.nikula@linux.intel.com>
> Suggested-by: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h          |  22 ++---
>  drivers/gpu/drm/i915/i915_pci.c          | 117 +++++++++++++----------
>  drivers/gpu/drm/i915/intel_device_info.c |   4 +
>  drivers/gpu/drm/i915/intel_device_info.h |  31 +++---
>  drivers/gpu/drm/i915/intel_display.c     |   4 +-
>  drivers/gpu/drm/i915/intel_fbc.c         |   2 +-
>  6 files changed, 102 insertions(+), 78 deletions(-)

< ... >

>  static void sseu_dump(const struct sseu_dev_info *sseu, struct drm_printer *p)
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> index 88f97210dc49..858324947537 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -89,35 +89,38 @@ enum intel_ppgtt {
>  	func(is_alpha_support); \
>  	/* Keep has_* in alphabetical order */ \
>  	func(has_64bit_reloc); \
> -	func(has_csr); \
> -	func(has_ddi); \
> -	func(has_dp_mst); \
>  	func(has_reset_engine); \
> -	func(has_fbc); \
>  	func(has_fpga_dbg); \
> -	func(has_gmch_display); \
>  	func(has_guc); \
>  	func(has_guc_ct); \
> -	func(has_hotplug); \
>  	func(has_l3_dpf); \
>  	func(has_llc); \
>  	func(has_logical_ring_contexts); \
>  	func(has_logical_ring_elsq); \
>  	func(has_logical_ring_preemption); \
> -	func(has_overlay); \
>  	func(has_pooled_eu); \
> -	func(has_psr); \
>  	func(has_rc6); \
>  	func(has_rc6p); \
>  	func(has_runtime_pm); \
>  	func(has_snoop); \
>  	func(has_coherent_ggtt); \
>  	func(unfenced_needs_alignment); \
> +	func(hws_needs_physical);
> +
> +#define DEV_INFO_DISPLAY_FOR_EACH_FLAG(func) \
> +	/* Keep in alphabetical order */ \
>  	func(cursor_needs_physical); \
> -	func(hws_needs_physical); \
> +	func(has_csr); \
> +	func(has_ddi); \
> +	func(has_dp_mst); \
> +	func(has_fbc); \
> +	func(has_gmch_display); \
> +	func(has_hotplug); \
> +	func(has_ipc); \
> +	func(has_overlay); \
> +	func(has_psr); \
>  	func(overlay_needs_physical); \
> -	func(supports_tv); \
> -	func(has_ipc);
> +	func(supports_tv);
>  
>  #define GEN_MAX_SLICES		(6) /* CNL upper bound */
>  #define GEN_MAX_SUBSLICES	(8) /* ICL upper bound */
> @@ -192,6 +195,12 @@ struct intel_device_info {
>  		u16 degamma_lut_size;
>  		u16 gamma_lut_size;
>  	} color;
> +
> +	struct {
> +#define DEFINE_FLAG(name) u8 name:1
> +		DEV_INFO_DISPLAY_FOR_EACH_FLAG(DEFINE_FLAG);
> +#undef DEFINE_FLAG

why did this go to the end of the struct? I think the logical place for
it would be just after the other flags.

Other than that,

Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

Lucas De Marchi

> +	} display;
>  };
>  
>  struct intel_driver_caps {
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 6a8bed5baa87..aee697551955 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9667,7 +9667,7 @@ static u32 intel_cursor_base(const struct intel_plane_state *plane_state)
>  	const struct drm_i915_gem_object *obj = intel_fb_obj(fb);
>  	u32 base;
>  
> -	if (INTEL_INFO(dev_priv)->cursor_needs_physical)
> +	if (INTEL_INFO(dev_priv)->display.cursor_needs_physical)
>  		base = obj->phys_handle->busaddr;
>  	else
>  		base = intel_plane_ggtt_offset(plane_state);
> @@ -13240,7 +13240,7 @@ static int intel_plane_pin_fb(struct intel_plane_state *plane_state)
>  	struct i915_vma *vma;
>  
>  	if (plane->id == PLANE_CURSOR &&
> -	    INTEL_INFO(dev_priv)->cursor_needs_physical) {
> +	    INTEL_INFO(dev_priv)->display.cursor_needs_physical) {
>  		struct drm_i915_gem_object *obj = intel_fb_obj(fb);
>  		const int align = intel_cursor_alignment(dev_priv);
>  		int err;
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 14cbaf4a0e93..f23570c44323 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -1309,7 +1309,7 @@ void intel_fbc_init(struct drm_i915_private *dev_priv)
>  	fbc->active = false;
>  
>  	if (need_fbc_vtd_wa(dev_priv))
> -		mkwrite_device_info(dev_priv)->has_fbc = false;
> +		mkwrite_device_info(dev_priv)->display.has_fbc = false;
>  
>  	i915_modparams.enable_fbc = intel_sanitize_fbc_option(dev_priv);
>  	DRM_DEBUG_KMS("Sanitized enable_fbc value: %d\n",
> -- 
> 2.19.2
> 
> _______________________________________________
> 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