[PATCH 0/9] Add support for XLCDC to sam9x7 SoC family.

[PATCH 0/9] Add support for XLCDC to sam9x7 SoC family.

[Manikandan Muralidharan]

From: Manikandan <manikandan.m@microchip.com>

This patch series aims to add support for XLCDC IP of sam9x7 SoC family
to the DRM subsystem.XLCDC IP has additional registers and new
configuration bits compared to the existing register set of HLCDC IP.
The compatible string "microchip,sam9x7-xlcdc", defined for sam9x7 SoC
family helps to differentiate the XLCDC and existing HLCDC code
within the same driver.

Durai Manickam KR (2):
  drm: atmel-hlcdc: Define SAM9X7 XLCDC specific registers
  drm: atmel-hlcdc: add compatible string check for XLCDC and HLCDC

Manikandan Muralidharan (7):
  dt-bindings: mfd: Add bindings for SAM9X7 LCD controller
  mfd: atmel-hlcdc: Add compatible for SAM9X7 HLCD controller
  drm: atmel-hlcdc: add LCD controller layer definition for SAM9X7
  drm: atmel_hlcdc: Add support for XLCDC in atmel LCD driver
  drm: atmel-hlcdc: add DPI mode support for XLCDC
  drm: atmel-hlcdc: add vertical and horizontal scaling support for
    XLCDC
  drm: atmel-hlcdc: add support for DSI output formats

 .../devicetree/bindings/mfd/atmel-hlcdc.txt   |   1 +
 .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    | 167 ++++++++--
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c  | 105 ++++++
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h  | 113 +++++++
 .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   | 315 ++++++++++++++----
 drivers/mfd/atmel-hlcdc.c                     |   1 +
 include/linux/mfd/atmel-hlcdc.h               |  10 +
 7 files changed, 609 insertions(+), 103 deletions(-)

-- 
2.25.1

[PATCH 1/9] dt-bindings: mfd: Add bindings for SAM9X7 LCD controller

[Manikandan Muralidharan]

Add new compatible string for the XLCD controller on SAM9X7 SoC.

Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
---
 Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
index 5f8880cc757e..7c77b6bf4adb 100644
--- a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
+++ b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
@@ -8,6 +8,7 @@ Required properties:
    "atmel,sama5d3-hlcdc"
    "atmel,sama5d4-hlcdc"
    "microchip,sam9x60-hlcdc"
+   "microchip,sam9x7-xlcdc"
  - reg: base address and size of the HLCDC device registers.
  - clock-names: the name of the 3 clocks requested by the HLCDC device.
    Should contain "periph_clk", "sys_clk" and "slow_clk".
-- 
2.25.1

Re: [PATCH 1/9] dt-bindings: mfd: Add bindings for SAM9X7 LCD controller

[Krzysztof Kozlowski]

On 13/06/2023 09:04, Manikandan Muralidharan wrote:
> Add new compatible string for the XLCD controller on SAM9X7 SoC.
> 
> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> ---
>  Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
> index 5f8880cc757e..7c77b6bf4adb 100644
> --- a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
> +++ b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
> @@ -8,6 +8,7 @@ Required properties:
>     "atmel,sama5d3-hlcdc"
>     "atmel,sama5d4-hlcdc"
>     "microchip,sam9x60-hlcdc"
> +   "microchip,sam9x7-xlcdc"

Google says sam9x7 is a series, not a SoC. Please add compatibles for
specific SoCs, not for series.

Best regards,
Krzysztof

Re: [PATCH 1/9] dt-bindings: mfd: Add bindings for SAM9X7 LCD controller

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1177 bytes --]

On Tue, Jun 13, 2023 at 08:17:13PM +0200, Krzysztof Kozlowski wrote:
> On 13/06/2023 09:04, Manikandan Muralidharan wrote:
> > Add new compatible string for the XLCD controller on SAM9X7 SoC.
> > 
> > Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> > ---
> >  Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
> > index 5f8880cc757e..7c77b6bf4adb 100644
> > --- a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
> > +++ b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
> > @@ -8,6 +8,7 @@ Required properties:
> >     "atmel,sama5d3-hlcdc"
> >     "atmel,sama5d4-hlcdc"
> >     "microchip,sam9x60-hlcdc"
> > +   "microchip,sam9x7-xlcdc"
> 
> Google says sam9x7 is a series, not a SoC. Please add compatibles for
> specific SoCs, not for series.

We had this one a few weeks ago, see
https://lore.kernel.org/all/add5e49e-8416-ba9f-819a-da944938c05f@microchip.com/
and its parents. Outcome of that seemed to be that using "sam9x7" was fine.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/9] dt-bindings: mfd: Add bindings for SAM9X7 LCD controller

[Nicolas Ferre]

On 13/06/2023 at 20:21, Conor Dooley wrote:
> On Tue, Jun 13, 2023 at 08:17:13PM +0200, Krzysztof Kozlowski wrote:
>> On 13/06/2023 09:04, Manikandan Muralidharan wrote:
>>> Add new compatible string for the XLCD controller on SAM9X7 SoC.
>>>
>>> Signed-off-by: Manikandan Muralidharan<manikandan.m@microchip.com>
>>> ---
>>>   Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
>>> index 5f8880cc757e..7c77b6bf4adb 100644
>>> --- a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
>>> +++ b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
>>> @@ -8,6 +8,7 @@ Required properties:
>>>      "atmel,sama5d3-hlcdc"
>>>      "atmel,sama5d4-hlcdc"
>>>      "microchip,sam9x60-hlcdc"
>>> +   "microchip,sam9x7-xlcdc"
>> Google says sam9x7 is a series, not a SoC. Please add compatibles for
>> specific SoCs, not for series.
> We had this one a few weeks ago, see
> https://lore.kernel.org/all/add5e49e-8416-ba9f-819a-da944938c05f@microchip.com/
> and its parents. Outcome of that seemed to be that using "sam9x7" was fine.

And it's where it begins to be funny, as the LCD is precisely one aspect 
where we differentiate between sam9x75, sam9x72 and sam9x70...
So please Manikandan sort this out if difference between these chips 
will be better handled with different compatibility string, in 
particular about //, LVDS and MIPI-DSI variants!

Regards,
   Nicolas

-- 
Nicolas Ferre

Re: [PATCH 1/9] dt-bindings: mfd: Add bindings for SAM9X7 LCD controller

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2006 bytes --]

On Wed, Jun 14, 2023 at 04:40:50PM +0200, Nicolas Ferre wrote:
> On 13/06/2023 at 20:21, Conor Dooley wrote:
> > On Tue, Jun 13, 2023 at 08:17:13PM +0200, Krzysztof Kozlowski wrote:
> > > On 13/06/2023 09:04, Manikandan Muralidharan wrote:
> > > > Add new compatible string for the XLCD controller on SAM9X7 SoC.
> > > > 
> > > > Signed-off-by: Manikandan Muralidharan<manikandan.m@microchip.com>
> > > > ---
> > > >   Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt | 1 +
> > > >   1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
> > > > index 5f8880cc757e..7c77b6bf4adb 100644
> > > > --- a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
> > > > +++ b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
> > > > @@ -8,6 +8,7 @@ Required properties:
> > > >      "atmel,sama5d3-hlcdc"
> > > >      "atmel,sama5d4-hlcdc"
> > > >      "microchip,sam9x60-hlcdc"
> > > > +   "microchip,sam9x7-xlcdc"
> > > Google says sam9x7 is a series, not a SoC. Please add compatibles for
> > > specific SoCs, not for series.
> > We had this one a few weeks ago, see
> > https://lore.kernel.org/all/add5e49e-8416-ba9f-819a-da944938c05f@microchip.com/
> > and its parents. Outcome of that seemed to be that using "sam9x7" was fine.
> 
> And it's where it begins to be funny, as the LCD is precisely one aspect
> where we differentiate between sam9x75, sam9x72 and sam9x70...

Oh dear, just my luck...

> So please Manikandan sort this out if difference between these chips will be
> better handled with different compatibility string, in particular about //,
> LVDS and MIPI-DSI variants!

Yeah, providing some information about what the differences actually are
would be good, for the same of the actually-knowledgeable-about-displays
folk in the audience (IOW, not me).
Probably then the display/atmel/hlcdc-dc.txt binding needs an update too?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/9] dt-bindings: mfd: Add bindings for SAM9X7 LCD controller

[Manikandan.M]

On 14/06/23 20:10, Nicolas Ferre wrote:
> On 13/06/2023 at 20:21, Conor Dooley wrote:
>> On Tue, Jun 13, 2023 at 08:17:13PM +0200, Krzysztof Kozlowski wrote:
>>> On 13/06/2023 09:04, Manikandan Muralidharan wrote:
>>>> Add new compatible string for the XLCD controller on SAM9X7 SoC.
>>>>
>>>> Signed-off-by: Manikandan Muralidharan<manikandan.m@microchip.com>
>>>> ---
>>>>   Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt | 1 +
>>>>   1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt 
>>>> b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
>>>> index 5f8880cc757e..7c77b6bf4adb 100644
>>>> --- a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
>>>> +++ b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
>>>> @@ -8,6 +8,7 @@ Required properties:
>>>>      "atmel,sama5d3-hlcdc"
>>>>      "atmel,sama5d4-hlcdc"
>>>>      "microchip,sam9x60-hlcdc"
>>>> +   "microchip,sam9x7-xlcdc"
>>> Google says sam9x7 is a series, not a SoC. Please add compatibles for
>>> specific SoCs, not for series.
>> We had this one a few weeks ago, see
>> https://lore.kernel.org/all/add5e49e-8416-ba9f-819a-da944938c05f@microchip.com/
>> and its parents. Outcome of that seemed to be that using "sam9x7" was 
>> fine.
> 
> And it's where it begins to be funny, as the LCD is precisely one aspect 
> where we differentiate between sam9x75, sam9x72 and sam9x70...
> So please Manikandan sort this out if difference between these chips 
> will be better handled with different compatibility string, in 
> particular about //, LVDS and MIPI-DSI variants!
Yes Sure, I will replace the compatible as s/sam9x7/sam9x75/g to handle 
the different variants of sam9x7 better.
> 
> Regards,
>    Nicolas
> 

-- 
Thanks and Regards,
Manikandan M.

Re: [PATCH 1/9] dt-bindings: mfd: Add bindings for SAM9X7 LCD controller

[Nicolas Ferre]

On 16/06/2023 at 08:44, Manikandan M - I67131 wrote:
> On 14/06/23 20:10, Nicolas Ferre wrote:
>> On 13/06/2023 at 20:21, Conor Dooley wrote:
>>> On Tue, Jun 13, 2023 at 08:17:13PM +0200, Krzysztof Kozlowski wrote:
>>>> On 13/06/2023 09:04, Manikandan Muralidharan wrote:
>>>>> Add new compatible string for the XLCD controller on SAM9X7 SoC.
>>>>>
>>>>> Signed-off-by: Manikandan Muralidharan<manikandan.m@microchip.com>
>>>>> ---
>>>>>    Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt | 1 +
>>>>>    1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
>>>>> b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
>>>>> index 5f8880cc757e..7c77b6bf4adb 100644
>>>>> --- a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
>>>>> +++ b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
>>>>> @@ -8,6 +8,7 @@ Required properties:
>>>>>       "atmel,sama5d3-hlcdc"
>>>>>       "atmel,sama5d4-hlcdc"
>>>>>       "microchip,sam9x60-hlcdc"
>>>>> +   "microchip,sam9x7-xlcdc"
>>>> Google says sam9x7 is a series, not a SoC. Please add compatibles for
>>>> specific SoCs, not for series.
>>> We had this one a few weeks ago, see
>>> https://lore.kernel.org/all/add5e49e-8416-ba9f-819a-da944938c05f@microchip.com/
>>> and its parents. Outcome of that seemed to be that using "sam9x7" was
>>> fine.
>>
>> And it's where it begins to be funny, as the LCD is precisely one aspect
>> where we differentiate between sam9x75, sam9x72 and sam9x70...
>> So please Manikandan sort this out if difference between these chips
>> will be better handled with different compatibility string, in
>> particular about //, LVDS and MIPI-DSI variants!
> Yes Sure, I will replace the compatible as s/sam9x7/sam9x75/g to handle
> the different variants of sam9x7 better.

Moving to sam9x75 is probably good. But what is your plan for 
differentiating parallel and LVDS (on sam9x72) and precisely this 
sam9x75 variant which in addition has MIPI-DSI?

Regards,
  Nicolas


-- 
Nicolas Ferre

Re: [PATCH 1/9] dt-bindings: mfd: Add bindings for SAM9X7 LCD controller

[Manikandan.M]

On 21/06/23 13:17, Nicolas Ferre wrote:
> On 16/06/2023 at 08:44, Manikandan M - I67131 wrote:
>> On 14/06/23 20:10, Nicolas Ferre wrote:
>>> On 13/06/2023 at 20:21, Conor Dooley wrote:
>>>> On Tue, Jun 13, 2023 at 08:17:13PM +0200, Krzysztof Kozlowski wrote:
>>>>> On 13/06/2023 09:04, Manikandan Muralidharan wrote:
>>>>>> Add new compatible string for the XLCD controller on SAM9X7 SoC.
>>>>>>
>>>>>> Signed-off-by: Manikandan Muralidharan<manikandan.m@microchip.com>
>>>>>> ---
>>>>>>    Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt | 1 +
>>>>>>    1 file changed, 1 insertion(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
>>>>>> b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
>>>>>> index 5f8880cc757e..7c77b6bf4adb 100644
>>>>>> --- a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
>>>>>> +++ b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
>>>>>> @@ -8,6 +8,7 @@ Required properties:
>>>>>>       "atmel,sama5d3-hlcdc"
>>>>>>       "atmel,sama5d4-hlcdc"
>>>>>>       "microchip,sam9x60-hlcdc"
>>>>>> +   "microchip,sam9x7-xlcdc"
>>>>> Google says sam9x7 is a series, not a SoC. Please add compatibles for
>>>>> specific SoCs, not for series.
>>>> We had this one a few weeks ago, see
>>>> https://lore.kernel.org/all/add5e49e-8416-ba9f-819a-da944938c05f@microchip.com/
>>>> and its parents. Outcome of that seemed to be that using "sam9x7" was
>>>> fine.
>>>
>>> And it's where it begins to be funny, as the LCD is precisely one aspect
>>> where we differentiate between sam9x75, sam9x72 and sam9x70...
>>> So please Manikandan sort this out if difference between these chips
>>> will be better handled with different compatibility string, in
>>> particular about //, LVDS and MIPI-DSI variants!
>> Yes Sure, I will replace the compatible as s/sam9x7/sam9x75/g to handle
>> the different variants of sam9x7 better.
> 
> Moving to sam9x75 is probably good. But what is your plan for 
> differentiating parallel and LVDS (on sam9x72) and precisely this 
> sam9x75 variant which in addition has MIPI-DSI?
In sam9x75 with support for LVDS and MIPI, Parallel interfacing 
peripherals, the additions performed on the LCD controller driver is the 
same.Considering the LCDC IP used in sam9x75, there are no registers 
sets that needs modification per connecting peripheral variants, only 
the clock and DRM_ENCODER_MODE_XXX (set by connecting peripheral driver) 
differs, which can be handled in DT, atmel-lcdc MFD driver and 
peripheral driver.

In future, sam9x72 with XLCD controller can be differentiated with 
sam9x72 compatible string and with a IP version flag(is_xlcdc_v2) in its 
driver data if an upgraded XLCD IP is used with difference in bits or 
register set exist compared to current IP.
> 
> Regards,
>   Nicolas
> 
> 

-- 
Thanks and Regards,
Manikandan M.

Re: [PATCH 1/9] dt-bindings: mfd: Add bindings for SAM9X7 LCD controller

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3595 bytes --]

On Mon, Jun 26, 2023 at 05:31:59AM +0000, Manikandan.M@microchip.com wrote:
> On 21/06/23 13:17, Nicolas Ferre wrote:
> > On 16/06/2023 at 08:44, Manikandan M - I67131 wrote:
> >> On 14/06/23 20:10, Nicolas Ferre wrote:
> >>> On 13/06/2023 at 20:21, Conor Dooley wrote:
> >>>> On Tue, Jun 13, 2023 at 08:17:13PM +0200, Krzysztof Kozlowski wrote:
> >>>>> On 13/06/2023 09:04, Manikandan Muralidharan wrote:
> >>>>>> Add new compatible string for the XLCD controller on SAM9X7 SoC.
> >>>>>>
> >>>>>> Signed-off-by: Manikandan Muralidharan<manikandan.m@microchip.com>
> >>>>>> ---
> >>>>>>    Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt | 1 +
> >>>>>>    1 file changed, 1 insertion(+)
> >>>>>>
> >>>>>> diff --git a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
> >>>>>> b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
> >>>>>> index 5f8880cc757e..7c77b6bf4adb 100644
> >>>>>> --- a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
> >>>>>> +++ b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
> >>>>>> @@ -8,6 +8,7 @@ Required properties:
> >>>>>>       "atmel,sama5d3-hlcdc"
> >>>>>>       "atmel,sama5d4-hlcdc"
> >>>>>>       "microchip,sam9x60-hlcdc"
> >>>>>> +   "microchip,sam9x7-xlcdc"
> >>>>> Google says sam9x7 is a series, not a SoC. Please add compatibles for
> >>>>> specific SoCs, not for series.
> >>>> We had this one a few weeks ago, see
> >>>> https://lore.kernel.org/all/add5e49e-8416-ba9f-819a-da944938c05f@microchip.com/
> >>>> and its parents. Outcome of that seemed to be that using "sam9x7" was
> >>>> fine.
> >>>
> >>> And it's where it begins to be funny, as the LCD is precisely one aspect
> >>> where we differentiate between sam9x75, sam9x72 and sam9x70...
> >>> So please Manikandan sort this out if difference between these chips
> >>> will be better handled with different compatibility string, in
> >>> particular about //, LVDS and MIPI-DSI variants!
> >> Yes Sure, I will replace the compatible as s/sam9x7/sam9x75/g to handle
> >> the different variants of sam9x7 better.
> > 
> > Moving to sam9x75 is probably good. But what is your plan for 
> > differentiating parallel and LVDS (on sam9x72) and precisely this 
> > sam9x75 variant which in addition has MIPI-DSI?
> In sam9x75 with support for LVDS and MIPI, Parallel interfacing 
> peripherals, the additions performed on the LCD controller driver is the 
> same.Considering the LCDC IP used in sam9x75, there are no registers 
> sets that needs modification per connecting peripheral variants, only 
> the clock and DRM_ENCODER_MODE_XXX (set by connecting peripheral driver) 
> differs, which can be handled in DT, atmel-lcdc MFD driver and 
> peripheral driver.
> 
> In future, sam9x72 with XLCD controller can be differentiated with 
> sam9x72 compatible string and with a IP version flag(is_xlcdc_v2) in its 
> driver data if an upgraded XLCD IP is used with difference in bits or 
> register set exist compared to current IP.

Trying to covert that into what the binding will look like...
sam9x72 & sam9x75 each get their own compatibles for the lcd controller.
From there, we permit `compatible = "microchip,sam9x75-foo"` in
isolation. It *sounds* like the basic featureset of the sam9x75 is
compatible with the sam9x72, so for that we permit
`compatible = "microchip,sam9x72-foo", "microchip,sam9x75-foo"`.
Although, if the hardware for the sam9x72 isn't set in stone yet, it
might be best to hold off on documenting it until things settle down,
and only add the sam9x75 for now.

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/9] dt-bindings: mfd: Add bindings for SAM9X7 LCD controller

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1269 bytes --]

On Tue, Jun 13, 2023 at 12:34:18PM +0530, Manikandan Muralidharan wrote:
> Add new compatible string for the XLCD controller on SAM9X7 SoC.

You should probably indicate here why this is not compatible with the
existing SoCs that are supported. To hazard a guess, it is the HLCDC IP
(I forget the exact letters!)?
If so,
Acked-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

> 
> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> ---
>  Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
> index 5f8880cc757e..7c77b6bf4adb 100644
> --- a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
> +++ b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
> @@ -8,6 +8,7 @@ Required properties:
>     "atmel,sama5d3-hlcdc"
>     "atmel,sama5d4-hlcdc"
>     "microchip,sam9x60-hlcdc"
> +   "microchip,sam9x7-xlcdc"
>   - reg: base address and size of the HLCDC device registers.
>   - clock-names: the name of the 3 clocks requested by the HLCDC device.
>     Should contain "periph_clk", "sys_clk" and "slow_clk".
> -- 
> 2.25.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/9] dt-bindings: mfd: Add bindings for SAM9X7 LCD controller

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1649 bytes --]

On Tue, Jun 13, 2023 at 07:18:25PM +0100, Conor Dooley wrote:
> On Tue, Jun 13, 2023 at 12:34:18PM +0530, Manikandan Muralidharan wrote:
> > Add new compatible string for the XLCD controller on SAM9X7 SoC.
> 
> You should probably indicate here why this is not compatible with the
> existing SoCs that are supported. To hazard a guess, it is the HLCDC IP
> (I forget the exact letters!)?

Manikandan pointed out off list that this was not clear.
Looking at it again, I think I actually truncated my sentence - it
should've been something like "it is the HLCDC IP ... is not a subset of
the XLCDC IP." Sorry for that Manikandan.

> If so,
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> 
> Cheers,
> Conor.
> 
> > 
> > Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> > ---
> >  Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
> > index 5f8880cc757e..7c77b6bf4adb 100644
> > --- a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
> > +++ b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
> > @@ -8,6 +8,7 @@ Required properties:
> >     "atmel,sama5d3-hlcdc"
> >     "atmel,sama5d4-hlcdc"
> >     "microchip,sam9x60-hlcdc"
> > +   "microchip,sam9x7-xlcdc"
> >   - reg: base address and size of the HLCDC device registers.
> >   - clock-names: the name of the 3 clocks requested by the HLCDC device.
> >     Should contain "periph_clk", "sys_clk" and "slow_clk".
> > -- 
> > 2.25.1
> > 



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 2/9] mfd: atmel-hlcdc: Add compatible for SAM9X7 HLCD controller

[Manikandan Muralidharan]

Add compatible for SAM9X7 HLCD controller.

Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
---
 drivers/mfd/atmel-hlcdc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mfd/atmel-hlcdc.c b/drivers/mfd/atmel-hlcdc.c
index 3c2414ba4b01..8755c91ce854 100644
--- a/drivers/mfd/atmel-hlcdc.c
+++ b/drivers/mfd/atmel-hlcdc.c
@@ -141,6 +141,7 @@ static const struct of_device_id atmel_hlcdc_match[] = {
 	{ .compatible = "atmel,sama5d3-hlcdc" },
 	{ .compatible = "atmel,sama5d4-hlcdc" },
 	{ .compatible = "microchip,sam9x60-hlcdc" },
+	{ .compatible = "microchip,sam9x7-xlcdc" },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, atmel_hlcdc_match);
-- 
2.25.1

Re: [PATCH 2/9] mfd: atmel-hlcdc: Add compatible for SAM9X7 HLCD controller

[Lee Jones]

On Tue, 13 Jun 2023, Manikandan Muralidharan wrote:

> Add compatible for SAM9X7 HLCD controller.
> 
> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> ---
>  drivers/mfd/atmel-hlcdc.c | 1 +
>  1 file changed, 1 insertion(+)

