lkml.org 
[lkml]   [2012]   [Nov]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC,v2,8/8] drm: tegra: Add gr2d device
    On 11/26/2012 09:19 PM, Terje Bergström <tbergstrom@nvidia.com> wrote:
    > Add client driver for 2D device.
    >
    > Signed-off-by: Arto Merilainen <amerilainen@nvidia.com>
    > Signed-off-by: Terje Bergstrom <tbergstrom@nvidia.com>
    >
    > ---
    >
    [...]
    > +
    > +static int
    > +tegra_drm_ioctl_open_channel(struct drm_device *drm, void *data,
    > + struct drm_file *file_priv)
    > +{
    > + struct tegra_drm_open_channel_args *args = data;
    > + struct tegra_drm_client *client;
    > + struct tegra_drm_context *context;
    > + struct tegra_drm_fpriv *fpriv = tegra_drm_fpriv(file_priv);
    > + int err = 0;
    > +
    > + dev_dbg(drm->dev, "> %s(fpriv=%p, class=%x)\n", __func__,
    > + fpriv, args->class);
    > +
    > + context = kzalloc(sizeof(*context), GFP_KERNEL);
    > + if (!context) {
    > + err = -ENOMEM;
    > + goto out;

    Change to "return -ENOMEM". Otherwise the NULL "context" will be kfree.

    > + }
    > +
    > + list_for_each_entry(client, &tegra_drm_subdrv_list, list) {
    > + if (client->class == args->class) {
    > + dev_dbg(drm->dev, "opening client %x\n", args->class);
    > + context->client = client;
    > + err = client->ops->open_channel(client, context);
    > + if (err)
    > + goto out;
    > +
    > + dev_dbg(drm->dev, "context %p\n", context);
    > + list_add(&context->list, &fpriv->contexts);
    > + args->context = context;
    > + goto out;
    > + }
    > + }
    > + err = -ENODEV;
    > +
    > +out:
    > + if (err)
    > + kfree(context);
    > +
    > + dev_dbg(drm->dev, "< %s() = %d\n", __func__, err);
    > + return err;
    > +}
    > +
    > +static int
    > +tegra_drm_ioctl_close_channel(struct drm_device *drm, void *data,
    > + struct drm_file *file_priv)
    > +{
    > + struct tegra_drm_open_channel_args *args = data;
    > + struct tegra_drm_context *context;
    > + struct tegra_drm_fpriv *fpriv = tegra_drm_fpriv(file_priv);
    > + int err = 0;
    > +
    > + dev_dbg(drm->dev, "> %s(fpriv=%p)\n", __func__, fpriv);
    > + list_for_each_entry(context, &fpriv->contexts, list) {

    Consider "list_for_each_entry_safe" cause you remove list members during
    the loop.

    > + if (context == args->context) {
    > + context->client->ops->close_channel(context);
    > + list_del(&context->list);
    > + kfree(context);
    > + goto out;
    > + }
    > + }
    > + err = -EINVAL;
    > +
    > +out:
    > + dev_dbg(drm->dev, "< %s() = %d\n", __func__, err);
    > + return err;
    > +}
    > +
    [...]
    > +
    > +static int gr2d_submit(struct tegra_drm_context *context,
    > + struct tegra_drm_submit_args *args)

    I'm still in the middle of code reading of job submit, so I'll get you
    back if I find something.

    [...]
    > +
    > +static struct of_device_id gr2d_match[] __devinitdata = {
    > + { .compatible = "nvidia,tegra20-gr2d", },
    > + { .compatible = "nvidia,tegra30-gr2d", },

    Just as swarren mentioned, you'd better place "nvidia,tegra30-gr2d" in
    the front of "nvidia,tegra20-gr2d"...

    [...]
    > +
    > +#define DRM_TEGRA_GEM_CREATE 0x00
    > +
    > +#define DRM_IOCTL_TEGRA_GEM_CREATE DRM_IOWR(DRM_COMMAND_BASE + \
    > + DRM_TEGRA_GEM_CREATE, struct tegra_gem_create)
    > +

    Just a very minor suggestion: could you put them at the end of this
    file? I mean, stay with other IOCTLs like SYNCPT_READ, SYNCPT_WAIT...

    [...]
    > +
    > +#define DRM_TEGRA_DRM_SYNCPT_READ 0x01
    > +#define DRM_TEGRA_DRM_SYNCPT_INCR 0x02
    > +#define DRM_TEGRA_DRM_SYNCPT_WAIT 0x03
    > +#define DRM_TEGRA_DRM_OPEN_CHANNEL 0x04
    > +#define DRM_TEGRA_DRM_CLOSE_CHANNEL 0x05
    > +#define DRM_TEGRA_DRM_GET_SYNCPOINTS 0x06
    > +#define DRM_TEGRA_DRM_GET_MODMUTEXES 0x07
    > +#define DRM_TEGRA_DRM_SUBMIT 0x08
    > +
    > +#define DRM_IOCTL_TEGRA_DRM_SYNCPT_READ DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_DRM_SYNCPT_READ, struct tegra_drm_syncpt_read_args)
    > +#define DRM_IOCTL_TEGRA_DRM_SYNCPT_INCR DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_DRM_SYNCPT_INCR, struct tegra_drm_syncpt_incr_args)
    > +#define DRM_IOCTL_TEGRA_DRM_SYNCPT_WAIT DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_DRM_SYNCPT_WAIT, struct tegra_drm_syncpt_wait_args)
    > +#define DRM_IOCTL_TEGRA_DRM_OPEN_CHANNEL DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_DRM_OPEN_CHANNEL, struct tegra_drm_open_channel_args)
    > +#define DRM_IOCTL_TEGRA_DRM_CLOSE_CHANNEL DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_DRM_CLOSE_CHANNEL, struct tegra_drm_open_channel_args)
    > +#define DRM_IOCTL_TEGRA_DRM_GET_SYNCPOINTS DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_DRM_GET_SYNCPOINTS, struct tegra_drm_get_channel_param_args)
    > +#define DRM_IOCTL_TEGRA_DRM_GET_MODMUTEXES DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_DRM_GET_MODMUTEXES, struct tegra_drm_get_channel_param_args)
    > +#define DRM_IOCTL_TEGRA_DRM_SUBMIT DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_DRM_SUBMIT, struct tegra_drm_submit_args)
    > +
    > +#endif
    >
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2012-11-29 09:21    [W:3.209 / U:0.036 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site