Re: [PATCH v2] drm/atomic: convert drm_atomic_get_{old,new}_colorop_state() into proper functions

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

On Fri, 19 Dec 2025, Alex Hung <alex.hung@amd.com> wrote:
> Reviewed-by: Alex Hung <alex.hung@amd.com>

Thanks, pushed to drm-misc-next.

BR,
Jani.c

>
> On 12/19/25 04:49, Jani Nikula wrote:
>> There is no real reason to include drm_colorop.h from drm_atomic.h, as
>> drm_atomic_get_{old,new}_colorop_state() have no real reason to be
>> static inline.
>> 
>> Convert the static inlines to proper functions, and drop the include to
>> reduce the include dependencies and improve data hiding.
>> 
>> v2: Fix vkms build failures (Alex)
>> 
>> Fixes: cfc27680ee20 ("drm/colorop: Introduce new drm_colorop mode object")
>> Cc: Simon Ser <contact@emersion.fr>
>> Cc: Alex Hung <alex.hung@amd.com>
>> Cc: Harry Wentland <harry.wentland@amd.com>
>> Cc: Daniel Stone <daniels@collabora.com>
>> Cc: Melissa Wen <mwen@igalia.com>
>> Cc: Sebastian Wick <sebastian.wick@redhat.com>
>> Cc: Alex Hung <alex.hung@amd.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> 
>> ---
>> 
>> Including the massive Cc list because I don't want to keep doing this
>> afterwards. This stuff needs to be blocked and fixed during review. Just
>> stop including headers from headers. It's a PITA to clean up.
>> ---
>>   .../amd/display/amdgpu_dm/amdgpu_dm_color.c   |  3 ++
>>   drivers/gpu/drm/drm_atomic.c                  | 32 +++++++++++++++
>>   drivers/gpu/drm/drm_atomic_helper.c           |  1 +
>>   .../drm/i915/display/intel_display_types.h    |  1 +
>>   drivers/gpu/drm/vkms/vkms_composer.c          |  1 +
>>   drivers/gpu/drm/vkms/vkms_drv.c               |  1 +
>>   include/drm/drm_atomic.h                      | 39 ++++---------------
>>   7 files changed, 47 insertions(+), 31 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
>> index 1dcc79b35225..20a76d81d532 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
>> @@ -23,6 +23,9 @@
>>    * Authors: AMD
>>    *
>>    */
>> +
>> +#include <drm/drm_colorop.h>
>> +
>>   #include "amdgpu.h"
>>   #include "amdgpu_mode.h"
>>   #include "amdgpu_dm.h"
>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>> index 6d3ea8056b60..52738b80ddbe 100644
>> --- a/drivers/gpu/drm/drm_atomic.c
>> +++ b/drivers/gpu/drm/drm_atomic.c
>> @@ -641,6 +641,38 @@ drm_atomic_get_colorop_state(struct drm_atomic_state *state,
>>   }
>>   EXPORT_SYMBOL(drm_atomic_get_colorop_state);
>>   
>> +/**
>> + * drm_atomic_get_old_colorop_state - get colorop state, if it exists
>> + * @state: global atomic state object
>> + * @colorop: colorop to grab
>> + *
>> + * This function returns the old colorop state for the given colorop, or
>> + * NULL if the colorop is not part of the global atomic state.
>> + */
>> +struct drm_colorop_state *
>> +drm_atomic_get_old_colorop_state(struct drm_atomic_state *state,
>> +				 struct drm_colorop *colorop)
>> +{
>> +	return state->colorops[drm_colorop_index(colorop)].old_state;
>> +}
>> +EXPORT_SYMBOL(drm_atomic_get_old_colorop_state);
>> +
>> +/**
>> + * drm_atomic_get_new_colorop_state - get colorop state, if it exists
>> + * @state: global atomic state object
>> + * @colorop: colorop to grab
>> + *
>> + * This function returns the new colorop state for the given colorop, or
>> + * NULL if the colorop is not part of the global atomic state.
>> + */
>> +struct drm_colorop_state *
>> +drm_atomic_get_new_colorop_state(struct drm_atomic_state *state,
>> +				 struct drm_colorop *colorop)
>> +{
>> +	return state->colorops[drm_colorop_index(colorop)].new_state;
>> +}
>> +EXPORT_SYMBOL(drm_atomic_get_new_colorop_state);
>> +
>>   static bool
>>   plane_switching_crtc(const struct drm_plane_state *old_plane_state,
>>   		     const struct drm_plane_state *new_plane_state)
>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>> index 10adac9397cf..5840e9cc6f66 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> @@ -34,6 +34,7 @@
>>   #include <drm/drm_atomic_uapi.h>
>>   #include <drm/drm_blend.h>
>>   #include <drm/drm_bridge.h>
>> +#include <drm/drm_colorop.h>
>>   #include <drm/drm_damage_helper.h>
>>   #include <drm/drm_device.h>
>>   #include <drm/drm_drv.h>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
>> index 6ff53cd58052..eb2e3f1e83c9 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>> @@ -34,6 +34,7 @@
>>   #include <drm/display/drm_dp_tunnel.h>
>>   #include <drm/display/drm_dsc.h>
>>   #include <drm/drm_atomic.h>
>> +#include <drm/drm_colorop.h>
>>   #include <drm/drm_crtc.h>
>>   #include <drm/drm_encoder.h>
>>   #include <drm/drm_framebuffer.h>
>> diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
>> index 3cf3f26e0d8e..cd85de4ffd03 100644
>> --- a/drivers/gpu/drm/vkms/vkms_composer.c
>> +++ b/drivers/gpu/drm/vkms/vkms_composer.c
>> @@ -5,6 +5,7 @@
>>   #include <drm/drm_atomic.h>
>>   #include <drm/drm_atomic_helper.h>
>>   #include <drm/drm_blend.h>
>> +#include <drm/drm_colorop.h>
>>   #include <drm/drm_fourcc.h>
>>   #include <drm/drm_fixed.h>
>>   #include <drm/drm_gem_framebuffer_helper.h>
>> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
>> index dd1402f43773..434c295f44ba 100644
>> --- a/drivers/gpu/drm/vkms/vkms_drv.c
>> +++ b/drivers/gpu/drm/vkms/vkms_drv.c
>> @@ -17,6 +17,7 @@
>>   #include <drm/drm_gem.h>
>>   #include <drm/drm_atomic.h>
>>   #include <drm/drm_atomic_helper.h>
>> +#include <drm/drm_colorop.h>
>>   #include <drm/drm_drv.h>
>>   #include <drm/drm_fbdev_shmem.h>
>>   #include <drm/drm_file.h>
>> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
>> index 74ce26fa8838..178f8f62c80f 100644
>> --- a/include/drm/drm_atomic.h
>> +++ b/include/drm/drm_atomic.h
>> @@ -30,7 +30,6 @@
>>   
>>   #include <drm/drm_crtc.h>
>>   #include <drm/drm_util.h>
>> -#include <drm/drm_colorop.h>
>>   
>>   /**
>>    * struct drm_crtc_commit - track modeset commits on a CRTC
>> @@ -712,6 +711,14 @@ drm_atomic_get_plane_state(struct drm_atomic_state *state,
>>   struct drm_colorop_state *
>>   drm_atomic_get_colorop_state(struct drm_atomic_state *state,
>>   			     struct drm_colorop *colorop);
>> +
>> +struct drm_colorop_state *
>> +drm_atomic_get_old_colorop_state(struct drm_atomic_state *state,
>> +				 struct drm_colorop *colorop);
>> +struct drm_colorop_state *
>> +drm_atomic_get_new_colorop_state(struct drm_atomic_state *state,
>> +				 struct drm_colorop *colorop);
>> +
>>   struct drm_connector_state * __must_check
>>   drm_atomic_get_connector_state(struct drm_atomic_state *state,
>>   			       struct drm_connector *connector);
>> @@ -808,36 +815,6 @@ drm_atomic_get_new_plane_state(const struct drm_atomic_state *state,
>>   	return state->planes[drm_plane_index(plane)].new_state;
>>   }
>>   
>> -/**
>> - * drm_atomic_get_old_colorop_state - get colorop state, if it exists
>> - * @state: global atomic state object
>> - * @colorop: colorop to grab
>> - *
>> - * This function returns the old colorop state for the given colorop, or
>> - * NULL if the colorop is not part of the global atomic state.
>> - */
>> -static inline struct drm_colorop_state *
>> -drm_atomic_get_old_colorop_state(struct drm_atomic_state *state,
>> -				 struct drm_colorop *colorop)
>> -{
>> -	return state->colorops[drm_colorop_index(colorop)].old_state;
>> -}
>> -
>> -/**
>> - * drm_atomic_get_new_colorop_state - get colorop state, if it exists
>> - * @state: global atomic state object
>> - * @colorop: colorop to grab
>> - *
>> - * This function returns the new colorop state for the given colorop, or
>> - * NULL if the colorop is not part of the global atomic state.
>> - */
>> -static inline struct drm_colorop_state *
>> -drm_atomic_get_new_colorop_state(struct drm_atomic_state *state,
>> -				 struct drm_colorop *colorop)
>> -{
>> -	return state->colorops[drm_colorop_index(colorop)].new_state;
>> -}
>> -
>>   /**
>>    * drm_atomic_get_old_connector_state - get connector state, if it exists
>>    * @state: global atomic state object
>

-- 
Jani Nikula, Intel