Linux-Aspeed Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ARM: dts: Aspeed: Witherspoon: Update BMC partitioning
From: Eddie James @ 2019-03-27 15:10 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <CACPK8Xer53amWbQ-6dvEaXfHu=grk3ZWsH9NKCa_Xi_JKUnCMg@mail.gmail.com>


On 3/27/19 1:20 AM, Joel Stanley wrote:
> On Tue, 12 Mar 2019 at 16:50, Adriana Kobylak <anoo@linux.ibm.com> wrote:
>> From: "Edward A. James" <eajames@us.ibm.com>
>>
>> Add simplified partitions for BMC and alternate flash. Include these by
>> default in Witherspoon.
>>
>> Signed-off-by: Edward A. James <eajames@us.ibm.com>
>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>> ---
>>   .../boot/dts/aspeed-bmc-alt-opp-flash-layout-ubi.dtsi  | 18 ++++++++++++++++++
>>   arch/arm/boot/dts/aspeed-bmc-opp-flash-layout-ubi.dtsi | 18 ++++++++++++++++++
>>   arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts       |  3 ++-
>>   3 files changed, 38 insertions(+), 1 deletion(-)
>>   create mode 100644 arch/arm/boot/dts/aspeed-bmc-alt-opp-flash-layout-ubi.dtsi
>>   create mode 100644 arch/arm/boot/dts/aspeed-bmc-opp-flash-layout-ubi.dtsi
>>
>> diff --git a/arch/arm/boot/dts/aspeed-bmc-alt-opp-flash-layout-ubi.dtsi b/arch/arm/boot/dts/aspeed-bmc-alt-opp-flash-layout-ubi.dtsi
>> new file mode 100644
>> index 0000000..9277599
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/aspeed-bmc-alt-opp-flash-layout-ubi.dtsi
> As there are no other machines that use this layout, I think you
> should have the layout in the dts file.

I think the idea was we might have more machines that use this layout in 
the future...

>
>> @@ -0,0 +1,18 @@
>> +               label = "alt-bmc";
>> +               partitions {
>> +                               #address-cells = < 1 >;
>> +                               #size-cells = < 1 >;
>> +                               compatible = "fixed-partitions";
>> +                               u-boot at 0 {
>> +                                       reg = < 0 0x60000 >;
>> +                                       label = "alt-u-boot";
>> +                               };
>> +                               u-boot-env at 60000 {
>> +                                       reg = < 0x60000 0x20000 >;
>> +                                       label = "alt-u-boot-env";
>> +                               };
>> +                               obmc-ubi at 80000 {
>> +                                       reg = < 0x80000 0x1F80000 >;
>> +                                       label = "alt-obmc-ubi";
>> +                               };
>> +               };
>> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-flash-layout-ubi.dtsi b/arch/arm/boot/dts/aspeed-bmc-opp-flash-layout-ubi.dtsi
>> new file mode 100644
>> index 0000000..0059ad1
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-flash-layout-ubi.dtsi
>> @@ -0,0 +1,18 @@
>> +               label = "bmc";
>> +               partitions {
>> +                               #address-cells = < 1 >;
>> +                               #size-cells = < 1 >;
>> +                               compatible = "fixed-partitions";
>> +                               u-boot at 0 {
>> +                                       reg = < 0 0x60000 >;
>> +                                       label = "u-boot";
>> +                               };
>> +                               u-boot-env at 60000 {
>> +                                       reg = < 0x60000 0x20000 >;
>> +                                       label = "u-boot-env";
>> +                               };
>> +                               obmc-ubi at 80000 {
>> +                                       reg = < 0x80000 0x1F80000 >;
>> +                                       label = "obmc-ubi";
>> +                               };
>> +               };
>> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
>> index c51e3e8..058b9b7 100644
>> --- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
>> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
>> @@ -191,7 +191,7 @@
>>                  label = "bmc";
>>                  m25p,fast-read;
>>                  spi-max-frequency = <50000000>;
>> -#include "openbmc-flash-layout.dtsi"
>> +#include "aspeed-bmc-opp-flash-layout-ubi.dtsi"
>>          };
>>
>>          flash at 1 {
>> @@ -199,6 +199,7 @@
>>                  label = "alt";
>>                  m25p,fast-read;
>>                  spi-max-frequency = <50000000>;
>> +#include "aspeed-bmc-alt-opp-flash-layout-ubi.dtsi"
>>          };
>>   };
>>
>> --
>> 1.8.3.1
>>


^ permalink raw reply

* [PATCH v7 2/2] drivers/misc: Add Aspeed P2A control driver
From: Greg KH @ 2019-03-27 16:45 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190312163101.155910-1-venture@google.com>

On Tue, Mar 12, 2019 at 09:31:01AM -0700, Patrick Venture wrote:
> +	phys_addr_t mem_base;

Is this really a 32bit value?

Your ioctl thinks it is:

> +struct aspeed_p2a_ctrl_mapping {
> +	__u32 addr;

Does this driver not work on a 64bit kernel?

> +	__u32 length;
> +	__u32 flags;
> +};

addr really should be __u32 here so you don't have to mess with 32/64
bit user/kernel issues, right?

thanks,

greg k-h

^ permalink raw reply

* [Potential Spoof] Re: [Potential Spoof] Re: [Potential Spoof] Re: [PATCH v2] misc: aspeed-lpc-ctrl: make parameter optional
From: Vijay Khemka @ 2019-03-27 17:44 UTC (permalink / raw)
  To: linux-aspeed

Hi Joel,
Can you please apply this below patch to kernel 5.0.

Regards
-Vijay

?On 3/18/19, 12:46 PM, "openbmc on behalf of Vijay Khemka" <openbmc-bounces+vijaykhemka=fb.com at lists.ozlabs.org on behalf of vijaykhemka@fb.com> wrote:

    Hi Joel,
    Can you please apply this patch as " Documentation/devicetree/bindings/mfd/aspeed-lpc.txt" has already been applied
    
    Regards
    -Vijay
    
    On 3/5/19, 4:15 PM, "Linux-aspeed on behalf of Vijay Khemka" <linux-aspeed-bounces+vijaykhemka=fb.com at lists.ozlabs.org on behalf of vijaykhemka@fb.com> wrote:
    
        Joel,
        Did this patch apply upstream. Somehow I can't find this patch in linux or linux-next or our obmc dev4.19.
        
        Regards
        -Vijay
        
        On 1/17/19, 10:53 AM, "Linux-aspeed on behalf of Vijay Khemka" <linux-aspeed-bounces+vijaykhemka=fb.com at lists.ozlabs.org on behalf of vijaykhemka@fb.com> wrote:
        
            
            
            On 1/16/19, 10:17 PM, "Joel Stanley" <joel@jms.id.au> wrote:
            
                On Thu, 17 Jan 2019 at 09:02, Vijay Khemka <vijaykhemka@fb.com> wrote:
                >
                > Makiing memory-region and flash as optional parameter in device
                > tree if user needs to use these parameter through ioctl then
                > need to define in devicetree.
                >
                > Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
                
                Thanks! This looks okay to me. I tested it on one of our systems which
                uses both flash and reserved memory and it was fine.
                
                Reviewed-by: Joel Stanley <joel@jms.id.au>
                
                Can you also send a patch to update the bindings at
                Documentation/devicetree/bindings/mfd/aspeed-lpc.txt ? I think the
                only change you need to make is to move the memory region and flash
                properties to optional (instead of required).
            
            Sure I will do this.
                
                Cheers,
                
                Joel
                
                > ---
                >  drivers/misc/aspeed-lpc-ctrl.c | 58 +++++++++++++++++++++-------------
                >  1 file changed, 36 insertions(+), 22 deletions(-)
                >
                > diff --git a/drivers/misc/aspeed-lpc-ctrl.c b/drivers/misc/aspeed-lpc-ctrl.c
                > index a024f8042259..332210e06e98 100644
                > --- a/drivers/misc/aspeed-lpc-ctrl.c
                > +++ b/drivers/misc/aspeed-lpc-ctrl.c
                > @@ -68,6 +68,7 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
                >                 unsigned long param)
                >  {
                >         struct aspeed_lpc_ctrl *lpc_ctrl = file_aspeed_lpc_ctrl(file);
                > +       struct device *dev = file->private_data;
                >         void __user *p = (void __user *)param;
                >         struct aspeed_lpc_ctrl_mapping map;
                >         u32 addr;
                > @@ -90,6 +91,12 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
                >                 if (map.window_id != 0)
                >                         return -EINVAL;
                >
                > +               /* If memory-region is not described in device tree */
                > +               if (!lpc_ctrl->mem_size) {
                > +                       dev_err(dev, "Didn't find reserved memory\n");
                > +                       return -EINVAL;
                > +               }
                > +
                >                 map.size = lpc_ctrl->mem_size;
                >
                >                 return copy_to_user(p, &map, sizeof(map)) ? -EFAULT : 0;
                > @@ -126,9 +133,18 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
                >                         return -EINVAL;
                >
                >                 if (map.window_type == ASPEED_LPC_CTRL_WINDOW_FLASH) {
                > +                       if (!lpc_ctrl->pnor_size) {
                > +                               dev_err(dev, "Didn't find host pnor flash\n");
                > +                               return -EINVAL;
                > +                       }
                >                         addr = lpc_ctrl->pnor_base;
                >                         size = lpc_ctrl->pnor_size;
                >                 } else if (map.window_type == ASPEED_LPC_CTRL_WINDOW_MEMORY) {
                > +                       /* If memory-region is not described in device tree */
                > +                       if (!lpc_ctrl->mem_size) {
                > +                               dev_err(dev, "Didn't find reserved memory\n");
                > +                               return -EINVAL;
                > +                       }
                >                         addr = lpc_ctrl->mem_base;
                >                         size = lpc_ctrl->mem_size;
                >                 } else {
                > @@ -196,17 +212,17 @@ static int aspeed_lpc_ctrl_probe(struct platform_device *pdev)
                >         if (!lpc_ctrl)
                >                 return -ENOMEM;
                >
                > +       /* If flash is described in device tree then store */
                >         node = of_parse_phandle(dev->of_node, "flash", 0);
                >         if (!node) {
                > -               dev_err(dev, "Didn't find host pnor flash node\n");
                > -               return -ENODEV;
                > -       }
                > -
                > -       rc = of_address_to_resource(node, 1, &resm);
                > -       of_node_put(node);
                > -       if (rc) {
                > -               dev_err(dev, "Couldn't address to resource for flash\n");
                > -               return rc;
                > +               dev_dbg(dev, "Didn't find host pnor flash node\n");
                > +       } else {
                > +               rc = of_address_to_resource(node, 1, &resm);
                > +               of_node_put(node);
                > +               if (rc) {
                > +                       dev_err(dev, "Couldn't address to resource for flash\n");
                > +                       return rc;
                > +               }
                >         }
                >
                >         lpc_ctrl->pnor_size = resource_size(&resm);
                > @@ -214,22 +230,22 @@ static int aspeed_lpc_ctrl_probe(struct platform_device *pdev)
                >
                >         dev_set_drvdata(&pdev->dev, lpc_ctrl);
                >
                > +       /* If memory-region is described in device tree then store */
                >         node = of_parse_phandle(dev->of_node, "memory-region", 0);
                >         if (!node) {
                > -               dev_err(dev, "Didn't find reserved memory\n");
                > -               return -EINVAL;
                > -       }
                > +               dev_dbg(dev, "Didn't find reserved memory\n");
                > +       } else {
                > +               rc = of_address_to_resource(node, 0, &resm);
                > +               of_node_put(node);
                > +               if (rc) {
                > +                       dev_err(dev, "Couldn't address to resource for reserved memory\n");
                > +                       return -ENOMEM;
                > +               }
                >
                > -       rc = of_address_to_resource(node, 0, &resm);
                > -       of_node_put(node);
                > -       if (rc) {
                > -               dev_err(dev, "Couldn't address to resource for reserved memory\n");
                > -               return -ENOMEM;
                > +               lpc_ctrl->mem_size = resource_size(&resm);
                > +               lpc_ctrl->mem_base = resm.start;
                >         }
                >
                > -       lpc_ctrl->mem_size = resource_size(&resm);
                > -       lpc_ctrl->mem_base = resm.start;
                > -
                >         lpc_ctrl->regmap = syscon_node_to_regmap(
                >                         pdev->dev.parent->of_node);
                >         if (IS_ERR(lpc_ctrl->regmap)) {
                > @@ -258,8 +274,6 @@ static int aspeed_lpc_ctrl_probe(struct platform_device *pdev)
                >                 goto err;
                >         }
                >
                > -       dev_info(dev, "Loaded at %pr\n", &resm);
                > -
                >         return 0;
                >
                >  err:
                > --
                > 2.17.1
                >
                
            
            
        
        
    
    


^ permalink raw reply

* [Potential Spoof] Re: [Potential Spoof] Re: [PATCH] ARM: dts: aspeed: tiogapass: Enable VUART
From: Vijay Khemka @ 2019-03-27 17:45 UTC (permalink / raw)
  To: linux-aspeed

Hi Joel,
Please apply this patch.

Regards
-Vijay

?On 3/18/19, 12:46 PM, "openbmc on behalf of Vijay Khemka" <openbmc-bounces+vijaykhemka=fb.com at lists.ozlabs.org on behalf of vijaykhemka@fb.com> wrote:

    Hi Joel,
    Please apply this patch.
    
    Regards
    -Vijay
    
    On 3/5/19, 12:06 PM, "openbmc on behalf of Vijay Khemka" <openbmc-bounces+vijaykhemka=fb.com at lists.ozlabs.org on behalf of vijaykhemka@fb.com> wrote:
    
        Please review below patch.
        
        Regards
        -Vijay
        
        On 1/30/19, 10:14 AM, "Vijay Khemka" <vijaykhemka@fb.com> wrote:
        
            Enabling vuart for Facebook tiogapass
            
            Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
            ---
             arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts | 5 +++++
             1 file changed, 5 insertions(+)
            
            diff --git a/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts b/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts
            index 42e0d7a8e8d0..a058fb2985f7 100644
            --- a/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts
            +++ b/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts
            @@ -64,6 +64,11 @@
             	status = "okay";
             };
             
            +&vuart {
            +	// VUART Host Console
            +	status = "okay";
            +};
            +
             &uart1 {
             	// Host Console
             	status = "okay";
            -- 
            2.17.1
            
            
        
        
    
    


^ permalink raw reply

* [PATCH v7 2/2] drivers/misc: Add Aspeed P2A control driver
From: Patrick Venture @ 2019-03-27 18:44 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190327164542.GC11712@kroah.com>

On Wed, Mar 27, 2019 at 11:28 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, Mar 12, 2019 at 09:31:01AM -0700, Patrick Venture wrote:
> > +     phys_addr_t mem_base;
>
> Is this really a 32bit value?

It's going to be a 32-bit value if this is in the dts for one of the
correspondingly supported aspeed models.

>
> Your ioctl thinks it is:
>
> > +struct aspeed_p2a_ctrl_mapping {
> > +     __u32 addr;
>
> Does this driver not work on a 64bit kernel?

This driver is aimed at only 32-bit hardware (ast2400/2500).  I
modeled the approach after the aspeed-lpc-ctrl driver as it's
providing similar functionality.

>
> > +     __u32 length;
> > +     __u32 flags;
> > +};
>
> addr really should be __u32 here so you don't have to mess with 32/64
> bit user/kernel issues, right?

Add is __u32 there.  Are you suggesting it shouldn't be?
>
> thanks,
>
> greg k-h

^ permalink raw reply

* [PATCH v7 2/2] drivers/misc: Add Aspeed P2A control driver
From: Greg KH @ 2019-03-27 18:54 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <CAO=notxFvvOmpjMyXvt5PyuWsB92eraN+=8mWk8_aY2w3sK-eQ@mail.gmail.com>

On Wed, Mar 27, 2019 at 11:44:36AM -0700, Patrick Venture wrote:
> On Wed, Mar 27, 2019 at 11:28 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Tue, Mar 12, 2019 at 09:31:01AM -0700, Patrick Venture wrote:
> > > +     phys_addr_t mem_base;
> >
> > Is this really a 32bit value?
> 
> It's going to be a 32-bit value if this is in the dts for one of the
> correspondingly supported aspeed models.
> 
> >
> > Your ioctl thinks it is:
> >
> > > +struct aspeed_p2a_ctrl_mapping {
> > > +     __u32 addr;
> >
> > Does this driver not work on a 64bit kernel?
> 
> This driver is aimed at only 32-bit hardware (ast2400/2500).  I
> modeled the approach after the aspeed-lpc-ctrl driver as it's
> providing similar functionality.
> 
> >
> > > +     __u32 length;
> > > +     __u32 flags;
> > > +};
> >
> > addr really should be __u32 here so you don't have to mess with 32/64
> > bit user/kernel issues, right?
> 
> Add is __u32 there.  Are you suggesting it shouldn't be?

Ugh, yes, sorry, I meant to say "__u64".

If you all insist that this is all that is ever going to be needed, ok,
but I reserve the right to complain in 4 years when this needs to be
changed :)

thanks,

greg k-h

^ permalink raw reply

* [PATCH v7 2/2] drivers/misc: Add Aspeed P2A control driver
From: Patrick Venture @ 2019-03-27 19:01 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190327185410.GA14828@kroah.com>

On Wed, Mar 27, 2019 at 11:54 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Wed, Mar 27, 2019 at 11:44:36AM -0700, Patrick Venture wrote:
> > On Wed, Mar 27, 2019 at 11:28 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Tue, Mar 12, 2019 at 09:31:01AM -0700, Patrick Venture wrote:
> > > > +     phys_addr_t mem_base;
> > >
> > > Is this really a 32bit value?
> >
> > It's going to be a 32-bit value if this is in the dts for one of the
> > correspondingly supported aspeed models.
> >
> > >
> > > Your ioctl thinks it is:
> > >
> > > > +struct aspeed_p2a_ctrl_mapping {
> > > > +     __u32 addr;
> > >
> > > Does this driver not work on a 64bit kernel?
> >
> > This driver is aimed at only 32-bit hardware (ast2400/2500).  I
> > modeled the approach after the aspeed-lpc-ctrl driver as it's
> > providing similar functionality.
> >
> > >
> > > > +     __u32 length;
> > > > +     __u32 flags;
> > > > +};
> > >
> > > addr really should be __u32 here so you don't have to mess with 32/64
> > > bit user/kernel issues, right?
> >
> > Add is __u32 there.  Are you suggesting it shouldn't be?
>
> Ugh, yes, sorry, I meant to say "__u64".
>
> If you all insist that this is all that is ever going to be needed, ok,
> but I reserve the right to complain in 4 years when this needs to be
> changed :)