Applied, thanks

-- 
Lee Jones [李琼斯]

Re: [PATCH 2/9] mfd: atmel-hlcdc: Add compatible for SAM9X7 HLCD controller

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 766 bytes --]

On Wed, Jun 21, 2023 at 06:56:45PM +0100, Lee Jones wrote:
> On Tue, 13 Jun 2023, Manikandan Muralidharan wrote:
> 
> > Add compatible for SAM9X7 HLCD controller.
> > 
> > Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> > ---
> >  drivers/mfd/atmel-hlcdc.c | 1 +
> >  1 file changed, 1 insertion(+)
> 
> Applied, thanks

Hmm, Nicolas pointed out that this compatible is likely insufficient,
and it'll likely need to be sam9x70 & sam9x75 as there are differences
between what each of these SoCs support.
https://lore.kernel.org/all/ef09246c-9220-4c71-4ac2-2792d9ca519d@microchip.com/
I guess it doesn't really matter, since the binding didn't get applied
and what's in the driver can be arbitrarily changed?

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 2/9] mfd: atmel-hlcdc: Add compatible for SAM9X7 HLCD controller

[Lee Jones]

On Wed, 21 Jun 2023, Conor Dooley wrote:

> On Wed, Jun 21, 2023 at 06:56:45PM +0100, Lee Jones wrote:
> > On Tue, 13 Jun 2023, Manikandan Muralidharan wrote:
> > 
> > > Add compatible for SAM9X7 HLCD controller.
> > > 
> > > Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> > > ---
> > >  drivers/mfd/atmel-hlcdc.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > 
> > Applied, thanks
> 
> Hmm, Nicolas pointed out that this compatible is likely insufficient,
> and it'll likely need to be sam9x70 & sam9x75 as there are differences
> between what each of these SoCs support.
> https://lore.kernel.org/all/ef09246c-9220-4c71-4ac2-2792d9ca519d@microchip.com/
> I guess it doesn't really matter, since the binding didn't get applied
> and what's in the driver can be arbitrarily changed?

Unapplied, thanks. :)

-- 
Lee Jones [李琼斯]

[PATCH 3/9] drm: atmel-hlcdc: add LCD controller layer definition for SAM9X7

[Manikandan Muralidharan]

Add the LCD controller layer definition and descriptor structure for SAM9X7
for the following layers,
- Base Layer
- Overlay1 Layer
- Overlay2 Layer
- High End Overlay

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

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index fa0f9a93d50d..d7ad828e9e8c 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -462,6 +462,98 @@ static const struct atmel_hlcdc_dc_desc atmel_hlcdc_dc_sam9x60 = {
 	.layers = atmel_hlcdc_sam9x60_layers,
 };
 
