diff mbox series

[v6,1/7] drm: atmel-hlcdc: add flag and driver ops to differentiate XLCDC and HLCDC IP

Message ID 20230927094732.490228-2-manikandan.m@microchip.com
State New
Headers
Series Add support for XLCDC to sam9x7 SoC family. |

Commit Message

Manikandan Muralidharan Sept. 27, 2023, 9:47 a.m. UTC 
  Add is_xlcdc flag and LCD IP specific ops in driver data to differentiate
XLCDC and HLCDC code within the atmel-hlcdc driver files.

Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 68 ++++++++++++++++++++
 1 file changed, 68 insertions(+)

Comments

claudiu beznea Sept. 28, 2023, 6:01 a.m. UTC | #1 
Hi, Manikandan,

On 27.09.2023 12:47, Manikandan Muralidharan wrote:
> +void atmel_hlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
> +				    struct atmel_hlcdc_plane_state *state);
> +void atmel_xlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
> +				    struct atmel_hlcdc_plane_state *state);
> +void update_hlcdc_buffers(struct atmel_hlcdc_plane *plane,
> +			  struct atmel_hlcdc_plane_state *state,
> +			  u32 sr, int i);
> +void update_xlcdc_buffers(struct atmel_hlcdc_plane *plane,
> +			  struct atmel_hlcdc_plane_state *state,
> +			  u32 sr, int i);
> +void hlcdc_atomic_disable(struct atmel_hlcdc_plane *plane);
> +void xlcdc_atomic_disable(struct atmel_hlcdc_plane *plane);
> +void
> +atmel_hlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
> +					  struct atmel_hlcdc_plane_state *state);
> +void
> +atmel_xlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
> +					  struct atmel_hlcdc_plane_state *state);
> +void hlcdc_atomic_update(struct atmel_hlcdc_plane *plane,
> +			 struct atmel_hlcdc_dc *dc);
> +void xlcdc_atomic_update(struct atmel_hlcdc_plane *plane,
> +			 struct atmel_hlcdc_dc *dc);
> +void hlcdc_csc_init(struct atmel_hlcdc_plane *plane,
> +		    const struct atmel_hlcdc_layer_desc *desc);
> +void xlcdc_csc_init(struct atmel_hlcdc_plane *plane,
> +		    const struct atmel_hlcdc_layer_desc *desc);
> +void hlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
> +		   const struct atmel_hlcdc_layer_desc *desc);
> +void xlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
> +		   const struct atmel_hlcdc_layer_desc *desc);
> +

These are still here... Isn't the solution I proposed to you in the
previous version good enough?

Thank you,
Claudiu Beznea
Manikandan Muralidharan Oct. 3, 2023, 4:18 a.m. UTC | #2 
On 28/09/23 11:31 am, claudiu beznea wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi, Manikandan,
> 
> On 27.09.2023 12:47, Manikandan Muralidharan wrote:
>> +void atmel_hlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
>> +                                 struct atmel_hlcdc_plane_state *state);
>> +void atmel_xlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
>> +                                 struct atmel_hlcdc_plane_state *state);
>> +void update_hlcdc_buffers(struct atmel_hlcdc_plane *plane,
>> +                       struct atmel_hlcdc_plane_state *state,
>> +                       u32 sr, int i);
>> +void update_xlcdc_buffers(struct atmel_hlcdc_plane *plane,
>> +                       struct atmel_hlcdc_plane_state *state,
>> +                       u32 sr, int i);
>> +void hlcdc_atomic_disable(struct atmel_hlcdc_plane *plane);
>> +void xlcdc_atomic_disable(struct atmel_hlcdc_plane *plane);
>> +void
>> +atmel_hlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
>> +                                       struct atmel_hlcdc_plane_state *state);
>> +void
>> +atmel_xlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
>> +                                       struct atmel_hlcdc_plane_state *state);
>> +void hlcdc_atomic_update(struct atmel_hlcdc_plane *plane,
>> +                      struct atmel_hlcdc_dc *dc);
>> +void xlcdc_atomic_update(struct atmel_hlcdc_plane *plane,
>> +                      struct atmel_hlcdc_dc *dc);
>> +void hlcdc_csc_init(struct atmel_hlcdc_plane *plane,
>> +                 const struct atmel_hlcdc_layer_desc *desc);
>> +void xlcdc_csc_init(struct atmel_hlcdc_plane *plane,
>> +                 const struct atmel_hlcdc_layer_desc *desc);
>> +void hlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
>> +                const struct atmel_hlcdc_layer_desc *desc);
>> +void xlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
>> +                const struct atmel_hlcdc_layer_desc *desc);
>> +
> 
> These are still here... Isn't the solution I proposed to you in the
> previous version good enough?
Hi Claudiu