In the event the ast2600 comes out and is 64-bit -- I can't imagine
that's likely to happen.  I can take solace that this won't be the
only thing that needs retrofitting.  But it wouldn't kill me to just
make the change.  I'll just have to tweak it to return failure in the
event the address provided isn't found in any region...

Is that all that needs to change for 64-bit addressing support - given
your read of the driver?

>
> thanks,
>
> greg k-h

^ permalink raw reply

* [PATCH v7 2/2] drivers/misc: Add Aspeed P2A control driver
From: Patrick Venture @ 2019-03-27 19:50 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <CAO=notzwnDtJ9jmEUDsy_tqVKH-+4rO9gd8rpVjKKvQqJsODTQ@mail.gmail.com>

On Wed, Mar 27, 2019 at 12:01 PM Patrick Venture <venture@google.com> wrote:
>
> On Wed, Mar 27, 2019 at 11:54 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Wed, Mar 27, 2019 at 11:44:36AM -0700, Patrick Venture wrote:
> > > On Wed, Mar 27, 2019 at 11:28 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Tue, Mar 12, 2019 at 09:31:01AM -0700, Patrick Venture wrote:
> > > > > +     phys_addr_t mem_base;
> > > >
> > > > Is this really a 32bit value?
> > >
> > > It's going to be a 32-bit value if this is in the dts for one of the
> > > correspondingly supported aspeed models.
> > >
> > > >
> > > > Your ioctl thinks it is:
> > > >
> > > > > +struct aspeed_p2a_ctrl_mapping {
> > > > > +     __u32 addr;
> > > >
> > > > Does this driver not work on a 64bit kernel?
> > >
> > > This driver is aimed at only 32-bit hardware (ast2400/2500).  I
> > > modeled the approach after the aspeed-lpc-ctrl driver as it's
> > > providing similar functionality.
> > >
> > > >
> > > > > +     __u32 length;
> > > > > +     __u32 flags;
> > > > > +};
> > > >
> > > > addr really should be __u32 here so you don't have to mess with 32/64
> > > > bit user/kernel issues, right?
> > >
> > > Add is __u32 there.  Are you suggesting it shouldn't be?
> >
> > Ugh, yes, sorry, I meant to say "__u64".
> >
> > If you all insist that this is all that is ever going to be needed, ok,
> > but I reserve the right to complain in 4 years when this needs to be
> > changed :)
>
> In the event the ast2600 comes out and is 64-bit -- I can't imagine
> that's likely to happen.  I can take solace that this won't be the
> only thing that needs retrofitting.  But it wouldn't kill me to just
> make the change.  I'll just have to tweak it to return failure in the
> event the address provided isn't found in any region...
>
> Is that all that needs to change for 64-bit addressing support - given
> your read of the driver?

I should have v8 for review shortly.

>
> >
> > thanks,
> >
> > greg k-h

^ permalink raw reply

* [PATCH v8 1/2] dt-bindings: misc: aspeed-p2a-ctrl: add support
From: Patrick Venture @ 2019-03-27 21:21 UTC (permalink / raw)
  To: linux-aspeed

Document the ast2400, ast2500 PCI-to-AHB bridge control driver bindings.

Signed-off-by: Patrick Venture <venture@google.com>
---
Changes for v8:
- None
Changes for v7:
- Moved node under the syscon node it requires
Changes for v6:
- None
Changes for v5:
- None
Changes for v4:
- None
Changes for v3:
- None
Changes for v2:
- Added comment about syscon required parameter.
---
 .../bindings/misc/aspeed-p2a-ctrl.txt         | 48 +++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/aspeed-p2a-ctrl.txt

diff --git a/Documentation/devicetree/bindings/misc/aspeed-p2a-ctrl.txt b/Documentation/devicetree/bindings/misc/aspeed-p2a-ctrl.txt
new file mode 100644
index 0000000000000..088cc4e3dc54b
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/aspeed-p2a-ctrl.txt
@@ -0,0 +1,48 @@
+======================================================================
+Device tree bindings for Aspeed AST2400/AST2500 PCI-to-AHB Bridge Control Driver
+======================================================================
+
+The bridge is available on platforms with the VGA enabled on the Aspeed device.
+In this case, the host has access to a 64KiB window into all of the BMC's
+memory.  The BMC can disable this bridge.  If the bridge is enabled, the host
+has read access to all the regions of memory, however the host only has read
+and write access depending on a register controlled by the BMC.
+
+Required properties:
+===================
+
+ - compatible: must be one of:
+	- "aspeed,ast2400-p2a-ctrl"
+	- "aspeed,ast2500-p2a-ctrl"
+
+ - syscon: handle to syscon device node controlling PCI.
+
+Optional properties:
+===================
+
+- memory-region: A phandle to a reserved_memory region to be used for the PCI
+		to AHB mapping
+
+The p2a-control node should be the child of a syscon node with the required
+property:
+
+- compatible : Should be one of the following:
+		"aspeed,ast2400-scu", "syscon", "simple-mfd"
+		"aspeed,g4-scu", "syscon", "simple-mfd"
+		"aspeed,ast2500-scu", "syscon", "simple-mfd"
+		"aspeed,g5-scu", "syscon", "simple-mfd"
+
+Example:
+
+g4 Example
+----------
+
+syscon: scu at 1e6e2000 {
+	compatible = "aspeed,ast2400-scu", "syscon", "simple-mfd";
+	reg = <0x1e6e2000 0x1a8>;
+
+	p2a: p2a-control {
+		compatible = "aspeed,ast2400-p2a-ctrl";
+		memory-region = <&reserved_memory>;
+	};
+};
-- 
2.21.0.392.gf8f6787159e-goog


^ permalink raw reply related

* [PATCH v8 2/2] drivers/misc: Add Aspeed P2A control driver
From: Patrick Venture @ 2019-03-27 21:22 UTC (permalink / raw)
  To: linux-aspeed

The ASPEED AST2400, and AST2500 in some configurations include a
PCI-to-AHB MMIO bridge.  This bridge allows a server to read and write
in the BMC's physical address space.  This feature is especially useful
when using this bridge to send large files to the BMC.

The host may use this to send down a firmware image by staging data at a
specific memory address, and in a coordinated effort with the BMC's
software stack and kernel, transmit the bytes.

This driver enables the BMC to unlock the PCI bridge on demand, and
configure it via ioctl to allow the host to write bytes to an agreed
upon location.  In the primary use-case, the region to use is known
apriori on the BMC, and the host requests this information.  Once this
request is received, the BMC's software stack will enable the bridge and
the region and then using some software flow control (possibly via IPMI
packets), copy the bytes down.  Once the process is complete, the BMC
will disable the bridge and unset any region involved.

The default behavior of this bridge when present is: enabled and all
regions marked read-write.  This driver will fix the regions to be
read-only and then disable the bridge entirely.

The memory regions protected are:
 * BMC flash MMIO window
 * System flash MMIO windows
 * SOC IO (peripheral MMIO)
 * DRAM

The DRAM region itself is all of DRAM and cannot be further specified.
Once the PCI bridge is enabled, the host can read all of DRAM, and if
the DRAM section is write-enabled, then it can write to all of it.

Signed-off-by: Patrick Venture <venture@google.com>
---
Changes for v8:
 - Promoted u32 address values to u64 to be compatible with either.
Changes for v7:
- Moved node under the syscon node and changed therefore how it grabs the phandle for the regmap.
Changes for v6:
- Cleaned up documentation
- Added missing machine-readable copyright lines.
- Fixed over 80 chars instances.
- Changed error from invalid memory-region node to ENODEV.
Changes for v5:
- Fixup missing exit condition and remove extra jump.
Changes for v4:
- Added ioctl for reading back the memory-region configuration.
- Cleaned up some unused variables.
Changes for v3:
- Deleted unused read and write methods.
Changes for v2:
- Dropped unused reads.
- Moved call to disable bridge to before registering device.
- Switch from using regs to using a syscon regmap. <<< IN PROGRESS
- Updated the commit message. <<< TODO
- Updated the bit flipped for SCU180_ENP2A
- Dropped boolean region_specified variable.
- Renamed p2a_ctrl in _probe to misc_ctrl per suggestion.
- Renamed aspeed_p2a_region_search to aspeed_p2a_region_acquire
- Updated commit message.
---
 drivers/misc/Kconfig                 |   8 +
 drivers/misc/Makefile                |   1 +
 drivers/misc/aspeed-p2a-ctrl.c       | 448 +++++++++++++++++++++++++++
 include/uapi/linux/aspeed-p2a-ctrl.h |  62 ++++
 4 files changed, 519 insertions(+)
 create mode 100644 drivers/misc/aspeed-p2a-ctrl.c
 create mode 100644 include/uapi/linux/aspeed-p2a-ctrl.h

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 42ab8ec92a046..3209ee020b153 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -496,6 +496,14 @@ config VEXPRESS_SYSCFG
 	  bus. System Configuration interface is one of the possible means
 	  of generating transactions on this bus.
 
+config ASPEED_P2A_CTRL
+	depends on (ARCH_ASPEED || COMPILE_TEST) && REGMAP && MFD_SYSCON
+	tristate "Aspeed ast2400/2500 HOST P2A VGA MMIO to BMC bridge control"
+	help
+	  Control Aspeed ast2400/2500 HOST P2A VGA MMIO to BMC mappings through
+	  ioctl()s, the driver also provides an interface for userspace mappings to
+	  a pre-defined region.
+
 config ASPEED_LPC_CTRL
 	depends on (ARCH_ASPEED || COMPILE_TEST) && REGMAP && MFD_SYSCON
 	tristate "Aspeed ast2400/2500 HOST LPC to BMC bridge control"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index d5b7d3404dc78..c36239573a5ca 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -56,6 +56,7 @@ obj-$(CONFIG_VEXPRESS_SYSCFG)	+= vexpress-syscfg.o
 obj-$(CONFIG_CXL_BASE)		+= cxl/
 obj-$(CONFIG_ASPEED_LPC_CTRL)	+= aspeed-lpc-ctrl.o
 obj-$(CONFIG_ASPEED_LPC_SNOOP)	+= aspeed-lpc-snoop.o
+obj-$(CONFIG_ASPEED_P2A_CTRL)	+= aspeed-p2a-ctrl.o
 obj-$(CONFIG_PCI_ENDPOINT_TEST)	+= pci_endpoint_test.o
 obj-$(CONFIG_OCXL)		+= ocxl/
 obj-y				+= cardreader/