+static const struct atmel_hlcdc_layer_desc atmel_xlcdc_sam9x7_layers[] = {
+	{
+		.name = "base",
+		.formats = &atmel_hlcdc_plane_rgb_formats,
+		.regs_offset = 0x60,
+		.id = 0,
+		.type = ATMEL_HLCDC_BASE_LAYER,
+		.cfgs_offset = 0x1c,
+		.layout = {
+			.xstride = { 2 },
+			.default_color = 3,
+			.general_config = 4,
+			.disc_pos = 5,
+			.disc_size = 6,
+		},
+		.clut_offset = 0x700,
+	},
+	{
+		.name = "overlay1",
+		.formats = &atmel_hlcdc_plane_rgb_formats,
+		.regs_offset = 0x160,
+		.id = 1,
+		.type = ATMEL_HLCDC_OVERLAY_LAYER,
+		.cfgs_offset = 0x1c,
+		.layout = {
+			.pos = 2,
+			.size = 3,
+			.xstride = { 4 },
+			.pstride = { 5 },
+			.default_color = 6,
+			.chroma_key = 7,
+			.chroma_key_mask = 8,
+			.general_config = 9,
+		},
+		.clut_offset = 0xb00,
+	},
+	{
+		.name = "overlay2",
+		.formats = &atmel_hlcdc_plane_rgb_formats,
+		.regs_offset = 0x260,
+		.id = 2,
+		.type = ATMEL_HLCDC_OVERLAY_LAYER,
+		.cfgs_offset = 0x1c,
+		.layout = {
+			.pos = 2,
+			.size = 3,
+			.xstride = { 4 },
+			.pstride = { 5 },
+			.default_color = 6,
+			.chroma_key = 7,
+			.chroma_key_mask = 8,
+			.general_config = 9,
+		},
+		.clut_offset = 0xf00,
+	},
+	{
+		.name = "high-end-overlay",
+		.formats = &atmel_hlcdc_plane_rgb_and_yuv_formats,
+		.regs_offset = 0x360,
+		.id = 3,
+		.type = ATMEL_HLCDC_OVERLAY_LAYER,
+		.cfgs_offset = 0x30,
+		.layout = {
+			.pos = 2,
+			.size = 3,
+			.memsize = 4,
+			.xstride = { 5, 7 },
+			.pstride = { 6, 8 },
+			.default_color = 9,
+			.chroma_key = 10,
+			.chroma_key_mask = 11,
+			.general_config = 12,
+			.csc = 16,
+			.scaler_config = 23,
+		},
+		.clut_offset = 0x1300,
+	},
+};
+
+static const struct atmel_hlcdc_dc_desc atmel_xlcdc_dc_sam9x7 = {
+	.min_width = 0,
+	.min_height = 0,
+	.max_width = 2048,
+	.max_height = 2048,
+	.max_spw = 0xff,
+	.max_vpw = 0xff,
+	.max_hpw = 0x3ff,
+	.fixed_clksrc = true,
+	.nlayers = ARRAY_SIZE(atmel_xlcdc_sam9x7_layers),
+	.layers = atmel_xlcdc_sam9x7_layers,
+};
+
 static const struct of_device_id atmel_hlcdc_of_match[] = {
 	{
 		.compatible = "atmel,at91sam9n12-hlcdc",
@@ -487,6 +579,10 @@ static const struct of_device_id atmel_hlcdc_of_match[] = {
 		.compatible = "microchip,sam9x60-hlcdc",
 		.data = &atmel_hlcdc_dc_sam9x60,
 	},
+	{
+		.compatible = "microchip,sam9x7-xlcdc",
+		.data = &atmel_xlcdc_dc_sam9x7,
+	},
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, atmel_hlcdc_of_match);
-- 
2.25.1

Re: [PATCH 3/9] drm: atmel-hlcdc: add LCD controller layer definition for SAM9X7

[Sam Ravnborg]

Hi Manikandan,

On Tue, Jun 13, 2023 at 12:34:20PM +0530, Manikandan Muralidharan wrote:
> Add the LCD controller layer definition and descriptor structure for SAM9X7
> for the following layers,
> - Base Layer
> - Overlay1 Layer
> - Overlay2 Layer
> - High End Overlay
> 
> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 96 ++++++++++++++++++++
>  1 file changed, 96 insertions(+)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> index fa0f9a93d50d..d7ad828e9e8c 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> @@ -462,6 +462,98 @@ static const struct atmel_hlcdc_dc_desc atmel_hlcdc_dc_sam9x60 = {
>  	.layers = atmel_hlcdc_sam9x60_layers,
>  };
>  
> +static const struct atmel_hlcdc_layer_desc atmel_xlcdc_sam9x7_layers[] = {
> +	{
> +		.name = "base",
> +		.formats = &atmel_hlcdc_plane_rgb_formats,
> +		.regs_offset = 0x60,
> +		.id = 0,
> +		.type = ATMEL_HLCDC_BASE_LAYER,
> +		.cfgs_offset = 0x1c,
> +		.layout = {
> +			.xstride = { 2 },
> +			.default_color = 3,
> +			.general_config = 4,
> +			.disc_pos = 5,
> +			.disc_size = 6,
> +		},
> +		.clut_offset = 0x700,
> +	},
> +	{
> +		.name = "overlay1",
> +		.formats = &atmel_hlcdc_plane_rgb_formats,
> +		.regs_offset = 0x160,
> +		.id = 1,
> +		.type = ATMEL_HLCDC_OVERLAY_LAYER,
> +		.cfgs_offset = 0x1c,
> +		.layout = {
> +			.pos = 2,
> +			.size = 3,
> +			.xstride = { 4 },
> +			.pstride = { 5 },
> +			.default_color = 6,
> +			.chroma_key = 7,
> +			.chroma_key_mask = 8,
> +			.general_config = 9,
> +		},
> +		.clut_offset = 0xb00,
> +	},
> +	{
> +		.name = "overlay2",
> +		.formats = &atmel_hlcdc_plane_rgb_formats,
> +		.regs_offset = 0x260,
> +		.id = 2,
> +		.type = ATMEL_HLCDC_OVERLAY_LAYER,
> +		.cfgs_offset = 0x1c,
> +		.layout = {
> +			.pos = 2,
> +			.size = 3,
> +			.xstride = { 4 },
> +			.pstride = { 5 },
> +			.default_color = 6,
> +			.chroma_key = 7,
> +			.chroma_key_mask = 8,
> +			.general_config = 9,
> +		},
> +		.clut_offset = 0xf00,
> +	},
> +	{
> +		.name = "high-end-overlay",
> +		.formats = &atmel_hlcdc_plane_rgb_and_yuv_formats,
> +		.regs_offset = 0x360,
> +		.id = 3,
> +		.type = ATMEL_HLCDC_OVERLAY_LAYER,
> +		.cfgs_offset = 0x30,
> +		.layout = {
> +			.pos = 2,
> +			.size = 3,
> +			.memsize = 4,
> +			.xstride = { 5, 7 },
> +			.pstride = { 6, 8 },
> +			.default_color = 9,
> +			.chroma_key = 10,
> +			.chroma_key_mask = 11,
> +			.general_config = 12,
> +			.csc = 16,
> +			.scaler_config = 23,
> +		},
> +		.clut_offset = 0x1300,
> +	},
> +};
> +
> +static const struct atmel_hlcdc_dc_desc atmel_xlcdc_dc_sam9x7 = {
> +	.min_width = 0,
> +	.min_height = 0,
> +	.max_width = 2048,
> +	.max_height = 2048,
> +	.max_spw = 0xff,
> +	.max_vpw = 0xff,
> +	.max_hpw = 0x3ff,
> +	.fixed_clksrc = true,
> +	.nlayers = ARRAY_SIZE(atmel_xlcdc_sam9x7_layers),
> +	.layers = atmel_xlcdc_sam9x7_layers,
> +};

As already suggested by someone else, add is_xlcdc to struct
atmel_hlcdc_dc_desc, so the check for the compatible can be dropped.
It would be better to put it here.


	Sam

[PATCH 4/9] drm: atmel-hlcdc: Define SAM9X7 XLCDC specific registers

[Manikandan Muralidharan]

From: Durai Manickam KR <durai.manickamkr@microchip.com>

The register address of the XLCDC IP used in SAM9X7 are different from
the previous HLCDC.Defining those address space with valid macros.

Signed-off-by: Durai Manickam KR <durai.manickamkr@microchip.com>
[manikandan.m@microchip.com: Remove unused macro definitions]
Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 108 +++++++++++++++++++
 include/linux/mfd/atmel-hlcdc.h              |  10 ++
 2 files changed, 118 insertions(+)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
index 5b5c774e0edf..aed1742b3665 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
@@ -15,6 +15,7 @@
 
 #include <drm/drm_plane.h>
 
+/* LCD controller common registers */
 #define ATMEL_HLCDC_LAYER_CHER			0x0
 #define ATMEL_HLCDC_LAYER_CHDR			0x4
 #define ATMEL_HLCDC_LAYER_CHSR			0x8
@@ -128,6 +129,113 @@
 
 #define ATMEL_HLCDC_MAX_LAYERS			6
 
+/* XLCDC controller specific registers */
+#define ATMEL_XLCDC_LAYER_ENR			0x10
+#define ATMEL_XLCDC_LAYER_EN			BIT(0)
+
+#define ATMEL_XLCDC_LAYER_IER			0x0
+#define ATMEL_XLCDC_LAYER_IDR			0x4
+#define ATMEL_XLCDC_LAYER_IMR			0x8
+#define ATMEL_XLCDC_LAYER_ISR			0xc
+#define ATMEL_XLCDC_LAYER_DONE_IRQ(p)		BIT(0 + (8 * (p)))
+#define ATMEL_XLCDC_LAYER_ERROR_IRQ(p)		BIT(1 + (8 * (p)))
+#define ATMEL_XLCDC_LAYER_OVR_IRQ(p)		BIT(2 + (8 * (p)))
+
+#define ATMEL_XLCDC_LAYER_PLANE_ADDR(p)		(((p) * 0x4) + 0x18)
+
+#define ATMEL_XLCDC_LAYER_DMA_CFG		0
+#define ATMEL_XLCDC_LAYER_DMA_BLEN_MASK		GENMASK(6, 4)
+#define ATMEL_XLCDC_LAYER_DMA_BLEN_SINGLE	(0 << 4)
+#define ATMEL_XLCDC_LAYER_DMA_BLEN_INCR32	(4 << 4)
+#define ATMEL_XLCDC_LAYER_DMA_BLENCC_MASK	GENMASK(10, 8)
+#define ATMEL_XLCDC_LAYER_DMA_BLENCC_SINGLE	(0 << 8)
+#define ATMEL_XLCDC_LAYER_DMA_BLENCC_INCR4	(1 << 8)
+#define ATMEL_XLCDC_LAYER_DMA_BLENCC_INCR8	(2 << 8)
+#define ATMEL_XLCDC_LAYER_DMA_BLENCC_INCR16	(3 << 8)
+#define ATMEL_XLCDC_LAYER_DMA_BLENCC_INCR32	(4 << 8)
+
+#define ATMEL_XLCDC_GAM				BIT(2)
+
+#define ATMEL_XLCDC_LAYER_POS(x, y)		((x) | ((y) << 16))
+#define ATMEL_XLCDC_LAYER_SIZE(w, h)		(((w) - 1) | (((h) - 1) << 16))
+
+#define ATMEL_XLCDC_LAYER_DMA			BIT(0)
+#define ATMEL_XLCDC_LAYER_REP			BIT(1)
+#define ATMEL_XLCDC_LAYER_CRKEY			BIT(2)
+#define ATMEL_XLCDC_LAYER_DSTKEY		BIT(3)
+#define ATMEL_XLCDC_LAYER_DISCEN                BIT(4)
+#define ATMEL_XLCDC_LAYER_VIDPRI		BIT(5)
+#define ATMEL_XLCDC_LAYER_SFACTC_MASK		GENMASK(8, 6)
+#define ATMEL_XLCDC_LAYER_SFACTC_ONE		(0 << 6)
+#define ATMEL_XLCDC_LAYER_SFACTC_ZERO		(1 << 6)
+#define ATMEL_XLCDC_LAYER_SFACTC_A0		(2 << 6)
+#define ATMEL_XLCDC_LAYER_SFACTC_A0_MULT_AD	(3 << 6)
+#define ATMEL_XLCDC_LAYER_SFACTC_A0_MULT_AS	(4 << 6)
+#define ATMEL_XLCDC_LAYER_SFACTC_M_A0_MULT_AD	(5 << 6)
+#define ATMEL_XLCDC_LAYER_SFACTA_MASK		GENMASK(10, 9)
+#define ATMEL_XLCDC_LAYER_SFACTA_ZERO		(0 << 9)
+#define ATMEL_XLCDC_LAYER_SFACTA_ONE		(1 << 9)
+#define ATMEL_XLCDC_LAYER_SFACTA_A0		(2 << 9)
+#define ATMEL_XLCDC_LAYER_SFACTA_A1		(3 << 9)
+#define ATMEL_XLCDC_LAYER_DFACTC_MASK		GENMASK(13, 11)
+#define ATMEL_XLCDC_LAYER_DFACTC_ZERO		(0 << 11)
+#define ATMEL_XLCDC_LAYER_DFACTC_ONE		(1 << 11)
+#define ATMEL_XLCDC_LAYER_DFACTC_A0		(2 << 11)
+#define ATMEL_XLCDC_LAYER_DFACTC_A1		(3 << 11)
+#define ATMEL_XLCDC_LAYER_DFACTC_A0_MULT_AD	(4 << 11)
+#define ATMEL_XLCDC_LAYER_DFACTC_M_A0_MULT_AD	(5 << 11)
+#define ATMEL_XLCDC_LAYER_DFACTC_M_A0_MULT_AS	(6 << 11)
+#define ATMEL_XLCDC_LAYER_DFACTC_M_A0		(7 << 11)
+#define ATMEL_XLCDC_LAYER_DFACTA_MASK		GENMASK(15, 14)
+#define ATMEL_XLCDC_LAYER_DFACTA_ZERO		(0 << 14)
+#define ATMEL_XLCDC_LAYER_DFACTA_ONE		(1 << 14)
+#define ATMEL_XLCDC_LAYER_DFACTA_M_A0_MULT_AS	(2 << 14)
+#define ATMEL_XLCDC_LAYER_DFACTA_A1		(3 << 14)
+#define ATMEL_XLCDC_LAYER_A0_SHIFT		16
+#define ATMEL_XLCDC_LAYER_A0_MASK		\
+	GENMASK(23, ATMEL_XLCDC_LAYER_A0_SHIFT)
+#define ATMEL_XLCDC_LAYER_A0(x)			\
+	((x) << ATMEL_XLCDC_LAYER_A0_SHIFT)
+#define ATMEL_XLCDC_LAYER_A1_SHIFT		24
+#define ATMEL_XLCDC_LAYER_A1_MASK		\
+	GENMASK(31, ATMEL_XLCDC_LAYER_A1_SHIFT)
+#define ATMEL_XLCDC_LAYER_A1(x)			\
+	((x) << ATMEL_XLCDC_LAYER_A1_SHIFT)
+
+#define ATMEL_XLCDC_LAYER_DISC_POS(x, y)	((x) | ((y) << 16))
+#define ATMEL_XLCDC_LAYER_DISC_SIZE(w, h)	(((w) - 1) | (((h) - 1) << 16))
+
+#define ATMEL_XLCDC_LAYER_VSCALER_LUMA_ENABLE		BIT(0)
+#define ATMEL_XLCDC_LAYER_VSCALER_CHROMA_ENABLE		BIT(1)
+#define ATMEL_XLCDC_LAYER_HSCALER_LUMA_ENABLE		BIT(4)
+#define ATMEL_XLCDC_LAYER_HSCALER_CHROMA_ENABLE		BIT(5)
+
+#define ATMEL_XLCDC_LAYER_VXSYCFG_ZERO		(0 << 0)
+#define ATMEL_XLCDC_LAYER_VXSYCFG_ONE		(1 << 0)
+#define ATMEL_XLCDC_LAYER_VXSYCFG_TWO		(2 << 0)
+#define ATMEL_XLCDC_LAYER_VXSYCFG_THREE		(3 << 0)
+#define ATMEL_XLCDC_LAYER_VXSYTAP2_ENABLE	BIT(4)
+#define ATMEL_XLCDC_LAYER_VXSYBICU_ENABLE	BIT(5)
+#define ATMEL_XLCDC_LAYER_VXSCCFG_ZERO		(0 << 16)
+#define ATMEL_XLCDC_LAYER_VXSCCFG_ONE		(1 << 16)
+#define ATMEL_XLCDC_LAYER_VXSCCFG_TWO		(2 << 16)
+#define ATMEL_XLCDC_LAYER_VXSCCFG_THREE		(3 << 16)
+#define ATMEL_XLCDC_LAYER_VXSCTAP2_ENABLE	BIT(20)
+#define ATMEL_XLCDC_LAYER_VXSCBICU_ENABLE	BIT(21)
+
+#define ATMEL_XLCDC_LAYER_HXSYCFG_ZERO		(0 << 0)
+#define ATMEL_XLCDC_LAYER_HXSYCFG_ONE		(1 << 0)
+#define ATMEL_XLCDC_LAYER_HXSYCFG_TWO		(2 << 0)
+#define ATMEL_XLCDC_LAYER_HXSYCFG_THREE		(3 << 0)
+#define ATMEL_XLCDC_LAYER_HXSYTAP2_ENABLE	BIT(4)
+#define ATMEL_XLCDC_LAYER_HXSYBICU_ENABLE	BIT(5)
+#define ATMEL_XLCDC_LAYER_HXSCCFG_ZERO		(0 << 16)
+#define ATMEL_XLCDC_LAYER_HXSCCFG_ONE		(1 << 16)
+#define ATMEL_XLCDC_LAYER_HXSCCFG_TWO		(2 << 16)
+#define ATMEL_XLCDC_LAYER_HXSCCFG_THREE		(3 << 16)
+#define ATMEL_XLCDC_LAYER_HXSCTAP2_ENABLE	BIT(20)
+#define ATMEL_XLCDC_LAYER_HXSCBICU_ENABLE	BIT(21)
+
 /**
  * Atmel HLCDC Layer registers layout structure
  *
diff --git a/include/linux/mfd/atmel-hlcdc.h b/include/linux/mfd/atmel-hlcdc.h
index a186119a49b5..80d675a03b39 100644
--- a/include/linux/mfd/atmel-hlcdc.h
+++ b/include/linux/mfd/atmel-hlcdc.h
@@ -22,6 +22,8 @@
 #define ATMEL_HLCDC_DITHER		BIT(6)
 #define ATMEL_HLCDC_DISPDLY		BIT(7)
 #define ATMEL_HLCDC_MODE_MASK		GENMASK(9, 8)
+#define ATMEL_XLCDC_MODE_MASK		GENMASK(10, 8)
+#define ATMEL_XLCDC_DPI			BIT(11)
 #define ATMEL_HLCDC_PP			BIT(10)
 #define ATMEL_HLCDC_VSPSU		BIT(12)
 #define ATMEL_HLCDC_VSPHO		BIT(13)
@@ -34,6 +36,12 @@
 #define ATMEL_HLCDC_IDR			0x30
 #define ATMEL_HLCDC_IMR			0x34
 #define ATMEL_HLCDC_ISR			0x38
+#define ATMEL_XLCDC_ATTRE		0x3c
+
+#define ATMEL_XLCDC_BASE_UPDATE		BIT(0)
+#define ATMEL_XLCDC_OVR1_UPDATE		BIT(1)
+#define ATMEL_XLCDC_OVR3_UPDATE		BIT(2)
+#define ATMEL_XLCDC_HEO_UPDATE		BIT(3)
 
 #define ATMEL_HLCDC_CLKPOL		BIT(0)
 #define ATMEL_HLCDC_CLKSEL		BIT(2)
@@ -48,6 +56,8 @@
 #define ATMEL_HLCDC_DISP		BIT(2)
 #define ATMEL_HLCDC_PWM			BIT(3)
 #define ATMEL_HLCDC_SIP			BIT(4)
+#define ATMEL_XLCDC_SD			BIT(5)
+#define ATMEL_XLCDC_CM			BIT(6)
 
 #define ATMEL_HLCDC_SOF			BIT(0)
 #define ATMEL_HLCDC_SYNCDIS		BIT(1)
-- 
2.25.1

Re: [PATCH 4/9] drm: atmel-hlcdc: Define SAM9X7 XLCDC specific registers

[Claudiu.Beznea]

Hi, Manikandan,

On 13.06.2023 10:04, Manikandan Muralidharan wrote:
> From: Durai Manickam KR <durai.manickamkr@microchip.com>
> 
> The register address of the XLCDC IP used in SAM9X7 are different from
> the previous HLCDC.Defining those address space with valid macros.
> 
> Signed-off-by: Durai Manickam KR <durai.manickamkr@microchip.com>
> [manikandan.m@microchip.com: Remove unused macro definitions]
> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 108 +++++++++++++++++++
>  include/linux/mfd/atmel-hlcdc.h              |  10 ++
>  2 files changed, 118 insertions(+)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> index 5b5c774e0edf..aed1742b3665 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> @@ -15,6 +15,7 @@
>  
>  #include <drm/drm_plane.h>
>  
> +/* LCD controller common registers */
>  #define ATMEL_HLCDC_LAYER_CHER			0x0
>  #define ATMEL_HLCDC_LAYER_CHDR			0x4
>  #define ATMEL_HLCDC_LAYER_CHSR			0x8
> @@ -128,6 +129,113 @@
>  
>  #define ATMEL_HLCDC_MAX_LAYERS			6
>  
> +/* XLCDC controller specific registers */
> +#define ATMEL_XLCDC_LAYER_ENR			0x10
> +#define ATMEL_XLCDC_LAYER_EN			BIT(0)
> +
> +#define ATMEL_XLCDC_LAYER_IER			0x0
> +#define ATMEL_XLCDC_LAYER_IDR			0x4
> +#define ATMEL_XLCDC_LAYER_IMR			0x8
> +#define ATMEL_XLCDC_LAYER_ISR			0xc
> +#define ATMEL_XLCDC_LAYER_DONE_IRQ(p)		BIT(0 + (8 * (p)))
> +#define ATMEL_XLCDC_LAYER_ERROR_IRQ(p)		BIT(1 + (8 * (p)))
> +#define ATMEL_XLCDC_LAYER_OVR_IRQ(p)		BIT(2 + (8 * (p)))
> +
> +#define ATMEL_XLCDC_LAYER_PLANE_ADDR(p)		(((p) * 0x4) + 0x18)
> +
> +#define ATMEL_XLCDC_LAYER_DMA_CFG		0
> +#define ATMEL_XLCDC_LAYER_DMA_BLEN_MASK		GENMASK(6, 4)
> +#define ATMEL_XLCDC_LAYER_DMA_BLEN_SINGLE	(0 << 4)
> +#define ATMEL_XLCDC_LAYER_DMA_BLEN_INCR32	(4 << 4)
> +#define ATMEL_XLCDC_LAYER_DMA_BLENCC_MASK	GENMASK(10, 8)
> +#define ATMEL_XLCDC_LAYER_DMA_BLENCC_SINGLE	(0 << 8)
> +#define ATMEL_XLCDC_LAYER_DMA_BLENCC_INCR4	(1 << 8)
> +#define ATMEL_XLCDC_LAYER_DMA_BLENCC_INCR8	(2 << 8)
> +#define ATMEL_XLCDC_LAYER_DMA_BLENCC_INCR16	(3 << 8)
> +#define ATMEL_XLCDC_LAYER_DMA_BLENCC_INCR32	(4 << 8)
> +
> +#define ATMEL_XLCDC_GAM				BIT(2)
> +
> +#define ATMEL_XLCDC_LAYER_POS(x, y)		((x) | ((y) << 16))
> +#define ATMEL_XLCDC_LAYER_SIZE(w, h)		(((w) - 1) | (((h) - 1) << 16))
> +
> +#define ATMEL_XLCDC_LAYER_DMA			BIT(0)
> +#define ATMEL_XLCDC_LAYER_REP			BIT(1)
> +#define ATMEL_XLCDC_LAYER_CRKEY			BIT(2)
> +#define ATMEL_XLCDC_LAYER_DSTKEY		BIT(3)
> +#define ATMEL_XLCDC_LAYER_DISCEN                BIT(4)
> +#define ATMEL_XLCDC_LAYER_VIDPRI		BIT(5)
> +#define ATMEL_XLCDC_LAYER_SFACTC_MASK		GENMASK(8, 6)
> +#define ATMEL_XLCDC_LAYER_SFACTC_ONE		(0 << 6)
> +#define ATMEL_XLCDC_LAYER_SFACTC_ZERO		(1 << 6)
> +#define ATMEL_XLCDC_LAYER_SFACTC_A0		(2 << 6)
> +#define ATMEL_XLCDC_LAYER_SFACTC_A0_MULT_AD	(3 << 6)
> +#define ATMEL_XLCDC_LAYER_SFACTC_A0_MULT_AS	(4 << 6)
> +#define ATMEL_XLCDC_LAYER_SFACTC_M_A0_MULT_AD	(5 << 6)
> +#define ATMEL_XLCDC_LAYER_SFACTA_MASK		GENMASK(10, 9)
> +#define ATMEL_XLCDC_LAYER_SFACTA_ZERO		(0 << 9)
> +#define ATMEL_XLCDC_LAYER_SFACTA_ONE		(1 << 9)
> +#define ATMEL_XLCDC_LAYER_SFACTA_A0		(2 << 9)
> +#define ATMEL_XLCDC_LAYER_SFACTA_A1		(3 << 9)
> +#define ATMEL_XLCDC_LAYER_DFACTC_MASK		GENMASK(13, 11)
> +#define ATMEL_XLCDC_LAYER_DFACTC_ZERO		(0 << 11)
> +#define ATMEL_XLCDC_LAYER_DFACTC_ONE		(1 << 11)
> +#define ATMEL_XLCDC_LAYER_DFACTC_A0		(2 << 11)
> +#define ATMEL_XLCDC_LAYER_DFACTC_A1		(3 << 11)
> +#define ATMEL_XLCDC_LAYER_DFACTC_A0_MULT_AD	(4 << 11)
> +#define ATMEL_XLCDC_LAYER_DFACTC_M_A0_MULT_AD	(5 << 11)
> +#define ATMEL_XLCDC_LAYER_DFACTC_M_A0_MULT_AS	(6 << 11)
> +#define ATMEL_XLCDC_LAYER_DFACTC_M_A0		(7 << 11)
> +#define ATMEL_XLCDC_LAYER_DFACTA_MASK		GENMASK(15, 14)
> +#define ATMEL_XLCDC_LAYER_DFACTA_ZERO		(0 << 14)
> +#define ATMEL_XLCDC_LAYER_DFACTA_ONE		(1 << 14)
> +#define ATMEL_XLCDC_LAYER_DFACTA_M_A0_MULT_AS	(2 << 14)
> +#define ATMEL_XLCDC_LAYER_DFACTA_A1		(3 << 14)
> +#define ATMEL_XLCDC_LAYER_A0_SHIFT		16
> +#define ATMEL_XLCDC_LAYER_A0_MASK		\
> +	GENMASK(23, ATMEL_XLCDC_LAYER_A0_SHIFT)
> +#define ATMEL_XLCDC_LAYER_A0(x)			\
> +	((x) << ATMEL_XLCDC_LAYER_A0_SHIFT)
> +#define ATMEL_XLCDC_LAYER_A1_SHIFT		24
> +#define ATMEL_XLCDC_LAYER_A1_MASK		\
> +	GENMASK(31, ATMEL_XLCDC_LAYER_A1_SHIFT)
> +#define ATMEL_XLCDC_LAYER_A1(x)			\
> +	((x) << ATMEL_XLCDC_LAYER_A1_SHIFT)
> +
> +#define ATMEL_XLCDC_LAYER_DISC_POS(x, y)	((x) | ((y) << 16))
> +#define ATMEL_XLCDC_LAYER_DISC_SIZE(w, h)	(((w) - 1) | (((h) - 1) << 16))
> +
> +#define ATMEL_XLCDC_LAYER_VSCALER_LUMA_ENABLE		BIT(0)
> +#define ATMEL_XLCDC_LAYER_VSCALER_CHROMA_ENABLE		BIT(1)
> +#define ATMEL_XLCDC_LAYER_HSCALER_LUMA_ENABLE		BIT(4)
> +#define ATMEL_XLCDC_LAYER_HSCALER_CHROMA_ENABLE		BIT(5)
> +
> +#define ATMEL_XLCDC_LAYER_VXSYCFG_ZERO		(0 << 0)
> +#define ATMEL_XLCDC_LAYER_VXSYCFG_ONE		(1 << 0)
> +#define ATMEL_XLCDC_LAYER_VXSYCFG_TWO		(2 << 0)
> +#define ATMEL_XLCDC_LAYER_VXSYCFG_THREE		(3 << 0)
> +#define ATMEL_XLCDC_LAYER_VXSYTAP2_ENABLE	BIT(4)
> +#define ATMEL_XLCDC_LAYER_VXSYBICU_ENABLE	BIT(5)
> +#define ATMEL_XLCDC_LAYER_VXSCCFG_ZERO		(0 << 16)
> +#define ATMEL_XLCDC_LAYER_VXSCCFG_ONE		(1 << 16)
> +#define ATMEL_XLCDC_LAYER_VXSCCFG_TWO		(2 << 16)
> +#define ATMEL_XLCDC_LAYER_VXSCCFG_THREE		(3 << 16)
> +#define ATMEL_XLCDC_LAYER_VXSCTAP2_ENABLE	BIT(20)
> +#define ATMEL_XLCDC_LAYER_VXSCBICU_ENABLE	BIT(21)
> +
> +#define ATMEL_XLCDC_LAYER_HXSYCFG_ZERO		(0 << 0)
> +#define ATMEL_XLCDC_LAYER_HXSYCFG_ONE		(1 << 0)
> +#define ATMEL_XLCDC_LAYER_HXSYCFG_TWO		(2 << 0)
> +#define ATMEL_XLCDC_LAYER_HXSYCFG_THREE		(3 << 0)
> +#define ATMEL_XLCDC_LAYER_HXSYTAP2_ENABLE	BIT(4)
> +#define ATMEL_XLCDC_LAYER_HXSYBICU_ENABLE	BIT(5)
> +#define ATMEL_XLCDC_LAYER_HXSCCFG_ZERO		(0 << 16)
> +#define ATMEL_XLCDC_LAYER_HXSCCFG_ONE		(1 << 16)
> +#define ATMEL_XLCDC_LAYER_HXSCCFG_TWO		(2 << 16)
> +#define ATMEL_XLCDC_LAYER_HXSCCFG_THREE		(3 << 16)
> +#define ATMEL_XLCDC_LAYER_HXSCTAP2_ENABLE	BIT(20)
> +#define ATMEL_XLCDC_LAYER_HXSCBICU_ENABLE	BIT(21)
> +

There are a bunch of defines included in this file not used anywhere in the
driver. Please check and keep only necessary.

Thank you,
Claudiu

>  /**
>   * Atmel HLCDC Layer registers layout structure
>   *
> diff --git a/include/linux/mfd/atmel-hlcdc.h b/include/linux/mfd/atmel-hlcdc.h
> index a186119a49b5..80d675a03b39 100644
> --- a/include/linux/mfd/atmel-hlcdc.h
> +++ b/include/linux/mfd/atmel-hlcdc.h
> @@ -22,6 +22,8 @@
>  #define ATMEL_HLCDC_DITHER		BIT(6)
>  #define ATMEL_HLCDC_DISPDLY		BIT(7)
>  #define ATMEL_HLCDC_MODE_MASK		GENMASK(9, 8)
> +#define ATMEL_XLCDC_MODE_MASK		GENMASK(10, 8)
> +#define ATMEL_XLCDC_DPI			BIT(11)
>  #define ATMEL_HLCDC_PP			BIT(10)
>  #define ATMEL_HLCDC_VSPSU		BIT(12)
>  #define ATMEL_HLCDC_VSPHO		BIT(13)
> @@ -34,6 +36,12 @@
>  #define ATMEL_HLCDC_IDR			0x30
>  #define ATMEL_HLCDC_IMR			0x34
>  #define ATMEL_HLCDC_ISR			0x38
> +#define ATMEL_XLCDC_ATTRE		0x3c
> +
> +#define ATMEL_XLCDC_BASE_UPDATE		BIT(0)
> +#define ATMEL_XLCDC_OVR1_UPDATE		BIT(1)
> +#define ATMEL_XLCDC_OVR3_UPDATE		BIT(2)
> +#define ATMEL_XLCDC_HEO_UPDATE		BIT(3)
>  
>  #define ATMEL_HLCDC_CLKPOL		BIT(0)
>  #define ATMEL_HLCDC_CLKSEL		BIT(2)
> @@ -48,6 +56,8 @@
>  #define ATMEL_HLCDC_DISP		BIT(2)
>  #define ATMEL_HLCDC_PWM			BIT(3)
>  #define ATMEL_HLCDC_SIP			BIT(4)
> +#define ATMEL_XLCDC_SD			BIT(5)
> +#define ATMEL_XLCDC_CM			BIT(6)
>  
>  #define ATMEL_HLCDC_SOF			BIT(0)
>  #define ATMEL_HLCDC_SYNCDIS		BIT(1)

Re: [PATCH 4/9] drm: atmel-hlcdc: Define SAM9X7 XLCDC specific registers

[Manikandan.M]

On 14/06/23 12:17, Claudiu Beznea - M18063 wrote:
> Hi, Manikandan,
> 
> On 13.06.2023 10:04, Manikandan Muralidharan wrote:
>> From: Durai Manickam KR <durai.manickamkr@microchip.com>
>>
>> The register address of the XLCDC IP used in SAM9X7 are different from
>> the previous HLCDC.Defining those address space with valid macros.
>>
>> Signed-off-by: Durai Manickam KR <durai.manickamkr@microchip.com>
>> [manikandan.m@microchip.com: Remove unused macro definitions]
>> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
>> ---
>>   drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 108 +++++++++++++++++++
>>   include/linux/mfd/atmel-hlcdc.h              |  10 ++
>>   2 files changed, 118 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
>> index 5b5c774e0edf..aed1742b3665 100644
>> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
>> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
>> @@ -15,6 +15,7 @@
>>   
>>   #include <drm/drm_plane.h>
>>   
>> +/* LCD controller common registers */
>>   #define ATMEL_HLCDC_LAYER_CHER			0x0
>>   #define ATMEL_HLCDC_LAYER_CHDR			0x4
>>   #define ATMEL_HLCDC_LAYER_CHSR			0x8
>> @@ -128,6 +129,113 @@
>>   
>>   #define ATMEL_HLCDC_MAX_LAYERS			6
>>   
>> +/* XLCDC controller specific registers */
>> +#define ATMEL_XLCDC_LAYER_ENR			0x10
>> +#define ATMEL_XLCDC_LAYER_EN			BIT(0)
>> +
>> +#define ATMEL_XLCDC_LAYER_IER			0x0
>> +#define ATMEL_XLCDC_LAYER_IDR			0x4
>> +#define ATMEL_XLCDC_LAYER_IMR			0x8
>> +#define ATMEL_XLCDC_LAYER_ISR			0xc
>> +#define ATMEL_XLCDC_LAYER_DONE_IRQ(p)		BIT(0 + (8 * (p)))
>> +#define ATMEL_XLCDC_LAYER_ERROR_IRQ(p)		BIT(1 + (8 * (p)))
>> +#define ATMEL_XLCDC_LAYER_OVR_IRQ(p)		BIT(2 + (8 * (p)))
>> +
>> +#define ATMEL_XLCDC_LAYER_PLANE_ADDR(p)		(((p) * 0x4) + 0x18)
>> +
>> +#define ATMEL_XLCDC_LAYER_DMA_CFG		0
>> +#define ATMEL_XLCDC_LAYER_DMA_BLEN_MASK		GENMASK(6, 4)
>> +#define ATMEL_XLCDC_LAYER_DMA_BLEN_SINGLE	(0 << 4)
>> +#define ATMEL_XLCDC_LAYER_DMA_BLEN_INCR32	(4 << 4)
>> +#define ATMEL_XLCDC_LAYER_DMA_BLENCC_MASK	GENMASK(10, 8)
>> +#define ATMEL_XLCDC_LAYER_DMA_BLENCC_SINGLE	(0 << 8)
>> +#define ATMEL_XLCDC_LAYER_DMA_BLENCC_INCR4	(1 << 8)
>> +#define ATMEL_XLCDC_LAYER_DMA_BLENCC_INCR8	(2 << 8)
>> +#define ATMEL_XLCDC_LAYER_DMA_BLENCC_INCR16	(3 << 8)
>> +#define ATMEL_XLCDC_LAYER_DMA_BLENCC_INCR32	(4 << 8)
>> +
>> +#define ATMEL_XLCDC_GAM				BIT(2)
>> +
>> +#define ATMEL_XLCDC_LAYER_POS(x, y)		((x) | ((y) << 16))
>> +#define ATMEL_XLCDC_LAYER_SIZE(w, h)		(((w) - 1) | (((h) - 1) << 16))
>> +
>> +#define ATMEL_XLCDC_LAYER_DMA			BIT(0)
>> +#define ATMEL_XLCDC_LAYER_REP			BIT(1)
>> +#define ATMEL_XLCDC_LAYER_CRKEY			BIT(2)
>> +#define ATMEL_XLCDC_LAYER_DSTKEY		BIT(3)
>> +#define ATMEL_XLCDC_LAYER_DISCEN                BIT(4)
>> +#define ATMEL_XLCDC_LAYER_VIDPRI		BIT(5)
>> +#define ATMEL_XLCDC_LAYER_SFACTC_MASK		GENMASK(8, 6)
>> +#define ATMEL_XLCDC_LAYER_SFACTC_ONE		(0 << 6)
>> +#define ATMEL_XLCDC_LAYER_SFACTC_ZERO		(1 << 6)
>> +#define ATMEL_XLCDC_LAYER_SFACTC_A0		(2 << 6)
>> +#define ATMEL_XLCDC_LAYER_SFACTC_A0_MULT_AD	(3 << 6)
>> +#define ATMEL_XLCDC_LAYER_SFACTC_A0_MULT_AS	(4 << 6)
>> +#define ATMEL_XLCDC_LAYER_SFACTC_M_A0_MULT_AD	(5 << 6)
>> +#define ATMEL_XLCDC_LAYER_SFACTA_MASK		GENMASK(10, 9)
>> +#define ATMEL_XLCDC_LAYER_SFACTA_ZERO		(0 << 9)
>> +#define ATMEL_XLCDC_LAYER_SFACTA_ONE		(1 << 9)
>> +#define ATMEL_XLCDC_LAYER_SFACTA_A0		(2 << 9)
>> +#define ATMEL_XLCDC_LAYER_SFACTA_A1		(3 << 9)
>> +#define ATMEL_XLCDC_LAYER_DFACTC_MASK		GENMASK(13, 11)
>> +#define ATMEL_XLCDC_LAYER_DFACTC_ZERO		(0 << 11)
>> +#define ATMEL_XLCDC_LAYER_DFACTC_ONE		(1 << 11)
>> +#define ATMEL_XLCDC_LAYER_DFACTC_A0		(2 << 11)
>> +#define ATMEL_XLCDC_LAYER_DFACTC_A1		(3 << 11)
>> +#define ATMEL_XLCDC_LAYER_DFACTC_A0_MULT_AD	(4 << 11)
>> +#define ATMEL_XLCDC_LAYER_DFACTC_M_A0_MULT_AD	(5 << 11)
>> +#define ATMEL_XLCDC_LAYER_DFACTC_M_A0_MULT_AS	(6 << 11)
>> +#define ATMEL_XLCDC_LAYER_DFACTC_M_A0		(7 << 11)
>> +#define ATMEL_XLCDC_LAYER_DFACTA_MASK		GENMASK(15, 14)
>> +#define ATMEL_XLCDC_LAYER_DFACTA_ZERO		(0 << 14)
>> +#define ATMEL_XLCDC_LAYER_DFACTA_ONE		(1 << 14)
>> +#define ATMEL_XLCDC_LAYER_DFACTA_M_A0_MULT_AS	(2 << 14)
>> +#define ATMEL_XLCDC_LAYER_DFACTA_A1		(3 << 14)
>> +#define ATMEL_XLCDC_LAYER_A0_SHIFT		16
>> +#define ATMEL_XLCDC_LAYER_A0_MASK		\
>> +	GENMASK(23, ATMEL_XLCDC_LAYER_A0_SHIFT)
>> +#define ATMEL_XLCDC_LAYER_A0(x)			\
>> +	((x) << ATMEL_XLCDC_LAYER_A0_SHIFT)
>> +#define ATMEL_XLCDC_LAYER_A1_SHIFT		24
>> +#define ATMEL_XLCDC_LAYER_A1_MASK		\
>> +	GENMASK(31, ATMEL_XLCDC_LAYER_A1_SHIFT)
>> +#define ATMEL_XLCDC_LAYER_A1(x)			\
>> +	((x) << ATMEL_XLCDC_LAYER_A1_SHIFT)
>> +
>> +#define ATMEL_XLCDC_LAYER_DISC_POS(x, y)	((x) | ((y) << 16))
>> +#define ATMEL_XLCDC_LAYER_DISC_SIZE(w, h)	(((w) - 1) | (((h) - 1) << 16))
>> +
>> +#define ATMEL_XLCDC_LAYER_VSCALER_LUMA_ENABLE		BIT(0)
>> +#define ATMEL_XLCDC_LAYER_VSCALER_CHROMA_ENABLE		BIT(1)
>> +#define ATMEL_XLCDC_LAYER_HSCALER_LUMA_ENABLE		BIT(4)
>> +#define ATMEL_XLCDC_LAYER_HSCALER_CHROMA_ENABLE		BIT(5)
>> +
>> +#define ATMEL_XLCDC_LAYER_VXSYCFG_ZERO		(0 << 0)
>> +#define ATMEL_XLCDC_LAYER_VXSYCFG_ONE		(1 << 0)
>> +#define ATMEL_XLCDC_LAYER_VXSYCFG_TWO		(2 << 0)
>> +#define ATMEL_XLCDC_LAYER_VXSYCFG_THREE		(3 << 0)
>> +#define ATMEL_XLCDC_LAYER_VXSYTAP2_ENABLE	BIT(4)
>> +#define ATMEL_XLCDC_LAYER_VXSYBICU_ENABLE	BIT(5)
>> +#define ATMEL_XLCDC_LAYER_VXSCCFG_ZERO		(0 << 16)
>> +#define ATMEL_XLCDC_LAYER_VXSCCFG_ONE		(1 << 16)
>> +#define ATMEL_XLCDC_LAYER_VXSCCFG_TWO		(2 << 16)
>> +#define ATMEL_XLCDC_LAYER_VXSCCFG_THREE		(3 << 16)
>> +#define ATMEL_XLCDC_LAYER_VXSCTAP2_ENABLE	BIT(20)
>> +#define ATMEL_XLCDC_LAYER_VXSCBICU_ENABLE	BIT(21)
>> +
>> +#define ATMEL_XLCDC_LAYER_HXSYCFG_ZERO		(0 << 0)
>> +#define ATMEL_XLCDC_LAYER_HXSYCFG_ONE		(1 << 0)
>> +#define ATMEL_XLCDC_LAYER_HXSYCFG_TWO		(2 << 0)
>> +#define ATMEL_XLCDC_LAYER_HXSYCFG_THREE		(3 << 0)
>> +#define ATMEL_XLCDC_LAYER_HXSYTAP2_ENABLE	BIT(4)
>> +#define ATMEL_XLCDC_LAYER_HXSYBICU_ENABLE	BIT(5)
>> +#define ATMEL_XLCDC_LAYER_HXSCCFG_ZERO		(0 << 16)
>> +#define ATMEL_XLCDC_LAYER_HXSCCFG_ONE		(1 << 16)
>> +#define ATMEL_XLCDC_LAYER_HXSCCFG_TWO		(2 << 16)
>> +#define ATMEL_XLCDC_LAYER_HXSCCFG_THREE		(3 << 16)
>> +#define ATMEL_XLCDC_LAYER_HXSCTAP2_ENABLE	BIT(20)
>> +#define ATMEL_XLCDC_LAYER_HXSCBICU_ENABLE	BIT(21)
>> +
> 
> There are a bunch of defines included in this file not used anywhere in the
> driver. Please check and keep only necessary.
They were defined as mentioned in the datasheet for future use.I will 
remove those that are not used for XLCDC driver code.
> 
> Thank you,
> Claudiu
> 
>>   /**
>>    * Atmel HLCDC Layer registers layout structure
>>    *
>> diff --git a/include/linux/mfd/atmel-hlcdc.h b/include/linux/mfd/atmel-hlcdc.h
>> index a186119a49b5..80d675a03b39 100644
>> --- a/include/linux/mfd/atmel-hlcdc.h
>> +++ b/include/linux/mfd/atmel-hlcdc.h
>> @@ -22,6 +22,8 @@
>>   #define ATMEL_HLCDC_DITHER		BIT(6)
>>   #define ATMEL_HLCDC_DISPDLY		BIT(7)
>>   #define ATMEL_HLCDC_MODE_MASK		GENMASK(9, 8)
>> +#define ATMEL_XLCDC_MODE_MASK		GENMASK(10, 8)
>> +#define ATMEL_XLCDC_DPI			BIT(11)
>>   #define ATMEL_HLCDC_PP			BIT(10)
>>   #define ATMEL_HLCDC_VSPSU		BIT(12)
>>   #define ATMEL_HLCDC_VSPHO		BIT(13)
>> @@ -34,6 +36,12 @@
>>   #define ATMEL_HLCDC_IDR			0x30
>>   #define ATMEL_HLCDC_IMR			0x34
>>   #define ATMEL_HLCDC_ISR			0x38
>> +#define ATMEL_XLCDC_ATTRE		0x3c
>> +
>> +#define ATMEL_XLCDC_BASE_UPDATE		BIT(0)
>> +#define ATMEL_XLCDC_OVR1_UPDATE		BIT(1)
>> +#define ATMEL_XLCDC_OVR3_UPDATE		BIT(2)
>> +#define ATMEL_XLCDC_HEO_UPDATE		BIT(3)
>>   
>>   #define ATMEL_HLCDC_CLKPOL		BIT(0)
>>   #define ATMEL_HLCDC_CLKSEL		BIT(2)
>> @@ -48,6 +56,8 @@
>>   #define ATMEL_HLCDC_DISP		BIT(2)
>>   #define ATMEL_HLCDC_PWM			BIT(3)
>>   #define ATMEL_HLCDC_SIP			BIT(4)
>> +#define ATMEL_XLCDC_SD			BIT(5)
>> +#define ATMEL_XLCDC_CM			BIT(6)
>>   
>>   #define ATMEL_HLCDC_SOF			BIT(0)
>>   #define ATMEL_HLCDC_SYNCDIS		BIT(1)
> 

-- 
Thanks and Regards,
Manikandan M.

[PATCH 5/9] drm: atmel-hlcdc: add compatible string check for XLCDC and HLCDC

[Manikandan Muralidharan]

From: Durai Manickam KR <durai.manickamkr@microchip.com>

Add compatible string check to differentiate XLCDC and HLCDC code
within the atmel-hlcdc driver files.

Signed-off-by: Durai Manickam KR <durai.manickamkr@microchip.com>
Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 7 +++++++
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index d7ad828e9e8c..fbbd2592efc7 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -761,6 +761,13 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev)
 	if (!dc)
 		return -ENOMEM;
 
+	/* SAM9X7 supports XLCDC */
+	if (!strcmp(match->compatible, "microchip,sam9x7-xlcdc"))
+		dc->is_xlcdc = true;
+	else
+		/* Other SoC's that supports HLCDC IP */
+		dc->is_xlcdc = false;
+
 	dc->desc = match->data;
 	dc->hlcdc = dev_get_drvdata(dev->dev->parent);
 	dev->dev_private = dc;
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
index aed1742b3665..804e4d476f2b 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
@@ -451,6 +451,7 @@ struct atmel_hlcdc_dc {
 		u32 imr;
 		struct drm_atomic_state *state;
 	} suspend;
+	bool is_xlcdc;
 };
 
 extern struct atmel_hlcdc_formats atmel_hlcdc_plane_rgb_formats;
