Re: [PATCH 3/6] drm/i915/display: add intel_display_conversion.c to hide stuff better

[Jani Nikula <jani.nikula@intel.com>]

On Thu, 31 Oct 2024, Raag Jadav <raag.jadav@intel.com> wrote:
> On Thu, Oct 31, 2024 at 01:38:33PM +0200, Jani Nikula wrote:
>> The __to_intel_display() generics require the definition of struct
>> drm_i915_private i.e. inclusion of i915_drv.h. Add
>> intel_display_conversion.c with a __i915_to_display() function to do the
>> conversion without the intel_display_conversion.h having an implicit
>> dependency on i915_drv.h.
>> 
>> The long term goal is to remove __to_intel_display() and the
>> intel_display_conversion.[ch] files altoghether, and this is merely a
>> transitional step to make the dependencies on i915_drv.h explicit.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/Makefile                           | 1 +
>>  drivers/gpu/drm/i915/display/intel_display_conversion.c | 9 +++++++++
>>  drivers/gpu/drm/i915/display/intel_display_conversion.h | 9 +++++++--
>>  drivers/gpu/drm/xe/Makefile                             | 1 +
>>  4 files changed, 18 insertions(+), 2 deletions(-)
>>  create mode 100644 drivers/gpu/drm/i915/display/intel_display_conversion.c
>> 
>> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>> index ac47d7e988fc..43686d843ef7 100644
>> --- a/drivers/gpu/drm/i915/Makefile
>> +++ b/drivers/gpu/drm/i915/Makefile
>> @@ -237,6 +237,7 @@ i915-y += \
>>  	display/intel_crtc_state_dump.o \
>>  	display/intel_cursor.o \
>>  	display/intel_display.o \
>> +	display/intel_display_conversion.o \
>>  	display/intel_display_driver.o \
>>  	display/intel_display_irq.o \
>>  	display/intel_display_params.o \
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_conversion.c b/drivers/gpu/drm/i915/display/intel_display_conversion.c
>> new file mode 100644
>> index 000000000000..bdd947f5ccd8
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/display/intel_display_conversion.c
>> @@ -0,0 +1,9 @@
>> +// SPDX-License-Identifier: MIT
>> +/* Copyright © 2024 Intel Corporation */
>> +
>> +#include "i915_drv.h"
>> +
>> +struct intel_display *__i915_to_display(struct drm_i915_private *i915)
>> +{
>> +	return &i915->display;
>> +}
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_conversion.h b/drivers/gpu/drm/i915/display/intel_display_conversion.h
>> index ad8545c8055d..790d0be698dc 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_conversion.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_conversion.h
>> @@ -8,14 +8,19 @@
>>  #ifndef __INTEL_DISPLAY_CONVERSION__
>>  #define __INTEL_DISPLAY_CONVERSION__
>>  
>> +struct drm_i915_private;
>> +struct intel_display;
>> +
>> +struct intel_display *__i915_to_display(struct drm_i915_private *i915);
>> +
>>  /*
>>   * Transitional macro to optionally convert struct drm_i915_private * to struct
>>   * intel_display *, also accepting the latter.
>>   */
>>  #define __to_intel_display(p)						\
>>  	_Generic(p,							\
>> -		 const struct drm_i915_private *: (&((const struct drm_i915_private *)(p))->display), \
>> -		 struct drm_i915_private *: (&((struct drm_i915_private *)(p))->display), \
>> +		 const struct drm_i915_private *: __i915_to_display((struct drm_i915_private *)(p)), \
>> +		 struct drm_i915_private *: __i915_to_display((struct drm_i915_private *)(p)), \
>>  		 const struct intel_display *: (p),			\
>>  		 struct intel_display *: (p))
>
> Perhaps,
>
> #define TYPE_ENTRY(type, x)	\
> 	const type: x,		\
> 	type: x
>
> May snip the duplication.

Yeah I don't really care because this is supposed to go away soon.

See to_intel_display() in intel_display_types.h for what the nicer
implementation is, for the more permanent stuff.

BR,
Jani.

>
> Raag

-- 
Jani Nikula, Intel