diff --git a/drivers/misc/aspeed-p2a-ctrl.c b/drivers/misc/aspeed-p2a-ctrl.c
new file mode 100644
index 0000000000000..06afbfe51a279
--- /dev/null
+++ b/drivers/misc/aspeed-p2a-ctrl.c
@@ -0,0 +1,448 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2019 Google Inc
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ * Provides a simple driver to control the ASPEED P2A interface which allows
+ * the host to read and write to various regions of the BMC's memory.
+ */
+
+#include <linux/fs.h>
+#include <linux/io.h>
+#include <linux/mfd/syscon.h>
+#include <linux/miscdevice.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+
+#include <linux/aspeed-p2a-ctrl.h>
+
+#define DEVICE_NAME	"aspeed-p2a-ctrl"
+
+/* SCU2C is a Misc. Control Register. */
+#define SCU2C 0x2c
+/* SCU180 is the PCIe Configuration Setting Control Register. */
+#define SCU180 0x180
+/* Bit 1 controls the P2A bridge, while bit 0 controls the entire VGA device
+ * on the PCI bus.
+ */
+#define SCU180_ENP2A BIT(1)
+
+/* The ast2400/2500 both have six ranges. */
+#define P2A_REGION_COUNT 6
+
+struct region {
+	u64 min;
+	u64 max;
+	u32 bit;
+};
+
+struct aspeed_p2a_model_data {
+	/* min, max, bit */
+	struct region regions[P2A_REGION_COUNT];
+};
+
+struct aspeed_p2a_ctrl {
+	struct miscdevice miscdev;
+	struct regmap *regmap;
+
+	const struct aspeed_p2a_model_data *config;
+
+	/* Access to these needs to be locked, held via probe, mapping ioctl,
+	 * and release, remove.
+	 */
+	struct mutex tracking;
+	u32 readers;
+	u32 readerwriters[P2A_REGION_COUNT];
+
+	phys_addr_t mem_base;
+	resource_size_t mem_size;
+};
+
+struct aspeed_p2a_user {
+	struct file *file;
+	struct aspeed_p2a_ctrl *parent;
+
+	/* The entire memory space is opened for reading once the bridge is
+	 * enabled, therefore this needs only to be tracked once per user.
+	 * If any user has it open for read, the bridge must stay enabled.
+	 */
+	u32 read;
+
+	/* Each entry of the array corresponds to a P2A Region.  If the user
+	 * opens for read or readwrite, the reference goes up here.  On
+	 * release, this array is walked and references adjusted accordingly.
+	 */
+	u32 readwrite[P2A_REGION_COUNT];
+};
+
+static void aspeed_p2a_enable_bridge(struct aspeed_p2a_ctrl *p2a_ctrl)
+{
+	regmap_update_bits(p2a_ctrl->regmap,
+		SCU180, SCU180_ENP2A, SCU180_ENP2A);
+}
+
+static void aspeed_p2a_disable_bridge(struct aspeed_p2a_ctrl *p2a_ctrl)
+{
+	regmap_update_bits(p2a_ctrl->regmap, SCU180, SCU180_ENP2A, 0);
+}
+
+static int aspeed_p2a_mmap(struct file *file, struct vm_area_struct *vma)
+{
+	struct aspeed_p2a_user *priv = file->private_data;
+	struct aspeed_p2a_ctrl *ctrl = priv->parent;
+
+	if (ctrl->mem_base == 0 && ctrl->mem_size == 0)
+		return -EINVAL;
+
+	unsigned long vsize = vma->vm_end - vma->vm_start;
+	pgprot_t prot = vma->vm_page_prot;
+
+	if (vma->vm_pgoff + vsize > ctrl->mem_base + ctrl->mem_size)
+		return -EINVAL;
+
+	/* ast2400/2500 AHB accesses are not cache coherent */
+	prot = pgprot_noncached(prot);
+
+	if (remap_pfn_range(vma, vma->vm_start,
+		(ctrl->mem_base >> PAGE_SHIFT) + vma->vm_pgoff,
+		vsize, prot))
+		return -EAGAIN;
+
+	return 0;
+}
+
+static bool aspeed_p2a_region_acquire(struct aspeed_p2a_user *priv,
+		struct aspeed_p2a_ctrl *ctrl,
+		struct aspeed_p2a_ctrl_mapping *map)
+{
+	int i;
+	u64 base, end;
+	bool matched = false;
+
+	base = map->addr;
+	end = map->addr + (map->length - 1);
+
+	/* If the value is a legal u32, it will find a match. */
+	for (i = 0; i < P2A_REGION_COUNT; i++) {
+		const struct region *curr = &ctrl->config->regions[i];
+
+		/* If the top of this region is lower than your base, skip it.
+		 */
+		if (curr->max < base)
+			continue;
+
+		/* If the bottom of this region is higher than your end, bail.
+		 */
+		if (curr->min > end)
+			break;
+
+		/* Lock this and update it, therefore it someone else is
+		 * closing their file out, this'll preserve the increment.
+		 */
+		mutex_lock(&ctrl->tracking);
+		ctrl->readerwriters[i] += 1;
+		mutex_unlock(&ctrl->tracking);
+
+		/* Track with the user, so when they close their file, we can
+		 * decrement properly.
+		 */
+		priv->readwrite[i] += 1;
+
+		/* Enable the region as read-write. */
+		regmap_update_bits(ctrl->regmap, SCU2C, curr->bit, 0);
+		matched = true;
+	}
+
+	return matched;
+}
+
+static long aspeed_p2a_ioctl(struct file *file, unsigned int cmd,
+		unsigned long data)
+{
+	struct aspeed_p2a_user *priv = file->private_data;
+	struct aspeed_p2a_ctrl *ctrl = priv->parent;
+	void __user *arg = (void __user *)data;
+	struct aspeed_p2a_ctrl_mapping map;
+
+	if (copy_from_user(&map, arg, sizeof(map)))
+		return -EFAULT;
+
+	switch (cmd) {
+	case ASPEED_P2A_CTRL_IOCTL_SET_WINDOW:
+		/* If they want a region to be read-only, since the entire
+		 * region is read-only once enabled, we just need to track this
+		 * user wants to read from the bridge, and if it's not enabled.
+		 * Enable it.
+		 */
+		if (map.flags == ASPEED_P2A_CTRL_READ_ONLY) {
+			mutex_lock(&ctrl->tracking);
+			ctrl->readers += 1;
+			mutex_unlock(&ctrl->tracking);
+
+			/* Track with the user, so when they close their file,
+			 * we can decrement properly.
+			 */
+			priv->read += 1;
+		} else if (map.flags == ASPEED_P2A_CTRL_READWRITE) {
+			/* If we don't acquire any region return error. */
+			if (!aspeed_p2a_region_acquire(priv, ctrl, &map)) {
+				return -EINVAL;
+			}
+		} else {
+			/* Invalid map flags. */
+			return -EINVAL;
+		}
+
+		aspeed_p2a_enable_bridge(ctrl);
+		return 0;
+	case ASPEED_P2A_CTRL_IOCTL_GET_MEMORY_CONFIG:
+		/* This is a request for the memory-region and corresponding
+		 * length that is used by the driver for mmap.
+		 */
+
+		map.flags = 0;
+		map.addr = ctrl->mem_base;
+		map.length = ctrl->mem_size;
+
+		return copy_to_user(arg, &map, sizeof(map)) ? -EFAULT : 0;
+	}
+
+	return -EINVAL;
+}
+
+
+/*
+ * When a user opens this file, we create a structure to track their mappings.
+ *
+ * A user can map a region as read-only (bridge enabled), or read-write (bit
+ * flipped, and bridge enabled).  Either way, this tracking is used, s.t. when
+ * they release the device references are handled.
+ *
+ * The bridge is not enabled until a user calls an ioctl to map a region,
+ * simply opening the device does not enable it.
+ */
+static int aspeed_p2a_open(struct inode *inode, struct file *file)
+{
+	struct aspeed_p2a_user *priv;
+
+	priv = kmalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->file = file;
+	priv->read = 0;
+	memset(priv->readwrite, 0, sizeof(priv->readwrite));
+
+	/* The file's private_data is initialized to the p2a_ctrl. */
+	priv->parent = file->private_data;
+
+	/* Set the file's private_data to the user's data. */
+	file->private_data = priv;
+
+	return 0;
+}
+
+/*
+ * This will close the users mappings.  It will go through what they had opened
+ * for readwrite, and decrement those counts.  If at the end, this is the last
+ * user, it'll close the bridge.
+ */
+static int aspeed_p2a_release(struct inode *inode, struct file *file)
+{
+	int i;
+	u32 value;
+	u32 bits = 0;
+	bool open_regions = false;
+	struct aspeed_p2a_user *priv = file->private_data;
+
+	/* Lock others from changing these values until everything is updated
+	 * in one pass.
+	 */
+	mutex_lock(&priv->parent->tracking);
+
+	priv->parent->readers -= priv->read;
+
+	for (i = 0; i < P2A_REGION_COUNT; i++) {
+		priv->parent->readerwriters[i] -= priv->readwrite[i];
+
+		if (priv->parent->readerwriters[i] > 0)
+			open_regions = true;
+		else
+			bits |= priv->parent->config->regions[i].bit;
+	}
+
+	/* Setting a bit to 1 disables the region, so let's just OR with the
+	 * above to disable any.
+	 */
+
+	/* Note, if another user is trying to ioctl, they can't grab tracking,
+	 * and therefore can't grab either register mutex.
+	 * If another user is trying to close, they can't grab tracking either.
+	 */
+	regmap_update_bits(priv->parent->regmap, SCU2C, bits, bits);
+
+	/* If parent->readers is zero and open windows is 0, disable the
+	 * bridge.
+	 */
+	if (!open_regions && priv->parent->readers == 0)
+		aspeed_p2a_disable_bridge(priv->parent);
+
+	mutex_unlock(&priv->parent->tracking);
+
+	kfree(priv);
+
+	return 0;
+}
+
+static const struct file_operations aspeed_p2a_ctrl_fops = {
+	.owner = THIS_MODULE,
+	.mmap = aspeed_p2a_mmap,
+	.unlocked_ioctl = aspeed_p2a_ioctl,
+	.open = aspeed_p2a_open,
+	.release = aspeed_p2a_release,
+};
+
+/* The regions are controlled by SCU2C */
+static void aspeed_p2a_disable_all(struct aspeed_p2a_ctrl *p2a_ctrl)
+{
+	int i;
+	u32 value = 0;
+
+	for (i = 0; i < P2A_REGION_COUNT; i++)
+		value |= p2a_ctrl->config->regions[i].bit;
+
+	regmap_update_bits(p2a_ctrl->regmap, SCU2C, value, value);
+
+	/* Disable the bridge. */
+	aspeed_p2a_disable_bridge(p2a_ctrl);
+}
+
+static int aspeed_p2a_ctrl_probe(struct platform_device *pdev)
+{
+	struct aspeed_p2a_ctrl *misc_ctrl;
+	struct device *dev;
+	struct resource *res, resm;
+	struct device_node *node;
+	int rc = 0;
+
+	dev = &pdev->dev;
+
+	misc_ctrl = devm_kzalloc(dev, sizeof(*misc_ctrl), GFP_KERNEL);
+	if (!misc_ctrl)
+		return -ENOMEM;
+
+	mutex_init(&misc_ctrl->tracking);
+	misc_ctrl->readers = 0;
+	memset(misc_ctrl->readerwriters, 0, sizeof(misc_ctrl->readerwriters));
+
+	misc_ctrl->mem_size = 0;
+	misc_ctrl->mem_base = 0;
+
+	/* optional. */
+	node = of_parse_phandle(dev->of_node, "memory-region", 0);
+	if (node) {
+		rc = of_address_to_resource(node, 0, &resm);
+		of_node_put(node);
+		if (rc) {
+			dev_err(dev, "Couldn't address to resource for reserved memory\n");
+			return -ENODEV;
+		}
+
+		misc_ctrl->mem_size = resource_size(&resm);
+		misc_ctrl->mem_base = resm.start;
+	}
+
+	misc_ctrl->regmap = syscon_node_to_regmap(pdev->dev.parent->of_node);
+	if (IS_ERR(misc_ctrl->regmap)) {
+		dev_err(dev, "Couldn't get regmap\n");
+		return -ENODEV;
+	}
+
+	misc_ctrl->config = of_device_get_match_data(dev);
+
+	dev_set_drvdata(&pdev->dev, misc_ctrl);
+
+	aspeed_p2a_disable_all(misc_ctrl);
+
+	misc_ctrl->miscdev.minor = MISC_DYNAMIC_MINOR;
+	misc_ctrl->miscdev.name = DEVICE_NAME;
+	misc_ctrl->miscdev.fops = &aspeed_p2a_ctrl_fops;
+	misc_ctrl->miscdev.parent = dev;
+
+	rc = misc_register(&misc_ctrl->miscdev);
+	if (rc)
+		dev_err(dev, "Unable to register device\n");
+
+	return rc;
+}
+
+static int aspeed_p2a_ctrl_remove(struct platform_device *pdev)
+{
+	struct aspeed_p2a_ctrl *p2a_ctrl = dev_get_drvdata(&pdev->dev);
+
+	misc_deregister(&p2a_ctrl->miscdev);
+
+	return 0;
+}
+
+#define SCU2C_DRAM	BIT(25)
+#define SCU2C_SPI	BIT(24)
+#define SCU2C_SOC	BIT(23)
+#define SCU2C_FLASH	BIT(22)
+
+static const struct aspeed_p2a_model_data ast2400_model_data = {
+	.regions = {
+		{0x00000000, 0x17FFFFFF, SCU2C_FLASH},
+		{0x18000000, 0x1FFFFFFF, SCU2C_SOC},
+		{0x20000000, 0x2FFFFFFF, SCU2C_FLASH},
+		{0x30000000, 0x3FFFFFFF, SCU2C_SPI},
+		{0x40000000, 0x5FFFFFFF, SCU2C_DRAM},
+		{0x60000000, 0xFFFFFFFF, SCU2C_SOC},
+	}
+};
+
+static const struct aspeed_p2a_model_data ast2500_model_data = {
+	.regions = {
+		{0x00000000, 0x0FFFFFFF, SCU2C_FLASH},
+		{0x10000000, 0x1FFFFFFF, SCU2C_SOC},
+		{0x20000000, 0x3FFFFFFF, SCU2C_FLASH},
+		{0x40000000, 0x5FFFFFFF, SCU2C_SOC},
+		{0x60000000, 0x7FFFFFFF, SCU2C_SPI},
+		{0x80000000, 0xFFFFFFFF, SCU2C_DRAM},
+	}
+};
+
+static const struct of_device_id aspeed_p2a_ctrl_match[] = {
+	{ .compatible = "aspeed,ast2400-p2a-ctrl",
+	  .data = &ast2400_model_data },
+	{ .compatible = "aspeed,ast2500-p2a-ctrl",
+	  .data = &ast2500_model_data },
+	{ },
+};
+
+static struct platform_driver aspeed_p2a_ctrl_driver = {
+	.driver = {
+		.name		= DEVICE_NAME,
+		.of_match_table = aspeed_p2a_ctrl_match,
+	},
+	.probe = aspeed_p2a_ctrl_probe,
+	.remove = aspeed_p2a_ctrl_remove,
+};
+
+module_platform_driver(aspeed_p2a_ctrl_driver);
+
+MODULE_DEVICE_TABLE(of, aspeed_p2a_ctrl_match);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Patrick Venture <venture@google.com>");
+MODULE_DESCRIPTION("Control for aspeed 2400/2500 P2A VGA HOST to BMC mappings");
diff --git a/include/uapi/linux/aspeed-p2a-ctrl.h b/include/uapi/linux/aspeed-p2a-ctrl.h
new file mode 100644
index 0000000000000..033355552a6e3
--- /dev/null
+++ b/include/uapi/linux/aspeed-p2a-ctrl.h
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
+/*
+ * Copyright 2019 Google Inc
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ * Provides a simple driver to control the ASPEED P2A interface which allows
+ * the host to read and write to various regions of the BMC's memory.
+ */
+
+#ifndef _UAPI_LINUX_ASPEED_P2A_CTRL_H
+#define _UAPI_LINUX_ASPEED_P2A_CTRL_H
+
+#include <linux/ioctl.h>
+#include <linux/types.h>
+
+#define ASPEED_P2A_CTRL_READ_ONLY 0
+#define ASPEED_P2A_CTRL_READWRITE 1
+
+/*
+ * This driver provides a mechanism for enabling or disabling the read-write
+ * property of specific windows into the ASPEED BMC's memory.
+ *
+ * A user can map a region of the BMC's memory as read-only or read-write, with
+ * the caveat that once any region is mapped, all regions are unlocked for
+ * reading.
+ */
+
+/*
+ * Unlock a region of BMC physical memory for access from the host.
+ *
+ * Also used to read back the optional memory-region configuration for the
+ * driver.
+ */
+struct aspeed_p2a_ctrl_mapping {
+	__u64 addr;
+	__u32 length;
+	__u32 flags;
+};
+
+#define __ASPEED_P2A_CTRL_IOCTL_MAGIC 0xb3
+
+/*
+ * This IOCTL is meant to configure a region or regions of memory given a
+ * starting address and length to be readable by the host, or
+ * readable-writeable.
+ */
+#define ASPEED_P2A_CTRL_IOCTL_SET_WINDOW _IOW(__ASPEED_P2A_CTRL_IOCTL_MAGIC, \
+		0x00, struct aspeed_p2a_ctrl_mapping)
+
+/*
+ * This IOCTL is meant to read back to the user the base address and length of
+ * the memory-region specified to the driver for use with mmap.
+ */
+#define ASPEED_P2A_CTRL_IOCTL_GET_MEMORY_CONFIG \
+	_IOWR(__ASPEED_P2A_CTRL_IOCTL_MAGIC, \
+		0x01, struct aspeed_p2a_ctrl_mapping)
+
+#endif /* _UAPI_LINUX_ASPEED_P2A_CTRL_H */
-- 
2.21.0.392.gf8f6787159e-goog


^ permalink raw reply related

* [RFC linux 0/2] rtc: aspeed: Add alarm support
From: Joel Stanley @ 2019-03-28  0:18 UTC (permalink / raw)
  To: linux-aspeed

These are some work in progress patches to add alarm support on top of
the recently upstreamed RTC driver.

TODO:
 - Make rtctest pass. The select() tests fail currently
 - Add locking to the alarm callbacks

Joel Stanley (2):
  rtc: aspeed: Add locking when touching registers
  rtc: aspeed: Add alarm support

 drivers/rtc/rtc-aspeed.c | 109 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 108 insertions(+), 1 deletion(-)

-- 
2.20.1


^ permalink raw reply

* [RFC linux 1/2] rtc: aspeed: Add locking when touching registers
From: Joel Stanley @ 2019-03-28  0:18 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190328001832.11076-1-joel@jms.id.au>

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/rtc/rtc-aspeed.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/rtc/rtc-aspeed.c b/drivers/rtc/rtc-aspeed.c
index af3eb676d7c3..a423ce6483f8 100644
--- a/drivers/rtc/rtc-aspeed.c
+++ b/drivers/rtc/rtc-aspeed.c
@@ -10,6 +10,7 @@
 struct aspeed_rtc {
 	struct rtc_device *rtc_dev;
 	void __iomem *base;
+	spinlock_t lock;
 };
 
 #define RTC_TIME	0x00
@@ -25,7 +26,10 @@ static int aspeed_rtc_read_time(struct device *dev, struct rtc_time *tm)
 	unsigned int cent, year;
 	u32 reg1, reg2;
 
+	spin_lock_irqsave(&rtc->lock, flags);
+
 	if (!(readl(rtc->base + RTC_CTRL) & RTC_ENABLE)) {
+		spin_unlock_irqrestore(&rtc->lock, flags);
 		dev_dbg(dev, "%s failing as rtc disabled\n", __func__);
 		return -EINVAL;
 	}
@@ -47,6 +51,8 @@ static int aspeed_rtc_read_time(struct device *dev, struct rtc_time *tm)
 
 	dev_dbg(dev, "%s %ptR", __func__, tm);
 
+	spin_unlock_irqrestore(&rtc->lock, flags);
+
 	return 0;
 }
 
@@ -65,6 +71,8 @@ static int aspeed_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	reg2 = ((cent & 0x1f) << 16) | ((year & 0x7f) << 8) |
 		((tm->tm_mon + 1) & 0xf);
 
+	spin_lock_irqsave(&rtc->lock, flags);
+
 	ctrl = readl(rtc->base + RTC_CTRL);
 	writel(ctrl | RTC_UNLOCK, rtc->base + RTC_CTRL);
 
@@ -74,6 +82,8 @@ static int aspeed_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	/* Re-lock and ensure enable is set now that a time is programmed */
 	writel(ctrl | RTC_ENABLE, rtc->base + RTC_CTRL);
 
+	spin_unlock_irqrestore(&rtc->lock, flags);
+
 	return 0;
 }
 
@@ -111,6 +121,8 @@ static int aspeed_rtc_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	spin_lock_init(&rtc->lock);
+
 	return 0;
 }
 
-- 
2.20.1


^ permalink raw reply related

* [RFC linux 2/2] rtc: aspeed: Add alarm support
From: Joel Stanley @ 2019-03-28  0:18 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190328001832.11076-1-joel@jms.id.au>

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/rtc/rtc-aspeed.c | 97 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 96 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-aspeed.c b/drivers/rtc/rtc-aspeed.c
index a423ce6483f8..8b6163567d10 100644
--- a/drivers/rtc/rtc-aspeed.c
+++ b/drivers/rtc/rtc-aspeed.c
@@ -11,12 +11,23 @@ struct aspeed_rtc {
 	struct rtc_device *rtc_dev;
 	void __iomem *base;
 	spinlock_t lock;
+	int irq;
 };
 
 #define RTC_TIME	0x00
 #define RTC_YEAR	0x04
+#define RTC_ALARM	0x08
 #define RTC_CTRL	0x10
-
+#define RTC_ALARM_STS	0x14
+
+/* Control register */
+#define ALARM_WAKEUP	BIT(8)
+#define ALARM_IRQ_SEC	BIT(7)
+#define ALARM_DAY	BIT(6)
+#define ALARM_HOUR	BIT(5)
+#define ALARM_MIN	BIT(4)
+#define ALARM_SEC	BIT(3)
+#define ALARM_COMBINED	BIT(2)
 #define RTC_UNLOCK	BIT(1)
 #define RTC_ENABLE	BIT(0)
 
@@ -87,9 +98,81 @@ static int aspeed_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	return 0;
 }
 
+static int aspeed_rtc_alarm_irq_enable(struct device *dev, unsigned int en)
+{
+	struct aspeed_rtc *rtc = dev_get_drvdata(dev);
+	u32 reg = readl(rtc->base + RTC_CTRL);
+
+	if (en)
+		reg |= ALARM_DAY | ALARM_HOUR | ALARM_MIN | ALARM_SEC |
+			ALARM_COMBINED;
+
+	writel(reg, rtc->base + RTC_CTRL);
+
+	return 0;
+}
+
+static int aspeed_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
+{
+	struct aspeed_rtc *rtc = dev_get_drvdata(dev);
+	u32 reg;
+
+	reg = readl(rtc->base + RTC_ALARM);
+
+	alarm->time.tm_mday = (reg >> 24) & 0x1f;
+	alarm->time.tm_hour = (reg >> 16) & 0x1f;
+	alarm->time.tm_min = (reg >> 8) & 0x3f;
+	alarm->time.tm_sec = reg & 0x3f;
+
+	return 0;
+}
+
+
+static int aspeed_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
+{
+	struct aspeed_rtc *rtc = dev_get_drvdata(dev);
+	u8 day, hour, min, sec;
+	u32 reg;
+
+	aspeed_rtc_alarm_irq_enable(dev, 0);
+
+	day = alarm->time.tm_mday & 0x1f;
+	hour = alarm->time.tm_hour & 0x1f;
+	min = alarm->time.tm_min & 0x3f;
+	sec = alarm->time.tm_sec & 0x3f;
+
+	reg = (day << 24) | (hour << 16) | (min << 8) | sec;
+
+	writel(reg, rtc->base + RTC_ALARM);
+
+	aspeed_rtc_alarm_irq_enable(dev, alarm->enabled);
+
+	return 0;
+}
+
+static irqreturn_t aspeed_rtc_irq_handler(int irq, void *id)
+{
+	struct device *dev = id;
+	struct aspeed_rtc *rtc = dev_get_drvdata(dev);
+	u32 reg;
+
+	reg = readl(rtc->base + RTC_ALARM_STS);
+	if (reg & BIT(0))
+		return IRQ_NONE;
+
+	writel(reg, rtc->base + RTC_ALARM_STS);
+	rtc_update_irq(rtc->rtc_dev, 1, RTC_IRQF | RTC_AF);
+	return IRQ_HANDLED;
+}
+
+
+
 static const struct rtc_class_ops aspeed_rtc_ops = {
 	.read_time = aspeed_rtc_read_time,
 	.set_time = aspeed_rtc_set_time,
+	.read_alarm = aspeed_rtc_read_alarm,
+	.set_alarm = aspeed_rtc_set_alarm,
+	.alarm_irq_enable = aspeed_rtc_alarm_irq_enable,
 };
 
 static int aspeed_rtc_probe(struct platform_device *pdev)
@@ -107,6 +190,10 @@ static int aspeed_rtc_probe(struct platform_device *pdev)
 	if (IS_ERR(rtc->base))
 		return PTR_ERR(rtc->base);
 
+	rtc->irq = platform_get_irq(pdev, 0);
+	if (rtc->irq < 0)
+		return -EINVAL;
+
 	rtc->rtc_dev = devm_rtc_allocate_device(&pdev->dev);
 	if (IS_ERR(rtc->rtc_dev))
 		return PTR_ERR(rtc->rtc_dev);
@@ -117,6 +204,14 @@ static int aspeed_rtc_probe(struct platform_device *pdev)
 	rtc->rtc_dev->range_min = RTC_TIMESTAMP_BEGIN_1900;
 	rtc->rtc_dev->range_max = 38814989399LL; /* 3199-12-31 23:59:59 */
 
+	ret = devm_request_irq(&pdev->dev, rtc->irq,
+			       aspeed_rtc_irq_handler, 0,
+			       dev_name(&pdev->dev), &pdev->dev);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to request irq\n");
+		return ret;
+	}
+
 	ret = rtc_register_device(rtc->rtc_dev);
 	if (ret)
 		return ret;
-- 
2.20.1


^ permalink raw reply related

* [PATCH 0/2] drm: Add ASPEED BMC 'GFX' driver
From: Joel Stanley @ 2019-03-28  5:43 UTC (permalink / raw)
  To: linux-aspeed

This driver is for the ASPEED BMC SoC's GFX display hardware. This
driver runs on the ARM based BMC systems, unlike the ast driver which
runs on a host CPU and is is for a PCIe graphics device that happens to
live in the BMC's silicon, but is otherwise available for use by the
BMC.