-- 
2.25.1

Re: [PATCH 5/9] drm: atmel-hlcdc: add compatible string check for XLCDC and HLCDC

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1816 bytes --]

On Tue, Jun 13, 2023 at 12:34:22PM +0530, Manikandan Muralidharan wrote:
> From: Durai Manickam KR <durai.manickamkr@microchip.com>
> 
> Add compatible string check to differentiate XLCDC and HLCDC code
> within the atmel-hlcdc driver files.
> 
> Signed-off-by: Durai Manickam KR <durai.manickamkr@microchip.com>
> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 7 +++++++
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 1 +
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> index d7ad828e9e8c..fbbd2592efc7 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> @@ -761,6 +761,13 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev)
>  	if (!dc)
>  		return -ENOMEM;
>  
> +	/* SAM9X7 supports XLCDC */
> +	if (!strcmp(match->compatible, "microchip,sam9x7-xlcdc"))
> +		dc->is_xlcdc = true;
> +	else
> +		/* Other SoC's that supports HLCDC IP */

Should this be "Other SoCs that do not support HLCDC IP"?

> +		dc->is_xlcdc = false;
> +
>  	dc->desc = match->data;
>  	dc->hlcdc = dev_get_drvdata(dev->dev->parent);
>  	dev->dev_private = dc;
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> index aed1742b3665..804e4d476f2b 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> @@ -451,6 +451,7 @@ struct atmel_hlcdc_dc {
>  		u32 imr;
>  		struct drm_atomic_state *state;
>  	} suspend;
> +	bool is_xlcdc;
>  };
>  
>  extern struct atmel_hlcdc_formats atmel_hlcdc_plane_rgb_formats;
> -- 
> 2.25.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 5/9] drm: atmel-hlcdc: add compatible string check for XLCDC and HLCDC

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 324 bytes --]

On Tue, Jun 13, 2023 at 07:16:06PM +0100, Conor Dooley wrote:
> On Tue, Jun 13, 2023 at 12:34:22PM +0530, Manikandan Muralidharan wrote:

> > +		/* Other SoC's that supports HLCDC IP */
> 
> Should this be "Other SoCs that do not support HLCDC IP"?

Clearly a reading comprehension fail here. Sorry about the noise!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 5/9] drm: atmel-hlcdc: add compatible string check for XLCDC and HLCDC

[Claudiu.Beznea]

On 13.06.2023 10:04, Manikandan Muralidharan wrote:
> From: Durai Manickam KR <durai.manickamkr@microchip.com>
> 
> Add compatible string check to differentiate XLCDC and HLCDC code
> within the atmel-hlcdc driver files.
> 
> Signed-off-by: Durai Manickam KR <durai.manickamkr@microchip.com>
> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 7 +++++++
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 1 +
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> index d7ad828e9e8c..fbbd2592efc7 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> @@ -761,6 +761,13 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev)
>  	if (!dc)
>  		return -ENOMEM;
>  
> +	/* SAM9X7 supports XLCDC */
> +	if (!strcmp(match->compatible, "microchip,sam9x7-xlcdc"))

This could be avoided if ix_xlcd in added in driver data.

> +		dc->is_xlcdc = true;
> +	else
> +		/* Other SoC's that supports HLCDC IP */
> +		dc->is_xlcdc = false;
> +
>  	dc->desc = match->data;
>  	dc->hlcdc = dev_get_drvdata(dev->dev->parent);
>  	dev->dev_private = dc;
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> index aed1742b3665..804e4d476f2b 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> @@ -451,6 +451,7 @@ struct atmel_hlcdc_dc {
>  		u32 imr;
>  		struct drm_atomic_state *state;
>  	} suspend;
> +	bool is_xlcdc;
>  };
>  
>  extern struct atmel_hlcdc_formats atmel_hlcdc_plane_rgb_formats;

Re: [PATCH 5/9] drm: atmel-hlcdc: add compatible string check for XLCDC and HLCDC

[Manikandan.M]

On 14/06/23 12:19, Claudiu Beznea - M18063 wrote:
> On 13.06.2023 10:04, Manikandan Muralidharan wrote:
>> From: Durai Manickam KR <durai.manickamkr@microchip.com>
>>
>> Add compatible string check to differentiate XLCDC and HLCDC code
>> within the atmel-hlcdc driver files.
>>
>> Signed-off-by: Durai Manickam KR <durai.manickamkr@microchip.com>
>> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
>> ---
>>   drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 7 +++++++
>>   drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 1 +
>>   2 files changed, 8 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> index d7ad828e9e8c..fbbd2592efc7 100644
>> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> @@ -761,6 +761,13 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev)
>>   	if (!dc)
>>   		return -ENOMEM;
>>   
>> +	/* SAM9X7 supports XLCDC */
>> +	if (!strcmp(match->compatible, "microchip,sam9x7-xlcdc"))
> 
> This could be avoided if ix_xlcd in added in driver data.
Sure, I will check for the feasibility of the code.
Thank you
> 
>> +		dc->is_xlcdc = true;
>> +	else
>> +		/* Other SoC's that supports HLCDC IP */
>> +		dc->is_xlcdc = false;
>> +
>>   	dc->desc = match->data;
>>   	dc->hlcdc = dev_get_drvdata(dev->dev->parent);
>>   	dev->dev_private = dc;
>> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
>> index aed1742b3665..804e4d476f2b 100644
>> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
>> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
>> @@ -451,6 +451,7 @@ struct atmel_hlcdc_dc {
>>   		u32 imr;
>>   		struct drm_atomic_state *state;
>>   	} suspend;
>> +	bool is_xlcdc;
>>   };
>>   
>>   extern struct atmel_hlcdc_formats atmel_hlcdc_plane_rgb_formats;
> 

-- 
Thanks and Regards,
Manikandan M.

[PATCH 6/9] drm: atmel_hlcdc: Add support for XLCDC in atmel LCD driver

[Manikandan Muralidharan]

- XLCDC in SAM9X7 has different sets of registers and additional
configuration bits when compared to previous HLCDC IP. Read/write
operation on the controller registers is now separated using the
XLCDC status flag.
	- HEO scaling, window resampling, Alpha blending, YUV-to-RGB
conversion in XLCDC is derived and handled using additional
configuration bits and registers.
	- Writing one to the Enable fields of each layer in LCD_ATTRE
is required to reflect the values set in Configuration, FBA, Enable
registers of each layer

Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
[Hari.PrasathGE@microchip.com: update the attribute field for each layer]
Signed-off-by: Hari Prasath Gujulan Elango <Hari.PrasathGE@microchip.com>
[durai.manickamkr@microchip.com: implement status flag to seprate register update]
Signed-off-by: Durai Manickam KR <durai.manickamkr@microchip.com>
---
 .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    |  28 +-
 .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   | 297 ++++++++++++++----
 2 files changed, 256 insertions(+), 69 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index 58184cd6ab0b..7c9cf7c0c75d 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -139,10 +139,10 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
 	state = drm_crtc_state_to_atmel_hlcdc_crtc_state(c->state);
 	cfg = state->output_mode << 8;
 
-	if (adj->flags & DRM_MODE_FLAG_NVSYNC)
+	if (!crtc->dc->is_xlcdc && (adj->flags & DRM_MODE_FLAG_NVSYNC))
 		cfg |= ATMEL_HLCDC_VSPOL;
 
-	if (adj->flags & DRM_MODE_FLAG_NHSYNC)
+	if (!crtc->dc->is_xlcdc && (adj->flags & DRM_MODE_FLAG_NHSYNC))
 		cfg |= ATMEL_HLCDC_HSPOL;
 
 	regmap_update_bits(regmap, ATMEL_HLCDC_CFG(5),
@@ -177,6 +177,18 @@ static void atmel_hlcdc_crtc_atomic_disable(struct drm_crtc *c,
 
 	pm_runtime_get_sync(dev->dev);
 
+	if (crtc->dc->is_xlcdc) {
+		regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_XLCDC_CM);
+		while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
+		       (status & ATMEL_XLCDC_CM))
+			cpu_relax();
+
+		regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_XLCDC_SD);
+		while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
+		       !(status & ATMEL_XLCDC_SD))
+			cpu_relax();
+	}
+
 	regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_HLCDC_DISP);
 	while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
 	       (status & ATMEL_HLCDC_DISP))
@@ -231,6 +243,18 @@ static void atmel_hlcdc_crtc_atomic_enable(struct drm_crtc *c,
 	       !(status & ATMEL_HLCDC_DISP))
 		cpu_relax();
 
+	if (crtc->dc->is_xlcdc) {
+		regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_XLCDC_CM);
+		while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
+		       !(status & ATMEL_XLCDC_CM))
+			cpu_relax();
+
+		regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_XLCDC_SD);
+		while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
+		       (status & ATMEL_XLCDC_SD))
+			cpu_relax();
+	}
+
 	pm_runtime_put_sync(dev->dev);
 
 }
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
index daa508504f47..fe33476818c4 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
@@ -330,11 +330,59 @@ static void atmel_hlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
 								     yfactor));
 }
 
+static void atmel_xlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
+					   struct atmel_hlcdc_plane_state *state)
+{
+	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
+	u32 xfactor, yfactor;
+
+	if (!desc->layout.scaler_config)
+		return;
+
+	if (state->crtc_w == state->src_w && state->crtc_h == state->src_h) {
+		atmel_hlcdc_layer_write_cfg(&plane->layer,
+					    desc->layout.scaler_config, 0);
+		return;
+	}
+
+	/* xfactor = round[(2^20 * XMEMSIZE)/XSIZE)] */
+	xfactor = (1048576 * state->src_w) / state->crtc_w;
+
+	/* yfactor = round[(2^20 * YMEMSIZE)/YSIZE)] */
+	yfactor = (1048576 * state->src_h) / state->crtc_h;
+
+	atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config,
+				    ATMEL_XLCDC_LAYER_VSCALER_LUMA_ENABLE |
+				    ATMEL_XLCDC_LAYER_VSCALER_CHROMA_ENABLE |
+				    ATMEL_XLCDC_LAYER_HSCALER_LUMA_ENABLE |
+				    ATMEL_XLCDC_LAYER_HSCALER_CHROMA_ENABLE);
+
+	atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 1,
+				    yfactor);
+	atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 3,
+				    xfactor);
+
+	/* As per YCbCr window resampling configuration */
+	if (state->base.fb->format->format == DRM_FORMAT_YUV420) {
+		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 2,
+					    yfactor / 2);
+		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 4,
+					    xfactor / 2);
+	} else {
+		/* As per ARGB window resampling configuration */
+		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 2,
+					    yfactor);
+		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 4,
+					    xfactor);
+	}
+}
+
 static void
 atmel_hlcdc_plane_update_pos_and_size(struct atmel_hlcdc_plane *plane,
 				      struct atmel_hlcdc_plane_state *state)
 {
 	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
+	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
 
 	if (desc->layout.size)
 		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.size,
@@ -352,7 +400,10 @@ atmel_hlcdc_plane_update_pos_and_size(struct atmel_hlcdc_plane *plane,
 					ATMEL_HLCDC_LAYER_POS(state->crtc_x,
 							      state->crtc_y));
 
-	atmel_hlcdc_plane_setup_scaler(plane, state);
+	if (!(dc->is_xlcdc))
+		atmel_hlcdc_plane_setup_scaler(plane, state);
+	else
+		atmel_xlcdc_plane_setup_scaler(plane, state);
 }
 
 static void
@@ -362,33 +413,58 @@ atmel_hlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
 	unsigned int cfg = ATMEL_HLCDC_LAYER_DMA_BLEN_INCR16 | state->ahb_id;
 	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
 	const struct drm_format_info *format = state->base.fb->format;
-
+	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
 	/*
 	 * Rotation optimization is not working on RGB888 (rotation is still
 	 * working but without any optimization).
 	 */
-	if (format->format == DRM_FORMAT_RGB888)
+	if ((!(dc->is_xlcdc)) && format->format == DRM_FORMAT_RGB888)
 		cfg |= ATMEL_HLCDC_LAYER_DMA_ROTDIS;
 
-	atmel_hlcdc_layer_write_cfg(&plane->layer, ATMEL_HLCDC_LAYER_DMA_CFG,
-				    cfg);
+	if (!(dc->is_xlcdc)) {
+		atmel_hlcdc_layer_write_cfg(&plane->layer, ATMEL_HLCDC_LAYER_DMA_CFG,
+					    cfg);
 
-	cfg = ATMEL_HLCDC_LAYER_DMA | ATMEL_HLCDC_LAYER_REP;
+		cfg = ATMEL_HLCDC_LAYER_DMA | ATMEL_HLCDC_LAYER_REP;
+	} else {
+		atmel_hlcdc_layer_write_cfg(&plane->layer, ATMEL_XLCDC_LAYER_DMA_CFG,
+					    cfg);
+
+		cfg = ATMEL_XLCDC_LAYER_DMA | ATMEL_XLCDC_LAYER_REP;
+	}
 
 	if (plane->base.type != DRM_PLANE_TYPE_PRIMARY) {
-		cfg |= ATMEL_HLCDC_LAYER_OVR | ATMEL_HLCDC_LAYER_ITER2BL |
-		       ATMEL_HLCDC_LAYER_ITER;
+		if (!(dc->is_xlcdc)) {
+			cfg |= ATMEL_HLCDC_LAYER_OVR | ATMEL_HLCDC_LAYER_ITER2BL |
+			       ATMEL_HLCDC_LAYER_ITER;
+
+			if (format->has_alpha)
+				cfg |= ATMEL_HLCDC_LAYER_LAEN;
+			else
+				cfg |= ATMEL_HLCDC_LAYER_GAEN |
+				       ATMEL_HLCDC_LAYER_GA(state->base.alpha);
+		} else {
+			/*
+			 * Alpha Blending bits specific to SAM9X7 SoC
+			 */
+			cfg |= ATMEL_XLCDC_LAYER_SFACTC_A0_MULT_AS |
+			       ATMEL_XLCDC_LAYER_SFACTA_ONE |
+			       ATMEL_XLCDC_LAYER_DFACTC_M_A0_MULT_AS |
+			       ATMEL_XLCDC_LAYER_DFACTA_ONE;
+			if (format->has_alpha)
+				cfg |= ATMEL_XLCDC_LAYER_A0(0xff);
+			else
+				cfg |= ATMEL_XLCDC_LAYER_A0(state->base.alpha);
+		}
+	}
 
-		if (format->has_alpha)
-			cfg |= ATMEL_HLCDC_LAYER_LAEN;
+	if (state->disc_h && state->disc_w) {
+		if (!(dc->is_xlcdc))
+			cfg |= ATMEL_HLCDC_LAYER_DISCEN;
 		else
-			cfg |= ATMEL_HLCDC_LAYER_GAEN |
-			       ATMEL_HLCDC_LAYER_GA(state->base.alpha);
+			cfg |= ATMEL_XLCDC_LAYER_DISCEN;
 	}
 
-	if (state->disc_h && state->disc_w)
-		cfg |= ATMEL_HLCDC_LAYER_DISCEN;
-
 	atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.general_config,
 				    cfg);
 }
@@ -441,33 +517,42 @@ static void atmel_hlcdc_plane_update_buffers(struct atmel_hlcdc_plane *plane,
 					struct atmel_hlcdc_plane_state *state)
 {
 	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
+	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
 	struct drm_framebuffer *fb = state->base.fb;
 	u32 sr;
 	int i;
 
-	sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
+	if (!(dc->is_xlcdc))
+		sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
+	else
+		sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_XLCDC_LAYER_ENR);
 
 	for (i = 0; i < state->nplanes; i++) {
 		struct drm_gem_dma_object *gem = drm_fb_dma_get_gem_obj(fb, i);
 
 		state->dscrs[i]->addr = gem->dma_addr + state->offsets[i];
 
-		atmel_hlcdc_layer_write_reg(&plane->layer,
-					    ATMEL_HLCDC_LAYER_PLANE_HEAD(i),
-					    state->dscrs[i]->self);
-
-		if (!(sr & ATMEL_HLCDC_LAYER_EN)) {
-			atmel_hlcdc_layer_write_reg(&plane->layer,
-					ATMEL_HLCDC_LAYER_PLANE_ADDR(i),
-					state->dscrs[i]->addr);
+		if (!(dc->is_xlcdc)) {
 			atmel_hlcdc_layer_write_reg(&plane->layer,
-					ATMEL_HLCDC_LAYER_PLANE_CTRL(i),
-					state->dscrs[i]->ctrl);
+						    ATMEL_HLCDC_LAYER_PLANE_HEAD(i),
+						    state->dscrs[i]->self);
+
+			if (!(sr & ATMEL_HLCDC_LAYER_EN)) {
+				atmel_hlcdc_layer_write_reg(&plane->layer,
+							    ATMEL_HLCDC_LAYER_PLANE_ADDR(i),
+							    state->dscrs[i]->addr);
+				atmel_hlcdc_layer_write_reg(&plane->layer,
+							    ATMEL_HLCDC_LAYER_PLANE_CTRL(i),
+							    state->dscrs[i]->ctrl);
+				atmel_hlcdc_layer_write_reg(&plane->layer,
+							    ATMEL_HLCDC_LAYER_PLANE_NEXT(i),
+							    state->dscrs[i]->self);
+			}
+		} else {
 			atmel_hlcdc_layer_write_reg(&plane->layer,
-					ATMEL_HLCDC_LAYER_PLANE_NEXT(i),
-					state->dscrs[i]->self);
+						    ATMEL_XLCDC_LAYER_PLANE_ADDR(i),
+						    state->dscrs[i]->addr);
 		}
-
 		if (desc->layout.xstride[i])
 			atmel_hlcdc_layer_write_cfg(&plane->layer,
 						    desc->layout.xstride[i],
@@ -716,19 +801,31 @@ static void atmel_hlcdc_plane_atomic_disable(struct drm_plane *p,
 					     struct drm_atomic_state *state)
 {
 	struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
-
+	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
 	/* Disable interrupts */
-	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IDR,
-				    0xffffffff);
+	if (!(dc->is_xlcdc))
+		atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IDR,
+					    0xffffffff);
+	else
+		atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_IDR,
+					    0xffffffff);
 
 	/* Disable the layer */
