On 12/13/2018 07:09 PM, Joel Stanley wrote:
On Wed, 12 Dec 2018 at 04:09, Eddie James <eaja...@linux.ibm.com> wrote:
The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs
can capture and compress video data from digital or analog sources. With
the Aspeed chip acting a service processor, the Video Engine can capture
the host processor graphics output.
+ASPEED VIDEO ENGINE DRIVER
+M:     Eddie James <eaja...@linux.ibm.com>
+L:     linux-media@vger.kernel.org
+L:     open...@lists.ozlabs.org (moderated for non-subscribers)
We tend to use the linux-aspeed list for upstream kernel discussions.
Up to you if you want to use the openbmc list though.

  source "drivers/media/platform/omap/Kconfig"

+config VIDEO_ASPEED
+       tristate "Aspeed AST2400 and AST2500 Video Engine driver"
+       depends on VIDEO_V4L2
+       select VIDEOBUF2_DMA_CONTIG
+       help
+         Support for the Aspeed Video Engine (VE) embedded in the Aspeed
+         AST2400 and AST2500 SOCs. The VE can capture and compress video data
+         from digital or analog sources.
This might need updating in response to my questions below about
ast2400 testing.

+++ b/drivers/media/platform/aspeed-video.c
@@ -0,0 +1,1729 @@
+// SPDX-License-Identifier: GPL-2.0+
You need to put this there as well:

// Copyright 2018 IBM Corp


+static int aspeed_video_init(struct aspeed_video *video)
+{
+       int irq;
+       int rc;
+       struct device *dev = video->dev;
+
+       irq = irq_of_parse_and_map(dev->of_node, 0);
+       if (!irq) {
+               dev_err(dev, "Unable to find IRQ\n");
+               return -ENODEV;
+       }
+
+       rc = devm_request_irq(dev, irq, aspeed_video_irq, IRQF_SHARED,
The datasheet indicates this IRQ is for the video engline only, so I
don't think you want IRQF_SHARED.

+                             DEVICE_NAME, video);
+       if (rc < 0) {
+               dev_err(dev, "Unable to request IRQ %d\n", irq);
+               return rc;
+       }
+
+       video->eclk = devm_clk_get(dev, "eclk");
+       if (IS_ERR(video->eclk)) {
+               dev_err(dev, "Unable to get ECLK\n");
+               return PTR_ERR(video->eclk);
+       }
+
+       video->vclk = devm_clk_get(dev, "vclk");
+       if (IS_ERR(video->vclk)) {
+               dev_err(dev, "Unable to get VCLK\n");
+               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);
+       }
As discussed in the clock driver, this can go as you've already
released the reset when enabling the eclk.

I use the reset control by itself during a resolution change, so I would like to have it available.


However, you're requesting the clock without enabling it. You need to
do a clk_prepare_enable().

That's because the clock is only enabled when the device is opened. No reason to turn it on at boot time.


+
+       rc = of_reserved_mem_device_init(dev);
+       if (rc) {
+               dev_err(dev, "Unable to reserve memory\n");
+               return rc;
+       }
+
+       rc = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
+       if (rc) {
+               dev_err(dev, "Failed to set DMA mask\n");
+               of_reserved_mem_device_release(dev);
+               return rc;
+       }
+
+       if (!aspeed_video_alloc_buf(video, &video->jpeg,
+                                   VE_JPEG_HEADER_SIZE)) {
+               dev_err(dev, "Failed to allocate DMA for JPEG header\n");
+               of_reserved_mem_device_release(dev);
+               return rc;
+       }
+
+       aspeed_video_init_jpeg_table(video->jpeg.virt, video->yuv420);
+
+       return 0;
+}
+
+static const struct of_device_id aspeed_video_of_match[] = {
+       { .compatible = "aspeed,ast2400-video-engine" },
I noticed the clock driver did not have the changed required for the
2400. Have you tested this on the ast2400?

The clocking setup is different on the ast2400. Xiuzhi on the openbmc list has been testing on the ast2400 successfully.

Thanks,
Eddie



+       { .compatible = "aspeed,ast2500-video-engine" },
+       {}
+};
+MODULE_DEVICE_TABLE(of, aspeed_video_of_match);
+
+static struct platform_driver aspeed_video_driver = {
+       .driver = {
+               .name = DEVICE_NAME,
+               .of_match_table = aspeed_video_of_match,
+       },
+       .probe = aspeed_video_probe,
+       .remove = aspeed_video_remove,
+};

Reply via email to