I submitted a RFC about a year ago.

 https://patchwork.kernel.org/patch/10339605/

I've fixed all of the issues I had in the RFC, and made the cleanups
that Eric suggested in that review. This version is based on
next-20190327.

Joel Stanley (2):
  dt-bindings: gpu: Add ASPEED GFX bindings document
  drm: Add ASPEED GFX driver

 .../devicetree/bindings/gpu/aspeed-gfx.txt    |  41 +++
 drivers/gpu/drm/Kconfig                       |   2 +
 drivers/gpu/drm/Makefile                      |   1 +
 drivers/gpu/drm/aspeed/Kconfig                |  15 +
 drivers/gpu/drm/aspeed/Makefile               |   3 +
 drivers/gpu/drm/aspeed/aspeed_gfx.h           | 104 +++++++
 drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c      | 248 ++++++++++++++++
 drivers/gpu/drm/aspeed/aspeed_gfx_drv.c       | 269 ++++++++++++++++++
 drivers/gpu/drm/aspeed/aspeed_gfx_out.c       |  49 ++++
 9 files changed, 732 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpu/aspeed-gfx.txt
 create mode 100644 drivers/gpu/drm/aspeed/Kconfig
 create mode 100644 drivers/gpu/drm/aspeed/Makefile
 create mode 100644 drivers/gpu/drm/aspeed/aspeed_gfx.h
 create mode 100644 drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
 create mode 100644 drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
 create mode 100644 drivers/gpu/drm/aspeed/aspeed_gfx_out.c

-- 
2.20.1


^ permalink raw reply

* [PATCH 1/2] dt-bindings: gpu: Add ASPEED GFX bindings document
From: Joel Stanley @ 2019-03-28  5:43 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190328054316.17939-1-joel@jms.id.au>

This describes the ASPEED BMC SoC's display controller.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 .../devicetree/bindings/gpu/aspeed-gfx.txt    | 41 +++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpu/aspeed-gfx.txt

diff --git a/Documentation/devicetree/bindings/gpu/aspeed-gfx.txt b/Documentation/devicetree/bindings/gpu/aspeed-gfx.txt
new file mode 100644
index 000000000000..a74033332668
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpu/aspeed-gfx.txt
@@ -0,0 +1,41 @@
+Device tree configuration for the GFX display deivce on the AST2500 SoCs.
+
+Required properties:
+  - compatible
+    * Must be one of the following:
+      + aspeed,ast2500-gfx
+      + aspeed,ast2400-gfx
+    * In addition, the ASPEED pinctrl bindings require the 'syscon' property to
+      be present
+
+  - reg: Physical base address and length of the GFX registers
+
+  - interrupts: interrupt number for the GFX device
+
+  - clocks: clock number used to generate the pixel clock
+
+  - resets: reset line that must be released to use the GFX device
+
+  - memory-region:
+    Phandle to a memory region to allocate from, as defined in
+    Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
+
+
+Example:
+
+gfx: display at 1e6e6000 {
+	compatible = "aspeed,ast2500-gfx", "syscon";
+	reg = <0x1e6e6000 0x1000>;
+	reg-io-width = <4>;
+	clocks = <&syscon ASPEED_CLK_GATE_D1CLK>;
+	resets = <&syscon ASPEED_RESET_CRT1>;
+	interrupts = <0x19>;
+	memory-region = <&gfx_memory>;
+};
+
+gfx_memory: framebuffer {
+	size = <0x01000000>;
+	alignment = <0x01000000>;
+	compatible = "shared-dma-pool";
+	reusable;
+};
-- 
2.20.1


^ permalink raw reply related

* [PATCH 2/2] drm: Add ASPEED GFX driver
From: Joel Stanley @ 2019-03-28  5:43 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190328054316.17939-1-joel@jms.id.au>

This driver is for the ASPEED BMC SoC's GFX display hardware. This
driver runs on the ARM based BMC systems, unlike the ast driver which
runs on a host CPU and is is for a PCI graphics device.

Signed-off-by: Joel Stanley <joel@jms.id.au>
--
Changes since RFC:
 drm_fbdev_cma_init -> drm_fb_cma_fbdev_init and use generic lastclose callback
 Use generic irq handling instead of drm_irq_install
 Add doc to driver
 Get rid of unncessary reads in irq enable/disable path
 Rebase on linux-next

 drivers/gpu/drm/Kconfig                  |   2 +
 drivers/gpu/drm/Makefile                 |   1 +
 drivers/gpu/drm/aspeed/Kconfig           |  15 ++
 drivers/gpu/drm/aspeed/Makefile          |   3 +
 drivers/gpu/drm/aspeed/aspeed_gfx.h      | 104 +++++++++
 drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c | 248 +++++++++++++++++++++
 drivers/gpu/drm/aspeed/aspeed_gfx_drv.c  | 269 +++++++++++++++++++++++
 drivers/gpu/drm/aspeed/aspeed_gfx_out.c  |  49 +++++
 8 files changed, 691 insertions(+)
 create mode 100644 drivers/gpu/drm/aspeed/Kconfig
 create mode 100644 drivers/gpu/drm/aspeed/Makefile
 create mode 100644 drivers/gpu/drm/aspeed/aspeed_gfx.h
 create mode 100644 drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
 create mode 100644 drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
 create mode 100644 drivers/gpu/drm/aspeed/aspeed_gfx_out.c

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 82bb221ec94e..b1ec8f85c2a8 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -335,6 +335,8 @@ source "drivers/gpu/drm/xen/Kconfig"
 
 source "drivers/gpu/drm/vboxvideo/Kconfig"
 
+source "drivers/gpu/drm/aspeed/Kconfig"
+
 # Keep legacy drivers last
 
 menuconfig DRM_LEGACY
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 0baf148e3687..df8835045310 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -110,3 +110,4 @@ obj-$(CONFIG_DRM_PL111) += pl111/
 obj-$(CONFIG_DRM_TVE200) += tve200/
 obj-$(CONFIG_DRM_XEN) += xen/
 obj-$(CONFIG_DRM_VBOXVIDEO) += vboxvideo/