-	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHDR,
-				    ATMEL_HLCDC_LAYER_RST |
-				    ATMEL_HLCDC_LAYER_A2Q |
-				    ATMEL_HLCDC_LAYER_UPDATE);
+	if (!(dc->is_xlcdc))
+		atmel_hlcdc_layer_write_reg(&plane->layer,
+					    ATMEL_HLCDC_LAYER_CHDR,
+					    ATMEL_HLCDC_LAYER_RST |
+					    ATMEL_HLCDC_LAYER_A2Q |
+					    ATMEL_HLCDC_LAYER_UPDATE);
+	else
+		atmel_hlcdc_layer_write_reg(&plane->layer,
+					    ATMEL_XLCDC_LAYER_ENR, 0);
 
 	/* Clear all pending interrupts */
-	atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
+	if (!(dc->is_xlcdc))
+		atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
+	else
+		atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_XLCDC_LAYER_ISR);
 }
 
 static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
@@ -739,6 +836,7 @@ static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
 	struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
 	struct atmel_hlcdc_plane_state *hstate =
 			drm_plane_state_to_atmel_hlcdc_plane_state(new_s);
+	struct atmel_hlcdc_dc *dc = p->dev->dev_private;
 	u32 sr;
 
 	if (!new_s->crtc || !new_s->fb)
@@ -756,23 +854,46 @@ static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
 	atmel_hlcdc_plane_update_buffers(plane, hstate);
 	atmel_hlcdc_plane_update_disc_area(plane, hstate);
 
-	/* Enable the overrun interrupts. */
-	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IER,
-				    ATMEL_HLCDC_LAYER_OVR_IRQ(0) |
-				    ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
-				    ATMEL_HLCDC_LAYER_OVR_IRQ(2));
-
-	/* Apply the new config at the next SOF event. */
-	sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
-	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHER,
-			ATMEL_HLCDC_LAYER_UPDATE |
-			(sr & ATMEL_HLCDC_LAYER_EN ?
-			 ATMEL_HLCDC_LAYER_A2Q : ATMEL_HLCDC_LAYER_EN));
+	if (!(dc->is_xlcdc)) {
+		/* Enable the overrun interrupts. */
+		atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IER,
+					    ATMEL_HLCDC_LAYER_OVR_IRQ(0) |
+					    ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
+					    ATMEL_HLCDC_LAYER_OVR_IRQ(2));
+
+		/* Apply the new config at the next SOF event. */
+		sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
+		atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHER,
+					    ATMEL_HLCDC_LAYER_UPDATE |
+					    (sr & ATMEL_HLCDC_LAYER_EN ?
+					    ATMEL_HLCDC_LAYER_A2Q : ATMEL_HLCDC_LAYER_EN));
+	} else {
+		/* Enable the overrun interrupts. */
+		atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_IER,
+					    ATMEL_XLCDC_LAYER_OVR_IRQ(0) |
+					    ATMEL_XLCDC_LAYER_OVR_IRQ(1) |
+					    ATMEL_XLCDC_LAYER_OVR_IRQ(2));
+
+		atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_ENR,
+					    ATMEL_XLCDC_LAYER_EN);
+	}
+
+	/*
+	 * Updating XLCDC_xxxCFGx, XLCDC_xxxFBA and XLCDC_xxxEN,
+	 * (where xxx indicates each layer) requires writing one to the
+	 * Update Attribute field for each layer in LCDC_ATTRE register for SAM9X7.
+	 */
+	if (dc->is_xlcdc) {
+		regmap_write(dc->hlcdc->regmap, ATMEL_XLCDC_ATTRE, ATMEL_XLCDC_BASE_UPDATE |
+			     ATMEL_XLCDC_OVR1_UPDATE | ATMEL_XLCDC_OVR3_UPDATE |
+			     ATMEL_XLCDC_HEO_UPDATE);
+	}
 }
 
 static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane)
 {
 	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
+	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
 
 	if (desc->type == ATMEL_HLCDC_OVERLAY_LAYER ||
 	    desc->type == ATMEL_HLCDC_CURSOR_LAYER) {
@@ -796,20 +917,50 @@ static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane)
 			return ret;
 	}
 
-	if (desc->layout.csc) {
+	if (!(dc->is_xlcdc)) {
+		if (desc->layout.csc) {
+			/*
+			 * TODO: decare a "yuv-to-rgb-conv-factors" property to let
+			 * userspace modify these factors (using a BLOB property ?).
+			 */
+			atmel_hlcdc_layer_write_cfg(&plane->layer,
+						    desc->layout.csc,
+						    0x4c900091);
+			atmel_hlcdc_layer_write_cfg(&plane->layer,
+						    desc->layout.csc + 1,
+						    0x7a5f5090);
+			atmel_hlcdc_layer_write_cfg(&plane->layer,
+						    desc->layout.csc + 2,
+						    0x40040890);
+		}
+	} else {
 		/*
-		 * TODO: decare a "yuv-to-rgb-conv-factors" property to let
-		 * userspace modify these factors (using a BLOB property ?).
+		 * yuv-to-rgb-conv-factors are now defined from LCDC_HEOCFG16 to
+		 * LCDC_HEOCFG21 registers in SAM9X7.
 		 */
-		atmel_hlcdc_layer_write_cfg(&plane->layer,
-					    desc->layout.csc,
-					    0x4c900091);
-		atmel_hlcdc_layer_write_cfg(&plane->layer,
-					    desc->layout.csc + 1,
-					    0x7a5f5090);
-		atmel_hlcdc_layer_write_cfg(&plane->layer,
-					    desc->layout.csc + 2,
-					    0x40040890);
+		if (desc->layout.csc) {
+			atmel_hlcdc_layer_write_cfg(&plane->layer,
+						    desc->layout.csc,
+						    0x00000488);
+			atmel_hlcdc_layer_write_cfg(&plane->layer,
+						    desc->layout.csc + 1,
+						    0x00000648);
+			atmel_hlcdc_layer_write_cfg(&plane->layer,
+						    desc->layout.csc + 2,
+						    0x1EA00480);
+			atmel_hlcdc_layer_write_cfg(&plane->layer,
+						    desc->layout.csc + 3,
+						    0x00001D28);
+			atmel_hlcdc_layer_write_cfg(&plane->layer,
+						    desc->layout.csc + 4,
+						    0x08100480);
+			atmel_hlcdc_layer_write_cfg(&plane->layer,
+						    desc->layout.csc + 5,
+						    0x00000000);
+			atmel_hlcdc_layer_write_cfg(&plane->layer,
+						    desc->layout.csc + 6,
+						    0x00000007);
+		}
 	}
 
 	return 0;
@@ -819,19 +970,31 @@ void atmel_hlcdc_plane_irq(struct atmel_hlcdc_plane *plane)
 {
 	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
 	u32 isr;
+	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
 
-	isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
+	if (!(dc->is_xlcdc))
+		isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
+	else
+		isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_XLCDC_LAYER_ISR);
 
 	/*
 	 * There's not much we can do in case of overrun except informing
 	 * the user. However, we are in interrupt context here, hence the
 	 * use of dev_dbg().
 	 */
-	if (isr &
-	    (ATMEL_HLCDC_LAYER_OVR_IRQ(0) | ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
-	     ATMEL_HLCDC_LAYER_OVR_IRQ(2)))
-		dev_dbg(plane->base.dev->dev, "overrun on plane %s\n",
-			desc->name);
+	if (!(dc->is_xlcdc)) {
+		if (isr &
+		    (ATMEL_HLCDC_LAYER_OVR_IRQ(0) | ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
+		     ATMEL_HLCDC_LAYER_OVR_IRQ(2)))
+			dev_dbg(plane->base.dev->dev, "overrun on plane %s\n",
+				desc->name);
+	} else {
+		if (isr &
+		    (ATMEL_XLCDC_LAYER_OVR_IRQ(0) | ATMEL_XLCDC_LAYER_OVR_IRQ(1) |
+		     ATMEL_XLCDC_LAYER_OVR_IRQ(2)))
+			dev_dbg(plane->base.dev->dev, "overrun on plane %s\n",
+				desc->name);
+	}
 }
 
 static const struct drm_plane_helper_funcs atmel_hlcdc_layer_plane_helper_funcs = {
-- 
2.25.1

Re: [PATCH 6/9] drm: atmel_hlcdc: Add support for XLCDC in atmel LCD driver

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1032 bytes --]

On Tue, Jun 13, 2023 at 12:34:23PM +0530, Manikandan Muralidharan wrote:
> - XLCDC in SAM9X7 has different sets of registers and additional
> configuration bits when compared to previous HLCDC IP. Read/write
> operation on the controller registers is now separated using the
> XLCDC status flag.
> 	- HEO scaling, window resampling, Alpha blending, YUV-to-RGB
> conversion in XLCDC is derived and handled using additional
> configuration bits and registers.
> 	- Writing one to the Enable fields of each layer in LCD_ATTRE
> is required to reflect the values set in Configuration, FBA, Enable
> registers of each layer
> 
> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> [Hari.PrasathGE@microchip.com: update the attribute field for each layer]
> Signed-off-by: Hari Prasath Gujulan Elango <Hari.PrasathGE@microchip.com>
> [durai.manickamkr@microchip.com: implement status flag to seprate register update]

These things inside [] look suspiciously like they should be
co-developed-bys...


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 6/9] drm: atmel_hlcdc: Add support for XLCDC in atmel LCD driver

[Manikandan.M]

On 13/06/23 23:47, Conor Dooley wrote:
> On Tue, Jun 13, 2023 at 12:34:23PM +0530, Manikandan Muralidharan wrote:
>> - XLCDC in SAM9X7 has different sets of registers and additional
>> configuration bits when compared to previous HLCDC IP. Read/write
>> operation on the controller registers is now separated using the
>> XLCDC status flag.
>> 	- HEO scaling, window resampling, Alpha blending, YUV-to-RGB
>> conversion in XLCDC is derived and handled using additional
>> configuration bits and registers.
>> 	- Writing one to the Enable fields of each layer in LCD_ATTRE
>> is required to reflect the values set in Configuration, FBA, Enable
>> registers of each layer
>>
>> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
>> [Hari.PrasathGE@microchip.com: update the attribute field for each layer]
>> Signed-off-by: Hari Prasath Gujulan Elango <Hari.PrasathGE@microchip.com>
>> [durai.manickamkr@microchip.com: implement status flag to seprate register update]
> 
> These things inside [] look suspiciously like they should be
> co-developed-bys...
Ok, I will update as suggested.
> 

-- 
Thanks and Regards,
Manikandan M.

Re: [PATCH 6/9] drm: atmel_hlcdc: Add support for XLCDC in atmel LCD driver

[Claudiu.Beznea]

On 13.06.2023 10:04, Manikandan Muralidharan wrote:
> - XLCDC in SAM9X7 has different sets of registers and additional
> configuration bits when compared to previous HLCDC IP. Read/write
> operation on the controller registers is now separated using the
> XLCDC status flag.
> 	- HEO scaling, window resampling, Alpha blending, YUV-to-RGB
> conversion in XLCDC is derived and handled using additional
> configuration bits and registers.
> 	- Writing one to the Enable fields of each layer in LCD_ATTRE
> is required to reflect the values set in Configuration, FBA, Enable
> registers of each layer
> 
> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> [Hari.PrasathGE@microchip.com: update the attribute field for each layer]
> Signed-off-by: Hari Prasath Gujulan Elango <Hari.PrasathGE@microchip.com>
> [durai.manickamkr@microchip.com: implement status flag to seprate register update]
> Signed-off-by: Durai Manickam KR <durai.manickamkr@microchip.com>
> ---
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    |  28 +-
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   | 297 ++++++++++++++----
>  2 files changed, 256 insertions(+), 69 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index 58184cd6ab0b..7c9cf7c0c75d 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -139,10 +139,10 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
>  	state = drm_crtc_state_to_atmel_hlcdc_crtc_state(c->state);
>  	cfg = state->output_mode << 8;
>  
> -	if (adj->flags & DRM_MODE_FLAG_NVSYNC)
> +	if (!crtc->dc->is_xlcdc && (adj->flags & DRM_MODE_FLAG_NVSYNC))
>  		cfg |= ATMEL_HLCDC_VSPOL;
>  
> -	if (adj->flags & DRM_MODE_FLAG_NHSYNC)
> +	if (!crtc->dc->is_xlcdc && (adj->flags & DRM_MODE_FLAG_NHSYNC))
>  		cfg |= ATMEL_HLCDC_HSPOL;
>  
>  	regmap_update_bits(regmap, ATMEL_HLCDC_CFG(5),
> @@ -177,6 +177,18 @@ static void atmel_hlcdc_crtc_atomic_disable(struct drm_crtc *c,
>  
>  	pm_runtime_get_sync(dev->dev);
>  
> +	if (crtc->dc->is_xlcdc) {
> +		regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_XLCDC_CM);
> +		while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
> +		       (status & ATMEL_XLCDC_CM))
> +			cpu_relax();

A timeout would be god here to avoid potential infinite loop (for different
reasons). You can check regmap_read_poll_timeout(). Same for the rest of
while loops above.

> +
> +		regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_XLCDC_SD);
> +		while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
> +		       !(status & ATMEL_XLCDC_SD))
> +			cpu_relax();
> +	}
> +
>  	regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_HLCDC_DISP);
>  	while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
>  	       (status & ATMEL_HLCDC_DISP))
> @@ -231,6 +243,18 @@ static void atmel_hlcdc_crtc_atomic_enable(struct drm_crtc *c,
>  	       !(status & ATMEL_HLCDC_DISP))
>  		cpu_relax();
>  
> +	if (crtc->dc->is_xlcdc) {
> +		regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_XLCDC_CM);
> +		while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
> +		       !(status & ATMEL_XLCDC_CM))
> +			cpu_relax();
> +
> +		regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_XLCDC_SD);
> +		while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
> +		       (status & ATMEL_XLCDC_SD))
> +			cpu_relax();
> +	}
> +
>  	pm_runtime_put_sync(dev->dev);
>  
>  }
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> index daa508504f47..fe33476818c4 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> @@ -330,11 +330,59 @@ static void atmel_hlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
>  								     yfactor));
>  }
>  
> +static void atmel_xlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
> +					   struct atmel_hlcdc_plane_state *state)
> +{
> +	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
> +	u32 xfactor, yfactor;
> +
> +	if (!desc->layout.scaler_config)
> +		return;
> +
> +	if (state->crtc_w == state->src_w && state->crtc_h == state->src_h) {
> +		atmel_hlcdc_layer_write_cfg(&plane->layer,
> +					    desc->layout.scaler_config, 0);
> +		return;
> +	}
> +
> +	/* xfactor = round[(2^20 * XMEMSIZE)/XSIZE)] */
> +	xfactor = (1048576 * state->src_w) / state->crtc_w;
> +
> +	/* yfactor = round[(2^20 * YMEMSIZE)/YSIZE)] */
> +	yfactor = (1048576 * state->src_h) / state->crtc_h;
> +
> +	atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config,
> +				    ATMEL_XLCDC_LAYER_VSCALER_LUMA_ENABLE |
> +				    ATMEL_XLCDC_LAYER_VSCALER_CHROMA_ENABLE |
> +				    ATMEL_XLCDC_LAYER_HSCALER_LUMA_ENABLE |
> +				    ATMEL_XLCDC_LAYER_HSCALER_CHROMA_ENABLE);
> +
> +	atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 1,
> +				    yfactor);
> +	atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 3,
> +				    xfactor);
> +
> +	/* As per YCbCr window resampling configuration */
> +	if (state->base.fb->format->format == DRM_FORMAT_YUV420) {
> +		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 2,
> +					    yfactor / 2);
> +		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 4,
> +					    xfactor / 2);
> +	} else {
> +		/* As per ARGB window resampling configuration */
> +		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 2,
> +					    yfactor);
> +		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 4,
> +					    xfactor);
> +	}
> +}
> +
>  static void
>  atmel_hlcdc_plane_update_pos_and_size(struct atmel_hlcdc_plane *plane,
>  				      struct atmel_hlcdc_plane_state *state)
>  {
>  	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
> +	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>  
>  	if (desc->layout.size)
>  		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.size,
> @@ -352,7 +400,10 @@ atmel_hlcdc_plane_update_pos_and_size(struct atmel_hlcdc_plane *plane,
>  					ATMEL_HLCDC_LAYER_POS(state->crtc_x,
>  							      state->crtc_y));
>  
> -	atmel_hlcdc_plane_setup_scaler(plane, state);
> +	if (!(dc->is_xlcdc))

You can reverse the logic here to avoid !dc->is_xlcdc. Also, no need for ()
around dc->is_xlcdc

> +		atmel_hlcdc_plane_setup_scaler(plane, state);
> +	else
> +		atmel_xlcdc_plane_setup_scaler(plane, state);
>  }
>  
>  static void
> @@ -362,33 +413,58 @@ atmel_hlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
>  	unsigned int cfg = ATMEL_HLCDC_LAYER_DMA_BLEN_INCR16 | state->ahb_id;
>  	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
>  	const struct drm_format_info *format = state->base.fb->format;
> -

extra line

> +	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>  	/*
>  	 * Rotation optimization is not working on RGB888 (rotation is still
>  	 * working but without any optimization).
>  	 */
> -	if (format->format == DRM_FORMAT_RGB888)
> +	if ((!(dc->is_xlcdc)) && format->format == DRM_FORMAT_RGB888)

No need for extra ().

>  		cfg |= ATMEL_HLCDC_LAYER_DMA_ROTDIS;
>  
> -	atmel_hlcdc_layer_write_cfg(&plane->layer, ATMEL_HLCDC_LAYER_DMA_CFG,
> -				    cfg);
> +	if (!(dc->is_xlcdc)) {

no need for extra ()

> +		atmel_hlcdc_layer_write_cfg(&plane->layer, ATMEL_HLCDC_LAYER_DMA_CFG,
> +					    cfg);
>  
> -	cfg = ATMEL_HLCDC_LAYER_DMA | ATMEL_HLCDC_LAYER_REP;
> +		cfg = ATMEL_HLCDC_LAYER_DMA | ATMEL_HLCDC_LAYER_REP;
> +	} else {
> +		atmel_hlcdc_layer_write_cfg(&plane->layer, ATMEL_XLCDC_LAYER_DMA_CFG,
> +					    cfg);
> +
> +		cfg = ATMEL_XLCDC_LAYER_DMA | ATMEL_XLCDC_LAYER_REP;
> +	}
>  
>  	if (plane->base.type != DRM_PLANE_TYPE_PRIMARY) {
> -		cfg |= ATMEL_HLCDC_LAYER_OVR | ATMEL_HLCDC_LAYER_ITER2BL |
> -		       ATMEL_HLCDC_LAYER_ITER;
> +		if (!(dc->is_xlcdc)) {

no need for extra (). Same for the rest of occurencies above. Also, you can
reverse the logic to avoid ! in front of (dc->is_xlcdc)

> +			cfg |= ATMEL_HLCDC_LAYER_OVR | ATMEL_HLCDC_LAYER_ITER2BL |
> +			       ATMEL_HLCDC_LAYER_ITER;
> +
> +			if (format->has_alpha)
> +				cfg |= ATMEL_HLCDC_LAYER_LAEN;
> +			else
> +				cfg |= ATMEL_HLCDC_LAYER_GAEN |
> +				       ATMEL_HLCDC_LAYER_GA(state->base.alpha);
> +		} else {
> +			/*
> +			 * Alpha Blending bits specific to SAM9X7 SoC
> +			 */
> +			cfg |= ATMEL_XLCDC_LAYER_SFACTC_A0_MULT_AS |
> +			       ATMEL_XLCDC_LAYER_SFACTA_ONE |
> +			       ATMEL_XLCDC_LAYER_DFACTC_M_A0_MULT_AS |
> +			       ATMEL_XLCDC_LAYER_DFACTA_ONE;
> +			if (format->has_alpha)
> +				cfg |= ATMEL_XLCDC_LAYER_A0(0xff);
> +			else
> +				cfg |= ATMEL_XLCDC_LAYER_A0(state->base.alpha);
> +		}
> +	}
>  
> -		if (format->has_alpha)
> -			cfg |= ATMEL_HLCDC_LAYER_LAEN;
> +	if (state->disc_h && state->disc_w) {
> +		if (!(dc->is_xlcdc))
> +			cfg |= ATMEL_HLCDC_LAYER_DISCEN;
>  		else
> -			cfg |= ATMEL_HLCDC_LAYER_GAEN |
> -			       ATMEL_HLCDC_LAYER_GA(state->base.alpha);
> +			cfg |= ATMEL_XLCDC_LAYER_DISCEN;
>  	}
>  
> -	if (state->disc_h && state->disc_w)
> -		cfg |= ATMEL_HLCDC_LAYER_DISCEN;
> -
>  	atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.general_config,
>  				    cfg);
>  }
> @@ -441,33 +517,42 @@ static void atmel_hlcdc_plane_update_buffers(struct atmel_hlcdc_plane *plane,
>  					struct atmel_hlcdc_plane_state *state)
>  {
>  	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
> +	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>  	struct drm_framebuffer *fb = state->base.fb;
>  	u32 sr;
>  	int i;
>  
> -	sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
> +	if (!(dc->is_xlcdc))

You can reverse the logic

> +		sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
> +	else
> +		sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_XLCDC_LAYER_ENR);
>  
>  	for (i = 0; i < state->nplanes; i++) {
>  		struct drm_gem_dma_object *gem = drm_fb_dma_get_gem_obj(fb, i);
>  
>  		state->dscrs[i]->addr = gem->dma_addr + state->offsets[i];
>  
> -		atmel_hlcdc_layer_write_reg(&plane->layer,
> -					    ATMEL_HLCDC_LAYER_PLANE_HEAD(i),
> -					    state->dscrs[i]->self);
> -
> -		if (!(sr & ATMEL_HLCDC_LAYER_EN)) {
> -			atmel_hlcdc_layer_write_reg(&plane->layer,
> -					ATMEL_HLCDC_LAYER_PLANE_ADDR(i),
> -					state->dscrs[i]->addr);
> +		if (!(dc->is_xlcdc)) {

you can reverse the logic

>  			atmel_hlcdc_layer_write_reg(&plane->layer,
> -					ATMEL_HLCDC_LAYER_PLANE_CTRL(i),
> -					state->dscrs[i]->ctrl);
> +						    ATMEL_HLCDC_LAYER_PLANE_HEAD(i),
> +						    state->dscrs[i]->self);
> +
> +			if (!(sr & ATMEL_HLCDC_LAYER_EN)) {
> +				atmel_hlcdc_layer_write_reg(&plane->layer,
> +							    ATMEL_HLCDC_LAYER_PLANE_ADDR(i),
> +							    state->dscrs[i]->addr);
> +				atmel_hlcdc_layer_write_reg(&plane->layer,
> +							    ATMEL_HLCDC_LAYER_PLANE_CTRL(i),
> +							    state->dscrs[i]->ctrl);
> +				atmel_hlcdc_layer_write_reg(&plane->layer,
> +							    ATMEL_HLCDC_LAYER_PLANE_NEXT(i),
> +							    state->dscrs[i]->self);
> +			}
> +		} else {
>  			atmel_hlcdc_layer_write_reg(&plane->layer,
> -					ATMEL_HLCDC_LAYER_PLANE_NEXT(i),
> -					state->dscrs[i]->self);
> +						    ATMEL_XLCDC_LAYER_PLANE_ADDR(i),
> +						    state->dscrs[i]->addr);
>  		}
> -
>  		if (desc->layout.xstride[i])
>  			atmel_hlcdc_layer_write_cfg(&plane->layer,
>  						    desc->layout.xstride[i],
> @@ -716,19 +801,31 @@ static void atmel_hlcdc_plane_atomic_disable(struct drm_plane *p,
>  					     struct drm_atomic_state *state)
>  {
>  	struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
> -

You can keep this line afte the above variable.

> +	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>  	/* Disable interrupts */
> -	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IDR,
> -				    0xffffffff);
> +	if (!(dc->is_xlcdc))

you can reverse the logic

> +		atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IDR,
> +					    0xffffffff);
> +	else
> +		atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_IDR,
> +					    0xffffffff);
>  
>  	/* Disable the layer */
> -	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHDR,
> -				    ATMEL_HLCDC_LAYER_RST |
> -				    ATMEL_HLCDC_LAYER_A2Q |
> -				    ATMEL_HLCDC_LAYER_UPDATE);
> +	if (!(dc->is_xlcdc))

