Re: [PATCH 3/4] drm/ttm, drm/vmwgfx: Use RCU locking for object lookups v3
On Tue, Nov 20, 2012 at 4:44 PM, Thomas Hellstrom wrote: > On 11/20/2012 07:19 AM, Dave Airlie wrote: >> >> On Tue, Nov 6, 2012 at 9:31 PM, Thomas Hellstrom >> wrote: >>> >>> The mostly used lookup+get put+potential_destroy path of TTM objects >>> is converted to use RCU locks. This will substantially decrease the >>> amount >>> of locked bus cycles during normal operation. >>> Since we use kfree_rcu to free the objects, no rcu synchronization is >>> needed >>> at module unload time. >> >> As this is the first use of RCU in a drm driver from what I can see, >> let me remind that the >> RCU patent agreement AFAIK only covers GPL works. >> >> So non-GPL or other OSes porting this code should take not of this. >> >> Dave. > > > From VMware's side this won't be a problem, since other VMware kernel > modules (VMCI IIRC) use RCU. > > In any case I have a new version of the "vmwgfx optimization" patch series > that mostly add documentation and > annotation (by using a drm_ht_xxx_rcu) interface for hashtab, after an > internal review by Dmitry Torkov. I see you've already > applied the original patch series. Do you want me to send out the new one or > rebase it against current drm-next? Can you rebase it on top of -next? Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 05/17] DRM/KMS/EDID: Fix up EEDID Map Blocks if Extension block count has changed.
At Mon, 19 Nov 2012 15:23:06 -0500, "Egbert Eich " <"e...@novell.com> wrote: > > EEDID v1.3 mandates map blocks if more than one EDID extension block > is used while in v1.4 they are optional. > If the extension count has been changed (because some extension > blocks were not readable) those map blocks need fixing. > In case of v1.4 or higher we simply eliminate all map blocks as > this is less time consuming. For v1.3 we scrap any exsisting map > blocks and recreate them from scratch. > > Signed-off-by: Egbert Eich > --- > drivers/gpu/drm/drm_edid.c | 105 +-- > 1 files changed, 100 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index ec6f3bb..d1b9d67 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -44,7 +44,11 @@ > #define EDID_DETAILED_TIMINGS 4 > > #define EDID_EXTENSION_FLAG_OFFSET ((size_t)&((struct edid *)0)->extensions) > -#define EDID_CHECKSUM_OFFSET ((size_t)&((struct edid *)0)->checksum) > +#define EDID_CHECKSUM_OFFSET ((size_t)&((struct edid *)0)->checksum) > +#define EDID_VERSION_MAJOR_OFFSET ((size_t)&((struct edid *)0)->version) > +#define EDID_VERSION_MINOR_OFFSET ((size_t)&((struct edid *)0)->revision) Better to use offsetof(). Takashi ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 08/17] DRM/KMS/EDID: Use Extension Block Fixup Code also for 'firmware' EDID.
At Mon, 19 Nov 2012 15:23:09 -0500,
"Egbert Eich " <"e...@novell.com> wrote:
>
> Signed-off-by: Egbert Eich
> ---
> drivers/gpu/drm/drm_edid.c | 77
> ---
> drivers/gpu/drm/drm_edid_load.c | 54 ++-
> include/drm/drm_edid.h |1 +
> 3 files changed, 77 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index d1b9d67..7bdae6e 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -408,6 +408,29 @@ fixup_blockmaps(u8 **blockp, int eblock_cnt)
> return eblock_cnt;
> }
>
> +static int
> +fixup_edid(u8 **blockp, int valid_extensions)
> +{
> + u8 *new = NULL;
> +
> + if (valid_extensions != (*blockp)[EDID_EXTENSION_FLAG_OFFSET]) {
> +
> + if (valid_extensions)
> + valid_extensions = fixup_blockmaps(blockp,
> valid_extensions);
> +
> + if (valid_extensions >= 0) {
> + (*blockp)[EDID_CHECKSUM_OFFSET] +=
> (*blockp)[EDID_EXTENSION_FLAG_OFFSET] - valid_extensions;
> + (*blockp)[EDID_EXTENSION_FLAG_OFFSET] =
> valid_extensions;
> + new = krealloc(*blockp, (valid_extensions + 1) *
> EDID_LENGTH, GFP_KERNEL);
> + }
> + if (!new)
> + kfree(*blockp);
> +
> + *blockp = new;
> + }
> + return (new ? valid_extensions : -ENOMEM);
So this function returns -ENOMEM if valid_extensions is equal with
block[EDID_EXTENSION_FLAG_OFFSET]...?
Takashi
> +}
> +
> static u8 *
> drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
> {
> @@ -474,19 +497,7 @@ drm_do_get_edid(struct drm_connector *connector, struct
> i2c_adapter *adapter)
> }
>
> no_more:
> - if (valid_extensions != block[EDID_EXTENSION_FLAG_OFFSET]) {
> - if (valid_extensions)
> - valid_extensions = fixup_blockmaps(&block,
> valid_extensions);
> - if (valid_extensions >= 0) {
> - block[EDID_CHECKSUM_OFFSET] +=
> block[EDID_EXTENSION_FLAG_OFFSET] - valid_extensions;
> - block[EDID_EXTENSION_FLAG_OFFSET] = valid_extensions;
> - new = krealloc(block, (valid_extensions + 1) *
> EDID_LENGTH, GFP_KERNEL);
> - if (!new)
> - goto out;
> - } else
> - goto out;
> - block = new;
> - }
> + fixup_edid(&block, valid_extensions);
>
> return block;
>
> @@ -503,6 +514,46 @@ out:
> }
>
> /**
> + * Validate an entire EDID blob.
> + * \param connector: drm_connector struct of the used connector.
> + * \param blockp: pointer to address of an raw EDID data block.
> + * \param len: size if block in bytes.
> + *
> + * validate block and return corrected block in \param block.
> + * \return: number of valid extensions or -errno if unsuccessful.
> + */
> +int
> +drm_validate_edid_blob(struct drm_connector *connector, u8 **blockp, int len)
> +{
> + int n_blocks = len / EDID_LENGTH;
> + int valid_extensions = 0, ret = 0;
> + bool print_bad_edid = !connector->bad_edid_counter || (drm_debug &
> DRM_UT_KMS);
> +
> + if (!blockp || !*blockp)
> + ret = -EINVAL;
> + else if (!n_blocks || !drm_edid_block_valid(*blockp, 0,
> print_bad_edid)) {
> + kfree(*blockp);
> + *blockp = NULL;
> + ret = -EINVAL;
> + }
> + if (!ret) {
> + n_blocks--;
> + if ((*blockp)[EDID_EXTENSION_FLAG_OFFSET] < n_blocks)
> + n_blocks = (*blockp)[EDID_EXTENSION_FLAG_OFFSET];
> +
> + while (n_blocks--) {
> + if (drm_edid_block_valid(*blockp + (valid_extensions +
> 1) * EDID_LENGTH,
> + valid_extensions + 1,
> print_bad_edid))
> + valid_extensions++;
> + }
> + ret = fixup_edid(blockp, valid_extensions);
> + }
> + if (ret < 0)
> + connector->bad_edid_counter++;
> + return ret;
> +}
> +
> +/**
> * Probe DDC presence.
> *
> * \param adapter : i2c device adaptor
> diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
> index 38d3943..6541c1f 100644
> --- a/drivers/gpu/drm/drm_edid_load.c
> +++ b/drivers/gpu/drm/drm_edid_load.c
> @@ -119,11 +119,10 @@ static u8 *edid_load(struct drm_connector *connector,
> char *name,
> {
> const struct firmware *fw;
> struct platform_device *pdev;
> - u8 *fwdata = NULL, *edid, *new_edid;
> + u8 *fwdata = NULL;
> + struct edid *edid;
> int fwsize, expected;
> int builtin = 0, err = 0;
> - int i, valid_extensions = 0;
> - bool print_bad_edid = !connector->bad_edid_counter || (drm_debug &
> DRM_UT_KMS);
>
> pdev = platform_device_register_simp
Re: [PATCH 07/17] DRM/KMS/EDID: Move drm_edid_load.o to drm.ko
At Mon, 19 Nov 2012 15:23:08 -0500, "Egbert Eich " <"e...@novell.com> wrote: > > EDIDs are an integral concept of connectors, connectors are a concept > of drm core also drm_edid.o is already part of this drm core. > Overridden, 'firmware-supplied' EDIDs should be treated exactly the > same as device supplied ones. > Therefore move drm_edid_load from the helper level to the drm core. > > Signed-off-by: Egbert Eich You need to fix Kconfig as well. And I think the dependency on CONFIG_FW_LOADER is missing there, too. It should have "select FW_LOADER". Takashi > --- > drivers/gpu/drm/Makefile |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > index 2ff5cef..76ca269 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -16,11 +16,11 @@ drm-y :=drm_auth.o drm_buffer.o drm_bufs.o > drm_cache.o \ > > drm-$(CONFIG_COMPAT) += drm_ioc32.o > drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o > +drm-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o > > drm-usb-y := drm_usb.o > > drm_kms_helper-y := drm_fb_helper.o drm_crtc_helper.o drm_dp_i2c_helper.o > -drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o > drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o > > obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o > -- > 1.7.7 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 10/17] DRM/KMS/EDID: Cache EDID blobs with extensions.
At Mon, 19 Nov 2012 15:23:11 -0500,
"Egbert Eich " <"e...@novell.com> wrote:
>
> According the the VESA specs there can be up to 254 EEDID extension blocks.
> Since we may read the EDID (including extensions) in 10 second intervals to
> probe for display hotplugging (at least in cases where no hardware hotplug
> detection exists) and I2C transfer is rather slow we may end up consuming
> a considerable amount on CPU time for just that.
> This patch caches the EDID block if it contains at least one extension.
> To determine if the blocks match we only tranfer the base block, on a match
> we use the cached data.
>
> Signed-off-by: Egbert Eich
> ---
> drivers/gpu/drm/drm_crtc.c |1 +
> drivers/gpu/drm/drm_edid.c | 43 ++-
> include/drm/drm_crtc.h |1 +
> include/drm/drm_edid.h |1 +
> 4 files changed, 45 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 3533609..e283355 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -598,6 +598,7 @@ void drm_connector_cleanup(struct drm_connector
> *connector)
> drm_mode_remove(connector, mode);
>
> mutex_lock(&dev->mode_config.mutex);
> + drm_cache_edid(connector, NULL);
> drm_mode_object_put(dev, &connector->base);
> list_del(&connector->head);
> dev->mode_config.num_connector--;
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 3e7df61..410a54d 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -431,6 +431,41 @@ fixup_edid(u8 **blockp, int valid_extensions)
> return (new ? valid_extensions : -ENOMEM);
> }
>
> +static bool
> +compare_get_edid_from_cache(struct drm_connector *connector, struct edid
> **edidp)
> +{
> + if (connector->edid_cache &&
> + connector->edid_cache->prod_code[0] == (*edidp)->prod_code[0] &&
> + connector->edid_cache->prod_code[1] == (*edidp)->prod_code[1] &&
> + connector->edid_cache->serial == (*edidp)->serial &&
> + connector->edid_cache->input == (*edidp)->input) {
> + int size = (connector->edid_cache->extensions + 1) *
> EDID_LENGTH;
> + struct edid *new = kmalloc(size, GFP_KERNEL);
> + if (!new)
> + return false;
> + DRM_DEBUG_KMS("Got EDID for %s from
> cache.\n",drm_get_connector_name(connector));
> + memcpy(new, connector->edid_cache, size);
> + kfree(*edidp);
You can use kmemdup().
> + *edidp = new;
> + return true;
> + }
> + return false;
> +}
> +
> +void
> +drm_cache_edid(struct drm_connector *connector, struct edid *edid)
> +{
> + struct edid *new = NULL;
> + kfree(connector->edid_cache);
> + if (edid) {
> + int size = (edid->extensions + 1) * EDID_LENGTH;
> + new = kmalloc(size, GFP_KERNEL);
> + if (new)
> + memcpy(new, edid, size);
Ditto.
Takashi
> + }
> + connector->edid_cache = new;
> +}
> +
> static u8 *
> drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
> {
> @@ -462,10 +497,14 @@ drm_do_get_edid(struct drm_connector *connector, struct
> i2c_adapter *adapter)
> if (i == 4)
> goto carp;
>
> - /* if there's no extensions, we're done */
> + /* if there are no extensions, we're done - don't bother caching */
> if (block[EDID_EXTENSION_FLAG_OFFSET] == 0)
> return block;
>
> + /* see if EDID is in the cache - no need to read all extension blocks */
> + if (compare_get_edid_from_cache(connector, (struct edid **)&block))
> + return block;
> +
> new = krealloc(block, (block[EDID_EXTENSION_FLAG_OFFSET] + 1) *
> EDID_LENGTH, GFP_KERNEL);
> if (!new)
> goto out;
> @@ -505,6 +544,7 @@ drm_do_get_edid(struct drm_connector *connector, struct
> i2c_adapter *adapter)
>
> no_more:
> fixup_edid(&block, valid_extensions);
> + drm_cache_edid(connector, (struct edid *)block);
>
> return block;
>
> @@ -513,6 +553,7 @@ carp:
> dev_warn(connector->dev->dev, "%s: EDID block %d invalid.\n",
>drm_get_connector_name(connector), j);
> }
> + drm_cache_edid(connector, NULL);
> connector->bad_edid_counter++;
>
> out:
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 49dd8c2..6a1054c 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -601,6 +601,7 @@ struct drm_connector {
> struct drm_encoder *encoder; /* currently active encoder */
>
> /* EDID bits */
> + struct edid *edid_cache;
> uint8_t eld[MAX_ELD_BYTES];
> bool dvi_dual;
> int max_tmds_clock; /* in MHz */
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 53c619c..c880510 100644
> --- a/incl
[Bug 56405] Distorted graphics on Radeon HD 6620G
https://bugs.freedesktop.org/show_bug.cgi?id=56405 --- Comment #21 from Michel Dänzer --- (In reply to comment #19) > I have 15 commits left to test. Which ones? > Anyway I'm not even convinced my test is a good test. Because since a while I > only get the r600_dri.so built. And I only exchange this object. FWIW, that's perfectly plausible as you're narrowing down the range of potential culprit commits. This object does contain the bulk of relevant code. -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 13/17] DRM/KMS/EDID: Allow for multiple Connectors when specifying 'firmware'-EDID Files.
At Mon, 19 Nov 2012 15:23:14 -0500,
"Egbert Eich " <"e...@novell.com> wrote:
>
> So far it was only possible to load an EDID for a single connector (unless
> no connector was specified at all in which case the same EDID file was used
> for all).
> This patch extends the EDID loader so that EDID files can be specified for
> more than one connector. A semicolon is used to separate the different
> connector
> sections.
> The option now looks like this:
>
> edid_firmware="[:][;:]..."
>
> Signed-off-by: Egbert Eich
> ---
> drivers/gpu/drm/drm_edid_load.c | 45 ++
> 1 files changed, 31 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
> index 1475c6f..a40a88505 100644
> --- a/drivers/gpu/drm/drm_edid_load.c
> +++ b/drivers/gpu/drm/drm_edid_load.c
> @@ -196,26 +196,43 @@ struct edid *
> drm_load_edid_firmware(struct drm_connector *connector)
> {
> char *connector_name = drm_get_connector_name(connector);
> - char *edidname = edid_firmware, *last, *colon;
> - struct edid *edid;
> + char *edidname, *last, *colon;
> + struct edid *edid = NULL;
> + char *next, *mem;
> + bool many = false;
>
> - if (*edidname == '\0')
> + if (*edid_firmware == '\0')
> return NULL;
>
> - colon = strchr(edidname, ':');
> - if (colon != NULL) {
> - if (strncmp(connector_name, edidname, colon - edidname))
> - return NULL;
> - edidname = colon + 1;
> - if (*edidname == '\0')
> - return NULL;
> + edidname = mem = kstrndup(edid_firmware, PATH_MAX, GFP_KERNEL);
> + do {
Missing NULL check of edidname.
Takashi
> + next = strchr(edidname, ';');
> + if (next)
> + *(next++) = '\0';
> + colon = strchr(edidname, ':');
> + if (colon == NULL) {
> + if (next || many)
> + edidname = NULL;
> + break;
> + } else if (!strncmp(connector_name, edidname, colon -
> edidname)) {
> + edidname = colon + 1;
> + break;
> + }
> + edidname = next;
> + many = true;
> + } while (edidname);
> +
> + if (edidname && *edidname != '\0') {
> + last = edidname + strlen(edidname) - 1;
> + if (*last == '\n')
> + *last = '\0';
> +
> + if (strlen(edidname) > 0)
> + edid = edid_load(connector, edidname, connector_name);
> }
>
> - last = edidname + strlen(edidname) - 1;
> - if (*last == '\n')
> - *last = '\0';
> + kfree(mem);
>
> - edid = edid_load(connector, edidname, connector_name);
> if (IS_ERR_OR_NULL(edid))
> return NULL;
>
> --
> 1.7.7
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/exynos: remove 'pages' and 'page_size' elements in exynos gem buffer
On Tue, Nov 20, 2012 at 12:49 PM, Kyungmin Park wrote:
> On 11/20/12, Prathyush K wrote:
> > On Mon, Nov 19, 2012 at 3:14 PM, Kyungmin Park
> > wrote:
> >
> >> Hi,
> >>
> >> On 11/15/12, Prathyush K wrote:
> >> > The 'pages' structure is not required since we can use the 'sgt'. Even
> >> for
> >> > CONTIG buffers, a SGT is created (which will have just one sgl). This
> >> > SGT
> >> > can be used during mmap instead of 'pages'. The 'page_size' element of
> >> the
> >> > structure is also not used anywhere and is removed.
> >> > This patch also fixes a memory leak where the 'pages' structure was
> >> > being
> >> > allocated during gem buffer allocation but not being freed during
> >> > deallocate.
> >> >
> >> > Signed-off-by: Prathyush K
> >> > ---
> >> > drivers/gpu/drm/exynos/exynos_drm_buf.c| 20 --
> >> > drivers/gpu/drm/exynos/exynos_drm_buf.h|4 +-
> >> > drivers/gpu/drm/exynos/exynos_drm_dmabuf.c |3 +-
> >> > drivers/gpu/drm/exynos/exynos_drm_gem.c| 39
> >> > +++
> >> > drivers/gpu/drm/exynos/exynos_drm_gem.h|4 ---
> >> > 5 files changed, 19 insertions(+), 51 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_buf.c
> >> > b/drivers/gpu/drm/exynos/exynos_drm_buf.c
> >> > index 48c5896..72bf97b 100644
> >> > --- a/drivers/gpu/drm/exynos/exynos_drm_buf.c
> >> > +++ b/drivers/gpu/drm/exynos/exynos_drm_buf.c
> >> > @@ -34,8 +34,6 @@ static int lowlevel_buffer_allocate(struct
> drm_device
> >> > *dev,
> >> > unsigned int flags, struct exynos_drm_gem_buf *buf)
> >> > {
> >> > int ret = 0;
> >> > - unsigned int npages, i = 0;
> >> > - struct scatterlist *sgl;
> >> > enum dma_attr attr = DMA_ATTR_FORCE_CONTIGUOUS;
> >> >
> >> > DRM_DEBUG_KMS("%s\n", __FILE__);
> >> > @@ -73,22 +71,6 @@ static int lowlevel_buffer_allocate(struct
> >> > drm_device
> >> > *dev,
> >> > goto err_free_sgt;
> >> > }
> >> >
> >> > - npages = buf->sgt->nents;
> >> > -
> >> > - buf->pages = kzalloc(sizeof(struct page) * npages, GFP_KERNEL);
> >> > - if (!buf->pages) {
> >> > - DRM_ERROR("failed to allocate pages.\n");
> >> > - ret = -ENOMEM;
> >> > - goto err_free_table;
> >> > - }
> >> > -
> >> > - sgl = buf->sgt->sgl;
> >> > - while (i < npages) {
> >> > - buf->pages[i] = sg_page(sgl);
> >> > - sgl = sg_next(sgl);
> >> > - i++;
> >> > - }
> >> > -
> >> > DRM_DEBUG_KMS("vaddr(0x%lx), dma_addr(0x%lx), size(0x%lx)\n",
> >> > (unsigned long)buf->kvaddr,
> >> > (unsigned long)buf->dma_addr,
> >> > @@ -96,8 +78,6 @@ static int lowlevel_buffer_allocate(struct
> drm_device
> >> > *dev,
> >> >
> >> > return ret;
> >> >
> >> > -err_free_table:
> >> > - sg_free_table(buf->sgt);
> >> > err_free_sgt:
> >> > kfree(buf->sgt);
> >> > buf->sgt = NULL;
> >> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_buf.h
> >> > b/drivers/gpu/drm/exynos/exynos_drm_buf.h
> >> > index 3388e4e..25cf162 100644
> >> > --- a/drivers/gpu/drm/exynos/exynos_drm_buf.h
> >> > +++ b/drivers/gpu/drm/exynos/exynos_drm_buf.h
> >> > @@ -34,12 +34,12 @@ struct exynos_drm_gem_buf
> >> > *exynos_drm_init_buf(struct
> >> > drm_device *dev,
> >> > void exynos_drm_fini_buf(struct drm_device *dev,
> >> > struct exynos_drm_gem_buf *buffer);
> >> >
> >> > -/* allocate physical memory region and setup sgt and pages. */
> >> > +/* allocate physical memory region and setup sgt. */
> >> > int exynos_drm_alloc_buf(struct drm_device *dev,
> >> > struct exynos_drm_gem_buf *buf,
> >> > unsigned int flags);
> >> >
> >> > -/* release physical memory region, sgt and pages. */
> >> > +/* release physical memory region, and sgt. */
> >> > void exynos_drm_free_buf(struct drm_device *dev,
> >> > unsigned int flags,
> >> > struct exynos_drm_gem_buf *buffer);
> >> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
> >> > b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
> >> > index b98da30..615a049 100644
> >> > --- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
> >> > +++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
> >> > @@ -93,8 +93,7 @@ static struct sg_table *
> >> > goto err_unlock;
> >> > }
> >> >
> >> > - DRM_DEBUG_PRIME("buffer size = 0x%lx page_size = 0x%lx\n",
> >> > - buf->size, buf->page_size);
> >> > + DRM_DEBUG_PRIME("buffer size = 0x%lx\n", buf->size);
> >> >
> >> > err_unlock:
> >> > mutex_unlock(&dev->struct_mutex);
> >> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> >> > b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> >> > index 50d73f1..40999ac 100644
> >> > --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> >> > +++ b/drivers/gpu/drm/exynos/exyn
[PATCH v2] drm/exynos: remove 'pages' and 'page_size' elements in exynos gem buffer
Changelog v2:
Removed redundant check for invalid sgl.
Added check for valid page_offset in the beginning of exynos_drm_gem_map_buf.
Changelog v1:
The 'pages' structure is not required since we can use the 'sgt'. Even for
CONTIG buffers, a SGT is created (which will have just one sgl). This SGT
can be used during mmap instead of 'pages'. The 'page_size' element of the
structure is also not used anywhere and is removed.
This patch also fixes a memory leak where the 'pages' structure was being
allocated during gem buffer allocation but not being freed during deallocate.
Signed-off-by: Prathyush K
---
drivers/gpu/drm/exynos/exynos_drm_buf.c| 20 -
drivers/gpu/drm/exynos/exynos_drm_buf.h|4 +-
drivers/gpu/drm/exynos/exynos_drm_dmabuf.c |3 +-
drivers/gpu/drm/exynos/exynos_drm_gem.c| 41 ++-
drivers/gpu/drm/exynos/exynos_drm_gem.h|4 ---
5 files changed, 18 insertions(+), 54 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_buf.c
b/drivers/gpu/drm/exynos/exynos_drm_buf.c
index 48c5896..72bf97b 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_buf.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_buf.c
@@ -34,8 +34,6 @@ static int lowlevel_buffer_allocate(struct drm_device *dev,
unsigned int flags, struct exynos_drm_gem_buf *buf)
{
int ret = 0;
- unsigned int npages, i = 0;
- struct scatterlist *sgl;
enum dma_attr attr = DMA_ATTR_FORCE_CONTIGUOUS;
DRM_DEBUG_KMS("%s\n", __FILE__);
@@ -73,22 +71,6 @@ static int lowlevel_buffer_allocate(struct drm_device *dev,
goto err_free_sgt;
}
- npages = buf->sgt->nents;
-
- buf->pages = kzalloc(sizeof(struct page) * npages, GFP_KERNEL);
- if (!buf->pages) {
- DRM_ERROR("failed to allocate pages.\n");
- ret = -ENOMEM;
- goto err_free_table;
- }
-
- sgl = buf->sgt->sgl;
- while (i < npages) {
- buf->pages[i] = sg_page(sgl);
- sgl = sg_next(sgl);
- i++;
- }
-
DRM_DEBUG_KMS("vaddr(0x%lx), dma_addr(0x%lx), size(0x%lx)\n",
(unsigned long)buf->kvaddr,
(unsigned long)buf->dma_addr,
@@ -96,8 +78,6 @@ static int lowlevel_buffer_allocate(struct drm_device *dev,
return ret;
-err_free_table:
- sg_free_table(buf->sgt);
err_free_sgt:
kfree(buf->sgt);
buf->sgt = NULL;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_buf.h
b/drivers/gpu/drm/exynos/exynos_drm_buf.h
index 3388e4e..25cf162 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_buf.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_buf.h
@@ -34,12 +34,12 @@ struct exynos_drm_gem_buf *exynos_drm_init_buf(struct
drm_device *dev,
void exynos_drm_fini_buf(struct drm_device *dev,
struct exynos_drm_gem_buf *buffer);
-/* allocate physical memory region and setup sgt and pages. */
+/* allocate physical memory region and setup sgt. */
int exynos_drm_alloc_buf(struct drm_device *dev,
struct exynos_drm_gem_buf *buf,
unsigned int flags);
-/* release physical memory region, sgt and pages. */
+/* release physical memory region, and sgt. */
void exynos_drm_free_buf(struct drm_device *dev,
unsigned int flags,
struct exynos_drm_gem_buf *buffer);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
index d9307bd..539da9f 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
@@ -87,8 +87,7 @@ static struct sg_table *
goto err_unlock;
}
- DRM_DEBUG_PRIME("buffer size = 0x%lx page_size = 0x%lx\n",
- buf->size, buf->page_size);
+ DRM_DEBUG_PRIME("buffer size = 0x%lx\n", buf->size);
err_unlock:
mutex_unlock(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c
b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index fdabb0f..7e2727a 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -99,34 +99,23 @@ static int exynos_drm_gem_map_buf(struct drm_gem_object
*obj,
unsigned long pfn;
int i;
- if (exynos_gem_obj->flags & EXYNOS_BO_NONCONTIG) {
- if (!buf->sgt)
- return -EINTR;
-
- sgl = buf->sgt->sgl;
- for_each_sg(buf->sgt->sgl, sgl, buf->sgt->nents, i) {
- if (!sgl) {
- DRM_ERROR("invalid SG table\n");
- return -EINTR;
- }
- if (page_offset < (sgl->length >> PAGE_SHIFT))
- break;
- page_offset -= (sgl->length >> PAGE_SHIFT);
- }
-
-
[PATCH] drm/exynos: fix memory leak: free EDID block
drm_get_edid() returns a pointer to an EDID block. The caller
is responsible to free this pointer itself.
Here the pointer gets assigned to the local variable raw_edid.
Therefore it should be freed before the variable goes out of
scope.
Signed-off-by: Egbert Eich
---
drivers/gpu/drm/exynos/exynos_hdmi.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 2c115f8..bc87bca 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -1293,6 +1293,7 @@ static int hdmi_get_edid(void *ctx, struct drm_connector
*connector,
DRM_DEBUG_KMS("%s : width[%d] x height[%d]\n",
(hdata->dvi_mode ? "dvi monitor" : "hdmi monitor"),
raw_edid->width_cm, raw_edid->height_cm);
+ kfree(raw_edid);
} else {
return -ENODEV;
}
--
1.7.7
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RESEND][PATCH] drm/prime: drop reference on imported dma-buf come from gem
Op 20-11-12 02:03, 김승우 schreef: > Hi Maarten, > > On 2012년 11월 19일 19:27, Maarten Lankhorst wrote: >> Op 15-11-12 04:52, Seung-Woo Kim schreef: >>> Increasing ref counts of both dma-buf and gem for imported dma-buf >>> come from gem makes memory leak. release function of dma-buf cannot >>> be called because f_count of dma-buf increased by importing gem and >>> gem ref count cannot be decrease because of exported dma-buf. >>> >>> So I add dma_buf_put() for imported gem come from its own gem into >>> each drivers having prime_import and prime_export capabilities. With >>> this, only gem ref count is increased if importing gem exported from >>> gem of same driver. >>> >>> Signed-off-by: Seung-Woo Kim >>> Signed-off-by: Kyungmin.park >>> Cc: Inki Dae >>> Cc: Daniel Vetter >>> Cc: Rob Clark >>> Cc: Alex Deucher >>> --- >>> Mmap failiure in i915 was reported by Jani, and I think it was fixed >>> with Dave's commit "drm/prime: add exported buffers to current fprivs >>> imported buffer list (v2)", so I resend this patch. >>> >>> I can send exynos only, but this issue is common in all drm driver >>> having both prime inport and export, IMHO, it's better to send for >>> all drivers. >> I fear that this won't solve the issue completely and keeps open a small >> race. > I don't believe my patch can fix all issue related with gem prime > completely. But it seems that it can solve unrecoverable memory leak > caused by re-importing GEM. Anyway, can you give me an example of other > race issue? When the dma_buf is unreffed and refcount drops to 0, work is queued to free it. Until then export_dma_buf member points to something, but refcount is 0 on it. Importing to itself, then closing fd then re-export from the new place would probably trigger it. >> I don't like the current destruction path either. It really looks like >> export_dma_buf >> should be unset when the exported buffer is closed in the original file. >> Anything else is racy. >> Especially setting export_dma_buf to NULL won't work with the current >> delayed fput semantics. > I cannot figure out all aspect of delayed fput, but until delayed work > is called, dma_buf is not released so export_dma_buf is valid. > Considering this, I can't find possible issue of delayed fput. I'm fairly sure that when refcount drops to 0, even though the memory may not be freed yet, you no longer have a guarantee that the memory is still valid. And of course after importing into a different process with its own drm namespace, how does file_priv->prime.lock still protect serialization? I don't see any hierarchy either. export_dma_buf needs to be set to NULL before the dma_buf_put is called that is used for the reference inside export_dma_buf. The release function should only release a reference to whatever backing memory is used. >> So to me it seems instead we should do something like this: >> >> - dmabuf_release callback is a noop, no ref is held by the dma-buf. >> - attach and detach ops increase reference by 1*. >> >> - when the buffer is exported, export_dma_buf is set by core code and kept >> around >> alive until the gem object is closed. >> >> - dma_buf_put is not called by import function. This reference removed as >> part >> of gem object close instead. > Considering re-import, where drm file priv is different from exporter's > file priv, attach and detach are not called because gem object is reused. > > How do you think remove checking whether dma_buf is came from driver own > exporter? Because without this checking, gem object is newly created, > and then re-import issue will disappear. The whole refcounting is a circular mess, sadly with no easy solution. :/ It seems to me that using gem reference counts is solving this problem at the wrong layer. A secondary type of reference count is needed for the underlying memory backing. For those who use ttm, I would recommend that the dma_buf increases refcount on ttm_bo by one, so that the gem object can be destroyed and release its reference on the dma_buf. WIthout a secondary refcount you end up in a position where you can't reliably and race free unref the gem object and dma_buf at the same time, since they're both independent interfaces with possibly different lifetimes. It would really help if there were hard rules on lifetime of the export_dma_buf member, until then we're just patching one broken thing with another. ~Maarten ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v11 0/6] of: add display helper
On Thu, Nov 15, 2012 at 02:15:06PM +0100, Steffen Trumtrar wrote: > Hi! > > Changes since v10: > - fix function name (drm_)display_mode_from_videomode > - add acked-by, reviewed-by, tested-by > > Regards, > Steffen > > > Steffen Trumtrar (6): > video: add display_timing and videomode > video: add of helper for videomode > fbmon: add videomode helpers > fbmon: add of_videomode helpers > drm_modes: add videomode helpers > drm_modes: add of_videomode helpers > > .../devicetree/bindings/video/display-timings.txt | 107 ++ > drivers/gpu/drm/drm_modes.c| 70 +++ > drivers/video/Kconfig | 19 ++ > drivers/video/Makefile |4 + > drivers/video/display_timing.c | 24 +++ > drivers/video/fbmon.c | 86 > drivers/video/of_display_timing.c | 212 > > drivers/video/of_videomode.c | 47 + > drivers/video/videomode.c | 45 + > include/drm/drmP.h | 12 ++ > include/linux/display_timing.h | 69 +++ > include/linux/fb.h | 12 ++ > include/linux/of_display_timings.h | 20 ++ > include/linux/of_videomode.h | 17 ++ > include/linux/videomode.h | 40 > 15 files changed, 784 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/video/display-timings.txt > create mode 100644 drivers/video/display_timing.c > create mode 100644 drivers/video/of_display_timing.c > create mode 100644 drivers/video/of_videomode.c > create mode 100644 drivers/video/videomode.c > create mode 100644 include/linux/display_timing.h > create mode 100644 include/linux/of_display_timings.h > create mode 100644 include/linux/of_videomode.h > create mode 100644 include/linux/videomode.h > > -- > > > Ping! Any comments or taker for v11? Errors are fixed, driver is tested and working, DT binding got two ACKs. So, the series is finished as far as I can tell. As I'm not sure if I reached the right maintainers with the current CC, I did another get_maintainers and added Florian Schandinat and David Airlie to the list. If I need to resend the series, please tell. Regards, Steffen -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v11 0/6] of: add display helper
Hi Steffen, On Tuesday 20 November 2012 11:39:18 Steffen Trumtrar wrote: > On Thu, Nov 15, 2012 at 02:15:06PM +0100, Steffen Trumtrar wrote: > > Hi! > > > > Changes since v10: > > - fix function name (drm_)display_mode_from_videomode > > - add acked-by, reviewed-by, tested-by > > > > Regards, > > Steffen > > > > Steffen Trumtrar (6): > > video: add display_timing and videomode > > video: add of helper for videomode > > fbmon: add videomode helpers > > fbmon: add of_videomode helpers > > drm_modes: add videomode helpers > > drm_modes: add of_videomode helpers > > > > .../devicetree/bindings/video/display-timings.txt | 107 ++ > > drivers/gpu/drm/drm_modes.c| 70 +++ > > drivers/video/Kconfig | 19 ++ > > drivers/video/Makefile |4 + > > drivers/video/display_timing.c | 24 +++ > > drivers/video/fbmon.c | 86 > > drivers/video/of_display_timing.c | 212 +++ > > drivers/video/of_videomode.c | 47 + > > drivers/video/videomode.c | 45 + > > include/drm/drmP.h | 12 ++ > > include/linux/display_timing.h | 69 +++ > > include/linux/fb.h | 12 ++ > > include/linux/of_display_timings.h | 20 ++ > > include/linux/of_videomode.h | 17 ++ > > include/linux/videomode.h | 40 > > 15 files changed, 784 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/video/display-timings.txt create mode > > 100644 drivers/video/display_timing.c > > create mode 100644 drivers/video/of_display_timing.c > > create mode 100644 drivers/video/of_videomode.c > > create mode 100644 drivers/video/videomode.c > > create mode 100644 include/linux/display_timing.h > > create mode 100644 include/linux/of_display_timings.h > > create mode 100644 include/linux/of_videomode.h > > create mode 100644 include/linux/videomode.h > > Ping! > > Any comments or taker for v11? Errors are fixed, driver is tested and > working, DT binding got two ACKs. So, the series is finished as far as I > can tell. Just one last comment, sorry for not bringing this up earlier. Could you constify pointer arguments to functions where applicable ? For instance int videomode_from_timing(struct display_timings *disp, struct videomode *vm, unsigned int index); The function shouldn't modify disp, so you can make it const. Same for most of the API functions. You can then add my Reviewed-by: Laurent Pinchart > As I'm not sure if I reached the right maintainers with the current CC, I > did another get_maintainers and added Florian Schandinat and David Airlie > to the list. If I need to resend the series, please tell. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v11 0/6] of: add display helper
On Tue, Nov 20, 2012 at 11:52:06AM +0100, Laurent Pinchart wrote: > Hi Steffen, > > On Tuesday 20 November 2012 11:39:18 Steffen Trumtrar wrote: > > On Thu, Nov 15, 2012 at 02:15:06PM +0100, Steffen Trumtrar wrote: > > > Hi! > > > > > > Changes since v10: > > > - fix function name (drm_)display_mode_from_videomode > > > - add acked-by, reviewed-by, tested-by > > > > > > Regards, > > > Steffen > > > > > > Steffen Trumtrar (6): > > > video: add display_timing and videomode > > > video: add of helper for videomode > > > fbmon: add videomode helpers > > > fbmon: add of_videomode helpers > > > drm_modes: add videomode helpers > > > drm_modes: add of_videomode helpers > > > > > > .../devicetree/bindings/video/display-timings.txt | 107 ++ > > > drivers/gpu/drm/drm_modes.c| 70 +++ > > > drivers/video/Kconfig | 19 ++ > > > drivers/video/Makefile |4 + > > > drivers/video/display_timing.c | 24 +++ > > > drivers/video/fbmon.c | 86 > > > drivers/video/of_display_timing.c | 212 +++ > > > drivers/video/of_videomode.c | 47 + > > > drivers/video/videomode.c | 45 + > > > include/drm/drmP.h | 12 ++ > > > include/linux/display_timing.h | 69 +++ > > > include/linux/fb.h | 12 ++ > > > include/linux/of_display_timings.h | 20 ++ > > > include/linux/of_videomode.h | 17 ++ > > > include/linux/videomode.h | 40 > > > 15 files changed, 784 insertions(+) > > > create mode 100644 > > > Documentation/devicetree/bindings/video/display-timings.txt create mode > > > 100644 drivers/video/display_timing.c > > > create mode 100644 drivers/video/of_display_timing.c > > > create mode 100644 drivers/video/of_videomode.c > > > create mode 100644 drivers/video/videomode.c > > > create mode 100644 include/linux/display_timing.h > > > create mode 100644 include/linux/of_display_timings.h > > > create mode 100644 include/linux/of_videomode.h > > > create mode 100644 include/linux/videomode.h > > > > Ping! > > > > Any comments or taker for v11? Errors are fixed, driver is tested and > > working, DT binding got two ACKs. So, the series is finished as far as I > > can tell. > > Just one last comment, sorry for not bringing this up earlier. Could you > constify pointer arguments to functions where applicable ? For instance > > int videomode_from_timing(struct display_timings *disp, struct videomode *vm, > unsigned int index); > > The function shouldn't modify disp, so you can make it const. Same for most > of > the API functions. > Okay. Will do. > You can then add my > > Reviewed-by: Laurent Pinchart > Thanks. Regards, Steffen -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/10] drm/ttm: change fence_lock to inner lock, v3
Op 20-11-12 08:48, Thomas Hellstrom schreef: > On 11/19/2012 04:33 PM, Maarten Lankhorst wrote: >> Op 19-11-12 16:04, Thomas Hellstrom schreef: >>> On 11/19/2012 03:17 PM, Thomas Hellstrom wrote: Hi, This patch looks mostly good, although I think ttm_bo_cleanup_refs becomes overly complicated: Could this do, or am I missing something? >>> Actually, my version is bad, because ttm_bo_wait() is called with the lru >>> lock held. >>> >>> /Thomas >> Oh digging through it made me remember why I had to release the reservation >> early and >> had to allow move_notify to be called without reservation. >> >> Fortunately move_notify has a NULL parameter, which is the only time that >> happens, >> so you can still check do BUG_ON(mem != NULL && !ttm_bo_reserved(bo)); in >> your >> move_notify handler. >> >> 05/10 removed the loop and assumed no new fence could be attached after the >> driver has >> declared the bo dead. >> >> However, at that point it may no longer hold a reservation to confirm this, >> that's why >> I moved the cleanup to be done in the release_list handler. It could still >> be done in >> ttm_bo_release, but we no longer have a reservation after we waited. Getting >> a reservation can fail if the bo is imported for example. >> >> While it would be true that in that case a new fence may be attached as >> well, that >> would be less harmful since that operation wouldn't involve this device, so >> the >> ttm bo can still be removed in that case. When that time comes I should >> probably >> fix up that WARN_ON(ret) in ttm_bo_cleanup_refs. :-) >> >> I did add a WARN_ON(!atomic_read(&bo->kref.refcount)); to >> ttm_bo_reserve and ttm_eu_reserve_buffers to be sure nothing is done on the >> device >> itself. If that is too paranoid, those WARN_ON's could be dropped. I prefer >> to leave them >> in for a kernel release or 2. But according to the rules that would be the >> only time you >> could attach a new fence and trigger the WARN_ON for now.. > > Hmm, I'd appreciate if you could group patches with functional changes that > depend on eachother togeteher, > and "this is done because ...", which makes it much easier to review, (and to > follow the commit history in case > something goes terribly wrong and we need to revert). > > Meanwhile I'll take a look at the final ttm_bo.c and see if I can spot any > culprits. > > In general, as long as a bo is on a LRU list, we must be able to attach > fences because of accelerated eviction. I thought it was deliberately designed in such a way that it was kept on the lru list, but since it's also on the ddestroy list it won't start accelerated eviction, since it branches into cleanup_refs early, and lru_lock still protects all the list entries. Of course any previous acceleration may still happen, but since we take a reservation first before waiting, we're already sure that any previous acceleration command has finished fencing, and no new one can start since it appears on the ddestroy list which would force it to perform the same wait. The wait is legal, and no new fences can be attached. I do agree all those patches probably needs a lot longer commit message to explain it though. :-) ~Maarten ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/2] drm: exynos: hdmi: sending AVI and AUI info frames
Hi All, Kindly review the following patch-set. regards, Rahul Sharma On Fri, Nov 9, 2012 at 9:51 PM, Rahul Sharma wrote: > This patch set adds provision for composing and sending AVI and AUI > infoframes by exynos drm hdmi driver. > > It also adds provision to get CEA Video ID Code through the display mode > which is required for making AVI infoframe. > > Based on exynos-drm-fixes branch of > git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git > > Rahul Sharma (1): > drm: exynos: compose and send avi and aui info frames > > Stephane Marchesin (1): > drm: get cea video id code for a given display mode > > drivers/gpu/drm/drm_edid.c | 20 +++ > drivers/gpu/drm/exynos/exynos_hdmi.c | 97 > +- > drivers/gpu/drm/exynos/exynos_hdmi.h | 23 > drivers/gpu/drm/exynos/regs-hdmi.h | 17 +- > include/drm/drm_crtc.h |1 + > 5 files changed, 154 insertions(+), 4 deletions(-) > ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/10] drm/ttm: change fence_lock to inner lock, v3
Op 20-11-12 12:33, Maarten Lankhorst schreef: > Op 20-11-12 08:48, Thomas Hellstrom schreef: >> On 11/19/2012 04:33 PM, Maarten Lankhorst wrote: >>> Op 19-11-12 16:04, Thomas Hellstrom schreef: On 11/19/2012 03:17 PM, Thomas Hellstrom wrote: > Hi, > > This patch looks mostly good, although I think ttm_bo_cleanup_refs > becomes overly complicated: > Could this do, or am I missing something? > Actually, my version is bad, because ttm_bo_wait() is called with the lru lock held. /Thomas >>> Oh digging through it made me remember why I had to release the reservation >>> early and >>> had to allow move_notify to be called without reservation. >>> >>> Fortunately move_notify has a NULL parameter, which is the only time that >>> happens, >>> so you can still check do BUG_ON(mem != NULL && !ttm_bo_reserved(bo)); in >>> your >>> move_notify handler. >>> >>> 05/10 removed the loop and assumed no new fence could be attached after the >>> driver has >>> declared the bo dead. >>> >>> However, at that point it may no longer hold a reservation to confirm this, >>> that's why >>> I moved the cleanup to be done in the release_list handler. It could still >>> be done in >>> ttm_bo_release, but we no longer have a reservation after we waited. Getting >>> a reservation can fail if the bo is imported for example. >>> >>> While it would be true that in that case a new fence may be attached as >>> well, that >>> would be less harmful since that operation wouldn't involve this device, so >>> the >>> ttm bo can still be removed in that case. When that time comes I should >>> probably >>> fix up that WARN_ON(ret) in ttm_bo_cleanup_refs. :-) >>> >>> I did add a WARN_ON(!atomic_read(&bo->kref.refcount)); to >>> ttm_bo_reserve and ttm_eu_reserve_buffers to be sure nothing is done on the >>> device >>> itself. If that is too paranoid, those WARN_ON's could be dropped. I prefer >>> to leave them >>> in for a kernel release or 2. But according to the rules that would be the >>> only time you >>> could attach a new fence and trigger the WARN_ON for now.. >> Hmm, I'd appreciate if you could group patches with functional changes that >> depend on eachother togeteher, >> and "this is done because ...", which makes it much easier to review, (and >> to follow the commit history in case >> something goes terribly wrong and we need to revert). >> >> Meanwhile I'll take a look at the final ttm_bo.c and see if I can spot any >> culprits. >> >> In general, as long as a bo is on a LRU list, we must be able to attach >> fences because of accelerated eviction. > I thought it was deliberately designed in such a way that it was kept on the > lru list, > but since it's also on the ddestroy list it won't start accelerated eviction, > since it branches into cleanup_refs early, and lru_lock still protects all > the list entries. > > Of course any previous acceleration may still happen, but since we take a > reservation first before waiting, > we're already sure that any previous acceleration command has finished > fencing, and no new one can > start since it appears on the ddestroy list which would force it to perform > the same wait. > > The wait is legal, and no new fences can be attached. > > I do agree all those patches probably needs a lot longer commit message to > explain it though. :-) > Or maybe an alternative patch.. We could move the checks. There are only 2 places that are allowed to hold reservations at that point right? ttm_bo_swapout and evict_mem_first. If cleanup_refs_or_queue fails because reservation fails, it must mean it's in one of those 2 places. If it succeeds, we can remove it from the lru list and swap list, and if wait fails move it to ddestroy list. unreserve in swapout doesn't add it back to any lists. No special handling needed there. unreserve in evict_mem_first does, but we could take the lock before unreserve, and only re-add it to the swap/lru list when it's not on ddestroy. That way we wouldn't need to call ttm_bo_cleanup_refs from multiple places, and the cleanup would only ever need to be done in the ttm_bo_delayed_delete without race. I thought it was a feature that it still appeared on the lru list after death, so evict_mem_first could wait on it, but if it's an annoyance it could be easily fixed like that. But even if it's a feature to be preserved, evict_mem_first and swapout could be modified to check the ddestroy list first for buffers to destroy. In that case those functions would explicitly prefer waiting for destruction of bo's before queueing new work to swapout or evict bo's. ~Maarten ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/10] drm/ttm: change fence_lock to inner lock, v3
On 11/20/2012 12:33 PM, Maarten Lankhorst wrote: Op 20-11-12 08:48, Thomas Hellstrom schreef: On 11/19/2012 04:33 PM, Maarten Lankhorst wrote: Op 19-11-12 16:04, Thomas Hellstrom schreef: On 11/19/2012 03:17 PM, Thomas Hellstrom wrote: Hi, This patch looks mostly good, although I think ttm_bo_cleanup_refs becomes overly complicated: Could this do, or am I missing something? Actually, my version is bad, because ttm_bo_wait() is called with the lru lock held. /Thomas Oh digging through it made me remember why I had to release the reservation early and had to allow move_notify to be called without reservation. Fortunately move_notify has a NULL parameter, which is the only time that happens, so you can still check do BUG_ON(mem != NULL && !ttm_bo_reserved(bo)); in your move_notify handler. 05/10 removed the loop and assumed no new fence could be attached after the driver has declared the bo dead. However, at that point it may no longer hold a reservation to confirm this, that's why I moved the cleanup to be done in the release_list handler. It could still be done in ttm_bo_release, but we no longer have a reservation after we waited. Getting a reservation can fail if the bo is imported for example. While it would be true that in that case a new fence may be attached as well, that would be less harmful since that operation wouldn't involve this device, so the ttm bo can still be removed in that case. When that time comes I should probably fix up that WARN_ON(ret) in ttm_bo_cleanup_refs. :-) I did add a WARN_ON(!atomic_read(&bo->kref.refcount)); to ttm_bo_reserve and ttm_eu_reserve_buffers to be sure nothing is done on the device itself. If that is too paranoid, those WARN_ON's could be dropped. I prefer to leave them in for a kernel release or 2. But according to the rules that would be the only time you could attach a new fence and trigger the WARN_ON for now.. Hmm, I'd appreciate if you could group patches with functional changes that depend on eachother togeteher, and "this is done because ...", which makes it much easier to review, (and to follow the commit history in case something goes terribly wrong and we need to revert). Meanwhile I'll take a look at the final ttm_bo.c and see if I can spot any culprits. In general, as long as a bo is on a LRU list, we must be able to attach fences because of accelerated eviction. I thought it was deliberately designed in such a way that it was kept on the lru list, but since it's also on the ddestroy list it won't start accelerated eviction, since it branches into cleanup_refs early, and lru_lock still protects all the list entries. I used bad wording. I meant that unbinding might be accelerated, but currently (quite inefficiently) do synchronized unbinding, assuming that only the CPU can do that. When we start to support unsynchronized moves, we need to be able to attach fences at least at the last move_notify(bo, NULL); /Thomas ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH -next 0/5] drm/ttm: Get rid of a number of atomic read-modify-write ops addons
Changes that didn't make it into the drm/ttm: Get rid of a number of atomic read-modify-write ops v3 patch series. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/5] drm: Add a hash-tab rcu-safe API
While hashtab should now be RCU-safe, Add a drm_ht_xxx_api for consumers
to use to make it obvious what locking mechanism is used.
Document the way the rcu-safe interface should be used.
Don't use rcu-safe list traversal in modify operations where we should use
a spinlock / mutex anyway.
Signed-off-by: Thomas Hellstrom
---
drivers/gpu/drm/drm_hashtab.c | 26 ++
include/drm/drm_hashtab.h | 14 ++
2 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_hashtab.c b/drivers/gpu/drm/drm_hashtab.c
index 5729e39..8025454 100644
--- a/drivers/gpu/drm/drm_hashtab.c
+++ b/drivers/gpu/drm/drm_hashtab.c
@@ -67,7 +67,7 @@ void drm_ht_verbose_list(struct drm_open_hash *ht, unsigned
long key)
hashed_key = hash_long(key, ht->order);
DRM_DEBUG("Key is 0x%08lx, Hashed key is 0x%08x\n", key, hashed_key);
h_list = &ht->table[hashed_key];
- hlist_for_each_entry_rcu(entry, list, h_list, head)
+ hlist_for_each_entry(entry, list, h_list, head)
DRM_DEBUG("count %d, key: 0x%08lx\n", count++, entry->key);
}
@@ -81,7 +81,7 @@ static struct hlist_node *drm_ht_find_key(struct
drm_open_hash *ht,
hashed_key = hash_long(key, ht->order);
h_list = &ht->table[hashed_key];
- hlist_for_each_entry_rcu(entry, list, h_list, head) {
+ hlist_for_each_entry(entry, list, h_list, head) {
if (entry->key == key)
return list;
if (entry->key > key)
@@ -90,6 +90,24 @@ static struct hlist_node *drm_ht_find_key(struct
drm_open_hash *ht,
return NULL;
}
+static struct hlist_node *drm_ht_find_key_rcu(struct drm_open_hash *ht,
+ unsigned long key)
+{
+ struct drm_hash_item *entry;
+ struct hlist_head *h_list;
+ struct hlist_node *list;
+ unsigned int hashed_key;
+
+ hashed_key = hash_long(key, ht->order);
+ h_list = &ht->table[hashed_key];
+ hlist_for_each_entry_rcu(entry, list, h_list, head) {
+ if (entry->key == key)
+ return list;
+ if (entry->key > key)
+ break;
+ }
+ return NULL;
+}
int drm_ht_insert_item(struct drm_open_hash *ht, struct drm_hash_item *item)
{
@@ -102,7 +120,7 @@ int drm_ht_insert_item(struct drm_open_hash *ht, struct
drm_hash_item *item)
hashed_key = hash_long(key, ht->order);
h_list = &ht->table[hashed_key];
parent = NULL;
- hlist_for_each_entry_rcu(entry, list, h_list, head) {
+ hlist_for_each_entry(entry, list, h_list, head) {
if (entry->key == key)
return -EINVAL;
if (entry->key > key)
@@ -152,7 +170,7 @@ int drm_ht_find_item(struct drm_open_hash *ht, unsigned
long key,
{
struct hlist_node *list;
- list = drm_ht_find_key(ht, key);
+ list = drm_ht_find_key_rcu(ht, key);
if (!list)
return -EINVAL;
diff --git a/include/drm/drm_hashtab.h b/include/drm/drm_hashtab.h
index 3650d5d..fce2ef3 100644
--- a/include/drm/drm_hashtab.h
+++ b/include/drm/drm_hashtab.h
@@ -61,5 +61,19 @@ extern int drm_ht_remove_key(struct drm_open_hash *ht,
unsigned long key);
extern int drm_ht_remove_item(struct drm_open_hash *ht, struct drm_hash_item
*item);
extern void drm_ht_remove(struct drm_open_hash *ht);
+/*
+ * RCU-safe interface
+ *
+ * The user of this API needs to make sure that two or more instances of the
+ * hash table manipulation functions are never run simultaneously.
+ * The lookup function drm_ht_find_item_rcu may, however, run simultaneously
+ * with any of the manipulation functions as long as it's called from within
+ * an RCU read-locked section.
+ */
+#define drm_ht_insert_item_rcu drm_ht_insert_item
+#define drm_ht_just_insert_please_rcu drm_ht_just_insert_please
+#define drm_ht_remove_key_rcu drm_ht_remove_key
+#define drm_ht_remove_item_rcu drm_ht_remove_item
+#define drm_ht_find_item_rcu drm_ht_find_item
#endif
--
1.7.4.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/5] kref: Add kref_get_unless_zero documentation
Document how kref_get_unless_zero should be used and how it helps
solve a typical kref / locking problem.
Signed-off-by: Thomas Hellstrom
---
Documentation/kref.txt | 88
1 files changed, 88 insertions(+), 0 deletions(-)
diff --git a/Documentation/kref.txt b/Documentation/kref.txt
index 48ba715..ddf85a5 100644
--- a/Documentation/kref.txt
+++ b/Documentation/kref.txt
@@ -213,3 +213,91 @@ presentation on krefs, which can be found at:
and:
http://www.kroah.com/linux/talks/ols_2004_kref_talk/
+
+The above example could also be optimized using kref_get_unless_zero() in
+the following way:
+
+static struct my_data *get_entry()
+{
+ struct my_data *entry = NULL;
+ mutex_lock(&mutex);
+ if (!list_empty(&q)) {
+ entry = container_of(q.next, struct my_data, link);
+ if (!kref_get_unless_zero(&entry->refcount))
+ entry = NULL;
+ }
+ mutex_unlock(&mutex);
+ return entry;
+}
+
+static void release_entry(struct kref *ref)
+{
+ struct my_data *entry = container_of(ref, struct my_data, refcount);
+
+ mutex_lock(&mutex);
+ list_del(&entry->link);
+ mutex_unlock(&mutex);
+ kfree(entry);
+}
+
+static void put_entry(struct my_data *entry)
+{
+ kref_put(&entry->refcount, release_entry);
+}
+
+Which is useful to remove the mutex lock around kref_put() in put_entry(), but
+it's important that kref_get_unless_zero is enclosed in the same critical
+section that finds the entry in the lookup table,
+otherwise kref_get_unless_zero may reference already freed memory.
+Note that it is illegal to use kref_get_unless_zero without checking its
+return value. If you are sure (by already having a valid pointer) that
+kref_get_unless_zero() will return true, then use kref_get() instead.
+
+The function kref_get_unless_zero also makes it possible to use rcu
+locking for lookups in the above example:
+
+struct my_data
+{
+ struct rcu_head rhead;
+ .
+ struct kref refcount;
+ .
+ .
+};
+
+static struct my_data *get_entry_rcu()
+{
+ struct my_data *entry = NULL;
+ rcu_read_lock();
+ if (!list_empty(&q)) {
+ entry = container_of(q.next, struct my_data, link);
+ if (!kref_get_unless_zero(&entry->refcount))
+ entry = NULL;
+ }
+ rcu_read_unlock();
+ return entry;
+}
+
+static void release_entry_rcu(struct kref *ref)
+{
+ struct my_data *entry = container_of(ref, struct my_data, refcount);
+
+ mutex_lock(&mutex);
+ list_del_rcu(&entry->link);
+ mutex_unlock(&mutex);
+ kfree_rcu(entry, rhead);
+}
+
+static void put_entry(struct my_data *entry)
+{
+ kref_put(&entry->refcount, release_entry_rcu);
+}
+
+But note that the struct kref member needs to remain in valid memory for a
+rcu grace period after release_entry_rcu was called. That can be accomplished
+by using kfree_rcu(entry, rhead) as done above, or by calling synchronize_rcu()
+before using kfree, but note that synchronize_rcu() may sleep for a
+substantial amount of time.
+
+
+Thomas Hellstrom
--
1.7.4.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/5] drm/vmwgfx: Free user-space fence objects correctly
They need to be freed after an rcu grace period. Signed-off-by: Thomas Hellstrom --- drivers/gpu/drm/vmwgfx/vmwgfx_fence.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c index bc187fa..c62d20e 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c @@ -537,7 +537,7 @@ static void vmw_user_fence_destroy(struct vmw_fence_obj *fence) container_of(fence, struct vmw_user_fence, fence); struct vmw_fence_manager *fman = fence->fman; - kfree(ufence); + ttm_base_object_kfree(ufence, base); /* * Free kernel space accounting. */ -- 1.7.4.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/5] drm/ttm: Use the hashtab _rcu interface for ttm_objects
Also move a kref_init() out of spinlocked region
Signed-off-by: Thomas Hellstrom
---
drivers/gpu/drm/ttm/ttm_object.c | 21 ++---
1 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_object.c b/drivers/gpu/drm/ttm/ttm_object.c
index 0e13d9f..58a5f32 100644
--- a/drivers/gpu/drm/ttm/ttm_object.c
+++ b/drivers/gpu/drm/ttm/ttm_object.c
@@ -157,11 +157,11 @@ int ttm_base_object_init(struct ttm_object_file *tfile,
base->refcount_release = refcount_release;
base->ref_obj_release = ref_obj_release;
base->object_type = object_type;
- spin_lock(&tdev->object_lock);
kref_init(&base->refcount);
- ret = drm_ht_just_insert_please(&tdev->object_hash,
- &base->hash,
- (unsigned long)base, 31, 0, 0);
+ spin_lock(&tdev->object_lock);
+ ret = drm_ht_just_insert_please_rcu(&tdev->object_hash,
+ &base->hash,
+ (unsigned long)base, 31, 0, 0);
spin_unlock(&tdev->object_lock);
if (unlikely(ret != 0))
goto out_err0;
@@ -175,7 +175,7 @@ int ttm_base_object_init(struct ttm_object_file *tfile,
return 0;
out_err1:
spin_lock(&tdev->object_lock);
- (void)drm_ht_remove_item(&tdev->object_hash, &base->hash);
+ (void)drm_ht_remove_item_rcu(&tdev->object_hash, &base->hash);
spin_unlock(&tdev->object_lock);
out_err0:
return ret;
@@ -189,8 +189,15 @@ static void ttm_release_base(struct kref *kref)
struct ttm_object_device *tdev = base->tfile->tdev;
spin_lock(&tdev->object_lock);
- (void)drm_ht_remove_item(&tdev->object_hash, &base->hash);
+ (void)drm_ht_remove_item_rcu(&tdev->object_hash, &base->hash);
spin_unlock(&tdev->object_lock);
+
+ /*
+* Note: We don't use synchronize_rcu() here because it's far
+* too slow. It's up to the user to free the object using
+* call_rcu() or ttm_base_object_kfree().
+*/
+
if (base->refcount_release) {
ttm_object_file_unref(&base->tfile);
base->refcount_release(&base);
@@ -216,7 +223,7 @@ struct ttm_base_object *ttm_base_object_lookup(struct
ttm_object_file *tfile,
int ret;
rcu_read_lock();
- ret = drm_ht_find_item(&tdev->object_hash, key, &hash);
+ ret = drm_ht_find_item_rcu(&tdev->object_hash, key, &hash);
if (likely(ret == 0)) {
base = drm_hash_entry(hash, struct ttm_base_object, hash);
--
1.7.4.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH -next 0/3] vmwgfx stuff for -next rebased
The last three patches from that patch series that didn't apply. It looks like they conflicted with the TTM removal of sync_obj_arg. /Thomas ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] drm/vmwgfx: Add and make use of a header for surface size calculation.
Signed-off-by: Thomas Hellstrom
Reviewed-by: Brian Paul
Reviewed-by: Dmitry Torokhov
---
drivers/gpu/drm/vmwgfx/svga3d_surfacedefs.h | 909 +++
drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 113 +---
2 files changed, 928 insertions(+), 94 deletions(-)
create mode 100644 drivers/gpu/drm/vmwgfx/svga3d_surfacedefs.h
diff --git a/drivers/gpu/drm/vmwgfx/svga3d_surfacedefs.h
b/drivers/gpu/drm/vmwgfx/svga3d_surfacedefs.h
new file mode 100644
index 000..8369c3b
--- /dev/null
+++ b/drivers/gpu/drm/vmwgfx/svga3d_surfacedefs.h
@@ -0,0 +1,909 @@
+/**
+ *
+ * Copyright © 2008-2012 VMware, Inc., Palo Alto, CA., USA
+ * All Rights Reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sub license, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
+ * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+ * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+ * USE OR OTHER DEALINGS IN THE SOFTWARE.
+ *
+ **/
+
+#ifdef __KERNEL__
+
+#include
+#define surf_size_struct struct drm_vmw_size
+
+#else /* __KERNEL__ */
+
+#ifndef ARRAY_SIZE
+#define ARRAY_SIZE(_A) (sizeof(_A) / sizeof((_A)[0]))
+#endif /* ARRAY_SIZE */
+
+#define DIV_ROUND_UP(x, y) (((x) + (y) - 1) / (y))
+#define max_t(type, x, y) ((x) > (y) ? (x) : (y))
+#define surf_size_struct SVGA3dSize
+#define u32 uint32
+
+#endif /* __KERNEL__ */
+
+#include "svga3d_reg.h"
+
+/*
+ * enum svga3d_block_desc describes the active data channels in a block.
+ *
+ * There can be at-most four active channels in a block:
+ *1. Red, bump W, luminance and depth are stored in the first channel.
+ *2. Green, bump V and stencil are stored in the second channel.
+ *3. Blue and bump U are stored in the third channel.
+ *4. Alpha and bump Q are stored in the fourth channel.
+ *
+ * Block channels can be used to store compressed and buffer data:
+ *1. For compressed formats, only the data channel is used and its size
+ * is equal to that of a singular block in the compression scheme.
+ *2. For buffer formats, only the data channel is used and its size is
+ * exactly one byte in length.
+ *3. In each case the bit depth represent the size of a singular block.
+ *
+ * Note: Compressed and IEEE formats do not use the bitMask structure.
+ */
+
+enum svga3d_block_desc {
+ SVGA3DBLOCKDESC_NONE= 0, /* No channels are active */
+ SVGA3DBLOCKDESC_BLUE= 1 << 0,/* Block with red channel
+ data */
+ SVGA3DBLOCKDESC_U = 1 << 0,/* Block with bump U channel
+ data */
+ SVGA3DBLOCKDESC_UV_VIDEO= 1 << 7,/* Block with alternating video
+ U and V */
+ SVGA3DBLOCKDESC_GREEN = 1 << 1,/* Block with green channel
+ data */
+ SVGA3DBLOCKDESC_V = 1 << 1,/* Block with bump V channel
+ data */
+ SVGA3DBLOCKDESC_STENCIL = 1 << 1,/* Block with a stencil
+ channel */
+ SVGA3DBLOCKDESC_RED = 1 << 2,/* Block with blue channel
+ data */
+ SVGA3DBLOCKDESC_W = 1 << 2,/* Block with bump W channel
+ data */
+ SVGA3DBLOCKDESC_LUMINANCE = 1 << 2,/* Block with luminance channel
+ data */
+ SVGA3DBLOCKDESC_Y = 1 << 2,/* Block with video luminance
+ data */
+ SVGA3DBLOCKDESC_DEPTH = 1 << 2,/* Block with depth channel */
+ SVGA3DBLOCKDESC_ALPHA = 1 << 3,/* Block with an alpha
+
[PATCH -next] drm/ttm: Optimize vm locking using kref_get_unless_zero
Removes the need for a write lock each time we call ttm_bo_unref().
Signed-off-by: Thomas Hellstrom
---
drivers/gpu/drm/ttm/ttm_bo.c|4 +---
drivers/gpu/drm/ttm/ttm_bo_vm.c |4 ++--
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 7426fe5..2dd0238 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -696,6 +696,7 @@ static void ttm_bo_release(struct kref *kref)
struct ttm_bo_device *bdev = bo->bdev;
struct ttm_mem_type_manager *man = &bdev->man[bo->mem.mem_type];
+ write_lock(&bdev->vm_lock);
if (likely(bo->vm_node != NULL)) {
rb_erase(&bo->vm_rb, &bdev->addr_space_rb);
drm_mm_put_block(bo->vm_node);
@@ -707,7 +708,6 @@ static void ttm_bo_release(struct kref *kref)
ttm_mem_io_unlock(man);
ttm_bo_cleanup_refs_or_queue(bo);
kref_put(&bo->list_kref, ttm_bo_release_list);
- write_lock(&bdev->vm_lock);
}
void ttm_bo_unref(struct ttm_buffer_object **p_bo)
@@ -716,9 +716,7 @@ void ttm_bo_unref(struct ttm_buffer_object **p_bo)
struct ttm_bo_device *bdev = bo->bdev;
*p_bo = NULL;
- write_lock(&bdev->vm_lock);
kref_put(&bo->kref, ttm_bo_release);
- write_unlock(&bdev->vm_lock);
}
EXPORT_SYMBOL(ttm_bo_unref);
diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 3ba72db..74705f3 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -259,8 +259,8 @@ int ttm_bo_mmap(struct file *filp, struct vm_area_struct
*vma,
read_lock(&bdev->vm_lock);
bo = ttm_bo_vm_lookup_rb(bdev, vma->vm_pgoff,
(vma->vm_end - vma->vm_start) >> PAGE_SHIFT);
- if (likely(bo != NULL))
- ttm_bo_reference(bo);
+ if (likely(bo != NULL) && !kref_get_unless_zero(&bo->kref))
+ bo = NULL;
read_unlock(&bdev->vm_lock);
if (unlikely(bo == NULL)) {
--
1.7.4.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/5] drm/ttm: Fix locking in an error path
Signed-off-by: Thomas Hellstrom --- drivers/gpu/drm/ttm/ttm_object.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_object.c b/drivers/gpu/drm/ttm/ttm_object.c index f18eeb4..0e13d9f 100644 --- a/drivers/gpu/drm/ttm/ttm_object.c +++ b/drivers/gpu/drm/ttm/ttm_object.c @@ -174,7 +174,9 @@ int ttm_base_object_init(struct ttm_object_file *tfile, return 0; out_err1: + spin_lock(&tdev->object_lock); (void)drm_ht_remove_item(&tdev->object_hash, &base->hash); + spin_unlock(&tdev->object_lock); out_err0: return ret; } -- 1.7.4.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm-next update
On 11/20/2012 07:39 AM, Dave Airlie wrote: Okay just pushed out a bunch of -next queued stuff, I've been stuck on another project for a couple of weeks and haven't really being paying enough attention to -next, so this is a heads up, if someone has something big they want merged this window and I haven't seen it yet or merged it yet, you should probably mention it in a reply to this mail with a summary of where you think its at. (e.g. atomic nuclear modesetting flipping). personal messages follow: Thomas: I merged some of the vmware patches I saw, I got to [6/8] drm/vmwgfx: Refactor resource management it didn't apply, it wasn't trivial, so you need to resend the 6/7/8 on top of this tree, or point me to what I missed that makes it all magically work! It appears they conflicted with the TTM sync_obj_arg removal patches. Please apply [PATCH -next 0/5] drm/ttm: Get rid of a number of atomic read-modify-write ops addons (This is the diff for the latest version of Get rid of a number of atomic read-modify-write ops) [PATCH -next 0/3] vmwgfx stuff for -next rebased (This is the patches 6/8 that didn't apply previously) [PATCH -next] drm/ttm: Optimize vm locking using kref_get_unless_zero (Small patch that gets rid of a write lock around buffer object unreferences) Thanks, Thomas ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/10] drm/ttm: change fence_lock to inner lock, v3
Op 20-11-12 13:03, Thomas Hellstrom schreef: > On 11/20/2012 12:33 PM, Maarten Lankhorst wrote: >> Op 20-11-12 08:48, Thomas Hellstrom schreef: >>> On 11/19/2012 04:33 PM, Maarten Lankhorst wrote: Op 19-11-12 16:04, Thomas Hellstrom schreef: > On 11/19/2012 03:17 PM, Thomas Hellstrom wrote: >> Hi, >> >> This patch looks mostly good, although I think ttm_bo_cleanup_refs >> becomes overly complicated: >> Could this do, or am I missing something? >> > Actually, my version is bad, because ttm_bo_wait() is called with the lru > lock held. > > /Thomas Oh digging through it made me remember why I had to release the reservation early and had to allow move_notify to be called without reservation. Fortunately move_notify has a NULL parameter, which is the only time that happens, so you can still check do BUG_ON(mem != NULL && !ttm_bo_reserved(bo)); in your move_notify handler. 05/10 removed the loop and assumed no new fence could be attached after the driver has declared the bo dead. However, at that point it may no longer hold a reservation to confirm this, that's why I moved the cleanup to be done in the release_list handler. It could still be done in ttm_bo_release, but we no longer have a reservation after we waited. Getting a reservation can fail if the bo is imported for example. While it would be true that in that case a new fence may be attached as well, that would be less harmful since that operation wouldn't involve this device, so the ttm bo can still be removed in that case. When that time comes I should probably fix up that WARN_ON(ret) in ttm_bo_cleanup_refs. :-) I did add a WARN_ON(!atomic_read(&bo->kref.refcount)); to ttm_bo_reserve and ttm_eu_reserve_buffers to be sure nothing is done on the device itself. If that is too paranoid, those WARN_ON's could be dropped. I prefer to leave them in for a kernel release or 2. But according to the rules that would be the only time you could attach a new fence and trigger the WARN_ON for now.. >>> Hmm, I'd appreciate if you could group patches with functional changes that >>> depend on eachother togeteher, >>> and "this is done because ...", which makes it much easier to review, (and >>> to follow the commit history in case >>> something goes terribly wrong and we need to revert). >>> >>> Meanwhile I'll take a look at the final ttm_bo.c and see if I can spot any >>> culprits. >>> >>> In general, as long as a bo is on a LRU list, we must be able to attach >>> fences because of accelerated eviction. >> I thought it was deliberately designed in such a way that it was kept on the >> lru list, >> but since it's also on the ddestroy list it won't start accelerated eviction, >> since it branches into cleanup_refs early, and lru_lock still protects all >> the list entries. > I used bad wording. I meant that unbinding might be accelerated, but > currently (quite inefficiently) > do synchronized unbinding, assuming that only the CPU can do that. When we > start to support > unsynchronized moves, we need to be able to attach fences at least at the > last move_notify(bo, NULL); Would you need to wait in that case on fence_wait being completed before calling move_notify? If not, you would still only need to perform one wait, but you'd have to make sure move_notify only gets called by 1 thread before checking the fence pointer and performing a wait. At that point you still hold the lru_lock though, so it shouldn't be too hard to make something safe. ~Maarten ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/2] drm: exynos: hdmi: sending AVI and AUI info frames
Hi, To get the proper review, please add proper maintainers. Thank you, Kyungmin Park On Tue, Nov 20, 2012 at 8:55 PM, Rahul Sharma wrote: > Hi All, > > Kindly review the following patch-set. > > regards, > Rahul Sharma > > On Fri, Nov 9, 2012 at 9:51 PM, Rahul Sharma wrote: >> This patch set adds provision for composing and sending AVI and AUI >> infoframes by exynos drm hdmi driver. >> >> It also adds provision to get CEA Video ID Code through the display mode >> which is required for making AVI infoframe. >> >> Based on exynos-drm-fixes branch of >> git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git >> >> Rahul Sharma (1): >> drm: exynos: compose and send avi and aui info frames >> >> Stephane Marchesin (1): >> drm: get cea video id code for a given display mode >> >> drivers/gpu/drm/drm_edid.c | 20 +++ >> drivers/gpu/drm/exynos/exynos_hdmi.c | 97 >> +- >> drivers/gpu/drm/exynos/exynos_hdmi.h | 23 >> drivers/gpu/drm/exynos/regs-hdmi.h | 17 +- >> include/drm/drm_crtc.h |1 + >> 5 files changed, 154 insertions(+), 4 deletions(-) >> > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm-next update
On Tue, Nov 20, 2012 at 04:39:39PM +1000, Dave Airlie wrote: > Okay just pushed out a bunch of -next queued stuff, > > I've been stuck on another project for a couple of weeks and haven't > really being paying enough attention to -next, so this is a heads up, > if someone has something big they want merged this window and I > haven't seen it yet or merged it yet, you should probably mention it > in a reply to this mail with a summary of where you think its at. > (e.g. atomic nuclear modesetting flipping). I don't the atomic stuff is quite ready for merging yet. However my code is more or less feature complete now. I have implemented everything I planned to originally, apart from adding more properties for various things. And as I mentioned before, Weston is now running quite nicely on top of my kernel branch. What's still missing is some refactoring, and possibly some fixes. And hardly anyone has commented on it, so please everyone have a look and let me know what you think. Maybe I need to start a blog to get people interested... Oh and there's now that 8byte get_user() issue that needs to be sorted out on x86-32. As far as the i915 specific stuff goes, I need to get someone to look at the GPU sync stuff. And then there's the bigger question of whether we could do the whole atomic page flip thing via the ring, but that's not something that we need to worry about today. -- Ville Syrjälä Intel OTC ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/exynos: modify burst size based on overlay size
The BURST size of fimd is adjusted based on the number of bytes to be
read. This is calculated based on the overlay width and the number of
bits per pixel. There are three burst lengths supported - 4 words,
8 words and 16 words where each word is 8 bytes long.
Signed-off-by: Prathyush K
---
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 34 ++
1 files changed, 25 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 19f12d1..655b2dd 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -384,6 +384,7 @@ static void fimd_win_set_pixfmt(struct device *dev,
unsigned int win)
struct fimd_context *ctx = get_fimd_context(dev);
struct fimd_win_data *win_data = &ctx->win_data[win];
unsigned long val;
+ unsigned long bytes;
DRM_DEBUG_KMS("%s\n", __FILE__);
@@ -393,48 +394,63 @@ static void fimd_win_set_pixfmt(struct device *dev,
unsigned int win)
case 1:
val |= WINCON0_BPPMODE_1BPP;
val |= WINCONx_BITSWP;
- val |= WINCONx_BURSTLEN_4WORD;
+ bytes = win_data->fb_width >> 3;
break;
case 2:
val |= WINCON0_BPPMODE_2BPP;
val |= WINCONx_BITSWP;
- val |= WINCONx_BURSTLEN_8WORD;
+ bytes = win_data->fb_width >> 2;
break;
case 4:
val |= WINCON0_BPPMODE_4BPP;
val |= WINCONx_BITSWP;
- val |= WINCONx_BURSTLEN_8WORD;
+ bytes = win_data->fb_width >> 1;
break;
case 8:
val |= WINCON0_BPPMODE_8BPP_PALETTE;
- val |= WINCONx_BURSTLEN_8WORD;
val |= WINCONx_BYTSWP;
+ bytes = win_data->fb_width;
break;
case 16:
val |= WINCON0_BPPMODE_16BPP_565;
val |= WINCONx_HAWSWP;
- val |= WINCONx_BURSTLEN_16WORD;
+ bytes = win_data->fb_width << 1;
break;
case 24:
val |= WINCON0_BPPMODE_24BPP_888;
val |= WINCONx_WSWP;
- val |= WINCONx_BURSTLEN_16WORD;
+ bytes = win_data->fb_width * 3;
break;
case 32:
val |= WINCON1_BPPMODE_28BPP_A4888
| WINCON1_BLD_PIX | WINCON1_ALPHA_SEL;
val |= WINCONx_WSWP;
- val |= WINCONx_BURSTLEN_16WORD;
+ bytes = win_data->fb_width << 2;
break;
default:
DRM_DEBUG_KMS("invalid pixel size so using unpacked 24bpp.\n");
-
+ bytes = win_data->fb_width * 3;
val |= WINCON0_BPPMODE_24BPP_888;
val |= WINCONx_WSWP;
- val |= WINCONx_BURSTLEN_16WORD;
break;
}
+ /*
+* Adjust the burst size based on the number of bytes to be read.
+* Each WORD of the BURST is 8 bytes long. There are 3 BURST sizes
+* supported by fimd.
+* WINCONx_BURSTLEN_4WORD = 32 bytes
+* WINCONx_BURSTLEN_8WORD = 64 bytes
+* WINCONx_BURSTLEN_16WORD = 128 bytes
+*/
+ if (bytes < 128)
+ if (bytes < 64)
+ val |= WINCONx_BURSTLEN_4WORD;
+ else
+ val |= WINCONx_BURSTLEN_8WORD;
+ else
+ val |= WINCONx_BURSTLEN_16WORD;
+
DRM_DEBUG_KMS("bpp = %d\n", win_data->bpp);
writel(val, ctx->regs + WINCON(win));
--
1.7.0.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/exynos: modify burst size based on overlay size
Hi,
On Tue, Nov 20, 2012 at 11:21 PM, Prathyush K wrote:
> The BURST size of fimd is adjusted based on the number of bytes to be
> read. This is calculated based on the overlay width and the number of
> bits per pixel. There are three burst lengths supported - 4 words,
> 8 words and 16 words where each word is 8 bytes long.
Interesting, BTW can you describe the problem? what's the issue and
why this patch is needed?
does it improve the performance or reduce the fimd bandwidth?
Thank you,
Kyungmin Park
>
> Signed-off-by: Prathyush K
> ---
> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 34
> ++
> 1 files changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 19f12d1..655b2dd 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -384,6 +384,7 @@ static void fimd_win_set_pixfmt(struct device *dev,
> unsigned int win)
> struct fimd_context *ctx = get_fimd_context(dev);
> struct fimd_win_data *win_data = &ctx->win_data[win];
> unsigned long val;
> + unsigned long bytes;
>
> DRM_DEBUG_KMS("%s\n", __FILE__);
>
> @@ -393,48 +394,63 @@ static void fimd_win_set_pixfmt(struct device *dev,
> unsigned int win)
> case 1:
> val |= WINCON0_BPPMODE_1BPP;
> val |= WINCONx_BITSWP;
> - val |= WINCONx_BURSTLEN_4WORD;
> + bytes = win_data->fb_width >> 3;
> break;
> case 2:
> val |= WINCON0_BPPMODE_2BPP;
> val |= WINCONx_BITSWP;
> - val |= WINCONx_BURSTLEN_8WORD;
> + bytes = win_data->fb_width >> 2;
> break;
> case 4:
> val |= WINCON0_BPPMODE_4BPP;
> val |= WINCONx_BITSWP;
> - val |= WINCONx_BURSTLEN_8WORD;
> + bytes = win_data->fb_width >> 1;
> break;
> case 8:
> val |= WINCON0_BPPMODE_8BPP_PALETTE;
> - val |= WINCONx_BURSTLEN_8WORD;
> val |= WINCONx_BYTSWP;
> + bytes = win_data->fb_width;
> break;
> case 16:
> val |= WINCON0_BPPMODE_16BPP_565;
> val |= WINCONx_HAWSWP;
> - val |= WINCONx_BURSTLEN_16WORD;
> + bytes = win_data->fb_width << 1;
> break;
> case 24:
> val |= WINCON0_BPPMODE_24BPP_888;
> val |= WINCONx_WSWP;
> - val |= WINCONx_BURSTLEN_16WORD;
> + bytes = win_data->fb_width * 3;
> break;
> case 32:
> val |= WINCON1_BPPMODE_28BPP_A4888
> | WINCON1_BLD_PIX | WINCON1_ALPHA_SEL;
> val |= WINCONx_WSWP;
> - val |= WINCONx_BURSTLEN_16WORD;
> + bytes = win_data->fb_width << 2;
> break;
> default:
> DRM_DEBUG_KMS("invalid pixel size so using unpacked
> 24bpp.\n");
> -
> + bytes = win_data->fb_width * 3;
> val |= WINCON0_BPPMODE_24BPP_888;
> val |= WINCONx_WSWP;
> - val |= WINCONx_BURSTLEN_16WORD;
> break;
> }
>
> + /*
> +* Adjust the burst size based on the number of bytes to be read.
> +* Each WORD of the BURST is 8 bytes long. There are 3 BURST sizes
> +* supported by fimd.
> +* WINCONx_BURSTLEN_4WORD = 32 bytes
> +* WINCONx_BURSTLEN_8WORD = 64 bytes
> +* WINCONx_BURSTLEN_16WORD = 128 bytes
> +*/
> + if (bytes < 128)
> + if (bytes < 64)
> + val |= WINCONx_BURSTLEN_4WORD;
> + else
> + val |= WINCONx_BURSTLEN_8WORD;
> + else
> + val |= WINCONx_BURSTLEN_16WORD;
> +
> DRM_DEBUG_KMS("bpp = %d\n", win_data->bpp);
>
> writel(val, ctx->regs + WINCON(win));
> --
> 1.7.0.4
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/10] drm/ttm: change fence_lock to inner lock, v3
On 11/20/2012 02:13 PM, Maarten Lankhorst wrote: Op 20-11-12 13:03, Thomas Hellstrom schreef: On 11/20/2012 12:33 PM, Maarten Lankhorst wrote: Op 20-11-12 08:48, Thomas Hellstrom schreef: On 11/19/2012 04:33 PM, Maarten Lankhorst wrote: Op 19-11-12 16:04, Thomas Hellstrom schreef: On 11/19/2012 03:17 PM, Thomas Hellstrom wrote: Hi, This patch looks mostly good, although I think ttm_bo_cleanup_refs becomes overly complicated: Could this do, or am I missing something? Actually, my version is bad, because ttm_bo_wait() is called with the lru lock held. /Thomas Oh digging through it made me remember why I had to release the reservation early and had to allow move_notify to be called without reservation. Fortunately move_notify has a NULL parameter, which is the only time that happens, so you can still check do BUG_ON(mem != NULL && !ttm_bo_reserved(bo)); in your move_notify handler. 05/10 removed the loop and assumed no new fence could be attached after the driver has declared the bo dead. However, at that point it may no longer hold a reservation to confirm this, that's why I moved the cleanup to be done in the release_list handler. It could still be done in ttm_bo_release, but we no longer have a reservation after we waited. Getting a reservation can fail if the bo is imported for example. While it would be true that in that case a new fence may be attached as well, that would be less harmful since that operation wouldn't involve this device, so the ttm bo can still be removed in that case. When that time comes I should probably fix up that WARN_ON(ret) in ttm_bo_cleanup_refs. :-) I did add a WARN_ON(!atomic_read(&bo->kref.refcount)); to ttm_bo_reserve and ttm_eu_reserve_buffers to be sure nothing is done on the device itself. If that is too paranoid, those WARN_ON's could be dropped. I prefer to leave them in for a kernel release or 2. But according to the rules that would be the only time you could attach a new fence and trigger the WARN_ON for now.. Hmm, I'd appreciate if you could group patches with functional changes that depend on eachother togeteher, and "this is done because ...", which makes it much easier to review, (and to follow the commit history in case something goes terribly wrong and we need to revert). Meanwhile I'll take a look at the final ttm_bo.c and see if I can spot any culprits. In general, as long as a bo is on a LRU list, we must be able to attach fences because of accelerated eviction. I thought it was deliberately designed in such a way that it was kept on the lru list, but since it's also on the ddestroy list it won't start accelerated eviction, since it branches into cleanup_refs early, and lru_lock still protects all the list entries. I used bad wording. I meant that unbinding might be accelerated, but currently (quite inefficiently) do synchronized unbinding, assuming that only the CPU can do that. When we start to support unsynchronized moves, we need to be able to attach fences at least at the last move_notify(bo, NULL); Would you need to wait in that case on fence_wait being completed before calling move_notify? If not, you would still only need to perform one wait, but you'd have to make sure move_notify only gets called by 1 thread before checking the fence pointer and performing a wait. At that point you still hold the lru_lock though, so it shouldn't be too hard to make something safe. I think typically a driver that wants to implement asynchronous moves don't want to wait before calling move_notify, but may wait in move_notify or move. Typically (upcoming vmwgfx) it would invalidate the buffer in move_notify(bo, NULL), attach a fence and then use the normal delayed destroy to wait on that fence before destroying the buffer. Otherwise, since binds / unbinds are handled in the GPU command stream there's never any need to wait for moves except when there's a CPU access. /Thomas ~Maarten ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 56139] [bisected] kernel 3.7.0-rc1 breaks 6950 (boot/grub2 and suspend/resume) (CAYMAN)
https://bugs.freedesktop.org/show_bug.cgi?id=56139 --- Comment #41 from Alex Deucher --- (In reply to comment #40) > In evergreen_reg.h, /* CRTC blocks at 0x6df0, 0x79f0, 0x105f0, 0x111f0, > 0x11df0, 0x129f0 */ is not used to define the registers following this > comment it. It seems to correspond to display controller offsets. > > Is the comment at the wrong place or are the registers not defined with the > right addresses? They are correct. The crtc blocks are repeated at different offsets within the register aperture. So if you wanted to access EVERGREEN_CRTC_CONTROL for crtc 0, you access: EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC0_REGISTER_OFFSET, for crtc 1: EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC1_REGISTER_OFFSET, etc. -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Fwd: [i915] linux-3.7-rc1 onwards, display goes black.
On Mon, Nov 19, 2012 at 06:32:44PM +0100, Daniel Vetter wrote: > On Fri, Nov 16, 2012 at 02:40:09PM -0300, Tomas M wrote: > > I sent this to the lkml but i got no reply. maybe im missing something > > obvious? my report lacks information? or this list is the apropiate > > place to send ;) > > I tend to not read lkml, too much stuff there ;-) Anyway, please boot with > drm.debug=0xe added to your kernel, and the grab the complete dmesg. > > For easier tracking it'd be great if you could file this as a bug on > bugs.freedesktop.org against DRI -> DRM/Intel Also, please try the patch at https://patchwork.kernel.org/patch/1728111/ it makes the vbt edp handling a bit more robust, which could be enough already to get the mbp retina going again. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v12 6/6] drm_modes: add of_videomode helpers
Add helper to get drm_display_mode from devicetree.
Signed-off-by: Steffen Trumtrar
Reviewed-by: Thierry Reding
Acked-by: Thierry Reding
Tested-by: Thierry Reding
Tested-by: Philipp Zabel
Reviewed-by: Laurent Pinchart
---
drivers/gpu/drm/drm_modes.c | 35 ++-
include/drm/drmP.h |6 ++
2 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 0073b27..04feef8 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -35,7 +35,8 @@
#include
#include
#include
-#include
+#include
+#include
/**
* drm_mode_debug_printmodeline - debug print a mode
@@ -541,6 +542,38 @@ int drm_display_mode_from_videomode(const struct videomode
*vm,
EXPORT_SYMBOL_GPL(drm_display_mode_from_videomode);
#endif
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+/**
+ * of_get_drm_display_mode - get a drm_display_mode from devicetree
+ * @np: device_node with the timing specification
+ * @dmode: will be set to the return value
+ * @index: index into the list of display timings in devicetree
+ *
+ * This function is expensive and should only be used, if only one mode is to
be
+ * read from DT. To get multiple modes start with of_get_display_timings and
+ * work with that instead.
+ */
+int of_get_drm_display_mode(const struct device_node *np,
+ struct drm_display_mode *dmode, unsigned int index)
+{
+ struct videomode vm;
+ int ret;
+
+ ret = of_get_videomode(np, &vm, index);
+ if (ret)
+ return ret;
+
+ drm_display_mode_from_videomode(&vm, dmode);
+
+ pr_info("%s: got %dx%d display mode from %s\n", __func__, vm.hactive,
+ vm.vactive, np->name);
+ drm_mode_debug_printmodeline(dmode);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_drm_display_mode);
+#endif
+
/**
* drm_mode_set_name - set the name on a mode
* @mode: name will be set in this mode
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index de2f6cf..377280f 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -56,6 +56,7 @@
#include
#include
#include
+#include
#include
#if defined(__alpha__) || defined(__powerpc__)
#include/* For pte_wrprotect */
@@ -1459,6 +1460,11 @@ drm_mode_create_from_cmdline_mode(struct drm_device *dev,
extern int drm_display_mode_from_videomode(const struct videomode *vm,
struct drm_display_mode *dmode);
#endif
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+extern int of_get_drm_display_mode(const struct device_node *np,
+ struct drm_display_mode *dmode,
+ unsigned int index);
+#endif
/* Modesetting support */
extern void drm_vblank_pre_modeset(struct drm_device *dev, int crtc);
--
1.7.10.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v12 3/6] fbmon: add videomode helpers
Add a function to convert from the generic videomode to a fb_videomode.
Signed-off-by: Steffen Trumtrar
Reviewed-by: Thierry Reding
Acked-by: Thierry Reding
Tested-by: Thierry Reding
Tested-by: Philipp Zabel
Reviewed-by: Laurent Pinchart
---
drivers/video/fbmon.c | 46 ++
include/linux/fb.h|6 ++
2 files changed, 52 insertions(+)
diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index cef6557..c1939a6 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -31,6 +31,7 @@
#include
#include
#include
+#include
#ifdef CONFIG_PPC_OF
#include
#include
@@ -1373,6 +1374,51 @@ int fb_get_mode(int flags, u32 val, struct
fb_var_screeninfo *var, struct fb_inf
kfree(timings);
return err;
}
+
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+int fb_videomode_from_videomode(const struct videomode *vm,
+ struct fb_videomode *fbmode)
+{
+ unsigned int htotal, vtotal;
+
+ fbmode->xres = vm->hactive;
+ fbmode->left_margin = vm->hback_porch;
+ fbmode->right_margin = vm->hfront_porch;
+ fbmode->hsync_len = vm->hsync_len;
+
+ fbmode->yres = vm->vactive;
+ fbmode->upper_margin = vm->vback_porch;
+ fbmode->lower_margin = vm->vfront_porch;
+ fbmode->vsync_len = vm->vsync_len;
+
+ fbmode->pixclock = KHZ2PICOS(vm->pixelclock / 1000);
+
+ fbmode->sync = 0;
+ fbmode->vmode = 0;
+ if (vm->hah)
+ fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
+ if (vm->vah)
+ fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
+ if (vm->interlaced)
+ fbmode->vmode |= FB_VMODE_INTERLACED;
+ if (vm->doublescan)
+ fbmode->vmode |= FB_VMODE_DOUBLE;
+ if (vm->de)
+ fbmode->sync |= FB_SYNC_DATA_ENABLE_HIGH_ACT;
+ fbmode->flag = 0;
+
+ htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
+vm->hsync_len;
+ vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
+vm->vsync_len;
+ fbmode->refresh = (vm->pixelclock * 1000) / (htotal * vtotal);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
+#endif
+
+
#else
int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
{
diff --git a/include/linux/fb.h b/include/linux/fb.h
index c7a9571..920cbe3 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -14,6 +14,7 @@
#include
#include
#include
+#include
struct vm_area_struct;
struct fb_info;
@@ -714,6 +715,11 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+extern int fb_videomode_from_videomode(const struct videomode *vm,
+ struct fb_videomode *fbmode);
+#endif
+
/* drivers/video/modedb.c */
#define VESA_MODEDB_SIZE 34
extern void fb_var_to_videomode(struct fb_videomode *mode,
--
1.7.10.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v12 2/6] video: add of helper for videomode
This adds support for reading display timings from DT or/and convert one of
those
timings to a videomode.
The of_display_timing implementation supports multiple children where each
property can have up to 3 values. All children are read into an array, that
can be queried.
of_get_videomode converts exactly one of that timings to a struct videomode.
Signed-off-by: Steffen Trumtrar
Signed-off-by: Philipp Zabel
Acked-by: Stephen Warren
Reviewed-by: Thierry Reding
Acked-by: Thierry Reding
Tested-by: Thierry Reding
Tested-by: Philipp Zabel
Reviewed-by: Laurent Pinchart
---
.../devicetree/bindings/video/display-timings.txt | 107 ++
drivers/video/Kconfig | 13 ++
drivers/video/Makefile |2 +
drivers/video/of_display_timing.c | 216
drivers/video/of_videomode.c | 48 +
include/linux/of_display_timings.h | 20 ++
include/linux/of_videomode.h | 18 ++
7 files changed, 424 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/display-timings.txt
create mode 100644 drivers/video/of_display_timing.c
create mode 100644 drivers/video/of_videomode.c
create mode 100644 include/linux/of_display_timings.h
create mode 100644 include/linux/of_videomode.h
diff --git a/Documentation/devicetree/bindings/video/display-timings.txt
b/Documentation/devicetree/bindings/video/display-timings.txt
new file mode 100644
index 000..a05cade
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/display-timings.txt
@@ -0,0 +1,107 @@
+display-timings bindings
+
+
+display-timings node
+
+
+required properties:
+ - none
+
+optional properties:
+ - native-mode: The native mode for the display, in case multiple modes are
+ provided. When omitted, assume the first node is the native.
+
+timings subnode
+---
+
+required properties:
+ - hactive, vactive: Display resolution
+ - hfront-porch, hback-porch, hsync-len: Horizontal Display timing parameters
+ in pixels
+ vfront-porch, vback-porch, vsync-len: Vertical display timing parameters in
+ lines
+ - clock-frequency: display clock in Hz
+
+optional properties:
+ - hsync-active: Hsync pulse is active low/high/ignored
+ - vsync-active: Vsync pulse is active low/high/ignored
+ - de-active: Data-Enable pulse is active low/high/ignored
+ - pixelclk-inverted: pixelclock is inverted/non-inverted/ignored
+ - interlaced (bool)
+ - doublescan (bool)
+
+All the optional properties that are not bool follow the following logic:
+<1>: high active
+<0>: low active
+omitted: not used on hardware
+
+There are different ways of describing the capabilities of a display. The
devicetree
+representation corresponds to the one commonly found in datasheets for
displays.
+If a display supports multiple signal timings, the native-mode can be
specified.
+
+The parameters are defined as
+
+struct display_timing
+=
+
+ +--+-+--+---+
+ | |↑| | |
+ | ||vback_porch | | |
+ | |↓| | |
+ +--###--+---+
+ | #↑# | |
+ | #|# | |
+ | hback #|# hfront | hsync |
+ | porch #| hactive # porch | len |
+ |<>#<---+--->#<>|<->|
+ | #|# | |
+ | #|vactive # | |
+ | #|# | |
+ | #↓# | |
+ +--###--+---+
+ | |↑| | |
+ | ||vfront_porch| | |
+ | |↓| | |
+ +--+-+--+---+
+ | |↑| | |
+ | ||vsync_len | | |
+ | |↓| | |
+ +--+-+--+---+
+
+
+Example:
+
+ display-timings {
+
[PATCH v12 5/6] drm_modes: add videomode helpers
Add conversion from videomode to drm_display_mode
Signed-off-by: Steffen Trumtrar
Reviewed-by: Thierry Reding
Acked-by: Thierry Reding
Tested-by: Thierry Reding
Tested-by: Philipp Zabel
Reviewed-by: Laurent Pinchart
---
drivers/gpu/drm/drm_modes.c | 37 +
include/drm/drmP.h |6 ++
2 files changed, 43 insertions(+)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 59450f3..0073b27 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -35,6 +35,7 @@
#include
#include
#include
+#include
/**
* drm_mode_debug_printmodeline - debug print a mode
@@ -504,6 +505,42 @@ drm_gtf_mode(struct drm_device *dev, int hdisplay, int
vdisplay, int vrefresh,
}
EXPORT_SYMBOL(drm_gtf_mode);
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+int drm_display_mode_from_videomode(const struct videomode *vm,
+ struct drm_display_mode *dmode)
+{
+ dmode->hdisplay = vm->hactive;
+ dmode->hsync_start = dmode->hdisplay + vm->hfront_porch;
+ dmode->hsync_end = dmode->hsync_start + vm->hsync_len;
+ dmode->htotal = dmode->hsync_end + vm->hback_porch;
+
+ dmode->vdisplay = vm->vactive;
+ dmode->vsync_start = dmode->vdisplay + vm->vfront_porch;
+ dmode->vsync_end = dmode->vsync_start + vm->vsync_len;
+ dmode->vtotal = dmode->vsync_end + vm->vback_porch;
+
+ dmode->clock = vm->pixelclock / 1000;
+
+ dmode->flags = 0;
+ if (vm->hah)
+ dmode->flags |= DRM_MODE_FLAG_PHSYNC;
+ else
+ dmode->flags |= DRM_MODE_FLAG_NHSYNC;
+ if (vm->vah)
+ dmode->flags |= DRM_MODE_FLAG_PVSYNC;
+ else
+ dmode->flags |= DRM_MODE_FLAG_NVSYNC;
+ if (vm->interlaced)
+ dmode->flags |= DRM_MODE_FLAG_INTERLACE;
+ if (vm->doublescan)
+ dmode->flags |= DRM_MODE_FLAG_DBLSCAN;
+ drm_mode_set_name(dmode);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(drm_display_mode_from_videomode);
+#endif
+
/**
* drm_mode_set_name - set the name on a mode
* @mode: name will be set in this mode
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 3fd8280..de2f6cf 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -56,6 +56,7 @@
#include
#include
#include
+#include
#if defined(__alpha__) || defined(__powerpc__)
#include/* For pte_wrprotect */
#endif
@@ -1454,6 +1455,11 @@ extern struct drm_display_mode *
drm_mode_create_from_cmdline_mode(struct drm_device *dev,
struct drm_cmdline_mode *cmd);
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+extern int drm_display_mode_from_videomode(const struct videomode *vm,
+ struct drm_display_mode *dmode);
+#endif
+
/* Modesetting support */
extern void drm_vblank_pre_modeset(struct drm_device *dev, int crtc);
extern void drm_vblank_post_modeset(struct drm_device *dev, int crtc);
--
1.7.10.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v12 1/6] video: add display_timing and videomode
Add display_timing structure and the according helper functions. This allows
the description of a display via its supported timing parameters.
Every timing parameter can be specified as a single value or a range
.
Also, add helper functions to convert from display timings to a generic
videomode
structure. This videomode can then be converted to the corresponding subsystem
mode representation (e.g. fb_videomode).
Signed-off-by: Steffen Trumtrar
Reviewed-by: Thierry Reding
Acked-by: Thierry Reding
Tested-by: Thierry Reding
Tested-by: Philipp Zabel
Reviewed-by: Laurent Pinchart
---
drivers/video/Kconfig |6
drivers/video/Makefile |2 ++
drivers/video/display_timing.c | 24 ++
drivers/video/videomode.c | 46 ++
include/linux/display_timing.h | 70
include/linux/videomode.h | 40 +++
6 files changed, 188 insertions(+)
create mode 100644 drivers/video/display_timing.c
create mode 100644 drivers/video/videomode.c
create mode 100644 include/linux/display_timing.h
create mode 100644 include/linux/videomode.h
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index d08d799..2a23b18 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -33,6 +33,12 @@ config VIDEO_OUTPUT_CONTROL
This framework adds support for low-level control of the video
output switch.
+config DISPLAY_TIMING
+ bool
+
+config VIDEOMODE
+ bool
+
menuconfig FB
tristate "Support for frame buffer devices"
---help---
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 23e948e..fc30439 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -167,3 +167,5 @@ obj-$(CONFIG_FB_VIRTUAL) += vfb.o
#video output switch sysfs driver
obj-$(CONFIG_VIDEO_OUTPUT_CONTROL) += output.o
+obj-$(CONFIG_DISPLAY_TIMING) += display_timing.o
+obj-$(CONFIG_VIDEOMODE) += videomode.o
diff --git a/drivers/video/display_timing.c b/drivers/video/display_timing.c
new file mode 100644
index 000..ac9bbbc
--- /dev/null
+++ b/drivers/video/display_timing.c
@@ -0,0 +1,24 @@
+/*
+ * generic display timing functions
+ *
+ * Copyright (c) 2012 Steffen Trumtrar , Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+
+#include
+#include
+#include
+
+void display_timings_release(struct display_timings *disp)
+{
+ if (disp->timings) {
+ unsigned int i;
+
+ for (i = 0; i < disp->num_timings; i++)
+ kfree(disp->timings[i]);
+ kfree(disp->timings);
+ }
+ kfree(disp);
+}
+EXPORT_SYMBOL_GPL(display_timings_release);
diff --git a/drivers/video/videomode.c b/drivers/video/videomode.c
new file mode 100644
index 000..e24f879
--- /dev/null
+++ b/drivers/video/videomode.c
@@ -0,0 +1,46 @@
+/*
+ * generic display timing functions
+ *
+ * Copyright (c) 2012 Steffen Trumtrar , Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+int videomode_from_timing(const struct display_timings *disp,
+ struct videomode *vm, unsigned int index)
+{
+ struct display_timing *dt;
+
+ dt = display_timings_get(disp, index);
+ if (!dt)
+ return -EINVAL;
+
+ vm->pixelclock = display_timing_get_value(&dt->pixelclock, 0);
+ vm->hactive = display_timing_get_value(&dt->hactive, 0);
+ vm->hfront_porch = display_timing_get_value(&dt->hfront_porch, 0);
+ vm->hback_porch = display_timing_get_value(&dt->hback_porch, 0);
+ vm->hsync_len = display_timing_get_value(&dt->hsync_len, 0);
+
+ vm->vactive = display_timing_get_value(&dt->vactive, 0);
+ vm->vfront_porch = display_timing_get_value(&dt->vfront_porch, 0);
+ vm->vback_porch = display_timing_get_value(&dt->vback_porch, 0);
+ vm->vsync_len = display_timing_get_value(&dt->vsync_len, 0);
+
+ vm->vah = dt->vsync_pol_active;
+ vm->hah = dt->hsync_pol_active;
+ vm->de = dt->de_pol_active;
+ vm->pixelclk_pol = dt->pixelclk_pol;
+
+ vm->interlaced = dt->interlaced;
+ vm->doublescan = dt->doublescan;
+
+ return 0;
+}
+
+EXPORT_SYMBOL_GPL(videomode_from_timing);
diff --git a/include/linux/display_timing.h b/include/linux/display_timing.h
new file mode 100644
index 000..d5bf03f
--- /dev/null
+++ b/include/linux/display_timing.h
@@ -0,0 +1,70 @@
+/*
+ * Copyright 2012 Steffen Trumtrar
+ *
+ * description of display timings
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_DISPLAY_TIMINGS_H
+#define __LINUX_DISPLAY_TIMINGS_H
+
+#include
+
+struct timing_entry {
+ u32 min;
+ u32 typ;
+ u32 max;
+};
+
+struct display_timing {
+ struct timing_entry pixelclock;
+
+ struct timing_entry hactive;
+ struct timing_entry hfront_porch;
+ struct timing_ent
[PATCH v12 4/6] fbmon: add of_videomode helpers
Add helper to get fb_videomode from devicetree.
Signed-off-by: Steffen Trumtrar
Reviewed-by: Thierry Reding
Acked-by: Thierry Reding
Tested-by: Thierry Reding
Tested-by: Philipp Zabel
Reviewed-by: Laurent Pinchart
---
drivers/video/fbmon.c | 42 +-
include/linux/fb.h|7 +++
2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index c1939a6..16c353c 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -31,7 +31,7 @@
#include
#include
#include
-#include
+#include
#ifdef CONFIG_PPC_OF
#include
#include
@@ -1418,6 +1418,46 @@ int fb_videomode_from_videomode(const struct videomode
*vm,
EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
#endif
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+static inline void dump_fb_videomode(const struct fb_videomode *m)
+{
+ pr_debug("fb_videomode = %ux%u@%uHz (%ukHz) %u %u %u %u %u %u %u %u
%u\n",
+m->xres, m->yres, m->refresh, m->pixclock, m->left_margin,
+m->right_margin, m->upper_margin, m->lower_margin,
+m->hsync_len, m->vsync_len, m->sync, m->vmode, m->flag);
+}
+
+/**
+ * of_get_fb_videomode - get a fb_videomode from devicetree
+ * @np: device_node with the timing specification
+ * @fb: will be set to the return value
+ * @index: index into the list of display timings in devicetree
+ *
+ * DESCRIPTION:
+ * This function is expensive and should only be used, if only one mode is to
be
+ * read from DT. To get multiple modes start with of_get_display_timings ond
+ * work with that instead.
+ */
+int of_get_fb_videomode(const struct device_node *np, struct fb_videomode *fb,
+ unsigned int index)
+{
+ struct videomode vm;
+ int ret;
+
+ ret = of_get_videomode(np, &vm, index);
+ if (ret)
+ return ret;
+
+ fb_videomode_from_videomode(&vm, fb);
+
+ pr_info("%s: got %dx%d display mode from %s\n", __func__, vm.hactive,
+ vm.vactive, np->name);
+ dump_fb_videomode(fb);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_fb_videomode);
+#endif
#else
int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 920cbe3..41b5e49 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -15,6 +15,8 @@
#include
#include
#include
+#include
+#include
struct vm_area_struct;
struct fb_info;
@@ -715,6 +717,11 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+extern int of_get_fb_videomode(const struct device_node *np,
+ struct fb_videomode *fb,
+ unsigned int index);
+#endif
#if IS_ENABLED(CONFIG_VIDEOMODE)
extern int fb_videomode_from_videomode(const struct videomode *vm,
struct fb_videomode *fbmode);
--
1.7.10.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v12 0/6] of: add display helper
Hi! Changes since v11: - make pointers const where applicable - add reviewed-by Laurent Pinchart Regards, Steffen Steffen Trumtrar (6): video: add display_timing and videomode video: add of helper for videomode fbmon: add videomode helpers fbmon: add of_videomode helpers drm_modes: add videomode helpers drm_modes: add of_videomode helpers .../devicetree/bindings/video/display-timings.txt | 107 ++ drivers/gpu/drm/drm_modes.c| 70 +++ drivers/video/Kconfig | 19 ++ drivers/video/Makefile |4 + drivers/video/display_timing.c | 24 +++ drivers/video/fbmon.c | 86 drivers/video/of_display_timing.c | 216 drivers/video/of_videomode.c | 48 + drivers/video/videomode.c | 46 + include/drm/drmP.h | 12 ++ include/linux/display_timing.h | 70 +++ include/linux/fb.h | 13 ++ include/linux/of_display_timings.h | 20 ++ include/linux/of_videomode.h | 18 ++ include/linux/videomode.h | 40 15 files changed, 793 insertions(+) create mode 100644 Documentation/devicetree/bindings/video/display-timings.txt create mode 100644 drivers/video/display_timing.c create mode 100644 drivers/video/of_display_timing.c create mode 100644 drivers/video/of_videomode.c create mode 100644 drivers/video/videomode.c create mode 100644 include/linux/display_timing.h create mode 100644 include/linux/of_display_timings.h create mode 100644 include/linux/of_videomode.h create mode 100644 include/linux/videomode.h -- 1.7.10.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v12 0/6] of: add display helper
On Tuesday 20 November 2012 16:54:50 Steffen Trumtrar wrote: > Hi! > > Changes since v11: > - make pointers const where applicable > - add reviewed-by Laurent Pinchart Looks good to me. Acked-by: Laurent Pinchart Through which tree do you plan to push this ? > Regards, > Steffen > > > Steffen Trumtrar (6): > video: add display_timing and videomode > video: add of helper for videomode > fbmon: add videomode helpers > fbmon: add of_videomode helpers > drm_modes: add videomode helpers > drm_modes: add of_videomode helpers > > .../devicetree/bindings/video/display-timings.txt | 107 ++ > drivers/gpu/drm/drm_modes.c| 70 +++ > drivers/video/Kconfig | 19 ++ > drivers/video/Makefile |4 + > drivers/video/display_timing.c | 24 +++ > drivers/video/fbmon.c | 86 > drivers/video/of_display_timing.c | 216 + > drivers/video/of_videomode.c | 48 + > drivers/video/videomode.c | 46 + > include/drm/drmP.h | 12 ++ > include/linux/display_timing.h | 70 +++ > include/linux/fb.h | 13 ++ > include/linux/of_display_timings.h | 20 ++ > include/linux/of_videomode.h | 18 ++ > include/linux/videomode.h | 40 > 15 files changed, 793 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/video/display-timings.txt create mode > 100644 drivers/video/display_timing.c > create mode 100644 drivers/video/of_display_timing.c > create mode 100644 drivers/video/of_videomode.c > create mode 100644 drivers/video/videomode.c > create mode 100644 include/linux/display_timing.h > create mode 100644 include/linux/of_display_timings.h > create mode 100644 include/linux/of_videomode.h > create mode 100644 include/linux/videomode.h -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] radeon: add AGPMode 1 quirk for RV250
On Mon, Nov 19, 2012 at 3:17 PM, Paul Bolle wrote:
> The Intel 82855PM host bridge / Mobility FireGL 9000 RV250 combination
> in an (outdated) ThinkPad T41 needs AGPMode 1 for suspend/resume (under
> KMS, that is). So add a quirk for it.
>
> (Change R250 to RV250 in comment for preceding quirk too.)
>
> Signed-off-by: Paul Bolle
Thanks, added to my -fixes queue.
Alex
> ---
> 0) Last tested on v3.6.7. The patch was cherry-picked on top of
> v3.7-rc6.
>
> 1) A similar patch was submitted twenty months ago via
> https://bugzilla.redhat.com/show_bug.cgi?id=531825 , but nothing
> happened after that.
>
> drivers/gpu/drm/radeon/radeon_agp.c |5 -
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_agp.c
> b/drivers/gpu/drm/radeon/radeon_agp.c
> index 10ea17a..4243334 100644
> --- a/drivers/gpu/drm/radeon/radeon_agp.c
> +++ b/drivers/gpu/drm/radeon/radeon_agp.c
> @@ -69,9 +69,12 @@ static struct radeon_agpmode_quirk
> radeon_agpmode_quirk_list[] = {
> /* Intel 82830 830 Chipset Host Bridge / Mobility M6 LY Needs AGPMode
> 2 (fdo #17360)*/
> { PCI_VENDOR_ID_INTEL, 0x3575, PCI_VENDOR_ID_ATI, 0x4c59,
> PCI_VENDOR_ID_DELL, 0x00e3, 2},
> - /* Intel 82852/82855 host bridge / Mobility FireGL 9000 R250 Needs
> AGPMode 1 (lp #296617) */
> + /* Intel 82852/82855 host bridge / Mobility FireGL 9000 RV250 Needs
> AGPMode 1 (lp #296617) */
> { PCI_VENDOR_ID_INTEL, 0x3580, PCI_VENDOR_ID_ATI, 0x4c66,
> PCI_VENDOR_ID_DELL, 0x0149, 1},
> + /* Intel 82855PM host bridge / Mobility FireGL 9000 RV250 Needs
> AGPMode 1 for suspend/resume */
> + { PCI_VENDOR_ID_INTEL, 0x3340, PCI_VENDOR_ID_ATI, 0x4c66,
> + PCI_VENDOR_ID_IBM, 0x0531, 1},
> /* Intel 82852/82855 host bridge / Mobility 9600 M10 RV350 Needs
> AGPMode 1 (deb #467460) */
> { PCI_VENDOR_ID_INTEL, 0x3580, PCI_VENDOR_ID_ATI, 0x4e50,
> 0x1025, 0x0061, 1},
> --
> 1.7.7.6
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 28622] radeon video lockup
https://bugzilla.kernel.org/show_bug.cgi?id=28622 Alan changed: What|Removed |Added Status|NEW |RESOLVED Resolution||OBSOLETE -- Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[pull] radeon drm-fixes-3.7
From: Alex Deucher Hi Dave, A couple more small fixes for 3.7: - another evergreen_mc fix - add an AGP quirk for an old RV250 The following changes since commit 6f755116c93ca35f496ccf1910dcd28cd16713e3: Merge branch 'drm-intel-fixes' of git://people.freedesktop.org/~danvet/drm-intel into drm-fixes (2012-11-16 10:00:43 +1000) are available in the git repository at: git://people.freedesktop.org/~agd5f/linux drm-fixes-3.7 Alex Deucher (1): drm/radeon: properly track the crtc not_enabled case evergreen_mc_stop() Paul Bolle (1): radeon: add AGPMode 1 quirk for RV250 drivers/gpu/drm/radeon/evergreen.c |2 ++ drivers/gpu/drm/radeon/radeon_agp.c |5 - 2 files changed, 6 insertions(+), 1 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm-next update
On Tue, Nov 20, 2012 at 7:40 AM, Ville Syrjälä wrote: > On Tue, Nov 20, 2012 at 04:39:39PM +1000, Dave Airlie wrote: >> Okay just pushed out a bunch of -next queued stuff, >> >> I've been stuck on another project for a couple of weeks and haven't >> really being paying enough attention to -next, so this is a heads up, >> if someone has something big they want merged this window and I >> haven't seen it yet or merged it yet, you should probably mention it >> in a reply to this mail with a summary of where you think its at. >> (e.g. atomic nuclear modesetting flipping). > > I don't the atomic stuff is quite ready for merging yet. However my > code is more or less feature complete now. I have implemented everything > I planned to originally, apart from adding more properties for various > things. And as I mentioned before, Weston is now running quite nicely > on top of my kernel branch. > > What's still missing is some refactoring, and possibly some fixes. And > hardly anyone has commented on it, so please everyone have a look and > let me know what you think. Maybe I need to start a blog to get people > interested... it would be nice to at least pull in the object and signed property type stuff from my branch, since that is effecting userspace facing API.. All the plane/crtc/etc state object stuff doesn't effect abi so could come later, I suppose. It does touch a lot of drivers even if they aren't converted to 'atomic', but OTOH I think it makes things much cleaner and easier to 'atomicify' a driver without duplicating so much stuff in each driver. So long term I still think it is the right thing. But not sure how much more time I'll spend on it in the near term. BR, -R > Oh and there's now that 8byte get_user() issue that needs to be sorted > out on x86-32. > > As far as the i915 specific stuff goes, I need to get someone to look > at the GPU sync stuff. And then there's the bigger question of whether > we could do the whole atomic page flip thing via the ring, but that's > not something that we need to worry about today. > > -- > Ville Syrjälä > Intel OTC > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm-next update
On Tue, Nov 20, 2012 at 11:27:20AM -0600, Rob Clark wrote: > On Tue, Nov 20, 2012 at 7:40 AM, Ville Syrjälä > wrote: > > On Tue, Nov 20, 2012 at 04:39:39PM +1000, Dave Airlie wrote: > >> Okay just pushed out a bunch of -next queued stuff, > >> > >> I've been stuck on another project for a couple of weeks and haven't > >> really being paying enough attention to -next, so this is a heads up, > >> if someone has something big they want merged this window and I > >> haven't seen it yet or merged it yet, you should probably mention it > >> in a reply to this mail with a summary of where you think its at. > >> (e.g. atomic nuclear modesetting flipping). > > > > I don't the atomic stuff is quite ready for merging yet. However my > > code is more or less feature complete now. I have implemented everything > > I planned to originally, apart from adding more properties for various > > things. And as I mentioned before, Weston is now running quite nicely > > on top of my kernel branch. > > > > What's still missing is some refactoring, and possibly some fixes. And > > hardly anyone has commented on it, so please everyone have a look and > > let me know what you think. Maybe I need to start a blog to get people > > interested... > > it would be nice to at least pull in the object and signed property > type stuff from my branch, since that is effecting userspace facing > API.. Yeah, I still need to go over your code properly. What I remember from the last time I looked at it was that the signed property type is fine by me, the dynamic flag I don't really agree with, since you probably can't set it for most things anyway, and the state stuff is touching a lot of places at the same time, which is somehting I've been trying to avoid at this stage. > All the plane/crtc/etc state object stuff doesn't effect abi so could > come later, I suppose. It does touch a lot of drivers even if they > aren't converted to 'atomic', but OTOH I think it makes things much > cleaner and easier to 'atomicify' a driver without duplicating so much > stuff in each driver. So long term I still think it is the right > thing. But not sure how much more time I'll spend on it in the near > term. I don't really want to push a huge change to all code paths at the same time. The current setcrtc/pageflip code is known to work (sort of at least), so if something were to regress we'd possibly have to back out the whole thing. So I'd like to get the atomic stuff in as a separate thing first. Once it's in we can start to fix the state handling, and also start calling into the atomic code from the legacy code paths. Another detail we nee to figure out is the actual properties that we're using. I personally don't like the crtc.SRC_X and crtc.SRC_Y properties that my code has for example. They don't behave like the plane properties with the same names, so maybe we should use different names for them. Well, idelly we wouldn't even have them, but moving all scanout duties over to planes is another thing I really don't want to tie into the atomic stuff at this point in time. I suppose we can't deprecate any properties easily, so we need to make sure we can all live with the ones we add initially. -- Ville Syrjälä Intel OTC ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 56405] Distorted graphics on Radeon HD 6620G
https://bugs.freedesktop.org/show_bug.cgi?id=56405 --- Comment #22 from Michael Dressel --- Created attachment 70326 --> https://bugs.freedesktop.org/attachment.cgi?id=70326&action=edit bisect log I think all the information can be derived by replaying bisect log information from the file I attached. -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 56405] Distorted graphics on Radeon HD 6620G
https://bugs.freedesktop.org/show_bug.cgi?id=56405 --- Comment #23 from Michael Dressel --- Created attachment 70327 --> https://bugs.freedesktop.org/attachment.cgi?id=70327&action=edit patch I used for compilation This is git diff -p output showing the patches I applied to get some thing compiled. -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 26345] [845G] CPU/GPU incoherency
https://bugs.freedesktop.org/show_bug.cgi?id=26345 Chris Wilson changed: What|Removed |Added QA Contact|xorg-t...@lists.x.org |intel-gfx-bugs@lists.freede ||sktop.org -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [GIT PULL] exynos-drm-next
Hello, On 11/20/2012 8:35 AM, Inki Dae wrote: Hi Dave, This patch set adds iommu support, userptr feature to g2d, minor fixups and code cleanups. And the iommu feature has dependency of the below patches related to dma mapping framework. For this, ccing DMA Mapping framework maintainer, Marek Szyprowski. Marek, please give me ack. common: DMA-mapping: add DMA_ATTR_FORCE_CONTIGUOUS attribute ARM: dma-mapping: add support for DMA_ATTR_FORCE_CONTIGUOUS attribute. For the dma-mapping related changes: Acked-by: Marek Szyprowski This patch is used to allocate fully physically contiguous memory region. And now the below patch is being reviewed and it's going to be merged soon. drm/exynos: remove 'pages' and 'page_size' elements in exynos gem buffer And this includes also previous pull request to exynos-drm-fixes. If there is any problem, plese let me know. Thanks, Inki Dae The following changes since commit bf6f036848ab2151c2498f11cb7d31a52a95dd5c: drm/vmwgfx: Make vmw_dmabuf_unreference handle NULL objects (2012-11-20 16:19:59 +1000) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git exynos-drm-next Imre Deak (4): drm/exynos: hold event_lock while accessing pageflip_event_list drm/exynos: call drm_vblank_put for each queued flip event drm/exynos: fix lockdep for event_lock wrt. vbl_time_lock drm: hold event_lock while accessing vblank_event_list Inki Dae (12): drm/exynos: fix linux framebuffer address setting. drm/exynos: remove unnecessary code. drm/exynos: fix overlay updating issue drm/exynos: add iommu support for exynos drm framework drm/exynos: add iommu support to fimd driver drm/exynos: add iommu support for hdmi driver drm/exynos: add iommu support for g2d drm/exynos: remove EXYNOS_BO_NONCONTIG type checking. drm/exynos: add userptr feature for g2d module drm/exynos: add vm_ops to specific gem mmaper drm/exynos: release fb pended by page flip drm/exynos: make sure that overlay data are updated Marek Szyprowski (2): common: DMA-mapping: add DMA_ATTR_FORCE_CONTIGUOUS attribute ARM: dma-mapping: add support for DMA_ATTR_FORCE_CONTIGUOUS attribute Prathyush K (2): drm/exynos: remove unnecessary sg_alloc_table call drm/exynos: add exynos drm specific fb_mmap function Rahul Sharma (1): drm: exynos: fix for mapping of dma buffers Sachin Kamat (1): drm/exynos: Make exynos4/5_fimd_driver_data static Documentation/DMA-attributes.txt|9 + arch/arm/mm/dma-mapping.c | 41 ++- drivers/gpu/drm/drm_irq.c |3 + drivers/gpu/drm/exynos/Kconfig |6 + drivers/gpu/drm/exynos/Makefile |1 + drivers/gpu/drm/exynos/exynos_drm_buf.c | 88 ++--- drivers/gpu/drm/exynos/exynos_drm_crtc.c| 47 +++- drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 85 ++--- drivers/gpu/drm/exynos/exynos_drm_drv.c | 23 ++- drivers/gpu/drm/exynos/exynos_drm_drv.h | 14 +- drivers/gpu/drm/exynos/exynos_drm_encoder.c | 49 +++- drivers/gpu/drm/exynos/exynos_drm_encoder.h |1 + drivers/gpu/drm/exynos/exynos_drm_fb.c | 79 +- drivers/gpu/drm/exynos/exynos_drm_fb.h |6 + drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 42 +++- drivers/gpu/drm/exynos/exynos_drm_fimd.c| 33 +-- drivers/gpu/drm/exynos/exynos_drm_g2d.c | 491 +++ drivers/gpu/drm/exynos/exynos_drm_gem.c | 446 + drivers/gpu/drm/exynos/exynos_drm_gem.h | 52 +++- drivers/gpu/drm/exynos/exynos_drm_hdmi.c| 15 + drivers/gpu/drm/exynos/exynos_drm_hdmi.h|1 + drivers/gpu/drm/exynos/exynos_drm_iommu.c | 150 drivers/gpu/drm/exynos/exynos_drm_iommu.h | 85 + drivers/gpu/drm/exynos/exynos_drm_plane.c | 19 +- drivers/gpu/drm/exynos/exynos_drm_vidi.c| 20 +- drivers/gpu/drm/exynos/exynos_hdmi.c| 21 ++ drivers/gpu/drm/exynos/exynos_mixer.c | 11 +- include/linux/dma-attrs.h |1 + include/uapi/drm/exynos_drm.h | 13 +- 29 files changed, 1371 insertions(+), 481 deletions(-) create mode 100644 drivers/gpu/drm/exynos/exynos_drm_iommu.c create mode 100644 drivers/gpu/drm/exynos/exynos_drm_iommu.h Best regards -- Marek Szyprowski Samsung Poland R&D Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Linux 3.7-rc6
> drm/i915: do not ignore eDP bpc settings from vbt
As advertised, this patch breaks the Macbook Pro Retina, which seems
unfair. The patch below is certainly not the best remedy, but it does
work. Tested on a MacbookPro10,1.
Thanks,
Henrik
---
From: Henrik Rydberg
Date: Tue, 20 Nov 2012 11:16:05 +0100
Subject: [PATCH] drm/i915: Ignore eDP bpc settings from vbt based on dmi data
Status: O
Content-Length: 1637
Lines: 51
Commit 2f4f649a6 breaks the Retina display on MacBookPro10 laptops.
This patch reintroduces the logic reverted by that patch, but only
for the Retina machines.
Signed-off-by: Henrik Rydberg
---
drivers/gpu/drm/i915/intel_display.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 4154bcd..97658d1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3776,6 +3776,24 @@ static inline bool intel_panel_use_ssc(struct
drm_i915_private *dev_priv)
&& !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE);
}
+static int dmi_ignore_bpc_from_vbt_callback(const struct dmi_system_id *id)
+{
+ DRM_INFO("Ignoring bpc from vbt on %s\n", id->ident);
+ return 1;
+}
+
+static const struct dmi_system_id dmi_ignore_bpc_from_vbt[] = {
+ {
+ .callback = dmi_ignore_bpc_from_vbt_callback,
+ .ident = "Apple MacBook Pro Retina",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro10"),
+ },
+ },
+ { } /* terminating entry */
+};
+
/**
* intel_choose_pipe_bpp_dither - figure out what color depth the pipe should
send
* @crtc: CRTC structure
@@ -3841,7 +3859,8 @@ static bool intel_choose_pipe_bpp_dither(struct drm_crtc
*crtc,
}
}
- if (intel_encoder->type == INTEL_OUTPUT_EDP) {
+ if (intel_encoder->type == INTEL_OUTPUT_EDP &&
+ !dmi_check_system(dmi_ignore_bpc_from_vbt)) {
/* Use VBT settings if we have an eDP panel */
unsigned int edp_bpc = dev_priv->edp.bpp / 3;
--
1.8.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1] drm: exynos: fix for loosing display mode header during mode adjustment
This patch is to preserve the display mode header during the mode adjustment.
Display mode header is overwritten with the adjusted mode header which is
throwing the stack dump.
Signed-off-by: Rahul Sharma
---
drivers/gpu/drm/exynos/exynos_hdmi.c |9 +
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 2c115f8..be7b676 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -1978,9 +1978,18 @@ static void hdmi_mode_fixup(void *ctx, struct
drm_connector *connector,
index = hdmi_v14_conf_index(m);
if (index >= 0) {
+ struct drm_mode_object base;
+ struct list_head head;
+
DRM_INFO("desired mode doesn't exist so\n");
DRM_INFO("use the most suitable mode among modes.\n");
+
+ /* preserve display mode header while copying. */
+ head = adjusted_mode->head;
+ base = adjusted_mode->base;
memcpy(adjusted_mode, m, sizeof(*m));
+ adjusted_mode->head = head;
+ adjusted_mode->base = base;
break;
}
}
--
1.7.0.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v12 0/6] of: add display helper
On Tue, Nov 20, 2012 at 05:13:19PM +0100, Laurent Pinchart wrote: > On Tuesday 20 November 2012 16:54:50 Steffen Trumtrar wrote: > > Hi! > > > > Changes since v11: > > - make pointers const where applicable > > - add reviewed-by Laurent Pinchart > > Looks good to me. > > Acked-by: Laurent Pinchart > > Through which tree do you plan to push this ? We have no idea yet, and none of the people on Cc: have volunteered so far... what do you think? rsc -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 56405] Distorted graphics on Radeon HD 6620G
https://bugs.freedesktop.org/show_bug.cgi?id=56405 --- Comment #24 from Michel Dänzer --- (In reply to comment #22) > bisect log It does look like you're getting close to isolating a commit. Most likely it's one of the remaining r600g commits, but there's still too many of them to guess which one. (In reply to comment #23) > patch I used for compilation Some of this looks a little dubious, but I guess it should do for the bisection. For the latest compile problem, have you tried manually applying the two revert commits at the end of the remaining commits? -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 56405] Distorted graphics on Radeon HD 6620G
https://bugs.freedesktop.org/show_bug.cgi?id=56405 --- Comment #25 from Michael Dressel --- I have not tried that before. Now I cherry picked 0b616b2f5164621e3a63caeb15c6eb1d01bbc55a 4a162f6eba73a8cb2654cd99a2bd12bd468925a6 on top of 302862defa61b2cee1ae24159aca306f090ca854 The r600_dri.so did compile now. But now gnome starts in fallback mode with the new r600_dri.so. And in this mode I can not test the problem. -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 56405] Distorted graphics on Radeon HD 6620G
https://bugs.freedesktop.org/show_bug.cgi?id=56405 --- Comment #26 from Michel Dänzer --- (In reply to comment #25) > But now gnome starts in fallback mode with the new r600_dri.so. Does LIBGL_DEBUG=verbose glxinfo | grep render give any hints as to what might be wrong with this r600_dri.so? -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 56405] Distorted graphics on Radeon HD 6620G
https://bugs.freedesktop.org/show_bug.cgi?id=56405 --- Comment #27 from Michael Dressel --- (In reply to comment #26) > (In reply to comment #25) > LIBGL_DEBUG=verbose glxinfo | grep render I don't have glxinfo -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 56405] Distorted graphics on Radeon HD 6620G
https://bugs.freedesktop.org/show_bug.cgi?id=56405 --- Comment #28 from Michael Dressel --- (In reply to comment #27) > (In reply to comment #26) > > (In reply to comment #25) > > LIBGL_DEBUG=verbose glxinfo | grep render > I don't have glxinfo Ah I found it on archlinux AUR in the package mesa-demo-git. I'm just building it. -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v12 0/6] of: add display helper
On Tue, Nov 20, 2012 at 07:11:29PM +0100, Robert Schwebel wrote: > On Tue, Nov 20, 2012 at 05:13:19PM +0100, Laurent Pinchart wrote: > > On Tuesday 20 November 2012 16:54:50 Steffen Trumtrar wrote: > > > Hi! > > > > > > Changes since v11: > > > - make pointers const where applicable > > > - add reviewed-by Laurent Pinchart > > > > Looks good to me. > > > > Acked-by: Laurent Pinchart > > > > Through which tree do you plan to push this ? > > We have no idea yet, and none of the people on Cc: have volunteered > so far... what do you think? The customary approach would be to take the patches through separate trees, but I think this particular series is sufficiently interwoven to warrant taking them all through one tree. However the least that we should do is collect Acked-by's from the other tree maintainers. Given that most of the patches modify files in drivers/video, Florian's fbdev tree would be the most obvious candidate, right? If Florian agrees to take the patches, all we would need is David's Acked-by. How does that sound? Thierry pgpqcmQR6GaGc.pgp Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 56405] Distorted graphics on Radeon HD 6620G
https://bugs.freedesktop.org/show_bug.cgi?id=56405 --- Comment #29 from Michael Dressel --- (In reply to comment #28) > (In reply to comment #27) > > (In reply to comment #26) > > > (In reply to comment #25) > > > LIBGL_DEBUG=verbose glxinfo | grep render > > I don't have glxinfo > > Ah I found it on archlinux AUR in the > package mesa-demo-git. I'm just > building it. Here is the result: libGL: OpenDriver: trying /usr/lib/xorg/modules/dri/tls/r600_dri.so libGL: OpenDriver: trying /usr/lib/xorg/modules/dri/r600_dri.so libGL error: dlopen /usr/lib/xorg/modules/dri/r600_dri.so failed (/usr/lib/libglsl.so: undefined symbol: hash_table_string_hash) libGL error: unable to load driver: r600_dri.so libGL error: driver pointer missing libGL: OpenDriver: trying /usr/lib/xorg/modules/dri/tls/swrast_dri.so libGL: OpenDriver: trying /usr/lib/xorg/modules/dri/swrast_dri.so -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/exynos: fix memory leak: free EDID block
On Tue, Nov 20, 2012 at 4:30 AM, Egbert Eich wrote:
> drm_get_edid() returns a pointer to an EDID block. The caller
> is responsible to free this pointer itself.
> Here the pointer gets assigned to the local variable raw_edid.
> Therefore it should be freed before the variable goes out of
> scope.
>
> Signed-off-by: Egbert Eich
> ---
> drivers/gpu/drm/exynos/exynos_hdmi.c |1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
> b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index 2c115f8..bc87bca 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -1293,6 +1293,7 @@ static int hdmi_get_edid(void *ctx, struct
> drm_connector *connector,
> DRM_DEBUG_KMS("%s : width[%d] x height[%d]\n",
> (hdata->dvi_mode ? "dvi monitor" : "hdmi monitor"),
> raw_edid->width_cm, raw_edid->height_cm);
> + kfree(raw_edid);
This will actually cause the memory to be freed twice.
The reason this happens is drm_get_edid attaches this to
connector->display_info.raw_edid, which is then freed in the
exynos_drm_connector function that gets the edid.
The whole thing is ugly, and needs to be revised. I've uploaded a
patch to refactor this against the chromium tree, but haven't yet
rebased against upstream. See
https://gerrit.chromium.org/gerrit/#/c/38406/
For now, please drop this patch.
Sean
> } else {
> return -ENODEV;
> }
> --
> 1.7.7
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/exynos: fix memory leak: free EDID block
Sean Paul writes:
> On Tue, Nov 20, 2012 at 4:30 AM, Egbert Eich wrote:
> > drm_get_edid() returns a pointer to an EDID block. The caller
> > is responsible to free this pointer itself.
> > Here the pointer gets assigned to the local variable raw_edid.
> > Therefore it should be freed before the variable goes out of
> > scope.
> >
> > Signed-off-by: Egbert Eich
> > ---
> > drivers/gpu/drm/exynos/exynos_hdmi.c |1 +
> > 1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
> > b/drivers/gpu/drm/exynos/exynos_hdmi.c
> > index 2c115f8..bc87bca 100644
> > --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> > +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> > @@ -1293,6 +1293,7 @@ static int hdmi_get_edid(void *ctx, struct
> > drm_connector *connector,
> > DRM_DEBUG_KMS("%s : width[%d] x height[%d]\n",
> > (hdata->dvi_mode ? "dvi monitor" : "hdmi monitor"),
> > raw_edid->width_cm, raw_edid->height_cm);
> > + kfree(raw_edid);
>
> This will actually cause the memory to be freed twice.
>
> The reason this happens is drm_get_edid attaches this to
> connector->display_info.raw_edid, which is then freed in the
> exynos_drm_connector function that gets the edid.
No, the raw_edid member is gone from struct drm_display_info.
All other drivers free the edid data returned by drm_get_edid()
themselves.
I would actually prefer if the edid data would be freed by
drm_connector_destroy rather than by the drivers as this would
allow us to cache an edid without creating copies to pass to
the drivers. However some drivers store the pointer to returned
edid block somewhere in their own data structures, so if we
do this it can easily get out of sync when we reread the
edid and it has changed for whatever reason in which case
the driver will have a stale and invalid pointer.
>
> The whole thing is ugly, and needs to be revised. I've uploaded a
> patch to refactor this against the chromium tree, but haven't yet
> rebased against upstream. See
> https://gerrit.chromium.org/gerrit/#/c/38406/
>
> For now, please drop this patch.
>
I've looked at the code in the exynos driver in the drm_next
branch - there's nothing that destroys this edid data any more.
Cheers,
Egbert.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/exynos: fix memory leak: free EDID block
On Tue, Nov 20, 2012 at 3:29 PM, Egbert Eich wrote:
> Sean Paul writes:
> > On Tue, Nov 20, 2012 at 4:30 AM, Egbert Eich wrote:
> > > drm_get_edid() returns a pointer to an EDID block. The caller
> > > is responsible to free this pointer itself.
> > > Here the pointer gets assigned to the local variable raw_edid.
> > > Therefore it should be freed before the variable goes out of
> > > scope.
> > >
> > > Signed-off-by: Egbert Eich
Reviewed-by: Sean Paul
> > > ---
> > > drivers/gpu/drm/exynos/exynos_hdmi.c |1 +
> > > 1 files changed, 1 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
> b/drivers/gpu/drm/exynos/exynos_hdmi.c
> > > index 2c115f8..bc87bca 100644
> > > --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> > > +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> > > @@ -1293,6 +1293,7 @@ static int hdmi_get_edid(void *ctx, struct
> drm_connector *connector,
> > > DRM_DEBUG_KMS("%s : width[%d] x height[%d]\n",
> > > (hdata->dvi_mode ? "dvi monitor" : "hdmi
> monitor"),
> > > raw_edid->width_cm, raw_edid->height_cm);
> > > + kfree(raw_edid);
> >
> > This will actually cause the memory to be freed twice.
> >
> > The reason this happens is drm_get_edid attaches this to
> > connector->display_info.raw_edid, which is then freed in the
> > exynos_drm_connector function that gets the edid.
>
> No, the raw_edid member is gone from struct drm_display_info.
> All other drivers free the edid data returned by drm_get_edid()
> themselves.
A ha! I should have checked first, my apologies.
> I would actually prefer if the edid data would be freed by
> drm_connector_destroy rather than by the drivers as this would
> allow us to cache an edid without creating copies to pass to
> the drivers. However some drivers store the pointer to returned
> edid block somewhere in their own data structures, so if we
> do this it can easily get out of sync when we reread the
> edid and it has changed for whatever reason in which case
> the driver will have a stale and invalid pointer.
>
Agreed, that would be nice.
> >
> > The whole thing is ugly, and needs to be revised. I've uploaded a
> > patch to refactor this against the chromium tree, but haven't yet
> > rebased against upstream. See
> > https://gerrit.chromium.org/gerrit/#/c/38406/
> >
> > For now, please drop this patch.
> >
>
> I've looked at the code in the exynos driver in the drm_next
> branch - there's nothing that destroys this edid data any more.
>
Yep, right you are. Your patch does the right thing, thanks for
setting me straight :)
I hope we won't need to live with this double-allocation + copy for too long.
Sean
> Cheers,
> Egbert.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 57349] New: [nouveau, EFI] xf86-video-nouveau-1.0.2 and above fail to find device with X 1.13.0
https://bugs.freedesktop.org/show_bug.cgi?id=57349 Priority: medium Bug ID: 57349 Assignee: dri-devel@lists.freedesktop.org Summary: [nouveau, EFI] xf86-video-nouveau-1.0.2 and above fail to find device with X 1.13.0 Severity: normal Classification: Unclassified OS: Linux (All) Reporter: bonbon...@internet.lu Hardware: x86-64 (AMD64) Status: NEW Version: XOrg CVS Component: DRM/other Product: DRI Commit 2f48b8f462a03cb92db9e9a7ae1957eb27473965 "nouveau: add platform bus support" added in -1.0.2 prevents X from starting on a MacBookAir booted via EFI. As debugged on IRC with airlied, the cause is that kernel reports "boot_vga" being "0" for the GeForce 9400M (C79) when efifb is not being used (not compiled into kernel whereas nouveau is). [booting with efifb built-in causes "boot_vga" to be "1"] When there is no other GPU around and KMS (+ fbcon) is active on that GPU X should accept it as well (or kernel adjust boot_vga as needed even while efifb is not compiled in/used). Kernel used if 3.7-rc5 (with efifb disabled), 3.7-rc6 (with efifb built-in) -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 57350] New: [nouveau, linux-3.7-rc] Broken cursor and kernel log swamped with "PAGE_NOT_PRESENT" errors
https://bugs.freedesktop.org/show_bug.cgi?id=57350 Priority: medium Bug ID: 57350 Assignee: dri-devel@lists.freedesktop.org Summary: [nouveau, linux-3.7-rc] Broken cursor and kernel log swamped with "PAGE_NOT_PRESENT" errors Severity: normal Classification: Unclassified OS: All Reporter: bonbon...@internet.lu Hardware: Other Status: NEW Version: XOrg CVS Component: DRM/other Product: DRI On a MacBookAir with GeForce 9400M (C79) when X starts after booting (using Enlightenment with compositing) a horizontal bar in the middle of cursor (about 1/3 of cursor height) is transparent instead of showing cursor. At the same time, kernel log is being filled with following error: nouveau E[ PFB][:02:00.0] trapped read at $addr on channel $fixed_addr BAR/PFIFO_READ/FB reason: PAGE_NOT_PRESENT $fixed_addr is always the same (0xfee0) and $addr changes each time (e.g. 0x40fe00 then 0x40fe0c then 0x40fe14 and continues incrementing) Doing a suspend to ram and resume makes the cursor show correctly and error message not appearing anymore in kernel log. Software versions: linux-3.7-rc5, linux-3.7-rc6 xorg-server-1.13.0 xf86-video-nouveau-1.0.4 libdrm-2.4.40 mesa-9.0 It was also the case with xorg-server-1.12.2, xf86-video-nouveau GIT snapshot from march 2012, libdrm-2.4.33, mesa-8.0.3) but did not happen with this setup with older kernel (linux-3.5.x, not sure anymore about linux-3.6.x) -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 57352] New: [nouveau] Kernel Tux logo incorrectly displayed at boot
https://bugs.freedesktop.org/show_bug.cgi?id=57352 Priority: medium Bug ID: 57352 Assignee: dri-devel@lists.freedesktop.org Summary: [nouveau] Kernel Tux logo incorrectly displayed at boot Severity: normal Classification: Unclassified OS: All Reporter: bonbon...@internet.lu Hardware: Other Status: NEW Version: XOrg CVS Component: DRM/other Product: DRI When booting with build-in nouveau (KMS enabled) the Tux logos displayed are not recognizable. It looks like incorrect tiling configuration of either source or destination buffer when rendering the logo. Normal fbcon console text displays as expected. This happens with linux-3.7-rc5 and newer (don't know since when). Hardware is MacBookAir with GeForce 9400M (C79) [10de:0870]. (screenshot following later on) -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm-next update
On Tue, Nov 20, 2012 at 11:53 AM, Ville Syrjälä wrote: > On Tue, Nov 20, 2012 at 11:27:20AM -0600, Rob Clark wrote: >> On Tue, Nov 20, 2012 at 7:40 AM, Ville Syrjälä >> wrote: >> > On Tue, Nov 20, 2012 at 04:39:39PM +1000, Dave Airlie wrote: >> >> Okay just pushed out a bunch of -next queued stuff, >> >> >> >> I've been stuck on another project for a couple of weeks and haven't >> >> really being paying enough attention to -next, so this is a heads up, >> >> if someone has something big they want merged this window and I >> >> haven't seen it yet or merged it yet, you should probably mention it >> >> in a reply to this mail with a summary of where you think its at. >> >> (e.g. atomic nuclear modesetting flipping). >> > >> > I don't the atomic stuff is quite ready for merging yet. However my >> > code is more or less feature complete now. I have implemented everything >> > I planned to originally, apart from adding more properties for various >> > things. And as I mentioned before, Weston is now running quite nicely >> > on top of my kernel branch. >> > >> > What's still missing is some refactoring, and possibly some fixes. And >> > hardly anyone has commented on it, so please everyone have a look and >> > let me know what you think. Maybe I need to start a blog to get people >> > interested... >> >> it would be nice to at least pull in the object and signed property >> type stuff from my branch, since that is effecting userspace facing >> API.. > > Yeah, I still need to go over your code properly. What I remember from > the last time I looked at it was that the signed property type is fine > by me, the dynamic flag I don't really agree with, since you probably > can't set it for most things anyway, and the state stuff is touching a > lot of places at the same time, which is somehting I've been trying to > avoid at this stage. The dynamic flag is a bit of an optimization. It is really only intended to be set for properties that can always be changed without risk of fail. There is no obligation for a driver to set it, since it should be an opt-in sort of thing for the driver. Which also means that it is safe to add at a later stage, so I don't mind leaving it for later. But object and signed property types would be good to have from the beginning. >> All the plane/crtc/etc state object stuff doesn't effect abi so could >> come later, I suppose. It does touch a lot of drivers even if they >> aren't converted to 'atomic', but OTOH I think it makes things much >> cleaner and easier to 'atomicify' a driver without duplicating so much >> stuff in each driver. So long term I still think it is the right >> thing. But not sure how much more time I'll spend on it in the near >> term. > > I don't really want to push a huge change to all code paths at the same > time. The current setcrtc/pageflip code is known to work (sort of at > least), so if something were to regress we'd possibly have to back out > the whole thing. So I'd like to get the atomic stuff in as a separate > thing first. true, although most of the changes for drivers with the state stuff, before they migrate to atomic, are just braindead search/replace stuff, so not much risk of breaking things. Just a pain to merge through a bunch of different driver trees, that is all. > Once it's in we can start to fix the state handling, and also start > calling into the atomic code from the legacy code paths. yeah, handling legacy ioctls with "atomicified" drivers is an issue.. mainly with the setplane ioctl because we never really defined if it was async or not or what the meaning is if it is called and the driver is still pending a flip. The good news is there aren't many users of that API yet. And if we pass the flags to the atomic fxns to indicate if the update should be async or not, we could just not pass the async flag in the legacy setplane path. So I think this is doable.. but maybe a bit more risky. We could always handle conversion of legacy paths to atomic as a later patch, if that helps reduce the risk. > Another detail we nee to figure out is the actual properties that > we're using. I personally don't like the crtc.SRC_X and crtc.SRC_Y > properties that my code has for example. They don't behave like the > plane properties with the same names, so maybe we should use different > names for them. Well, idelly we wouldn't even have them, but moving all > scanout duties over to planes is another thing I really don't want to > tie into the atomic stuff at this point in time. I suppose we can't > deprecate any properties easily, so we need to make sure we can all > live with the ones we add initially. hmm, maybe it is a good idea to give different property names for crtc src x/y. I guess it isn't completely different from plane SRC_X/Y except there is no scaling and CRTC_X/Y is always 0,0.. OTOH, separating plane and crtc would be a generally nice thing to do and maybe avoids some hacks. So I wouldn't object to trying to do that firs
drm/exynos: Unreference fb in exynos_disable_plane()
From: YoungJun Cho
This patch is for unreferencing the (current)fb if plane->fb is
existed in exynos_disable_plane(). In exynos_update_plane(),
the new fb reference count can be bigger than 1.
So it can't be removed for that reference count.
And this patch is based on exynos-drm-next.
Signed-off-by: YoungJun Cho
Signed-off-by: Inki Dae
Signed-off-by: Kyungmin Park
---
drivers/gpu/drm/exynos/exynos_drm_plane.c | 12 +++-
1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c
b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index 083fd5f..dc7e057 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -161,13 +161,15 @@ exynos_update_plane(struct drm_plane *plane, struct
drm_crtc *crtc,
if (plane->fb)
drm_framebuffer_unreference(plane->fb);
+ plane->fb = fb;
+
/*
* Take a reference to new fb.
*
* Taking a reference means that this plane's dma is going to access
* memory region to the new fb.
*/
- drm_framebuffer_reference(fb);
+ drm_framebuffer_reference(plane->fb);
plane->crtc = crtc;
exynos_plane_commit(plane);
@@ -180,6 +182,14 @@ static int exynos_disable_plane(struct drm_plane *plane)
{
DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
+ /*
+* Unreference the (current)fb if plane->fb is existed.
+* In exynos_update_plane(), the new fb reference count can be bigger
+* than 1. So it can't be removed for that reference count.
+*/
+ if (plane->fb)
+ drm_framebuffer_unreference(plane->fb);
+
exynos_plane_dpms(plane, DRM_MODE_DPMS_OFF);
return 0;
--
1.7.4.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/exynos: fix memory leak: free EDID block
Ok, I modifed the subject of your patch like below and applied.
drm/exynos: fix memory lean to EDID block
Thanks,
Inki Dae
2012/11/20 Egbert Eich
> drm_get_edid() returns a pointer to an EDID block. The caller
> is responsible to free this pointer itself.
> Here the pointer gets assigned to the local variable raw_edid.
> Therefore it should be freed before the variable goes out of
> scope.
>
> Signed-off-by: Egbert Eich
> ---
> drivers/gpu/drm/exynos/exynos_hdmi.c |1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
> b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index 2c115f8..bc87bca 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -1293,6 +1293,7 @@ static int hdmi_get_edid(void *ctx, struct
> drm_connector *connector,
> DRM_DEBUG_KMS("%s : width[%d] x height[%d]\n",
> (hdata->dvi_mode ? "dvi monitor" : "hdmi monitor"),
> raw_edid->width_cm, raw_edid->height_cm);
> + kfree(raw_edid);
> } else {
> return -ENODEV;
> }
> --
> 1.7.7
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1] drm: exynos: fix for loosing display mode header during mode adjustment
Applied.
Thanks,
Inki Dae
2012/11/20 Rahul Sharma
> This patch is to preserve the display mode header during the mode
> adjustment.
> Display mode header is overwritten with the adjusted mode header which is
> throwing the stack dump.
>
> Signed-off-by: Rahul Sharma
> ---
> drivers/gpu/drm/exynos/exynos_hdmi.c |9 +
> 1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
> b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index 2c115f8..be7b676 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -1978,9 +1978,18 @@ static void hdmi_mode_fixup(void *ctx, struct
> drm_connector *connector,
> index = hdmi_v14_conf_index(m);
>
> if (index >= 0) {
> + struct drm_mode_object base;
> + struct list_head head;
> +
> DRM_INFO("desired mode doesn't exist so\n");
> DRM_INFO("use the most suitable mode among
> modes.\n");
> +
> + /* preserve display mode header while copying. */
> + head = adjusted_mode->head;
> + base = adjusted_mode->base;
> memcpy(adjusted_mode, m, sizeof(*m));
> + adjusted_mode->head = head;
> + adjusted_mode->base = base;
> break;
> }
> }
> --
> 1.7.0.4
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RESEND][PATCH] drm/prime: drop reference on imported dma-buf come from gem
On 2012년 11월 20일 19:26, Maarten Lankhorst wrote: > Op 20-11-12 02:03, 김승우 schreef: >> Hi Maarten, >> >> On 2012년 11월 19일 19:27, Maarten Lankhorst wrote: >>> Op 15-11-12 04:52, Seung-Woo Kim schreef: Increasing ref counts of both dma-buf and gem for imported dma-buf come from gem makes memory leak. release function of dma-buf cannot be called because f_count of dma-buf increased by importing gem and gem ref count cannot be decrease because of exported dma-buf. So I add dma_buf_put() for imported gem come from its own gem into each drivers having prime_import and prime_export capabilities. With this, only gem ref count is increased if importing gem exported from gem of same driver. Signed-off-by: Seung-Woo Kim Signed-off-by: Kyungmin.park Cc: Inki Dae Cc: Daniel Vetter Cc: Rob Clark Cc: Alex Deucher --- Mmap failiure in i915 was reported by Jani, and I think it was fixed with Dave's commit "drm/prime: add exported buffers to current fprivs imported buffer list (v2)", so I resend this patch. I can send exynos only, but this issue is common in all drm driver having both prime inport and export, IMHO, it's better to send for all drivers. >>> I fear that this won't solve the issue completely and keeps open a small >>> race. >> I don't believe my patch can fix all issue related with gem prime >> completely. But it seems that it can solve unrecoverable memory leak >> caused by re-importing GEM. Anyway, can you give me an example of other >> race issue? > When the dma_buf is unreffed and refcount drops to 0, work is queued to free > it. > > Until then export_dma_buf member points to something, but refcount is 0 on it. > > Importing to itself, then closing fd then re-export from the new place would > probably trigger it. > Ok, I understood about issue from delayed fput also in your below comment. Anyway, IMO, it is already on current drm prime. >>> I don't like the current destruction path either. It really looks like >>> export_dma_buf >>> should be unset when the exported buffer is closed in the original file. >>> Anything else is racy. >>> Especially setting export_dma_buf to NULL won't work with the current >>> delayed fput semantics. >> I cannot figure out all aspect of delayed fput, but until delayed work >> is called, dma_buf is not released so export_dma_buf is valid. >> Considering this, I can't find possible issue of delayed fput. > I'm fairly sure that when refcount drops to 0, even though the memory may not > be freed yet, > you no longer have a guarantee that the memory is still valid. I missed that delayed fput is triggered after recount drops to 0, and now I understood it can cause invalid access. > > And of course after importing into a different process with its own drm > namespace, how does > file_priv->prime.lock still protect serialization? > > I don't see any hierarchy either. export_dma_buf needs to be set to NULL > before the dma_buf_put > is called that is used for the reference inside export_dma_buf. > > The release function should only release a reference to whatever backing > memory is used. > >>> So to me it seems instead we should do something like this: >>> >>> - dmabuf_release callback is a noop, no ref is held by the dma-buf. >>> - attach and detach ops increase reference by 1*. >>> >>> - when the buffer is exported, export_dma_buf is set by core code and kept >>> around >>> alive until the gem object is closed. >>> >>> - dma_buf_put is not called by import function. This reference removed as >>> part >>> of gem object close instead. >> Considering re-import, where drm file priv is different from exporter's >> file priv, attach and detach are not called because gem object is reused. >> >> How do you think remove checking whether dma_buf is came from driver own >> exporter? Because without this checking, gem object is newly created, >> and then re-import issue will disappear. > The whole refcounting is a circular mess, sadly with no easy solution. :/ > > It seems to me that using gem reference counts is solving this problem at the > wrong layer. > A secondary type of reference count is needed for the underlying memory > backing. > > For those who use ttm, I would recommend that the dma_buf increases refcount > on ttm_bo by one, > so that the gem object can be destroyed and release its reference on the > dma_buf. > > WIthout a secondary refcount you end up in a position where you can't > reliably and race free unref > the gem object and dma_buf at the same time, since they're both independent > interfaces with possibly > different lifetimes. If there is no checking whether dma_buf is came from driver own exporter, gem_object is newly allocated and refcount of it can be a secondary refcount at least form mermoy leak issue. So I mentioned about removing check for exporter. But I prefer processing re-import as gem_open without c
Re: [PATCH 3/4] drm/ttm, drm/vmwgfx: Use RCU locking for object lookups v3
On Tue, Nov 20, 2012 at 4:44 PM, Thomas Hellstrom wrote: > On 11/20/2012 07:19 AM, Dave Airlie wrote: >> >> On Tue, Nov 6, 2012 at 9:31 PM, Thomas Hellstrom >> wrote: >>> >>> The mostly used lookup+get put+potential_destroy path of TTM objects >>> is converted to use RCU locks. This will substantially decrease the >>> amount >>> of locked bus cycles during normal operation. >>> Since we use kfree_rcu to free the objects, no rcu synchronization is >>> needed >>> at module unload time. >> >> As this is the first use of RCU in a drm driver from what I can see, >> let me remind that the >> RCU patent agreement AFAIK only covers GPL works. >> >> So non-GPL or other OSes porting this code should take not of this. >> >> Dave. > > > From VMware's side this won't be a problem, since other VMware kernel > modules (VMCI IIRC) use RCU. > > In any case I have a new version of the "vmwgfx optimization" patch series > that mostly add documentation and > annotation (by using a drm_ht_xxx_rcu) interface for hashtab, after an > internal review by Dmitry Torkov. I see you've already > applied the original patch series. Do you want me to send out the new one or > rebase it against current drm-next? Can you rebase it on top of -next? Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 05/17] DRM/KMS/EDID: Fix up EEDID Map Blocks if Extension block count has changed.
At Mon, 19 Nov 2012 15:23:06 -0500, "Egbert Eich " <"e...@novell.com> wrote: > > EEDID v1.3 mandates map blocks if more than one EDID extension block > is used while in v1.4 they are optional. > If the extension count has been changed (because some extension > blocks were not readable) those map blocks need fixing. > In case of v1.4 or higher we simply eliminate all map blocks as > this is less time consuming. For v1.3 we scrap any exsisting map > blocks and recreate them from scratch. > > Signed-off-by: Egbert Eich > --- > drivers/gpu/drm/drm_edid.c | 105 +-- > 1 files changed, 100 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index ec6f3bb..d1b9d67 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -44,7 +44,11 @@ > #define EDID_DETAILED_TIMINGS 4 > > #define EDID_EXTENSION_FLAG_OFFSET ((size_t)&((struct edid *)0)->extensions) > -#define EDID_CHECKSUM_OFFSET ((size_t)&((struct edid *)0)->checksum) > +#define EDID_CHECKSUM_OFFSET ((size_t)&((struct edid *)0)->checksum) > +#define EDID_VERSION_MAJOR_OFFSET ((size_t)&((struct edid *)0)->version) > +#define EDID_VERSION_MINOR_OFFSET ((size_t)&((struct edid *)0)->revision) Better to use offsetof(). Takashi ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 08/17] DRM/KMS/EDID: Use Extension Block Fixup Code also for 'firmware' EDID.
At Mon, 19 Nov 2012 15:23:09 -0500,
"Egbert Eich " <"e...@novell.com> wrote:
>
> Signed-off-by: Egbert Eich
> ---
> drivers/gpu/drm/drm_edid.c | 77
> ---
> drivers/gpu/drm/drm_edid_load.c | 54 ++-
> include/drm/drm_edid.h |1 +
> 3 files changed, 77 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index d1b9d67..7bdae6e 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -408,6 +408,29 @@ fixup_blockmaps(u8 **blockp, int eblock_cnt)
> return eblock_cnt;
> }
>
> +static int
> +fixup_edid(u8 **blockp, int valid_extensions)
> +{
> + u8 *new = NULL;
> +
> + if (valid_extensions != (*blockp)[EDID_EXTENSION_FLAG_OFFSET]) {
> +
> + if (valid_extensions)
> + valid_extensions = fixup_blockmaps(blockp,
> valid_extensions);
> +
> + if (valid_extensions >= 0) {
> + (*blockp)[EDID_CHECKSUM_OFFSET] +=
> (*blockp)[EDID_EXTENSION_FLAG_OFFSET] - valid_extensions;
> + (*blockp)[EDID_EXTENSION_FLAG_OFFSET] =
> valid_extensions;
> + new = krealloc(*blockp, (valid_extensions + 1) *
> EDID_LENGTH, GFP_KERNEL);
> + }
> + if (!new)
> + kfree(*blockp);
> +
> + *blockp = new;
> + }
> + return (new ? valid_extensions : -ENOMEM);
So this function returns -ENOMEM if valid_extensions is equal with
block[EDID_EXTENSION_FLAG_OFFSET]...?
Takashi
> +}
> +
> static u8 *
> drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
> {
> @@ -474,19 +497,7 @@ drm_do_get_edid(struct drm_connector *connector, struct
> i2c_adapter *adapter)
> }
>
> no_more:
> - if (valid_extensions != block[EDID_EXTENSION_FLAG_OFFSET]) {
> - if (valid_extensions)
> - valid_extensions = fixup_blockmaps(&block,
> valid_extensions);
> - if (valid_extensions >= 0) {
> - block[EDID_CHECKSUM_OFFSET] +=
> block[EDID_EXTENSION_FLAG_OFFSET] - valid_extensions;
> - block[EDID_EXTENSION_FLAG_OFFSET] = valid_extensions;
> - new = krealloc(block, (valid_extensions + 1) *
> EDID_LENGTH, GFP_KERNEL);
> - if (!new)
> - goto out;
> - } else
> - goto out;
> - block = new;
> - }
> + fixup_edid(&block, valid_extensions);
>
> return block;
>
> @@ -503,6 +514,46 @@ out:
> }
>
> /**
> + * Validate an entire EDID blob.
> + * \param connector: drm_connector struct of the used connector.
> + * \param blockp: pointer to address of an raw EDID data block.
> + * \param len: size if block in bytes.
> + *
> + * validate block and return corrected block in \param block.
> + * \return: number of valid extensions or -errno if unsuccessful.
> + */
> +int
> +drm_validate_edid_blob(struct drm_connector *connector, u8 **blockp, int len)
> +{
> + int n_blocks = len / EDID_LENGTH;
> + int valid_extensions = 0, ret = 0;
> + bool print_bad_edid = !connector->bad_edid_counter || (drm_debug &
> DRM_UT_KMS);
> +
> + if (!blockp || !*blockp)
> + ret = -EINVAL;
> + else if (!n_blocks || !drm_edid_block_valid(*blockp, 0,
> print_bad_edid)) {
> + kfree(*blockp);
> + *blockp = NULL;
> + ret = -EINVAL;
> + }
> + if (!ret) {
> + n_blocks--;
> + if ((*blockp)[EDID_EXTENSION_FLAG_OFFSET] < n_blocks)
> + n_blocks = (*blockp)[EDID_EXTENSION_FLAG_OFFSET];
> +
> + while (n_blocks--) {
> + if (drm_edid_block_valid(*blockp + (valid_extensions +
> 1) * EDID_LENGTH,
> + valid_extensions + 1,
> print_bad_edid))
> + valid_extensions++;
> + }
> + ret = fixup_edid(blockp, valid_extensions);
> + }
> + if (ret < 0)
> + connector->bad_edid_counter++;
> + return ret;
> +}
> +
> +/**
> * Probe DDC presence.
> *
> * \param adapter : i2c device adaptor
> diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
> index 38d3943..6541c1f 100644
> --- a/drivers/gpu/drm/drm_edid_load.c
> +++ b/drivers/gpu/drm/drm_edid_load.c
> @@ -119,11 +119,10 @@ static u8 *edid_load(struct drm_connector *connector,
> char *name,
> {
> const struct firmware *fw;
> struct platform_device *pdev;
> - u8 *fwdata = NULL, *edid, *new_edid;
> + u8 *fwdata = NULL;
> + struct edid *edid;
> int fwsize, expected;
> int builtin = 0, err = 0;
> - int i, valid_extensions = 0;
> - bool print_bad_edid = !connector->bad_edid_counter || (drm_debug &
> DRM_UT_KMS);
>
> pdev = platform_device_register_simp
Re: [PATCH 07/17] DRM/KMS/EDID: Move drm_edid_load.o to drm.ko
At Mon, 19 Nov 2012 15:23:08 -0500, "Egbert Eich " <"e...@novell.com> wrote: > > EDIDs are an integral concept of connectors, connectors are a concept > of drm core also drm_edid.o is already part of this drm core. > Overridden, 'firmware-supplied' EDIDs should be treated exactly the > same as device supplied ones. > Therefore move drm_edid_load from the helper level to the drm core. > > Signed-off-by: Egbert Eich You need to fix Kconfig as well. And I think the dependency on CONFIG_FW_LOADER is missing there, too. It should have "select FW_LOADER". Takashi > --- > drivers/gpu/drm/Makefile |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > index 2ff5cef..76ca269 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -16,11 +16,11 @@ drm-y :=drm_auth.o drm_buffer.o drm_bufs.o > drm_cache.o \ > > drm-$(CONFIG_COMPAT) += drm_ioc32.o > drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o > +drm-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o > > drm-usb-y := drm_usb.o > > drm_kms_helper-y := drm_fb_helper.o drm_crtc_helper.o drm_dp_i2c_helper.o > -drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o > drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o > > obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o > -- > 1.7.7 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 10/17] DRM/KMS/EDID: Cache EDID blobs with extensions.
At Mon, 19 Nov 2012 15:23:11 -0500,
"Egbert Eich " <"e...@novell.com> wrote:
>
> According the the VESA specs there can be up to 254 EEDID extension blocks.
> Since we may read the EDID (including extensions) in 10 second intervals to
> probe for display hotplugging (at least in cases where no hardware hotplug
> detection exists) and I2C transfer is rather slow we may end up consuming
> a considerable amount on CPU time for just that.
> This patch caches the EDID block if it contains at least one extension.
> To determine if the blocks match we only tranfer the base block, on a match
> we use the cached data.
>
> Signed-off-by: Egbert Eich
> ---
> drivers/gpu/drm/drm_crtc.c |1 +
> drivers/gpu/drm/drm_edid.c | 43 ++-
> include/drm/drm_crtc.h |1 +
> include/drm/drm_edid.h |1 +
> 4 files changed, 45 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 3533609..e283355 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -598,6 +598,7 @@ void drm_connector_cleanup(struct drm_connector
> *connector)
> drm_mode_remove(connector, mode);
>
> mutex_lock(&dev->mode_config.mutex);
> + drm_cache_edid(connector, NULL);
> drm_mode_object_put(dev, &connector->base);
> list_del(&connector->head);
> dev->mode_config.num_connector--;
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 3e7df61..410a54d 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -431,6 +431,41 @@ fixup_edid(u8 **blockp, int valid_extensions)
> return (new ? valid_extensions : -ENOMEM);
> }
>
> +static bool
> +compare_get_edid_from_cache(struct drm_connector *connector, struct edid
> **edidp)
> +{
> + if (connector->edid_cache &&
> + connector->edid_cache->prod_code[0] == (*edidp)->prod_code[0] &&
> + connector->edid_cache->prod_code[1] == (*edidp)->prod_code[1] &&
> + connector->edid_cache->serial == (*edidp)->serial &&
> + connector->edid_cache->input == (*edidp)->input) {
> + int size = (connector->edid_cache->extensions + 1) *
> EDID_LENGTH;
> + struct edid *new = kmalloc(size, GFP_KERNEL);
> + if (!new)
> + return false;
> + DRM_DEBUG_KMS("Got EDID for %s from
> cache.\n",drm_get_connector_name(connector));
> + memcpy(new, connector->edid_cache, size);
> + kfree(*edidp);
You can use kmemdup().
> + *edidp = new;
> + return true;
> + }
> + return false;
> +}
> +
> +void
> +drm_cache_edid(struct drm_connector *connector, struct edid *edid)
> +{
> + struct edid *new = NULL;
> + kfree(connector->edid_cache);
> + if (edid) {
> + int size = (edid->extensions + 1) * EDID_LENGTH;
> + new = kmalloc(size, GFP_KERNEL);
> + if (new)
> + memcpy(new, edid, size);
Ditto.
Takashi
> + }
> + connector->edid_cache = new;
> +}
> +
> static u8 *
> drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
> {
> @@ -462,10 +497,14 @@ drm_do_get_edid(struct drm_connector *connector, struct
> i2c_adapter *adapter)
> if (i == 4)
> goto carp;
>
> - /* if there's no extensions, we're done */
> + /* if there are no extensions, we're done - don't bother caching */
> if (block[EDID_EXTENSION_FLAG_OFFSET] == 0)
> return block;
>
> + /* see if EDID is in the cache - no need to read all extension blocks */
> + if (compare_get_edid_from_cache(connector, (struct edid **)&block))
> + return block;
> +
> new = krealloc(block, (block[EDID_EXTENSION_FLAG_OFFSET] + 1) *
> EDID_LENGTH, GFP_KERNEL);
> if (!new)
> goto out;
> @@ -505,6 +544,7 @@ drm_do_get_edid(struct drm_connector *connector, struct
> i2c_adapter *adapter)
>
> no_more:
> fixup_edid(&block, valid_extensions);
> + drm_cache_edid(connector, (struct edid *)block);
>
> return block;
>
> @@ -513,6 +553,7 @@ carp:
> dev_warn(connector->dev->dev, "%s: EDID block %d invalid.\n",
>drm_get_connector_name(connector), j);
> }
> + drm_cache_edid(connector, NULL);
> connector->bad_edid_counter++;
>
> out:
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 49dd8c2..6a1054c 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -601,6 +601,7 @@ struct drm_connector {
> struct drm_encoder *encoder; /* currently active encoder */
>
> /* EDID bits */
> + struct edid *edid_cache;
> uint8_t eld[MAX_ELD_BYTES];
> bool dvi_dual;
> int max_tmds_clock; /* in MHz */
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 53c619c..c880510 100644
> --- a/incl
[Bug 56405] Distorted graphics on Radeon HD 6620G
https://bugs.freedesktop.org/show_bug.cgi?id=56405 --- Comment #21 from Michel Dänzer --- (In reply to comment #19) > I have 15 commits left to test. Which ones? > Anyway I'm not even convinced my test is a good test. Because since a while I > only get the r600_dri.so built. And I only exchange this object. FWIW, that's perfectly plausible as you're narrowing down the range of potential culprit commits. This object does contain the bulk of relevant code. -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 13/17] DRM/KMS/EDID: Allow for multiple Connectors when specifying 'firmware'-EDID Files.
At Mon, 19 Nov 2012 15:23:14 -0500,
"Egbert Eich " <"e...@novell.com> wrote:
>
> So far it was only possible to load an EDID for a single connector (unless
> no connector was specified at all in which case the same EDID file was used
> for all).
> This patch extends the EDID loader so that EDID files can be specified for
> more than one connector. A semicolon is used to separate the different
> connector
> sections.
> The option now looks like this:
>
> edid_firmware="[:][;:]..."
>
> Signed-off-by: Egbert Eich
> ---
> drivers/gpu/drm/drm_edid_load.c | 45 ++
> 1 files changed, 31 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
> index 1475c6f..a40a88505 100644
> --- a/drivers/gpu/drm/drm_edid_load.c
> +++ b/drivers/gpu/drm/drm_edid_load.c
> @@ -196,26 +196,43 @@ struct edid *
> drm_load_edid_firmware(struct drm_connector *connector)
> {
> char *connector_name = drm_get_connector_name(connector);
> - char *edidname = edid_firmware, *last, *colon;
> - struct edid *edid;
> + char *edidname, *last, *colon;
> + struct edid *edid = NULL;
> + char *next, *mem;
> + bool many = false;
>
> - if (*edidname == '\0')
> + if (*edid_firmware == '\0')
> return NULL;
>
> - colon = strchr(edidname, ':');
> - if (colon != NULL) {
> - if (strncmp(connector_name, edidname, colon - edidname))
> - return NULL;
> - edidname = colon + 1;
> - if (*edidname == '\0')
> - return NULL;
> + edidname = mem = kstrndup(edid_firmware, PATH_MAX, GFP_KERNEL);
> + do {
Missing NULL check of edidname.
Takashi
> + next = strchr(edidname, ';');
> + if (next)
> + *(next++) = '\0';
> + colon = strchr(edidname, ':');
> + if (colon == NULL) {
> + if (next || many)
> + edidname = NULL;
> + break;
> + } else if (!strncmp(connector_name, edidname, colon -
> edidname)) {
> + edidname = colon + 1;
> + break;
> + }
> + edidname = next;
> + many = true;
> + } while (edidname);
> +
> + if (edidname && *edidname != '\0') {
> + last = edidname + strlen(edidname) - 1;
> + if (*last == '\n')
> + *last = '\0';
> +
> + if (strlen(edidname) > 0)
> + edid = edid_load(connector, edidname, connector_name);
> }
>
> - last = edidname + strlen(edidname) - 1;
> - if (*last == '\n')
> - *last = '\0';
> + kfree(mem);
>
> - edid = edid_load(connector, edidname, connector_name);
> if (IS_ERR_OR_NULL(edid))
> return NULL;
>
> --
> 1.7.7
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/exynos: remove 'pages' and 'page_size' elements in exynos gem buffer
On Tue, Nov 20, 2012 at 12:49 PM, Kyungmin Park wrote:
> On 11/20/12, Prathyush K wrote:
> > On Mon, Nov 19, 2012 at 3:14 PM, Kyungmin Park
> > wrote:
> >
> >> Hi,
> >>
> >> On 11/15/12, Prathyush K wrote:
> >> > The 'pages' structure is not required since we can use the 'sgt'. Even
> >> for
> >> > CONTIG buffers, a SGT is created (which will have just one sgl). This
> >> > SGT
> >> > can be used during mmap instead of 'pages'. The 'page_size' element of
> >> the
> >> > structure is also not used anywhere and is removed.
> >> > This patch also fixes a memory leak where the 'pages' structure was
> >> > being
> >> > allocated during gem buffer allocation but not being freed during
> >> > deallocate.
> >> >
> >> > Signed-off-by: Prathyush K
> >> > ---
> >> > drivers/gpu/drm/exynos/exynos_drm_buf.c| 20 --
> >> > drivers/gpu/drm/exynos/exynos_drm_buf.h|4 +-
> >> > drivers/gpu/drm/exynos/exynos_drm_dmabuf.c |3 +-
> >> > drivers/gpu/drm/exynos/exynos_drm_gem.c| 39
> >> > +++
> >> > drivers/gpu/drm/exynos/exynos_drm_gem.h|4 ---
> >> > 5 files changed, 19 insertions(+), 51 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_buf.c
> >> > b/drivers/gpu/drm/exynos/exynos_drm_buf.c
> >> > index 48c5896..72bf97b 100644
> >> > --- a/drivers/gpu/drm/exynos/exynos_drm_buf.c
> >> > +++ b/drivers/gpu/drm/exynos/exynos_drm_buf.c
> >> > @@ -34,8 +34,6 @@ static int lowlevel_buffer_allocate(struct
> drm_device
> >> > *dev,
> >> > unsigned int flags, struct exynos_drm_gem_buf *buf)
> >> > {
> >> > int ret = 0;
> >> > - unsigned int npages, i = 0;
> >> > - struct scatterlist *sgl;
> >> > enum dma_attr attr = DMA_ATTR_FORCE_CONTIGUOUS;
> >> >
> >> > DRM_DEBUG_KMS("%s\n", __FILE__);
> >> > @@ -73,22 +71,6 @@ static int lowlevel_buffer_allocate(struct
> >> > drm_device
> >> > *dev,
> >> > goto err_free_sgt;
> >> > }
> >> >
> >> > - npages = buf->sgt->nents;
> >> > -
> >> > - buf->pages = kzalloc(sizeof(struct page) * npages, GFP_KERNEL);
> >> > - if (!buf->pages) {
> >> > - DRM_ERROR("failed to allocate pages.\n");
> >> > - ret = -ENOMEM;
> >> > - goto err_free_table;
> >> > - }
> >> > -
> >> > - sgl = buf->sgt->sgl;
> >> > - while (i < npages) {
> >> > - buf->pages[i] = sg_page(sgl);
> >> > - sgl = sg_next(sgl);
> >> > - i++;
> >> > - }
> >> > -
> >> > DRM_DEBUG_KMS("vaddr(0x%lx), dma_addr(0x%lx), size(0x%lx)\n",
> >> > (unsigned long)buf->kvaddr,
> >> > (unsigned long)buf->dma_addr,
> >> > @@ -96,8 +78,6 @@ static int lowlevel_buffer_allocate(struct
> drm_device
> >> > *dev,
> >> >
> >> > return ret;
> >> >
> >> > -err_free_table:
> >> > - sg_free_table(buf->sgt);
> >> > err_free_sgt:
> >> > kfree(buf->sgt);
> >> > buf->sgt = NULL;
> >> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_buf.h
> >> > b/drivers/gpu/drm/exynos/exynos_drm_buf.h
> >> > index 3388e4e..25cf162 100644
> >> > --- a/drivers/gpu/drm/exynos/exynos_drm_buf.h
> >> > +++ b/drivers/gpu/drm/exynos/exynos_drm_buf.h
> >> > @@ -34,12 +34,12 @@ struct exynos_drm_gem_buf
> >> > *exynos_drm_init_buf(struct
> >> > drm_device *dev,
> >> > void exynos_drm_fini_buf(struct drm_device *dev,
> >> > struct exynos_drm_gem_buf *buffer);
> >> >
> >> > -/* allocate physical memory region and setup sgt and pages. */
> >> > +/* allocate physical memory region and setup sgt. */
> >> > int exynos_drm_alloc_buf(struct drm_device *dev,
> >> > struct exynos_drm_gem_buf *buf,
> >> > unsigned int flags);
> >> >
> >> > -/* release physical memory region, sgt and pages. */
> >> > +/* release physical memory region, and sgt. */
> >> > void exynos_drm_free_buf(struct drm_device *dev,
> >> > unsigned int flags,
> >> > struct exynos_drm_gem_buf *buffer);
> >> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
> >> > b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
> >> > index b98da30..615a049 100644
> >> > --- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
> >> > +++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
> >> > @@ -93,8 +93,7 @@ static struct sg_table *
> >> > goto err_unlock;
> >> > }
> >> >
> >> > - DRM_DEBUG_PRIME("buffer size = 0x%lx page_size = 0x%lx\n",
> >> > - buf->size, buf->page_size);
> >> > + DRM_DEBUG_PRIME("buffer size = 0x%lx\n", buf->size);
> >> >
> >> > err_unlock:
> >> > mutex_unlock(&dev->struct_mutex);
> >> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> >> > b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> >> > index 50d73f1..40999ac 100644
> >> > --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> >> > +++ b/drivers/gpu/drm/exynos/exyn
[PATCH v2] drm/exynos: remove 'pages' and 'page_size' elements in exynos gem buffer
Changelog v2:
Removed redundant check for invalid sgl.
Added check for valid page_offset in the beginning of exynos_drm_gem_map_buf.
Changelog v1:
The 'pages' structure is not required since we can use the 'sgt'. Even for
CONTIG buffers, a SGT is created (which will have just one sgl). This SGT
can be used during mmap instead of 'pages'. The 'page_size' element of the
structure is also not used anywhere and is removed.
This patch also fixes a memory leak where the 'pages' structure was being
allocated during gem buffer allocation but not being freed during deallocate.
Signed-off-by: Prathyush K
---
drivers/gpu/drm/exynos/exynos_drm_buf.c| 20 -
drivers/gpu/drm/exynos/exynos_drm_buf.h|4 +-
drivers/gpu/drm/exynos/exynos_drm_dmabuf.c |3 +-
drivers/gpu/drm/exynos/exynos_drm_gem.c| 41 ++-
drivers/gpu/drm/exynos/exynos_drm_gem.h|4 ---
5 files changed, 18 insertions(+), 54 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_buf.c
b/drivers/gpu/drm/exynos/exynos_drm_buf.c
index 48c5896..72bf97b 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_buf.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_buf.c
@@ -34,8 +34,6 @@ static int lowlevel_buffer_allocate(struct drm_device *dev,
unsigned int flags, struct exynos_drm_gem_buf *buf)
{
int ret = 0;
- unsigned int npages, i = 0;
- struct scatterlist *sgl;
enum dma_attr attr = DMA_ATTR_FORCE_CONTIGUOUS;
DRM_DEBUG_KMS("%s\n", __FILE__);
@@ -73,22 +71,6 @@ static int lowlevel_buffer_allocate(struct drm_device *dev,
goto err_free_sgt;
}
- npages = buf->sgt->nents;
-
- buf->pages = kzalloc(sizeof(struct page) * npages, GFP_KERNEL);
- if (!buf->pages) {
- DRM_ERROR("failed to allocate pages.\n");
- ret = -ENOMEM;
- goto err_free_table;
- }
-
- sgl = buf->sgt->sgl;
- while (i < npages) {
- buf->pages[i] = sg_page(sgl);
- sgl = sg_next(sgl);
- i++;
- }
-
DRM_DEBUG_KMS("vaddr(0x%lx), dma_addr(0x%lx), size(0x%lx)\n",
(unsigned long)buf->kvaddr,
(unsigned long)buf->dma_addr,
@@ -96,8 +78,6 @@ static int lowlevel_buffer_allocate(struct drm_device *dev,
return ret;
-err_free_table:
- sg_free_table(buf->sgt);
err_free_sgt:
kfree(buf->sgt);
buf->sgt = NULL;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_buf.h
b/drivers/gpu/drm/exynos/exynos_drm_buf.h
index 3388e4e..25cf162 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_buf.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_buf.h
@@ -34,12 +34,12 @@ struct exynos_drm_gem_buf *exynos_drm_init_buf(struct
drm_device *dev,
void exynos_drm_fini_buf(struct drm_device *dev,
struct exynos_drm_gem_buf *buffer);
-/* allocate physical memory region and setup sgt and pages. */
+/* allocate physical memory region and setup sgt. */
int exynos_drm_alloc_buf(struct drm_device *dev,
struct exynos_drm_gem_buf *buf,
unsigned int flags);
-/* release physical memory region, sgt and pages. */
+/* release physical memory region, and sgt. */
void exynos_drm_free_buf(struct drm_device *dev,
unsigned int flags,
struct exynos_drm_gem_buf *buffer);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
index d9307bd..539da9f 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
@@ -87,8 +87,7 @@ static struct sg_table *
goto err_unlock;
}
- DRM_DEBUG_PRIME("buffer size = 0x%lx page_size = 0x%lx\n",
- buf->size, buf->page_size);
+ DRM_DEBUG_PRIME("buffer size = 0x%lx\n", buf->size);
err_unlock:
mutex_unlock(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c
b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index fdabb0f..7e2727a 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -99,34 +99,23 @@ static int exynos_drm_gem_map_buf(struct drm_gem_object
*obj,
unsigned long pfn;
int i;
- if (exynos_gem_obj->flags & EXYNOS_BO_NONCONTIG) {
- if (!buf->sgt)
- return -EINTR;
-
- sgl = buf->sgt->sgl;
- for_each_sg(buf->sgt->sgl, sgl, buf->sgt->nents, i) {
- if (!sgl) {
- DRM_ERROR("invalid SG table\n");
- return -EINTR;
- }
- if (page_offset < (sgl->length >> PAGE_SHIFT))
- break;
- page_offset -= (sgl->length >> PAGE_SHIFT);
- }
-
-
[PATCH] drm/exynos: fix memory leak: free EDID block
drm_get_edid() returns a pointer to an EDID block. The caller
is responsible to free this pointer itself.
Here the pointer gets assigned to the local variable raw_edid.
Therefore it should be freed before the variable goes out of
scope.
Signed-off-by: Egbert Eich
---
drivers/gpu/drm/exynos/exynos_hdmi.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 2c115f8..bc87bca 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -1293,6 +1293,7 @@ static int hdmi_get_edid(void *ctx, struct drm_connector
*connector,
DRM_DEBUG_KMS("%s : width[%d] x height[%d]\n",
(hdata->dvi_mode ? "dvi monitor" : "hdmi monitor"),
raw_edid->width_cm, raw_edid->height_cm);
+ kfree(raw_edid);
} else {
return -ENODEV;
}
--
1.7.7
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RESEND][PATCH] drm/prime: drop reference on imported dma-buf come from gem
Op 20-11-12 02:03, 김승우 schreef: > Hi Maarten, > > On 2012년 11월 19일 19:27, Maarten Lankhorst wrote: >> Op 15-11-12 04:52, Seung-Woo Kim schreef: >>> Increasing ref counts of both dma-buf and gem for imported dma-buf >>> come from gem makes memory leak. release function of dma-buf cannot >>> be called because f_count of dma-buf increased by importing gem and >>> gem ref count cannot be decrease because of exported dma-buf. >>> >>> So I add dma_buf_put() for imported gem come from its own gem into >>> each drivers having prime_import and prime_export capabilities. With >>> this, only gem ref count is increased if importing gem exported from >>> gem of same driver. >>> >>> Signed-off-by: Seung-Woo Kim >>> Signed-off-by: Kyungmin.park >>> Cc: Inki Dae >>> Cc: Daniel Vetter >>> Cc: Rob Clark >>> Cc: Alex Deucher >>> --- >>> Mmap failiure in i915 was reported by Jani, and I think it was fixed >>> with Dave's commit "drm/prime: add exported buffers to current fprivs >>> imported buffer list (v2)", so I resend this patch. >>> >>> I can send exynos only, but this issue is common in all drm driver >>> having both prime inport and export, IMHO, it's better to send for >>> all drivers. >> I fear that this won't solve the issue completely and keeps open a small >> race. > I don't believe my patch can fix all issue related with gem prime > completely. But it seems that it can solve unrecoverable memory leak > caused by re-importing GEM. Anyway, can you give me an example of other > race issue? When the dma_buf is unreffed and refcount drops to 0, work is queued to free it. Until then export_dma_buf member points to something, but refcount is 0 on it. Importing to itself, then closing fd then re-export from the new place would probably trigger it. >> I don't like the current destruction path either. It really looks like >> export_dma_buf >> should be unset when the exported buffer is closed in the original file. >> Anything else is racy. >> Especially setting export_dma_buf to NULL won't work with the current >> delayed fput semantics. > I cannot figure out all aspect of delayed fput, but until delayed work > is called, dma_buf is not released so export_dma_buf is valid. > Considering this, I can't find possible issue of delayed fput. I'm fairly sure that when refcount drops to 0, even though the memory may not be freed yet, you no longer have a guarantee that the memory is still valid. And of course after importing into a different process with its own drm namespace, how does file_priv->prime.lock still protect serialization? I don't see any hierarchy either. export_dma_buf needs to be set to NULL before the dma_buf_put is called that is used for the reference inside export_dma_buf. The release function should only release a reference to whatever backing memory is used. >> So to me it seems instead we should do something like this: >> >> - dmabuf_release callback is a noop, no ref is held by the dma-buf. >> - attach and detach ops increase reference by 1*. >> >> - when the buffer is exported, export_dma_buf is set by core code and kept >> around >> alive until the gem object is closed. >> >> - dma_buf_put is not called by import function. This reference removed as >> part >> of gem object close instead. > Considering re-import, where drm file priv is different from exporter's > file priv, attach and detach are not called because gem object is reused. > > How do you think remove checking whether dma_buf is came from driver own > exporter? Because without this checking, gem object is newly created, > and then re-import issue will disappear. The whole refcounting is a circular mess, sadly with no easy solution. :/ It seems to me that using gem reference counts is solving this problem at the wrong layer. A secondary type of reference count is needed for the underlying memory backing. For those who use ttm, I would recommend that the dma_buf increases refcount on ttm_bo by one, so that the gem object can be destroyed and release its reference on the dma_buf. WIthout a secondary refcount you end up in a position where you can't reliably and race free unref the gem object and dma_buf at the same time, since they're both independent interfaces with possibly different lifetimes. It would really help if there were hard rules on lifetime of the export_dma_buf member, until then we're just patching one broken thing with another. ~Maarten ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v11 0/6] of: add display helper
On Thu, Nov 15, 2012 at 02:15:06PM +0100, Steffen Trumtrar wrote: > Hi! > > Changes since v10: > - fix function name (drm_)display_mode_from_videomode > - add acked-by, reviewed-by, tested-by > > Regards, > Steffen > > > Steffen Trumtrar (6): > video: add display_timing and videomode > video: add of helper for videomode > fbmon: add videomode helpers > fbmon: add of_videomode helpers > drm_modes: add videomode helpers > drm_modes: add of_videomode helpers > > .../devicetree/bindings/video/display-timings.txt | 107 ++ > drivers/gpu/drm/drm_modes.c| 70 +++ > drivers/video/Kconfig | 19 ++ > drivers/video/Makefile |4 + > drivers/video/display_timing.c | 24 +++ > drivers/video/fbmon.c | 86 > drivers/video/of_display_timing.c | 212 > > drivers/video/of_videomode.c | 47 + > drivers/video/videomode.c | 45 + > include/drm/drmP.h | 12 ++ > include/linux/display_timing.h | 69 +++ > include/linux/fb.h | 12 ++ > include/linux/of_display_timings.h | 20 ++ > include/linux/of_videomode.h | 17 ++ > include/linux/videomode.h | 40 > 15 files changed, 784 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/video/display-timings.txt > create mode 100644 drivers/video/display_timing.c > create mode 100644 drivers/video/of_display_timing.c > create mode 100644 drivers/video/of_videomode.c > create mode 100644 drivers/video/videomode.c > create mode 100644 include/linux/display_timing.h > create mode 100644 include/linux/of_display_timings.h > create mode 100644 include/linux/of_videomode.h > create mode 100644 include/linux/videomode.h > > -- > > > Ping! Any comments or taker for v11? Errors are fixed, driver is tested and working, DT binding got two ACKs. So, the series is finished as far as I can tell. As I'm not sure if I reached the right maintainers with the current CC, I did another get_maintainers and added Florian Schandinat and David Airlie to the list. If I need to resend the series, please tell. Regards, Steffen -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v11 0/6] of: add display helper
Hi Steffen, On Tuesday 20 November 2012 11:39:18 Steffen Trumtrar wrote: > On Thu, Nov 15, 2012 at 02:15:06PM +0100, Steffen Trumtrar wrote: > > Hi! > > > > Changes since v10: > > - fix function name (drm_)display_mode_from_videomode > > - add acked-by, reviewed-by, tested-by > > > > Regards, > > Steffen > > > > Steffen Trumtrar (6): > > video: add display_timing and videomode > > video: add of helper for videomode > > fbmon: add videomode helpers > > fbmon: add of_videomode helpers > > drm_modes: add videomode helpers > > drm_modes: add of_videomode helpers > > > > .../devicetree/bindings/video/display-timings.txt | 107 ++ > > drivers/gpu/drm/drm_modes.c| 70 +++ > > drivers/video/Kconfig | 19 ++ > > drivers/video/Makefile |4 + > > drivers/video/display_timing.c | 24 +++ > > drivers/video/fbmon.c | 86 > > drivers/video/of_display_timing.c | 212 +++ > > drivers/video/of_videomode.c | 47 + > > drivers/video/videomode.c | 45 + > > include/drm/drmP.h | 12 ++ > > include/linux/display_timing.h | 69 +++ > > include/linux/fb.h | 12 ++ > > include/linux/of_display_timings.h | 20 ++ > > include/linux/of_videomode.h | 17 ++ > > include/linux/videomode.h | 40 > > 15 files changed, 784 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/video/display-timings.txt create mode > > 100644 drivers/video/display_timing.c > > create mode 100644 drivers/video/of_display_timing.c > > create mode 100644 drivers/video/of_videomode.c > > create mode 100644 drivers/video/videomode.c > > create mode 100644 include/linux/display_timing.h > > create mode 100644 include/linux/of_display_timings.h > > create mode 100644 include/linux/of_videomode.h > > create mode 100644 include/linux/videomode.h > > Ping! > > Any comments or taker for v11? Errors are fixed, driver is tested and > working, DT binding got two ACKs. So, the series is finished as far as I > can tell. Just one last comment, sorry for not bringing this up earlier. Could you constify pointer arguments to functions where applicable ? For instance int videomode_from_timing(struct display_timings *disp, struct videomode *vm, unsigned int index); The function shouldn't modify disp, so you can make it const. Same for most of the API functions. You can then add my Reviewed-by: Laurent Pinchart > As I'm not sure if I reached the right maintainers with the current CC, I > did another get_maintainers and added Florian Schandinat and David Airlie > to the list. If I need to resend the series, please tell. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v11 0/6] of: add display helper
On Tue, Nov 20, 2012 at 11:52:06AM +0100, Laurent Pinchart wrote: > Hi Steffen, > > On Tuesday 20 November 2012 11:39:18 Steffen Trumtrar wrote: > > On Thu, Nov 15, 2012 at 02:15:06PM +0100, Steffen Trumtrar wrote: > > > Hi! > > > > > > Changes since v10: > > > - fix function name (drm_)display_mode_from_videomode > > > - add acked-by, reviewed-by, tested-by > > > > > > Regards, > > > Steffen > > > > > > Steffen Trumtrar (6): > > > video: add display_timing and videomode > > > video: add of helper for videomode > > > fbmon: add videomode helpers > > > fbmon: add of_videomode helpers > > > drm_modes: add videomode helpers > > > drm_modes: add of_videomode helpers > > > > > > .../devicetree/bindings/video/display-timings.txt | 107 ++ > > > drivers/gpu/drm/drm_modes.c| 70 +++ > > > drivers/video/Kconfig | 19 ++ > > > drivers/video/Makefile |4 + > > > drivers/video/display_timing.c | 24 +++ > > > drivers/video/fbmon.c | 86 > > > drivers/video/of_display_timing.c | 212 +++ > > > drivers/video/of_videomode.c | 47 + > > > drivers/video/videomode.c | 45 + > > > include/drm/drmP.h | 12 ++ > > > include/linux/display_timing.h | 69 +++ > > > include/linux/fb.h | 12 ++ > > > include/linux/of_display_timings.h | 20 ++ > > > include/linux/of_videomode.h | 17 ++ > > > include/linux/videomode.h | 40 > > > 15 files changed, 784 insertions(+) > > > create mode 100644 > > > Documentation/devicetree/bindings/video/display-timings.txt create mode > > > 100644 drivers/video/display_timing.c > > > create mode 100644 drivers/video/of_display_timing.c > > > create mode 100644 drivers/video/of_videomode.c > > > create mode 100644 drivers/video/videomode.c > > > create mode 100644 include/linux/display_timing.h > > > create mode 100644 include/linux/of_display_timings.h > > > create mode 100644 include/linux/of_videomode.h > > > create mode 100644 include/linux/videomode.h > > > > Ping! > > > > Any comments or taker for v11? Errors are fixed, driver is tested and > > working, DT binding got two ACKs. So, the series is finished as far as I > > can tell. > > Just one last comment, sorry for not bringing this up earlier. Could you > constify pointer arguments to functions where applicable ? For instance > > int videomode_from_timing(struct display_timings *disp, struct videomode *vm, > unsigned int index); > > The function shouldn't modify disp, so you can make it const. Same for most > of > the API functions. > Okay. Will do. > You can then add my > > Reviewed-by: Laurent Pinchart > Thanks. Regards, Steffen -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/10] drm/ttm: change fence_lock to inner lock, v3
Op 20-11-12 08:48, Thomas Hellstrom schreef: > On 11/19/2012 04:33 PM, Maarten Lankhorst wrote: >> Op 19-11-12 16:04, Thomas Hellstrom schreef: >>> On 11/19/2012 03:17 PM, Thomas Hellstrom wrote: Hi, This patch looks mostly good, although I think ttm_bo_cleanup_refs becomes overly complicated: Could this do, or am I missing something? >>> Actually, my version is bad, because ttm_bo_wait() is called with the lru >>> lock held. >>> >>> /Thomas >> Oh digging through it made me remember why I had to release the reservation >> early and >> had to allow move_notify to be called without reservation. >> >> Fortunately move_notify has a NULL parameter, which is the only time that >> happens, >> so you can still check do BUG_ON(mem != NULL && !ttm_bo_reserved(bo)); in >> your >> move_notify handler. >> >> 05/10 removed the loop and assumed no new fence could be attached after the >> driver has >> declared the bo dead. >> >> However, at that point it may no longer hold a reservation to confirm this, >> that's why >> I moved the cleanup to be done in the release_list handler. It could still >> be done in >> ttm_bo_release, but we no longer have a reservation after we waited. Getting >> a reservation can fail if the bo is imported for example. >> >> While it would be true that in that case a new fence may be attached as >> well, that >> would be less harmful since that operation wouldn't involve this device, so >> the >> ttm bo can still be removed in that case. When that time comes I should >> probably >> fix up that WARN_ON(ret) in ttm_bo_cleanup_refs. :-) >> >> I did add a WARN_ON(!atomic_read(&bo->kref.refcount)); to >> ttm_bo_reserve and ttm_eu_reserve_buffers to be sure nothing is done on the >> device >> itself. If that is too paranoid, those WARN_ON's could be dropped. I prefer >> to leave them >> in for a kernel release or 2. But according to the rules that would be the >> only time you >> could attach a new fence and trigger the WARN_ON for now.. > > Hmm, I'd appreciate if you could group patches with functional changes that > depend on eachother togeteher, > and "this is done because ...", which makes it much easier to review, (and to > follow the commit history in case > something goes terribly wrong and we need to revert). > > Meanwhile I'll take a look at the final ttm_bo.c and see if I can spot any > culprits. > > In general, as long as a bo is on a LRU list, we must be able to attach > fences because of accelerated eviction. I thought it was deliberately designed in such a way that it was kept on the lru list, but since it's also on the ddestroy list it won't start accelerated eviction, since it branches into cleanup_refs early, and lru_lock still protects all the list entries. Of course any previous acceleration may still happen, but since we take a reservation first before waiting, we're already sure that any previous acceleration command has finished fencing, and no new one can start since it appears on the ddestroy list which would force it to perform the same wait. The wait is legal, and no new fences can be attached. I do agree all those patches probably needs a lot longer commit message to explain it though. :-) ~Maarten ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/2] drm: exynos: hdmi: sending AVI and AUI info frames
Hi All, Kindly review the following patch-set. regards, Rahul Sharma On Fri, Nov 9, 2012 at 9:51 PM, Rahul Sharma wrote: > This patch set adds provision for composing and sending AVI and AUI > infoframes by exynos drm hdmi driver. > > It also adds provision to get CEA Video ID Code through the display mode > which is required for making AVI infoframe. > > Based on exynos-drm-fixes branch of > git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git > > Rahul Sharma (1): > drm: exynos: compose and send avi and aui info frames > > Stephane Marchesin (1): > drm: get cea video id code for a given display mode > > drivers/gpu/drm/drm_edid.c | 20 +++ > drivers/gpu/drm/exynos/exynos_hdmi.c | 97 > +- > drivers/gpu/drm/exynos/exynos_hdmi.h | 23 > drivers/gpu/drm/exynos/regs-hdmi.h | 17 +- > include/drm/drm_crtc.h |1 + > 5 files changed, 154 insertions(+), 4 deletions(-) > ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/10] drm/ttm: change fence_lock to inner lock, v3
Op 20-11-12 12:33, Maarten Lankhorst schreef: > Op 20-11-12 08:48, Thomas Hellstrom schreef: >> On 11/19/2012 04:33 PM, Maarten Lankhorst wrote: >>> Op 19-11-12 16:04, Thomas Hellstrom schreef: On 11/19/2012 03:17 PM, Thomas Hellstrom wrote: > Hi, > > This patch looks mostly good, although I think ttm_bo_cleanup_refs > becomes overly complicated: > Could this do, or am I missing something? > Actually, my version is bad, because ttm_bo_wait() is called with the lru lock held. /Thomas >>> Oh digging through it made me remember why I had to release the reservation >>> early and >>> had to allow move_notify to be called without reservation. >>> >>> Fortunately move_notify has a NULL parameter, which is the only time that >>> happens, >>> so you can still check do BUG_ON(mem != NULL && !ttm_bo_reserved(bo)); in >>> your >>> move_notify handler. >>> >>> 05/10 removed the loop and assumed no new fence could be attached after the >>> driver has >>> declared the bo dead. >>> >>> However, at that point it may no longer hold a reservation to confirm this, >>> that's why >>> I moved the cleanup to be done in the release_list handler. It could still >>> be done in >>> ttm_bo_release, but we no longer have a reservation after we waited. Getting >>> a reservation can fail if the bo is imported for example. >>> >>> While it would be true that in that case a new fence may be attached as >>> well, that >>> would be less harmful since that operation wouldn't involve this device, so >>> the >>> ttm bo can still be removed in that case. When that time comes I should >>> probably >>> fix up that WARN_ON(ret) in ttm_bo_cleanup_refs. :-) >>> >>> I did add a WARN_ON(!atomic_read(&bo->kref.refcount)); to >>> ttm_bo_reserve and ttm_eu_reserve_buffers to be sure nothing is done on the >>> device >>> itself. If that is too paranoid, those WARN_ON's could be dropped. I prefer >>> to leave them >>> in for a kernel release or 2. But according to the rules that would be the >>> only time you >>> could attach a new fence and trigger the WARN_ON for now.. >> Hmm, I'd appreciate if you could group patches with functional changes that >> depend on eachother togeteher, >> and "this is done because ...", which makes it much easier to review, (and >> to follow the commit history in case >> something goes terribly wrong and we need to revert). >> >> Meanwhile I'll take a look at the final ttm_bo.c and see if I can spot any >> culprits. >> >> In general, as long as a bo is on a LRU list, we must be able to attach >> fences because of accelerated eviction. > I thought it was deliberately designed in such a way that it was kept on the > lru list, > but since it's also on the ddestroy list it won't start accelerated eviction, > since it branches into cleanup_refs early, and lru_lock still protects all > the list entries. > > Of course any previous acceleration may still happen, but since we take a > reservation first before waiting, > we're already sure that any previous acceleration command has finished > fencing, and no new one can > start since it appears on the ddestroy list which would force it to perform > the same wait. > > The wait is legal, and no new fences can be attached. > > I do agree all those patches probably needs a lot longer commit message to > explain it though. :-) > Or maybe an alternative patch.. We could move the checks. There are only 2 places that are allowed to hold reservations at that point right? ttm_bo_swapout and evict_mem_first. If cleanup_refs_or_queue fails because reservation fails, it must mean it's in one of those 2 places. If it succeeds, we can remove it from the lru list and swap list, and if wait fails move it to ddestroy list. unreserve in swapout doesn't add it back to any lists. No special handling needed there. unreserve in evict_mem_first does, but we could take the lock before unreserve, and only re-add it to the swap/lru list when it's not on ddestroy. That way we wouldn't need to call ttm_bo_cleanup_refs from multiple places, and the cleanup would only ever need to be done in the ttm_bo_delayed_delete without race. I thought it was a feature that it still appeared on the lru list after death, so evict_mem_first could wait on it, but if it's an annoyance it could be easily fixed like that. But even if it's a feature to be preserved, evict_mem_first and swapout could be modified to check the ddestroy list first for buffers to destroy. In that case those functions would explicitly prefer waiting for destruction of bo's before queueing new work to swapout or evict bo's. ~Maarten ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/10] drm/ttm: change fence_lock to inner lock, v3
On 11/20/2012 12:33 PM, Maarten Lankhorst wrote: Op 20-11-12 08:48, Thomas Hellstrom schreef: On 11/19/2012 04:33 PM, Maarten Lankhorst wrote: Op 19-11-12 16:04, Thomas Hellstrom schreef: On 11/19/2012 03:17 PM, Thomas Hellstrom wrote: Hi, This patch looks mostly good, although I think ttm_bo_cleanup_refs becomes overly complicated: Could this do, or am I missing something? Actually, my version is bad, because ttm_bo_wait() is called with the lru lock held. /Thomas Oh digging through it made me remember why I had to release the reservation early and had to allow move_notify to be called without reservation. Fortunately move_notify has a NULL parameter, which is the only time that happens, so you can still check do BUG_ON(mem != NULL && !ttm_bo_reserved(bo)); in your move_notify handler. 05/10 removed the loop and assumed no new fence could be attached after the driver has declared the bo dead. However, at that point it may no longer hold a reservation to confirm this, that's why I moved the cleanup to be done in the release_list handler. It could still be done in ttm_bo_release, but we no longer have a reservation after we waited. Getting a reservation can fail if the bo is imported for example. While it would be true that in that case a new fence may be attached as well, that would be less harmful since that operation wouldn't involve this device, so the ttm bo can still be removed in that case. When that time comes I should probably fix up that WARN_ON(ret) in ttm_bo_cleanup_refs. :-) I did add a WARN_ON(!atomic_read(&bo->kref.refcount)); to ttm_bo_reserve and ttm_eu_reserve_buffers to be sure nothing is done on the device itself. If that is too paranoid, those WARN_ON's could be dropped. I prefer to leave them in for a kernel release or 2. But according to the rules that would be the only time you could attach a new fence and trigger the WARN_ON for now.. Hmm, I'd appreciate if you could group patches with functional changes that depend on eachother togeteher, and "this is done because ...", which makes it much easier to review, (and to follow the commit history in case something goes terribly wrong and we need to revert). Meanwhile I'll take a look at the final ttm_bo.c and see if I can spot any culprits. In general, as long as a bo is on a LRU list, we must be able to attach fences because of accelerated eviction. I thought it was deliberately designed in such a way that it was kept on the lru list, but since it's also on the ddestroy list it won't start accelerated eviction, since it branches into cleanup_refs early, and lru_lock still protects all the list entries. I used bad wording. I meant that unbinding might be accelerated, but currently (quite inefficiently) do synchronized unbinding, assuming that only the CPU can do that. When we start to support unsynchronized moves, we need to be able to attach fences at least at the last move_notify(bo, NULL); /Thomas ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH -next 0/5] drm/ttm: Get rid of a number of atomic read-modify-write ops addons
Changes that didn't make it into the drm/ttm: Get rid of a number of atomic read-modify-write ops v3 patch series. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/5] drm: Add a hash-tab rcu-safe API
While hashtab should now be RCU-safe, Add a drm_ht_xxx_api for consumers
to use to make it obvious what locking mechanism is used.
Document the way the rcu-safe interface should be used.
Don't use rcu-safe list traversal in modify operations where we should use
a spinlock / mutex anyway.
Signed-off-by: Thomas Hellstrom
---
drivers/gpu/drm/drm_hashtab.c | 26 ++
include/drm/drm_hashtab.h | 14 ++
2 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_hashtab.c b/drivers/gpu/drm/drm_hashtab.c
index 5729e39..8025454 100644
--- a/drivers/gpu/drm/drm_hashtab.c
+++ b/drivers/gpu/drm/drm_hashtab.c
@@ -67,7 +67,7 @@ void drm_ht_verbose_list(struct drm_open_hash *ht, unsigned
long key)
hashed_key = hash_long(key, ht->order);
DRM_DEBUG("Key is 0x%08lx, Hashed key is 0x%08x\n", key, hashed_key);
h_list = &ht->table[hashed_key];
- hlist_for_each_entry_rcu(entry, list, h_list, head)
+ hlist_for_each_entry(entry, list, h_list, head)
DRM_DEBUG("count %d, key: 0x%08lx\n", count++, entry->key);
}
@@ -81,7 +81,7 @@ static struct hlist_node *drm_ht_find_key(struct
drm_open_hash *ht,
hashed_key = hash_long(key, ht->order);
h_list = &ht->table[hashed_key];
- hlist_for_each_entry_rcu(entry, list, h_list, head) {
+ hlist_for_each_entry(entry, list, h_list, head) {
if (entry->key == key)
return list;
if (entry->key > key)
@@ -90,6 +90,24 @@ static struct hlist_node *drm_ht_find_key(struct
drm_open_hash *ht,
return NULL;
}
+static struct hlist_node *drm_ht_find_key_rcu(struct drm_open_hash *ht,
+ unsigned long key)
+{
+ struct drm_hash_item *entry;
+ struct hlist_head *h_list;
+ struct hlist_node *list;
+ unsigned int hashed_key;
+
+ hashed_key = hash_long(key, ht->order);
+ h_list = &ht->table[hashed_key];
+ hlist_for_each_entry_rcu(entry, list, h_list, head) {
+ if (entry->key == key)
+ return list;
+ if (entry->key > key)
+ break;
+ }
+ return NULL;
+}
int drm_ht_insert_item(struct drm_open_hash *ht, struct drm_hash_item *item)
{
@@ -102,7 +120,7 @@ int drm_ht_insert_item(struct drm_open_hash *ht, struct
drm_hash_item *item)
hashed_key = hash_long(key, ht->order);
h_list = &ht->table[hashed_key];
parent = NULL;
- hlist_for_each_entry_rcu(entry, list, h_list, head) {
+ hlist_for_each_entry(entry, list, h_list, head) {
if (entry->key == key)
return -EINVAL;
if (entry->key > key)
@@ -152,7 +170,7 @@ int drm_ht_find_item(struct drm_open_hash *ht, unsigned
long key,
{
struct hlist_node *list;
- list = drm_ht_find_key(ht, key);
+ list = drm_ht_find_key_rcu(ht, key);
if (!list)
return -EINVAL;
diff --git a/include/drm/drm_hashtab.h b/include/drm/drm_hashtab.h
index 3650d5d..fce2ef3 100644
--- a/include/drm/drm_hashtab.h
+++ b/include/drm/drm_hashtab.h
@@ -61,5 +61,19 @@ extern int drm_ht_remove_key(struct drm_open_hash *ht,
unsigned long key);
extern int drm_ht_remove_item(struct drm_open_hash *ht, struct drm_hash_item
*item);
extern void drm_ht_remove(struct drm_open_hash *ht);
+/*
+ * RCU-safe interface
+ *
+ * The user of this API needs to make sure that two or more instances of the
+ * hash table manipulation functions are never run simultaneously.
+ * The lookup function drm_ht_find_item_rcu may, however, run simultaneously
+ * with any of the manipulation functions as long as it's called from within
+ * an RCU read-locked section.
+ */
+#define drm_ht_insert_item_rcu drm_ht_insert_item
+#define drm_ht_just_insert_please_rcu drm_ht_just_insert_please
+#define drm_ht_remove_key_rcu drm_ht_remove_key
+#define drm_ht_remove_item_rcu drm_ht_remove_item
+#define drm_ht_find_item_rcu drm_ht_find_item
#endif
--
1.7.4.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/5] kref: Add kref_get_unless_zero documentation
Document how kref_get_unless_zero should be used and how it helps
solve a typical kref / locking problem.
Signed-off-by: Thomas Hellstrom
---
Documentation/kref.txt | 88
1 files changed, 88 insertions(+), 0 deletions(-)
diff --git a/Documentation/kref.txt b/Documentation/kref.txt
index 48ba715..ddf85a5 100644
--- a/Documentation/kref.txt
+++ b/Documentation/kref.txt
@@ -213,3 +213,91 @@ presentation on krefs, which can be found at:
and:
http://www.kroah.com/linux/talks/ols_2004_kref_talk/
+
+The above example could also be optimized using kref_get_unless_zero() in
+the following way:
+
+static struct my_data *get_entry()
+{
+ struct my_data *entry = NULL;
+ mutex_lock(&mutex);
+ if (!list_empty(&q)) {
+ entry = container_of(q.next, struct my_data, link);
+ if (!kref_get_unless_zero(&entry->refcount))
+ entry = NULL;
+ }
+ mutex_unlock(&mutex);
+ return entry;
+}
+
+static void release_entry(struct kref *ref)
+{
+ struct my_data *entry = container_of(ref, struct my_data, refcount);
+
+ mutex_lock(&mutex);
+ list_del(&entry->link);
+ mutex_unlock(&mutex);
+ kfree(entry);
+}
+
+static void put_entry(struct my_data *entry)
+{
+ kref_put(&entry->refcount, release_entry);
+}
+
+Which is useful to remove the mutex lock around kref_put() in put_entry(), but
+it's important that kref_get_unless_zero is enclosed in the same critical
+section that finds the entry in the lookup table,
+otherwise kref_get_unless_zero may reference already freed memory.
+Note that it is illegal to use kref_get_unless_zero without checking its
+return value. If you are sure (by already having a valid pointer) that
+kref_get_unless_zero() will return true, then use kref_get() instead.
+
+The function kref_get_unless_zero also makes it possible to use rcu
+locking for lookups in the above example:
+
+struct my_data
+{
+ struct rcu_head rhead;
+ .
+ struct kref refcount;
+ .
+ .
+};
+
+static struct my_data *get_entry_rcu()
+{
+ struct my_data *entry = NULL;
+ rcu_read_lock();
+ if (!list_empty(&q)) {
+ entry = container_of(q.next, struct my_data, link);
+ if (!kref_get_unless_zero(&entry->refcount))
+ entry = NULL;
+ }
+ rcu_read_unlock();
+ return entry;
+}
+
+static void release_entry_rcu(struct kref *ref)
+{
+ struct my_data *entry = container_of(ref, struct my_data, refcount);
+
+ mutex_lock(&mutex);
+ list_del_rcu(&entry->link);
+ mutex_unlock(&mutex);
+ kfree_rcu(entry, rhead);
+}
+
+static void put_entry(struct my_data *entry)
+{
+ kref_put(&entry->refcount, release_entry_rcu);
+}
+
+But note that the struct kref member needs to remain in valid memory for a
+rcu grace period after release_entry_rcu was called. That can be accomplished
+by using kfree_rcu(entry, rhead) as done above, or by calling synchronize_rcu()
+before using kfree, but note that synchronize_rcu() may sleep for a
+substantial amount of time.
+
+
+Thomas Hellstrom
--
1.7.4.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/5] drm/vmwgfx: Free user-space fence objects correctly
They need to be freed after an rcu grace period. Signed-off-by: Thomas Hellstrom --- drivers/gpu/drm/vmwgfx/vmwgfx_fence.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c index bc187fa..c62d20e 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c @@ -537,7 +537,7 @@ static void vmw_user_fence_destroy(struct vmw_fence_obj *fence) container_of(fence, struct vmw_user_fence, fence); struct vmw_fence_manager *fman = fence->fman; - kfree(ufence); + ttm_base_object_kfree(ufence, base); /* * Free kernel space accounting. */ -- 1.7.4.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/5] drm/ttm: Use the hashtab _rcu interface for ttm_objects
Also move a kref_init() out of spinlocked region
Signed-off-by: Thomas Hellstrom
---
drivers/gpu/drm/ttm/ttm_object.c | 21 ++---
1 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_object.c b/drivers/gpu/drm/ttm/ttm_object.c
index 0e13d9f..58a5f32 100644
--- a/drivers/gpu/drm/ttm/ttm_object.c
+++ b/drivers/gpu/drm/ttm/ttm_object.c
@@ -157,11 +157,11 @@ int ttm_base_object_init(struct ttm_object_file *tfile,
base->refcount_release = refcount_release;
base->ref_obj_release = ref_obj_release;
base->object_type = object_type;
- spin_lock(&tdev->object_lock);
kref_init(&base->refcount);
- ret = drm_ht_just_insert_please(&tdev->object_hash,
- &base->hash,
- (unsigned long)base, 31, 0, 0);
+ spin_lock(&tdev->object_lock);
+ ret = drm_ht_just_insert_please_rcu(&tdev->object_hash,
+ &base->hash,
+ (unsigned long)base, 31, 0, 0);
spin_unlock(&tdev->object_lock);
if (unlikely(ret != 0))
goto out_err0;
@@ -175,7 +175,7 @@ int ttm_base_object_init(struct ttm_object_file *tfile,
return 0;
out_err1:
spin_lock(&tdev->object_lock);
- (void)drm_ht_remove_item(&tdev->object_hash, &base->hash);
+ (void)drm_ht_remove_item_rcu(&tdev->object_hash, &base->hash);
spin_unlock(&tdev->object_lock);
out_err0:
return ret;
@@ -189,8 +189,15 @@ static void ttm_release_base(struct kref *kref)
struct ttm_object_device *tdev = base->tfile->tdev;
spin_lock(&tdev->object_lock);
- (void)drm_ht_remove_item(&tdev->object_hash, &base->hash);
+ (void)drm_ht_remove_item_rcu(&tdev->object_hash, &base->hash);
spin_unlock(&tdev->object_lock);
+
+ /*
+* Note: We don't use synchronize_rcu() here because it's far
+* too slow. It's up to the user to free the object using
+* call_rcu() or ttm_base_object_kfree().
+*/
+
if (base->refcount_release) {
ttm_object_file_unref(&base->tfile);
base->refcount_release(&base);
@@ -216,7 +223,7 @@ struct ttm_base_object *ttm_base_object_lookup(struct
ttm_object_file *tfile,
int ret;
rcu_read_lock();
- ret = drm_ht_find_item(&tdev->object_hash, key, &hash);
+ ret = drm_ht_find_item_rcu(&tdev->object_hash, key, &hash);
if (likely(ret == 0)) {
base = drm_hash_entry(hash, struct ttm_base_object, hash);
--
1.7.4.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH -next 0/3] vmwgfx stuff for -next rebased
The last three patches from that patch series that didn't apply. It looks like they conflicted with the TTM removal of sync_obj_arg. /Thomas ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] drm/vmwgfx: Add and make use of a header for surface size calculation.
Signed-off-by: Thomas Hellstrom
Reviewed-by: Brian Paul
Reviewed-by: Dmitry Torokhov
---
drivers/gpu/drm/vmwgfx/svga3d_surfacedefs.h | 909 +++
drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 113 +---
2 files changed, 928 insertions(+), 94 deletions(-)
create mode 100644 drivers/gpu/drm/vmwgfx/svga3d_surfacedefs.h
diff --git a/drivers/gpu/drm/vmwgfx/svga3d_surfacedefs.h
b/drivers/gpu/drm/vmwgfx/svga3d_surfacedefs.h
new file mode 100644
index 000..8369c3b
--- /dev/null
+++ b/drivers/gpu/drm/vmwgfx/svga3d_surfacedefs.h
@@ -0,0 +1,909 @@
+/**
+ *
+ * Copyright © 2008-2012 VMware, Inc., Palo Alto, CA., USA
+ * All Rights Reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sub license, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
+ * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+ * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+ * USE OR OTHER DEALINGS IN THE SOFTWARE.
+ *
+ **/
+
+#ifdef __KERNEL__
+
+#include
+#define surf_size_struct struct drm_vmw_size
+
+#else /* __KERNEL__ */
+
+#ifndef ARRAY_SIZE
+#define ARRAY_SIZE(_A) (sizeof(_A) / sizeof((_A)[0]))
+#endif /* ARRAY_SIZE */
+
+#define DIV_ROUND_UP(x, y) (((x) + (y) - 1) / (y))
+#define max_t(type, x, y) ((x) > (y) ? (x) : (y))
+#define surf_size_struct SVGA3dSize
+#define u32 uint32
+
+#endif /* __KERNEL__ */
+
+#include "svga3d_reg.h"
+
+/*
+ * enum svga3d_block_desc describes the active data channels in a block.
+ *
+ * There can be at-most four active channels in a block:
+ *1. Red, bump W, luminance and depth are stored in the first channel.
+ *2. Green, bump V and stencil are stored in the second channel.
+ *3. Blue and bump U are stored in the third channel.
+ *4. Alpha and bump Q are stored in the fourth channel.
+ *
+ * Block channels can be used to store compressed and buffer data:
+ *1. For compressed formats, only the data channel is used and its size
+ * is equal to that of a singular block in the compression scheme.
+ *2. For buffer formats, only the data channel is used and its size is
+ * exactly one byte in length.
+ *3. In each case the bit depth represent the size of a singular block.
+ *
+ * Note: Compressed and IEEE formats do not use the bitMask structure.
+ */
+
+enum svga3d_block_desc {
+ SVGA3DBLOCKDESC_NONE= 0, /* No channels are active */
+ SVGA3DBLOCKDESC_BLUE= 1 << 0,/* Block with red channel
+ data */
+ SVGA3DBLOCKDESC_U = 1 << 0,/* Block with bump U channel
+ data */
+ SVGA3DBLOCKDESC_UV_VIDEO= 1 << 7,/* Block with alternating video
+ U and V */
+ SVGA3DBLOCKDESC_GREEN = 1 << 1,/* Block with green channel
+ data */
+ SVGA3DBLOCKDESC_V = 1 << 1,/* Block with bump V channel
+ data */
+ SVGA3DBLOCKDESC_STENCIL = 1 << 1,/* Block with a stencil
+ channel */
+ SVGA3DBLOCKDESC_RED = 1 << 2,/* Block with blue channel
+ data */
+ SVGA3DBLOCKDESC_W = 1 << 2,/* Block with bump W channel
+ data */
+ SVGA3DBLOCKDESC_LUMINANCE = 1 << 2,/* Block with luminance channel
+ data */
+ SVGA3DBLOCKDESC_Y = 1 << 2,/* Block with video luminance
+ data */
+ SVGA3DBLOCKDESC_DEPTH = 1 << 2,/* Block with depth channel */
+ SVGA3DBLOCKDESC_ALPHA = 1 << 3,/* Block with an alpha
+
[PATCH -next] drm/ttm: Optimize vm locking using kref_get_unless_zero
Removes the need for a write lock each time we call ttm_bo_unref().
Signed-off-by: Thomas Hellstrom
---
drivers/gpu/drm/ttm/ttm_bo.c|4 +---
drivers/gpu/drm/ttm/ttm_bo_vm.c |4 ++--
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 7426fe5..2dd0238 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -696,6 +696,7 @@ static void ttm_bo_release(struct kref *kref)
struct ttm_bo_device *bdev = bo->bdev;
struct ttm_mem_type_manager *man = &bdev->man[bo->mem.mem_type];
+ write_lock(&bdev->vm_lock);
if (likely(bo->vm_node != NULL)) {
rb_erase(&bo->vm_rb, &bdev->addr_space_rb);
drm_mm_put_block(bo->vm_node);
@@ -707,7 +708,6 @@ static void ttm_bo_release(struct kref *kref)
ttm_mem_io_unlock(man);
ttm_bo_cleanup_refs_or_queue(bo);
kref_put(&bo->list_kref, ttm_bo_release_list);
- write_lock(&bdev->vm_lock);
}
void ttm_bo_unref(struct ttm_buffer_object **p_bo)
@@ -716,9 +716,7 @@ void ttm_bo_unref(struct ttm_buffer_object **p_bo)
struct ttm_bo_device *bdev = bo->bdev;
*p_bo = NULL;
- write_lock(&bdev->vm_lock);
kref_put(&bo->kref, ttm_bo_release);
- write_unlock(&bdev->vm_lock);
}
EXPORT_SYMBOL(ttm_bo_unref);
diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 3ba72db..74705f3 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -259,8 +259,8 @@ int ttm_bo_mmap(struct file *filp, struct vm_area_struct
*vma,
read_lock(&bdev->vm_lock);
bo = ttm_bo_vm_lookup_rb(bdev, vma->vm_pgoff,
(vma->vm_end - vma->vm_start) >> PAGE_SHIFT);
- if (likely(bo != NULL))
- ttm_bo_reference(bo);
+ if (likely(bo != NULL) && !kref_get_unless_zero(&bo->kref))
+ bo = NULL;
read_unlock(&bdev->vm_lock);
if (unlikely(bo == NULL)) {
--
1.7.4.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