+obj-$(CONFIG_DRM_ASPEED_GFX) += aspeed/
diff --git a/drivers/gpu/drm/aspeed/Kconfig b/drivers/gpu/drm/aspeed/Kconfig
new file mode 100644
index 000000000000..6f1e64c0a6ce
--- /dev/null
+++ b/drivers/gpu/drm/aspeed/Kconfig
@@ -0,0 +1,15 @@
+config DRM_ASPEED_GFX
+	tristate "ASPEED BMC Display Controller"
+	depends on DRM && OF
+	select DRM_KMS_HELPER
+	select DRM_KMS_FB_HELPER
+	select DRM_KMS_CMA_HELPER
+	select DRM_PANEL
+	select DMA_CMA
+	select CMA
+	select MFD_SYSCON
+	help
+	  Chose this option if you have an ASPEED AST2400/AST2500
+	  SOC Display Controller (aka GFX).
+
+	  If M is selected this module will be called aspeed_gfx.
diff --git a/drivers/gpu/drm/aspeed/Makefile b/drivers/gpu/drm/aspeed/Makefile
new file mode 100644
index 000000000000..6e194cd790d8
--- /dev/null
+++ b/drivers/gpu/drm/aspeed/Makefile
@@ -0,0 +1,3 @@
+aspeed_gfx-y := aspeed_gfx_drv.o aspeed_gfx_crtc.o aspeed_gfx_out.o
+
+obj-$(CONFIG_DRM_ASPEED_GFX) += aspeed_gfx.o
diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx.h b/drivers/gpu/drm/aspeed/aspeed_gfx.h
new file mode 100644
index 000000000000..fb56e425bd48
--- /dev/null
+++ b/drivers/gpu/drm/aspeed/aspeed_gfx.h
@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright 2018 IBM Corporation
+
+#include <drm/drmP.h>
+#include <drm/drm_simple_kms_helper.h>
+
+struct aspeed_gfx {
+	void __iomem			*base;
+	struct clk			*clk;
+	struct reset_control		*rst;
+	struct regmap			*scu;
+
+	struct drm_simple_display_pipe	pipe;
+	struct drm_connector		connector;
+	struct drm_fbdev_cma		*fbdev;
+};
+
+int aspeed_gfx_create_pipe(struct drm_device *drm);
+int aspeed_gfx_create_output(struct drm_device *drm);
+
+#define CRT_CTRL1		0x60 /* CRT Control I */
+#define CRT_CTRL2		0x64 /* CRT Control II */
+#define CRT_STATUS		0x68 /* CRT Status */
+#define CRT_MISC		0x6c /* CRT Misc Setting */
+#define CRT_HORIZ0		0x70 /* CRT Horizontal Total & Display Enable End */
+#define CRT_HORIZ1		0x74 /* CRT Horizontal Retrace Start & End */
+#define CRT_VERT0		0x78 /* CRT Vertical Total & Display Enable End */
+#define CRT_VERT1		0x7C /* CRT Vertical Retrace Start & End */
+#define CRT_ADDR		0x80 /* CRT Display Starting Address */
+#define CRT_OFFSET		0x84 /* CRT Display Offset & Terminal Count */
+#define CRT_THROD		0x88 /* CRT Threshold */
+#define CRT_XSCALE		0x8C /* CRT Scaling-Up Factor */
+#define CRT_CURSOR0		0x90 /* CRT Hardware Cursor X & Y Offset */
+#define CRT_CURSOR1		0x94 /* CRT Hardware Cursor X & Y Position */
+#define CRT_CURSOR2		0x98 /* CRT Hardware Cursor Pattern Address */
+#define CRT_9C			0x9C
+#define CRT_OSD_H		0xA0 /* CRT OSD Horizontal Start/End */
+#define CRT_OSD_V		0xA4 /* CRT OSD Vertical Start/End */
+#define CRT_OSD_ADDR		0xA8 /* CRT OSD Pattern Address */
+#define CRT_OSD_DISP		0xAC /* CRT OSD Offset */
+#define CRT_OSD_THRESH		0xB0 /* CRT OSD Threshold & Alpha */
+#define CRT_B4			0xB4
+#define CRT_STS_V		0xB8 /* CRT Status V */
+#define CRT_SCRATCH		0xBC /* Scratchpad */
+#define CRT_BB0_ADDR		0xD0 /* CRT Display BB0 Starting Address */
+#define CRT_BB1_ADDR		0xD4 /* CRT Display BB1 Starting Address */
+#define CRT_BB_COUNT		0xD8 /* CRT Display BB Terminal Count */
+#define OSD_COLOR1		0xE0 /* OSD Color Palette Index 1 & 0 */
+#define OSD_COLOR2		0xE4 /* OSD Color Palette Index 3 & 2 */
+#define OSD_COLOR3		0xE8 /* OSD Color Palette Index 5 & 4 */
+#define OSD_COLOR4		0xEC /* OSD Color Palette Index 7 & 6 */
+#define OSD_COLOR5		0xF0 /* OSD Color Palette Index 9 & 8 */
+#define OSD_COLOR6		0xF4 /* OSD Color Palette Index 11 & 10 */
+#define OSD_COLOR7		0xF8 /* OSD Color Palette Index 13 & 12 */
+#define OSD_COLOR8		0xFC /* OSD Color Palette Index 15 & 14 */
+
+/* CTRL1 */
+#define CRT_CTRL_EN			BIT(0)
+#define CRT_CTRL_HW_CURSOR_EN		BIT(1)
+#define CRT_CTRL_OSD_EN			BIT(2)
+#define CRT_CTRL_INTERLACED		BIT(3)
+#define CRT_CTRL_COLOR_RGB565		(0 << 7)
+#define CRT_CTRL_COLOR_YUV444		(1 << 7)
+#define CRT_CTRL_COLOR_XRGB8888		(2 << 7)
+#define CRT_CTRL_COLOR_RGB888		(3 << 7)
+#define CRT_CTRL_COLOR_YUV444_2RGB	(5 << 7)
+#define CRT_CTRL_COLOR_YUV422		(7 << 7)
+#define CRT_CTRL_COLOR_MASK		GENMASK(9, 7)
+#define CRT_CTRL_HSYNC_NEGATIVE		BIT(16)
+#define CRT_CTRL_VSYNC_NEGATIVE		BIT(17)
+#define CRT_CTRL_VERTICAL_INTR_EN	BIT(30)
+#define CRT_CTRL_VERTICAL_INTR_STS	BIT(31)
+
+/* CTRL2 */
+#define CRT_CTRL_DAC_EN			BIT(0)
+#define CRT_CTRL_VBLANK_LINE(x)		(((x) << 20) & CRT_CTRL_VBLANK_LINE_MASK)
+#define CRT_CTRL_VBLANK_LINE_MASK	GENMASK(20, 31)
+
+/* CRT_HORIZ0 */
+#define CRT_H_TOTAL(x)			(x)
+#define CRT_H_DE(x)			((x) << 16)
+
+/* CRT_HORIZ1 */
+#define CRT_H_RS_START(x)		(x)
+#define CRT_H_RS_END(x)			((x) << 16)
+
+/* CRT_VIRT0 */
+#define CRT_V_TOTAL(x)			(x)
+#define CRT_V_DE(x)			((x) << 16)
+
+/* CRT_VIRT1 */
+#define CRT_V_RS_START(x)		(x)
+#define CRT_V_RS_END(x)			((x) << 16)
+
+/* CRT_OFFSET */
+#define CRT_DISP_OFFSET(x)		(x)
+#define CRT_TERM_COUNT(x)		((x) << 16)
+
+/* CRT_THROD */
+#define CRT_THROD_LOW(x)		(x)
+#define CRT_THROD_HIGH(x)		((x) << 8)
+
+/* Default Threshold Seting */
+#define G5_CRT_THROD_VAL	(CRT_THROD_LOW(0x24) | CRT_THROD_HIGH(0x3C))
diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
new file mode 100644
index 000000000000..e2d1d7497352
--- /dev/null
+++ b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
@@ -0,0 +1,248 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright 2018 IBM Corporation
+
+#include <linux/clk.h>
+#include <linux/reset.h>
+#include <linux/regmap.h>
+
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_device.h>
+#include <drm/drm_fb_cma_helper.h>
+#include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_simple_kms_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_panel.h>
+
+#include "aspeed_gfx.h"
+
+static struct aspeed_gfx *
+drm_pipe_to_aspeed_gfx(struct drm_simple_display_pipe *pipe)
+{
+	return container_of(pipe, struct aspeed_gfx, pipe);
+}
+
+static int aspeed_gfx_set_pixel_fmt(struct aspeed_gfx *priv, u32 *bpp)
+{
+	struct drm_crtc *crtc = &priv->pipe.crtc;
+	struct drm_device *drm = crtc->dev;
+	const u32 format = crtc->primary->state->fb->format->format;
+	u32 ctrl1;
+
+	ctrl1 = readl(priv->base + CRT_CTRL1);
+	ctrl1 &= ~CRT_CTRL_COLOR_MASK;
+
+	switch (format) {
+	case DRM_FORMAT_RGB565:
+		dev_dbg(drm->dev, "Setting up RGB565 mode\n");
+		ctrl1 |= CRT_CTRL_COLOR_RGB565;
+		*bpp = 16;
+		break;
+	case DRM_FORMAT_XRGB8888:
+		dev_dbg(drm->dev, "Setting up XRGB8888 mode\n");
+		ctrl1 |= CRT_CTRL_COLOR_XRGB8888;
+		*bpp = 32;
+		break;
+	default:
+		dev_err(drm->dev, "Unhandled pixel format %08x\n", format);
+		return -EINVAL;
+	}
+
+	writel(ctrl1, priv->base + CRT_CTRL1);
+
+	return 0;
+}
+
+static void aspeed_gfx_enable_controller(struct aspeed_gfx *priv)
+{
+	u32 ctrl1 = readl(priv->base + CRT_CTRL1);
+	u32 ctrl2 = readl(priv->base + CRT_CTRL2);
+
+	/* SCU2C: set DAC source for display output to Graphics CRT (GFX) */
+	regmap_update_bits(priv->scu, 0x2c, BIT(16), BIT(16));
+
+	writel(ctrl1 | CRT_CTRL_EN, priv->base + CRT_CTRL1);
+	writel(ctrl2 | CRT_CTRL_DAC_EN, priv->base + CRT_CTRL2);
+}
+
+static void aspeed_gfx_disable_controller(struct aspeed_gfx *priv)
+{
+	u32 ctrl1 = readl(priv->base + CRT_CTRL1);
+	u32 ctrl2 = readl(priv->base + CRT_CTRL2);
+
+	writel(ctrl1 & ~CRT_CTRL_EN, priv->base + CRT_CTRL1);
+	writel(ctrl2 & ~CRT_CTRL_DAC_EN, priv->base + CRT_CTRL2);
+
+	regmap_update_bits(priv->scu, 0x2c, BIT(16), 0);
+}
+
+static void aspeed_gfx_crtc_mode_set_nofb(struct aspeed_gfx *priv)
+{
+	struct drm_display_mode *m = &priv->pipe.crtc.state->adjusted_mode;
+	u32 ctrl1, d_offset, t_count, bpp;
+	int err;
+
+	err = aspeed_gfx_set_pixel_fmt(priv, &bpp);
+	if (err)
+		return;
+
+#if 0
+	/* TODO: we have only been able to test with the 40MHz USB clock. The
+	 * clock is fixed, so we cannot adjust it here. */
+	clk_set_rate(priv->pixel_clk, m->crtc_clock * 1000);
+#endif
+
+	ctrl1 = readl(priv->base + CRT_CTRL1);
+	ctrl1 &= ~(CRT_CTRL_INTERLACED |
+			CRT_CTRL_HSYNC_NEGATIVE |
+			CRT_CTRL_VSYNC_NEGATIVE);
+
+	if (m->flags & DRM_MODE_FLAG_INTERLACE)
+		ctrl1 |= CRT_CTRL_INTERLACED;
+
+	if (!(m->flags & DRM_MODE_FLAG_PHSYNC))
+		ctrl1 |= CRT_CTRL_HSYNC_NEGATIVE;
+
+	if (!(m->flags & DRM_MODE_FLAG_PVSYNC))
+		ctrl1 |= CRT_CTRL_VSYNC_NEGATIVE;
+
+	writel(ctrl1, priv->base + CRT_CTRL1);
+
+	/* Horizontal timing */
+	writel(CRT_H_TOTAL(m->htotal - 1) | CRT_H_DE(m->hdisplay - 1),
+			priv->base + CRT_HORIZ0);
+	writel(CRT_H_RS_START(m->hsync_start - 1) | CRT_H_RS_END(m->hsync_end),
+			priv->base + CRT_HORIZ1);
+
+
+	/* Vertical timing */
+	writel(CRT_V_TOTAL(m->vtotal - 1) | CRT_V_DE(m->vdisplay - 1),
+			priv->base + CRT_VERT0);
+	writel(CRT_V_RS_START(m->vsync_start) | CRT_V_RS_END(m->vsync_end),
+			priv->base + CRT_VERT1);
+
+	/*
+	 * Display Offset: address difference between consecutive scan lines
+	 * Terminal Count: memory size of one scan line
+	 */
+	d_offset = m->hdisplay * bpp / 8;
+	t_count = (m->hdisplay * bpp + 127) / 128;
+	writel(CRT_DISP_OFFSET(d_offset) | CRT_TERM_COUNT(t_count),
+			priv->base + CRT_OFFSET);
+
+	/*
+	 * Threshold: FIFO thresholds of refill and stop (16 byte chunks
+	 * per line, rounded up)
+	 */
+	writel(G5_CRT_THROD_VAL, priv->base + CRT_THROD);
+}
+
+static void aspeed_gfx_pipe_enable(struct drm_simple_display_pipe *pipe,
+			      struct drm_crtc_state *crtc_state,
+			      struct drm_plane_state *plane_state)
+{
+	struct aspeed_gfx *priv = drm_pipe_to_aspeed_gfx(pipe);
+	struct drm_crtc *crtc = &pipe->crtc;
+
+	aspeed_gfx_crtc_mode_set_nofb(priv);
+	aspeed_gfx_enable_controller(priv);
+	drm_crtc_vblank_on(crtc);
+}
+
+static void aspeed_gfx_pipe_disable(struct drm_simple_display_pipe *pipe)
+{
+	struct aspeed_gfx *priv = drm_pipe_to_aspeed_gfx(pipe);
+	struct drm_crtc *crtc = &pipe->crtc;
+
+	drm_crtc_vblank_off(crtc);
+	aspeed_gfx_disable_controller(priv);
+}
+
+static void aspeed_gfx_pipe_update(struct drm_simple_display_pipe *pipe,
+				   struct drm_plane_state *plane_state)
+{
+	struct aspeed_gfx *priv = drm_pipe_to_aspeed_gfx(pipe);
+	struct drm_crtc *crtc = &pipe->crtc;
+	struct drm_framebuffer *fb = pipe->plane.state->fb;
+	struct drm_pending_vblank_event *event;
+	struct drm_gem_cma_object *gem;
+
+	if (!crtc)
+		return;
+
+	spin_lock_irq(&crtc->dev->event_lock);
+	event = crtc->state->event;
+	if (event) {
+		crtc->state->event = NULL;
+
+		if (drm_crtc_vblank_get(crtc) == 0)
+			drm_crtc_arm_vblank_event(crtc, event);
+		else
+			drm_crtc_send_vblank_event(crtc, event);
+	}
+	spin_unlock_irq(&crtc->dev->event_lock);
+
+	if (!fb)
+		return;
+
+	gem = drm_fb_cma_get_gem_obj(fb, 0);
+	if (!gem)
+		return;
+	writel(gem->paddr, priv->base + CRT_ADDR);
+}
+
+static int aspeed_gfx_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
+				 struct drm_plane_state *plane_state)
+{
+	return drm_gem_fb_prepare_fb(&pipe->plane, plane_state);
+}
+
+static int aspeed_gfx_enable_vblank(struct drm_simple_display_pipe *pipe)
+{
+	struct aspeed_gfx *priv = drm_pipe_to_aspeed_gfx(pipe);
+	u32 reg = readl(priv->base + CRT_CTRL1);
+
+	/* Clear pending VBLANK IRQ */
+	writel(reg | CRT_CTRL_VERTICAL_INTR_STS, priv->base + CRT_CTRL1);
+
+	reg |= CRT_CTRL_VERTICAL_INTR_EN;
+	writel(reg, priv->base + CRT_CTRL1);
+
+	return 0;
+}
+
+static void aspeed_gfx_disable_vblank(struct drm_simple_display_pipe *pipe)
+{
+	struct aspeed_gfx *priv = drm_pipe_to_aspeed_gfx(pipe);
+	u32 reg = readl(priv->base + CRT_CTRL1);
+
+	reg &= ~CRT_CTRL_VERTICAL_INTR_EN;
+	writel(reg, priv->base + CRT_CTRL1);
+
+	/* Clear pending VBLANK IRQ */
+	writel(reg | CRT_CTRL_VERTICAL_INTR_STS, priv->base + CRT_CTRL1);
+}
+
+static struct drm_simple_display_pipe_funcs aspeed_gfx_funcs = {
+	.enable		= aspeed_gfx_pipe_enable,
+	.disable	= aspeed_gfx_pipe_disable,
+	.update		= aspeed_gfx_pipe_update,
+	.prepare_fb	= aspeed_gfx_pipe_prepare_fb,
+	.enable_vblank	= aspeed_gfx_enable_vblank,
+	.disable_vblank	= aspeed_gfx_disable_vblank,
+};
+
+static const uint32_t aspeed_gfx_formats[] = {
+	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_RGB565,
+};
+
+int aspeed_gfx_create_pipe(struct drm_device *drm)
+{
+	struct aspeed_gfx *priv = drm->dev_private;
+
+	return drm_simple_display_pipe_init(drm, &priv->pipe, &aspeed_gfx_funcs,
+					    aspeed_gfx_formats,
+					    ARRAY_SIZE(aspeed_gfx_formats),
+					    NULL,
+					    &priv->connector);
+}
diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
new file mode 100644
index 000000000000..6b88d658ac1f
--- /dev/null
+++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
@@ -0,0 +1,269 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright 2018 IBM Corporation
+
+#include <linux/module.h>
+#include <linux/irq.h>
+#include <linux/clk.h>
+#include <linux/of.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/reset.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
+
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_device.h>
+#include <drm/drm_fb_cma_helper.h>
+#include <drm/drm_fb_helper.h>
+#include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_simple_kms_helper.h>
+#include <drm/drm_probe_helper.h>
+
+#include "aspeed_gfx.h"
+
+/**
+ * DOC: ASPEED GFX Driver
+ *
+ * This driver is for the ASPEED BMC SoC's GFX display hardware. This
+ * driver runs on the ARM based BMC systems, unlike the ast driver which
+ * runs on a host CPU and is is for a PCI graphics device.
+ *
+ * The AST2500 supports a total of 3 output paths:
+ *
+ *   1. VGA output, the output target can choose either or both to the DAC
+ *   or DVO interface.
+ *
+ *   2. Graphics CRT output, the output target can choose either or both to
+ *   the DAC or DVO interface.
+ *
+ *   3. Video input from DVO, the video input can be used for video engine
+ *   capture or DAC display output.
+ *
+ * Output options are selected in SCU2C.
+ *
+ * The "VGA mode" device is the PCI attached controller. The "Graphics CRT"
+ * is the ARM's internal display controller.
+ *
+ * The driver only supports a simple configuration consisting of a 40MHz
+ * pixel clock, fixed by hardware limitations, and the VGA output path.
+ */
+
+static const struct drm_mode_config_funcs aspeed_gfx_mode_config_funcs = {
+	.fb_create		= drm_gem_fb_create,
+	.atomic_check		= drm_atomic_helper_check,
+	.atomic_commit		= drm_atomic_helper_commit,
+};
+
+static void aspeed_gfx_setup_mode_config(struct drm_device *drm)
+{
+	drm_mode_config_init(drm);
+
+	drm->mode_config.min_width = 0;
+	drm->mode_config.min_height = 0;
+	drm->mode_config.max_width = 800;
+	drm->mode_config.max_height = 600;
+	drm->mode_config.funcs = &aspeed_gfx_mode_config_funcs;
+}
+
+static irqreturn_t aspeed_gfx_irq_handler(int irq, void *data)
+{
+	struct drm_device *drm = data;
+	struct aspeed_gfx *priv = drm->dev_private;
+	u32 reg;
+
+	reg = readl(priv->base + CRT_CTRL1);
+
+	if (reg & CRT_CTRL_VERTICAL_INTR_STS) {
+		drm_crtc_handle_vblank(&priv->pipe.crtc);
+		writel(reg, priv->base + CRT_CTRL1);
+		return IRQ_HANDLED;
+	}
+
+	return IRQ_NONE;
+}
+
+
+
+static int aspeed_gfx_load(struct drm_device *drm)
+{
+	struct platform_device *pdev = to_platform_device(drm->dev);
+	struct aspeed_gfx *priv;
+	struct resource *res;
+	int ret;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	drm->dev_private = priv;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	priv->base = devm_ioremap_resource(drm->dev, res);
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
+
+	priv->scu = syscon_regmap_lookup_by_compatible("aspeed,ast2500-scu");
+	if (IS_ERR(priv->scu)) {
+		dev_err(&pdev->dev, "failed to find SCU regmap\n");
+		return PTR_ERR(priv->scu);
+	}
+
+	ret = of_reserved_mem_device_init(drm->dev);
+	if (ret) {
+		dev_err(&pdev->dev,
+			"failed to initialize reserved mem: %d\n", ret);
+		return ret;
+	}
+
+	ret = dma_set_mask_and_coherent(drm->dev, DMA_BIT_MASK(32));
+	if (ret) {
+		dev_err(&pdev->dev, "failed to set DMA mask: %d\n", ret);
+		return ret;
+	}
+
+	priv->rst = devm_reset_control_get_exclusive(&pdev->dev, NULL);
+	if (IS_ERR(priv->rst)) {
+		dev_err(&pdev->dev,
+			"missing or invalid reset controller device tree entry");
+		return PTR_ERR(priv->rst);
+	}
+	reset_control_deassert(priv->rst);
+
+	priv->clk = devm_clk_get(drm->dev, NULL);
+	if (IS_ERR(priv->clk)) {
+		dev_err(&pdev->dev,
+			"missing or invalid clk device tree entry");
+		return PTR_ERR(priv->clk);
+	}
+	clk_prepare_enable(priv->clk);
+
+	/* Sanitize control registers */
+	writel(0, priv->base + CRT_CTRL1);
+	writel(0, priv->base + CRT_CTRL2);
+
+	aspeed_gfx_setup_mode_config(drm);
+
+	ret = drm_vblank_init(drm, 1);
+	if (ret < 0) {
+		dev_err(drm->dev, "Failed to initialise vblank\n");
+		return ret;
+	}
+
+	ret = aspeed_gfx_create_output(drm);
+	if (ret < 0) {
+		dev_err(drm->dev, "Failed to create outputs\n");
+		return ret;
+	}
+
+	ret = aspeed_gfx_create_pipe(drm);
+	if (ret < 0) {
+		dev_err(drm->dev, "Cannot setup simple display pipe\n");
+		return ret;
+	}
+
+	ret = devm_request_irq(drm->dev, platform_get_irq(pdev, 0),
+			       aspeed_gfx_irq_handler, 0, "aspeed gfx", drm);
+	if (ret < 0) {
+		dev_err(drm->dev, "Failed to install IRQ handler\n");
+		return ret;
+	}
+
+	drm_mode_config_reset(drm);
+
+	drm_fbdev_generic_setup(drm, 32);
+
+	return 0;
+}
+
+static void aspeed_gfx_unload(struct drm_device *drm)
+{
+	drm_kms_helper_poll_fini(drm);
+	drm_mode_config_cleanup(drm);
+
+	drm->dev_private = NULL;
+}
+
+DEFINE_DRM_GEM_CMA_FOPS(fops);
+
+static struct drm_driver aspeed_gfx_driver = {
+	.driver_features        = DRIVER_GEM | DRIVER_MODESET |
+				DRIVER_PRIME | DRIVER_ATOMIC |
+				DRIVER_HAVE_IRQ,
+	.gem_free_object_unlocked = drm_gem_cma_free_object,
+	.gem_vm_ops             = &drm_gem_cma_vm_ops,
+	.dumb_create            = drm_gem_cma_dumb_create,
+	.prime_handle_to_fd     = drm_gem_prime_handle_to_fd,
+	.prime_fd_to_handle     = drm_gem_prime_fd_to_handle,
+	.gem_prime_export       = drm_gem_prime_export,
+	.gem_prime_import       = drm_gem_prime_import,
+	.gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
+	.gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
+	.gem_prime_vmap         = drm_gem_cma_prime_vmap,
+	.gem_prime_vunmap       = drm_gem_cma_prime_vunmap,
+	.gem_prime_mmap         = drm_gem_cma_prime_mmap,
+	.fops = &fops,
+	.name = "aspeed-gfx-drm",
+	.desc = "ASPEED GFX DRM",
+	.date = "20180319",
+	.major = 1,
+	.minor = 0,
+};
+
+static const struct of_device_id aspeed_gfx_match[] = {
+	{ .compatible = "aspeed,ast2400-gfx" },
+	{ .compatible = "aspeed,ast2500-gfx" },
+	{ }
+};
+
+static int aspeed_gfx_probe(struct platform_device *pdev)
+{
+	struct drm_device *drm;
+	int ret;
+
+	drm = drm_dev_alloc(&aspeed_gfx_driver, &pdev->dev);
+	if (IS_ERR(drm))
+		return PTR_ERR(drm);
+
+	ret = aspeed_gfx_load(drm);
+	if (ret)
+		goto err_free;
+
+	ret = drm_dev_register(drm, 0);
+	if (ret)
+		goto err_unload;
+
+	return 0;
+
+err_unload:
+	aspeed_gfx_unload(drm);
+err_free:
+	drm_dev_put(drm);
+
+	return ret;
+}
+
+static int aspeed_gfx_remove(struct platform_device *pdev)
+{
+	struct drm_device *drm = platform_get_drvdata(pdev);
+
+	drm_dev_unregister(drm);
+	aspeed_gfx_unload(drm);
+	drm_dev_put(drm);
+
+	return 0;
+}
+
+static struct platform_driver aspeed_gfx_platform_driver = {
+	.probe		= aspeed_gfx_probe,
+	.remove		= aspeed_gfx_remove,
+	.driver = {
+		.name = "aspeed_gfx",
+		.of_match_table = aspeed_gfx_match,
+	},
+};
+
+module_platform_driver(aspeed_gfx_platform_driver);
+
+MODULE_AUTHOR("Joel Stanley <joel@jms.id.au>");
+MODULE_DESCRIPTION("ASPEED BMC DRM/KMS driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_out.c b/drivers/gpu/drm/aspeed/aspeed_gfx_out.c
new file mode 100644
index 000000000000..7d2057e00056
--- /dev/null
+++ b/drivers/gpu/drm/aspeed/aspeed_gfx_out.c
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright 2018 IBM Corporation
+
+#include <drm/drmP.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_connector.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_probe_helper.h>
+
+#include "aspeed_gfx.h"
+
+static int aspeed_gfx_get_modes(struct drm_connector *connector)
+{
+	return drm_add_modes_noedid(connector, 800, 600);
+}
+
+static const struct
+drm_connector_helper_funcs aspeed_gfx_connector_helper_funcs = {
+	.get_modes = aspeed_gfx_get_modes,
+};
+
+static void aspeed_gfx_connector_destroy(struct drm_connector *connector)
+{
+	drm_connector_unregister(connector);
+	drm_connector_cleanup(connector);
+}
+
+static const struct drm_connector_funcs aspeed_gfx_connector_funcs = {
+	.fill_modes		= drm_helper_probe_single_connector_modes,
+	.destroy		= aspeed_gfx_connector_destroy,
+	.reset			= drm_atomic_helper_connector_reset,
+	.atomic_duplicate_state	= drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state	= drm_atomic_helper_connector_destroy_state,
+};
+
+int aspeed_gfx_create_output(struct drm_device *drm)
+{
+	struct aspeed_gfx *priv = drm->dev_private;
+	int ret;
+
+	priv->connector.dpms = DRM_MODE_DPMS_OFF;
+	priv->connector.polled = 0;
+	drm_connector_helper_add(&priv->connector,
+				 &aspeed_gfx_connector_helper_funcs);
+	ret = drm_connector_init(drm, &priv->connector,
+				 &aspeed_gfx_connector_funcs,
+				 DRM_MODE_CONNECTOR_Unknown);
+	return ret;
+}
-- 
2.20.1


^ permalink raw reply related

* [PATCH v7 2/2] drivers/misc: Add Aspeed P2A control driver
From: Greg KH @ 2019-03-28  6:52 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <CAO=notzwnDtJ9jmEUDsy_tqVKH-+4rO9gd8rpVjKKvQqJsODTQ@mail.gmail.com>

On Wed, Mar 27, 2019 at 12:01:50PM -0700, Patrick Venture wrote:
> On Wed, Mar 27, 2019 at 11:54 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Wed, Mar 27, 2019 at 11:44:36AM -0700, Patrick Venture wrote:
> > > On Wed, Mar 27, 2019 at 11:28 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Tue, Mar 12, 2019 at 09:31:01AM -0700, Patrick Venture wrote:
> > > > > +     phys_addr_t mem_base;
> > > >
> > > > Is this really a 32bit value?
> > >
> > > It's going to be a 32-bit value if this is in the dts for one of the
> > > correspondingly supported aspeed models.
> > >
> > > >
> > > > Your ioctl thinks it is:
> > > >
> > > > > +struct aspeed_p2a_ctrl_mapping {
> > > > > +     __u32 addr;
> > > >
> > > > Does this driver not work on a 64bit kernel?
> > >
> > > This driver is aimed at only 32-bit hardware (ast2400/2500).  I
> > > modeled the approach after the aspeed-lpc-ctrl driver as it's
> > > providing similar functionality.
> > >
> > > >
> > > > > +     __u32 length;
> > > > > +     __u32 flags;
> > > > > +};
> > > >
> > > > addr really should be __u32 here so you don't have to mess with 32/64
> > > > bit user/kernel issues, right?
> > >
> > > Add is __u32 there.  Are you suggesting it shouldn't be?
> >
> > Ugh, yes, sorry, I meant to say "__u64".
> >
> > If you all insist that this is all that is ever going to be needed, ok,
> > but I reserve the right to complain in 4 years when this needs to be
> > changed :)
> 
> In the event the ast2600 comes out and is 64-bit -- I can't imagine
> that's likely to happen.  I can take solace that this won't be the
> only thing that needs retrofitting.  But it wouldn't kill me to just
> make the change.  I'll just have to tweak it to return failure in the
> event the address provided isn't found in any region...
> 
> Is that all that needs to change for 64-bit addressing support - given
> your read of the driver?