same here.

> +		atmel_hlcdc_layer_write_reg(&plane->layer,
> +					    ATMEL_HLCDC_LAYER_CHDR,
> +					    ATMEL_HLCDC_LAYER_RST |
> +					    ATMEL_HLCDC_LAYER_A2Q |
> +					    ATMEL_HLCDC_LAYER_UPDATE);
> +	else
> +		atmel_hlcdc_layer_write_reg(&plane->layer,
> +					    ATMEL_XLCDC_LAYER_ENR, 0);
>  
>  	/* Clear all pending interrupts */
> -	atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
> +	if (!(dc->is_xlcdc))
> +		atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
> +	else
> +		atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_XLCDC_LAYER_ISR);
>  }
>  
>  static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
> @@ -739,6 +836,7 @@ static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
>  	struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
>  	struct atmel_hlcdc_plane_state *hstate =
>  			drm_plane_state_to_atmel_hlcdc_plane_state(new_s);
> +	struct atmel_hlcdc_dc *dc = p->dev->dev_private;
>  	u32 sr;
>  
>  	if (!new_s->crtc || !new_s->fb)
> @@ -756,23 +854,46 @@ static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
>  	atmel_hlcdc_plane_update_buffers(plane, hstate);
>  	atmel_hlcdc_plane_update_disc_area(plane, hstate);
>  
> -	/* Enable the overrun interrupts. */
> -	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IER,
> -				    ATMEL_HLCDC_LAYER_OVR_IRQ(0) |
> -				    ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
> -				    ATMEL_HLCDC_LAYER_OVR_IRQ(2));
> -
> -	/* Apply the new config at the next SOF event. */
> -	sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
> -	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHER,
> -			ATMEL_HLCDC_LAYER_UPDATE |
> -			(sr & ATMEL_HLCDC_LAYER_EN ?
> -			 ATMEL_HLCDC_LAYER_A2Q : ATMEL_HLCDC_LAYER_EN));
> +	if (!(dc->is_xlcdc)) {

same here

> +		/* Enable the overrun interrupts. */
> +		atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IER,
> +					    ATMEL_HLCDC_LAYER_OVR_IRQ(0) |
> +					    ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
> +					    ATMEL_HLCDC_LAYER_OVR_IRQ(2));
> +
> +		/* Apply the new config at the next SOF event. */
> +		sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
> +		atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHER,
> +					    ATMEL_HLCDC_LAYER_UPDATE |
> +					    (sr & ATMEL_HLCDC_LAYER_EN ?
> +					    ATMEL_HLCDC_LAYER_A2Q : ATMEL_HLCDC_LAYER_EN));
> +	} else {
> +		/* Enable the overrun interrupts. */
> +		atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_IER,
> +					    ATMEL_XLCDC_LAYER_OVR_IRQ(0) |
> +					    ATMEL_XLCDC_LAYER_OVR_IRQ(1) |
> +					    ATMEL_XLCDC_LAYER_OVR_IRQ(2));
> +
> +		atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_ENR,
> +					    ATMEL_XLCDC_LAYER_EN);
> +	}
> +
> +	/*
> +	 * Updating XLCDC_xxxCFGx, XLCDC_xxxFBA and XLCDC_xxxEN,
> +	 * (where xxx indicates each layer) requires writing one to the
> +	 * Update Attribute field for each layer in LCDC_ATTRE register for SAM9X7.
> +	 */
> +	if (dc->is_xlcdc) {
> +		regmap_write(dc->hlcdc->regmap, ATMEL_XLCDC_ATTRE, ATMEL_XLCDC_BASE_UPDATE |
> +			     ATMEL_XLCDC_OVR1_UPDATE | ATMEL_XLCDC_OVR3_UPDATE |
> +			     ATMEL_XLCDC_HEO_UPDATE);
> +	}
>  }
>  
>  static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane)
>  {
>  	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
> +	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>  
>  	if (desc->type == ATMEL_HLCDC_OVERLAY_LAYER ||
>  	    desc->type == ATMEL_HLCDC_CURSOR_LAYER) {
> @@ -796,20 +917,50 @@ static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane)
>  			return ret;
>  	}
>  
> -	if (desc->layout.csc) {
> +	if (!(dc->is_xlcdc)) {

you can reverse the logic

> +		if (desc->layout.csc) {
> +			/*
> +			 * TODO: decare a "yuv-to-rgb-conv-factors" property to let
> +			 * userspace modify these factors (using a BLOB property ?).
> +			 */
> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
> +						    desc->layout.csc,
> +						    0x4c900091);
> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
> +						    desc->layout.csc + 1,
> +						    0x7a5f5090);
> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
> +						    desc->layout.csc + 2,
> +						    0x40040890);
> +		}
> +	} else {
>  		/*
> -		 * TODO: decare a "yuv-to-rgb-conv-factors" property to let
> -		 * userspace modify these factors (using a BLOB property ?).
> +		 * yuv-to-rgb-conv-factors are now defined from LCDC_HEOCFG16 to
> +		 * LCDC_HEOCFG21 registers in SAM9X7.
>  		 */
> -		atmel_hlcdc_layer_write_cfg(&plane->layer,
> -					    desc->layout.csc,
> -					    0x4c900091);
> -		atmel_hlcdc_layer_write_cfg(&plane->layer,
> -					    desc->layout.csc + 1,
> -					    0x7a5f5090);
> -		atmel_hlcdc_layer_write_cfg(&plane->layer,
> -					    desc->layout.csc + 2,
> -					    0x40040890);
> +		if (desc->layout.csc) {
> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
> +						    desc->layout.csc,
> +						    0x00000488);
> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
> +						    desc->layout.csc + 1,
> +						    0x00000648);
> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
> +						    desc->layout.csc + 2,
> +						    0x1EA00480);
> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
> +						    desc->layout.csc + 3,
> +						    0x00001D28);
> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
> +						    desc->layout.csc + 4,
> +						    0x08100480);
> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
> +						    desc->layout.csc + 5,
> +						    0x00000000);
> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
> +						    desc->layout.csc + 6,
> +						    0x00000007);
> +		}
>  	}
>  
>  	return 0;
> @@ -819,19 +970,31 @@ void atmel_hlcdc_plane_irq(struct atmel_hlcdc_plane *plane)
>  {
>  	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
>  	u32 isr;
> +	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>  
> -	isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
> +	if (!(dc->is_xlcdc))

same here.

> +		isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
> +	else
> +		isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_XLCDC_LAYER_ISR);
>  
>  	/*
>  	 * There's not much we can do in case of overrun except informing
>  	 * the user. However, we are in interrupt context here, hence the
>  	 * use of dev_dbg().
>  	 */
> -	if (isr &
> -	    (ATMEL_HLCDC_LAYER_OVR_IRQ(0) | ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
> -	     ATMEL_HLCDC_LAYER_OVR_IRQ(2)))
> -		dev_dbg(plane->base.dev->dev, "overrun on plane %s\n",
> -			desc->name);
> +	if (!(dc->is_xlcdc)) {
> +		if (isr &
> +		    (ATMEL_HLCDC_LAYER_OVR_IRQ(0) | ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
> +		     ATMEL_HLCDC_LAYER_OVR_IRQ(2)))
> +			dev_dbg(plane->base.dev->dev, "overrun on plane %s\n",
> +				desc->name);
> +	} else {
> +		if (isr &
> +		    (ATMEL_XLCDC_LAYER_OVR_IRQ(0) | ATMEL_XLCDC_LAYER_OVR_IRQ(1) |
> +		     ATMEL_XLCDC_LAYER_OVR_IRQ(2)))
> +			dev_dbg(plane->base.dev->dev, "overrun on plane %s\n",
> +				desc->name);
> +	}
>  }
>  
>  static const struct drm_plane_helper_funcs atmel_hlcdc_layer_plane_helper_funcs = {

Re: [PATCH 6/9] drm: atmel_hlcdc: Add support for XLCDC in atmel LCD driver

[Manikandan.M]

On 14/06/23 12:40, Claudiu Beznea - M18063 wrote:
> On 13.06.2023 10:04, Manikandan Muralidharan wrote:
>> - XLCDC in SAM9X7 has different sets of registers and additional
>> configuration bits when compared to previous HLCDC IP. Read/write
>> operation on the controller registers is now separated using the
>> XLCDC status flag.
>> 	- HEO scaling, window resampling, Alpha blending, YUV-to-RGB
>> conversion in XLCDC is derived and handled using additional
>> configuration bits and registers.
>> 	- Writing one to the Enable fields of each layer in LCD_ATTRE
>> is required to reflect the values set in Configuration, FBA, Enable
>> registers of each layer
>>
>> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
>> [Hari.PrasathGE@microchip.com: update the attribute field for each layer]
>> Signed-off-by: Hari Prasath Gujulan Elango <Hari.PrasathGE@microchip.com>
>> [durai.manickamkr@microchip.com: implement status flag to seprate register update]
>> Signed-off-by: Durai Manickam KR <durai.manickamkr@microchip.com>
>> ---
>>   .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    |  28 +-
>>   .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   | 297 ++++++++++++++----
>>   2 files changed, 256 insertions(+), 69 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
>> index 58184cd6ab0b..7c9cf7c0c75d 100644
>> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
>> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
>> @@ -139,10 +139,10 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
>>   	state = drm_crtc_state_to_atmel_hlcdc_crtc_state(c->state);
>>   	cfg = state->output_mode << 8;
>>   
>> -	if (adj->flags & DRM_MODE_FLAG_NVSYNC)
>> +	if (!crtc->dc->is_xlcdc && (adj->flags & DRM_MODE_FLAG_NVSYNC))
>>   		cfg |= ATMEL_HLCDC_VSPOL;
>>   
>> -	if (adj->flags & DRM_MODE_FLAG_NHSYNC)
>> +	if (!crtc->dc->is_xlcdc && (adj->flags & DRM_MODE_FLAG_NHSYNC))
>>   		cfg |= ATMEL_HLCDC_HSPOL;
>>   
>>   	regmap_update_bits(regmap, ATMEL_HLCDC_CFG(5),
>> @@ -177,6 +177,18 @@ static void atmel_hlcdc_crtc_atomic_disable(struct drm_crtc *c,
>>   
>>   	pm_runtime_get_sync(dev->dev);
>>   
>> +	if (crtc->dc->is_xlcdc) {
>> +		regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_XLCDC_CM);
>> +		while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
>> +		       (status & ATMEL_XLCDC_CM))
>> +			cpu_relax();
> 
> A timeout would be god here to avoid potential infinite loop (for different
> reasons). You can check regmap_read_poll_timeout(). Same for the rest of
> while loops above.
I will address all the comments given, Thank you.
> 
>> +
>> +		regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_XLCDC_SD);
>> +		while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
>> +		       !(status & ATMEL_XLCDC_SD))
>> +			cpu_relax();
>> +	}
>> +
>>   	regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_HLCDC_DISP);
>>   	while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
>>   	       (status & ATMEL_HLCDC_DISP))
>> @@ -231,6 +243,18 @@ static void atmel_hlcdc_crtc_atomic_enable(struct drm_crtc *c,
>>   	       !(status & ATMEL_HLCDC_DISP))
>>   		cpu_relax();
>>   
>> +	if (crtc->dc->is_xlcdc) {
>> +		regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_XLCDC_CM);
>> +		while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
>> +		       !(status & ATMEL_XLCDC_CM))
>> +			cpu_relax();
>> +
>> +		regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_XLCDC_SD);
>> +		while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
>> +		       (status & ATMEL_XLCDC_SD))
>> +			cpu_relax();
>> +	}
>> +
>>   	pm_runtime_put_sync(dev->dev);
>>   
>>   }
>> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
>> index daa508504f47..fe33476818c4 100644
>> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
>> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
>> @@ -330,11 +330,59 @@ static void atmel_hlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
>>   								     yfactor));
>>   }
>>   
>> +static void atmel_xlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
>> +					   struct atmel_hlcdc_plane_state *state)
>> +{
>> +	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
>> +	u32 xfactor, yfactor;
>> +
>> +	if (!desc->layout.scaler_config)
>> +		return;
>> +
>> +	if (state->crtc_w == state->src_w && state->crtc_h == state->src_h) {
>> +		atmel_hlcdc_layer_write_cfg(&plane->layer,
>> +					    desc->layout.scaler_config, 0);
>> +		return;
>> +	}
>> +
>> +	/* xfactor = round[(2^20 * XMEMSIZE)/XSIZE)] */
>> +	xfactor = (1048576 * state->src_w) / state->crtc_w;
>> +
>> +	/* yfactor = round[(2^20 * YMEMSIZE)/YSIZE)] */
>> +	yfactor = (1048576 * state->src_h) / state->crtc_h;
>> +
>> +	atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config,
>> +				    ATMEL_XLCDC_LAYER_VSCALER_LUMA_ENABLE |
>> +				    ATMEL_XLCDC_LAYER_VSCALER_CHROMA_ENABLE |
>> +				    ATMEL_XLCDC_LAYER_HSCALER_LUMA_ENABLE |
>> +				    ATMEL_XLCDC_LAYER_HSCALER_CHROMA_ENABLE);
>> +
>> +	atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 1,
>> +				    yfactor);
>> +	atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 3,
>> +				    xfactor);
>> +
>> +	/* As per YCbCr window resampling configuration */
>> +	if (state->base.fb->format->format == DRM_FORMAT_YUV420) {
>> +		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 2,
>> +					    yfactor / 2);
>> +		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 4,
>> +					    xfactor / 2);
>> +	} else {
>> +		/* As per ARGB window resampling configuration */
>> +		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 2,
>> +					    yfactor);
>> +		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 4,
>> +					    xfactor);
>> +	}
>> +}
>> +
>>   static void
>>   atmel_hlcdc_plane_update_pos_and_size(struct atmel_hlcdc_plane *plane,
>>   				      struct atmel_hlcdc_plane_state *state)
>>   {
>>   	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
>> +	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>>   
>>   	if (desc->layout.size)
>>   		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.size,
>> @@ -352,7 +400,10 @@ atmel_hlcdc_plane_update_pos_and_size(struct atmel_hlcdc_plane *plane,
>>   					ATMEL_HLCDC_LAYER_POS(state->crtc_x,
>>   							      state->crtc_y));
>>   
>> -	atmel_hlcdc_plane_setup_scaler(plane, state);
>> +	if (!(dc->is_xlcdc))
> 
> You can reverse the logic here to avoid !dc->is_xlcdc. Also, no need for ()
> around dc->is_xlcdc
> 
>> +		atmel_hlcdc_plane_setup_scaler(plane, state);
>> +	else
>> +		atmel_xlcdc_plane_setup_scaler(plane, state);
>>   }
>>   
>>   static void
>> @@ -362,33 +413,58 @@ atmel_hlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
>>   	unsigned int cfg = ATMEL_HLCDC_LAYER_DMA_BLEN_INCR16 | state->ahb_id;
>>   	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
>>   	const struct drm_format_info *format = state->base.fb->format;
>> -
> 
> extra line
> 
>> +	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>>   	/*
>>   	 * Rotation optimization is not working on RGB888 (rotation is still
>>   	 * working but without any optimization).
>>   	 */
>> -	if (format->format == DRM_FORMAT_RGB888)
>> +	if ((!(dc->is_xlcdc)) && format->format == DRM_FORMAT_RGB888)
> 
> No need for extra ().
> 
>>   		cfg |= ATMEL_HLCDC_LAYER_DMA_ROTDIS;
>>   
>> -	atmel_hlcdc_layer_write_cfg(&plane->layer, ATMEL_HLCDC_LAYER_DMA_CFG,
>> -				    cfg);
>> +	if (!(dc->is_xlcdc)) {
> 
> no need for extra ()
> 
>> +		atmel_hlcdc_layer_write_cfg(&plane->layer, ATMEL_HLCDC_LAYER_DMA_CFG,
>> +					    cfg);
>>   
>> -	cfg = ATMEL_HLCDC_LAYER_DMA | ATMEL_HLCDC_LAYER_REP;
>> +		cfg = ATMEL_HLCDC_LAYER_DMA | ATMEL_HLCDC_LAYER_REP;
>> +	} else {
>> +		atmel_hlcdc_layer_write_cfg(&plane->layer, ATMEL_XLCDC_LAYER_DMA_CFG,
>> +					    cfg);
>> +
>> +		cfg = ATMEL_XLCDC_LAYER_DMA | ATMEL_XLCDC_LAYER_REP;
>> +	}
>>   
>>   	if (plane->base.type != DRM_PLANE_TYPE_PRIMARY) {
>> -		cfg |= ATMEL_HLCDC_LAYER_OVR | ATMEL_HLCDC_LAYER_ITER2BL |
>> -		       ATMEL_HLCDC_LAYER_ITER;
>> +		if (!(dc->is_xlcdc)) {
> 
> no need for extra (). Same for the rest of occurencies above. Also, you can
> reverse the logic to avoid ! in front of (dc->is_xlcdc)
> 
>> +			cfg |= ATMEL_HLCDC_LAYER_OVR | ATMEL_HLCDC_LAYER_ITER2BL |
>> +			       ATMEL_HLCDC_LAYER_ITER;
>> +
>> +			if (format->has_alpha)
>> +				cfg |= ATMEL_HLCDC_LAYER_LAEN;
>> +			else
>> +				cfg |= ATMEL_HLCDC_LAYER_GAEN |
>> +				       ATMEL_HLCDC_LAYER_GA(state->base.alpha);
>> +		} else {
>> +			/*
>> +			 * Alpha Blending bits specific to SAM9X7 SoC
>> +			 */
>> +			cfg |= ATMEL_XLCDC_LAYER_SFACTC_A0_MULT_AS |
>> +			       ATMEL_XLCDC_LAYER_SFACTA_ONE |
>> +			       ATMEL_XLCDC_LAYER_DFACTC_M_A0_MULT_AS |
>> +			       ATMEL_XLCDC_LAYER_DFACTA_ONE;
>> +			if (format->has_alpha)
>> +				cfg |= ATMEL_XLCDC_LAYER_A0(0xff);
>> +			else
>> +				cfg |= ATMEL_XLCDC_LAYER_A0(state->base.alpha);
>> +		}
>> +	}
>>   
>> -		if (format->has_alpha)
>> -			cfg |= ATMEL_HLCDC_LAYER_LAEN;
>> +	if (state->disc_h && state->disc_w) {
>> +		if (!(dc->is_xlcdc))
>> +			cfg |= ATMEL_HLCDC_LAYER_DISCEN;
>>   		else
>> -			cfg |= ATMEL_HLCDC_LAYER_GAEN |
>> -			       ATMEL_HLCDC_LAYER_GA(state->base.alpha);
>> +			cfg |= ATMEL_XLCDC_LAYER_DISCEN;
>>   	}
>>   
>> -	if (state->disc_h && state->disc_w)
>> -		cfg |= ATMEL_HLCDC_LAYER_DISCEN;
>> -
>>   	atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.general_config,
>>   				    cfg);
>>   }
>> @@ -441,33 +517,42 @@ static void atmel_hlcdc_plane_update_buffers(struct atmel_hlcdc_plane *plane,
>>   					struct atmel_hlcdc_plane_state *state)
>>   {
>>   	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
>> +	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>>   	struct drm_framebuffer *fb = state->base.fb;
>>   	u32 sr;
>>   	int i;
>>   
>> -	sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
>> +	if (!(dc->is_xlcdc))
> 
> You can reverse the logic
> 
>> +		sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
>> +	else
>> +		sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_XLCDC_LAYER_ENR);
>>   
>>   	for (i = 0; i < state->nplanes; i++) {
>>   		struct drm_gem_dma_object *gem = drm_fb_dma_get_gem_obj(fb, i);
>>   
>>   		state->dscrs[i]->addr = gem->dma_addr + state->offsets[i];
>>   
>> -		atmel_hlcdc_layer_write_reg(&plane->layer,
>> -					    ATMEL_HLCDC_LAYER_PLANE_HEAD(i),
>> -					    state->dscrs[i]->self);
>> -
>> -		if (!(sr & ATMEL_HLCDC_LAYER_EN)) {
>> -			atmel_hlcdc_layer_write_reg(&plane->layer,
>> -					ATMEL_HLCDC_LAYER_PLANE_ADDR(i),
>> -					state->dscrs[i]->addr);
>> +		if (!(dc->is_xlcdc)) {
> 
> you can reverse the logic
> 
>>   			atmel_hlcdc_layer_write_reg(&plane->layer,
>> -					ATMEL_HLCDC_LAYER_PLANE_CTRL(i),
>> -					state->dscrs[i]->ctrl);
>> +						    ATMEL_HLCDC_LAYER_PLANE_HEAD(i),
>> +						    state->dscrs[i]->self);
>> +
>> +			if (!(sr & ATMEL_HLCDC_LAYER_EN)) {
>> +				atmel_hlcdc_layer_write_reg(&plane->layer,
>> +							    ATMEL_HLCDC_LAYER_PLANE_ADDR(i),
>> +							    state->dscrs[i]->addr);
>> +				atmel_hlcdc_layer_write_reg(&plane->layer,
>> +							    ATMEL_HLCDC_LAYER_PLANE_CTRL(i),
>> +							    state->dscrs[i]->ctrl);
>> +				atmel_hlcdc_layer_write_reg(&plane->layer,
>> +							    ATMEL_HLCDC_LAYER_PLANE_NEXT(i),
>> +							    state->dscrs[i]->self);
>> +			}
>> +		} else {
>>   			atmel_hlcdc_layer_write_reg(&plane->layer,
>> -					ATMEL_HLCDC_LAYER_PLANE_NEXT(i),
>> -					state->dscrs[i]->self);
>> +						    ATMEL_XLCDC_LAYER_PLANE_ADDR(i),
>> +						    state->dscrs[i]->addr);
>>   		}
>> -
>>   		if (desc->layout.xstride[i])
>>   			atmel_hlcdc_layer_write_cfg(&plane->layer,
>>   						    desc->layout.xstride[i],
>> @@ -716,19 +801,31 @@ static void atmel_hlcdc_plane_atomic_disable(struct drm_plane *p,
>>   					     struct drm_atomic_state *state)
>>   {
>>   	struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
>> -
> 
> You can keep this line afte the above variable.
> 
>> +	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>>   	/* Disable interrupts */
>> -	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IDR,
>> -				    0xffffffff);
>> +	if (!(dc->is_xlcdc))
> 
> you can reverse the logic
> 
>> +		atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IDR,
>> +					    0xffffffff);
>> +	else
>> +		atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_IDR,
>> +					    0xffffffff);
>>   
>>   	/* Disable the layer */
>> -	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHDR,
>> -				    ATMEL_HLCDC_LAYER_RST |
>> -				    ATMEL_HLCDC_LAYER_A2Q |
>> -				    ATMEL_HLCDC_LAYER_UPDATE);
>> +	if (!(dc->is_xlcdc))
> 
> same here.
> 
>> +		atmel_hlcdc_layer_write_reg(&plane->layer,
>> +					    ATMEL_HLCDC_LAYER_CHDR,
>> +					    ATMEL_HLCDC_LAYER_RST |
>> +					    ATMEL_HLCDC_LAYER_A2Q |
>> +					    ATMEL_HLCDC_LAYER_UPDATE);
>> +	else
>> +		atmel_hlcdc_layer_write_reg(&plane->layer,
>> +					    ATMEL_XLCDC_LAYER_ENR, 0);
>>   
>>   	/* Clear all pending interrupts */
>> -	atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
>> +	if (!(dc->is_xlcdc))
>> +		atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
>> +	else
>> +		atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_XLCDC_LAYER_ISR);
>>   }
>>   
>>   static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
>> @@ -739,6 +836,7 @@ static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
>>   	struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
>>   	struct atmel_hlcdc_plane_state *hstate =
>>   			drm_plane_state_to_atmel_hlcdc_plane_state(new_s);
>> +	struct atmel_hlcdc_dc *dc = p->dev->dev_private;
>>   	u32 sr;
>>   
>>   	if (!new_s->crtc || !new_s->fb)
>> @@ -756,23 +854,46 @@ static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
>>   	atmel_hlcdc_plane_update_buffers(plane, hstate);
>>   	atmel_hlcdc_plane_update_disc_area(plane, hstate);
>>   
>> -	/* Enable the overrun interrupts. */
>> -	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IER,
>> -				    ATMEL_HLCDC_LAYER_OVR_IRQ(0) |
>> -				    ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
>> -				    ATMEL_HLCDC_LAYER_OVR_IRQ(2));
>> -
>> -	/* Apply the new config at the next SOF event. */
>> -	sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
>> -	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHER,
>> -			ATMEL_HLCDC_LAYER_UPDATE |
>> -			(sr & ATMEL_HLCDC_LAYER_EN ?
>> -			 ATMEL_HLCDC_LAYER_A2Q : ATMEL_HLCDC_LAYER_EN));
>> +	if (!(dc->is_xlcdc)) {
> 
> same here
> 
>> +		/* Enable the overrun interrupts. */
>> +		atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IER,
>> +					    ATMEL_HLCDC_LAYER_OVR_IRQ(0) |
>> +					    ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
>> +					    ATMEL_HLCDC_LAYER_OVR_IRQ(2));
>> +
>> +		/* Apply the new config at the next SOF event. */
>> +		sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
>> +		atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHER,
>> +					    ATMEL_HLCDC_LAYER_UPDATE |
>> +					    (sr & ATMEL_HLCDC_LAYER_EN ?
>> +					    ATMEL_HLCDC_LAYER_A2Q : ATMEL_HLCDC_LAYER_EN));
>> +	} else {
>> +		/* Enable the overrun interrupts. */
>> +		atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_IER,
>> +					    ATMEL_XLCDC_LAYER_OVR_IRQ(0) |
>> +					    ATMEL_XLCDC_LAYER_OVR_IRQ(1) |
>> +					    ATMEL_XLCDC_LAYER_OVR_IRQ(2));
>> +
>> +		atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_ENR,
>> +					    ATMEL_XLCDC_LAYER_EN);
>> +	}
>> +
>> +	/*
>> +	 * Updating XLCDC_xxxCFGx, XLCDC_xxxFBA and XLCDC_xxxEN,
>> +	 * (where xxx indicates each layer) requires writing one to the
>> +	 * Update Attribute field for each layer in LCDC_ATTRE register for SAM9X7.
>> +	 */
>> +	if (dc->is_xlcdc) {
>> +		regmap_write(dc->hlcdc->regmap, ATMEL_XLCDC_ATTRE, ATMEL_XLCDC_BASE_UPDATE |
>> +			     ATMEL_XLCDC_OVR1_UPDATE | ATMEL_XLCDC_OVR3_UPDATE |
>> +			     ATMEL_XLCDC_HEO_UPDATE);
>> +	}
>>   }
>>   
>>   static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane)
>>   {
>>   	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
>> +	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>>   
>>   	if (desc->type == ATMEL_HLCDC_OVERLAY_LAYER ||
>>   	    desc->type == ATMEL_HLCDC_CURSOR_LAYER) {
>> @@ -796,20 +917,50 @@ static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane)
>>   			return ret;
>>   	}
>>   
>> -	if (desc->layout.csc) {
>> +	if (!(dc->is_xlcdc)) {
> 
> you can reverse the logic
> 
>> +		if (desc->layout.csc) {
>> +			/*
>> +			 * TODO: decare a "yuv-to-rgb-conv-factors" property to let
>> +			 * userspace modify these factors (using a BLOB property ?).
>> +			 */
>> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
>> +						    desc->layout.csc,
>> +						    0x4c900091);
>> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
>> +						    desc->layout.csc + 1,
>> +						    0x7a5f5090);
>> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
>> +						    desc->layout.csc + 2,
>> +						    0x40040890);
>> +		}
>> +	} else {
>>   		/*
>> -		 * TODO: decare a "yuv-to-rgb-conv-factors" property to let
>> -		 * userspace modify these factors (using a BLOB property ?).
>> +		 * yuv-to-rgb-conv-factors are now defined from LCDC_HEOCFG16 to
>> +		 * LCDC_HEOCFG21 registers in SAM9X7.
>>   		 */
>> -		atmel_hlcdc_layer_write_cfg(&plane->layer,
>> -					    desc->layout.csc,
>> -					    0x4c900091);
>> -		atmel_hlcdc_layer_write_cfg(&plane->layer,
>> -					    desc->layout.csc + 1,
>> -					    0x7a5f5090);
>> -		atmel_hlcdc_layer_write_cfg(&plane->layer,
>> -					    desc->layout.csc + 2,
>> -					    0x40040890);
>> +		if (desc->layout.csc) {
>> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
>> +						    desc->layout.csc,
>> +						    0x00000488);
>> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
>> +						    desc->layout.csc + 1,
>> +						    0x00000648);
>> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
>> +						    desc->layout.csc + 2,
>> +						    0x1EA00480);
>> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
>> +						    desc->layout.csc + 3,
>> +						    0x00001D28);
>> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
>> +						    desc->layout.csc + 4,
>> +						    0x08100480);
>> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
>> +						    desc->layout.csc + 5,
>> +						    0x00000000);
>> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
>> +						    desc->layout.csc + 6,
>> +						    0x00000007);
>> +		}
>>   	}
>>   
>>   	return 0;
>> @@ -819,19 +970,31 @@ void atmel_hlcdc_plane_irq(struct atmel_hlcdc_plane *plane)
>>   {
>>   	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
>>   	u32 isr;
>> +	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>>   
>> -	isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
>> +	if (!(dc->is_xlcdc))
> 
> same here.
> 
>> +		isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
>> +	else
>> +		isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_XLCDC_LAYER_ISR);
>>   
>>   	/*
>>   	 * There's not much we can do in case of overrun except informing
>>   	 * the user. However, we are in interrupt context here, hence the
>>   	 * use of dev_dbg().
>>   	 */
>> -	if (isr &
>> -	    (ATMEL_HLCDC_LAYER_OVR_IRQ(0) | ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
>> -	     ATMEL_HLCDC_LAYER_OVR_IRQ(2)))
>> -		dev_dbg(plane->base.dev->dev, "overrun on plane %s\n",
>> -			desc->name);
>> +	if (!(dc->is_xlcdc)) {
>> +		if (isr &
>> +		    (ATMEL_HLCDC_LAYER_OVR_IRQ(0) | ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
>> +		     ATMEL_HLCDC_LAYER_OVR_IRQ(2)))
>> +			dev_dbg(plane->base.dev->dev, "overrun on plane %s\n",
>> +				desc->name);
>> +	} else {
>> +		if (isr &
>> +		    (ATMEL_XLCDC_LAYER_OVR_IRQ(0) | ATMEL_XLCDC_LAYER_OVR_IRQ(1) |
>> +		     ATMEL_XLCDC_LAYER_OVR_IRQ(2)))
>> +			dev_dbg(plane->base.dev->dev, "overrun on plane %s\n",
>> +				desc->name);
>> +	}
>>   }
>>   
>>   static const struct drm_plane_helper_funcs atmel_hlcdc_layer_plane_helper_funcs = {
> 

-- 
Thanks and Regards,
Manikandan M.

Re: [PATCH 6/9] drm: atmel_hlcdc: Add support for XLCDC in atmel LCD driver

[Sam Ravnborg]

Hi Manikandan,

On Tue, Jun 13, 2023 at 12:34:23PM +0530, Manikandan Muralidharan wrote:
> - XLCDC in SAM9X7 has different sets of registers and additional
> configuration bits when compared to previous HLCDC IP. Read/write
> operation on the controller registers is now separated using the
> XLCDC status flag.
> 	- HEO scaling, window resampling, Alpha blending, YUV-to-RGB
> conversion in XLCDC is derived and handled using additional
> configuration bits and registers.
> 	- Writing one to the Enable fields of each layer in LCD_ATTRE
> is required to reflect the values set in Configuration, FBA, Enable
> registers of each layer

In general things would benefit from a more clear separation between
hlcdc and xlcdc. In several cases two functions would be better than
testing as done today.

See some more specific comments in the following.

	Sam

> 
> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> [Hari.PrasathGE@microchip.com: update the attribute field for each layer]
> Signed-off-by: Hari Prasath Gujulan Elango <Hari.PrasathGE@microchip.com>
> [durai.manickamkr@microchip.com: implement status flag to seprate register update]
> Signed-off-by: Durai Manickam KR <durai.manickamkr@microchip.com>
> ---
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    |  28 +-
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   | 297 ++++++++++++++----
>  2 files changed, 256 insertions(+), 69 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index 58184cd6ab0b..7c9cf7c0c75d 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -139,10 +139,10 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
>  	state = drm_crtc_state_to_atmel_hlcdc_crtc_state(c->state);
>  	cfg = state->output_mode << 8;
>  
> -	if (adj->flags & DRM_MODE_FLAG_NVSYNC)
> +	if (!crtc->dc->is_xlcdc && (adj->flags & DRM_MODE_FLAG_NVSYNC))
>  		cfg |= ATMEL_HLCDC_VSPOL;
>  
> -	if (adj->flags & DRM_MODE_FLAG_NHSYNC)
> +	if (!crtc->dc->is_xlcdc && (adj->flags & DRM_MODE_FLAG_NHSYNC))
>  		cfg |= ATMEL_HLCDC_HSPOL;

From the above code I assume ATMEL_HLCDC_VSPOL and ATMEL_HLCDC_HSPOL are
specific to HLCDC and not part of XLCDC register set.

We can identify XLCDC specific registers as thy use "XLCDC" in the name.
But the "HLCDC" specific registers are not easy to spot.

Would it make sense to update the register definitions so we can see in
the register names which at XLCDC, which are HLCDC and which a common
(LCDC)?

It would require a little code crunch to do so as all users would need
an update. Dunno if this is worth it.
But then at least a comment in the register definition file.


>  
>  	regmap_update_bits(regmap, ATMEL_HLCDC_CFG(5),
> @@ -177,6 +177,18 @@ static void atmel_hlcdc_crtc_atomic_disable(struct drm_crtc *c,
>  
>  	pm_runtime_get_sync(dev->dev);
>  
> +	if (crtc->dc->is_xlcdc) {
> +		regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_XLCDC_CM);
> +		while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
> +		       (status & ATMEL_XLCDC_CM))
> +			cpu_relax();
> +
> +		regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_XLCDC_SD);
> +		while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
> +		       !(status & ATMEL_XLCDC_SD))
> +			cpu_relax();
> +	}

