Re: [v5 12/24] drm/i915/color: Add framework to program CSC

Wed, 05 Nov 2025 04:25:46 -0800 



On 10/28/2025 1:42 PM, Kandpal, Suraj wrote:




-----Original Message-----
From: Intel-xe <[email protected]> On Behalf Of
Kandpal, Suraj
Sent: Tuesday, October 28, 2025 1:40 PM
To: Shankar, Uma <[email protected]>; [email protected];
[email protected]; [email protected]
Cc: Borah, Chaitanya Kumar <[email protected]>;
[email protected]; [email protected];
[email protected]; [email protected]; [email protected];
[email protected]; [email protected];
[email protected]; Sharma, Swati2 <[email protected]>;
[email protected]; Shankar, Uma <[email protected]>
Subject: RE: [v5 12/24] drm/i915/color: Add framework to program CSC


Subject: [v5 12/24] drm/i915/color: Add framework to program CSC

From: Chaitanya Kumar Borah <[email protected]>

Add framework to program CSC. It enables copying of matrix from uapi
to intel plane state. Also adding helper functions which will
eventually program values to hardware.

Signed-off-by: Chaitanya Kumar Borah <[email protected]>
Signed-off-by: Uma Shankar <[email protected]>
---
  drivers/gpu/drm/i915/display/intel_color.c    | 14 +++++++
  drivers/gpu/drm/i915/display/intel_color.h    |  4 +-
  .../drm/i915/display/intel_display_types.h    |  1 +
  drivers/gpu/drm/i915/display/intel_plane.c    | 39 +++++++++++++++++++
  4 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c
b/drivers/gpu/drm/i915/display/intel_color.c
index 363c9590c5c1..7c53572f729b 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -3967,6 +3967,20 @@ static const struct intel_color_funcs
ilk_color_funcs = {  };

  /* TODO: Move to another file */
+static void
+intel_color_load_plane_csc_matrix(struct intel_dsb *dsb,
+                                 const struct intel_plane_state *plane_state)
{
+       /* CTM programming */


Add TODO


+}
+
+void intel_color_plane_program_pipeline(struct intel_dsb *dsb,
+                                       const struct intel_plane_state
*plane_state) {
+       if (plane_state->hw.ctm)
+               intel_color_load_plane_csc_matrix(dsb, plane_state); }
+


Also the functions above introduced seem to be out of place introduce them 
later where they are used.




The idea here is to get the plumbing in first and then enable the 
interface later on.


==
Chaitanya


Regards,
Suraj Kandpal


  struct intel_plane_colorop *intel_colorop_alloc(void)  {
        struct intel_plane_colorop *colorop; diff --git
a/drivers/gpu/drm/i915/display/intel_color.h
b/drivers/gpu/drm/i915/display/intel_color.h
index c2561b86bb26..420d596dbbae 100644
--- a/drivers/gpu/drm/i915/display/intel_color.h
+++ b/drivers/gpu/drm/i915/display/intel_color.h
@@ -13,6 +13,7 @@ struct intel_crtc_state;  struct intel_crtc;  struct
intel_display;  struct intel_dsb;
+struct intel_plane_state;
  struct drm_property_blob;
  struct drm_plane;
  struct drm_prop_enum_list;
@@ -49,5 +50,6 @@ struct intel_plane_colorop
*intel_colorop_alloc(void); struct intel_plane_colorop
*intel_plane_colorop_create(enum intel_color_block id);  int
intel_plane_tf_pipeline_init(struct drm_plane *plane, struct
drm_prop_enum_list *list);  int intel_plane_color_init(struct
drm_plane *plane);
-
+void intel_color_plane_program_pipeline(struct intel_dsb *dsb,
+                                       const struct intel_plane_state
*plane_state);
  #endif /* __INTEL_COLOR_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 4b5124a08cc9..c709df0cea9e 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -634,6 +634,7 @@ struct intel_plane_state {
                enum drm_color_encoding color_encoding;
                enum drm_color_range color_range;
                enum drm_scaling_filter scaling_filter;
+               struct drm_property_blob *ctm;
        } hw;

        struct i915_vma *ggtt_vma;
diff --git a/drivers/gpu/drm/i915/display/intel_plane.c
b/drivers/gpu/drm/i915/display/intel_plane.c
index 36fb07471deb..cc8f3e15c82e 100644
--- a/drivers/gpu/drm/i915/display/intel_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_plane.c
@@ -380,6 +380,43 @@ intel_plane_copy_uapi_plane_damage(struct
intel_plane_state *new_plane_state,
                *damage = drm_plane_state_src(&new_uapi_plane_state-

uapi);

  }

+static void
+intel_plane_colorop_replace_blob(struct intel_plane_state *plane_state,
+                                struct intel_plane_colorop *intel_colorop,
+                                struct drm_property_blob *blob)
+{
+       if (intel_colorop->id == CB_PLANE_CSC)
+               drm_property_replace_blob(&plane_state->hw.ctm, blob); }
+
+static void
+intel_plane_copy_uapi_to_hw_state_color(struct intel_plane_state


This should be intel_plane_color_copy_uapi_to_hw_state


*plane_state,
+                                       const struct intel_plane_state
*from_plane_state,
+                                       struct intel_crtc *crtc)
+{
+       struct drm_colorop *iter_colorop, *colorop;
+       struct drm_colorop_state *new_colorop_state;
+       struct drm_atomic_state *state = plane_state->uapi.state;
+       struct intel_plane_colorop *intel_colorop;
+       struct drm_property_blob *blob;
+       int i = 0;
+
+       iter_colorop = plane_state->uapi.color_pipeline;
+
+       while (iter_colorop) {
+               for_each_new_colorop_in_state(state, colorop,
new_colorop_state, i) {
+                       if (new_colorop_state->colorop == iter_colorop) {
+                               blob = new_colorop_state->bypass ? NULL :
new_colorop_state->data;
+                               intel_colorop =
to_intel_plane_colorop(colorop);
+
        intel_plane_colorop_replace_blob(plane_state,
+
intel_colorop,
+                                                                blob);


A break here why keep iterating if you have found what you are looking for I
think I am seeing more and more reason to have a separate file as Jani had
said Called intel_plane_color.c

Regards,
Suraj Kandpal

+                       }
+               }
+               iter_colorop = iter_colorop->next;
+       }
+}
+
  void intel_plane_copy_uapi_to_hw_state(struct intel_plane_state
*plane_state,
                                       const struct intel_plane_state

*from_plane_state,

                                       struct intel_crtc *crtc)
@@ -408,6 +445,8 @@ void intel_plane_copy_uapi_to_hw_state(struct
intel_plane_state *plane_state,

        plane_state->uapi.src = drm_plane_state_src(&from_plane_state-

uapi);

        plane_state->uapi.dst = drm_plane_state_dest(&from_plane_state-

uapi);

+
+       intel_plane_copy_uapi_to_hw_state_color(plane_state,
from_plane_state,
+crtc);
  }

  void intel_plane_copy_hw_state(struct intel_plane_state *plane_state,
--
2.42.0


























					Reply via email to