These changes were integrated in the current patch set based on the 
solution which you proposed in the previous series.
The XLCDC and HLCDC functions calls are moved to IP specific driver->ops
and their function declarations are made here in atmel_hlcdc_dc.h
Rest of the changes are integrated in Patch 4/7.
> 
> Thank you,
> Claudiu Beznea

-- 
Thanks and Regards,
Manikandan M.
claudiu beznea Oct. 3, 2023, 12:16 p.m. UTC | #3 
On 03.10.2023 07:18, Manikandan.M@microchip.com wrote:
> On 28/09/23 11:31 am, claudiu beznea wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> Hi, Manikandan,
>>
>> On 27.09.2023 12:47, Manikandan Muralidharan wrote:
>>> +void atmel_hlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
>>> +                                 struct atmel_hlcdc_plane_state *state);
>>> +void atmel_xlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
>>> +                                 struct atmel_hlcdc_plane_state *state);
>>> +void update_hlcdc_buffers(struct atmel_hlcdc_plane *plane,
>>> +                       struct atmel_hlcdc_plane_state *state,
>>> +                       u32 sr, int i);
>>> +void update_xlcdc_buffers(struct atmel_hlcdc_plane *plane,
>>> +                       struct atmel_hlcdc_plane_state *state,
>>> +                       u32 sr, int i);
>>> +void hlcdc_atomic_disable(struct atmel_hlcdc_plane *plane);
>>> +void xlcdc_atomic_disable(struct atmel_hlcdc_plane *plane);
>>> +void
>>> +atmel_hlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
>>> +                                       struct atmel_hlcdc_plane_state *state);
>>> +void
>>> +atmel_xlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
>>> +                                       struct atmel_hlcdc_plane_state *state);
>>> +void hlcdc_atomic_update(struct atmel_hlcdc_plane *plane,
>>> +                      struct atmel_hlcdc_dc *dc);
>>> +void xlcdc_atomic_update(struct atmel_hlcdc_plane *plane,
>>> +                      struct atmel_hlcdc_dc *dc);
>>> +void hlcdc_csc_init(struct atmel_hlcdc_plane *plane,
>>> +                 const struct atmel_hlcdc_layer_desc *desc);
>>> +void xlcdc_csc_init(struct atmel_hlcdc_plane *plane,
>>> +                 const struct atmel_hlcdc_layer_desc *desc);
>>> +void hlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
>>> +                const struct atmel_hlcdc_layer_desc *desc);
>>> +void xlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
>>> +                const struct atmel_hlcdc_layer_desc *desc);
>>> +
>>
>> These are still here... Isn't the solution I proposed to you in the
>> previous version good enough?
> Hi Claudiu
> 
> These changes were integrated in the current patch set based on the 
> solution which you proposed in the previous series.
> The XLCDC and HLCDC functions calls are moved to IP specific driver->ops
> and their function declarations are made here in atmel_hlcdc_dc.h
> Rest of the changes are integrated in Patch 4/7.