A small helper atmel_xlcdc_disable()?

> +
>  	regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_HLCDC_DISP);
>  	while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
>  	       (status & ATMEL_HLCDC_DISP))
> @@ -231,6 +243,18 @@ static void atmel_hlcdc_crtc_atomic_enable(struct drm_crtc *c,
>  	       !(status & ATMEL_HLCDC_DISP))
>  		cpu_relax();
>  
> +	if (crtc->dc->is_xlcdc) {
> +		regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_XLCDC_CM);
> +		while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
> +		       !(status & ATMEL_XLCDC_CM))
> +			cpu_relax();
> +
> +		regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_XLCDC_SD);
> +		while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
> +		       (status & ATMEL_XLCDC_SD))
> +			cpu_relax();
> +	}

A small helper atmel_xlcdc_enable()?

> +
>  	pm_runtime_put_sync(dev->dev);
>  
>  }
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> index daa508504f47..fe33476818c4 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> @@ -330,11 +330,59 @@ static void atmel_hlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
>  								     yfactor));
>  }
>  
> +static void atmel_xlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
> +					   struct atmel_hlcdc_plane_state *state)
> +{
> +	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
> +	u32 xfactor, yfactor;
> +
> +	if (!desc->layout.scaler_config)
> +		return;
> +
> +	if (state->crtc_w == state->src_w && state->crtc_h == state->src_h) {
> +		atmel_hlcdc_layer_write_cfg(&plane->layer,
> +					    desc->layout.scaler_config, 0);
> +		return;
> +	}
> +
> +	/* xfactor = round[(2^20 * XMEMSIZE)/XSIZE)] */
> +	xfactor = (1048576 * state->src_w) / state->crtc_w;
> +
> +	/* yfactor = round[(2^20 * YMEMSIZE)/YSIZE)] */
> +	yfactor = (1048576 * state->src_h) / state->crtc_h;
> +
> +	atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config,
> +				    ATMEL_XLCDC_LAYER_VSCALER_LUMA_ENABLE |
> +				    ATMEL_XLCDC_LAYER_VSCALER_CHROMA_ENABLE |
> +				    ATMEL_XLCDC_LAYER_HSCALER_LUMA_ENABLE |
> +				    ATMEL_XLCDC_LAYER_HSCALER_CHROMA_ENABLE);
> +
> +	atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 1,
> +				    yfactor);
> +	atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 3,
> +				    xfactor);
> +
> +	/* As per YCbCr window resampling configuration */
> +	if (state->base.fb->format->format == DRM_FORMAT_YUV420) {
> +		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 2,
> +					    yfactor / 2);
> +		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 4,
> +					    xfactor / 2);
> +	} else {
> +		/* As per ARGB window resampling configuration */
> +		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 2,
> +					    yfactor);
> +		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 4,
> +					    xfactor);
> +	}
> +}
> +
>  static void
>  atmel_hlcdc_plane_update_pos_and_size(struct atmel_hlcdc_plane *plane,
>  				      struct atmel_hlcdc_plane_state *state)
>  {
>  	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
> +	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>  
>  	if (desc->layout.size)
>  		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.size,
> @@ -352,7 +400,10 @@ atmel_hlcdc_plane_update_pos_and_size(struct atmel_hlcdc_plane *plane,
>  					ATMEL_HLCDC_LAYER_POS(state->crtc_x,
>  							      state->crtc_y));
>  
> -	atmel_hlcdc_plane_setup_scaler(plane, state);
> +	if (!(dc->is_xlcdc))
> +		atmel_hlcdc_plane_setup_scaler(plane, state);
> +	else
> +		atmel_xlcdc_plane_setup_scaler(plane, state);
>  }
>  
>  static void
> @@ -362,33 +413,58 @@ atmel_hlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
>  	unsigned int cfg = ATMEL_HLCDC_LAYER_DMA_BLEN_INCR16 | state->ahb_id;
>  	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
>  	const struct drm_format_info *format = state->base.fb->format;
> -
> +	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>  	/*
>  	 * Rotation optimization is not working on RGB888 (rotation is still
>  	 * working but without any optimization).
>  	 */
> -	if (format->format == DRM_FORMAT_RGB888)
> +	if ((!(dc->is_xlcdc)) && format->format == DRM_FORMAT_RGB888)
>  		cfg |= ATMEL_HLCDC_LAYER_DMA_ROTDIS;
>  
> -	atmel_hlcdc_layer_write_cfg(&plane->layer, ATMEL_HLCDC_LAYER_DMA_CFG,
> -				    cfg);
> +	if (!(dc->is_xlcdc)) {
> +		atmel_hlcdc_layer_write_cfg(&plane->layer, ATMEL_HLCDC_LAYER_DMA_CFG,
> +					    cfg);
>  
> -	cfg = ATMEL_HLCDC_LAYER_DMA | ATMEL_HLCDC_LAYER_REP;
> +		cfg = ATMEL_HLCDC_LAYER_DMA | ATMEL_HLCDC_LAYER_REP;
> +	} else {
> +		atmel_hlcdc_layer_write_cfg(&plane->layer, ATMEL_XLCDC_LAYER_DMA_CFG,
> +					    cfg);
> +
> +		cfg = ATMEL_XLCDC_LAYER_DMA | ATMEL_XLCDC_LAYER_REP;
> +	}
>  
>  	if (plane->base.type != DRM_PLANE_TYPE_PRIMARY) {
> -		cfg |= ATMEL_HLCDC_LAYER_OVR | ATMEL_HLCDC_LAYER_ITER2BL |
> -		       ATMEL_HLCDC_LAYER_ITER;
> +		if (!(dc->is_xlcdc)) {
> +			cfg |= ATMEL_HLCDC_LAYER_OVR | ATMEL_HLCDC_LAYER_ITER2BL |
> +			       ATMEL_HLCDC_LAYER_ITER;
> +
> +			if (format->has_alpha)
> +				cfg |= ATMEL_HLCDC_LAYER_LAEN;
> +			else
> +				cfg |= ATMEL_HLCDC_LAYER_GAEN |
> +				       ATMEL_HLCDC_LAYER_GA(state->base.alpha);
> +		} else {
> +			/*
> +			 * Alpha Blending bits specific to SAM9X7 SoC
> +			 */
> +			cfg |= ATMEL_XLCDC_LAYER_SFACTC_A0_MULT_AS |
> +			       ATMEL_XLCDC_LAYER_SFACTA_ONE |
> +			       ATMEL_XLCDC_LAYER_DFACTC_M_A0_MULT_AS |
> +			       ATMEL_XLCDC_LAYER_DFACTA_ONE;
> +			if (format->has_alpha)
> +				cfg |= ATMEL_XLCDC_LAYER_A0(0xff);
> +			else
> +				cfg |= ATMEL_XLCDC_LAYER_A0(state->base.alpha);
> +		}
> +	}
>  
> -		if (format->has_alpha)
> -			cfg |= ATMEL_HLCDC_LAYER_LAEN;
> +	if (state->disc_h && state->disc_w) {
> +		if (!(dc->is_xlcdc))
> +			cfg |= ATMEL_HLCDC_LAYER_DISCEN;
>  		else
> -			cfg |= ATMEL_HLCDC_LAYER_GAEN |
> -			       ATMEL_HLCDC_LAYER_GA(state->base.alpha);
> +			cfg |= ATMEL_XLCDC_LAYER_DISCEN;
>  	}
>  
> -	if (state->disc_h && state->disc_w)
> -		cfg |= ATMEL_HLCDC_LAYER_DISCEN;
> -
>  	atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.general_config,
>  				    cfg);
>  }

There is almost no code in atmel_hlcdc_plane_update_general_settings
that are shared anymore - please try to split out in two helpers, one
for hlcdc and one for xlcdc. Maybe add a third with some common code if
it makes sense.
That should result in much more readable code in the ind.


> @@ -441,33 +517,42 @@ static void atmel_hlcdc_plane_update_buffers(struct atmel_hlcdc_plane *plane,
>  					struct atmel_hlcdc_plane_state *state)
>  {
>  	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
> +	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>  	struct drm_framebuffer *fb = state->base.fb;
>  	u32 sr;
>  	int i;
>  
> -	sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
> +	if (!(dc->is_xlcdc))
> +		sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
> +	else
> +		sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_XLCDC_LAYER_ENR);
>  
>  	for (i = 0; i < state->nplanes; i++) {
>  		struct drm_gem_dma_object *gem = drm_fb_dma_get_gem_obj(fb, i);
>  
>  		state->dscrs[i]->addr = gem->dma_addr + state->offsets[i];
>  
> -		atmel_hlcdc_layer_write_reg(&plane->layer,
> -					    ATMEL_HLCDC_LAYER_PLANE_HEAD(i),
> -					    state->dscrs[i]->self);
> -
> -		if (!(sr & ATMEL_HLCDC_LAYER_EN)) {
> -			atmel_hlcdc_layer_write_reg(&plane->layer,
> -					ATMEL_HLCDC_LAYER_PLANE_ADDR(i),
> -					state->dscrs[i]->addr);
> +		if (!(dc->is_xlcdc)) {
>  			atmel_hlcdc_layer_write_reg(&plane->layer,
> -					ATMEL_HLCDC_LAYER_PLANE_CTRL(i),
> -					state->dscrs[i]->ctrl);
> +						    ATMEL_HLCDC_LAYER_PLANE_HEAD(i),
> +						    state->dscrs[i]->self);
> +
> +			if (!(sr & ATMEL_HLCDC_LAYER_EN)) {
> +				atmel_hlcdc_layer_write_reg(&plane->layer,
> +							    ATMEL_HLCDC_LAYER_PLANE_ADDR(i),
> +							    state->dscrs[i]->addr);
> +				atmel_hlcdc_layer_write_reg(&plane->layer,
> +							    ATMEL_HLCDC_LAYER_PLANE_CTRL(i),
> +							    state->dscrs[i]->ctrl);
> +				atmel_hlcdc_layer_write_reg(&plane->layer,
> +							    ATMEL_HLCDC_LAYER_PLANE_NEXT(i),
> +							    state->dscrs[i]->self);
> +			}
> +		} else {
>  			atmel_hlcdc_layer_write_reg(&plane->layer,
> -					ATMEL_HLCDC_LAYER_PLANE_NEXT(i),
> -					state->dscrs[i]->self);
> +						    ATMEL_XLCDC_LAYER_PLANE_ADDR(i),
> +						    state->dscrs[i]->addr);
>  		}
> -
>  		if (desc->layout.xstride[i])
>  			atmel_hlcdc_layer_write_cfg(&plane->layer,
>  						    desc->layout.xstride[i],

The xstide stuff is shared the rest is more or less XLCDC / HLCDC
specific. Try to split it out in a few xlcdc/hlcdc helpers to increase
readability.
Maybe do it in two steps. Introduce helper for hlcdc and thenadd xlcdc
stuff.

> @@ -716,19 +801,31 @@ static void atmel_hlcdc_plane_atomic_disable(struct drm_plane *p,
>  					     struct drm_atomic_state *state)
>  {
>  	struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
> -
> +	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>  	/* Disable interrupts */
> -	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IDR,
> -				    0xffffffff);
> +	if (!(dc->is_xlcdc))
> +		atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IDR,
> +					    0xffffffff);
> +	else
> +		atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_IDR,
> +					    0xffffffff);
>  
>  	/* Disable the layer */
> -	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHDR,
> -				    ATMEL_HLCDC_LAYER_RST |
> -				    ATMEL_HLCDC_LAYER_A2Q |
> -				    ATMEL_HLCDC_LAYER_UPDATE);
> +	if (!(dc->is_xlcdc))
> +		atmel_hlcdc_layer_write_reg(&plane->layer,
> +					    ATMEL_HLCDC_LAYER_CHDR,
> +					    ATMEL_HLCDC_LAYER_RST |
> +					    ATMEL_HLCDC_LAYER_A2Q |
> +					    ATMEL_HLCDC_LAYER_UPDATE);
> +	else
> +		atmel_hlcdc_layer_write_reg(&plane->layer,
> +					    ATMEL_XLCDC_LAYER_ENR, 0);
>  
>  	/* Clear all pending interrupts */
> -	atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
> +	if (!(dc->is_xlcdc))
> +		atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
> +	else
> +		atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_XLCDC_LAYER_ISR);
>  }
This looks like another good candidate for two functions, one for xlcdc,
one for hlcdc.

