Re: [PATCH 2/2] drm/i915/display: move enum i9xx_plane_id to intel_display_limits.h

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

On Fri, 13 Sep 2024, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> On Fri, Sep 13, 2024 at 04:54:39PM +0300, Jani Nikula wrote:
>> Move enum i9xx_plane_id from intel_display.h to intel_display_limits.h
>> to be able to reduce dependencies on intel_display.h.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_display.h        | 10 ----------
>>  drivers/gpu/drm/i915/display/intel_display_limits.h | 10 ++++++++++
>>  drivers/gpu/drm/i915/gvt/cmd_parser.c               |  1 -
>>  3 files changed, 10 insertions(+), 11 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
>> index d10608526eee..4bdb48084cab 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display.h
>> @@ -95,16 +95,6 @@ static inline bool transcoder_is_dsi(enum transcoder transcoder)
>>  	return transcoder == TRANSCODER_DSI_A || transcoder == TRANSCODER_DSI_C;
>>  }
>>  
>> -/*
>> - * Global legacy plane identifier. Valid only for primary/sprite
>> - * planes on pre-g4x, and only for primary planes on g4x-bdw.
>> - */
>> -enum i9xx_plane_id {
>> -	PLANE_A,
>> -	PLANE_B,
>> -	PLANE_C,
>> -};
>> -
>>  #define plane_name(p) ((p) + 'A')
>>  
>>  #define for_each_plane_id_on_crtc(__crtc, __p) \
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_limits.h b/drivers/gpu/drm/i915/display/intel_display_limits.h
>> index c4775c99dc83..f0fa27e365ab 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_limits.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_limits.h
>
> why here and not in somewhere like:
> drivers/gpu/drm/i915/display/i9xx_plane.h
> ?

Here it's next to enum plane_id. This entire file exists to provide a
minimal header for the enums.

I'm not sure what the right thing is, but putting this to i9xx_plane.h
means including that file in more places, just for this thing, while
they have no use at all for the interfaces provided by i9xx_plane.h.

BR,
Jani.



>
>> @@ -49,6 +49,16 @@ enum transcoder {
>>  	I915_MAX_TRANSCODERS
>>  };
>>  
>> +/*
>> + * Global legacy plane identifier. Valid only for primary/sprite
>> + * planes on pre-g4x, and only for primary planes on g4x-bdw.
>> + */
>> +enum i9xx_plane_id {
>> +	PLANE_A,
>> +	PLANE_B,
>> +	PLANE_C,
>> +};
>> +
>>  /*
>>   * Per-pipe plane identifier.
>>   * I915_MAX_PLANES in the enum below is the maximum (across all platforms)
>> diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c b/drivers/gpu/drm/i915/gvt/cmd_parser.c
>> index 2f4c9c66b40b..81d67a46cd9e 100644
>> --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
>> +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
>> @@ -50,7 +50,6 @@
>>  #include "trace.h"
>>  
>>  #include "display/i9xx_plane_regs.h"
>> -#include "display/intel_display.h"
>>  #include "display/intel_sprite_regs.h"
>>  #include "gem/i915_gem_context.h"
>>  #include "gem/i915_gem_pm.h"
>> -- 
>> 2.39.2
>> 

-- 
Jani Nikula, Intel