I still think (and I've checked it last time) you can remove these
declaration. See comment from previous version:

"You can get rid of these and keep the function definitions static to
atmel_hlcdc_plane.c if you define struct atmel_lcdc_dc_ops objects directly
to atmel_hlcdc_plane.c. In atmel_hlcdc_dc.c you can have something like:

extern const struct atmel_lcdc_dc_ops  atmel_hlcdc_ops;
extern const struct atmel_lcdc_dc_ops  atmel_xlcdc_ops;
"

>>
>> Thank you,
>> Claudiu Beznea
>
Manikandan Muralidharan Oct. 5, 2023, 6:55 a.m. UTC | #4 
On 03/10/23 5:46 pm, claudiu beznea wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 03.10.2023 07:18, Manikandan.M@microchip.com wrote:
>> On 28/09/23 11:31 am, claudiu beznea wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> Hi, Manikandan,
>>>
>>> On 27.09.2023 12:47, Manikandan Muralidharan wrote:
>>>> +void atmel_hlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
>>>> +                                 struct atmel_hlcdc_plane_state *state);
>>>> +void atmel_xlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
>>>> +                                 struct atmel_hlcdc_plane_state *state);
>>>> +void update_hlcdc_buffers(struct atmel_hlcdc_plane *plane,
>>>> +                       struct atmel_hlcdc_plane_state *state,
>>>> +                       u32 sr, int i);
>>>> +void update_xlcdc_buffers(struct atmel_hlcdc_plane *plane,
>>>> +                       struct atmel_hlcdc_plane_state *state,
>>>> +                       u32 sr, int i);
>>>> +void hlcdc_atomic_disable(struct atmel_hlcdc_plane *plane);
>>>> +void xlcdc_atomic_disable(struct atmel_hlcdc_plane *plane);
>>>> +void
>>>> +atmel_hlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
>>>> +                                       struct atmel_hlcdc_plane_state *state);
>>>> +void
>>>> +atmel_xlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
>>>> +                                       struct atmel_hlcdc_plane_state *state);
>>>> +void hlcdc_atomic_update(struct atmel_hlcdc_plane *plane,
>>>> +                      struct atmel_hlcdc_dc *dc);
>>>> +void xlcdc_atomic_update(struct atmel_hlcdc_plane *plane,
>>>> +                      struct atmel_hlcdc_dc *dc);
>>>> +void hlcdc_csc_init(struct atmel_hlcdc_plane *plane,
>>>> +                 const struct atmel_hlcdc_layer_desc *desc);
>>>> +void xlcdc_csc_init(struct atmel_hlcdc_plane *plane,
>>>> +                 const struct atmel_hlcdc_layer_desc *desc);
>>>> +void hlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
>>>> +                const struct atmel_hlcdc_layer_desc *desc);
>>>> +void xlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
>>>> +                const struct atmel_hlcdc_layer_desc *desc);
>>>> +
>>>
>>> These are still here... Isn't the solution I proposed to you in the
>>> previous version good enough?
>> Hi Claudiu
>>
>> These changes were integrated in the current patch set based on the
>> solution which you proposed in the previous series.
>> The XLCDC and HLCDC functions calls are moved to IP specific driver->ops
>> and their function declarations are made here in atmel_hlcdc_dc.h
>> Rest of the changes are integrated in Patch 4/7.
> 
> I still think (and I've checked it last time) you can remove these
> declaration. See comment from previous version:
> 
> "You can get rid of these and keep the function definitions static to
> atmel_hlcdc_plane.c if you define struct atmel_lcdc_dc_ops objects directly
> to atmel_hlcdc_plane.c. In atmel_hlcdc_dc.c you can have something like:
> 
> extern const struct atmel_lcdc_dc_ops  atmel_hlcdc_ops;
> extern const struct atmel_lcdc_dc_ops  atmel_xlcdc_ops;
> "
Hi Claudiu

Thank you. I will integrate the changes in the next version.
> 
>>>
>>> Thank you,
>>> Claudiu Beznea
>>

-- 
Thanks and Regards,
Manikandan M.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
index 5b5c774e0edf..c61fa1733da4 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
@@ -177,6 +177,9 @@  struct atmel_hlcdc_layer_cfg_layout {
 	int csc;
 };
 
+struct atmel_hlcdc_plane_state;
+struct atmel_lcdc_dc_ops;
+
 /**
  * Atmel HLCDC DMA descriptor structure
  *
@@ -304,8 +307,10 @@  atmel_hlcdc_layer_to_plane(struct atmel_hlcdc_layer *layer)
  * @conflicting_output_formats: true if RGBXXX output formats conflict with
  *				each other.
  * @fixed_clksrc: true if clock source is fixed
+ * @is_xlcdc: true if XLCDC IP is supported
  * @layers: a layer description table describing available layers
  * @nlayers: layer description table size
+ * @ops: atmel lcdc dc ops
  */
 struct atmel_hlcdc_dc_desc {
 	int min_width;
@@ -317,8 +322,10 @@  struct atmel_hlcdc_dc_desc {
 	int max_hpw;
 	bool conflicting_output_formats;
 	bool fixed_clksrc;
+	bool is_xlcdc;
 	const struct atmel_hlcdc_layer_desc *layers;
 	int nlayers;
+	const struct atmel_lcdc_dc_ops *ops;
 };
 
 /**
@@ -345,6 +352,67 @@  struct atmel_hlcdc_dc {
 	} suspend;
 };
 
+/**
+ * struct atmel_lcdc_dc_ops - describes atmel_lcdc ops group
+ * to differentiate HLCDC and XLCDC IP code support.
+ * @plane_setup_scaler: update the vertical and horizontal scaling factors
+ * @update_lcdc_buffers: update the each LCDC layers DMA registers.
+ * @lcdc_atomic_disable: disable LCDC interrupts and layers
+ * @lcdc_update_general_settings: update each LCDC layers general
+ * confiugration register.
+ * @lcdc_atomic_update: enable the LCDC layers and interrupts.
+ * @lcdc_csc_init: update the color space conversion co-efficient of
+ * High-end overlay register.
+ * @lcdc_irq_dbg: to raise alert incase of interrupt overrun in any LCDC layer.
+ */
+struct atmel_lcdc_dc_ops {
+	void (*plane_setup_scaler)(struct atmel_hlcdc_plane *plane,
+				   struct atmel_hlcdc_plane_state *state);
+	void (*update_lcdc_buffers)(struct atmel_hlcdc_plane *plane,
+				    struct atmel_hlcdc_plane_state *state,
+				    u32 sr, int i);
+	void (*lcdc_atomic_disable)(struct atmel_hlcdc_plane *plane);
+	void (*lcdc_update_general_settings)(struct atmel_hlcdc_plane *plane,
+					     struct atmel_hlcdc_plane_state *state);
+	void (*lcdc_atomic_update)(struct atmel_hlcdc_plane *plane,
+				   struct atmel_hlcdc_dc *dc);
+	void (*lcdc_csc_init)(struct atmel_hlcdc_plane *plane,
+			      const struct atmel_hlcdc_layer_desc *desc);
+	void (*lcdc_irq_dbg)(struct atmel_hlcdc_plane *plane,
+			     const struct atmel_hlcdc_layer_desc *desc);
+};
+
+void atmel_hlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
+				    struct atmel_hlcdc_plane_state *state);
+void atmel_xlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
+				    struct atmel_hlcdc_plane_state *state);
+void update_hlcdc_buffers(struct atmel_hlcdc_plane *plane,
+			  struct atmel_hlcdc_plane_state *state,
+			  u32 sr, int i);
+void update_xlcdc_buffers(struct atmel_hlcdc_plane *plane,
+			  struct atmel_hlcdc_plane_state *state,
+			  u32 sr, int i);
+void hlcdc_atomic_disable(struct atmel_hlcdc_plane *plane);
+void xlcdc_atomic_disable(struct atmel_hlcdc_plane *plane);
+void
+atmel_hlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
+					  struct atmel_hlcdc_plane_state *state);
+void
+atmel_xlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
+					  struct atmel_hlcdc_plane_state *state);
+void hlcdc_atomic_update(struct atmel_hlcdc_plane *plane,
+			 struct atmel_hlcdc_dc *dc);
+void xlcdc_atomic_update(struct atmel_hlcdc_plane *plane,
+			 struct atmel_hlcdc_dc *dc);
+void hlcdc_csc_init(struct atmel_hlcdc_plane *plane,
+		    const struct atmel_hlcdc_layer_desc *desc);
+void xlcdc_csc_init(struct atmel_hlcdc_plane *plane,
+		    const struct atmel_hlcdc_layer_desc *desc);
+void hlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
+		   const struct atmel_hlcdc_layer_desc *desc);
+void xlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
+		   const struct atmel_hlcdc_layer_desc *desc);
+
 extern struct atmel_hlcdc_formats atmel_hlcdc_plane_rgb_formats;
 extern struct atmel_hlcdc_formats atmel_hlcdc_plane_rgb_and_yuv_formats;