* [PATCH v8 1/2] dt-bindings: misc: aspeed-p2a-ctrl: add support
From: Patrick Venture @ 2019-03-29 15:11 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <CAO=notxD=5pa5sT15a+VrA7qVNPBoJoTo=5sLxibBxc+HWcG4g@mail.gmail.com>
On Fri, Mar 29, 2019 at 7:59 AM Patrick Venture <venture@google.com> wrote:
>
> 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?
Sorry. I see you already answered that when you said that adding a
tag isn't considered a change. I have therefore re-sent v8 of this
patch with your tag added.
>
> >
> > >
> > > Rob
^ permalink raw reply
* [PATCH v8 1/2] dt-bindings: misc: aspeed-p2a-ctrl: add support
From: Rob Herring @ 2019-03-29 19:45 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <CAO=notynUJGUn3tuJZ_jFPdz23_T1FPyzS46mm5msphLnLW_JA@mail.gmail.com>
On Fri, Mar 29, 2019 at 10:11 AM Patrick Venture <venture@google.com> wrote:
>
> On Fri, Mar 29, 2019 at 7:59 AM Patrick Venture <venture@google.com> wrote:
> >
> > 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?
>
> Sorry. I see you already answered that when you said that adding a
> tag isn't considered a change. I have therefore re-sent v8 of this
> patch with your tag added.
To repeat my form letter again, you don't need to send a series again
just to add tags. IOW, if you send a v9, then add the tags. If v8 is
what a maintainer applies, the maintain will take care of adding them
(or patchwork does it for us):
> 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.
^ permalink raw reply
* [PATCH] media: platform: Fix a kernel warning on clk control
From: Eddie James @ 2019-03-29 20:08 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190328212537.29511-1-jae.hyun.yoo@linux.intel.com>
On 3/28/19 4:25 PM, Jae Hyun Yoo wrote:
> 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.
Thanks Jae. Do you have a reliable way to reproduce this? Haven't seen
it myself.
>
> 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;
This can probably be a flag, like VIDEO_CLOCKS_ON, not a new boolean.
>
> 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);
I'm working on a patch to remove the use of the reset line too.
> +
> + /* 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);
Is there a reason you're locking extra code? Why not put the lock in the
aspeed_video_on/off functions?
> 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 ||
^ permalink raw reply
* [PATCH] media: platform: Fix a kernel warning on clk control
From: Jae Hyun Yoo @ 2019-03-29 20:47 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <db524edd-0c6c-2bb7-848d-2261286b4b21@linux.ibm.com>
Hi Eddie,
On 3/29/2019 1:08 PM, Eddie James wrote:
> On 3/28/19 4:25 PM, Jae Hyun Yoo wrote:
>> To prevent this issue, this commit adds spinlock protection and clock
>> status checking logic into the Aspeed video engine driver.
>
> Thanks Jae. Do you have a reliable way to reproduce this? Haven't seen
> it myself.
It could be observed when user space releases a video engine dev entry
handle while video mode is changing. You could reproduce it by repeating
reload of KVM web page just after trigger a host reset. It rarely
happens.
>> +??? bool is_video_on;
>
> This can probably be a flag, like VIDEO_CLOCKS_ON, not a new boolean.
Okay, I'll add the bit field into video->flags.
>> +??????? aspeed_video_reset(video);
> I'm working on a patch to remove the use of the reset line too.
Right, since clk-aspeed module can trigger a reset while enabling the
vclk or eclk, it could be removed later, but this patch is not for that.
>> +??? spin_lock(&video->lock);
>
> Is there a reason you're locking extra code? Why not put the lock in the
> aspeed_video_on/off functions?
aspeed_video_on() and aspeed_video_off() can be called from driver
context and from interrupt context so some video enable/disable
relating code also need to be serialized.
Will submit v2 soon after replacing the boolean variable with
VIDEO_CLOCKS_ON flag.
Thanks,
Jae
^ permalink raw reply
* [PATCH v2] media: platform: fix a kernel warning on clk control
From: Jae Hyun Yoo @ 2019-03-29 21:27 UTC (permalink / raw)
To: linux-aspeed
video_on and video_off functions in the Aspeed video engine driver are
being called from multiple contexts 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 | 32 ++++++++++++++++++++++++---
1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index 692e08ef38c0..744d22ecc620 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -188,6 +188,7 @@ enum {
VIDEO_STREAMING,
VIDEO_FRAME_INPRG,
VIDEO_STOPPED,
+ VIDEO_CLOCKS_ON,
};
struct aspeed_video_addr {
@@ -495,20 +496,30 @@ static void aspeed_video_reset(struct aspeed_video *video)
static void aspeed_video_off(struct aspeed_video *video)
{
+ if (!test_bit(VIDEO_CLOCKS_ON, &video->flags))
+ return;
+
aspeed_video_reset(video);
/* Turn off the relevant clocks */
clk_disable_unprepare(video->vclk);
clk_disable_unprepare(video->eclk);
+
+ clear_bit(VIDEO_CLOCKS_ON, &video->flags);
}
static void aspeed_video_on(struct aspeed_video *video)
{
+ if (test_bit(VIDEO_CLOCKS_ON, &video->flags))
+ return;
+
/* Turn on the relevant clocks */
clk_prepare_enable(video->eclk);
clk_prepare_enable(video->vclk);
aspeed_video_reset(video);
+
+ set_bit(VIDEO_CLOCKS_ON, &video->flags);
}
static void aspeed_video_bufs_done(struct aspeed_video *video,
@@ -526,12 +537,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 +964,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 +986,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 +1002,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 +1340,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 2/2] drm: Add ASPEED GFX driver
From: Noralf Trønnes @ 2019-04-01 17:48 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190328054316.17939-3-joel@jms.id.au>
Den 28.03.2019 06.43, skrev Joel Stanley:
> 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
This forces fbdev to always be built. If you drop this,
DRM_FBDEV_EMULATION which is yes by default will set it. This way it's
possible to build a kernel without fbdev.
> + 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>
Please don't include drmP.h, we're trying to get rid of it.
> +#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>
I prefer includes sorted alphabetically, but no requirement.
> +
> +#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;
This can't be NULL since it's embedded.
> +
> + 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,
As Daniel said you can use drm_gem_fb_simple_display_pipe_prepare_fb here.
> + .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.
'is is' -> 'is'
> + *
> + * 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,
There's a vtable on the GEM object now, so this section:
> + .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,
Can be substituted with this:
.gem_create_object = drm_cma_gem_create_object_default_funcs,
.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_import_sg_table = drm_gem_cma_prime_import_sg_table,
.gem_prime_mmap = drm_gem_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>
Same here, don't include 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,
As Daniel said, you can use drm_connector_cleanup directly here.
Otherwise looks good:
Reviewed-by: Noralf Tr?nnes <noralf@tronnes.org>
> + .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;
> +}
>
^ permalink raw reply
* [PATCH 2/2] drm: Add ASPEED GFX driver
From: Sam Ravnborg @ 2019-04-01 18:48 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <730c140f-b4a2-7df9-3d90-9d47d720f643@tronnes.org>
Hi Joel.
Replying to Noralf's mail as I lost the original mail.
> > 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>
>
> I prefer includes sorted alphabetically, but no requirement.
Please sort them as Noralf suggest, as this makes it much more obvious
when one is adding a duplicate header.
> > +
> > + 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");
Add error code to your dev_err() calls, so one later better
can tell what was wrong if river fails to load.
Same goes for other dev_xxx calls in this function / dirver.
(Most dev_xxx have the return code, only a few seems to miss it)
> > +static const struct of_device_id aspeed_gfx_match[] = {
> > + { .compatible = "aspeed,ast2400-gfx" },
> > + { .compatible = "aspeed,ast2500-gfx" },
> > + { }
Many drivers put a /* sentinel */ comment inside the empty {}
With the few things suggested above considered this is
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
Sam
^ permalink raw reply
* [PATCH 2/2] drm: Add ASPEED GFX driver
From: Joel Stanley @ 2019-04-02 2:09 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190328075317.GR2665@phenom.ffwll.local>
On Thu, 28 Mar 2019 at 07:53, Daniel Vetter <daniel@ffwll.ch> wrote:
> > +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.
I this is the only mode we can do with the clock that we have available.
> Very nice driver!
>
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Thanks!
> 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
That makes sense. I'll send out a v2 with the changes you and Noralf
suggested, and a maintainers patch, once I've given it a spin on
hardware.
> We could also officially put drm/ast in there, if you want to maintain
> that too.
I haven't had much to do with that driver (it's physically inside the
BMC, but a different piece of hardware). I can probably be included so
I'm on cc for changes.
Cheers,
Joel
>
> Either way, patch also needs a MAINTAINERS entry.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2 0/3] drm: Add ASPEED BMC 'GFX' driver
From: Joel Stanley @ 2019-04-02 2:19 UTC (permalink / raw)
To: linux-aspeed
v2: Address review from Noralf and Daniel, add maintainers patch
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.
Joel Stanley (3):
dt-bindings: gpu: Add ASPEED GFX bindings document
drm: Add ASPEED GFX driver
MAINTAINERS: Add ASPEED BMC GFX DRM driver entry
.../devicetree/bindings/gpu/aspeed-gfx.txt | 41 +++
MAINTAINERS | 8 +
drivers/gpu/drm/Kconfig | 2 +
drivers/gpu/drm/Makefile | 1 +
drivers/gpu/drm/aspeed/Kconfig | 14 +
drivers/gpu/drm/aspeed/Makefile | 3 +
drivers/gpu/drm/aspeed/aspeed_gfx.h | 104 +++++++
drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c | 239 ++++++++++++++++
drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 266 ++++++++++++++++++
drivers/gpu/drm/aspeed/aspeed_gfx_out.c | 42 +++
10 files changed, 720 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 v2 1/3] dt-bindings: gpu: Add ASPEED GFX bindings document
From: Joel Stanley @ 2019-04-02 2:19 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190402021933.13071-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 v2 2/3] drm: Add ASPEED GFX driver
From: Joel Stanley @ 2019-04-02 2:19 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190402021933.13071-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>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Noralf Tr?nnes <noralf@tronnes.org>
--
v2:
Use drm_gem_fb_simple_display_pipe_prepare_fb
Sort headers
Remove drmP.h inclusion
Replace gem callbacks with those suggested by Noralf
Use drm_connector_cleanup
Drop null check for embedddd crtc object
Tweaks to spelling in kerneldoc
Drop DRM_KMS_FB_HELPER to make fbdev emulation optional
Remove ast2400 compatible as the ast2400 support is untested
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 | 14 ++
drivers/gpu/drm/aspeed/Makefile | 3 +
drivers/gpu/drm/aspeed/aspeed_gfx.h | 104 +++++++++
drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c | 239 ++++++++++++++++++++
drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 266 +++++++++++++++++++++++
drivers/gpu/drm/aspeed/aspeed_gfx_out.c | 42 ++++
8 files changed, 671 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..42b74d18a41b
--- /dev/null
+++ b/drivers/gpu/drm/aspeed/Kconfig
@@ -0,0 +1,14 @@
+config DRM_ASPEED_GFX
+ tristate "ASPEED BMC Display Controller"
+ depends on DRM && OF
+ select DRM_KMS_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 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..2d5c96334a36
--- /dev/null
+++ b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
@@ -0,0 +1,239 @@
+// 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;
+
+ 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_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 = drm_gem_fb_simple_display_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..d1f4c0d62f38
--- /dev/null
+++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
@@ -0,0 +1,266 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright 2018 IBM Corporation
+
+#include <linux/clk.h>
+#include <linux/irq.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/regmap.h>
+#include <linux/reset.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_probe_helper.h>
+#include <drm/drm_simple_kms_helper.h>
+
+#include "aspeed_gfx.h"
+
+/**
+ * DOC: ASPEED GFX Driver
+ *
+ * This driver is for the ASPEED BMC SoC's 'GFX' display hardware, also called
+ * the 'SOC Display Controller' in the datasheet. This driver runs on the ARM
+ * based BMC systems, unlike the ast driver which runs on a host CPU and is for
+ * a PCIe 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.
+ *
+ * The driver was written with the 'AST2500 Software Programming Guide' v17,
+ * which is available under NDA from ASPEED.
+ */
+
+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_create_object = drm_cma_gem_create_object_default_funcs,
+ .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_import_sg_table = drm_gem_cma_prime_import_sg_table,
+ .gem_prime_mmap = drm_gem_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,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..67ee5fa10055
--- /dev/null
+++ b/drivers/gpu/drm/aspeed/aspeed_gfx_out.c
@@ -0,0 +1,42 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright 2018 IBM Corporation
+
+#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 const struct drm_connector_funcs aspeed_gfx_connector_funcs = {
+ .fill_modes = drm_helper_probe_single_connector_modes,
+ .destroy = drm_connector_cleanup,
+ .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 v2 3/3] MAINTAINERS: Add ASPEED BMC GFX DRM driver entry
From: Joel Stanley @ 2019-04-02 2:19 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190402021933.13071-1-joel@jms.id.au>
This hardware is found inside ASPEED Baseboard Management Controller
(BMC) system on chips. It is called the 'SOC Display Controller' or 'GFX'.
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
MAINTAINERS | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index c18f5f10cf91..c3ad730e26f3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4909,6 +4909,14 @@ M: Dave Airlie <airlied@redhat.com>
S: Odd Fixes
F: drivers/gpu/drm/ast/
+DRM DRIVER FOR ASPEED BMC GFX
+M: Joel Stanley <joel@jms.id.au>
+L: linux-aspeed at lists.ozlabs.org
+T: git git://anongit.freedesktop.org/drm/drm-misc
+S: Supported
+F: drivers/gpu/drm/aspeed/
+F: Documentation/devicetree/bindings/gpu/aspeed-gfx.txt
+
DRM DRIVER FOR BOCHS VIRTUAL GPU
M: Gerd Hoffmann <kraxel@redhat.com>
L: virtualization at lists.linux-foundation.org
--
2.20.1
^ permalink raw reply related
* [PATCH v2 1/3] dt-bindings: gpu: Add ASPEED GFX bindings document
From: Andrew Jeffery @ 2019-04-02 3:08 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190402021933.13071-2-joel@jms.id.au>
On Tue, 2 Apr 2019, at 12:49, Joel Stanley wrote:
> 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
Lets remove [1] now that we have this document as what you've described
here is a super-set. Happy for that to be a follow-up patch, so:
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
[1] Documentation/devicetree/bindings/mfd/aspeed-gfx.txt
> +
> + - 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
* [PATCH v8 2/2] drivers/misc: Add Aspeed P2A control driver
From: Andrew Jeffery @ 2019-04-02 3:36 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190327212210.81481-1-venture@google.com>
On Thu, 28 Mar 2019, at 07:52, Patrick Venture wrote:
> 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;
This is a performance rather than a correctness nit, so I'm happy for it to be
cleaned up in a follow-up patch, but if you're going to memset/assign a bunch
of members to zero why not just use devm_kmalloc() instead? Or keep
devm_kzalloc() and not do the memset()/assignments of 0 to the members?
> +
> + /* 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);
You're fetching the syscon from the parent node, but your bindings document
requires the use of a syscon property. The bindings document is out of sync with
the implementation.
I believe Rob suggested making the node a child of the syscon (which is what
you've implemented here), so it's the bindings document that should be fixed.
> + 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");
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
> 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
* [PATCH v8 1/2] dt-bindings: misc: aspeed-p2a-ctrl: add support
From: Andrew Jeffery @ 2019-04-02 3:42 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190329151025.204094-1-venture@google.com>
Hi Patrick,
I held off on reviewing this until we'd hashed out what we needed in the driver.
I have some comments below.
On Sat, 30 Mar 2019, at 01:40, Patrick Venture wrote:
> Document the ast2400, ast2500 PCI-to-AHB bridge control driver bindings.
>
> Signed-off-by: Patrick Venture <venture@google.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
> 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.
The p2a-ctrl node is meant to be a child of the syscon. I noted this in my review
of the associated driver - you need to remove the description of the syscon
property.
> +
> +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"
The note above should go where you've described the syscon property above.
Cheers,
Andrew
> +
> +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
* [PATCH v2 2/3] drm: Add ASPEED GFX driver
From: Sam Ravnborg @ 2019-04-02 6:25 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190402021933.13071-3-joel@jms.id.au>
Hi Joel
> 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>
A drmP.h include was left here, can we have this removed too.
> +
> +static struct drm_driver aspeed_gfx_driver = {
> + .driver_features = DRIVER_GEM | DRIVER_MODESET |
> + DRIVER_PRIME | DRIVER_ATOMIC |
> + DRIVER_HAVE_IRQ,
DRIVER_HAVE_IRQ is obsolete and not needed anymore.
See drm_drv.h for details.
With these few things fixed:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
^ permalink raw reply
* [PATCH 0/5] Aspeed: Enable video engine
From: Eddie James @ 2019-04-02 18:24 UTC (permalink / raw)
To: linux-aspeed
This series enables the video engine on Aspeed BMC platforms. The video engine
clocking is added to the Aspeed clock driver. The use of the video engine reset
line, originally planned to be made externally available in the clock driver,
is removed from the video engine driver. Finally a node for the video engine is
added to the AST2500 devicetree.
The series also includes a missing property for reserved memory in the
devicetree documentation, and a small change to make the video engine driver
start without reserved memory.
Eddie James (5):
media: platform: Aspeed: Remove use of reset line
media: platform: Aspeed: Make reserved memory optional
media: dt-bindings: aspeed-video: Add missing memory-region property
clk: Aspeed: Setup video engine clocking
ARM: dts: aspeed-g5: Add video engine
.../devicetree/bindings/media/aspeed-video.txt | 6 ++++
arch/arm/boot/dts/aspeed-g5.dtsi | 10 ++++++
drivers/clk/clk-aspeed.c | 42 ++++++++++++++++++++--
drivers/media/platform/aspeed-video.c | 33 ++++-------------
4 files changed, 61 insertions(+), 30 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [PATCH 1/5] media: platform: Aspeed: Remove use of reset line
From: Eddie James @ 2019-04-02 18:24 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <1554229478-5621-1-git-send-email-eajames@linux.ibm.com>
The reset line is toggled by enabling the clocks, so it's not necessary
to manually toggle the reset as well.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
drivers/media/platform/aspeed-video.c | 27 +++++----------------------
1 file changed, 5 insertions(+), 22 deletions(-)
diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index 692e08e..55c55a6 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -14,7 +14,6 @@
#include <linux/of_irq.h>
#include <linux/of_reserved_mem.h>
#include <linux/platform_device.h>
-#include <linux/reset.h>
#include <linux/sched.h>
#include <linux/spinlock.h>
#include <linux/string.h>
@@ -208,7 +207,6 @@ struct aspeed_video {
void __iomem *base;
struct clk *eclk;
struct clk *vclk;
- struct reset_control *rst;
struct device *dev;
struct v4l2_ctrl_handler ctrl_handler;
@@ -483,19 +481,10 @@ static void aspeed_video_enable_mode_detect(struct aspeed_video *video)
aspeed_video_update(video, VE_SEQ_CTRL, 0, VE_SEQ_CTRL_TRIG_MODE_DET);
}
-static void aspeed_video_reset(struct aspeed_video *video)
-{
- /* Reset the engine */
- reset_control_assert(video->rst);
-
- /* Don't usleep here; function may be called in interrupt context */
- udelay(100);
- reset_control_deassert(video->rst);
-}
-
static void aspeed_video_off(struct aspeed_video *video)
{
- aspeed_video_reset(video);
+ /* Disable interrupts */
+ aspeed_video_write(video, VE_INTERRUPT_CTRL, 0);
/* Turn off the relevant clocks */
clk_disable_unprepare(video->vclk);
@@ -507,8 +496,6 @@ static void aspeed_video_on(struct aspeed_video *video)
/* Turn on the relevant clocks */
clk_prepare_enable(video->eclk);
clk_prepare_enable(video->vclk);
-
- aspeed_video_reset(video);
}
static void aspeed_video_bufs_done(struct aspeed_video *video,
@@ -1464,7 +1451,9 @@ static void aspeed_video_stop_streaming(struct vb2_queue *q)
* Need to force stop any DMA and try and get HW into a good
* state for future calls to start streaming again.
*/
- aspeed_video_reset(video);
+ aspeed_video_off(video);
+ aspeed_video_on(video);
+
aspeed_video_init_regs(video);
aspeed_video_get_resolution(video);
@@ -1619,12 +1608,6 @@ static int aspeed_video_init(struct aspeed_video *video)
return PTR_ERR(video->vclk);
}
- video->rst = devm_reset_control_get_exclusive(dev, NULL);
- if (IS_ERR(video->rst)) {
- dev_err(dev, "Unable to get VE reset\n");
- return PTR_ERR(video->rst);
- }
-
rc = of_reserved_mem_device_init(dev);
if (rc) {
dev_err(dev, "Unable to reserve memory\n");
--
1.8.3.1
^ permalink raw reply related
* [PATCH 2/5] media: platform: Aspeed: Make reserved memory optional
From: Eddie James @ 2019-04-02 18:24 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <1554229478-5621-1-git-send-email-eajames@linux.ibm.com>
Reserved memory doesn't need to be required; system memory would work
fine.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
drivers/media/platform/aspeed-video.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index 55c55a6..8144fe3 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -1608,11 +1608,7 @@ static int aspeed_video_init(struct aspeed_video *video)
return PTR_ERR(video->vclk);
}
- rc = of_reserved_mem_device_init(dev);
- if (rc) {
- dev_err(dev, "Unable to reserve memory\n");
- return rc;
- }
+ of_reserved_mem_device_init(dev);
rc = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
if (rc) {
--
1.8.3.1
^ permalink raw reply related
* [PATCH 0/5] Aspeed: Enable video engine
From: Eddie James @ 2019-04-02 18:24 UTC (permalink / raw)
To: linux-aspeed
This series enables the video engine on Aspeed BMC platforms. The video engine
clocking is added to the Aspeed clock driver. The use of the video engine reset
line, originally planned to be made externally available in the clock driver,
is removed from the video engine driver. Finally a node for the video engine is
added to the AST2500 devicetree.
The series also includes a missing property for reserved memory in the
devicetree documentation, and a small change to make the video engine driver
start without reserved memory.
Eddie James (5):
media: platform: Aspeed: Remove use of reset line
media: platform: Aspeed: Make reserved memory optional
media: dt-bindings: aspeed-video: Add missing memory-region property
clk: Aspeed: Setup video engine clocking
ARM: dts: aspeed-g5: Add video engine
.../devicetree/bindings/media/aspeed-video.txt | 6 ++++
arch/arm/boot/dts/aspeed-g5.dtsi | 10 ++++++
drivers/clk/clk-aspeed.c | 42 ++++++++++++++++++++--
drivers/media/platform/aspeed-video.c | 33 ++++-------------
4 files changed, 61 insertions(+), 30 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [PATCH 1/5] media: platform: Aspeed: Remove use of reset line
From: Eddie James @ 2019-04-02 18:25 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <1554229504-5661-1-git-send-email-eajames@linux.ibm.com>
The reset line is toggled by enabling the clocks, so it's not necessary
to manually toggle the reset as well.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
drivers/media/platform/aspeed-video.c | 27 +++++----------------------
1 file changed, 5 insertions(+), 22 deletions(-)
diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index 692e08e..55c55a6 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -14,7 +14,6 @@
#include <linux/of_irq.h>
#include <linux/of_reserved_mem.h>
#include <linux/platform_device.h>
-#include <linux/reset.h>
#include <linux/sched.h>
#include <linux/spinlock.h>
#include <linux/string.h>
@@ -208,7 +207,6 @@ struct aspeed_video {
void __iomem *base;
struct clk *eclk;
struct clk *vclk;
- struct reset_control *rst;
struct device *dev;
struct v4l2_ctrl_handler ctrl_handler;
@@ -483,19 +481,10 @@ static void aspeed_video_enable_mode_detect(struct aspeed_video *video)
aspeed_video_update(video, VE_SEQ_CTRL, 0, VE_SEQ_CTRL_TRIG_MODE_DET);
}
-static void aspeed_video_reset(struct aspeed_video *video)
-{
- /* Reset the engine */
- reset_control_assert(video->rst);
-
- /* Don't usleep here; function may be called in interrupt context */
- udelay(100);
- reset_control_deassert(video->rst);
-}
-
static void aspeed_video_off(struct aspeed_video *video)
{
- aspeed_video_reset(video);
+ /* Disable interrupts */
+ aspeed_video_write(video, VE_INTERRUPT_CTRL, 0);
/* Turn off the relevant clocks */
clk_disable_unprepare(video->vclk);
@@ -507,8 +496,6 @@ static void aspeed_video_on(struct aspeed_video *video)
/* Turn on the relevant clocks */
clk_prepare_enable(video->eclk);
clk_prepare_enable(video->vclk);
-
- aspeed_video_reset(video);
}
static void aspeed_video_bufs_done(struct aspeed_video *video,
@@ -1464,7 +1451,9 @@ static void aspeed_video_stop_streaming(struct vb2_queue *q)
* Need to force stop any DMA and try and get HW into a good
* state for future calls to start streaming again.
*/
- aspeed_video_reset(video);
+ aspeed_video_off(video);
+ aspeed_video_on(video);
+
aspeed_video_init_regs(video);
aspeed_video_get_resolution(video);
@@ -1619,12 +1608,6 @@ static int aspeed_video_init(struct aspeed_video *video)
return PTR_ERR(video->vclk);
}
- video->rst = devm_reset_control_get_exclusive(dev, NULL);
- if (IS_ERR(video->rst)) {
- dev_err(dev, "Unable to get VE reset\n");
- return PTR_ERR(video->rst);
- }
-
rc = of_reserved_mem_device_init(dev);
if (rc) {
dev_err(dev, "Unable to reserve memory\n");
--
1.8.3.1
^ permalink raw reply related
* [PATCH 2/5] media: platform: Aspeed: Make reserved memory optional
From: Eddie James @ 2019-04-02 18:25 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <1554229504-5661-1-git-send-email-eajames@linux.ibm.com>
Reserved memory doesn't need to be required; system memory would work
fine.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
drivers/media/platform/aspeed-video.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index 55c55a6..8144fe3 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -1608,11 +1608,7 @@ static int aspeed_video_init(struct aspeed_video *video)
return PTR_ERR(video->vclk);
}
- rc = of_reserved_mem_device_init(dev);
- if (rc) {
- dev_err(dev, "Unable to reserve memory\n");
- return rc;
- }
+ of_reserved_mem_device_init(dev);
rc = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
if (rc) {
--
1.8.3.1
^ permalink raw reply related
* [PATCH 3/5] media: dt-bindings: aspeed-video: Add missing memory-region property
From: Eddie James @ 2019-04-02 18:25 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <1554229504-5661-1-git-send-email-eajames@linux.ibm.com>
Missed documenting this property in the initial commit.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
Documentation/devicetree/bindings/media/aspeed-video.txt | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/aspeed-video.txt b/Documentation/devicetree/bindings/media/aspeed-video.txt
index 78b464a..346c2d3 100644
--- a/Documentation/devicetree/bindings/media/aspeed-video.txt
+++ b/Documentation/devicetree/bindings/media/aspeed-video.txt
@@ -14,6 +14,11 @@ Required properties:
the VE
- interrupts: the interrupt associated with the VE on this platform
+Optional properties:
+ - memory-region:
+ phandle to a memory region to allocate from, as defined in
+ Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
+
Example:
video-engine at 1e700000 {
@@ -23,4 +28,5 @@ video-engine at 1e700000 {
clock-names = "vclk", "eclk";
resets = <&syscon ASPEED_RESET_VIDEO>;
interrupts = <7>;
+ memory-region = <&video_engine_memory>
};
--
1.8.3.1
^ permalink raw reply related
* [PATCH 4/5] clk: Aspeed: Setup video engine clocking
From: Eddie James @ 2019-04-02 18:25 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <1554229504-5661-1-git-send-email-eajames@linux.ibm.com>
Add eclk mux and clock divider table. Also change the video engine reset
to the correct clock; it was previously on the video capture but needs
to be on the video engine clock.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
drivers/clk/clk-aspeed.c | 42 +++++++++++++++++++++++++++++++++++++++---
1 file changed, 39 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
index 5961367..42b4df6 100644
--- a/drivers/clk/clk-aspeed.c
+++ b/drivers/clk/clk-aspeed.c
@@ -87,10 +87,10 @@ struct aspeed_clk_gate {
/* TODO: ask Aspeed about the actual parent data */
static const struct aspeed_gate_data aspeed_gates[] = {
/* clk rst name parent flags */
- [ASPEED_CLK_GATE_ECLK] = { 0, -1, "eclk-gate", "eclk", 0 }, /* Video Engine */
+ [ASPEED_CLK_GATE_ECLK] = { 0, 6, "eclk-gate", "eclk", 0 }, /* Video Engine */
[ASPEED_CLK_GATE_GCLK] = { 1, 7, "gclk-gate", NULL, 0 }, /* 2D engine */
[ASPEED_CLK_GATE_MCLK] = { 2, -1, "mclk-gate", "mpll", CLK_IS_CRITICAL }, /* SDRAM */
- [ASPEED_CLK_GATE_VCLK] = { 3, 6, "vclk-gate", NULL, 0 }, /* Video Capture */
+ [ASPEED_CLK_GATE_VCLK] = { 3, -1, "vclk-gate", NULL, 0 }, /* Video Capture */
[ASPEED_CLK_GATE_BCLK] = { 4, 8, "bclk-gate", "bclk", CLK_IS_CRITICAL }, /* PCIe/PCI */
[ASPEED_CLK_GATE_DCLK] = { 5, -1, "dclk-gate", NULL, CLK_IS_CRITICAL }, /* DAC */
[ASPEED_CLK_GATE_REFCLK] = { 6, -1, "refclk-gate", "clkin", CLK_IS_CRITICAL },
@@ -113,6 +113,24 @@ struct aspeed_clk_gate {
[ASPEED_CLK_GATE_LHCCLK] = { 28, -1, "lhclk-gate", "lhclk", 0 }, /* LPC master/LPC+ */
};
+static const char * const eclk_parent_names[] = {
+ "mpll",
+ "hpll",
+ "dpll",
+};
+
+static const struct clk_div_table ast2500_eclk_div_table[] = {
+ { 0x0, 2 },
+ { 0x1, 2 },
+ { 0x2, 3 },
+ { 0x3, 4 },
+ { 0x4, 5 },
+ { 0x5, 6 },
+ { 0x6, 7 },
+ { 0x7, 8 },
+ { 0 }
+};
+
static const struct clk_div_table ast2500_mac_div_table[] = {
{ 0x0, 4 }, /* Yep, really. Aspeed confirmed this is correct */
{ 0x1, 4 },
@@ -192,18 +210,21 @@ static struct clk_hw *aspeed_ast2500_calc_pll(const char *name, u32 val)
struct aspeed_clk_soc_data {
const struct clk_div_table *div_table;
+ const struct clk_div_table *eclk_div_table;
const struct clk_div_table *mac_div_table;
struct clk_hw *(*calc_pll)(const char *name, u32 val);
};
static const struct aspeed_clk_soc_data ast2500_data = {
.div_table = ast2500_div_table,
+ .eclk_div_table = ast2500_eclk_div_table,
.mac_div_table = ast2500_mac_div_table,
.calc_pll = aspeed_ast2500_calc_pll,
};
static const struct aspeed_clk_soc_data ast2400_data = {
.div_table = ast2400_div_table,
+ .eclk_div_table = ast2400_div_table,
.mac_div_table = ast2400_div_table,
.calc_pll = aspeed_ast2400_calc_pll,
};
@@ -522,6 +543,22 @@ static int aspeed_clk_probe(struct platform_device *pdev)
return PTR_ERR(hw);
aspeed_clk_data->hws[ASPEED_CLK_24M] = hw;
+ hw = clk_hw_register_mux(dev, "eclk-mux", eclk_parent_names,
+ ARRAY_SIZE(eclk_parent_names), 0,
+ scu_base + ASPEED_CLK_SELECTION, 2, 0x3, 0,
+ &aspeed_clk_lock);
+ if (IS_ERR(hw))
+ return PTR_ERR(hw);
+ aspeed_clk_data->hws[ASPEED_CLK_ECLK_MUX] = hw;
+
+ hw = clk_hw_register_divider_table(dev, "eclk", "eclk-mux", 0,
+ scu_base + ASPEED_CLK_SELECTION, 28,
+ 3, 0, soc_data->eclk_div_table,
+ &aspeed_clk_lock);
+ if (IS_ERR(hw))
+ return PTR_ERR(hw);
+ aspeed_clk_data->hws[ASPEED_CLK_ECLK] = hw;
+
/*
* TODO: There are a number of clocks that not included in this driver
* as more information is required:
@@ -531,7 +568,6 @@ static int aspeed_clk_probe(struct platform_device *pdev)
* RGMII
* RMII
* UART[1..5] clock source mux
- * Video Engine (ECLK) mux and clock divider
*/
for (i = 0; i < ARRAY_SIZE(aspeed_gates); i++) {
--
1.8.3.1
^ permalink raw reply related
* [PATCH 5/5] ARM: dts: aspeed-g5: Add video engine
From: Eddie James @ 2019-04-02 18:25 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <1554229504-5661-1-git-send-email-eajames@linux.ibm.com>
Add a node to describe the video engine on the AST2500.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
arch/arm/boot/dts/aspeed-g5.dtsi | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
index 85ed9db..c6d5edc 100644
--- a/arch/arm/boot/dts/aspeed-g5.dtsi
+++ b/arch/arm/boot/dts/aspeed-g5.dtsi
@@ -243,6 +243,16 @@
status = "disabled";
};
+ video: video at 1e700000 {
+ compatible = "aspeed,ast2500-video-engine";
+ reg = <0x1e700000 0x1000>;
+ clocks = <&syscon ASPEED_CLK_GATE_VCLK>,
+ <&syscon ASPEED_CLK_GATE_ECLK>;
+ clock-names = "vclk", "eclk";
+ interrupts = <7>;
+ status = "disabled";
+ };
+
sram: sram at 1e720000 {
compatible = "mmio-sram";
reg = <0x1e720000 0x9000>; // 36K
--
1.8.3.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox