lkml.org 
[lkml]   [2012]   [Dec]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCHv4 5/8] drm: tegra: Remove redundant host1x
    On 28.12.2012 23:21, Thierry Reding wrote:
    > Instead of going over this back and forth, I've decided to rewrite this
    > patch from scratch the way I think it should be done. Maybe that'll make
    > things clearer. I haven't tested it on real hardware yet because I don't
    > have access over the holidays, but I'll post the patch once I've
    > verified that it actually works. The code is based on patches 1-4 of
    > this series and is meant to replace patch 5.

    I'm still not happy that host1x needs to know about drm. That's just a
    wrong dependency, and wrong dependencies always end up biting back. We
    need to figure out solution that satisfies both mine and your
    requirements and reduce complexity.

    DC/HDMI/GR2D probes are using the global data only for managing the
    lists "drm_clients" and "clients". "clients" list is only required after
    tegra_drm_load(). "drm_clients" is required to establish driver load order.

    With dummy device, we can determine the registration (and probe) order.
    tegra-drm can register the drivers for DC/HDMI/GR2D devices first, and
    as last the device and driver for tegra-drm.

    tegra-drm probe will allocate the global data, enumerate all drivers and
    add them to the clients list. If one driver is not initialized, it'll
    return with -EPROBE_DEFER and will be called again later. When all this
    is successful, it'll call drm_platform_init().

    The advantages:
    * No management of drm_clients list
    * No mucking with drvdata
    * host1x doesn't need to know about drm
    * The global data is allocated and deallocated by tegra-drm probe and
    remove, and accessed only via drm_device->dev_private
    * Much less code

    Something like the attached patch - not tested, as I don't have access
    to hw now, but it shows the idea. It's based on patches 1-5 in the
    series, and could replace patch 5.

    Terje
    From a97d475d65e68ce37c345924171dc57c5e7729ee Mon Sep 17 00:00:00 2001
    From: Terje Bergstrom <tbergstrom@nvidia.com>
    Date: Sat, 29 Dec 2012 11:43:54 +0200
    Subject: [PATCH] drm: tegra: Postpone usage of global data

    Change-Id: Ibfd1d4eeb267ac185de4508a1547fb009b80e93a
    Signed-off-by: Terje Bergstrom <tbergstrom@nvidia.com>
    ---
    drivers/gpu/drm/tegra/dc.c | 18 -----
    drivers/gpu/drm/tegra/drm.c | 151 +++++++++++++-----------------------------
    drivers/gpu/drm/tegra/drm.h | 1 -
    drivers/gpu/drm/tegra/gr2d.c | 7 --
    drivers/gpu/drm/tegra/hdmi.c | 18 -----
    5 files changed, 47 insertions(+), 148 deletions(-)

    diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
    index 24bcd06..73056b7 100644
    --- a/drivers/gpu/drm/tegra/dc.c
    +++ b/drivers/gpu/drm/tegra/dc.c
    @@ -740,7 +740,6 @@ static int tegra_dc_probe(struct platform_device *pdev)
    struct resource *regs;
    struct tegra_dc *dc;
    int err;
    - struct tegradrm *tegradrm = platform_get_drvdata(pdev);

    dc = devm_kzalloc(&pdev->dev, sizeof(*dc), GFP_KERNEL);
    if (!dc)
    @@ -780,7 +779,6 @@ static int tegra_dc_probe(struct platform_device *pdev)
    INIT_LIST_HEAD(&dc->client.list);
    dc->client.ops = &dc_client_ops;
    dc->client.dev = &pdev->dev;
    - dc->client.tegradrm = tegradrm;

    err = tegra_dc_rgb_probe(dc);
    if (err < 0 && err != -ENODEV) {
    @@ -788,13 +786,6 @@ static int tegra_dc_probe(struct platform_device *pdev)
    return err;
    }

    - err = tegra_drm_register_client(tegradrm, &dc->client);
    - if (err < 0) {
    - dev_err(&pdev->dev, "failed to register tegra drm client: %d\n",
    - err);
    - return err;
    - }
    -
    platform_set_drvdata(pdev, dc);

    return 0;
    @@ -803,15 +794,6 @@ static int tegra_dc_probe(struct platform_device *pdev)
    static int tegra_dc_remove(struct platform_device *pdev)
    {
    struct tegra_dc *dc = platform_get_drvdata(pdev);
    - int err;
    -
    - err = tegra_drm_unregister_client(dc->client.tegradrm,
    - &dc->client);
    - if (err < 0) {
    - dev_err(&pdev->dev, "failed to unregister tegra_drm client: %d\n",
    - err);
    - return err;
    - }

    clk_disable_unprepare(dc->clk);

    diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
    index 637b621..520c281 100644
    --- a/drivers/gpu/drm/tegra/drm.c
    +++ b/drivers/gpu/drm/tegra/drm.c
    @@ -25,12 +25,6 @@
    #define DRIVER_MINOR 0
    #define DRIVER_PATCHLEVEL 0

    -struct tegra_drm_client_entry {
    - struct tegra_drm_client *client;
    - struct device_node *np;
    - struct list_head list;
    -};
    -
    static int tegra_drm_add_client(struct device *dev, void *data)
    {
    static const char * const compat[] = {
    @@ -42,102 +36,26 @@ static int tegra_drm_add_client(struct device *dev, void *data)
    "nvidia,tegra30-gr2d"
    };
    struct tegradrm *tegradrm = data;
    - struct tegra_drm_client_entry *client;
    + struct tegra_drm_client *client;
    unsigned int i;

    + /* Check if dev is a tegra-drm device, and add to client list */
    for (i = 0; i < ARRAY_SIZE(compat); i++) {
    if (of_device_is_compatible(dev->of_node, compat[i]) &&
    of_device_is_available(dev->of_node)) {
    - client = kzalloc(sizeof(*client), GFP_KERNEL);
    + client = dev_get_drvdata(dev);
    if (!client)
    - return -ENOMEM;
    + return -EPROBE_DEFER;

    INIT_LIST_HEAD(&client->list);
    - client->np = of_node_get(dev->of_node);

    - list_add_tail(&client->list, &tegradrm->drm_clients);
    - dev_set_drvdata(dev, tegradrm);
    + list_add_tail(&client->list, &tegradrm->clients);
    }
    }

    return 0;
    }

    -static int tegra_drm_parse_dt(struct tegradrm *tegradrm)
    -{
    - int err;
    - struct device *dev;
    -
    - /* host1x is parent of all devices */
    - dev = bus_find_device_by_name(&platform_bus_type, NULL, "host1x");
    - if (!dev)
    - return -ENODEV;
    -
    - /* find devices that are available and add them into the 'required'
    - * list */
    - err = device_for_each_child(dev, tegradrm, tegra_drm_add_client);
    -
    - return err;
    -}
    -
    -int tegra_drm_register_client(struct tegradrm *tegradrm,
    - struct tegra_drm_client *client)
    -{
    - struct tegra_drm_client_entry *drm, *tmp;
    - int err;
    -
    - mutex_lock(&tegradrm->clients_lock);
    - list_add_tail(&client->list, &tegradrm->clients);
    - mutex_unlock(&tegradrm->clients_lock);
    -
    - /* remove this device from 'required' list */
    - list_for_each_entry_safe(drm, tmp, &tegradrm->drm_clients, list)
    - if (drm->np == client->dev->of_node)
    - list_del(&drm->list);
    -
    - /* if all required devices are found, register drm device */
    - if (list_empty(&tegradrm->drm_clients)) {
    - struct platform_device *pdev =
    - to_platform_device(tegradrm->dev);
    - err = drm_platform_init(&tegra_drm_driver, pdev);
    - if (err < 0) {
    - dev_err(client->dev, "drm_platform_init(): %d\n", err);
    - return err;
    - }
    - }
    -
    - return 0;
    -}
    -
    -int tegra_drm_unregister_client(struct tegradrm *tegradrm,
    - struct tegra_drm_client *client)
    -{
    - struct tegra_drm_client *tmp;
    -
    - list_for_each_entry_safe(client, tmp, &tegradrm->clients, list) {
    - if (client->ops && client->ops->drm_exit) {
    - int err = client->ops->drm_exit(client);
    - if (err < 0) {
    - dev_err(client->dev,
    - "DRM cleanup failed for %s: %d\n",
    - dev_name(client->dev), err);
    - return err;
    - }
    - }
    -
    - /* if this is the last device, unregister the drm driver */
    - if (client->list.next == &tegradrm->clients) {
    - struct platform_device *pdev =
    - to_platform_device(tegradrm->dev);
    - drm_platform_exit(&tegra_drm_driver, pdev);
    - }
    -
    - list_del_init(&client->list);
    - }
    -
    - return 0;
    -}
    -
    static int tegra_drm_load(struct drm_device *drm, unsigned long flags)
    {
    struct tegra_drm_client *client;
    @@ -175,6 +93,21 @@ static int tegra_drm_load(struct drm_device *drm, unsigned long flags)

    static int tegra_drm_unload(struct drm_device *drm)
    {
    + struct tegradrm *tegradrm = drm->dev_private;
    + struct tegra_drm_client *client;
    +
    + list_for_each_entry(client, &tegradrm->clients, list) {
    + if (client->ops && client->ops->drm_exit) {
    + int err = client->ops->drm_exit(client);
    + if (err < 0) {
    + dev_err(client->dev,
    + "DRM cleanup failed for %s: %d\n",
    + dev_name(client->dev), err);
    + return err;
    + }
    + }
    + }
    +
    drm_kms_helper_poll_fini(drm);
    tegra_drm_fb_exit(drm);

    @@ -232,9 +165,17 @@ static int tegra_drm_probe(struct platform_device *pdev)
    mutex_init(&tegradrm->clients_lock);
    INIT_LIST_HEAD(&tegradrm->clients);

    - err = tegra_drm_parse_dt(tegradrm);
    + /* find devices that are available and add them into the client list */
    + err = bus_for_each_dev(&platform_bus_type, NULL,
    + tegradrm, tegra_drm_add_client);
    + if (err < 0) {
    + pr_err("failed to add clients: %d\n", err);
    + return err;
    + }
    +
    + err = drm_platform_init(&tegra_drm_driver, pdev);
    if (err < 0) {
    - pr_err("failed to parse DT: %d\n", err);
    + dev_err(&pdev->dev, "drm_platform_init(): %d\n", err);
    return err;
    }

    @@ -243,6 +184,7 @@ static int tegra_drm_probe(struct platform_device *pdev)

    static int tegra_drm_remove(struct platform_device *pdev)
    {
    + drm_platform_exit(&tegra_drm_driver, pdev);
    return 0;
    }

    @@ -260,18 +202,9 @@ static int __init tegra_drm_init(void)
    int err;
    struct platform_device *drm_device;

    - drm_device = platform_device_register_simple("tegradrm", -1, NULL, 0);
    - if (!drm_device)
    - return -ENOMEM;
    - dma_set_coherent_mask(&drm_device->dev, DMA_BIT_MASK(32));
    -
    - err = platform_driver_register(&tegra_drm_platform_driver);
    - if (err < 0)
    - goto unregister_tegra_dev;
    -
    err = platform_driver_register(&tegra_dc_driver);
    if (err < 0)
    - goto unregister_tegra_drv;
    + return err;

    err = platform_driver_register(&tegra_hdmi_driver);
    if (err < 0)
    @@ -280,26 +213,36 @@ static int __init tegra_drm_init(void)
    err = platform_driver_register(&tegra_gr2d_driver);
    if (err < 0)
    goto unregister_hdmi;
    +
    + drm_device = platform_device_register_simple("tegradrm", -1, NULL, 0);
    + if (!drm_device)
    + goto unregister_gr2d;
    + dma_set_coherent_mask(&drm_device->dev, DMA_BIT_MASK(32));
    +
    + err = platform_driver_register(&tegra_drm_platform_driver);
    + if (err < 0)
    + goto unregister_tegra_dev;
    +
    return 0;

    +unregister_tegra_dev:
    + platform_device_unregister(drm_device);
    +unregister_gr2d:
    + platform_driver_unregister(&tegra_gr2d_driver);
    unregister_hdmi:
    platform_driver_unregister(&tegra_hdmi_driver);
    unregister_dc:
    platform_driver_unregister(&tegra_dc_driver);
    -unregister_tegra_drv:
    - platform_driver_unregister(&tegra_drm_platform_driver);
    -unregister_tegra_dev:
    - platform_device_unregister(drm_device);
    return err;
    }
    module_init(tegra_drm_init);

    static void __exit tegra_drm_exit(void)
    {
    + platform_driver_unregister(&tegra_drm_platform_driver);
    platform_driver_unregister(&tegra_gr2d_driver);
    platform_driver_unregister(&tegra_hdmi_driver);
    platform_driver_unregister(&tegra_dc_driver);
    - platform_driver_unregister(&tegra_drm_platform_driver);
    }
    module_exit(tegra_drm_exit);

    diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
    index 06100d7..34ec9d2 100644
    --- a/drivers/gpu/drm/tegra/drm.h
    +++ b/drivers/gpu/drm/tegra/drm.h
    @@ -75,7 +75,6 @@ tegra_drm_fpriv(struct drm_file *file_priv)
    }

    struct tegra_drm_client {
    - struct tegradrm *tegradrm;
    struct device *dev;

    const struct tegra_drm_client_ops *ops;
    diff --git a/drivers/gpu/drm/tegra/gr2d.c b/drivers/gpu/drm/tegra/gr2d.c
    index 8865099..f39fef2 100644
    --- a/drivers/gpu/drm/tegra/gr2d.c
    +++ b/drivers/gpu/drm/tegra/gr2d.c
    @@ -247,7 +247,6 @@ static int __devinit gr2d_probe(struct platform_device *dev)
    {
    int err;
    struct gr2d *gr2d = NULL;
    - struct tegradrm *tegradrm = platform_get_drvdata(dev);

    gr2d = devm_kzalloc(&dev->dev, sizeof(*gr2d), GFP_KERNEL);
    if (!gr2d)
    @@ -278,11 +277,6 @@ static int __devinit gr2d_probe(struct platform_device *dev)
    gr2d->client.ops = &gr2d_client_ops;
    gr2d->client.dev = &dev->dev;
    gr2d->client.class = NV_GRAPHICS_2D_CLASS_ID;
    - gr2d->client.tegradrm = tegradrm;
    -
    - err = tegra_drm_register_client(tegradrm, &gr2d->client);
    - if (err)
    - return err;

    platform_set_drvdata(dev, gr2d);
    return 0;
    @@ -292,7 +286,6 @@ static int __exit gr2d_remove(struct platform_device *dev)
    {
    struct gr2d *gr2d = platform_get_drvdata(dev);
    host1x_syncpt_free(gr2d->syncpt);
    - tegra_drm_unregister_client(gr2d->client.tegradrm, &gr2d->client);
    return 0;
    }

    diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
    index fce3e66..d987510 100644
    --- a/drivers/gpu/drm/tegra/hdmi.c
    +++ b/drivers/gpu/drm/tegra/hdmi.c
    @@ -1214,7 +1214,6 @@ static int tegra_hdmi_probe(struct platform_device *pdev)
    struct tegra_hdmi *hdmi;
    struct resource *regs;
    int err;
    - struct tegradrm *tegradrm = platform_get_drvdata(pdev);

    hdmi = devm_kzalloc(&pdev->dev, sizeof(*hdmi), GFP_KERNEL);
    if (!hdmi)
    @@ -1285,14 +1284,6 @@ static int tegra_hdmi_probe(struct platform_device *pdev)
    hdmi->client.ops = &hdmi_client_ops;
    INIT_LIST_HEAD(&hdmi->client.list);
    hdmi->client.dev = &pdev->dev;
    - hdmi->client.tegradrm = tegradrm;
    -
    - err = tegra_drm_register_client(tegradrm, &hdmi->client);
    - if (err < 0) {
    - dev_err(&pdev->dev, "failed to register tegra drm client: %d\n",
    - err);
    - return err;
    - }

    platform_set_drvdata(pdev, hdmi);

    @@ -1302,15 +1293,6 @@ static int tegra_hdmi_probe(struct platform_device *pdev)
    static int tegra_hdmi_remove(struct platform_device *pdev)
    {
    struct tegra_hdmi *hdmi = platform_get_drvdata(pdev);
    - int err;
    -
    - err = tegra_drm_unregister_client(hdmi->client.tegradrm,
    - &hdmi->client);
    - if (err < 0) {
    - dev_err(&pdev->dev, "failed to unregister tegra drm client: %d\n",
    - err);
    - return err;
    - }

    clk_unprepare(hdmi->clk_parent);
    clk_unprepare(hdmi->clk);
    --
    1.7.9.5
    \
     
     \ /
      Last update: 2012-12-31 08:21    [W:4.402 / U:0.412 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site