That's all that I noticed at first glance, yes.  I do dislike having
custom user/kernel apis for random chips like this, but I don't know of
a way to have a generic api for them at the moment as I really do not
know what these chips do :(

One would think that the firmware api would work for you, but given the
complexity here, it does not seem that it would match up.

thanks,

greg k-h

^ permalink raw reply

* [PATCH 2/2] drm: Add ASPEED GFX driver
From: Daniel Vetter @ 2019-03-28  7:53 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190328054316.17939-3-joel@jms.id.au>

On Thu, Mar 28, 2019 at 04:13:16PM +1030, Joel Stanley wrote:
> This driver is for the ASPEED BMC SoC's GFX display hardware. This
> driver runs on the ARM based BMC systems, unlike the ast driver which
> runs on a host CPU and is is for a PCI graphics device.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> --
> Changes since RFC:
>  drm_fbdev_cma_init -> drm_fb_cma_fbdev_init and use generic lastclose callback
>  Use generic irq handling instead of drm_irq_install
>  Add doc to driver
>  Get rid of unncessary reads in irq enable/disable path
>  Rebase on linux-next
> 
>  drivers/gpu/drm/Kconfig                  |   2 +
>  drivers/gpu/drm/Makefile                 |   1 +
>  drivers/gpu/drm/aspeed/Kconfig           |  15 ++
>  drivers/gpu/drm/aspeed/Makefile          |   3 +
>  drivers/gpu/drm/aspeed/aspeed_gfx.h      | 104 +++++++++
>  drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c | 248 +++++++++++++++++++++
>  drivers/gpu/drm/aspeed/aspeed_gfx_drv.c  | 269 +++++++++++++++++++++++
>  drivers/gpu/drm/aspeed/aspeed_gfx_out.c  |  49 +++++
>  8 files changed, 691 insertions(+)
>  create mode 100644 drivers/gpu/drm/aspeed/Kconfig
>  create mode 100644 drivers/gpu/drm/aspeed/Makefile
>  create mode 100644 drivers/gpu/drm/aspeed/aspeed_gfx.h
>  create mode 100644 drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
>  create mode 100644 drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
>  create mode 100644 drivers/gpu/drm/aspeed/aspeed_gfx_out.c
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 82bb221ec94e..b1ec8f85c2a8 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -335,6 +335,8 @@ source "drivers/gpu/drm/xen/Kconfig"
>  
>  source "drivers/gpu/drm/vboxvideo/Kconfig"
>  
> +source "drivers/gpu/drm/aspeed/Kconfig"
> +
>  # Keep legacy drivers last
>  
>  menuconfig DRM_LEGACY
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 0baf148e3687..df8835045310 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -110,3 +110,4 @@ obj-$(CONFIG_DRM_PL111) += pl111/
>  obj-$(CONFIG_DRM_TVE200) += tve200/
>  obj-$(CONFIG_DRM_XEN) += xen/
>  obj-$(CONFIG_DRM_VBOXVIDEO) += vboxvideo/
> +obj-$(CONFIG_DRM_ASPEED_GFX) += aspeed/
> diff --git a/drivers/gpu/drm/aspeed/Kconfig b/drivers/gpu/drm/aspeed/Kconfig
> new file mode 100644
> index 000000000000..6f1e64c0a6ce
> --- /dev/null
> +++ b/drivers/gpu/drm/aspeed/Kconfig
> @@ -0,0 +1,15 @@
> +config DRM_ASPEED_GFX
> +	tristate "ASPEED BMC Display Controller"
> +	depends on DRM && OF
> +	select DRM_KMS_HELPER
> +	select DRM_KMS_FB_HELPER
> +	select DRM_KMS_CMA_HELPER
> +	select DRM_PANEL
> +	select DMA_CMA
> +	select CMA
> +	select MFD_SYSCON
> +	help
> +	  Chose this option if you have an ASPEED AST2400/AST2500
> +	  SOC Display Controller (aka GFX).
> +
> +	  If M is selected this module will be called aspeed_gfx.
> diff --git a/drivers/gpu/drm/aspeed/Makefile b/drivers/gpu/drm/aspeed/Makefile
> new file mode 100644
> index 000000000000..6e194cd790d8
> --- /dev/null
> +++ b/drivers/gpu/drm/aspeed/Makefile
> @@ -0,0 +1,3 @@
> +aspeed_gfx-y := aspeed_gfx_drv.o aspeed_gfx_crtc.o aspeed_gfx_out.o
> +
> +obj-$(CONFIG_DRM_ASPEED_GFX) += aspeed_gfx.o
> diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx.h b/drivers/gpu/drm/aspeed/aspeed_gfx.h
> new file mode 100644
> index 000000000000..fb56e425bd48
> --- /dev/null
> +++ b/drivers/gpu/drm/aspeed/aspeed_gfx.h
> @@ -0,0 +1,104 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Copyright 2018 IBM Corporation
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_simple_kms_helper.h>
> +
> +struct aspeed_gfx {
> +	void __iomem			*base;
> +	struct clk			*clk;
> +	struct reset_control		*rst;
> +	struct regmap			*scu;
> +
> +	struct drm_simple_display_pipe	pipe;
> +	struct drm_connector		connector;
> +	struct drm_fbdev_cma		*fbdev;
> +};
> +
> +int aspeed_gfx_create_pipe(struct drm_device *drm);
> +int aspeed_gfx_create_output(struct drm_device *drm);
> +
> +#define CRT_CTRL1		0x60 /* CRT Control I */
> +#define CRT_CTRL2		0x64 /* CRT Control II */
> +#define CRT_STATUS		0x68 /* CRT Status */
> +#define CRT_MISC		0x6c /* CRT Misc Setting */
> +#define CRT_HORIZ0		0x70 /* CRT Horizontal Total & Display Enable End */
> +#define CRT_HORIZ1		0x74 /* CRT Horizontal Retrace Start & End */
> +#define CRT_VERT0		0x78 /* CRT Vertical Total & Display Enable End */
> +#define CRT_VERT1		0x7C /* CRT Vertical Retrace Start & End */
> +#define CRT_ADDR		0x80 /* CRT Display Starting Address */
> +#define CRT_OFFSET		0x84 /* CRT Display Offset & Terminal Count */
> +#define CRT_THROD		0x88 /* CRT Threshold */
> +#define CRT_XSCALE		0x8C /* CRT Scaling-Up Factor */
> +#define CRT_CURSOR0		0x90 /* CRT Hardware Cursor X & Y Offset */
> +#define CRT_CURSOR1		0x94 /* CRT Hardware Cursor X & Y Position */
> +#define CRT_CURSOR2		0x98 /* CRT Hardware Cursor Pattern Address */
> +#define CRT_9C			0x9C
> +#define CRT_OSD_H		0xA0 /* CRT OSD Horizontal Start/End */
> +#define CRT_OSD_V		0xA4 /* CRT OSD Vertical Start/End */
> +#define CRT_OSD_ADDR		0xA8 /* CRT OSD Pattern Address */
> +#define CRT_OSD_DISP		0xAC /* CRT OSD Offset */
> +#define CRT_OSD_THRESH		0xB0 /* CRT OSD Threshold & Alpha */
> +#define CRT_B4			0xB4
> +#define CRT_STS_V		0xB8 /* CRT Status V */
> +#define CRT_SCRATCH		0xBC /* Scratchpad */
> +#define CRT_BB0_ADDR		0xD0 /* CRT Display BB0 Starting Address */
> +#define CRT_BB1_ADDR		0xD4 /* CRT Display BB1 Starting Address */
> +#define CRT_BB_COUNT		0xD8 /* CRT Display BB Terminal Count */
> +#define OSD_COLOR1		0xE0 /* OSD Color Palette Index 1 & 0 */
> +#define OSD_COLOR2		0xE4 /* OSD Color Palette Index 3 & 2 */
> +#define OSD_COLOR3		0xE8 /* OSD Color Palette Index 5 & 4 */
> +#define OSD_COLOR4		0xEC /* OSD Color Palette Index 7 & 6 */
> +#define OSD_COLOR5		0xF0 /* OSD Color Palette Index 9 & 8 */
> +#define OSD_COLOR6		0xF4 /* OSD Color Palette Index 11 & 10 */
> +#define OSD_COLOR7		0xF8 /* OSD Color Palette Index 13 & 12 */
> +#define OSD_COLOR8		0xFC /* OSD Color Palette Index 15 & 14 */
> +
> +/* CTRL1 */
> +#define CRT_CTRL_EN			BIT(0)
> +#define CRT_CTRL_HW_CURSOR_EN		BIT(1)
> +#define CRT_CTRL_OSD_EN			BIT(2)
> +#define CRT_CTRL_INTERLACED		BIT(3)
> +#define CRT_CTRL_COLOR_RGB565		(0 << 7)
> +#define CRT_CTRL_COLOR_YUV444		(1 << 7)
> +#define CRT_CTRL_COLOR_XRGB8888		(2 << 7)
> +#define CRT_CTRL_COLOR_RGB888		(3 << 7)
> +#define CRT_CTRL_COLOR_YUV444_2RGB	(5 << 7)
> +#define CRT_CTRL_COLOR_YUV422		(7 << 7)
> +#define CRT_CTRL_COLOR_MASK		GENMASK(9, 7)
> +#define CRT_CTRL_HSYNC_NEGATIVE		BIT(16)
> +#define CRT_CTRL_VSYNC_NEGATIVE		BIT(17)
> +#define CRT_CTRL_VERTICAL_INTR_EN	BIT(30)
> +#define CRT_CTRL_VERTICAL_INTR_STS	BIT(31)
> +
> +/* CTRL2 */
> +#define CRT_CTRL_DAC_EN			BIT(0)
> +#define CRT_CTRL_VBLANK_LINE(x)		(((x) << 20) & CRT_CTRL_VBLANK_LINE_MASK)
> +#define CRT_CTRL_VBLANK_LINE_MASK	GENMASK(20, 31)
> +
> +/* CRT_HORIZ0 */
> +#define CRT_H_TOTAL(x)			(x)
> +#define CRT_H_DE(x)			((x) << 16)
> +
> +/* CRT_HORIZ1 */
> +#define CRT_H_RS_START(x)		(x)
> +#define CRT_H_RS_END(x)			((x) << 16)
> +
> +/* CRT_VIRT0 */
> +#define CRT_V_TOTAL(x)			(x)
> +#define CRT_V_DE(x)			((x) << 16)
> +
> +/* CRT_VIRT1 */
> +#define CRT_V_RS_START(x)		(x)
> +#define CRT_V_RS_END(x)			((x) << 16)
> +
> +/* CRT_OFFSET */
> +#define CRT_DISP_OFFSET(x)		(x)
> +#define CRT_TERM_COUNT(x)		((x) << 16)
> +
> +/* CRT_THROD */
> +#define CRT_THROD_LOW(x)		(x)
> +#define CRT_THROD_HIGH(x)		((x) << 8)
> +
> +/* Default Threshold Seting */
> +#define G5_CRT_THROD_VAL	(CRT_THROD_LOW(0x24) | CRT_THROD_HIGH(0x3C))
> diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
> new file mode 100644
> index 000000000000..e2d1d7497352
> --- /dev/null
> +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
> @@ -0,0 +1,248 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Copyright 2018 IBM Corporation
> +
> +#include <linux/clk.h>
> +#include <linux/reset.h>
> +#include <linux/regmap.h>
> +
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_device.h>
> +#include <drm/drm_fb_cma_helper.h>
> +#include <drm/drm_gem_cma_helper.h>
> +#include <drm/drm_simple_kms_helper.h>
> +#include <drm/drm_gem_framebuffer_helper.h>
> +#include <drm/drm_panel.h>
> +
> +#include "aspeed_gfx.h"
> +
> +static struct aspeed_gfx *
> +drm_pipe_to_aspeed_gfx(struct drm_simple_display_pipe *pipe)
> +{
> +	return container_of(pipe, struct aspeed_gfx, pipe);
> +}
> +
> +static int aspeed_gfx_set_pixel_fmt(struct aspeed_gfx *priv, u32 *bpp)
> +{
> +	struct drm_crtc *crtc = &priv->pipe.crtc;
> +	struct drm_device *drm = crtc->dev;
> +	const u32 format = crtc->primary->state->fb->format->format;
> +	u32 ctrl1;
> +
> +	ctrl1 = readl(priv->base + CRT_CTRL1);
> +	ctrl1 &= ~CRT_CTRL_COLOR_MASK;
> +
> +	switch (format) {
> +	case DRM_FORMAT_RGB565:
> +		dev_dbg(drm->dev, "Setting up RGB565 mode\n");
> +		ctrl1 |= CRT_CTRL_COLOR_RGB565;
> +		*bpp = 16;
> +		break;
> +	case DRM_FORMAT_XRGB8888:
> +		dev_dbg(drm->dev, "Setting up XRGB8888 mode\n");
> +		ctrl1 |= CRT_CTRL_COLOR_XRGB8888;
> +		*bpp = 32;
> +		break;
> +	default:
> +		dev_err(drm->dev, "Unhandled pixel format %08x\n", format);
> +		return -EINVAL;
> +	}
> +
> +	writel(ctrl1, priv->base + CRT_CTRL1);
> +
> +	return 0;
> +}
> +
> +static void aspeed_gfx_enable_controller(struct aspeed_gfx *priv)
> +{
> +	u32 ctrl1 = readl(priv->base + CRT_CTRL1);
> +	u32 ctrl2 = readl(priv->base + CRT_CTRL2);
> +
> +	/* SCU2C: set DAC source for display output to Graphics CRT (GFX) */
> +	regmap_update_bits(priv->scu, 0x2c, BIT(16), BIT(16));
> +
> +	writel(ctrl1 | CRT_CTRL_EN, priv->base + CRT_CTRL1);
> +	writel(ctrl2 | CRT_CTRL_DAC_EN, priv->base + CRT_CTRL2);
> +}
> +
> +static void aspeed_gfx_disable_controller(struct aspeed_gfx *priv)
> +{
> +	u32 ctrl1 = readl(priv->base + CRT_CTRL1);
> +	u32 ctrl2 = readl(priv->base + CRT_CTRL2);
> +
> +	writel(ctrl1 & ~CRT_CTRL_EN, priv->base + CRT_CTRL1);
> +	writel(ctrl2 & ~CRT_CTRL_DAC_EN, priv->base + CRT_CTRL2);
> +
> +	regmap_update_bits(priv->scu, 0x2c, BIT(16), 0);
> +}
> +
> +static void aspeed_gfx_crtc_mode_set_nofb(struct aspeed_gfx *priv)
> +{
> +	struct drm_display_mode *m = &priv->pipe.crtc.state->adjusted_mode;
> +	u32 ctrl1, d_offset, t_count, bpp;
> +	int err;
> +
> +	err = aspeed_gfx_set_pixel_fmt(priv, &bpp);
> +	if (err)
> +		return;
> +
> +#if 0
> +	/* TODO: we have only been able to test with the 40MHz USB clock. The
> +	 * clock is fixed, so we cannot adjust it here. */
> +	clk_set_rate(priv->pixel_clk, m->crtc_clock * 1000);
> +#endif
> +
> +	ctrl1 = readl(priv->base + CRT_CTRL1);
> +	ctrl1 &= ~(CRT_CTRL_INTERLACED |
> +			CRT_CTRL_HSYNC_NEGATIVE |
> +			CRT_CTRL_VSYNC_NEGATIVE);
> +
> +	if (m->flags & DRM_MODE_FLAG_INTERLACE)
> +		ctrl1 |= CRT_CTRL_INTERLACED;
> +
> +	if (!(m->flags & DRM_MODE_FLAG_PHSYNC))
> +		ctrl1 |= CRT_CTRL_HSYNC_NEGATIVE;
> +
> +	if (!(m->flags & DRM_MODE_FLAG_PVSYNC))
> +		ctrl1 |= CRT_CTRL_VSYNC_NEGATIVE;
> +
> +	writel(ctrl1, priv->base + CRT_CTRL1);
> +
> +	/* Horizontal timing */
> +	writel(CRT_H_TOTAL(m->htotal - 1) | CRT_H_DE(m->hdisplay - 1),
> +			priv->base + CRT_HORIZ0);
> +	writel(CRT_H_RS_START(m->hsync_start - 1) | CRT_H_RS_END(m->hsync_end),
> +			priv->base + CRT_HORIZ1);
> +
> +
> +	/* Vertical timing */
> +	writel(CRT_V_TOTAL(m->vtotal - 1) | CRT_V_DE(m->vdisplay - 1),
> +			priv->base + CRT_VERT0);
> +	writel(CRT_V_RS_START(m->vsync_start) | CRT_V_RS_END(m->vsync_end),
> +			priv->base + CRT_VERT1);
> +
> +	/*
> +	 * Display Offset: address difference between consecutive scan lines
> +	 * Terminal Count: memory size of one scan line
> +	 */
> +	d_offset = m->hdisplay * bpp / 8;
> +	t_count = (m->hdisplay * bpp + 127) / 128;
> +	writel(CRT_DISP_OFFSET(d_offset) | CRT_TERM_COUNT(t_count),
> +			priv->base + CRT_OFFSET);
> +
> +	/*
> +	 * Threshold: FIFO thresholds of refill and stop (16 byte chunks
> +	 * per line, rounded up)
> +	 */
> +	writel(G5_CRT_THROD_VAL, priv->base + CRT_THROD);
> +}
> +
> +static void aspeed_gfx_pipe_enable(struct drm_simple_display_pipe *pipe,
> +			      struct drm_crtc_state *crtc_state,
> +			      struct drm_plane_state *plane_state)
> +{
> +	struct aspeed_gfx *priv = drm_pipe_to_aspeed_gfx(pipe);
> +	struct drm_crtc *crtc = &pipe->crtc;
> +
> +	aspeed_gfx_crtc_mode_set_nofb(priv);
> +	aspeed_gfx_enable_controller(priv);
> +	drm_crtc_vblank_on(crtc);
> +}
> +
> +static void aspeed_gfx_pipe_disable(struct drm_simple_display_pipe *pipe)
> +{
> +	struct aspeed_gfx *priv = drm_pipe_to_aspeed_gfx(pipe);
> +	struct drm_crtc *crtc = &pipe->crtc;
> +
> +	drm_crtc_vblank_off(crtc);
> +	aspeed_gfx_disable_controller(priv);
> +}
> +
> +static void aspeed_gfx_pipe_update(struct drm_simple_display_pipe *pipe,
> +				   struct drm_plane_state *plane_state)
> +{
> +	struct aspeed_gfx *priv = drm_pipe_to_aspeed_gfx(pipe);
> +	struct drm_crtc *crtc = &pipe->crtc;
> +	struct drm_framebuffer *fb = pipe->plane.state->fb;
> +	struct drm_pending_vblank_event *event;
> +	struct drm_gem_cma_object *gem;
> +
> +	if (!crtc)
> +		return;
> +
> +	spin_lock_irq(&crtc->dev->event_lock);
> +	event = crtc->state->event;
> +	if (event) {
> +		crtc->state->event = NULL;
> +
> +		if (drm_crtc_vblank_get(crtc) == 0)
> +			drm_crtc_arm_vblank_event(crtc, event);
> +		else
> +			drm_crtc_send_vblank_event(crtc, event);
> +	}
> +	spin_unlock_irq(&crtc->dev->event_lock);
> +
> +	if (!fb)
> +		return;
> +
> +	gem = drm_fb_cma_get_gem_obj(fb, 0);
> +	if (!gem)
> +		return;
> +	writel(gem->paddr, priv->base + CRT_ADDR);
> +}
> +
> +static int aspeed_gfx_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
> +				 struct drm_plane_state *plane_state)
> +{
> +	return drm_gem_fb_prepare_fb(&pipe->plane, plane_state);
> +}

