lkml.org 
[lkml]   [2011]   [Aug]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC][PATCH v3] DRM: add DRM Driver for Samsung SoC EXYNOS4210.
    > > +       entry->vaddr = dma_alloc_writecombine(dev->dev, entry->size,
    > > +                       (dma_addr_t *)&entry->paddr, GFP_KERNEL);
    > > +       if (!entry->paddr) {
    > > +               DRM_ERROR("failed to allocate buffer.\n");
    > > +               return -ENOMEM;
    > > +       }
    > > +
    > > +       DRM_DEBUG_KMS("allocated : vaddr(0x%x), paddr(0x%x), size(0x%x)\n",
    > > +                       (unsigned int)entry->vaddr, entry->paddr, entry->size);
    > > +
    > > +       return 0;
    > > +}
    > > +
    > [snip]
    >
    > > diff --git a/drivers/gpu/drm/samsung/samsung_drm_buf.h b/drivers/gpu/drm/samsung/samsung_drm_buf.h
    > > new file mode 100644
    > > index 0000000..d6a7e95
    > > --- /dev/null
    > > +++ b/drivers/gpu/drm/samsung/samsung_drm_buf.h
    > [snip]
    > > +/**
    > > + * samsung drm buffer entry structure.
    > > + *
    > > + * @paddr: physical address of allocated memory.
    > > + * @vaddr: kernel virtual address of allocated memory.
    > > + * @size: size of allocated memory.
    > > + */
    > > +struct samsung_drm_buf_entry {
    > > +       unsigned int paddr;

    This could be made 'dma_addr_t' and then you can drop all of the
    casts to (dma_addr_t *).

    .. snip..
    > > +static int samsung_drm_connector_get_modes(struct drm_connector *connector)

    Why not make the return be 'unsigned int'?

    .. snip..
    > > +/* get detection status of display device. */
    > > +static enum drm_connector_status
    > > +samsung_drm_connector_detect(struct drm_connector *connector, bool force)
    > > +{
    > > +       struct samsung_drm_connector *samsung_connector =
    > > +               to_samsung_connector(connector);
    > > +       struct samsung_drm_display *display =
    > > +               samsung_drm_get_manager(samsung_connector->encoder)->display;
    > > +       unsigned int ret = connector_status_unknown;

    Not 'enum drm_connector_status ret = connector_status_unknown' ?

    > > +
    > > +       DRM_DEBUG_KMS("%s\n", __FILE__);
    > > +
    > > +       if (display && display->is_connected) {
    > > +               if (display->is_connected())
    > > +                       ret = connector_status_connected;
    > > +               else
    > > +                       ret = connector_status_disconnected;
    > > +       }
    > > +
    > > +       return ret;
    > > +}

    .. snip..
    > > +static void samsung_drm_fb_destroy(struct drm_framebuffer *fb)
    > > +{
    > > +       struct samsung_drm_fb *samsung_fb = to_samsung_fb(fb);
    > > +       int ret;

    Get rid of 'ret'
    > > +
    > > +       DRM_DEBUG_KMS("%s\n", __FILE__);
    > > +
    > > +       drm_framebuffer_cleanup(fb);
    > > +
    > > +       if (samsung_fb->is_default) {
    > > +               ret = drm_gem_handle_delete(samsung_fb->file_priv,
    > > +                               samsung_fb->gem_handle);
    >
    > why not keep the gem buffer ptr, and do something like:
    >
    > drm_gem_object_unreference_unlocked(samsung_fb->bo)..
    >
    > this way, you get the right behavior if someone somewhere else took a
    > ref to the gem buffer object? And it avoids needing to keep the
    > file_priv ptr in the fb (which seems a bit strange)
    >
    >
    > > +               if (ret < 0)
    > > +                       DRM_ERROR("failed to delete drm_gem_handle.\n");

    And just do the check on the function return value here. You are not using
    the 'ret' for anything.
    > > +       }
    > > +
    > > +       kfree(samsung_fb);
    > > +}
    > > +
    > [snip]

    Hm, so I stopped here - just realized that I am missing some of the code
    and I should look at the original patch..

    --
    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: 2011-08-31 04:11    [W:0.049 / U:0.676 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site