>  
>  static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
> @@ -739,6 +836,7 @@ static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
>  	struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
>  	struct atmel_hlcdc_plane_state *hstate =
>  			drm_plane_state_to_atmel_hlcdc_plane_state(new_s);
> +	struct atmel_hlcdc_dc *dc = p->dev->dev_private;
>  	u32 sr;
>  
>  	if (!new_s->crtc || !new_s->fb)
> @@ -756,23 +854,46 @@ static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
>  	atmel_hlcdc_plane_update_buffers(plane, hstate);
>  	atmel_hlcdc_plane_update_disc_area(plane, hstate);
>  
> -	/* Enable the overrun interrupts. */
> -	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IER,
> -				    ATMEL_HLCDC_LAYER_OVR_IRQ(0) |
> -				    ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
> -				    ATMEL_HLCDC_LAYER_OVR_IRQ(2));
> -
> -	/* Apply the new config at the next SOF event. */
> -	sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
> -	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHER,
> -			ATMEL_HLCDC_LAYER_UPDATE |
> -			(sr & ATMEL_HLCDC_LAYER_EN ?
> -			 ATMEL_HLCDC_LAYER_A2Q : ATMEL_HLCDC_LAYER_EN));
> +	if (!(dc->is_xlcdc)) {
> +		/* Enable the overrun interrupts. */
> +		atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IER,
> +					    ATMEL_HLCDC_LAYER_OVR_IRQ(0) |
> +					    ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
> +					    ATMEL_HLCDC_LAYER_OVR_IRQ(2));
> +
> +		/* Apply the new config at the next SOF event. */
> +		sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
> +		atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHER,
> +					    ATMEL_HLCDC_LAYER_UPDATE |
> +					    (sr & ATMEL_HLCDC_LAYER_EN ?
> +					    ATMEL_HLCDC_LAYER_A2Q : ATMEL_HLCDC_LAYER_EN));
> +	} else {
> +		/* Enable the overrun interrupts. */
> +		atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_IER,
> +					    ATMEL_XLCDC_LAYER_OVR_IRQ(0) |
> +					    ATMEL_XLCDC_LAYER_OVR_IRQ(1) |
> +					    ATMEL_XLCDC_LAYER_OVR_IRQ(2));
> +
> +		atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_ENR,
> +					    ATMEL_XLCDC_LAYER_EN);
> +	}
> +
> +	/*
> +	 * Updating XLCDC_xxxCFGx, XLCDC_xxxFBA and XLCDC_xxxEN,
> +	 * (where xxx indicates each layer) requires writing one to the
> +	 * Update Attribute field for each layer in LCDC_ATTRE register for SAM9X7.
> +	 */
> +	if (dc->is_xlcdc) {
> +		regmap_write(dc->hlcdc->regmap, ATMEL_XLCDC_ATTRE, ATMEL_XLCDC_BASE_UPDATE |
> +			     ATMEL_XLCDC_OVR1_UPDATE | ATMEL_XLCDC_OVR3_UPDATE |
> +			     ATMEL_XLCDC_HEO_UPDATE);
> +	}
>  }
Likewise.

>  
>  static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane)
>  {
>  	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
> +	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>  
>  	if (desc->type == ATMEL_HLCDC_OVERLAY_LAYER ||
>  	    desc->type == ATMEL_HLCDC_CURSOR_LAYER) {
> @@ -796,20 +917,50 @@ static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane)
>  			return ret;
>  	}
>  
> -	if (desc->layout.csc) {
> +	if (!(dc->is_xlcdc)) {
> +		if (desc->layout.csc) {
> +			/*
> +			 * TODO: decare a "yuv-to-rgb-conv-factors" property to let
> +			 * userspace modify these factors (using a BLOB property ?).
> +			 */
> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
> +						    desc->layout.csc,
> +						    0x4c900091);
> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
> +						    desc->layout.csc + 1,
> +						    0x7a5f5090);
> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
> +						    desc->layout.csc + 2,
> +						    0x40040890);
> +		}
> +	} else {
>  		/*
> -		 * TODO: decare a "yuv-to-rgb-conv-factors" property to let
> -		 * userspace modify these factors (using a BLOB property ?).
> +		 * yuv-to-rgb-conv-factors are now defined from LCDC_HEOCFG16 to
> +		 * LCDC_HEOCFG21 registers in SAM9X7.
>  		 */
> -		atmel_hlcdc_layer_write_cfg(&plane->layer,
> -					    desc->layout.csc,
> -					    0x4c900091);
> -		atmel_hlcdc_layer_write_cfg(&plane->layer,
> -					    desc->layout.csc + 1,
> -					    0x7a5f5090);
> -		atmel_hlcdc_layer_write_cfg(&plane->layer,
> -					    desc->layout.csc + 2,
> -					    0x40040890);
> +		if (desc->layout.csc) {
> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
> +						    desc->layout.csc,
> +						    0x00000488);
> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
> +						    desc->layout.csc + 1,
> +						    0x00000648);
> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
> +						    desc->layout.csc + 2,
> +						    0x1EA00480);
> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
> +						    desc->layout.csc + 3,
> +						    0x00001D28);
> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
> +						    desc->layout.csc + 4,
> +						    0x08100480);
> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
> +						    desc->layout.csc + 5,
> +						    0x00000000);
> +			atmel_hlcdc_layer_write_cfg(&plane->layer,
> +						    desc->layout.csc + 6,
> +						    0x00000007);
> +		}

Refactor in two helpers?
atmel_xlcdc_layout_csc()
atmel_hlcdc_layout_csc()


>  	}
>  
>  	return 0;
> @@ -819,19 +970,31 @@ void atmel_hlcdc_plane_irq(struct atmel_hlcdc_plane *plane)
>  {
>  	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
>  	u32 isr;
> +	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>  
> -	isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
> +	if (!(dc->is_xlcdc))
> +		isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
> +	else
> +		isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_XLCDC_LAYER_ISR);
>  
>  	/*
>  	 * There's not much we can do in case of overrun except informing
>  	 * the user. However, we are in interrupt context here, hence the
>  	 * use of dev_dbg().
>  	 */
> -	if (isr &
> -	    (ATMEL_HLCDC_LAYER_OVR_IRQ(0) | ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
> -	     ATMEL_HLCDC_LAYER_OVR_IRQ(2)))
> -		dev_dbg(plane->base.dev->dev, "overrun on plane %s\n",
> -			desc->name);
> +	if (!(dc->is_xlcdc)) {
> +		if (isr &
> +		    (ATMEL_HLCDC_LAYER_OVR_IRQ(0) | ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
> +		     ATMEL_HLCDC_LAYER_OVR_IRQ(2)))
> +			dev_dbg(plane->base.dev->dev, "overrun on plane %s\n",
> +				desc->name);
> +	} else {
> +		if (isr &
> +		    (ATMEL_XLCDC_LAYER_OVR_IRQ(0) | ATMEL_XLCDC_LAYER_OVR_IRQ(1) |
> +		     ATMEL_XLCDC_LAYER_OVR_IRQ(2)))
> +			dev_dbg(plane->base.dev->dev, "overrun on plane %s\n",
> +				desc->name);
> +	}
>  }
Again, split up in two functions. Maybe decide which to call in
atmel_hlcdc_layer_irq()
>  
>  static const struct drm_plane_helper_funcs atmel_hlcdc_layer_plane_helper_funcs = {
> -- 
> 2.25.1

[PATCH 7/9] drm: atmel-hlcdc: add DPI mode support for XLCDC

[Manikandan Muralidharan]

Add support for Display Pixel Interface (DPI) Compatible Mode
support in atmel-hlcdc driver for XLCDC IP along with legacy
pixel mapping.DPI mode BIT is configured in LCDC_CFG5 register.

Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
[durai.manickamkr@microchip.com: update DPI mode bit using is_xlcdc flag]
Signed-off-by: Durai Manickam KR <durai.manickamkr@microchip.com>
---
 .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    | 22 ++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index 7c9cf7c0c75d..abdece982507 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -30,10 +30,12 @@
  *
  * @base: base CRTC state
  * @output_mode: RGBXXX output mode
+ * @dpi: output DPI mode
  */
 struct atmel_hlcdc_crtc_state {
 	struct drm_crtc_state base;
 	unsigned int output_mode;
+	bool dpi;
 };
 
 static inline struct atmel_hlcdc_crtc_state *
@@ -138,6 +140,8 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
 
 	state = drm_crtc_state_to_atmel_hlcdc_crtc_state(c->state);
 	cfg = state->output_mode << 8;
+	if (crtc->dc->is_xlcdc)
+		cfg |= state->dpi << 11;
 
 	if (!crtc->dc->is_xlcdc && (adj->flags & DRM_MODE_FLAG_NVSYNC))
 		cfg |= ATMEL_HLCDC_VSPOL;
@@ -150,7 +154,9 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
 			   ATMEL_HLCDC_VSPDLYS | ATMEL_HLCDC_VSPDLYE |
 			   ATMEL_HLCDC_DISPPOL | ATMEL_HLCDC_DISPDLY |
 			   ATMEL_HLCDC_VSPSU | ATMEL_HLCDC_VSPHO |
-			   ATMEL_HLCDC_GUARDTIME_MASK | ATMEL_HLCDC_MODE_MASK,
+			   ATMEL_HLCDC_GUARDTIME_MASK |
+			   (crtc->dc->is_xlcdc ? ATMEL_XLCDC_MODE_MASK |
+			   ATMEL_XLCDC_DPI : ATMEL_HLCDC_MODE_MASK),
 			   cfg);
 
 	clk_disable_unprepare(crtc->dc->hlcdc->sys_clk);
@@ -344,7 +350,15 @@ static int atmel_hlcdc_crtc_select_output_mode(struct drm_crtc_state *state)
 
 	hstate = drm_crtc_state_to_atmel_hlcdc_crtc_state(state);
 	hstate->output_mode = fls(output_fmts) - 1;
-
+	if (crtc->dc->is_xlcdc) {
+		/* check if MIPI DPI bit needs to be set */
+		if (fls(output_fmts) > 3) {
+			hstate->output_mode -= 4;
+			hstate->dpi = true;
+		} else {
+			hstate->dpi = false;
+		}
+	}
 	return 0;
 }
 
@@ -448,7 +462,7 @@ static struct drm_crtc_state *
 atmel_hlcdc_crtc_duplicate_state(struct drm_crtc *crtc)
 {
 	struct atmel_hlcdc_crtc_state *state, *cur;
-
+	struct atmel_hlcdc_crtc *c = drm_crtc_to_atmel_hlcdc_crtc(crtc);
 	if (WARN_ON(!crtc->state))
 		return NULL;
 
@@ -459,6 +473,8 @@ atmel_hlcdc_crtc_duplicate_state(struct drm_crtc *crtc)
 
 	cur = drm_crtc_state_to_atmel_hlcdc_crtc_state(crtc->state);
 	state->output_mode = cur->output_mode;
+	if (c->dc->is_xlcdc)
+		state->dpi = cur->dpi;
 
 	return &state->base;
 }
-- 
2.25.1

[PATCH 8/9] drm: atmel-hlcdc: add vertical and horizontal scaling support for XLCDC

[Manikandan Muralidharan]

update the LCDC_HEOCFG30 and LCDC_HEOCFG31 registers of XLCDC IP which
supports vertical and horizontal scaling with Bilinear and Bicubic
co-efficients taps for Chroma and Luma componenets of the Pixel.

Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c   |  2 ++
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h   |  4 ++++
 .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c    | 18 ++++++++++++++++++
 3 files changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index fbbd2592efc7..8fcaa4023155 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -536,6 +536,8 @@ static const struct atmel_hlcdc_layer_desc atmel_xlcdc_sam9x7_layers[] = {
 			.general_config = 12,
 			.csc = 16,
 			.scaler_config = 23,
+			.vxs_config = 30,
+			.hxs_config = 31,
 		},
 		.clut_offset = 0x1300,
 	},
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
index 804e4d476f2b..9aedfd0f6039 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
@@ -264,6 +264,8 @@
  * @disc_pos: discard area position register
  * @disc_size: discard area size register
  * @csc: color space conversion register
+ * @vxs_config: vertical scalar filter taps control register
+ * @hxs_config: horizontal scalar filter taps control register
  */
 struct atmel_hlcdc_layer_cfg_layout {
 	int xstride[ATMEL_HLCDC_LAYER_MAX_PLANES];
@@ -283,6 +285,8 @@ struct atmel_hlcdc_layer_cfg_layout {
 	int disc_pos;
 	int disc_size;
 	int csc;
+	int vxs_config;
+	int hxs_config;
 };
 
 /**
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
index fe33476818c4..613382baa553 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
@@ -961,6 +961,24 @@ static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane)
 						    desc->layout.csc + 6,
 						    0x00000007);
 		}
+		if (desc->layout.vxs_config && desc->layout.hxs_config) {
+			/*
+			 * Updating vxs.config and hxs.config fixes the
+			 * Green Color Issue in SAM9X7 EGT Video Player App
+			 */
+			atmel_hlcdc_layer_write_cfg(&plane->layer,
+						    desc->layout.vxs_config,
+						    ATMEL_XLCDC_LAYER_VXSYCFG_ONE |
+						    ATMEL_XLCDC_LAYER_VXSYTAP2_ENABLE |
+						    ATMEL_XLCDC_LAYER_VXSCCFG_ONE |
+						    ATMEL_XLCDC_LAYER_VXSCTAP2_ENABLE);
+			atmel_hlcdc_layer_write_cfg(&plane->layer,
+						    desc->layout.hxs_config,
+						    ATMEL_XLCDC_LAYER_HXSYCFG_ONE |
+						    ATMEL_XLCDC_LAYER_HXSYTAP2_ENABLE |
+						    ATMEL_XLCDC_LAYER_HXSCCFG_ONE |
+						    ATMEL_XLCDC_LAYER_HXSCTAP2_ENABLE);
+		}
 	}
 
 	return 0;
-- 
2.25.1

[PATCH 9/9] drm: atmel-hlcdc: add support for DSI output formats

[Manikandan Muralidharan]

Add support for the following DPI mode if the encoder type
is DSI as per the XLCDC IP datasheet:
- 16BPPCFG1
- 16BPPCFG2
- 16BPPCFG3
- 18BPPCFG1
- 18BPPCFG2
- 24BPP

Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
[durai.manickamkr@microchip.com: update output format using is_xlcdc flag]
Signed-off-by: Durai Manickam KR <durai.manickamkr@microchip.com>
---
 .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    | 117 +++++++++++++-----
 1 file changed, 86 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index abdece982507..dc8361ebf05b 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -265,11 +265,18 @@ static void atmel_hlcdc_crtc_atomic_enable(struct drm_crtc *c,
 
 }
 
-#define ATMEL_HLCDC_RGB444_OUTPUT	BIT(0)
-#define ATMEL_HLCDC_RGB565_OUTPUT	BIT(1)
-#define ATMEL_HLCDC_RGB666_OUTPUT	BIT(2)
-#define ATMEL_HLCDC_RGB888_OUTPUT	BIT(3)
-#define ATMEL_HLCDC_OUTPUT_MODE_MASK	GENMASK(3, 0)
+#define ATMEL_HLCDC_RGB444_OUTPUT		BIT(0)
+#define ATMEL_HLCDC_RGB565_OUTPUT		BIT(1)
+#define ATMEL_HLCDC_RGB666_OUTPUT		BIT(2)
+#define ATMEL_HLCDC_RGB888_OUTPUT		BIT(3)
+#define ATMEL_HLCDC_DPI_RGB565C1_OUTPUT		BIT(4)
+#define ATMEL_HLCDC_DPI_RGB565C2_OUTPUT		BIT(5)
+#define ATMEL_HLCDC_DPI_RGB565C3_OUTPUT		BIT(6)
+#define ATMEL_HLCDC_DPI_RGB666C1_OUTPUT		BIT(7)
+#define ATMEL_HLCDC_DPI_RGB666C2_OUTPUT		BIT(8)
+#define ATMEL_HLCDC_DPI_RGB888_OUTPUT		BIT(9)
+#define ATMEL_HLCDC_OUTPUT_MODE_MASK		GENMASK(3, 0)
+#define ATMEL_XLCDC_OUTPUT_MODE_MASK		GENMASK(9, 0)
 
 static int atmel_hlcdc_connector_output_mode(struct drm_connector_state *state)
 {
@@ -283,37 +290,83 @@ static int atmel_hlcdc_connector_output_mode(struct drm_connector_state *state)
 	if (!encoder)
 		encoder = connector->encoder;
 
-	switch (atmel_hlcdc_encoder_get_bus_fmt(encoder)) {
-	case 0:
-		break;
-	case MEDIA_BUS_FMT_RGB444_1X12:
-		return ATMEL_HLCDC_RGB444_OUTPUT;
-	case MEDIA_BUS_FMT_RGB565_1X16:
-		return ATMEL_HLCDC_RGB565_OUTPUT;
-	case MEDIA_BUS_FMT_RGB666_1X18:
-		return ATMEL_HLCDC_RGB666_OUTPUT;
-	case MEDIA_BUS_FMT_RGB888_1X24:
-		return ATMEL_HLCDC_RGB888_OUTPUT;
-	default:
-		return -EINVAL;
-	}
-
-	for (j = 0; j < info->num_bus_formats; j++) {
-		switch (info->bus_formats[j]) {
-		case MEDIA_BUS_FMT_RGB444_1X12:
-			supported_fmts |= ATMEL_HLCDC_RGB444_OUTPUT;
+	if (encoder->encoder_type == DRM_MODE_ENCODER_DSI) {
+		/*
+		 * atmel-hlcdc to support DSI formats with DSI video pipeline
+		 * when DRM_MODE_ENCODER_DSI type is set by
+		 * connector driver component.
+		 */
+		switch (atmel_hlcdc_encoder_get_bus_fmt(encoder)) {
+		case 0:
 			break;
 		case MEDIA_BUS_FMT_RGB565_1X16:
-			supported_fmts |= ATMEL_HLCDC_RGB565_OUTPUT;
-			break;
+			return ATMEL_HLCDC_DPI_RGB565C1_OUTPUT;
 		case MEDIA_BUS_FMT_RGB666_1X18:
-			supported_fmts |= ATMEL_HLCDC_RGB666_OUTPUT;
-			break;
+			return ATMEL_HLCDC_DPI_RGB666C1_OUTPUT;
+		case MEDIA_BUS_FMT_RGB666_1X24_CPADHI:
+			return ATMEL_HLCDC_DPI_RGB666C2_OUTPUT;
 		case MEDIA_BUS_FMT_RGB888_1X24:
-			supported_fmts |= ATMEL_HLCDC_RGB888_OUTPUT;
-			break;
+			return ATMEL_HLCDC_DPI_RGB888_OUTPUT;
 		default:
+			return -EINVAL;
+		}
+
+		for (j = 0; j < info->num_bus_formats; j++) {
+			switch (info->bus_formats[j]) {
+			case MEDIA_BUS_FMT_RGB565_1X16:
+				supported_fmts |=
+					ATMEL_HLCDC_DPI_RGB565C1_OUTPUT;
+				break;
+			case MEDIA_BUS_FMT_RGB666_1X18:
+				supported_fmts |=
+					ATMEL_HLCDC_DPI_RGB666C1_OUTPUT;
+				break;
+			case MEDIA_BUS_FMT_RGB666_1X24_CPADHI:
+				supported_fmts |=
+					ATMEL_HLCDC_DPI_RGB666C2_OUTPUT;
+				break;
+			case MEDIA_BUS_FMT_RGB888_1X24:
+				supported_fmts |=
+					ATMEL_HLCDC_DPI_RGB888_OUTPUT;
+				break;
+			default:
+				break;
+			}
+		}
+
+	} else {
+		switch (atmel_hlcdc_encoder_get_bus_fmt(encoder)) {
+		case 0:
 			break;
+		case MEDIA_BUS_FMT_RGB444_1X12:
+			return ATMEL_HLCDC_RGB444_OUTPUT;
+		case MEDIA_BUS_FMT_RGB565_1X16:
+			return ATMEL_HLCDC_RGB565_OUTPUT;
+		case MEDIA_BUS_FMT_RGB666_1X18:
+			return ATMEL_HLCDC_RGB666_OUTPUT;
+		case MEDIA_BUS_FMT_RGB888_1X24:
+			return ATMEL_HLCDC_RGB888_OUTPUT;
+		default:
+			return -EINVAL;
+		}
+
+		for (j = 0; j < info->num_bus_formats; j++) {
+			switch (info->bus_formats[j]) {
+			case MEDIA_BUS_FMT_RGB444_1X12:
+				supported_fmts |= ATMEL_HLCDC_RGB444_OUTPUT;
+				break;
+			case MEDIA_BUS_FMT_RGB565_1X16:
+				supported_fmts |= ATMEL_HLCDC_RGB565_OUTPUT;
+				break;
+			case MEDIA_BUS_FMT_RGB666_1X18:
+				supported_fmts |= ATMEL_HLCDC_RGB666_OUTPUT;
+				break;
+			case MEDIA_BUS_FMT_RGB888_1X24:
+				supported_fmts |= ATMEL_HLCDC_RGB888_OUTPUT;
+				break;
+			default:
+				break;
+			}
 		}
 	}
 
@@ -322,7 +375,7 @@ static int atmel_hlcdc_connector_output_mode(struct drm_connector_state *state)
 
 static int atmel_hlcdc_crtc_select_output_mode(struct drm_crtc_state *state)
 {
-	unsigned int output_fmts = ATMEL_HLCDC_OUTPUT_MODE_MASK;
+	unsigned int output_fmts;
 	struct atmel_hlcdc_crtc_state *hstate;
 	struct drm_connector_state *cstate;
 	struct drm_connector *connector;
@@ -330,6 +383,8 @@ static int atmel_hlcdc_crtc_select_output_mode(struct drm_crtc_state *state)
 	int i;
 
 	crtc = drm_crtc_to_atmel_hlcdc_crtc(state->crtc);
+	output_fmts = crtc->dc->is_xlcdc ? ATMEL_XLCDC_OUTPUT_MODE_MASK :
+		      ATMEL_HLCDC_OUTPUT_MODE_MASK;
 
 	for_each_new_connector_in_state(state->state, connector, cstate, i) {
 		unsigned int supported_fmts = 0;
-- 
2.25.1