drm_gem_fb_simple_display_pipe_prepare_fb

> +
> +static int aspeed_gfx_enable_vblank(struct drm_simple_display_pipe *pipe)
> +{
> +	struct aspeed_gfx *priv = drm_pipe_to_aspeed_gfx(pipe);
> +	u32 reg = readl(priv->base + CRT_CTRL1);
> +
> +	/* Clear pending VBLANK IRQ */
> +	writel(reg | CRT_CTRL_VERTICAL_INTR_STS, priv->base + CRT_CTRL1);
> +
> +	reg |= CRT_CTRL_VERTICAL_INTR_EN;
> +	writel(reg, priv->base + CRT_CTRL1);
> +
> +	return 0;
> +}
> +
> +static void aspeed_gfx_disable_vblank(struct drm_simple_display_pipe *pipe)
> +{
> +	struct aspeed_gfx *priv = drm_pipe_to_aspeed_gfx(pipe);
> +	u32 reg = readl(priv->base + CRT_CTRL1);
> +
> +	reg &= ~CRT_CTRL_VERTICAL_INTR_EN;
> +	writel(reg, priv->base + CRT_CTRL1);
> +
> +	/* Clear pending VBLANK IRQ */
> +	writel(reg | CRT_CTRL_VERTICAL_INTR_STS, priv->base + CRT_CTRL1);
> +}
> +
> +static struct drm_simple_display_pipe_funcs aspeed_gfx_funcs = {
> +	.enable		= aspeed_gfx_pipe_enable,
> +	.disable	= aspeed_gfx_pipe_disable,
> +	.update		= aspeed_gfx_pipe_update,
> +	.prepare_fb	= aspeed_gfx_pipe_prepare_fb,
> +	.enable_vblank	= aspeed_gfx_enable_vblank,
> +	.disable_vblank	= aspeed_gfx_disable_vblank,
> +};
> +
> +static const uint32_t aspeed_gfx_formats[] = {
> +	DRM_FORMAT_XRGB8888,
> +	DRM_FORMAT_RGB565,
> +};
> +
> +int aspeed_gfx_create_pipe(struct drm_device *drm)
> +{
> +	struct aspeed_gfx *priv = drm->dev_private;
> +
> +	return drm_simple_display_pipe_init(drm, &priv->pipe, &aspeed_gfx_funcs,
> +					    aspeed_gfx_formats,
> +					    ARRAY_SIZE(aspeed_gfx_formats),
> +					    NULL,
> +					    &priv->connector);
> +}
> diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
> new file mode 100644
> index 000000000000..6b88d658ac1f
> --- /dev/null
> +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
> @@ -0,0 +1,269 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Copyright 2018 IBM Corporation
> +
> +#include <linux/module.h>
> +#include <linux/irq.h>
> +#include <linux/clk.h>
> +#include <linux/of.h>
> +#include <linux/of_reserved_mem.h>
> +#include <linux/reset.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
> +
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_device.h>
> +#include <drm/drm_fb_cma_helper.h>
> +#include <drm/drm_fb_helper.h>
> +#include <drm/drm_gem_cma_helper.h>
> +#include <drm/drm_gem_framebuffer_helper.h>
> +#include <drm/drm_simple_kms_helper.h>
> +#include <drm/drm_probe_helper.h>
> +
> +#include "aspeed_gfx.h"
> +
> +/**
> + * DOC: ASPEED GFX Driver
> + *
> + * This driver is for the ASPEED BMC SoC's GFX display hardware. This
> + * driver runs on the ARM based BMC systems, unlike the ast driver which
> + * runs on a host CPU and is is for a PCI graphics device.
> + *
> + * The AST2500 supports a total of 3 output paths:
> + *
> + *   1. VGA output, the output target can choose either or both to the DAC
> + *   or DVO interface.
> + *
> + *   2. Graphics CRT output, the output target can choose either or both to
> + *   the DAC or DVO interface.
> + *
> + *   3. Video input from DVO, the video input can be used for video engine
> + *   capture or DAC display output.
> + *
> + * Output options are selected in SCU2C.
> + *
> + * The "VGA mode" device is the PCI attached controller. The "Graphics CRT"
> + * is the ARM's internal display controller.
> + *
> + * The driver only supports a simple configuration consisting of a 40MHz
> + * pixel clock, fixed by hardware limitations, and the VGA output path.
> + */
> +
> +static const struct drm_mode_config_funcs aspeed_gfx_mode_config_funcs = {
> +	.fb_create		= drm_gem_fb_create,
> +	.atomic_check		= drm_atomic_helper_check,
> +	.atomic_commit		= drm_atomic_helper_commit,
> +};
> +
> +static void aspeed_gfx_setup_mode_config(struct drm_device *drm)
> +{
> +	drm_mode_config_init(drm);
> +
> +	drm->mode_config.min_width = 0;
> +	drm->mode_config.min_height = 0;
> +	drm->mode_config.max_width = 800;
> +	drm->mode_config.max_height = 600;
> +	drm->mode_config.funcs = &aspeed_gfx_mode_config_funcs;
> +}
> +
> +static irqreturn_t aspeed_gfx_irq_handler(int irq, void *data)
> +{
> +	struct drm_device *drm = data;
> +	struct aspeed_gfx *priv = drm->dev_private;
> +	u32 reg;
> +
> +	reg = readl(priv->base + CRT_CTRL1);
> +
> +	if (reg & CRT_CTRL_VERTICAL_INTR_STS) {
> +		drm_crtc_handle_vblank(&priv->pipe.crtc);
> +		writel(reg, priv->base + CRT_CTRL1);
> +		return IRQ_HANDLED;
> +	}
> +
> +	return IRQ_NONE;
> +}
> +
> +
> +
> +static int aspeed_gfx_load(struct drm_device *drm)
> +{
> +	struct platform_device *pdev = to_platform_device(drm->dev);
> +	struct aspeed_gfx *priv;
> +	struct resource *res;
> +	int ret;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +	drm->dev_private = priv;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	priv->base = devm_ioremap_resource(drm->dev, res);
> +	if (IS_ERR(priv->base))
> +		return PTR_ERR(priv->base);
> +
> +	priv->scu = syscon_regmap_lookup_by_compatible("aspeed,ast2500-scu");
> +	if (IS_ERR(priv->scu)) {
> +		dev_err(&pdev->dev, "failed to find SCU regmap\n");
> +		return PTR_ERR(priv->scu);
> +	}
> +
> +	ret = of_reserved_mem_device_init(drm->dev);
> +	if (ret) {
> +		dev_err(&pdev->dev,
> +			"failed to initialize reserved mem: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = dma_set_mask_and_coherent(drm->dev, DMA_BIT_MASK(32));
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to set DMA mask: %d\n", ret);
> +		return ret;
> +	}
> +
> +	priv->rst = devm_reset_control_get_exclusive(&pdev->dev, NULL);
> +	if (IS_ERR(priv->rst)) {
> +		dev_err(&pdev->dev,
> +			"missing or invalid reset controller device tree entry");
> +		return PTR_ERR(priv->rst);
> +	}
> +	reset_control_deassert(priv->rst);
> +
> +	priv->clk = devm_clk_get(drm->dev, NULL);
> +	if (IS_ERR(priv->clk)) {
> +		dev_err(&pdev->dev,
> +			"missing or invalid clk device tree entry");
> +		return PTR_ERR(priv->clk);
> +	}
> +	clk_prepare_enable(priv->clk);
> +
> +	/* Sanitize control registers */
> +	writel(0, priv->base + CRT_CTRL1);
> +	writel(0, priv->base + CRT_CTRL2);
> +
> +	aspeed_gfx_setup_mode_config(drm);
> +
> +	ret = drm_vblank_init(drm, 1);
> +	if (ret < 0) {
> +		dev_err(drm->dev, "Failed to initialise vblank\n");
> +		return ret;
> +	}
> +
> +	ret = aspeed_gfx_create_output(drm);
> +	if (ret < 0) {
> +		dev_err(drm->dev, "Failed to create outputs\n");
> +		return ret;
> +	}
> +
> +	ret = aspeed_gfx_create_pipe(drm);
> +	if (ret < 0) {
> +		dev_err(drm->dev, "Cannot setup simple display pipe\n");
> +		return ret;
> +	}
> +
> +	ret = devm_request_irq(drm->dev, platform_get_irq(pdev, 0),
> +			       aspeed_gfx_irq_handler, 0, "aspeed gfx", drm);
> +	if (ret < 0) {
> +		dev_err(drm->dev, "Failed to install IRQ handler\n");
> +		return ret;
> +	}
> +
> +	drm_mode_config_reset(drm);
> +
> +	drm_fbdev_generic_setup(drm, 32);
> +
> +	return 0;
> +}
> +
> +static void aspeed_gfx_unload(struct drm_device *drm)
> +{
> +	drm_kms_helper_poll_fini(drm);
> +	drm_mode_config_cleanup(drm);
> +
> +	drm->dev_private = NULL;
> +}
> +
> +DEFINE_DRM_GEM_CMA_FOPS(fops);
> +
> +static struct drm_driver aspeed_gfx_driver = {
> +	.driver_features        = DRIVER_GEM | DRIVER_MODESET |
> +				DRIVER_PRIME | DRIVER_ATOMIC |
> +				DRIVER_HAVE_IRQ,
> +	.gem_free_object_unlocked = drm_gem_cma_free_object,
> +	.gem_vm_ops             = &drm_gem_cma_vm_ops,
> +	.dumb_create            = drm_gem_cma_dumb_create,
> +	.prime_handle_to_fd     = drm_gem_prime_handle_to_fd,
> +	.prime_fd_to_handle     = drm_gem_prime_fd_to_handle,
> +	.gem_prime_export       = drm_gem_prime_export,
> +	.gem_prime_import       = drm_gem_prime_import,
> +	.gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
> +	.gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
> +	.gem_prime_vmap         = drm_gem_cma_prime_vmap,
> +	.gem_prime_vunmap       = drm_gem_cma_prime_vunmap,
> +	.gem_prime_mmap         = drm_gem_cma_prime_mmap,
> +	.fops = &fops,
> +	.name = "aspeed-gfx-drm",
> +	.desc = "ASPEED GFX DRM",
> +	.date = "20180319",
> +	.major = 1,
> +	.minor = 0,
> +};
> +
> +static const struct of_device_id aspeed_gfx_match[] = {
> +	{ .compatible = "aspeed,ast2400-gfx" },
> +	{ .compatible = "aspeed,ast2500-gfx" },
> +	{ }
> +};
> +
> +static int aspeed_gfx_probe(struct platform_device *pdev)
> +{
> +	struct drm_device *drm;
> +	int ret;
> +
> +	drm = drm_dev_alloc(&aspeed_gfx_driver, &pdev->dev);
> +	if (IS_ERR(drm))
> +		return PTR_ERR(drm);
> +
> +	ret = aspeed_gfx_load(drm);
> +	if (ret)
> +		goto err_free;
> +
> +	ret = drm_dev_register(drm, 0);
> +	if (ret)
> +		goto err_unload;
> +
> +	return 0;
> +
> +err_unload:
> +	aspeed_gfx_unload(drm);
> +err_free:
> +	drm_dev_put(drm);
> +
> +	return ret;
> +}
> +
> +static int aspeed_gfx_remove(struct platform_device *pdev)
> +{
> +	struct drm_device *drm = platform_get_drvdata(pdev);
> +
> +	drm_dev_unregister(drm);
> +	aspeed_gfx_unload(drm);
> +	drm_dev_put(drm);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver aspeed_gfx_platform_driver = {
> +	.probe		= aspeed_gfx_probe,
> +	.remove		= aspeed_gfx_remove,
> +	.driver = {
> +		.name = "aspeed_gfx",
> +		.of_match_table = aspeed_gfx_match,
> +	},
> +};
> +
> +module_platform_driver(aspeed_gfx_platform_driver);
> +
> +MODULE_AUTHOR("Joel Stanley <joel@jms.id.au>");
> +MODULE_DESCRIPTION("ASPEED BMC DRM/KMS driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_out.c b/drivers/gpu/drm/aspeed/aspeed_gfx_out.c
> new file mode 100644
> index 000000000000..7d2057e00056
> --- /dev/null
> +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_out.c
> @@ -0,0 +1,49 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Copyright 2018 IBM Corporation
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_connector.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_probe_helper.h>
> +
> +#include "aspeed_gfx.h"
> +
> +static int aspeed_gfx_get_modes(struct drm_connector *connector)
> +{
> +	return drm_add_modes_noedid(connector, 800, 600);

Is this the only mode you do, or just a default? Iirc if you report
"connected", you'll get this as one of the fallback modes already.

> +}
> +
> +static const struct
> +drm_connector_helper_funcs aspeed_gfx_connector_helper_funcs = {
> +	.get_modes = aspeed_gfx_get_modes,
> +};
> +
> +static void aspeed_gfx_connector_destroy(struct drm_connector *connector)
> +{
> +	drm_connector_unregister(connector);

Only needed for hotunplugged connectors, drm_dev_unregister takes care of
this for you already.

> +	drm_connector_cleanup(connector);
> +}
> +
> +static const struct drm_connector_funcs aspeed_gfx_connector_funcs = {
> +	.fill_modes		= drm_helper_probe_single_connector_modes,
> +	.destroy		= aspeed_gfx_connector_destroy,
> +	.reset			= drm_atomic_helper_connector_reset,
> +	.atomic_duplicate_state	= drm_atomic_helper_connector_duplicate_state,
> +	.atomic_destroy_state	= drm_atomic_helper_connector_destroy_state,
> +};
> +
> +int aspeed_gfx_create_output(struct drm_device *drm)
> +{
> +	struct aspeed_gfx *priv = drm->dev_private;
> +	int ret;
> +
> +	priv->connector.dpms = DRM_MODE_DPMS_OFF;
> +	priv->connector.polled = 0;
> +	drm_connector_helper_add(&priv->connector,
> +				 &aspeed_gfx_connector_helper_funcs);
> +	ret = drm_connector_init(drm, &priv->connector,
> +				 &aspeed_gfx_connector_funcs,
> +				 DRM_MODE_CONNECTOR_Unknown);
> +	return ret;
> +}
> -- 
> 2.20.1

Very nice driver!

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

For merging/maintenance, do you want to put this into drm-misc?

https://drm.pages.freedesktop.org/maintainer-tools/committer-drm-misc.html#small-drivers

We could also officially put drm/ast in there, if you want to maintain
that too.

Either way, patch also needs a MAINTAINERS entry.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply

* [PATCH v7 2/2] drivers/misc: Add Aspeed P2A control driver
From: Patrick Venture @ 2019-03-28 14:13 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190328065231.GB20312@kroah.com>

On Wed, Mar 27, 2019 at 11:52 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Wed, Mar 27, 2019 at 12:01:50PM -0700, Patrick Venture wrote:
> > On Wed, Mar 27, 2019 at 11:54 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Wed, Mar 27, 2019 at 11:44:36AM -0700, Patrick Venture wrote:
> > > > On Wed, Mar 27, 2019 at 11:28 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > > >
> > > > > On Tue, Mar 12, 2019 at 09:31:01AM -0700, Patrick Venture wrote:
> > > > > > +     phys_addr_t mem_base;
> > > > >
> > > > > Is this really a 32bit value?
> > > >
> > > > It's going to be a 32-bit value if this is in the dts for one of the
> > > > correspondingly supported aspeed models.
> > > >
> > > > >
> > > > > Your ioctl thinks it is:
> > > > >
> > > > > > +struct aspeed_p2a_ctrl_mapping {
> > > > > > +     __u32 addr;
> > > > >
> > > > > Does this driver not work on a 64bit kernel?
> > > >
> > > > This driver is aimed at only 32-bit hardware (ast2400/2500).  I
> > > > modeled the approach after the aspeed-lpc-ctrl driver as it's
> > > > providing similar functionality.
> > > >
> > > > >
> > > > > > +     __u32 length;
> > > > > > +     __u32 flags;
> > > > > > +};
> > > > >
> > > > > addr really should be __u32 here so you don't have to mess with 32/64
> > > > > bit user/kernel issues, right?
> > > >
> > > > Add is __u32 there.  Are you suggesting it shouldn't be?
> > >
> > > Ugh, yes, sorry, I meant to say "__u64".
> > >
> > > If you all insist that this is all that is ever going to be needed, ok,
> > > but I reserve the right to complain in 4 years when this needs to be
> > > changed :)
> >
> > In the event the ast2600 comes out and is 64-bit -- I can't imagine
> > that's likely to happen.  I can take solace that this won't be the
> > only thing that needs retrofitting.  But it wouldn't kill me to just
> > make the change.  I'll just have to tweak it to return failure in the
> > event the address provided isn't found in any region...
> >
> > Is that all that needs to change for 64-bit addressing support - given
> > your read of the driver?
>
> That's all that I noticed at first glance, yes.

Thanks, that's addressed in v8.

> I do dislike having
> custom user/kernel apis for random chips like this, but I don't know of
> a way to have a generic api for them at the moment as I really do not
> know what these chips do :(
>
> One would think that the firmware api would work for you, but given the
> complexity here, it does not seem that it would match up.

Yeah, this driver is basically just allowing control over a bridge and
allows for a convenient common use-case for such bridges.  I don't
have enough exposure to see if there's some commonality for
configuration and control of bridges across different chips, and I
would imagine they're very distinct.

>
> thanks,
>
> greg k-h

Thanks

^ permalink raw reply

* [PATCH v8 1/2] dt-bindings: misc: aspeed-p2a-ctrl: add support
From: Rob Herring @ 2019-03-28 16:50 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190327212155.80730-1-venture@google.com>

On Wed, 27 Mar 2019 14:21:55 -0700, Patrick Venture wrote:
> Document the ast2400, ast2500 PCI-to-AHB bridge control driver bindings.
> 
> Signed-off-by: Patrick Venture <venture@google.com>
> ---
> Changes for v8:
> - None
> Changes for v7:
> - Moved node under the syscon node it requires
> Changes for v6:
> - None
> Changes for v5:
> - None
> Changes for v4:
> - None
> Changes for v3:
> - None
> Changes for v2:
> - Added comment about syscon required parameter.
> ---
>  .../bindings/misc/aspeed-p2a-ctrl.txt         | 48 +++++++++++++++++++
>  1 file changed, 48 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/aspeed-p2a-ctrl.txt
> 

Please add Acked-by/Reviewed-by tags when posting new versions. However,
there's no need to repost patches *only* to add the tags. The upstream
maintainer will do that for acks received on the version they apply.

If a tag was not added on purpose, please state why and what changed.

^ permalink raw reply

* [PATCH v8 1/2] dt-bindings: misc: aspeed-p2a-ctrl: add support
From: Patrick Venture @ 2019-03-28 17:03 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190328165009.GA25705@bogus>

On Thu, Mar 28, 2019 at 9:50 AM Rob Herring <robh@kernel.org> wrote:
>
> On Wed, 27 Mar 2019 14:21:55 -0700, Patrick Venture wrote:
> > Document the ast2400, ast2500 PCI-to-AHB bridge control driver bindings.
> >
> > Signed-off-by: Patrick Venture <venture@google.com>
> > ---
> > Changes for v8:
> > - None
> > Changes for v7:
> > - Moved node under the syscon node it requires
> > Changes for v6:
> > - None
> > Changes for v5:
> > - None
> > Changes for v4:
> > - None
> > Changes for v3:
> > - None
> > Changes for v2:
> > - Added comment about syscon required parameter.
> > ---
> >  .../bindings/misc/aspeed-p2a-ctrl.txt         | 48 +++++++++++++++++++
> >  1 file changed, 48 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/misc/aspeed-p2a-ctrl.txt
> >
>
> Please add Acked-by/Reviewed-by tags when posting new versions. However,
> there's no need to repost patches *only* to add the tags. The upstream
> maintainer will do that for acks received on the version they apply.
>
> If a tag was not added on purpose, please state why and what changed.

Adding tags in this case is adding a change version?  I was doing this
to keep the two patches version-synced.  I thought that was required.
There was a version change in the other patch in this set.

^ permalink raw reply

* [PATCH] media: platform: Fix a kernel warning on clk control
From: Jae Hyun Yoo @ 2019-03-28 21:25 UTC (permalink / raw)
  To: linux-aspeed

Video engine clock control functions in the Aspeed video engine driver are
being called from multiple context without any protection so video clocks
can be disabled twice and eventually it causes a kernel warning with stack
dump printing out like below:

[  120.034729] WARNING: CPU: 0 PID: 1334 at drivers/clk/clk.c:684 clk_core_unprepare+0x13c/0x170
[  120.043252] eclk-gate already unprepared
[  120.047283] CPU: 0 PID: 1334 Comm: obmc-ikvm Tainted: G        W         5.0.3-b94b74e8b52db91fe4e99e0bb481ec8bf2b5b47c #1
[  120.058417] Hardware name: Generic DT based system
[  120.063219] Backtrace:
[  120.065787] [<80107cdc>] (dump_backtrace) from [<80107f10>] (show_stack+0x20/0x24)
[  120.073371]  r7:803a4ff0 r6:00000009 r5:00000000 r4:96197e1c
[  120.079152] [<80107ef0>] (show_stack) from [<8068f7d8>] (dump_stack+0x20/0x28)
[  120.086479] [<8068f7b8>] (dump_stack) from [<8011604c>] (__warn.part.3+0xb4/0xdc)
[  120.094068] [<80115f98>] (__warn.part.3) from [<801160e0>] (warn_slowpath_fmt+0x6c/0x90)
[  120.102164]  r6:000002ac r5:8080c0b8 r4:80a07008
[  120.106893] [<80116078>] (warn_slowpath_fmt) from [<803a4ff0>] (clk_core_unprepare+0x13c/0x170)
[  120.115686]  r3:8080cf8c r2:8080c17c
[  120.119276]  r7:97d68e58 r6:9df23200 r5:9668c260 r4:96459260
[  120.125046] [<803a4eb4>] (clk_core_unprepare) from [<803a707c>] (clk_unprepare+0x34/0x3c)
[  120.133226]  r5:9668c260 r4:96459260
[  120.136932] [<803a7048>] (clk_unprepare) from [<804f34bc>] (aspeed_video_off+0x44/0x48)
[  120.145031]  r5:9668c260 r4:9668cbc0
[  120.148647] [<804f3478>] (aspeed_video_off) from [<804f3fd0>] (aspeed_video_release+0x94/0x118)
[  120.157435]  r5:966a0cb8 r4:966a0800
[  120.161049] [<804f3f3c>] (aspeed_video_release) from [<804d2c58>] (v4l2_release+0xd4/0xe8)
[  120.169404]  r7:97d68e58 r6:9d087810 r5:9df23200 r4:966a0b20
[  120.175168] [<804d2b84>] (v4l2_release) from [<80236224>] (__fput+0x98/0x1c4)
[  120.182316]  r5:96698e78 r4:9df23200
[  120.185994] [<8023618c>] (__fput) from [<802363b8>] (____fput+0x18/0x1c)
[  120.192712]  r9:80a0700c r8:801011e4 r7:00000000 r6:80a64bbc r5:961dd560 r4:961dd89c
[  120.200562] [<802363a0>] (____fput) from [<80131c08>] (task_work_run+0x7c/0xa4)
[  120.207994] [<80131b8c>] (task_work_run) from [<80106884>] (do_work_pending+0x4a8/0x578)
[  120.216163]  r7:801011e4 r6:80a07008 r5:96197fb0 r4:ffffe000
[  120.221856] [<801063dc>] (do_work_pending) from [<8010106c>] (slow_work_pending+0xc/0x20)
[  120.230116] Exception stack(0x96197fb0 to 0x96197ff8)
[  120.235254] 7fa0:                                     00000000 76ccf094 00000000 00000000
[  120.243438] 7fc0: 00000008 00a11978 7eab3c30 00000006 00000000 00000000 475b0fa4 00000000
[  120.251692] 7fe0: 00000002 7eab3a40 00000000 47720e38 80000010 00000008
[  120.258396]  r10:00000000 r9:96196000 r8:801011e4 r7:00000006 r6:7eab3c30 r5:00a11978
[  120.266291]  r4:00000008

To prevent this issue, this commit adds spinlock protection and clock
status checking logic into the Aspeed video engine driver.

Fixes: d2b4387f3bdf ("media: platform: Add Aspeed Video Engine driver")
Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
Cc: Eddie James <eajames@linux.ibm.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Joel Stanley <joel@jms.id.au>
Cc: Andrew Jeffery <andrew@aj.id.au>
---
 drivers/media/platform/aspeed-video.c | 46 ++++++++++++++++++++-------
 1 file changed, 35 insertions(+), 11 deletions(-)

diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index 692e08ef38c0..9663ba4281a8 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -227,6 +227,7 @@ struct aspeed_video {
 	struct list_head buffers;
 	unsigned long flags;
 	unsigned int sequence;
+	bool is_video_on;
 
 	unsigned int max_compressed_size;
 	struct aspeed_video_addr srcs[2];
@@ -495,20 +496,28 @@ static void aspeed_video_reset(struct aspeed_video *video)
 
 static void aspeed_video_off(struct aspeed_video *video)
 {
-	aspeed_video_reset(video);
+	if (video->is_video_on) {
+		aspeed_video_reset(video);
+
+		/* Turn off the relevant clocks */
+		clk_disable_unprepare(video->vclk);
+		clk_disable_unprepare(video->eclk);
 
-	/* Turn off the relevant clocks */
-	clk_disable_unprepare(video->vclk);
-	clk_disable_unprepare(video->eclk);
+		video->is_video_on = false;
+	}
 }
 
 static void aspeed_video_on(struct aspeed_video *video)
 {
-	/* Turn on the relevant clocks */
-	clk_prepare_enable(video->eclk);
-	clk_prepare_enable(video->vclk);
+	if (!video->is_video_on) {
+		/* Turn on the relevant clocks */
+		clk_prepare_enable(video->eclk);
+		clk_prepare_enable(video->vclk);
+
+		aspeed_video_reset(video);
 
-	aspeed_video_reset(video);
+		video->is_video_on = true;
+	}
 }
 
 static void aspeed_video_bufs_done(struct aspeed_video *video,
@@ -526,12 +535,14 @@ static void aspeed_video_bufs_done(struct aspeed_video *video,
 
 static void aspeed_video_irq_res_change(struct aspeed_video *video)
 {
+	spin_lock(&video->lock);
 	dev_dbg(video->dev, "Resolution changed; resetting\n");
 
 	set_bit(VIDEO_RES_CHANGE, &video->flags);
 	clear_bit(VIDEO_FRAME_INPRG, &video->flags);
 
 	aspeed_video_off(video);
+	spin_unlock(&video->lock);
 	aspeed_video_bufs_done(video, VB2_BUF_STATE_ERROR);
 
 	schedule_delayed_work(&video->res_work, RESOLUTION_CHANGE_DELAY);
@@ -951,9 +962,13 @@ static void aspeed_video_init_regs(struct aspeed_video *video)
 
 static void aspeed_video_start(struct aspeed_video *video)
 {
+	unsigned long flags;
+
+	spin_lock_irqsave(&video->lock, flags);
 	aspeed_video_on(video);
 
 	aspeed_video_init_regs(video);
+	spin_unlock_irqrestore(&video->lock, flags);
 
 	/* Resolution set to 640x480 if no signal found */
 	aspeed_video_get_resolution(video);
@@ -969,6 +984,9 @@ static void aspeed_video_start(struct aspeed_video *video)
 
 static void aspeed_video_stop(struct aspeed_video *video)
 {
+	unsigned long flags;
+
+	spin_lock_irqsave(&video->lock, flags);
 	set_bit(VIDEO_STOPPED, &video->flags);
 	cancel_delayed_work_sync(&video->res_work);
 
@@ -982,6 +1000,7 @@ static void aspeed_video_stop(struct aspeed_video *video)
 
 	video->v4l2_input_status = V4L2_IN_ST_NO_SIGNAL;
 	video->flags = 0;
+	spin_unlock_irqrestore(&video->lock, flags);
 }
 
 static int aspeed_video_querycap(struct file *file, void *fh,
@@ -1319,16 +1338,21 @@ static void aspeed_video_resolution_work(struct work_struct *work)
 	struct delayed_work *dwork = to_delayed_work(work);
 	struct aspeed_video *video = container_of(dwork, struct aspeed_video,
 						  res_work);
-	u32 input_status = video->v4l2_input_status;
+	unsigned long flags;
+	u32 input_status;
 
+	spin_lock_irqsave(&video->lock, flags);
+	input_status = video->v4l2_input_status;
 	aspeed_video_on(video);
 
 	/* Exit early in case no clients remain */
-	if (test_bit(VIDEO_STOPPED, &video->flags))
+	if (test_bit(VIDEO_STOPPED, &video->flags)) {
+		spin_unlock_irqrestore(&video->lock, flags);
 		goto done;
+	}
 
 	aspeed_video_init_regs(video);
-
+	spin_unlock_irqrestore(&video->lock, flags);
 	aspeed_video_get_resolution(video);
 
 	if (video->detected_timings.width != video->active_timings.width ||
-- 
2.21.0


^ permalink raw reply related

* [PATCH v8 1/2] dt-bindings: misc: aspeed-p2a-ctrl: add support
From: Rob Herring @ 2019-03-29 13:37 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <CAO=notxxYF4noXbR1BeVrwqCBHaVCaXy+jPV4OVbex=w=fV+Rg@mail.gmail.com>

On Thu, Mar 28, 2019 at 12:03 PM Patrick Venture <venture@google.com> wrote:
>
> On Thu, Mar 28, 2019 at 9:50 AM Rob Herring <robh@kernel.org> wrote:
> >
> > On Wed, 27 Mar 2019 14:21:55 -0700, Patrick Venture wrote:
> > > Document the ast2400, ast2500 PCI-to-AHB bridge control driver bindings.
> > >
> > > Signed-off-by: Patrick Venture <venture@google.com>
> > > ---
> > > Changes for v8:
> > > - None
> > > Changes for v7:
> > > - Moved node under the syscon node it requires
> > > Changes for v6:
> > > - None
> > > Changes for v5:
> > > - None
> > > Changes for v4:
> > > - None
> > > Changes for v3:
> > > - None
> > > Changes for v2:
> > > - Added comment about syscon required parameter.
> > > ---
> > >  .../bindings/misc/aspeed-p2a-ctrl.txt         | 48 +++++++++++++++++++
> > >  1 file changed, 48 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/misc/aspeed-p2a-ctrl.txt
> > >
> >
> > Please add Acked-by/Reviewed-by tags when posting new versions. However,
> > there's no need to repost patches *only* to add the tags. The upstream
> > maintainer will do that for acks received on the version they apply.
> >
> > If a tag was not added on purpose, please state why and what changed.
>
> Adding tags in this case is adding a change version?  I was doing this
> to keep the two patches version-synced.  I thought that was required.
> There was a version change in the other patch in this set.

Adding tags is not considered a change. I gave a Reviewed-by in v7.
Subsequent versions should carry that tag if there's no change (or
only minor changes) in this patch. What happens in the other patches
is not really important. Maintainers are not going to go searching
thru the versions to find all the ack/review tags. And if I've already
reviewed this, I don't want to look at it again.

Rob

^ permalink raw reply

* [PATCH v8 1/2] dt-bindings: misc: aspeed-p2a-ctrl: add support
From: Patrick Venture @ 2019-03-29 14:56 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <CAL_JsqJSvzOQbJSgz7z2wZj9-z=bfhzF_C+662Gnf68TgAPFaA@mail.gmail.com>

On Fri, Mar 29, 2019 at 6:38 AM Rob Herring <robh@kernel.org> wrote:
>
> On Thu, Mar 28, 2019 at 12:03 PM Patrick Venture <venture@google.com> wrote:
> >
> > On Thu, Mar 28, 2019 at 9:50 AM Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Wed, 27 Mar 2019 14:21:55 -0700, Patrick Venture wrote:
> > > > Document the ast2400, ast2500 PCI-to-AHB bridge control driver bindings.
> > > >
> > > > Signed-off-by: Patrick Venture <venture@google.com>
> > > > ---
> > > > Changes for v8:
> > > > - None
> > > > Changes for v7:
> > > > - Moved node under the syscon node it requires
> > > > Changes for v6:
> > > > - None
> > > > Changes for v5:
> > > > - None
> > > > Changes for v4:
> > > > - None
> > > > Changes for v3:
> > > > - None
> > > > Changes for v2:
> > > > - Added comment about syscon required parameter.
> > > > ---
> > > >  .../bindings/misc/aspeed-p2a-ctrl.txt         | 48 +++++++++++++++++++
> > > >  1 file changed, 48 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/misc/aspeed-p2a-ctrl.txt
> > > >
> > >
> > > Please add Acked-by/Reviewed-by tags when posting new versions. However,
> > > there's no need to repost patches *only* to add the tags. The upstream
> > > maintainer will do that for acks received on the version they apply.
> > >
> > > If a tag was not added on purpose, please state why and what changed.
> >
> > Adding tags in this case is adding a change version?  I was doing this
> > to keep the two patches version-synced.  I thought that was required.
> > There was a version change in the other patch in this set.
>
> Adding tags is not considered a change. I gave a Reviewed-by in v7.
> Subsequent versions should carry that tag if there's no change (or
> only minor changes) in this patch. What happens in the other patches
> is not really important. Maintainers are not going to go searching
> thru the versions to find all the ack/review tags. And if I've already
> reviewed this, I don't want to look at it again.

Thank you, I didn't realize that had happened.

>
> Rob

^ permalink raw reply

* [PATCH v8 1/2] dt-bindings: misc: aspeed-p2a-ctrl: add support
From: Patrick Venture @ 2019-03-29 14:59 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <CAO=notx6K7xTW1VG6BGJFBM5k8EQsWXn3WoWSPSrZRsxkwueSA@mail.gmail.com>

On Fri, Mar 29, 2019 at 7:56 AM Patrick Venture <venture@google.com> wrote:
>
> On Fri, Mar 29, 2019 at 6:38 AM Rob Herring <robh@kernel.org> wrote:
> >
> > On Thu, Mar 28, 2019 at 12:03 PM Patrick Venture <venture@google.com> wrote:
> > >
> > > On Thu, Mar 28, 2019 at 9:50 AM Rob Herring <robh@kernel.org> wrote:
> > > >
> > > > On Wed, 27 Mar 2019 14:21:55 -0700, Patrick Venture wrote:
> > > > > Document the ast2400, ast2500 PCI-to-AHB bridge control driver bindings.
> > > > >
> > > > > Signed-off-by: Patrick Venture <venture@google.com>
> > > > > ---
> > > > > Changes for v8:
> > > > > - None
> > > > > Changes for v7:
> > > > > - Moved node under the syscon node it requires
> > > > > Changes for v6:
> > > > > - None
> > > > > Changes for v5:
> > > > > - None
> > > > > Changes for v4:
> > > > > - None
> > > > > Changes for v3:
> > > > > - None
> > > > > Changes for v2:
> > > > > - Added comment about syscon required parameter.
> > > > > ---
> > > > >  .../bindings/misc/aspeed-p2a-ctrl.txt         | 48 +++++++++++++++++++
> > > > >  1 file changed, 48 insertions(+)
> > > > >  create mode 100644 Documentation/devicetree/bindings/misc/aspeed-p2a-ctrl.txt
> > > > >
> > > >
> > > > Please add Acked-by/Reviewed-by tags when posting new versions. However,
> > > > there's no need to repost patches *only* to add the tags. The upstream
> > > > maintainer will do that for acks received on the version they apply.
> > > >
> > > > If a tag was not added on purpose, please state why and what changed.
> > >
> > > Adding tags in this case is adding a change version?  I was doing this
> > > to keep the two patches version-synced.  I thought that was required.
> > > There was a version change in the other patch in this set.
> >
> > Adding tags is not considered a change. I gave a Reviewed-by in v7.
> > Subsequent versions should carry that tag if there's no change (or
> > only minor changes) in this patch. What happens in the other patches
> > is not really important. Maintainers are not going to go searching
> > thru the versions to find all the ack/review tags. And if I've already
> > reviewed this, I don't want to look at it again.
>
> Thank you, I didn't realize that had happened.

I went back through my email and found the line of your email that
included it.  I apologize.

So, before I send the updated patch with your ack -- do I need to send
a v9? or is this just me sending v8 again?

>
> >
> > Rob

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox