[resend PATCH] drm/nouveau: unpin buffers before releasing to prevent lockdep warnings
This will otherwise cause a lockdep splat if reservations were a real lock
type, so warn when nouveau forgets to unpin a buffer, and fix up the ones I've
hit.
Signed-off-by: Maarten Lankhorst
---
diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.c
b/drivers/gpu/drm/nouveau/nouveau_abi16.c
index cc79c79..acc6b08 100644
--- a/drivers/gpu/drm/nouveau/nouveau_abi16.c
+++ b/drivers/gpu/drm/nouveau/nouveau_abi16.c
@@ -123,6 +123,7 @@ nouveau_abi16_chan_fini(struct nouveau_abi16 *abi16,
if (chan->ntfy) {
nouveau_bo_vma_del(chan->ntfy, &chan->ntfy_vma);
+ nouveau_bo_unpin(chan->ntfy);
drm_gem_object_unreference_unlocked(chan->ntfy->gem);
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index 67a1a06..f337976 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -429,6 +429,7 @@ nouveau_fbcon_destroy(struct drm_device *dev, struct
nouveau_fbdev *fbcon)
if (nouveau_fb->nvbo) {
nouveau_bo_unmap(nouveau_fb->nvbo);
nouveau_bo_vma_del(nouveau_fb->nvbo, &nouveau_fb->vma);
+ nouveau_bo_unpin(nouveau_fb->nvbo);
drm_gem_object_unreference_unlocked(nouveau_fb->nvbo->gem);
nouveau_fb->nvbo = NULL;
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c
b/drivers/gpu/drm/nouveau/nouveau_gem.c
index 5972ecd..6d8391d 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -52,7 +52,8 @@ nouveau_gem_object_del(struct drm_gem_object *gem)
return;
nvbo->gem = NULL;
- if (unlikely(nvbo->pin_refcnt)) {
+ /* Lockdep hates you for doing reserve with gem object lock held */
+ if (WARN_ON_ONCE(nvbo->pin_refcnt)) {
nvbo->pin_refcnt = 1;
nouveau_bo_unpin(nvbo);
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_prime.c
b/drivers/gpu/drm/nouveau/nouveau_prime.c
index 366462c..bd785dd 100644
--- a/drivers/gpu/drm/nouveau/nouveau_prime.c
+++ b/drivers/gpu/drm/nouveau/nouveau_prime.c
@@ -57,10 +57,11 @@ static void nouveau_gem_dmabuf_release(struct dma_buf
*dma_buf)
{
struct nouveau_bo *nvbo = dma_buf->priv;
- if (nvbo->gem->export_dma_buf == dma_buf) {
+ if (nvbo->gem->export_dma_buf == dma_buf)
nvbo->gem->export_dma_buf = NULL;
- drm_gem_object_unreference_unlocked(nvbo->gem);
- }
+
+ nouveau_bo_unpin(nvbo);
+ drm_gem_object_unreference_unlocked(nvbo->gem);
}
static void *nouveau_gem_kmap_atomic(struct dma_buf *dma_buf, unsigned long
page_num)
@@ -175,13 +176,17 @@ struct dma_buf *nouveau_gem_prime_export(struct
drm_device *dev,
{
struct nouveau_bo *nvbo = nouveau_gem_object(obj);
int ret = 0;
+ struct dma_buf *buf;
/* pin buffer into GTT */
ret = nouveau_bo_pin(nvbo, TTM_PL_FLAG_TT);
if (ret)
return ERR_PTR(-EINVAL);
- return dma_buf_export(nvbo, &nouveau_dmabuf_ops, obj->size, flags);
+ buf = dma_buf_export(nvbo, &nouveau_dmabuf_ops, obj->size, flags);
+ if (IS_ERR(buf))
+ nouveau_bo_unpin(nvbo);
+ return buf;
}
struct drm_gem_object *nouveau_gem_prime_import(struct drm_device *dev,
[RFC 2/4] drm/exynos: add fimc ipp driver
Hi Eunchul,
IMHO, each function for source and destination has quite similar routine
and it seems that there are some redundant code. I'm not sure these
duplicated code can be removed with mergeing similar part.
Some comments are below.
On 2012? 10? 29? 22:10, Eunchul Kim wrote:
> FIMC is stand for Fully Interfactive Mobile Camera and
> supports image scaler/rotator/crop/flip/csc and input/output DMA operations.
> input DMA reads image data from the memory.
> output DMA writes image data to memory.
> FIMC supports image rotation and image effect functions.
> also supports writeback and display output operations.
>
> Signed-off-by: Eunchul Kim
> Signed-off-by: Jinyoung Jeon
> ---
> drivers/gpu/drm/exynos/Kconfig |6 +
> drivers/gpu/drm/exynos/Makefile |1 +
> drivers/gpu/drm/exynos/exynos_drm_drv.c | 15 +
> drivers/gpu/drm/exynos/exynos_drm_drv.h |1 +
> drivers/gpu/drm/exynos/exynos_drm_fimc.c | 2041
> ++
> drivers/gpu/drm/exynos/exynos_drm_fimc.h | 35 +
> drivers/gpu/drm/exynos/regs-fimc.h | 669 ++
> include/drm/exynos_drm.h | 33 +
> 8 files changed, 2801 insertions(+), 0 deletions(-)
> create mode 100644 drivers/gpu/drm/exynos/exynos_drm_fimc.c
> create mode 100644 drivers/gpu/drm/exynos/exynos_drm_fimc.h
> create mode 100644 drivers/gpu/drm/exynos/regs-fimc.h
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> new file mode 100644
> index 000..3adb452
> --- /dev/null
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> @@ -0,0 +1,2041 @@
> +/*
> + * Copyright (C) 2012 Samsung Electronics Co.Ltd
> + * Authors:
> + * Eunchul Kim
> + * Jinyoung Jeon
> + * Sangmin Lee
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + *
> + */
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +#include "regs-fimc.h"
> +#include "exynos_drm_drv.h"
> +#include "exynos_drm_gem.h"
> +#include "exynos_drm_ipp.h"
> +#include "exynos_drm_fimc.h"
> +
> +/*
> + * FIMC is stand for Fully Interactive Mobile Camera and
> + * supports image scaler/rotator and input/output DMA operations.
> + * input DMA reads image data from the memory.
> + * output DMA writes image data to memory.
> + * FIMC supports image rotation and image effect functions.
> + */
> +
> +#define FIMC_MAX_DEVS4
> +#define FIMC_MAX_SRC 2
> +#define FIMC_MAX_DST 32
> +#define FIMC_CLK_RATE16675
> +#define FIMC_CLK_RATE_R2 18000
> +#define FIMC_BUF_STOP1
> +#define FIMC_BUF_START 2
> +#define FIMC_REG_SZ 32
> +#define FIMC_WIDTH_ITU_709 1280
> +
> +#define get_fimc_context(dev)
> platform_get_drvdata(to_platform_device(dev))
> +#define get_ctx_from_ippdrv(ippdrv) container_of(ippdrv,\
> + struct fimc_context, ippdrv);
> +#define fimc_read(offset)readl(ctx->regs + (offset));
> +#define fimc_write(cfg, offset) writel(cfg, ctx->regs + (offset));
> +
> +enum fimc_wb {
> + FIMC_WB_NONE,
> + FIMC_WB_A,
> + FIMC_WB_B,
> +};
> +
> +/*
> + * A structure of scaler.
> + *
> + * @range: narrow, wide.
> + * @bypass: unused scaler path.
> + * @up_h: horizontal scale up.
> + * @up_v: vertical scale up.
> + * @hratio: horizontal ratio.
> + * @vratio: vertical ratio.
> + */
> +struct fimc_scaler {
> + boolrange;
> + bool bypass;
> + bool up_h;
> + bool up_v;
> + u32 hratio;
> + u32 vratio;
> +};
> +
> +/*
> + * A structure of scaler capability.
> + *
> + * find user manual table 43-1.
> + * @in_hori: scaler input horizontal size.
> + * @bypass: scaler bypass mode.
> + * @dst_h_wo_rot: target horizontal size without output rotation.
> + * @dst_h_rot: target horizontal size with output rotation.
> + * @rl_w_wo_rot: real width without input rotation.
> + * @rl_h_rot: real height without output rotation.
> + */
> +struct fimc_capability {
> + /* scaler */
> + u32 in_hori;
> + u32 bypass;
> + /* output rotator */
> + u32 dst_h_wo_rot;
> + u32 dst_h_rot;
> + /* input rotator */
> + u32 rl_w_wo_rot;
> + u32 rl_h_rot;
> +};
> +
> +/*
> + * A structure of fimc context.
> + *
> + * @ippdrv: prepare initialization using ippdrv.
> + * @regs_res: register resources.
> + * @regs: memory mapped io registers.
> + * @lock: locking of operations.
> + * @sclk_fimc_clk: fimc source clock.
> + * @fimc_clk: fimc clock.
> + * @wb_clk: writeback a clock.
> + * @wb_b_clk: writeback b clock.
> + * @sc: scaler infomations.
> + * @capa: scaler capability.
> + * @odr: ordering of YUV.
> + * @ver: fimc version.
> + * @pol: porarit
[RFC 2/4] drm/exynos: add fimc ipp driver
Dear Seung-Woo Kim
Thank's for your comment. and you gave the first comment. :)
I also considered about mergeing set_fmt, set_fmt_order.
but If we merge this function, we need to seperate all "switch case"
routine.
so, complexity has increased after mergeing.
and I designed this set_fmt function with two path.
first)
- set up the format information using relative register
second)
- set up the format ordering using relative register.
one more think about this design concept.
and If you need to merge this function, please give one more comments.
I will change and resent it.
and I commented about your comment.
Thank's
On 11/22/2012 11:30 AM, ??? wrote:
> Hi Eunchul,
>
> IMHO, each function for source and destination has quite similar routine
> and it seems that there are some redundant code. I'm not sure these
> duplicated code can be removed with mergeing similar part.
>
> Some comments are below.
>
> On 2012? 10? 29? 22:10, Eunchul Kim wrote:
>> FIMC is stand for Fully Interfactive Mobile Camera and
>> supports image scaler/rotator/crop/flip/csc and input/output DMA operations.
>> input DMA reads image data from the memory.
>> output DMA writes image data to memory.
>> FIMC supports image rotation and image effect functions.
>> also supports writeback and display output operations.
>>
>> Signed-off-by: Eunchul Kim
>> Signed-off-by: Jinyoung Jeon
>> ---
>> drivers/gpu/drm/exynos/Kconfig |6 +
>> drivers/gpu/drm/exynos/Makefile |1 +
>> drivers/gpu/drm/exynos/exynos_drm_drv.c | 15 +
>> drivers/gpu/drm/exynos/exynos_drm_drv.h |1 +
>> drivers/gpu/drm/exynos/exynos_drm_fimc.c | 2041
>> ++
>> drivers/gpu/drm/exynos/exynos_drm_fimc.h | 35 +
>> drivers/gpu/drm/exynos/regs-fimc.h | 669 ++
>> include/drm/exynos_drm.h | 33 +
>> 8 files changed, 2801 insertions(+), 0 deletions(-)
>> create mode 100644 drivers/gpu/drm/exynos/exynos_drm_fimc.c
>> create mode 100644 drivers/gpu/drm/exynos/exynos_drm_fimc.h
>> create mode 100644 drivers/gpu/drm/exynos/regs-fimc.h
>
>
>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>> b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>> new file mode 100644
>> index 000..3adb452
>> --- /dev/null
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>> @@ -0,0 +1,2041 @@
>> +/*
>> + * Copyright (C) 2012 Samsung Electronics Co.Ltd
>> + * Authors:
>> + * Eunchul Kim
>> + * Jinyoung Jeon
>> + * Sangmin Lee
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License as published by the
>> + * Free Software Foundation; either version 2 of the License, or (at your
>> + * option) any later version.
>> + *
>> + */
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#include
>> +#include
>> +#include "regs-fimc.h"
>> +#include "exynos_drm_drv.h"
>> +#include "exynos_drm_gem.h"
>> +#include "exynos_drm_ipp.h"
>> +#include "exynos_drm_fimc.h"
>> +
>> +/*
>> + * FIMC is stand for Fully Interactive Mobile Camera and
>> + * supports image scaler/rotator and input/output DMA operations.
>> + * input DMA reads image data from the memory.
>> + * output DMA writes image data to memory.
>> + * FIMC supports image rotation and image effect functions.
>> + */
>> +
>> +#define FIMC_MAX_DEVS 4
>> +#define FIMC_MAX_SRC2
>> +#define FIMC_MAX_DST32
>> +#define FIMC_CLK_RATE 16675
>> +#define FIMC_CLK_RATE_R218000
>> +#define FIMC_BUF_STOP 1
>> +#define FIMC_BUF_START 2
>> +#define FIMC_REG_SZ 32
>> +#define FIMC_WIDTH_ITU_709 1280
>> +
>> +#define get_fimc_context(dev)
>> platform_get_drvdata(to_platform_device(dev))
>> +#define get_ctx_from_ippdrv(ippdrv) container_of(ippdrv,\
>> +struct fimc_context, ippdrv);
>> +#define fimc_read(offset) readl(ctx->regs + (offset));
>> +#define fimc_write(cfg, offset) writel(cfg, ctx->regs + (offset));
>> +
>> +enum fimc_wb {
>> +FIMC_WB_NONE,
>> +FIMC_WB_A,
>> +FIMC_WB_B,
>> +};
>> +
>> +/*
>> + * A structure of scaler.
>> + *
>> + * @range: narrow, wide.
>> + * @bypass: unused scaler path.
>> + * @up_h: horizontal scale up.
>> + * @up_v: vertical scale up.
>> + * @hratio: horizontal ratio.
>> + * @vratio: vertical ratio.
>> + */
>> +struct fimc_scaler {
>> +boolrange;
>> +bool bypass;
>> +bool up_h;
>> +bool up_v;
>> +u32 hratio;
>> +u32 vratio;
>> +};
>> +
>> +/*
>> + * A structure of scaler capability.
>> + *
>> + * find user manual table 43-1.
>> + * @in_hori: scaler input horizontal size.
>> + * @bypass: scaler bypass mode.
>> + * @dst_h_wo_rot: target horizontal size without output rotation.
>> + * @dst_h_rot: target horizontal size with output rotation.
>> + * @rl_w_wo_rot: real width without input rotation.
>> + * @rl_h_rot: real hei
[git pull] drm fixes
Hi Linus, I let this go a few days longer than I normally do since you looked to be having lots of fun with various underwater things, so this is an all over set of fixes, nothing really stands out, nearly all of them fix a regression in one driver or another, or some sort of oops. its vmware/nouveua/radeon/intel/ttm scattered. Dave. The following changes since commit 77b67063bb6bce6d475e910d3b886a606d0d91f7: Linux 3.7-rc5 (2012-11-11 13:44:33 +0100) are available in the git repository at: git://people.freedesktop.org/~airlied/linux drm-fixes Akinobu Mita (1): drm/ttm: remove unneeded preempt_disable/enable Alex Deucher (3): drm/radeon: fix logic error in atombios_encoders.c drm/radeon: properly track the crtc not_enabled case evergreen_mc_stop() drm/radeon: add new SI pci id Dan Carpenter (1): vmwgfx: return an -EFAULT if copy_to_user() fails Dave Airlie (5): Merge branch 'drm-fixes-3.7' of git://people.freedesktop.org/~agd5f/linux into drm-fixes Merge branch 'drm-intel-fixes' of git://people.freedesktop.org/~danvet/drm-intel into drm-fixes Merge branch 'drm-nouveau-fixes' of git://anongit.freedesktop.org/git/nouveau/linux-2.6 into drm-fixes Merge branch 'drm-nouveau-fixes' of git://anongit.freedesktop.org/git/nouveau/linux-2.6 into drm-fixes Merge branch 'drm-fixes-3.7' of git://people.freedesktop.org/~agd5f/linux into drm-fixes Jani Nikula (3): drm/i915/crt: fix DPMS standby and suspend mode handling drm/i915/sdvo: clean up connectors on intel_sdvo_init() failures drm/i915: do not ignore eDP bpc settings from vbt Kelly Doran (1): drm/nvc0/disp: fix thinko in vblank regression fix.. Maarten Lankhorst (2): drm/nouveau: add missing pll_calc calls drm/nouveau: use the correct fence implementation for nv50 Marcin Slusarz (3): drm/nv40: allocate ctxprog with kmalloc drm/nouveau: fix crash with noaccel=1 drm/nouveau/bios: fix DCB v1.5 parsing Paul Bolle (1): radeon: add AGPMode 1 quirk for RV250 Zhao Yakui (1): ttm: Clear the ttm page allocated from high memory zone correctly drivers/gpu/drm/i915/intel_crt.c |2 +- drivers/gpu/drm/i915/intel_display.c | 11 ++ drivers/gpu/drm/i915/intel_sdvo.c | 22 +-- drivers/gpu/drm/nouveau/core/engine/disp/nv50.c| 19 ++-- .../gpu/drm/nouveau/core/engine/graph/ctxnv40.c| 12 -- drivers/gpu/drm/nouveau/core/engine/graph/nv40.c |4 ++- drivers/gpu/drm/nouveau/core/engine/graph/nv40.h |2 +- drivers/gpu/drm/nouveau/core/include/core/object.h | 14 .../gpu/drm/nouveau/core/include/subdev/clock.h|3 +- drivers/gpu/drm/nouveau/core/subdev/bios/dcb.c |2 +- drivers/gpu/drm/nouveau/core/subdev/clock/nva3.c | 19 + drivers/gpu/drm/nouveau/core/subdev/clock/nvc0.c |1 + drivers/gpu/drm/nouveau/nouveau_abi16.c|4 +++ drivers/gpu/drm/nouveau/nouveau_drm.c |3 +- drivers/gpu/drm/radeon/atombios_encoders.c |2 +- drivers/gpu/drm/radeon/evergreen.c |2 + drivers/gpu/drm/radeon/radeon_agp.c|5 +++- drivers/gpu/drm/ttm/ttm_page_alloc.c |5 +++- drivers/gpu/drm/ttm/ttm_tt.c |4 --- drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c |2 + include/drm/drm_pciids.h |1 + 21 files changed, 108 insertions(+), 31 deletions(-)
[PATCH v12 3/6] fbmon: add videomode helpers
Hi Laurent, On Wed, Nov 21, 2012 at 11:02:44PM +0100, Laurent Pinchart wrote: > Hi Steffen, > > > + > > + 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); > > This will fail if vm->pixelclock >= ((1 << 32) / 1000). The easiest solution > is probably to use 64-bit computation. You have displays with a pixelclock > 4GHz? Sascha -- 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- |
[PATCH] drm/exynos: use sgt instead of pages for framebuffer address
The 'pages' structure in the exynos gem buffer has been removed. So we get the fix.smem_start from the first sgl of the scatter gather table. Signed-off-by: Prathyush K --- drivers/gpu/drm/exynos/exynos_drm_fbdev.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c index 3d5910f..d37f281 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c @@ -117,8 +117,8 @@ static int exynos_drm_fbdev_update(struct drm_fb_helper *helper, dev->mode_config.fb_base = (resource_size_t)buffer->dma_addr; fbi->screen_base = buffer->kvaddr + offset; - fbi->fix.smem_start = (unsigned long)(page_to_phys(buffer->pages[0]) + - offset); + fbi->fix.smem_start = (unsigned long) + (page_to_phys(sg_page(buffer->sgt->sgl)) + offset); fbi->screen_size = size; fbi->fix.smem_len = size; -- 1.7.0.4
[PATCH 2/2] drm: exynos: compose and send avi and aui info frames
On 2012? 11? 21? 20:36, Rahul Sharma wrote:
> Hi Seung Woo,
>
> Thanks for your inputs. Please find my response below.
>
> On Wed, Nov 21, 2012 at 2:12 PM, ??? wrote:
>> Hi Rahul,
>>
>> Control part seems good, and my comment is below.
>>
>> On 2012? 11? 10? 01:21, Rahul Sharma wrote:
>>> This patch adds code for composing AVI and AUI info frames
>>> and send them every VSYNC.
>>>
>>> This patch is important for hdmi certification.
>>>
>>> Signed-off-by: Fahad Kunnathadi
>>> Signed-off-by: Shirish S
>>> Signed-off-by: Rahul Sharma
>>> ---
>>> drivers/gpu/drm/exynos/exynos_hdmi.c | 97
>>> +-
>>> drivers/gpu/drm/exynos/exynos_hdmi.h | 23
>>> drivers/gpu/drm/exynos/regs-hdmi.h | 17 +-
>>> 3 files changed, 133 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> b/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> index 2c115f8..bb8a045 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>>
>>
>>
>>> @@ -1993,6 +2084,8 @@ static void hdmi_mode_set(void *ctx, void *mode)
>>>
>>> DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
>>>
>>> + hdata->cur_video_id = drm_match_cea_mode(mode);
>>> +
>>
>> How do you think about using predefined cea video id in struct
>> hdmi_conf? drm_mode does not have cea video id, so drm_match_cea_mode()
>> compares only mode information. Considering this, IMHO, cea video id can
>> be embedded in struct hdmi_conf.
>>
>
> I feel, It will leads to duplication of video id information. In
> edid_cea_modes, modes are
> strictly arranged in the order of respective cea video ID codes.
> "drm_add_edid_modes"
> also passes the cea codes (recieved after edid data parsing) as the index to
> edid_cea_modes to get mode details.
It might be a concern related with your first patch, anyway
edid_cea_modes has few pair of exact same modes because struct drm_mode
does not have picture ratio. For example, video id 2 and 3 have exact
same values for struct drm_mode. So cea video id can be used to get a
mode, but a drm_mode is not sufficient to get exact video id.
Considering that exynos hdmi does not support video ids with same mode,
I suggested video id in struct hdmi_conf.
At the point of exynos drm, I can ack this patch.
>
> Secondly, mode to cea code translation is required by all platforms
> for AVI packet
> composition. By adding it to hdmi_conf, we are limiting its usage for exynos.
I agree with you at this point. I quickly checked i915 and radeon and I
found that they use fixed value for avi packet at sw level, but I don't
have information hw can properly build avi packet. If they also need
video id for building avi packet, video id translation can be used.
Best Regards,
- Seung-Woo Kim
>
> regards,
> Rahul Sharma.
>
>>> conf_idx = hdmi_conf_index(hdata, mode);
>>> if (conf_idx >= 0)
>>> hdata->cur_conf = conf_idx;
>>
>>
>>
>> Thanks and Regards,
>> - Seung-Woo Kim
>>
>> --
>> Seung-Woo Kim
>> Samsung Software R&D Center
>> --
>>
>> ___
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
[PATCH] drm/i915: set the AVI VIC of the HDMI mode
On Wed, Nov 21, 2012 at 01:39:48PM -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni
>
> We currently set "0" as the VIC value of the AVI InfoFrames. According
> to the specs this should be fine and work for every mode, so to my
> point of view we can't consider the current behavior as a bug. The
> problem is that we recently received a bug report (Kernel bug #50371)
> from a user that has an AV receiver that gives a black screen for any
> mode with VIC set to 0.
>
> So in order to make at least some modes work for him, this patch sets
> the correct VIC number when sending AVI InfoFrames. We add a generic
> drm function to calculate the VIC number and then call it from the
> i915 driver.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=50371
> Cc: Thierry Reding
> Signed-off-by: Paulo Zanoni
> ---
> drivers/gpu/drm/drm_edid.c| 19 +++
> drivers/gpu/drm/drm_modes.c |3 ++-
> drivers/gpu/drm/i915/intel_hdmi.c |2 ++
> include/drm/drm_crtc.h|4 +++-
> 4 files changed, 26 insertions(+), 2 deletions(-)
>
> Patch applies on top of Daniel's drm-intel-next-queued. I'm not sure who
> exactly
> is going to merge this (Dave or Daniel).
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index fadcd44..c57fc46 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2062,3 +2062,22 @@ int drm_add_modes_noedid(struct drm_connector
> *connector,
> return num_modes;
> }
> EXPORT_SYMBOL(drm_add_modes_noedid);
> +
> +/**
> + * drm_mode_vic - return the CEA-861 VIC of a given mode
The name in the comment here doesn't match the actual function name.
> + * @mode: mode
> + *
> + * RETURNS:
> + * The VIC number, 0 in case it's not a CEA-861 mode.
> + */
> +uint8_t drm_mode_cea_vic(const struct drm_display_mode *mode)
I understand the reason for returning an uint8_t here, but ...
> +{
> + uint8_t i;
> +
> + for (i = 0; i < drm_num_cea_modes; i++)
... maybe unsigned int would be better for the iteration variable here.
Looking at drm_edid_modes.h, drm_num_cea_modes is actually signed, which
isn't necessary to store an array size, so maybe that should be changed
as well.
> + if (drm_mode_equal(mode, &edid_cea_modes[i]))
> + return i + 1;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_mode_cea_vic);
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 59450f3..9ef6750 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -768,7 +768,8 @@ EXPORT_SYMBOL(drm_mode_duplicate);
> * RETURNS:
> * True if the modes are equal, false otherwise.
> */
> -bool drm_mode_equal(struct drm_display_mode *mode1, struct drm_display_mode
> *mode2)
> +bool drm_mode_equal(const struct drm_display_mode *mode1,
> + const struct drm_display_mode *mode2)
I think this change warrants a separate commit.
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20121122/3ea9637c/attachment-0001.pgp>
[PATCH] drm/i915: set the AVI VIC of the HDMI mode
On Wed, Nov 21, 2012 at 05:08:12PM +0100, Daniel Vetter wrote: > On Wed, Nov 21, 2012 at 4:47 PM, Thierry Reding > wrote: > > Oh great, so I copied that table for nothing. Thanks for Cc'ing, I can > > reuse that in the HDMI infoframe series. > > Wrt the infoframe series, I think it'd be awesome if you could convert > i915 and radeon (iirc the existing drivers with the "best" avi > infoframe support) over to the new code. This gives some nice > validation, both by testing on actual hw and that the interface is > sane, since it'll be used by 2-3 different drivers then. I'll have to rely on somebody else to do the testing since I don't have an HDMI capable hardware except Tegra to run this on. But yes, I had planned to convert Tegra and at least one other driver for reference. But I guess while at it I could just as well convert all of them. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20121122/d3ab1164/attachment.pgp>
[PATCH] drm/i915: set the AVI VIC of the HDMI mode
On Wed, Nov 21, 2012 at 05:09:31PM +0100, Daniel Vetter wrote: > On Wed, Nov 21, 2012 at 5:08 PM, Daniel Vetter wrote: > > On Wed, Nov 21, 2012 at 4:47 PM, Thierry Reding > > wrote: > >> Oh great, so I copied that table for nothing. Thanks for Cc'ing, I can > >> reuse that in the HDMI infoframe series. > > > > Wrt the infoframe series, I think it'd be awesome if you could convert > > i915 and radeon (iirc the existing drivers with the "best" avi > > infoframe support) over to the new code. This gives some nice > > validation, both by testing on actual hw and that the interface is > > sane, since it'll be used by 2-3 different drivers then. > > > > I think the best way is to pick the infoframe implementation you best > > like from one of these, move it into the helper, then improve it until > > you're happy. And then convert over 1-2 other drivers. At least that's > > been my approach for the recent dp helper refactoring. > > And if you could use that opportunity to integrate the kerneldoc for > edid/eld and the new infoframe code into the drm docbook, that would > be rather awesome ;-) I suck at documentation =), but I'll see what I can do. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20121122/cd88111f/attachment.pgp>
[PATCH] drm/exynos: use sgt instead of pages for framebuffer address
Right, I missed. Thanks, Inki Dae > -Original Message- > From: Prathyush K [mailto:prathyush.k at samsung.com] > Sent: Thursday, November 22, 2012 3:49 PM > To: dri-devel at lists.freedesktop.org > Cc: inki.dae at samsung.com > Subject: [PATCH] drm/exynos: use sgt instead of pages for framebuffer > address > > The 'pages' structure in the exynos gem buffer has been > removed. So we get the fix.smem_start from the first sgl > of the scatter gather table. > > Signed-off-by: Prathyush K > --- > drivers/gpu/drm/exynos/exynos_drm_fbdev.c |4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c > b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c > index 3d5910f..d37f281 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c > @@ -117,8 +117,8 @@ static int exynos_drm_fbdev_update(struct > drm_fb_helper *helper, > > dev->mode_config.fb_base = (resource_size_t)buffer->dma_addr; > fbi->screen_base = buffer->kvaddr + offset; > - fbi->fix.smem_start = (unsigned long)(page_to_phys(buffer->pages[0]) > + > - offset); > + fbi->fix.smem_start = (unsigned long) > + (page_to_phys(sg_page(buffer->sgt->sgl)) + offset); > fbi->screen_size = size; > fbi->fix.smem_len = size; > > -- > 1.7.0.4
[PATCH] drm/i915: set the AVI VIC of the HDMI mode
2012/11/22 Thierry Reding : > On Wed, Nov 21, 2012 at 05:08:12PM +0100, Daniel Vetter wrote: >> On Wed, Nov 21, 2012 at 4:47 PM, Thierry Reding >> wrote: >> > Oh great, so I copied that table for nothing. Thanks for Cc'ing, I can >> > reuse that in the HDMI infoframe series. >> >> Wrt the infoframe series, I think it'd be awesome if you could convert >> i915 and radeon (iirc the existing drivers with the "best" avi >> infoframe support) over to the new code. This gives some nice >> validation, both by testing on actual hw and that the interface is >> sane, since it'll be used by 2-3 different drivers then. > > I'll have to rely on somebody else to do the testing since I don't have > an HDMI capable hardware except Tegra to run this on. But yes, I had > planned to convert Tegra and at least one other driver for reference. > But I guess while at it I could just as well convert all of them. I'll take a look at radeon (I'll try to convert it to the new functions) over weekend. -- Rafa?
[PATCH] drm/i915: set the AVI VIC of the HDMI mode
On Thu, Nov 22, 2012 at 09:00:24AM +0100, Rafa? Mi?ecki wrote: > 2012/11/22 Thierry Reding : > > On Wed, Nov 21, 2012 at 05:08:12PM +0100, Daniel Vetter wrote: > >> On Wed, Nov 21, 2012 at 4:47 PM, Thierry Reding > >> wrote: > >> > Oh great, so I copied that table for nothing. Thanks for Cc'ing, I can > >> > reuse that in the HDMI infoframe series. > >> > >> Wrt the infoframe series, I think it'd be awesome if you could convert > >> i915 and radeon (iirc the existing drivers with the "best" avi > >> infoframe support) over to the new code. This gives some nice > >> validation, both by testing on actual hw and that the interface is > >> sane, since it'll be used by 2-3 different drivers then. > > > > I'll have to rely on somebody else to do the testing since I don't have > > an HDMI capable hardware except Tegra to run this on. But yes, I had > > planned to convert Tegra and at least one other driver for reference. > > But I guess while at it I could just as well convert all of them. > > I'll take a look at radeon (I'll try to convert it to the new > functions) over weekend. Okay, great! I think for radeon things should be the easiest since it doesn't currently fill in anything but the colorspace field. Judging by the bug report that Paulo mentioned this will probably not be enough for some hardware, but should be enough according to the specification. Having more data in the AVI infoframe shouldn't hurt, though. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20121122/d3ffed20/attachment.pgp>
[PATCH 1/4] drm/crtc: Remove redundant NULL check before kfree
On 21 November 2012 20:42, Sachin Kamat wrote: > Hi Dave, > > Please ignore this patch. > > Please ignore this mail. Sorry for the noise. -- With warm regards, Sachin
[PATCH 1/1] drm/exynos: Fix potential NULL pointer dereference in exynos_drm_encoder.c
Hi Inki, On 19 November 2012 15:32, Sachin Kamat wrote: > On 19 November 2012 15:30, Inki Dae wrote: >> >> >>> -Original Message- >>> From: Sachin Kamat [mailto:sachin.kamat at linaro.org] >>> Sent: Monday, November 19, 2012 6:56 PM >>> To: Inki Dae >>> Cc: dri-devel at lists.freedesktop.org; jy0922.shim at samsung.com; >>> patches at linaro.org >>> Subject: Re: [PATCH 1/1] drm/exynos: Fix potential NULL pointer >>> dereference in exynos_drm_encoder.c >>> >>> Hi Inki, >>> >>> Thanks for your review. My comments inline. >>> >>> On 19 November 2012 15:14, Inki Dae wrote: >>> > >>> > >>> >> -Original Message- >>> >> From: Sachin Kamat [mailto:sachin.kamat at linaro.org] >>> >> Sent: Monday, November 19, 2012 6:21 PM >>> >> To: dri-devel at lists.freedesktop.org >>> >> Cc: inki.dae at samsung.com; jy0922.shim at samsung.com; >>> > sachin.kamat at linaro.org; >>> >> patches at linaro.org >>> >> Subject: [PATCH 1/1] drm/exynos: Fix potential NULL pointer dereference >>> in >>> >> exynos_drm_encoder.c >>> >> >>> >> Check overlay_ops is not NULL as checked in the previous 'if' >> condition. >>> >> Fixes the following smatch error: >>> >> drivers/gpu/drm/exynos/exynos_drm_encoder.c:509 >>> >> exynos_drm_encoder_plane_disable() >>> >> error: we previously assumed 'overlay_ops' could be null (see line 499) >>> >> >>> >> Signed-off-by: Sachin Kamat >>> >> --- >>> >> drivers/gpu/drm/exynos/exynos_drm_encoder.c |2 +- >>> >> 1 files changed, 1 insertions(+), 1 deletions(-) >>> >> >>> >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_encoder.c >>> >> b/drivers/gpu/drm/exynos/exynos_drm_encoder.c >>> >> index e51503f..a44238e 100644 >>> >> --- a/drivers/gpu/drm/exynos/exynos_drm_encoder.c >>> >> +++ b/drivers/gpu/drm/exynos/exynos_drm_encoder.c >>> >> @@ -506,6 +506,6 @@ void exynos_drm_encoder_plane_disable(struct >>> >> drm_encoder *encoder, void *data) >>> >>* because the setting for disabling the overlay will be updated >>> >>* at vsync. >>> >>*/ >>> >> - if (overlay_ops->wait_for_vblank) >>> >> + if (overlay_ops && overlay_ops->wait_for_vblank) >>> >> overlay_ops->wait_for_vblank(manager->dev); >>> > >>> > This code will be removed at -next. >>> >>> Since this code is already in mainline, I think this patch should be >>> applied as a fix during this rc (for completeness). >>> You may subsequently delete it in the next release as per your plan. >>> >> >> And NULL pointer checking was already done above like below, >> if (overlay_ops && overlay_ops->disable) >> overlay_ops->disable(manager->dev, zpos); > Correct. But that check is applicable only for that one statement > (overlay_ops->disable(manager->dev, zpos);). > > Similar check needs to be added to below 'if' code too. What are your comments about this? > >> >> This is your missing point. >> >>> > >>> > Thanks, >>> > Inki Dae >>> > >>> >> } >>> >> -- >>> >> 1.7.4.1 >>> > >>> >>> >>> >>> -- >>> With warm regards, >>> Sachin >> > > > > -- > With warm regards, > Sachin -- With warm regards, Sachin
[PATCH 1/1] drm/exynos: Fix potential NULL pointer dereference in exynos_drm_encoder.c
> -Original Message- > From: Sachin Kamat [mailto:sachin.kamat at linaro.org] > Sent: Thursday, November 22, 2012 3:13 PM > To: Inki Dae > Cc: dri-devel at lists.freedesktop.org; jy0922.shim at samsung.com; > patches at linaro.org > Subject: Re: [PATCH 1/1] drm/exynos: Fix potential NULL pointer > dereference in exynos_drm_encoder.c > > Hi Inki, > > On 19 November 2012 15:32, Sachin Kamat wrote: > > On 19 November 2012 15:30, Inki Dae wrote: > >> > >> > >>> -Original Message- > >>> From: Sachin Kamat [mailto:sachin.kamat at linaro.org] > >>> Sent: Monday, November 19, 2012 6:56 PM > >>> To: Inki Dae > >>> Cc: dri-devel at lists.freedesktop.org; jy0922.shim at samsung.com; > >>> patches at linaro.org > >>> Subject: Re: [PATCH 1/1] drm/exynos: Fix potential NULL pointer > >>> dereference in exynos_drm_encoder.c > >>> > >>> Hi Inki, > >>> > >>> Thanks for your review. My comments inline. > >>> > >>> On 19 November 2012 15:14, Inki Dae wrote: > >>> > > >>> > > >>> >> -Original Message- > >>> >> From: Sachin Kamat [mailto:sachin.kamat at linaro.org] > >>> >> Sent: Monday, November 19, 2012 6:21 PM > >>> >> To: dri-devel at lists.freedesktop.org > >>> >> Cc: inki.dae at samsung.com; jy0922.shim at samsung.com; > >>> > sachin.kamat at linaro.org; > >>> >> patches at linaro.org > >>> >> Subject: [PATCH 1/1] drm/exynos: Fix potential NULL pointer > dereference > >>> in > >>> >> exynos_drm_encoder.c > >>> >> > >>> >> Check overlay_ops is not NULL as checked in the previous 'if' > >> condition. > >>> >> Fixes the following smatch error: > >>> >> drivers/gpu/drm/exynos/exynos_drm_encoder.c:509 > >>> >> exynos_drm_encoder_plane_disable() > >>> >> error: we previously assumed 'overlay_ops' could be null (see line > 499) > >>> >> > >>> >> Signed-off-by: Sachin Kamat > >>> >> --- > >>> >> drivers/gpu/drm/exynos/exynos_drm_encoder.c |2 +- > >>> >> 1 files changed, 1 insertions(+), 1 deletions(-) > >>> >> > >>> >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_encoder.c > >>> >> b/drivers/gpu/drm/exynos/exynos_drm_encoder.c > >>> >> index e51503f..a44238e 100644 > >>> >> --- a/drivers/gpu/drm/exynos/exynos_drm_encoder.c > >>> >> +++ b/drivers/gpu/drm/exynos/exynos_drm_encoder.c > >>> >> @@ -506,6 +506,6 @@ void exynos_drm_encoder_plane_disable(struct > >>> >> drm_encoder *encoder, void *data) > >>> >>* because the setting for disabling the overlay will be > updated > >>> >>* at vsync. > >>> >>*/ > >>> >> - if (overlay_ops->wait_for_vblank) > >>> >> + if (overlay_ops && overlay_ops->wait_for_vblank) > >>> >> overlay_ops->wait_for_vblank(manager->dev); > >>> > > >>> > This code will be removed at -next. > >>> > >>> Since this code is already in mainline, I think this patch should be > >>> applied as a fix during this rc (for completeness). > >>> You may subsequently delete it in the next release as per your plan. > >>> > >> > >> And NULL pointer checking was already done above like below, > >> if (overlay_ops && overlay_ops->disable) > >> overlay_ops->disable(manager->dev, zpos); > > Correct. But that check is applicable only for that one statement > > (overlay_ops->disable(manager->dev, zpos);). > > > > Similar check needs to be added to below 'if' code too. > > What are your comments about this? > Left condition first is checked so as I mentioned before, it doesn't need overlay_ops checking because that was checked already. why do you think overlay_ops should be checked again? > > > >> > >> This is your missing point. > >> > >>> > > >>> > Thanks, > >>> > Inki Dae > >>> > > >>> >> } > >>> >> -- > >>> >> 1.7.4.1 > >>> > > >>> > >>> > >>> > >>> -- > >>> With warm regards, > >>> Sachin > >> > > > > > > > > -- > > With warm regards, > > Sachin > > > > -- > With warm regards, > Sachin
[PATCH 1/1] drm/exynos: Fix potential NULL pointer dereference in exynos_drm_encoder.c
> -Original Message- > From: Sachin Kamat [mailto:sachin.kamat at linaro.org] > Sent: Thursday, November 22, 2012 5:19 PM > To: Inki Dae > Cc: dri-devel at lists.freedesktop.org; jy0922.shim at samsung.com; > patches at linaro.org > Subject: Re: [PATCH 1/1] drm/exynos: Fix potential NULL pointer > dereference in exynos_drm_encoder.c > > [snip] > >> >> And NULL pointer checking was already done above like below, > >> >> if (overlay_ops && overlay_ops->disable) > >> >> overlay_ops->disable(manager->dev, zpos); > >> > Correct. But that check is applicable only for that one statement > >> > (overlay_ops->disable(manager->dev, zpos);). > >> > > >> > Similar check needs to be added to below 'if' code too. > >> > >> What are your comments about this? > >> > > > > Left condition first is checked so as I mentioned before, it doesn't > need > > overlay_ops checking because that was checked already. why do you think > > overlay_ops should be checked again? > > > > Consider the case when overlay_ops is NULL. > > if (overlay_ops && overlay_ops->disable) > overlay_ops->disable(manager->dev, zpos); > > It does not enter this condition as overlay_ops is NULL and moves to > the next statement, > if (overlay_ops->wait_for_vblank) where it gets dereferenced. > > Please note we are not returning back from the first condition if > overlay_ops is NULL. > Hence we need to check the condition in second case too. > Ah~ Right. I didn't check it surely. :) Thanks, Inki Dae > -- > With warm regards, > Sachin
[Bug 43751] [TTM] Out of kernel memory
https://bugzilla.kernel.org/show_bug.cgi?id=43751 --- Comment #4 from wmotti at gmail.com 2012-11-22 08:34:19 --- Same problem here with 3.6.6 kernel -- Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are watching the assignee of the bug.
[PATCH v12 3/6] fbmon: add videomode helpers
Hi Sascha, On Thursday 22 November 2012 07:20:00 Sascha Hauer wrote: > On Wed, Nov 21, 2012 at 11:02:44PM +0100, Laurent Pinchart wrote: > > Hi Steffen, > > > > > + > > > + 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); > > > > This will fail if vm->pixelclock >= ((1 << 32) / 1000). The easiest > > solution is probably to use 64-bit computation. > > You have displays with a pixelclock > 4GHz? vm->pixelclock is expressed in Hz. vm->pixelclock * 1000 will thus overflow if the clock frequency is >= ~4.3 MHz. I have displays with a clock frequency higher than that :-) -- Regards, Laurent Pinchart
[PATCH] drm/edid: tune down debug message in parse_hdmi_vsdb
Those tend to be totally not interesting for end-users, and for
debugging we tend to dump the entire noise anyway by enabling all
debug messages.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=57388
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_edid.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 1648200..484c36a 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1639,7 +1639,7 @@ parse_hdmi_vsdb(struct drm_connector *connector, const u8
*db)
if (len >= 12)
connector->audio_latency[1] = db[12];
- DRM_LOG_KMS("HDMI: DVI dual %d, "
+ DRM_DEBUG_KMS("HDMI: DVI dual %d, "
"max TMDS clock %d, "
"latency present %d %d, "
"video latency %d %d, "
--
1.7.10.4
[PATCH v12 3/6] fbmon: add videomode helpers
On Thu, Nov 22, 2012 at 09:50:10AM +0100, Laurent Pinchart wrote: > Hi Sascha, > > On Thursday 22 November 2012 07:20:00 Sascha Hauer wrote: > > On Wed, Nov 21, 2012 at 11:02:44PM +0100, Laurent Pinchart wrote: > > > Hi Steffen, > > > > > > > + > > > > + 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); > > > > > > This will fail if vm->pixelclock >= ((1 << 32) / 1000). The easiest > > > solution is probably to use 64-bit computation. > > > > You have displays with a pixelclock > 4GHz? > > vm->pixelclock is expressed in Hz. vm->pixelclock * 1000 will thus overflow > if > the clock frequency is >= ~4.3 MHz. I have displays with a clock frequency > higher than that :-) If vm->pixelclock is in Hz, then the * 1000 above is wrong. Sascha -- 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- |
[PATCH v12 3/6] fbmon: add videomode helpers
On Thursday 22 November 2012 09:53:42 Sascha Hauer wrote: > On Thu, Nov 22, 2012 at 09:50:10AM +0100, Laurent Pinchart wrote: > > On Thursday 22 November 2012 07:20:00 Sascha Hauer wrote: > > > On Wed, Nov 21, 2012 at 11:02:44PM +0100, Laurent Pinchart wrote: > > > > Hi Steffen, > > > > > > > > > + > > > > > + 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); > > > > > > > > This will fail if vm->pixelclock >= ((1 << 32) / 1000). The easiest > > > > solution is probably to use 64-bit computation. > > > > > > You have displays with a pixelclock > 4GHz? > > > > vm->pixelclock is expressed in Hz. vm->pixelclock * 1000 will thus > > overflow if the clock frequency is >= ~4.3 MHz. I have displays with a > > clock frequency higher than that :-) > > If vm->pixelclock is in Hz, then the * 1000 above is wrong. My bad, I though refresh was expressed in mHz. So yes, the above computation is wrong. BTW it seems that the refreshrate field in struct videomode isn't used. It should then be removed. I've just realized that the struct videomode fields are not documented. kerneldoc in include/linux/videomode.h would be a good addition. -- Regards, Laurent Pinchart
[PATCH v12 2/6] video: add of helper for videomode
Hi!
On Wed, Nov 21, 2012 at 09:03:38AM -0600, Rob Herring wrote:
> On 11/21/2012 05:52 AM, Thierry Reding wrote:
> > On Wed, Nov 21, 2012 at 12:48:43PM +0100, Steffen Trumtrar wrote:
> >> Hi!
> >>
> >> On Wed, Nov 21, 2012 at 10:12:43AM +, Manjunathappa, Prakash wrote:
> >>> Hi Steffen,
> >>>
> >>> On Tue, Nov 20, 2012 at 21:24:52, Steffen Trumtrar wrote:
> +/**
> + * of_get_display_timings - parse all display_timing entries from a
> device_node
> + * @np: device_node with the subnodes
> + **/
> +struct display_timings *of_get_display_timings(const struct device_node
> *np)
> +{
> +struct device_node *timings_np;
> +struct device_node *entry;
> +struct device_node *native_mode;
> +struct display_timings *disp;
> +
> +if (!np) {
> +pr_err("%s: no devicenode given\n", __func__);
> +return NULL;
> +}
> +
> +timings_np = of_find_node_by_name(np, "display-timings");
> >>>
> >>> I get below build warnings on this line
> >>> drivers/video/of_display_timing.c: In function 'of_get_display_timings':
> >>> drivers/video/of_display_timing.c:109:2: warning: passing argument 1 of
> >>> 'of_find_node_by_name' discards qualifiers from pointer target type
> >>> include/linux/of.h:167:28: note: expected 'struct device_node *' but
> >>> argument is of type 'const struct device_node *'
> >>>
> + * of_display_timings_exists - check if a display-timings node is
> provided
> + * @np: device_node with the timing
> + **/
> +int of_display_timings_exists(const struct device_node *np)
> +{
> +struct device_node *timings_np;
> +
> +if (!np)
> +return -EINVAL;
> +
> +timings_np = of_parse_phandle(np, "display-timings", 0);
> >>>
> >>> Also here:
> >>> drivers/video/of_display_timing.c: In function
> >>> 'of_display_timings_exists':
> >>> drivers/video/of_display_timing.c:209:2: warning: passing argument 1 of
> >>> 'of_parse_phandle' discards qualifiers from pointer target type
> >>> include/linux/of.h:258:28: note: expected 'struct device_node *' but
> >>> argument is of type 'const struct device_node *'
> >>>
> >>
> >> The warnings are because the of-functions do not use const pointers where
> >> they
> >> should. I had two options: don't use const pointers even if they should be
> >> and
> >> have no warnings or use const pointers and have a correct API. (Third
> >> option:
> >> send patches for of-functions). I chose the second option.
> >
> > Maybe a better approach would be a combination of 1 and 3: don't use
> > const pointers for struct device_node for now and bring the issue up
> > with the OF maintainers, possibly with patches attached that fix the
> > problematic functions.
>
> Why does this need to be const? Since some DT functions increment
> refcount the node, I'm not sure that making struct device_node const in
> general is right thing to do. I do think it should be okay for
> of_parse_phandle.
>
Okay, that seems right. I went a little to far with const'ing.
I will send a patch for of_parse_phandle as this function does
not seem to change the pointer it is given, but returns a new one
on which the refcount gets incremented.
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- |
[PATCH v2 00/18] DRM/KMS/EDID: Various EDID handling related fixes.
The patches have been reordered and the changes suggested by Takashi Iwai have been worked in. Egbert Eich (18): 1. Make error handling of EDID extension blocks a bit more fault tolorant: * Don't fail when EDID extension blocks cannot be read or memory cannot be allocated. * Don't read EDID extension blocks there are no blocks to expect, either for EDID versions < 1.3 or when block 3 is equal to the base block (in which case the monitor is not EDDC capable and doesn't accept the segment address. DRM/KMS/EDID: Mask out Segment Bits when calculating Offset. DRM/KMS/EDID: 0x7e -> EDID_EXTENSION_FLAG_OFFSET (v2) DRM/KMS/EDID: Return Base EDID block if reading EEDID Blocks fails (v2) DRM/KMS/EDID: Don't fail when failing to allocate memory for EDID extensions. DRM/KMS/EDID: Test EDDC if EDID announces more than one Extension Block (v2) DRM/KMS/EDID: Don't expect extension blocks for EDID Versions < 1.3. DRM/KMS/EDID: Don't reallocate EDID blob when size has shrunk. 2. Fix EDID block maps: We skip invalid EEDID blocks, if we do so however we should fix the block maps to keep the EDID consistent. DRM/KMS/EDID: Fix up EEDID Map Blogs if Extension Block Count has changed (v2) 3. Improve handling of 'firmware'-supplied EDIDs: * Move EDID loading from a helper level to DRM core. * If there is a 'firmware'-supplied EDID matching the connector supply it whenever drm_get_edid() is called. * Allow to specify firmware for multiple connectors. * Use the same EDID fault handling as for DDC read EDIDs. DRM/KMS/EDID: Move drm_edid_load.o to drm.ko (v2) DRM/KMS/EDID: Feed 'firmware' supplied EDID blocks whenever the EDID is read (v2) DRM/KMS/EDID: Allow for multiple Connectors when specifying 'firmware'-EDID Files (v2) DRM/KMS/EDID: Use Extension Block Fixup Code also for 'firmware' EDID (v2) 4. Cache EDIDs with extension blocks. I2C transfer of EDIDs with several extension blocks can be quite time consuming. In many cases the EDID has not changed for a given connector since the last read. We detect this by comparing data from the base block. If a match is found we copy the EDID data from cache. This way only the base block needs to be transferred. DRM/KMS/EDID: Cache EDID blobs with extensions (v2) 5. Consolidate EDID fatal error handling: Store information about the error status of the last DDC read in the drm_connector structure. It combines the handling of all zero EDIDs (needed by the radeon driver) and the suppression of consecutive logging of the same EDID error. It will log again when the error pattern has changed (for inststance if the monitor has been exchanged). DRM/KMS/EDID: Consolidate EDID Error Handling (v2) 6. Move EDID-related functions to drm_edid.h The handling of EDID related DRM functions seemed to be a bit arbitrary: although drm_edid.h had been created, new functions were often added to drm_crtc.h DRM/KMS/ast: Include drm_edid.h in file using drm_get_edid(). DRM/KMS/gma500: Include drm_edid.h in file using drm_get_edid(). DRM/KMS/mgag200: Include drm_edid.h in file using drm_get_edid(). DRM/KMS/EDID: Move EDID related Functions to drm_edid.h. drivers/gpu/drm/Kconfig|3 +- drivers/gpu/drm/Makefile |2 +- drivers/gpu/drm/ast/ast_mode.c |1 + drivers/gpu/drm/drm_crtc.c |1 + drivers/gpu/drm/drm_crtc_helper.c |6 +- drivers/gpu/drm/drm_edid.c | 333 drivers/gpu/drm/drm_edid_load.c| 115 -- drivers/gpu/drm/gma500/cdv_intel_dp.c |1 + drivers/gpu/drm/gma500/oaktrail_lvds.c |1 + drivers/gpu/drm/gma500/psb_intel_modes.c |1 + drivers/gpu/drm/mgag200/mgag200_mode.c |1 + drivers/gpu/drm/radeon/radeon_connectors.c |2 +- include/drm/drm_crtc.h | 13 +- include/drm/drm_edid.h | 25 ++- 14 files changed, 370 insertions(+), 135 deletions(-) -- 1.7.7
[PATCH v2 04/18] DRM/KMS/EDID: Don't fail when failing to allocate memory for EDID extensions.
When we fail to allocate space for EDID extensions we should just
warn, fix up the EDID block count and return the base block instead
of failing.
Signed-off-by: Egbert Eich
---
drivers/gpu/drm/drm_edid.c |7 +--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 043ba42..a952cfe 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -347,8 +347,11 @@ drm_do_get_edid(struct drm_connector *connector, struct
i2c_adapter *adapter)
return block;
new = krealloc(block, (block[EDID_EXTENSION_FLAG_OFFSET] + 1) *
EDID_LENGTH, GFP_KERNEL);
- if (!new)
- goto out;
+ if (!new) {
+ dev_warn(connector->dev->dev, "%s: cannot allocate memory for
%d EDID blocks: truncating.\n",
+drm_get_connector_name(connector),
block[EDID_EXTENSION_FLAG_OFFSET] + 1);
+ goto done_fix_extension_count;
+ }
block = new;
for (j = 1; j <= block[EDID_EXTENSION_FLAG_OFFSET]; j++) {
--
1.7.7
[PATCH v2 05/18] DRM/KMS/EDID: Test EDDC if EDID announces more than one Extension Block (v2)
There are displays which announce EDID extension blocks in the
Extension Flag of the EDID base block although they are not EDDC
capable (ie. take a segment address at I2C slave address 0x30).
We test this by looking for an EDID header which is only possible
in the base block.
If the segment address is not taken into account, this block will
be identical to the base block in which case we stop reading further
EEDID blocks, correct the extension flag and just return the base
block.
v2: Split up EDID fixup code into separate commit.
Signed-off-by: Egbert Eich
---
drivers/gpu/drm/drm_edid.c | 13 +
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index a952cfe..5a0e331 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -364,6 +364,19 @@ drm_do_get_edid(struct drm_connector *connector, struct
i2c_adapter *adapter)
}
if (drm_edid_block_valid(block + (valid_extensions + 1)
* EDID_LENGTH, j, print_bad_edid)) {
valid_extensions++;
+ /* Test if base block announced extension
blocks although
+* display is not EDDC capable.
+*/
+ if (j == 2) {
+ int k;
+ for (k = 0; k < sizeof(edid_header);
k++)
+ if (block[(EDID_LENGTH * 2) +
k] != edid_header[k])
+ break;
+ if (k == sizeof(edid_header)) {
+ valid_extensions = 0;
+ goto done_fix_extension_count;
+ }
+ }
break;
}
}
--
1.7.7
[PATCH v2 08/18] DRM/KMS/EDID: Fix up EEDID Map Blogs if Extension Block Count has changed (v2)
EEDID v1.3 mandates map blogs 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 blogs as
this is less time consuming. For v1.3 we scrap any exsisting map
blocks and recreate them from scratch.
v2: Fixed conflits due to reordering of commits.
Signed-off-by: Egbert Eich
---
drivers/gpu/drm/drm_edid.c | 89
1 files changed, 89 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 0fe61fb..9b298fc 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -47,6 +47,7 @@
#define EDID_CHECKSUM_OFFSET offsetof(struct edid, checksum)
#define EDID_VERSION_MAJOR_OFFSET offsetof(struct edid, version)
#define EDID_VERSION_MINOR_OFFSET offsetof(struct edid, revision)
+#define EEDID_BLOCK_MAP_FLAG 0xF0
/*
* EDID blocks out in the wild have a variety of bugs, try to collect
* them here (note that userspace may work around broken monitors first,
@@ -320,6 +321,92 @@ static bool drm_edid_is_zero(u8 *in_edid, int length)
return true;
}
+static void
+fix_map(u8 *block, int cnt)
+{
+ int i;
+ u8 csum;
+
+ if (--cnt > 127)
+ cnt = 127;
+ memset(block, 0, 128);
+ block[0] = EEDID_BLOCK_MAP_FLAG;
+ csum = block[0];
+
+ for (i = 1; i < cnt; i++) {
+ block[i] = block[i * EDID_LENGTH];
+ csum += block[i];
+ }
+ block[127] = (u8)(0x100 - csum);
+}
+
+static int
+fixup_blockmaps(u8 **blockp, int eblock_cnt)
+{
+ int i;
+ u8 *block = *blockp;
+
+ if (block[EDID_VERSION_MAJOR_OFFSET] > 1)
+ return 0;
+ if (block[EDID_VERSION_MINOR_OFFSET] < 3)
+ return 0;
+ if (eblock_cnt == 1) {
+ if (block[EDID_LENGTH] == EEDID_BLOCK_MAP_FLAG)
+ return 0;
+ else
+ return 1;
+ }
+ if (block[EDID_VERSION_MINOR_OFFSET] >= 4) {
+ /* blockmaps are optional: simply toss them */
+ for (i = 1; i <= eblock_cnt; i++) {
+ if (block[i * EDID_LENGTH] == EEDID_BLOCK_MAP_FLAG) {
+ if (i < eblock_cnt)
+ memmove(&block[i * EDID_LENGTH],
+ &block[(i + 1) * EDID_LENGTH],
+ (eblock_cnt-i) * EDID_LENGTH);
+ i--;
+ eblock_cnt--;
+ }
+ }
+ } else {
+ int total_cnt = block[EDID_EXTENSION_FLAG_OFFSET];
+ for (i = 1; i <= eblock_cnt; i++) {
+ if (block[i * EDID_LENGTH] == EEDID_BLOCK_MAP_FLAG) {
+ if (i == 1 || i == 128) /* correct map block
locations */
+ continue;
+ if (i < eblock_cnt)
+ memmove(&block[i * EDID_LENGTH],
+ &block[(i + 1) * EDID_LENGTH],
+ (eblock_cnt-i) * EDID_LENGTH);
+ i--;
+ eblock_cnt--;
+ continue;
+ } else if (i == 1 || i == 128) {
+ if (eblock_cnt >= total_cnt) {
+ u8 *tmp_p;
+ tmp_p = krealloc(block, (eblock_cnt +
2) * EDID_LENGTH, GFP_KERNEL);
+ if (!tmp_p)
+ return eblock_cnt;
+ *blockp = block = tmp_p;
+ total_cnt = eblock_cnt + 1;
+ }
+ eblock_cnt++;
+ memmove(&block[(i + 1) * EDID_LENGTH],
+ &block[i * EDID_LENGTH],
+ (eblock_cnt-i) * EDID_LENGTH);
+ }
+ }
+ fix_map(&block[EDID_LENGTH], eblock_cnt);
+
+ if (eblock_cnt == 129)
+ return 128;
+
+ if (eblock_cnt > 129)
+ fix_map(&block[EDID_LENGTH * 128], eblock_cnt - 127);
+ }
+ return eblock_cnt;
+}
+
static u8 *
drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
{
@@ -394,6 +481,8 @@ drm_do_get_edid(struct drm_connector *connector, struct
i2c_adapter *adapter)
done_fix_extension_count:
if (valid_extensions != block[EDID_EXTENSIO
[PATCH v2 06/18] DRM/KMS/EDID: Don't expect extension blocks for EDID Versions < 1.3.
EDID extension blogs are only expected for EDIDs version 1.3 or higher.
If an EDID with a lower version is found fix the block count in the
extension flags and return the base block.
This should help to avoid issues with older displays with broken
DDC implementations.
Signed-off-by: Egbert Eich
---
drivers/gpu/drm/drm_edid.c |6 ++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 5a0e331..da2f7fa 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -45,6 +45,8 @@
#define EDID_EXTENSION_FLAG_OFFSET offsetof(struct edid, extensions)
#define EDID_CHECKSUM_OFFSET offsetof(struct edid, checksum)
+#define EDID_VERSION_MAJOR_OFFSET offsetof(struct edid, version)
+#define EDID_VERSION_MINOR_OFFSET offsetof(struct edid, revision)
/*
* EDID blocks out in the wild have a variety of bugs, try to collect
* them here (note that userspace may work around broken monitors first,
@@ -346,6 +348,10 @@ drm_do_get_edid(struct drm_connector *connector, struct
i2c_adapter *adapter)
if (block[EDID_EXTENSION_FLAG_OFFSET] == 0)
return block;
+ /* don't expect extension blocks in EDID Versions < 1.3: return base
block with correct extension flag */
+ if (block[EDID_VERSION_MINOR_OFFSET] < 3)
+ goto done_fix_extension_count;
+
new = krealloc(block, (block[EDID_EXTENSION_FLAG_OFFSET] + 1) *
EDID_LENGTH, GFP_KERNEL);
if (!new) {
dev_warn(connector->dev->dev, "%s: cannot allocate memory for
%d EDID blocks: truncating.\n",
--
1.7.7
[PATCH v2 07/18] DRM/KMS/EDID: Don't reallocate EDID blob when size has shrunk.
valid_extensions (the number of EDID extensions found to be valid)
can never be > block[EDID_EXTENSION_FLAG_OFFSET].
There is no point of reallocating the block in this case: the
extra blocks at the end of the EDID structure will not hurt,
also the implementation of krealloc() will just return the same
block.
Signed-off-by: Egbert Eich
---
drivers/gpu/drm/drm_edid.c |4
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index da2f7fa..0fe61fb 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -396,10 +396,6 @@ done_fix_extension_count:
if (valid_extensions != block[EDID_EXTENSION_FLAG_OFFSET]) {
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;
- block = new;
}
return block;
--
1.7.7
[PATCH v2 14/18] DRM/KMS/EDID: Consolidate EDID Error Handling (v2)
Consolidate the null_edid_counter and the bad_edid_counter
into EDID error state flags which for the last EDID read
are accessible from user.
Errors are looged it the same error has not been present
in the previous read of the EDID. This will reset the
EDID error status for example when the monitor is changed
but still prevents permanent EDID errors from piling up
the the kernel logs.
v2: Fixed conflits due to reordering of commits.
Set error state where missing.
Signed-off-by: Egbert Eich
---
drivers/gpu/drm/drm_edid.c | 115 +---
drivers/gpu/drm/radeon/radeon_connectors.c |2 +-
include/drm/drm_crtc.h |4 +-
include/drm/drm_edid.h | 10 +++
4 files changed, 81 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 28b877c..7d8363e 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -157,6 +157,17 @@ int drm_edid_header_is_valid(const u8 *raw_edid)
}
EXPORT_SYMBOL(drm_edid_header_is_valid);
+static bool drm_edid_is_zero(u8 *in_edid, int length)
+{
+ int i;
+ u32 *raw_edid = (u32 *)in_edid;
+
+ for (i = 0; i < length / 4; i++)
+ if (*(raw_edid + i) != 0)
+ return false;
+ return true;
+}
+
static int edid_fixup __read_mostly = 6;
module_param_named(edid_fixup, edid_fixup, int, 0400);
MODULE_PARM_DESC(edid_fixup,
@@ -166,11 +177,13 @@ MODULE_PARM_DESC(edid_fixup,
* Sanity check the EDID block (base or extension). Return 0 if the block
* doesn't check out, or 1 if it's valid.
*/
-bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
+unsigned
+drm_edid_block_check_error(u8 *raw_edid, int block, unsigned last_error_flags)
{
int i;
u8 csum = 0;
struct edid *edid = (struct edid *)raw_edid;
+ unsigned result = 0;
if (edid_fixup > 8 || edid_fixup < 0)
edid_fixup = 6;
@@ -182,27 +195,33 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool
print_bad_edid)
DRM_DEBUG("Fixing EDID header, your hardware may be
failing\n");
memcpy(raw_edid, edid_header, sizeof(edid_header));
} else {
- goto bad;
+ result |= EDID_ERR_NO_BLOCK0;
+ if (drm_edid_is_zero(raw_edid, EDID_LENGTH)) {
+ result |= EDID_ERR_NULL;
+ goto bad;
+ }
}
}
for (i = 0; i < EDID_LENGTH; i++)
csum += raw_edid[i];
if (csum) {
- if (print_bad_edid) {
+ if ((last_error_flags & EDID_ERR_CSUM) == 0)
DRM_ERROR("EDID checksum is invalid, remainder is
%d\n", csum);
- }
/* allow CEA to slide through, switches mangle this */
if (raw_edid[0] != 0x02)
- goto bad;
+ result |= EDID_ERR_CSUM;
}
+ if (result)
+ goto bad;
/* per-block-type checks */
switch (raw_edid[0]) {
case 0: /* base */
if (edid->version != 1) {
DRM_ERROR("EDID has major version %d, instead of 1\n",
edid->version);
+ result |= EDID_ERR_UNSUPPORTED_VERSION;
goto bad;
}
@@ -214,15 +233,23 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool
print_bad_edid)
break;
}
- return 1;
+ return 0;
bad:
- if (raw_edid && print_bad_edid) {
+ if (raw_edid && last_error_flags != result) {
printk(KERN_ERR "Raw EDID:\n");
print_hex_dump(KERN_ERR, " \t", DUMP_PREFIX_NONE, 16, 1,
raw_edid, EDID_LENGTH, false);
}
- return 0;
+ return result;
+}
+
+bool
+drm_edid_block_valid(u8 *raw_edid, int block, unsigned last_error_flags)
+{
+ if (!drm_edid_block_check_error(raw_edid, block, last_error_flags))
+ return true;
+ return false;
}
EXPORT_SYMBOL(drm_edid_block_valid);
@@ -241,7 +268,7 @@ bool drm_edid_is_valid(struct edid *edid)
return false;
for (i = 0; i <= edid->extensions; i++)
- if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true))
+ if (drm_edid_block_check_error(raw + i * EDID_LENGTH, i, true))
return false;
return true;
@@ -310,17 +337,6 @@ drm_do_probe_ddc_edid(struct i2c_adapter *adapter,
unsigned char *buf,
return ret == xfers ? 0 : -1;
}
-static bool drm_edid_is_zero(u8 *in_edid, int length)
-{
- int i;
- u32 *raw_edid = (u32 *)in_edid;
-
- for (i = 0; i < length / 4; i++)
- if (*(raw_edid + i) != 0)
- return fal
[PATCH v2 17/18] DRM/KMS/mgag200: Include drm_edid.h in file using drm_get_edid().
Signed-off-by: Egbert Eich --- drivers/gpu/drm/mgag200/mgag200_mode.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index d3d99a2..f89a0c1 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -15,6 +15,7 @@ #include #include +#include #include "mgag200_drv.h" -- 1.7.7
[PATCH v2 16/18] DRM/KMS/gma500: Include drm_edid.h in file using drm_get_edid().
Signed-off-by: Egbert Eich
---
drivers/gpu/drm/gma500/cdv_intel_dp.c|1 +
drivers/gpu/drm/gma500/oaktrail_lvds.c |1 +
drivers/gpu/drm/gma500/psb_intel_modes.c |1 +
3 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/gma500/cdv_intel_dp.c
b/drivers/gpu/drm/gma500/cdv_intel_dp.c
index e3a3978..37cad73 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_dp.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_dp.c
@@ -34,6 +34,7 @@
#include "psb_intel_drv.h"
#include "psb_intel_reg.h"
#include
+#include
#define _wait_for(COND, MS, W) ({ \
unsigned long timeout__ = jiffies + msecs_to_jiffies(MS); \
diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c
b/drivers/gpu/drm/gma500/oaktrail_lvds.c
index 558c77f..f7e20f5 100644
--- a/drivers/gpu/drm/gma500/oaktrail_lvds.c
+++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c
@@ -22,6 +22,7 @@
#include
#include
+#include
#include
#include "intel_bios.h"
diff --git a/drivers/gpu/drm/gma500/psb_intel_modes.c
b/drivers/gpu/drm/gma500/psb_intel_modes.c
index 4fca0d6..1da014d 100644
--- a/drivers/gpu/drm/gma500/psb_intel_modes.c
+++ b/drivers/gpu/drm/gma500/psb_intel_modes.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include "psb_intel_drv.h"
/**
--
1.7.7
[PATCH v2 15/18] DRM/KMS/ast: Include drm_edid.h in file using drm_get_edid().
Signed-off-by: Egbert Eich --- drivers/gpu/drm/ast/ast_mode.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index 7fc9f72..c27aa8d 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -31,6 +31,7 @@ #include #include #include +#include #include "ast_drv.h" #include "ast_tables.h" -- 1.7.7
[PATCH v2 09/18] DRM/KMS/EDID: Move drm_edid_load.o to drm.ko (v2)
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. v2: a. Also adapt Kconfig b. Add missing 'select FW_LOADER' Signed-off-by: Egbert Eich --- drivers/gpu/drm/Kconfig |3 ++- drivers/gpu/drm/Makefile |2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 18321b68b..4bc239b 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -36,7 +36,8 @@ config DRM_KMS_HELPER config DRM_LOAD_EDID_FIRMWARE bool "Allow to specify an EDID data set instead of probing for it" - depends on DRM_KMS_HELPER + depends on DRM + select FW_LOADER help Say Y here, if you want to use EDID data to be loaded from the /lib/firmware directory or one of the provided built-in 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
[PATCH v2 02/18] DRM/KMS/EDID: 0x7e -> EDID_EXTENSION_FLAG_OFFSET (v2)
v2: Use offsetof().
Signed-off-by: Egbert Eich
---
drivers/gpu/drm/drm_edid.c | 16 +---
1 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 049fa52..9e64069 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -43,6 +43,8 @@
#define EDID_STD_TIMINGS 8
#define EDID_DETAILED_TIMINGS 4
+#define EDID_EXTENSION_FLAG_OFFSET offsetof(struct edid, extensions)
+#define EDID_CHECKSUM_OFFSET offsetof(struct edid, checksum)
/*
* EDID blocks out in the wild have a variety of bugs, try to collect
* them here (note that userspace may work around broken monitors first,
@@ -341,15 +343,15 @@ drm_do_get_edid(struct drm_connector *connector, struct
i2c_adapter *adapter)
goto carp;
/* if there's no extensions, we're done */
- if (block[0x7e] == 0)
+ if (block[EDID_EXTENSION_FLAG_OFFSET] == 0)
return block;
- new = krealloc(block, (block[0x7e] + 1) * EDID_LENGTH, GFP_KERNEL);
+ new = krealloc(block, (block[EDID_EXTENSION_FLAG_OFFSET] + 1) *
EDID_LENGTH, GFP_KERNEL);
if (!new)
goto out;
block = new;
- for (j = 1; j <= block[0x7e]; j++) {
+ for (j = 1; j <= block[EDID_EXTENSION_FLAG_OFFSET]; j++) {
for (i = 0; i < 4; i++) {
if (drm_do_probe_ddc_edid(adapter,
block + (valid_extensions + 1) * EDID_LENGTH,
@@ -366,9 +368,9 @@ drm_do_get_edid(struct drm_connector *connector, struct
i2c_adapter *adapter)
drm_get_connector_name(connector), j);
}
- if (valid_extensions != block[0x7e]) {
- block[EDID_LENGTH-1] += block[0x7e] - valid_extensions;
- block[0x7e] = valid_extensions;
+ if (valid_extensions != block[EDID_EXTENSION_FLAG_OFFSET]) {
+ 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;
@@ -601,7 +603,7 @@ drm_for_each_detailed_block(u8 *raw_edid, detailed_cb *cb,
void *closure)
for (i = 0; i < EDID_DETAILED_TIMINGS; i++)
cb(&(edid->detailed_timings[i]), closure);
- for (i = 1; i <= raw_edid[0x7e]; i++) {
+ for (i = 1; i <= raw_edid[EDID_EXTENSION_FLAG_OFFSET]; i++) {
u8 *ext = raw_edid + (i * EDID_LENGTH);
switch (*ext) {
case CEA_EXT:
--
1.7.7
[PATCH v2 13/18] DRM/KMS/EDID: Cache EDID blobs with extensions (v2)
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.
V2: Use kmemdup() instead of a kmalloc()/memcpy() combo,
erase cache when reading a 'firmware'-supplied EDID or on error.
Signed-off-by: Egbert Eich
---
drivers/gpu/drm/drm_crtc.c |1 +
drivers/gpu/drm/drm_edid.c | 57 +--
include/drm/drm_crtc.h |1 +
include/drm/drm_edid.h |1 +
4 files changed, 52 insertions(+), 8 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 a47fa7f..28b877c 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -419,6 +419,38 @@ fixup_edid(u8 **blockp, int valid_extensions)
}
}
+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 = kmemdup(connector->edid_cache, size,
GFP_KERNEL);
+ if (!new)
+ return false;
+ DRM_DEBUG_KMS("Got EDID for %s from cache.\n",
drm_get_connector_name(connector));
+ kfree(*edidp);
+ *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 = kmemdup(edid, size, GFP_KERNEL);
+ }
+ connector->edid_cache = new;
+}
+
static u8 *
drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
{
@@ -429,28 +461,30 @@ drm_do_get_edid(struct drm_connector *connector, struct
i2c_adapter *adapter)
#ifdef CONFIG_DRM_LOAD_EDID_FIRMWARE
/* check if the user has specified a 'firmware' EDID file */
block = (u8 *)drm_load_edid_firmware(connector);
- if (block)
+ if (block) {
+ drm_cache_edid(connector, NULL);
return block;
+ }
#endif
if ((block = kmalloc(EDID_LENGTH, GFP_KERNEL)) == NULL)
- return NULL;
+ goto error;
/* base block fetch */
for (i = 0; i < 4; i++) {
if (drm_do_probe_ddc_edid(adapter, block, 0, EDID_LENGTH))
- goto out;
+ goto error_free;
if (drm_edid_block_valid(block, 0, print_bad_edid))
break;
if (i == 0 && drm_edid_is_zero(block, EDID_LENGTH)) {
connector->null_edid_counter++;
- goto carp;
+ goto error_carp;
}
}
if (i == 4)
- goto carp;
+ goto error_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;
@@ -458,6 +492,10 @@ drm_do_get_edid(struct drm_connector *connector, struct
i2c_adapter *adapter)
if (block[EDID_VERSION_MINOR_OFFSET] < 3)
goto done_fix_extension_count;
+ /* 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) {
dev_warn(connector->dev->dev, "%s: cannot allocate memory for
%d EDID blocks: truncating.\n",
@@ -500,18 +538,21 @@ drm_
[PATCH v2 11/18] DRM/KMS/EDID: Allow for multiple Connectors when specifying 'firmware'-EDID Files (v2)
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="[:][;:]..."
v2: Check for edidname != NULL before entering loop.
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 748f63b..9c15c4a 100644
--- a/drivers/gpu/drm/drm_edid_load.c
+++ b/drivers/gpu/drm/drm_edid_load.c
@@ -226,26 +226,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);
+ while (edidname) {
+ 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;
+ }
+
+ 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
[PATCH v2 18/18] DRM/KMS/EDID: Move EDID related Functions to drm_edid.h.
Signed-off-by: Egbert Eich --- include/drm/drm_crtc.h |8 include/drm/drm_edid.h |9 + 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index d402b3b..7eed9bd 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -878,9 +878,6 @@ extern char *drm_get_tv_subconnector_name(int val); extern char *drm_get_tv_select_name(int val); extern void drm_fb_release(struct drm_file *file_priv); extern int drm_mode_group_init_legacy_group(struct drm_device *dev, struct drm_mode_group *group); -extern bool drm_probe_ddc(struct i2c_adapter *adapter); -extern struct edid *drm_get_edid(struct drm_connector *connector, -struct i2c_adapter *adapter); extern int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid); extern void drm_mode_probed_add(struct drm_connector *connector, struct drm_display_mode *mode); extern void drm_mode_remove(struct drm_connector *connector, struct drm_display_mode *mode); @@ -1036,9 +1033,6 @@ extern int drm_mode_gamma_get_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); extern int drm_mode_gamma_set_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); -extern u8 *drm_find_cea_extension(struct edid *edid); -extern bool drm_detect_hdmi_monitor(struct edid *edid); -extern bool drm_detect_monitor_audio(struct edid *edid); extern int drm_mode_page_flip_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); extern struct drm_display_mode *drm_cvt_mode(struct drm_device *dev, @@ -1054,8 +1048,6 @@ extern struct drm_display_mode *drm_gtf_mode_complex(struct drm_device *dev, extern int drm_add_modes_noedid(struct drm_connector *connector, int hdisplay, int vdisplay); -extern int drm_edid_header_is_valid(const u8 *raw_edid); -extern bool drm_edid_is_valid(struct edid *edid); struct drm_display_mode *drm_mode_find_dmt(struct drm_device *dev, int hsize, int vsize, int fresh, bool rb); diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index 6bcaee5..72328bc 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -268,4 +268,13 @@ int drm_validate_edid_blob(struct drm_connector *connector, u8 **blockp, int len void drm_cache_edid(struct drm_connector *connector, struct edid *edid); unsigned drm_edid_block_check_error(u8 *raw_edid, int block, unsigned last_error_flags); +extern bool drm_probe_ddc(struct i2c_adapter *adapter); +extern struct edid *drm_get_edid(struct drm_connector *connector, +struct i2c_adapter *adapter); +extern u8 *drm_find_cea_extension(struct edid *edid); +extern bool drm_detect_hdmi_monitor(struct edid *edid); +extern bool drm_detect_monitor_audio(struct edid *edid); +extern int drm_edid_header_is_valid(const u8 *raw_edid); +extern bool drm_edid_is_valid(struct edid *edid); + #endif /* __DRM_EDID_H__ */ -- 1.7.7
[PATCH v2 01/18] DRM/KMS/EDID: Mask out Segment Bits when calculating Offset.
This patch is a bit cosmetic as the variable size will truncate
the start address anyway but for readability it should be made
explicite that the lowest bit in the EDID block number determines
the I2C start address.
Signed-off-by: Egbert Eich
---
drivers/gpu/drm/drm_edid.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index fadcd44..049fa52 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -258,7 +258,7 @@ static int
drm_do_probe_ddc_edid(struct i2c_adapter *adapter, unsigned char *buf,
int block, int len)
{
- unsigned char start = block * EDID_LENGTH;
+ unsigned char start = (block & 0x01) * EDID_LENGTH;
unsigned char segment = block >> 1;
unsigned char xfers = segment ? 3 : 2;
int ret, retries = 5;
--
1.7.7
[PATCH v2 10/18] DRM/KMS/EDID: Feed 'firmware' supplied EDID blocks whenever the EDID is read (v2)
Firmware supplied EDIDs where fed in in
drm_helper_probe_single_connector_modes()
in place of calling the driver supplied get_modes() function.
This has two problems:
1. Drivers don't call drm_get_edid() only from within get_modes().
2. The get_modes() replacement in drm_load_edid_firmware() provided only
the least common denominator of what driver provided get_modes() callbacks
do.
This patch now supplies a user provided 'firmware' EDID whenever drm_get_edid()
is called if one is available. The user supplied EDID is thus used the same way
as a display provided one (except for detecting the presence of a device thru
EDID). Also it does so not on the helper level any more, the possibility for
EDID loading now happens on the DRM KMS core level.
v2: Fixed formatting and conflicts due to reordering of commits.
Signed-off-by: Egbert Eich
---
drivers/gpu/drm/drm_crtc_helper.c |6 +-
drivers/gpu/drm/drm_edid.c|7 +++
drivers/gpu/drm/drm_edid_load.c | 22 +-
include/drm/drm_edid.h|4 +++-
4 files changed, 20 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc_helper.c
b/drivers/gpu/drm/drm_crtc_helper.c
index 1227adf..bc99595 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -119,11 +119,7 @@ int drm_helper_probe_single_connector_modes(struct
drm_connector *connector,
goto prune;
}
-#ifdef CONFIG_DRM_LOAD_EDID_FIRMWARE
- count = drm_load_edid_firmware(connector);
- if (count == 0)
-#endif
- count = (*connector_funcs->get_modes)(connector);
+ count = (*connector_funcs->get_modes)(connector);
if (count == 0 && connector->status == connector_status_connected)
count = drm_add_modes_noedid(connector, 1024, 768);
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 9b298fc..8239c42 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -414,6 +414,13 @@ drm_do_get_edid(struct drm_connector *connector, struct
i2c_adapter *adapter)
u8 *block, *new;
bool print_bad_edid = !connector->bad_edid_counter || (drm_debug &
DRM_UT_KMS);
+#ifdef CONFIG_DRM_LOAD_EDID_FIRMWARE
+ /* check if the user has specified a 'firmware' EDID file */
+ block = (u8 *)drm_load_edid_firmware(connector);
+ if (block)
+ return block;
+#endif
+
if ((block = kmalloc(EDID_LENGTH, GFP_KERNEL)) == NULL)
return NULL;
diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
index 38d3943..748f63b 100644
--- a/drivers/gpu/drm/drm_edid_load.c
+++ b/drivers/gpu/drm/drm_edid_load.c
@@ -114,7 +114,7 @@ static u8 generic_edid[GENERIC_EDIDS][128] = {
},
};
-static u8 *edid_load(struct drm_connector *connector, char *name,
+static struct edid *edid_load(struct drm_connector *connector, char *name,
char *connector_name)
{
const struct firmware *fw;
@@ -222,36 +222,32 @@ out:
return edid;
}
-int drm_load_edid_firmware(struct drm_connector *connector)
+struct edid *
+drm_load_edid_firmware(struct drm_connector *connector)
{
char *connector_name = drm_get_connector_name(connector);
char *edidname = edid_firmware, *last, *colon;
- int ret;
struct edid *edid;
if (*edidname == '\0')
- return 0;
+ return NULL;
colon = strchr(edidname, ':');
if (colon != NULL) {
if (strncmp(connector_name, edidname, colon - edidname))
- return 0;
+ return NULL;
edidname = colon + 1;
if (*edidname == '\0')
- return 0;
+ return NULL;
}
last = edidname + strlen(edidname) - 1;
if (*last == '\n')
*last = '\0';
- edid = (struct edid *) edid_load(connector, edidname, connector_name);
+ edid = edid_load(connector, edidname, connector_name);
if (IS_ERR_OR_NULL(edid))
- return 0;
+ return NULL;
- drm_mode_connector_update_edid_property(connector, edid);
- ret = drm_add_edid_modes(connector, edid);
- kfree(edid);
-
- return ret;
+ return edid;
}
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 0cac551..c0a77bd 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -252,6 +252,8 @@ int drm_av_sync_delay(struct drm_connector *connector,
struct drm_display_mode *mode);
struct drm_connector *drm_select_eld(struct drm_encoder *encoder,
struct drm_display_mode *mode);
-int drm_load_edid_firmware(struct drm_connector *connector);
+#ifdef CONFIG_DRM_LOAD_EDID_FIRMWARE
+struct edid *drm_load_edid_firmware(struct drm_connector *connector);
+#endif
#endif /* __DRM_EDID_H__ */
--
1.
[PATCH v2 03/18] DRM/KMS/EDID: Return Base EDID block if reading EEDID Blocks fails (v2)
If I2C readout fails for an extension block but we have read a
valid base block, don't fail completely but at least return the
base block.
v2: Make goto target names more telling.
Signed-off-by: Egbert Eich
---
drivers/gpu/drm/drm_edid.c |7 +--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 9e64069..043ba42 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -355,8 +355,10 @@ drm_do_get_edid(struct drm_connector *connector, struct
i2c_adapter *adapter)
for (i = 0; i < 4; i++) {
if (drm_do_probe_ddc_edid(adapter,
block + (valid_extensions + 1) * EDID_LENGTH,
- j, EDID_LENGTH))
- goto out;
+ j, EDID_LENGTH)) {
+ valid_extensions = 0;
+ goto done_fix_extension_count;
+ }
if (drm_edid_block_valid(block + (valid_extensions + 1)
* EDID_LENGTH, j, print_bad_edid)) {
valid_extensions++;
break;
@@ -368,6 +370,7 @@ drm_do_get_edid(struct drm_connector *connector, struct
i2c_adapter *adapter)
drm_get_connector_name(connector), j);
}
+done_fix_extension_count:
if (valid_extensions != block[EDID_EXTENSION_FLAG_OFFSET]) {
block[EDID_CHECKSUM_OFFSET] +=
block[EDID_EXTENSION_FLAG_OFFSET] - valid_extensions;
block[EDID_EXTENSION_FLAG_OFFSET] = valid_extensions;
--
1.7.7
[PATCH v2 12/18] DRM/KMS/EDID: Use Extension Block Fixup Code also for 'firmware' EDID (v2)
in drm_edid.c there's now code to fix extension blockmaps if the number
of extensions has changed. This code also rearranges the EDID blocks.
Replace the exisiting EDID rearrange code with a call to this code.
v2: Make adjustments required by patch reordering, add missing memcpy().
Signed-off-by: Egbert Eich
---
drivers/gpu/drm/drm_edid.c | 64 +++
drivers/gpu/drm/drm_edid_load.c | 54 +++-
include/drm/drm_edid.h |1 +
3 files changed, 71 insertions(+), 48 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 8239c42..a47fa7f 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -407,6 +407,18 @@ fixup_blockmaps(u8 **blockp, int eblock_cnt)
return eblock_cnt;
}
+static void
+fixup_edid(u8 **blockp, int valid_extensions)
+{
+ if (valid_extensions != (*blockp)[EDID_EXTENSION_FLAG_OFFSET]) {
+ if (valid_extensions)
+ valid_extensions = fixup_blockmaps(blockp,
valid_extensions);
+
+ (*blockp)[EDID_CHECKSUM_OFFSET] +=
(*blockp)[EDID_EXTENSION_FLAG_OFFSET] - valid_extensions;
+ (*blockp)[EDID_EXTENSION_FLAG_OFFSET] = valid_extensions;
+ }
+}
+
static u8 *
drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
{
@@ -487,12 +499,7 @@ drm_do_get_edid(struct drm_connector *connector, struct
i2c_adapter *adapter)
}
done_fix_extension_count:
- if (valid_extensions != block[EDID_EXTENSION_FLAG_OFFSET]) {
- if (valid_extensions)
- valid_extensions = fixup_blockmaps(&block,
valid_extensions);
- block[EDID_CHECKSUM_OFFSET] +=
block[EDID_EXTENSION_FLAG_OFFSET] - valid_extensions;
- block[EDID_EXTENSION_FLAG_OFFSET] = valid_extensions;
- }
+ fixup_edid(&block, valid_extensions);
return block;
@@ -509,6 +516,51 @@ 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) {
+ int cnt = 0;
+ n_blocks--;
+ if ((*blockp)[EDID_EXTENSION_FLAG_OFFSET] < n_blocks)
+ n_blocks = (*blockp)[EDID_EXTENSION_FLAG_OFFSET];
+
+ while (n_blocks--) {
+ cnt++;
+ if (drm_edid_block_valid(*blockp + cnt * EDID_LENGTH,
+valid_extensions + 1,
print_bad_edid)) {
+ valid_extensions++;
+ if (cnt != valid_extensions)
+ memcpy(*blockp + valid_extensions *
EDID_LENGTH,
+ *blockp + cnt * EDID_LENGTH,
EDID_LENGTH);
+ }
+ }
+ fixup_edid(blockp, valid_extensions);
+ } else
+ 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 9c15c4a..8f26790 100644
--- a/drivers/gpu/drm/drm_edid_load.c
+++ b/drivers/gpu/drm/drm_edid_load.c
@@ -119,11 +119,10 @@ static struct edid *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_simple(connector_name, -1, NULL, 0);
if (IS_ERR(pdev)) {
@@ -137,7 +136,7 @@ static struct edid *edid_load(struct drm_connector
*connector, char *name,
platform_device_unregister(pdev);
if (err) {
- i = 0;
+ int i = 0;
while (i < GENERIC_EDIDS && strcmp(name, generic_edid_name[i]))
i++;
if (i < GENERIC_EDIDS) {
@@ -174,49 +
[git pull] drm fixes
On Thu, Nov 22, 2012 at 2:50 PM, Linus Torvalds
wrote:
> On Wed, Nov 21, 2012 at 6:34 PM, Dave Airlie wrote:
>>
>> its vmware/nouveua/radeon/intel/ttm scattered.
>
> Hmm. That's not what I see. I just see nouveau and soem PCI ID addition.
>
>> 21 files changed, 108 insertions(+), 31 deletions(-)
>
> I get
>
> 14 files changed, 70 insertions(+), 21 deletions(-)
>
> probably due to this.
>
> Forgot to push? The tip of your tree that I see is 452f19201f35d
> ("Merge branch 'drm-fixes-3.7' of
> git://people.freedesktop.org/~agd5f/linux into drm-fixes")
>
> Or is it because you already sent me the intel fixes earlier?
Doh!, yes I picked wrong place to generate report from, okay here is
one corresponding to what you saw,
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/~airlied/linux drm-fixes
Alex Deucher (2):
drm/radeon: properly track the crtc not_enabled case evergreen_mc_stop()
drm/radeon: add new SI pci id
Dave Airlie (3):
Merge branch 'drm-nouveau-fixes' of
git://anongit.freedesktop.org/git/nouveau/linux-2.6 into drm-fixes
Merge branch 'drm-nouveau-fixes' of
git://anongit.freedesktop.org/git/nouveau/linux-2.6 into drm-fixes
Merge branch 'drm-fixes-3.7' of
git://people.freedesktop.org/~agd5f/linux into drm-fixes
Kelly Doran (1):
drm/nvc0/disp: fix thinko in vblank regression fix..
Maarten Lankhorst (2):
drm/nouveau: add missing pll_calc calls
drm/nouveau: use the correct fence implementation for nv50
Marcin Slusarz (3):
drm/nv40: allocate ctxprog with kmalloc
drm/nouveau: fix crash with noaccel=1
drm/nouveau/bios: fix DCB v1.5 parsing
Paul Bolle (1):
radeon: add AGPMode 1 quirk for RV250
drivers/gpu/drm/nouveau/core/engine/disp/nv50.c| 19 ---
.../gpu/drm/nouveau/core/engine/graph/ctxnv40.c| 12 +---
drivers/gpu/drm/nouveau/core/engine/graph/nv40.c |4 +++-
drivers/gpu/drm/nouveau/core/engine/graph/nv40.h |2 +-
drivers/gpu/drm/nouveau/core/include/core/object.h | 14 +-
.../gpu/drm/nouveau/core/include/subdev/clock.h|3 ++-
drivers/gpu/drm/nouveau/core/subdev/bios/dcb.c |2 +-
drivers/gpu/drm/nouveau/core/subdev/clock/nva3.c | 19 +++
drivers/gpu/drm/nouveau/core/subdev/clock/nvc0.c |1 +
drivers/gpu/drm/nouveau/nouveau_abi16.c|4
drivers/gpu/drm/nouveau/nouveau_drm.c |3 ++-
drivers/gpu/drm/radeon/evergreen.c |2 ++
drivers/gpu/drm/radeon/radeon_agp.c|5 -
include/drm/drm_pciids.h |1 +
14 files changed, 70 insertions(+), 21 deletions(-)
[PATCH v2 05/18] DRM/KMS/EDID: Test EDDC if EDID announces more than one Extension Block (v2)
On Thu, Nov 22, 2012 at 05:22:55AM -0500, Egbert Eich wrote:
> There are displays which announce EDID extension blocks in the
> Extension Flag of the EDID base block although they are not EDDC
> capable (ie. take a segment address at I2C slave address 0x30).
> We test this by looking for an EDID header which is only possible
> in the base block.
> If the segment address is not taken into account, this block will
> be identical to the base block in which case we stop reading further
> EEDID blocks, correct the extension flag and just return the base
> block.
>
> v2: Split up EDID fixup code into separate commit.
>
> Signed-off-by: Egbert Eich
> ---
> drivers/gpu/drm/drm_edid.c | 13 +
> 1 files changed, 13 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index a952cfe..5a0e331 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -364,6 +364,19 @@ drm_do_get_edid(struct drm_connector *connector, struct
> i2c_adapter *adapter)
> }
> if (drm_edid_block_valid(block + (valid_extensions + 1)
> * EDID_LENGTH, j, print_bad_edid)) {
> valid_extensions++;
> + /* Test if base block announced extension
> blocks although
> + * display is not EDDC capable.
> + */
> + if (j == 2) {
> + int k;
> + for (k = 0; k < sizeof(edid_header);
> k++)
> + if (block[(EDID_LENGTH * 2) +
> k] != edid_header[k])
> + break;
> + if (k == sizeof(edid_header)) {
> + valid_extensions = 0;
> + goto done_fix_extension_count;
> + }
memcmp()? Also couldn't we just memcmp() the whole block against the base
block, instead of just the header part?
Also the comment is somehow misleading. It talks about the base block
even though we're looking at the extension block.
--
Ville Syrj?l?
Intel OTC
[PATCH v12 3/6] fbmon: add videomode helpers
On Thu, Nov 22, 2012 at 10:07:07AM +0100, Laurent Pinchart wrote: > On Thursday 22 November 2012 09:53:42 Sascha Hauer wrote: > > On Thu, Nov 22, 2012 at 09:50:10AM +0100, Laurent Pinchart wrote: > > > On Thursday 22 November 2012 07:20:00 Sascha Hauer wrote: > > > > On Wed, Nov 21, 2012 at 11:02:44PM +0100, Laurent Pinchart wrote: > > > > > Hi Steffen, > > > > > > > > > > > + > > > > > > + 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); > > > > > > > > > > This will fail if vm->pixelclock >= ((1 << 32) / 1000). The easiest > > > > > solution is probably to use 64-bit computation. > > > > > > > > You have displays with a pixelclock > 4GHz? > > > > > > vm->pixelclock is expressed in Hz. vm->pixelclock * 1000 will thus > > > overflow if the clock frequency is >= ~4.3 MHz. I have displays with a > > > clock frequency higher than that :-) > > > > If vm->pixelclock is in Hz, then the * 1000 above is wrong. > > My bad, I though refresh was expressed in mHz. So yes, the above computation > is wrong. > Okay. I will fix that with the next version... > BTW it seems that the refreshrate field in struct videomode isn't used. It > should then be removed. > ...and remove this field. > I've just realized that the struct videomode fields are not documented. > kerneldoc in include/linux/videomode.h would be a good addition. > 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- |
[PATCH v2 05/18] DRM/KMS/EDID: Test EDDC if EDID announces more than one Extension Block (v2)
Ville Syrj?l? writes:
> On Thu, Nov 22, 2012 at 05:22:55AM -0500, Egbert Eich wrote:
> > There are displays which announce EDID extension blocks in the
> > Extension Flag of the EDID base block although they are not EDDC
> > capable (ie. take a segment address at I2C slave address 0x30).
> > We test this by looking for an EDID header which is only possible
> > in the base block.
> > If the segment address is not taken into account, this block will
> > be identical to the base block in which case we stop reading further
> > EEDID blocks, correct the extension flag and just return the base
> > block.
> >
> > v2: Split up EDID fixup code into separate commit.
> >
> > Signed-off-by: Egbert Eich
> > ---
> > drivers/gpu/drm/drm_edid.c | 13 +
> > 1 files changed, 13 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index a952cfe..5a0e331 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -364,6 +364,19 @@ drm_do_get_edid(struct drm_connector *connector,
> > struct i2c_adapter *adapter)
> >}
> >if (drm_edid_block_valid(block + (valid_extensions + 1)
> > * EDID_LENGTH, j, print_bad_edid)) {
> >valid_extensions++;
> > + /* Test if base block announced extension
> > blocks although
> > + * display is not EDDC capable.
> > + */
> > + if (j == 2) {
> > + int k;
> > + for (k = 0; k < sizeof(edid_header);
> > k++)
> > + if (block[(EDID_LENGTH * 2) +
> > k] != edid_header[k])
> > + break;
> > + if (k == sizeof(edid_header)) {
> > + valid_extensions = 0;
> > + goto done_fix_extension_count;
> > + }
>
> memcmp()? Also couldn't we just memcmp() the whole block against the base
> block, instead of just the header part?
I don't see an advantage of comparing the entire block with the base block:
the signature should already be unique. However I don't insist ;)
Regarding memcmp() you are definitely right, I will change the code.
>
> Also the comment is somehow misleading. It talks about the base block
> even though we're looking at the extension block.
Reason for this patch:
I had a bug report for a monitor announcing extension blocks in the extension
block flag of the base block (over 200!) although it wasn't EDDC capable.
For some reason it got past the ACK check when the segment number was written
to address 0x30 and happily transferred the base block for any odd numbered
block and some garbage for even ones.
The only reliable way we found to catch this condition early was to check if
block 2 had the header of a base block which will happen when the display
cannot deal with the segment number.
This was what I tried to summarize in very few words - maybe i should reword
it a bit.
Thanks!
Egbert.
[PATCH v2 05/18] DRM/KMS/EDID: Test EDDC if EDID announces more than one Extension Block (v2)
On Thu, Nov 22, 2012 at 01:07:28PM +0100, Egbert Eich wrote:
> Ville Syrj?l? writes:
> > On Thu, Nov 22, 2012 at 05:22:55AM -0500, Egbert Eich wrote:
> > > There are displays which announce EDID extension blocks in the
> > > Extension Flag of the EDID base block although they are not EDDC
> > > capable (ie. take a segment address at I2C slave address 0x30).
> > > We test this by looking for an EDID header which is only possible
> > > in the base block.
> > > If the segment address is not taken into account, this block will
> > > be identical to the base block in which case we stop reading further
> > > EEDID blocks, correct the extension flag and just return the base
> > > block.
> > >
> > > v2: Split up EDID fixup code into separate commit.
> > >
> > > Signed-off-by: Egbert Eich
> > > ---
> > > drivers/gpu/drm/drm_edid.c | 13 +
> > > 1 files changed, 13 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > > index a952cfe..5a0e331 100644
> > > --- a/drivers/gpu/drm/drm_edid.c
> > > +++ b/drivers/gpu/drm/drm_edid.c
> > > @@ -364,6 +364,19 @@ drm_do_get_edid(struct drm_connector *connector,
> struct i2c_adapter *adapter)
> > > }
> > > if (drm_edid_block_valid(block +
> (valid_extensions + 1) * EDID_LENGTH, j, print_bad_edid)) {
> > > valid_extensions++;
> > > +/* Test if base block announced
> extension blocks although
> > > + * display is not EDDC capable.
> > > + */
> > > +if (j == 2) {
> > > +int k;
> > > +for (k = 0; k <
> sizeof(edid_header); k++)
> > > +if (block[(EDID_LENGTH
> * 2) + k] != edid_header[k])
> > > +break;
> > > +if (k == sizeof(edid_header)) {
> > > +valid_extensions = 0;
> > > +goto
> done_fix_extension_count;
> > > +}
> >
> > memcmp()? Also couldn't we just memcmp() the whole block against the base
> > block, instead of just the header part?
>
> I don't see an advantage of comparing the entire block with the base block:
> the signature should already be unique. However I don't insist ;)
Me neither. I just figured it might reduce the chance of false
positives. But if you say that can't happen, I'll take your word
for it.
> Regarding memcmp() you are definitely right, I will change the code.
>
> >
> > Also the comment is somehow misleading. It talks about the base block
> > even though we're looking at the extension block.
>
>
> Reason for this patch:
> I had a bug report for a monitor announcing extension blocks in the extension
> block flag of the base block (over 200!) although it wasn't EDDC capable.
> For some reason it got past the ACK check when the segment number was written
> to address 0x30 and happily transferred the base block for any odd numbered
> block and some garbage for even ones.
That's nasty. When I saw your patch I was immediately thinking that this
is caused by the IGNORE_NAK flag, but then I read the current code, and
realized that we're not using that flag. It was used in the original
EDDC patch, and I was worried that this kind of bad behaviour would be
possible if use the flag. But it seems I underestimated how crappy the
monitor hardwar can be.
> The only reliable way we found to catch this condition early was to check if
> block 2 had the header of a base block which will happen when the display
> cannot deal with the segment number.
>
> This was what I tried to summarize in very few words - maybe i should reword
> it a bit.
Right. The idea seems reasonable, I just found the comment somehow a
bit confusing when I was reading the code following it.
So maybe something like 'Test if base block ..., by checking whether the
extension block is a duplicate the base block.' Although the use of
memcmp() will already make things much clearer.
--
Ville Syrj??l??
Intel OTC
Linux 3.7-rc6
On Thu, Nov 22, 2012 at 12:18 PM, Henrik Rydberg wrote: >> My apologies for the long delay in answering, I've somehow mixed up >> different bugreports and thought I've sent you a patch to test >> already. Anyway, please test >> >> https://patchwork.kernel.org/patch/1728111/ > > Tested-by: Henrik Rydberg Can you please boot with drm.debug=0xe added to your kernel cmdline with that patch applied and attach the full dmesg? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH v2 05/18] DRM/KMS/EDID: Test EDDC if EDID announces more than one Extension Block (v2)
Ville Syrj?l? writes: > > Me neither. I just figured it might reduce the chance of false > positives. But if you say that can't happen, I'll take your word > for it. > > > Regarding memcmp() you are definitely right, I will change the code. > > > > > > > > Also the comment is somehow misleading. It talks about the base block > > > even though we're looking at the extension block. > > > > > > Reason for this patch: > > I had a bug report for a monitor announcing extension blocks in the > > extension > > block flag of the base block (over 200!) although it wasn't EDDC capable. > > For some reason it got past the ACK check when the segment number was > > written > > to address 0x30 and happily transferred the base block for any odd > > numbered > > block and some garbage for even ones. > > That's nasty. When I saw your patch I was immediately thinking that this > is caused by the IGNORE_NAK flag, but then I read the current code, and > realized that we're not using that flag. It was used in the original > EDDC patch, and I was worried that this kind of bad behaviour would be > possible if use the flag. But it seems I underestimated how crappy the > monitor hardwar can be. Right. This flag would only make sense for the base block (to reset an EDDC capable monitor but not fail on non-EDDC capable ones). In fact I had a patch to add this, but then I realized that none of the I2C implemenations in the gfx drivers honored this flag. But then I reread the VESA spec for EDDC, this requires that the segment address in the monitor has to be reset on a STOP condition. So if there ever is the case that a monitor has a segment > 0 selected when drm_read_edid() is called for the first time (because of a system crash for instance) the validation will fail and causes a 2nd read attempt for which the segment address should be reset by the previous transfer. Regarding crappiness of monitor hardware: catch me at a bar and I can tell you more stories ... ;p > > > The only reliable way we found to catch this condition early was to check > > if > > block 2 had the header of a base block which will happen when the display > > cannot deal with the segment number. > > > > This was what I tried to summarize in very few words - maybe i should > > reword > > it a bit. > > Right. The idea seems reasonable, I just found the comment somehow a > bit confusing when I was reading the code following it. > > So maybe something like 'Test if base block ..., by checking whether the > extension block is a duplicate the base block.' Although the use of > memcmp() will already make things much clearer. Right. I've already reworked it. Will send out. Cheers, Egbert.
[PATCH 2/2] drm: exynos: compose and send avi and aui info frames
On Thu, Nov 22, 2012 at 12:06 PM, ??? wrote:
> On 2012? 11? 21? 20:36, Rahul Sharma wrote:
>> Hi Seung Woo,
>>
>> Thanks for your inputs. Please find my response below.
>>
>> On Wed, Nov 21, 2012 at 2:12 PM, ??? wrote:
>>> Hi Rahul,
>>>
>>> Control part seems good, and my comment is below.
>>>
>>> On 2012? 11? 10? 01:21, Rahul Sharma wrote:
This patch adds code for composing AVI and AUI info frames
and send them every VSYNC.
This patch is important for hdmi certification.
Signed-off-by: Fahad Kunnathadi
Signed-off-by: Shirish S
Signed-off-by: Rahul Sharma
---
drivers/gpu/drm/exynos/exynos_hdmi.c | 97
+-
drivers/gpu/drm/exynos/exynos_hdmi.h | 23
drivers/gpu/drm/exynos/regs-hdmi.h | 17 +-
3 files changed, 133 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 2c115f8..bb8a045 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>>>
>>>
>>>
@@ -1993,6 +2084,8 @@ static void hdmi_mode_set(void *ctx, void *mode)
DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
+ hdata->cur_video_id = drm_match_cea_mode(mode);
+
>>>
>>> How do you think about using predefined cea video id in struct
>>> hdmi_conf? drm_mode does not have cea video id, so drm_match_cea_mode()
>>> compares only mode information. Considering this, IMHO, cea video id can
>>> be embedded in struct hdmi_conf.
>>>
>>
>> I feel, It will leads to duplication of video id information. In
>> edid_cea_modes, modes are
>> strictly arranged in the order of respective cea video ID codes.
>> "drm_add_edid_modes"
>> also passes the cea codes (recieved after edid data parsing) as the index to
>> edid_cea_modes to get mode details.
>
> It might be a concern related with your first patch, anyway
> edid_cea_modes has few pair of exact same modes because struct drm_mode
> does not have picture ratio. For example, video id 2 and 3 have exact
> same values for struct drm_mode. So cea video id can be used to get a
> mode, but a drm_mode is not sufficient to get exact video id.
> Considering that exynos hdmi does not support video ids with same mode,
> I suggested video id in struct hdmi_conf.
> At the point of exynos drm, I can ack this patch.
You are right. This ambiguity is still present about the video code when
drm framework sets the mode to hdmi. hdmi_check_timing also doesn't care
about picture aspect ratio. I am not sure how to get exact vic from the mode.
I have submitted another patch that where vic is provided the hdmi_conf.
I preferred 16:9 aspect ratio. Kindly review that.
regards,
Rahul Sharma
>
>>
>> Secondly, mode to cea code translation is required by all platforms
>> for AVI packet
>> composition. By adding it to hdmi_conf, we are limiting its usage for exynos.
>
> I agree with you at this point. I quickly checked i915 and radeon and I
> found that they use fixed value for avi packet at sw level, but I don't
> have information hw can properly build avi packet. If they also need
> video id for building avi packet, video id translation can be used.
>
> Best Regards,
> - Seung-Woo Kim
>
>>
>> regards,
>> Rahul Sharma.
>>
conf_idx = hdmi_conf_index(hdata, mode);
if (conf_idx >= 0)
hdata->cur_conf = conf_idx;
>>>
>>>
>>>
>>> Thanks and Regards,
>>> - Seung-Woo Kim
>>>
>>> --
>>> Seung-Woo Kim
>>> Samsung Software R&D Center
>>> --
>>>
>>> ___
>>> dri-devel mailing list
>>> dri-devel at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
[PATCH] libdrm: add missing DRM_CAP_TIMESTAMP_MONOTONIC
Just add the definition according the kernel's copy of drm.h
Signed-off-by: Imre Deak
---
include/drm/drm.h |1 +
1 file changed, 1 insertion(+)
diff --git a/include/drm/drm.h b/include/drm/drm.h
index a847689..d14b973 100644
--- a/include/drm/drm.h
+++ b/include/drm/drm.h
@@ -779,6 +779,7 @@ struct drm_event_vblank {
#define DRM_CAP_DUMB_PREFERRED_DEPTH 0x3
#define DRM_CAP_DUMB_PREFER_SHADOW 0x4
#define DRM_CAP_PRIME 0x5
+#define DRM_CAP_TIMESTAMP_MONOTONIC 0x6
#define DRM_PRIME_CAP_IMPORT 0x1
#define DRM_PRIME_CAP_EXPORT 0x2
--
1.7.9.5
[PATCH v2 13/18] DRM/KMS/EDID: Cache EDID blobs with extensions (v2)
On Thu, Nov 22, 2012 at 05:23:03AM -0500, Egbert Eich 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.
>
> V2: Use kmemdup() instead of a kmalloc()/memcpy() combo,
> erase cache when reading a 'firmware'-supplied EDID or on error.
>
> Signed-off-by: Egbert Eich
> ---
> drivers/gpu/drm/drm_crtc.c |1 +
> drivers/gpu/drm/drm_edid.c | 57 +--
> include/drm/drm_crtc.h |1 +
> include/drm/drm_edid.h |1 +
> 4 files changed, 52 insertions(+), 8 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 a47fa7f..28b877c 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -419,6 +419,38 @@ fixup_edid(u8 **blockp, int valid_extensions)
> }
> }
>
> +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 = kmemdup(connector->edid_cache, size,
> GFP_KERNEL);
> + if (!new)
> + return false;
> + DRM_DEBUG_KMS("Got EDID for %s from cache.\n",
> drm_get_connector_name(connector));
> + kfree(*edidp);
> + *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 = kmemdup(edid, size, GFP_KERNEL);
> + }
> + connector->edid_cache = new;
> +}
> +
> static u8 *
> drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
> {
> @@ -429,28 +461,30 @@ drm_do_get_edid(struct drm_connector *connector, struct
> i2c_adapter *adapter)
> #ifdef CONFIG_DRM_LOAD_EDID_FIRMWARE
> /* check if the user has specified a 'firmware' EDID file */
> block = (u8 *)drm_load_edid_firmware(connector);
> - if (block)
> + if (block) {
> + drm_cache_edid(connector, NULL);
> return block;
> + }
> #endif
>
> if ((block = kmalloc(EDID_LENGTH, GFP_KERNEL)) == NULL)
> - return NULL;
> + goto error;
>
> /* base block fetch */
> for (i = 0; i < 4; i++) {
> if (drm_do_probe_ddc_edid(adapter, block, 0, EDID_LENGTH))
> - goto out;
> + goto error_free;
> if (drm_edid_block_valid(block, 0, print_bad_edid))
> break;
> if (i == 0 && drm_edid_is_zero(block, EDID_LENGTH)) {
> connector->null_edid_counter++;
> - goto carp;
> + goto error_carp;
> }
> }
> if (i == 4)
> - goto carp;
> + goto error_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;
Shouldn't you erase the cache here too?
--
Ville Syrj?l?
Intel OTC
[PATCH v2 13/18] DRM/KMS/EDID: Cache EDID blobs with extensions (v2)
Ville Syrj?l? writes: > On Thu, Nov 22, 2012 at 05:23:03AM -0500, Egbert Eich wrote: > > > > - /* 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; > > Shouldn't you erase the cache here too? > Indeed. And I had fixed it. It went into the wrong patch however :( Will correct. Thanks, Egbert.
[Bug 50881] Error compiling nouveau inside the kernel (not as a module) when ACPI is a module
https://bugzilla.kernel.org/show_bug.cgi?id=50881 Alan changed: What|Removed |Added CC||alan at lxorguk.ukuu.org.uk Component|Video(Other)|Video(DRI - non Intel) AssignedTo|drivers_video-other at kernel- |drivers_video-dri at kernel-bu |bugs.osdl.org |gs.osdl.org --- Comment #2 from Alan 2012-11-22 14:44:47 --- Looks to be the case: there is a missing depend line I think -- Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are watching the assignee of the bug.
[PATCH v3] DRM/KMS/EDID: Test EDDC if EDID announces more than one Extension Block (v3)
There are displays which announce EDID extension blocks in the
Extension Flag of the EDID base block although they are not EDDC
capable (ie. take a segment address at I2C slave address 0x30).
We test this by looking for an EDID header which is only possible
in the base block.
If the segment address is not taken into account, this block will
be identical to the base block in which case we stop reading further
EEDID blocks, correct the extension flag and just return the base
block.
v2: Split up EDID fixup code into separate commit.
v3: Worked in changes suggested by Ville Syrj?l? :
Reworded comment,
Used memcmp(),
Compared entire base block instead of signature only.
Signed-off-by: Egbert Eich
---
drivers/gpu/drm/drm_edid.c | 10 ++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index a952cfe..b42fbc0 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -364,6 +364,16 @@ drm_do_get_edid(struct drm_connector *connector, struct
i2c_adapter *adapter)
}
if (drm_edid_block_valid(block + (valid_extensions + 1)
* EDID_LENGTH, j, print_bad_edid)) {
valid_extensions++;
+ /* If extension block 2 is identical to the
base block the display is probably
+* not EDDC cabable - despite of what the
extension flag says - as it doesn't
+* select the correct segment address: detect
this condition and bail early.
+*/
+ if (j == 2) {
+ if (memcmp(block + EDID_LENGTH * 2,
block, EDID_LENGTH) == 0) {
+ valid_extensions = 0;
+ goto done_fix_extension_count;
+ }
+ }
break;
}
}
--
1.7.7
[PATCH v3] DRM/KMS/EDID: Cache EDID blobs with extensions (v3)
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.
V2: Use kmemdup() instead of a kmalloc()/memcpy() combo,
erase cache when reading a 'firmware'-supplied EDID or on error.
V3: Uncache when only one extension block is found. This chunk had
accidentally gone into the next patch of the series. Found by
Ville Syrj?l? .
Signed-off-by: Egbert Eich
---
drivers/gpu/drm/drm_crtc.c |1 +
drivers/gpu/drm/drm_edid.c | 60 +--
include/drm/drm_crtc.h |1 +
include/drm/drm_edid.h |1 +
4 files changed, 54 insertions(+), 9 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 e269739..dd0df60 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -419,6 +419,38 @@ fixup_edid(u8 **blockp, int valid_extensions)
}
}
+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 = kmemdup(connector->edid_cache, size,
GFP_KERNEL);
+ if (!new)
+ return false;
+ DRM_DEBUG_KMS("Got EDID for %s from cache.\n",
drm_get_connector_name(connector));
+ kfree(*edidp);
+ *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 = kmemdup(edid, size, GFP_KERNEL);
+ }
+ connector->edid_cache = new;
+}
+
static u8 *
drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
{
@@ -429,35 +461,41 @@ drm_do_get_edid(struct drm_connector *connector, struct
i2c_adapter *adapter)
#ifdef CONFIG_DRM_LOAD_EDID_FIRMWARE
/* check if the user has specified a 'firmware' EDID file */
block = (u8 *)drm_load_edid_firmware(connector);
- if (block)
+ if (block) {
+ drm_cache_edid(connector, NULL);
return block;
+ }
#endif
if ((block = kmalloc(EDID_LENGTH, GFP_KERNEL)) == NULL)
- return NULL;
+ goto error;
/* base block fetch */
for (i = 0; i < 4; i++) {
if (drm_do_probe_ddc_edid(adapter, block, 0, EDID_LENGTH))
- goto out;
+ goto error_free;
if (drm_edid_block_valid(block, 0, print_bad_edid))
break;
if (i == 0 && drm_edid_is_zero(block, EDID_LENGTH)) {
connector->null_edid_counter++;
- goto carp;
+ goto error_carp;
}
}
if (i == 4)
- goto carp;
+ goto error_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;
+ goto done;
/* don't expect extension blocks in EDID Versions < 1.3: return base
block with correct extension flag */
if (block[EDID_VERSION_MINOR_OFFSET] < 3)
goto done_fix_extension_count;
+ /* 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_OFFS
[PATCH v3] DRM/KMS/EDID: Consolidate EDID Error Handling (v3)
Consolidate the null_edid_counter and the bad_edid_counter
into EDID error state flags which for the last EDID read
are accessible from user.
Errors are looged it the same error has not been present
in the previous read of the EDID. This will reset the
EDID error status for example when the monitor is changed
but still prevents permanent EDID errors from piling up
the the kernel logs.
v2: Fixed conflits due to reordering of commits.
Set error state where missing.
v3: Don't update cache when returning block from cache.
Signed-off-by: Egbert Eich
---
drivers/gpu/drm/drm_edid.c | 117 +---
drivers/gpu/drm/radeon/radeon_connectors.c |2 +-
include/drm/drm_crtc.h |4 +-
include/drm/drm_edid.h | 10 +++
4 files changed, 82 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index dd0df60..aa9b34d 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -157,6 +157,17 @@ int drm_edid_header_is_valid(const u8 *raw_edid)
}
EXPORT_SYMBOL(drm_edid_header_is_valid);
+static bool drm_edid_is_zero(u8 *in_edid, int length)
+{
+ int i;
+ u32 *raw_edid = (u32 *)in_edid;
+
+ for (i = 0; i < length / 4; i++)
+ if (*(raw_edid + i) != 0)
+ return false;
+ return true;
+}
+
static int edid_fixup __read_mostly = 6;
module_param_named(edid_fixup, edid_fixup, int, 0400);
MODULE_PARM_DESC(edid_fixup,
@@ -166,11 +177,13 @@ MODULE_PARM_DESC(edid_fixup,
* Sanity check the EDID block (base or extension). Return 0 if the block
* doesn't check out, or 1 if it's valid.
*/
-bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
+unsigned
+drm_edid_block_check_error(u8 *raw_edid, int block, unsigned last_error_flags)
{
int i;
u8 csum = 0;
struct edid *edid = (struct edid *)raw_edid;
+ unsigned result = 0;
if (edid_fixup > 8 || edid_fixup < 0)
edid_fixup = 6;
@@ -182,27 +195,33 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool
print_bad_edid)
DRM_DEBUG("Fixing EDID header, your hardware may be
failing\n");
memcpy(raw_edid, edid_header, sizeof(edid_header));
} else {
- goto bad;
+ result |= EDID_ERR_NO_BLOCK0;
+ if (drm_edid_is_zero(raw_edid, EDID_LENGTH)) {
+ result |= EDID_ERR_NULL;
+ goto bad;
+ }
}
}
for (i = 0; i < EDID_LENGTH; i++)
csum += raw_edid[i];
if (csum) {
- if (print_bad_edid) {
+ if ((last_error_flags & EDID_ERR_CSUM) == 0)
DRM_ERROR("EDID checksum is invalid, remainder is
%d\n", csum);
- }
/* allow CEA to slide through, switches mangle this */
if (raw_edid[0] != 0x02)
- goto bad;
+ result |= EDID_ERR_CSUM;
}
+ if (result)
+ goto bad;
/* per-block-type checks */
switch (raw_edid[0]) {
case 0: /* base */
if (edid->version != 1) {
DRM_ERROR("EDID has major version %d, instead of 1\n",
edid->version);
+ result |= EDID_ERR_UNSUPPORTED_VERSION;
goto bad;
}
@@ -214,15 +233,23 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool
print_bad_edid)
break;
}
- return 1;
+ return 0;
bad:
- if (raw_edid && print_bad_edid) {
+ if (raw_edid && last_error_flags != result) {
printk(KERN_ERR "Raw EDID:\n");
print_hex_dump(KERN_ERR, " \t", DUMP_PREFIX_NONE, 16, 1,
raw_edid, EDID_LENGTH, false);
}
- return 0;
+ return result;
+}
+
+bool
+drm_edid_block_valid(u8 *raw_edid, int block, unsigned last_error_flags)
+{
+ if (!drm_edid_block_check_error(raw_edid, block, last_error_flags))
+ return true;
+ return false;
}
EXPORT_SYMBOL(drm_edid_block_valid);
@@ -241,7 +268,7 @@ bool drm_edid_is_valid(struct edid *edid)
return false;
for (i = 0; i <= edid->extensions; i++)
- if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true))
+ if (drm_edid_block_check_error(raw + i * EDID_LENGTH, i, true))
return false;
return true;
@@ -310,17 +337,6 @@ drm_do_probe_ddc_edid(struct i2c_adapter *adapter,
unsigned char *buf,
return ret == xfers ? 0 : -1;
}
-static bool drm_edid_is_zero(u8 *in_edid, int length)
-{
- int i;
- u32 *raw_edid = (u32 *)in_edid;
-
- for (i = 0; i < length / 4; i++)
- if (
[PATCH 04/10] drm/ttm: change fence_lock to inner lock, v3
Op 21-11-12 14:27, Thomas Hellstrom schreef: > On 11/21/2012 02:12 PM, Maarten Lankhorst wrote: >> Op 21-11-12 13:42, Thomas Hellstrom schreef: >>> On 11/21/2012 12:38 PM, Maarten Lankhorst wrote: Hey, Op 20-11-12 16:08, Thomas Hellstrom schreef: > 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 th
[PATCHv13 6/7] 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
Acked-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..3d0ccaa 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -85,6 +85,7 @@ struct module;
struct drm_file;
struct drm_device;
+struct videomode;
#include
#include
#include
@@ -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
[PATCHv13 1/7] viafb: rename display_timing to via_display_timing
The struct display_timing is specific to the via subsystem. The naming leads to
collisions with the new struct display_timing, that is supposed to be a shared
struct between different subsystems.
To clean this up, prepend the existing struct with the subsystem it is specific
to.
Signed-off-by: Steffen Trumtrar
---
drivers/video/via/hw.c |6 +++---
drivers/video/via/hw.h |2 +-
drivers/video/via/lcd.c |2 +-
drivers/video/via/share.h |2 +-
drivers/video/via/via_modesetting.c |8
drivers/video/via/via_modesetting.h |6 +++---
6 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/video/via/hw.c b/drivers/video/via/hw.c
index 898590d..5563c67 100644
--- a/drivers/video/via/hw.c
+++ b/drivers/video/via/hw.c
@@ -1467,10 +1467,10 @@ void viafb_set_vclock(u32 clk, int set_iga)
via_write_misc_reg_mask(0x0C, 0x0C); /* select external clock */
}
-struct display_timing var_to_timing(const struct fb_var_screeninfo *var,
+struct via_display_timing var_to_timing(const struct fb_var_screeninfo *var,
u16 cxres, u16 cyres)
{
- struct display_timing timing;
+ struct via_display_timing timing;
u16 dx = (var->xres - cxres) / 2, dy = (var->yres - cyres) / 2;
timing.hor_addr = cxres;
@@ -1491,7 +1491,7 @@ struct display_timing var_to_timing(const struct
fb_var_screeninfo *var,
void viafb_fill_crtc_timing(const struct fb_var_screeninfo *var,
u16 cxres, u16 cyres, int iga)
{
- struct display_timing crt_reg = var_to_timing(var,
+ struct via_display_timing crt_reg = var_to_timing(var,
cxres ? cxres : var->xres, cyres ? cyres : var->yres);
if (iga == IGA1)
diff --git a/drivers/video/via/hw.h b/drivers/video/via/hw.h
index 6be243c..c3f2572 100644
--- a/drivers/video/via/hw.h
+++ b/drivers/video/via/hw.h
@@ -637,7 +637,7 @@ extern int viafb_LCD_ON;
extern int viafb_DVI_ON;
extern int viafb_hotplug;
-struct display_timing var_to_timing(const struct fb_var_screeninfo *var,
+struct via_display_timing var_to_timing(const struct fb_var_screeninfo *var,
u16 cxres, u16 cyres);
void viafb_fill_crtc_timing(const struct fb_var_screeninfo *var,
u16 cxres, u16 cyres, int iga);
diff --git a/drivers/video/via/lcd.c b/drivers/video/via/lcd.c
index 1650379..022b0df 100644
--- a/drivers/video/via/lcd.c
+++ b/drivers/video/via/lcd.c
@@ -549,7 +549,7 @@ void viafb_lcd_set_mode(const struct fb_var_screeninfo
*var, u16 cxres,
int panel_hres = plvds_setting_info->lcd_panel_hres;
int panel_vres = plvds_setting_info->lcd_panel_vres;
u32 clock;
- struct display_timing timing;
+ struct via_display_timing timing;
struct fb_var_screeninfo panel_var;
const struct fb_videomode *mode_crt_table, *panel_crt_table;
diff --git a/drivers/video/via/share.h b/drivers/video/via/share.h
index 3158dfc..65c65c6 100644
--- a/drivers/video/via/share.h
+++ b/drivers/video/via/share.h
@@ -319,7 +319,7 @@ struct crt_mode_table {
int refresh_rate;
int h_sync_polarity;
int v_sync_polarity;
- struct display_timing crtc;
+ struct via_display_timing crtc;
};
struct io_reg {
diff --git a/drivers/video/via/via_modesetting.c
b/drivers/video/via/via_modesetting.c
index 0e431ae..0b414b0 100644
--- a/drivers/video/via/via_modesetting.c
+++ b/drivers/video/via/via_modesetting.c
@@ -30,9 +30,9 @@
#include "debug.h"
-void via_set_primary_timing(const struct display_timing *timing)
+void via_set_primary_timing(const struct via_display_timing *timing)
{
- struct display_timing raw;
+ struct via_display_timing raw;
raw.hor_total = timing->hor_total / 8 - 5;
raw.hor_addr = timing->hor_addr / 8 - 1;
@@ -88,9 +88,9 @@ void via_set_primary_timing(const struct display_timing
*timing)
via_write_reg_mask(VIACR, 0x17, 0x80, 0x80);
}
-void via_set_secondary_timing(const struct display_timing *timing)
+void via_set_secondary_timing(const struct via_display_timing *timing)
{
- struct display_timing raw;
+ struct via_display_timing raw;
raw.hor_total = timing->hor_total - 1;
raw.hor_addr = timing->hor_addr - 1;
diff --git a/drivers/video/via/via_modesetting.h
b/drivers/video/via/via_modesetting.h
index 06e09fe..f6a6503 100644
--- a/drivers/video/via/via_modesetting.h
+++ b/drivers/video/via/via_modesetting.h
@@ -33,7 +33,7 @@
#define VIA_PITCH_MAX 0x3FF8
-struct display_timing {
+struct via_display_timing {
u16 hor_total;
u16 hor_addr;
u16 hor_blank_start;
@@ -49,8 +49,8 @@ struct display_timing {
};
-void via_set_primary_timing(const struct display_timing *timing);
-void via_set_secondary_timing(const struct display_timing *timing);
+void via_set_primary_timing(const struct via_display_timing *timing);
+void via_set_secondary_timing(const struct via_display_timing *timing);
void via_set_primary_a
[PATCHv13 5/7] 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
Acked-by: Laurent Pinchart
---
drivers/video/fbmon.c | 42 +-
include/linux/fb.h|6 ++
2 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index a6a564d..cd0a035 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
@@ -1416,6 +1416,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 4404ec2..43a2f81 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -20,6 +20,7 @@ struct fb_info;
struct device;
struct file;
struct videomode;
+struct device_node;
/* Definitions below are used in the parsed monitor specs */
#define FB_DPMS_ACTIVE_OFF 1
@@ -715,6 +716,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
[PATCHv13 2/7] 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.
Also, add helper functions to convert from display timings to a generic
videomode
structure.
The struct display_timing specifies all needed parameters to describe the signal
properties of a display in one mode. This includes
- ranges for signals that may have min-, max- and typical values
- single integers for signals that can be on, off or are ignored
- booleans for signals that are either on or off
As a display may support multiple modes like this, a struct display_timings is
added, that holds all given struct display_timing pointers and declares the
native mode of the display.
Although a display may state that a signal can be in a range, it is driven with
fixed values that indicate a videomode. Therefore graphic drivers don't need all
the information of struct display_timing, but would generate a videomode from
the given set of supported signal timings and work with that.
The video subsystems all define their own structs that describe a mode and work
with that (e.g. fb_videomode or drm_display_mode). To slowly replace all those
various structures and allow code reuse across those subsystems, add struct
videomode as a generic description.
This patch only includes the most basic fields in struct videomode. All missing
fields that are needed to have a really generic video mode description can be
added at a later stage.
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
Acked-by: Laurent Pinchart
---
drivers/video/Kconfig |6 +++
drivers/video/Makefile |2 +
drivers/video/display_timing.c | 24 ++
drivers/video/videomode.c | 45 +
include/linux/display_timing.h | 104
include/linux/videomode.h | 52
6 files changed, 233 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..86a8558
--- /dev/null
+++ b/drivers/video/videomode.c
@@ -0,0 +1,45 @@
+/*
+ * 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, TE_TYP);
+ vm->hactive = display_timing_get_value(&dt->hactive, TE_TYP);
+ vm->hfront_porch = display_timing_get_value(&dt->hfront_porch, TE_TYP);
+ vm->hback_porch = display_timing_get_value(&dt->hback_porch, TE_TYP);
+ vm->hsync_len = display_timing_get_value(&dt->hsync_len, TE_TYP);
+
+ vm->vactive = display_timing_get_value(&dt->vactive, TE_TYP);
+ vm->vfront_porch = display_timing_get_value(&d
[PATCHv13 0/7] of: add display helper
Hi! Changes since v12: - rename struct display_timing to via_display_timing in via subsystem - fix refreshrate calculation - fix "const struct *" warnings (reported by: "Manjunathappa, Prakash" ) - some CodingStyle fixes - rewrite parts of commit messages and display-timings.txt - let display_timing_get_value get all values instead of just typical Regards, Steffen Steffen Trumtrar (7): viafb: rename display_timing to via_display_timing video: add display_timing and videomode video: add of helper for display timings/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| 69 ++ drivers/video/Kconfig | 21 ++ drivers/video/Makefile |4 + drivers/video/display_timing.c | 24 +++ drivers/video/fbmon.c | 84 drivers/video/of_display_timing.c | 223 drivers/video/of_videomode.c | 48 + drivers/video/via/hw.c |6 +- drivers/video/via/hw.h |2 +- drivers/video/via/lcd.c|2 +- drivers/video/via/share.h |2 +- drivers/video/via/via_modesetting.c|8 +- drivers/video/via/via_modesetting.h|6 +- drivers/video/videomode.c | 45 include/drm/drmP.h | 12 ++ include/linux/display_timing.h | 104 + include/linux/fb.h | 12 ++ include/linux/of_display_timings.h | 20 ++ include/linux/of_videomode.h | 18 ++ include/linux/videomode.h | 52 + 21 files changed, 856 insertions(+), 13 deletions(-) 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
[PATCHv13 7/7] 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
Acked-by: Laurent Pinchart
---
drivers/gpu/drm/drm_modes.c | 34 +-
include/drm/drmP.h |6 ++
2 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 0073b27..2d6edfa 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -35,7 +35,7 @@
#include
#include
#include
-#include
+#include
/**
* drm_mode_debug_printmodeline - debug print a mode
@@ -541,6 +541,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(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 3d0ccaa..84ecabd 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -86,6 +86,7 @@ struct drm_file;
struct drm_device;
struct videomode;
+struct device_node;
#include
#include
#include
@@ -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(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
[PATCHv13 4/7] 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
Acked-by: Laurent Pinchart
Signed-off-by: Steffen Trumtrar
---
drivers/video/fbmon.c | 44
include/linux/fb.h|6 ++
2 files changed, 50 insertions(+)
diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index cef6557..a6a564d 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,49 @@ 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;
+ 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 / (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..4404ec2 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -19,6 +19,7 @@ struct vm_area_struct;
struct fb_info;
struct device;
struct file;
+struct videomode;
/* Definitions below are used in the parsed monitor specs */
#define FB_DPMS_ACTIVE_OFF 1
@@ -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
[PATCHv13 3/7] video: add of helper for display timings/videomode
This adds support for reading display timings from DT into a struct display_timings. The of_display_timing implementation supports multiple subnodes. All children are read into an array, that can be queried. If no native mode is specified, the first subnode will be used. For cases, where the graphics drivers knows, there can be only one mode description or where the driver only supports one mode, a helper function of_get_videomode is added, that gets a struct videomode from DT. (As this function is implemented in an expensive fashion, it should only be used in the aforementioned case). This also demonstrates how of_display_timings may be utilized. 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 Acked-by: Laurent Pinchart --- .../devicetree/bindings/video/display-timings.txt | 107 ++ drivers/video/Kconfig | 15 ++ drivers/video/Makefile |2 + drivers/video/of_display_timing.c | 223 drivers/video/of_videomode.c | 48 + include/linux/of_display_timings.h | 20 ++ include/linux/of_videomode.h | 18 ++ 7 files changed, 433 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..2b25d58 --- /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 (active on falling edge)/ + non-inverted (active on rising edge)/ +ignored (ignore property) + - interlaced (bool): boolean to enable interlaced mode + - doublescan (bool): boolean to enable doublescan mode + - doubleclk (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 + + +--+-+--+---+ + | |?| | | + | ||vback_porch | | | + | |?| | | + +--###--+---+ + | #?# | | + | #|# | | + | hback #|# hfront | hsync | + | porch #| hactive # porch | len | + |<>#<---+--->#<>|<->| + | #|# | | + | #|vactive # | | + | #|# | | + | #?# | | + +--###--+---+ + | |?| | | + | ||vfront_porch| | | + | |
[PATCH v3] DRM/KMS/EDID: Consolidate EDID Error Handling (v3)
On Thu, Nov 22, 2012 at 09:44:42AM -0500, Egbert Eich wrote:
> Consolidate the null_edid_counter and the bad_edid_counter
> into EDID error state flags which for the last EDID read
> are accessible from user.
> Errors are looged it the same error has not been present
> in the previous read of the EDID. This will reset the
> EDID error status for example when the monitor is changed
> but still prevents permanent EDID errors from piling up
> the the kernel logs.
>
> v2: Fixed conflits due to reordering of commits.
> Set error state where missing.
> v3: Don't update cache when returning block from cache.
>
> Signed-off-by: Egbert Eich
> ---
> drivers/gpu/drm/drm_edid.c | 117
> +---
> drivers/gpu/drm/radeon/radeon_connectors.c |2 +-
> include/drm/drm_crtc.h |4 +-
> include/drm/drm_edid.h | 10 +++
> 4 files changed, 82 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index dd0df60..aa9b34d 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -157,6 +157,17 @@ int drm_edid_header_is_valid(const u8 *raw_edid)
> }
> EXPORT_SYMBOL(drm_edid_header_is_valid);
>
> +static bool drm_edid_is_zero(u8 *in_edid, int length)
> +{
> + int i;
> + u32 *raw_edid = (u32 *)in_edid;
> +
> + for (i = 0; i < length / 4; i++)
> + if (*(raw_edid + i) != 0)
> + return false;
> + return true;
You could use memchr_inv() here. But the compiler can't optimize it
since it's not inline, so I suppose it might make it slower.
> +}
> +
> static int edid_fixup __read_mostly = 6;
> module_param_named(edid_fixup, edid_fixup, int, 0400);
> MODULE_PARM_DESC(edid_fixup,
> @@ -166,11 +177,13 @@ MODULE_PARM_DESC(edid_fixup,
> * Sanity check the EDID block (base or extension). Return 0 if the block
> * doesn't check out, or 1 if it's valid.
> */
> -bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
> +unsigned
> +drm_edid_block_check_error(u8 *raw_edid, int block, unsigned
> last_error_flags)
> {
> int i;
> u8 csum = 0;
> struct edid *edid = (struct edid *)raw_edid;
> + unsigned result = 0;
>
> if (edid_fixup > 8 || edid_fixup < 0)
> edid_fixup = 6;
> @@ -182,27 +195,33 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool
> print_bad_edid)
> DRM_DEBUG("Fixing EDID header, your hardware may be
> failing\n");
> memcpy(raw_edid, edid_header, sizeof(edid_header));
> } else {
> - goto bad;
> + result |= EDID_ERR_NO_BLOCK0;
> + if (drm_edid_is_zero(raw_edid, EDID_LENGTH)) {
> + result |= EDID_ERR_NULL;
> + goto bad;
> + }
> }
> }
>
> for (i = 0; i < EDID_LENGTH; i++)
> csum += raw_edid[i];
> if (csum) {
> - if (print_bad_edid) {
> + if ((last_error_flags & EDID_ERR_CSUM) == 0)
> DRM_ERROR("EDID checksum is invalid, remainder is
> %d\n", csum);
> - }
>
> /* allow CEA to slide through, switches mangle this */
> if (raw_edid[0] != 0x02)
> - goto bad;
> + result |= EDID_ERR_CSUM;
> }
> + if (result)
> + goto bad;
>
> /* per-block-type checks */
> switch (raw_edid[0]) {
> case 0: /* base */
> if (edid->version != 1) {
> DRM_ERROR("EDID has major version %d, instead of 1\n",
> edid->version);
> + result |= EDID_ERR_UNSUPPORTED_VERSION;
> goto bad;
> }
>
> @@ -214,15 +233,23 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool
> print_bad_edid)
> break;
> }
>
> - return 1;
> + return 0;
>
> bad:
> - if (raw_edid && print_bad_edid) {
> + if (raw_edid && last_error_flags != result) {
> printk(KERN_ERR "Raw EDID:\n");
> print_hex_dump(KERN_ERR, " \t", DUMP_PREFIX_NONE, 16, 1,
> raw_edid, EDID_LENGTH, false);
> }
> - return 0;
> + return result;
> +}
> +
> +bool
> +drm_edid_block_valid(u8 *raw_edid, int block, unsigned last_error_flags)
> +{
> + if (!drm_edid_block_check_error(raw_edid, block, last_error_flags))
> + return true;
> + return false;
return !drm_edid_block_check_error();
> }
> EXPORT_SYMBOL(drm_edid_block_valid);
>
> @@ -241,7 +268,7 @@ bool drm_edid_is_valid(struct edid *edid)
> return false;
>
> for (i = 0; i <= edid->extensions; i++)
> - if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true))
> + if (drm_edid_block_check_error(raw + i * EDID_L
[PATCHv13 5/7] fbmon: add of_videomode helpers
Hi Steffen,
On Thursday 22 November 2012 17:00:13 Steffen Trumtrar wrote:
> 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
> Acked-by: Laurent Pinchart
This patch results in the following build warning:
drivers/video/fbmon.c: In function 'of_get_fb_videomode':
drivers/video/fbmon.c:1445: warning: passing argument 1 of 'of_get_videomode'
discards qualifiers from pointer target type
include/linux/of_videomode.h:15: note: expected 'struct device_node *' but
argument is of type 'const struct device_node *'
> ---
> drivers/video/fbmon.c | 42 +-
> include/linux/fb.h|6 ++
> 2 files changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
> index a6a564d..cd0a035 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
> @@ -1416,6 +1416,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 4404ec2..43a2f81 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -20,6 +20,7 @@ struct fb_info;
> struct device;
> struct file;
> struct videomode;
> +struct device_node;
>
> /* Definitions below are used in the parsed monitor specs */
> #define FB_DPMS_ACTIVE_OFF 1
> @@ -715,6 +716,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);
--
Regards,
Laurent Pinchart
Linux 3.7-rc6
Hi Daniel,
> >> My apologies for the long delay in answering, I've somehow mixed up
> >> different bugreports and thought I've sent you a patch to test
> >> already. Anyway, please test
> >>
> >> https://patchwork.kernel.org/patch/1728111/
> >
> > Tested-by: Henrik Rydberg
>
> Can you please boot with drm.debug=0xe added to your kernel cmdline
> with that patch applied and attach the full dmesg?
Are you looking for something in particular? The patch obviously works
because edp_bpp is never set. The reason seems to be
[1.634759] [drm:intel_parse_bios], VBT signature missing
In the source, a few lines above that message, we have something that
looks suspciciously like an off-by-one error:
/* Scour memory looking for the VBT signature */
for (i = 0; i + 4 < size; i++) {
If that matters or not, I do not know. Any more tests would have to
wait until tomorrow.
Thanks,
Henrik
[PATCHv13 4/7] fbmon: add videomode helpers
Hi Steffen,
On Thursday 22 November 2012 17:00:12 Steffen Trumtrar wrote:
> 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
> Acked-by: Laurent Pinchart
> Signed-off-by: Steffen Trumtrar
> ---
> drivers/video/fbmon.c | 44
> include/linux/fb.h|6 ++
> 2 files changed, 50 insertions(+)
>
> diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
> index cef6557..a6a564d 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,49 @@ 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);
This results in a division by 0 if vm->pixelclock is equal to zero. As the
information is missing from many board files, what would you think about the
following ?
fbmode->pixclock = vm->pixelclock ? KHZ2PICOS(vm->pixelclock / 1000) :
0;
> +
> + 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;
> + 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 / (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..4404ec2 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -19,6 +19,7 @@ struct vm_area_struct;
> struct fb_info;
> struct device;
> struct file;
> +struct videomode;
>
> /* Definitions below are used in the parsed monitor specs */
> #define FB_DPMS_ACTIVE_OFF 1
> @@ -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,
--
Regards,
Laurent Pinchart
[PATCH v3] DRM/KMS/EDID: Consolidate EDID Error Handling (v3)
Ville Syrj?l? writes:
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index dd0df60..aa9b34d 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -157,6 +157,17 @@ int drm_edid_header_is_valid(const u8 *raw_edid)
> > }
> > EXPORT_SYMBOL(drm_edid_header_is_valid);
> >
> > +static bool drm_edid_is_zero(u8 *in_edid, int length)
> > +{
> > + int i;
> > + u32 *raw_edid = (u32 *)in_edid;
> > +
> > + for (i = 0; i < length / 4; i++)
> > + if (*(raw_edid + i) != 0)
> > + return false;
> > + return true;
[...]
>
> You could use memchr_inv() here. But the compiler can't optimize it
> since it's not inline, so I suppose it might make it slower.
> > +
> > +bool
> > +drm_edid_block_valid(u8 *raw_edid, int block, unsigned last_error_flags)
> > +{
> > + if (!drm_edid_block_check_error(raw_edid, block, last_error_flags))
> > + return true;
> > + return false;
>
> return !drm_edid_block_check_error();
Right.
It's stupid anyway. See below.
>
> > }
> > EXPORT_SYMBOL(drm_edid_block_valid);
> >
> > @@ -241,7 +268,7 @@ bool drm_edid_is_valid(struct edid *edid)
> >return false;
> >
> >for (i = 0; i <= edid->extensions; i++)
> > - if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true))
> > + if (drm_edid_block_check_error(raw + i * EDID_LENGTH, i, true))
>
>
> That looks wrong. Also the
> 's/!drm_edid_block_valid/drm_edid_block_check_error'
Oops, right. Leftover from old code.
> change seems superfluous since you're not using the more detailed return
> value from drm_edid_block_check_error().
This should probably be changed.
For the drm_edid_block_valid() I'm already using the previous error state
as argument - but I don't tell the result of this read.
Doesn't make much sense :(
Something similar should be done for drm_edid_is_valid() - even if the
driver doesn't bother (for instance because this function is only called
once when the device structures are initialized).
The current code ignores the error state for extension blocks i guess it
should not if we want to avoid having repreated logging of errors in the
extension blocks.
What should be done is:
unsigned err = drm_edid_block_check_error();
result |= err;
if (!err) {
valid_extensions++;
...
}
> > - if (drm_edid_block_valid(block + (valid_extensions + 1)
> > * EDID_LENGTH, j, print_bad_edid)) {
> > + if (!drm_edid_block_check_error(block +
> > (valid_extensions + 1) * EDID_LENGTH, j, last_error_flags)) {
>
> Again the change of function seems superfluous.
>
Exactly. Please see above.
Thanks for looking at the patches!
Cheers,
Egbert.
[PATCH v3] DRM/KMS/EDID: Consolidate EDID Error Handling (v3)
On Thu, Nov 22, 2012 at 07:28:44PM +0100, Egbert Eich wrote:
> Ville Syrj?l? writes:
> > >
> > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > > index dd0df60..aa9b34d 100644
> > > --- a/drivers/gpu/drm/drm_edid.c
> > > +++ b/drivers/gpu/drm/drm_edid.c
> > > @@ -157,6 +157,17 @@ int drm_edid_header_is_valid(const u8 *raw_edid)
> > > }
> > > EXPORT_SYMBOL(drm_edid_header_is_valid);
> > >
> > > +static bool drm_edid_is_zero(u8 *in_edid, int length)
> > > +{
> > > +int i;
> > > +u32 *raw_edid = (u32 *)in_edid;
> > > +
> > > +for (i = 0; i < length / 4; i++)
> > > +if (*(raw_edid + i) != 0)
> > > +return false;
> > > +return true;
>
> [...]
> >
> > You could use memchr_inv() here. But the compiler can't optimize it
> > since it's not inline, so I suppose it might make it slower.
>
>
> > > +
> > > +bool
> > > +drm_edid_block_valid(u8 *raw_edid, int block, unsigned last_error_flags)
> > > +{
> > > +if (!drm_edid_block_check_error(raw_edid, block,
> last_error_flags))
> > > +return true;
> > > +return false;
> >
> > return !drm_edid_block_check_error();
>
> Right.
> It's stupid anyway. See below.
>
> >
> > > }
> > > EXPORT_SYMBOL(drm_edid_block_valid);
> > >
> > > @@ -241,7 +268,7 @@ bool drm_edid_is_valid(struct edid *edid)
> > > return false;
> > >
> > > for (i = 0; i <= edid->extensions; i++)
> > > -if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i,
> true))
> > > +if (drm_edid_block_check_error(raw + i * EDID_LENGTH,
> i, true))
> >
>
> >
> > That looks wrong. Also the
> 's/!drm_edid_block_valid/drm_edid_block_check_error'
>
> Oops, right. Leftover from old code.
>
> > change seems superfluous since you're not using the more detailed return
> > value from drm_edid_block_check_error().
>
> This should probably be changed.
> For the drm_edid_block_valid() I'm already using the previous error state
> as argument - but I don't tell the result of this read.
> Doesn't make much sense :(
>
> Something similar should be done for drm_edid_is_valid() - even if the
> driver doesn't bother (for instance because this function is only called
> once when the device structures are initialized).
>
> The current code ignores the error state for extension blocks i guess it
> should not if we want to avoid having repreated logging of errors in the
> extension blocks.
I'm not sure. The current code dump all failed extension block, doesn't
it? Maybe we actually do want that to happen, at least w/ debugs
enabled.
Then there are various retry loops in the code, which may or may not be
necessary. I have a feeling some of them may have been attempts at
papering over a bug that I fixed [1] in the i2c bitbanging code. But if
they are necessary, I'm not sure we really appreciate repeated dumps of
the same block.
[1]
https://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commit;h=8ee161ce5e0cfc689eb677f227a6248191165fac
--
Ville Syrj??l??
Intel OTC
[PATCH v4] DRM/KMS/EDID: Consolidate EDID Error Handling (v4)
Consolidate the null_edid_counter and the bad_edid_counter
into EDID error state flags which for the last EDID read
are accessible from user.
Errors are looged it the same error has not been present
in the previous read of the EDID. This will reset the
EDID error status for example when the monitor is changed
but still prevents permanent EDID errors from piling up
the the kernel logs.
v2: Fixed conflits due to reordering of commits.
Set error state where missing.
v3: Don't update cache when returning block from cache.
v4: Fixed drm_edid_is_valid() to return the actual error
state.
Included results of extension block checking in the
overall error state.
Removed drm_edid_block_valid() as there is noone
using it.
Signed-off-by: Egbert Eich
---
drivers/gpu/drm/drm_edid.c | 136
drivers/gpu/drm/radeon/radeon_atombios.c |2 +-
drivers/gpu/drm/radeon/radeon_connectors.c |2 +-
include/drm/drm_crtc.h |6 +-
include/drm/drm_edid.h | 10 ++
5 files changed, 94 insertions(+), 62 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index dd0df60..6cedd46 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -157,6 +157,17 @@ int drm_edid_header_is_valid(const u8 *raw_edid)
}
EXPORT_SYMBOL(drm_edid_header_is_valid);
+static bool drm_edid_is_zero(u8 *in_edid, int length)
+{
+ int i;
+ u32 *raw_edid = (u32 *)in_edid;
+
+ for (i = 0; i < length / 4; i++)
+ if (*(raw_edid + i) != 0)
+ return false;
+ return true;
+}
+
static int edid_fixup __read_mostly = 6;
module_param_named(edid_fixup, edid_fixup, int, 0400);
MODULE_PARM_DESC(edid_fixup,
@@ -166,11 +177,13 @@ MODULE_PARM_DESC(edid_fixup,
* Sanity check the EDID block (base or extension). Return 0 if the block
* doesn't check out, or 1 if it's valid.
*/
-bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
+unsigned
+drm_edid_block_check_error(u8 *raw_edid, int block, unsigned last_error_flags)
{
int i;
u8 csum = 0;
struct edid *edid = (struct edid *)raw_edid;
+ unsigned result = 0;
if (edid_fixup > 8 || edid_fixup < 0)
edid_fixup = 6;
@@ -182,27 +195,33 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool
print_bad_edid)
DRM_DEBUG("Fixing EDID header, your hardware may be
failing\n");
memcpy(raw_edid, edid_header, sizeof(edid_header));
} else {
- goto bad;
+ result |= EDID_ERR_NO_BLOCK0;
+ if (drm_edid_is_zero(raw_edid, EDID_LENGTH)) {
+ result |= EDID_ERR_NULL;
+ goto bad;
+ }
}
}
for (i = 0; i < EDID_LENGTH; i++)
csum += raw_edid[i];
if (csum) {
- if (print_bad_edid) {
+ if ((last_error_flags & EDID_ERR_CSUM) == 0)
DRM_ERROR("EDID checksum is invalid, remainder is
%d\n", csum);
- }
/* allow CEA to slide through, switches mangle this */
if (raw_edid[0] != 0x02)
- goto bad;
+ result |= EDID_ERR_CSUM;
}
+ if (result)
+ goto bad;
/* per-block-type checks */
switch (raw_edid[0]) {
case 0: /* base */
if (edid->version != 1) {
DRM_ERROR("EDID has major version %d, instead of 1\n",
edid->version);
+ result |= EDID_ERR_UNSUPPORTED_VERSION;
goto bad;
}
@@ -214,17 +233,16 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool
print_bad_edid)
break;
}
- return 1;
+ return 0;
bad:
- if (raw_edid && print_bad_edid) {
+ if (raw_edid && last_error_flags != result) {
printk(KERN_ERR "Raw EDID:\n");
print_hex_dump(KERN_ERR, " \t", DUMP_PREFIX_NONE, 16, 1,
raw_edid, EDID_LENGTH, false);
}
- return 0;
+ return result;
}
-EXPORT_SYMBOL(drm_edid_block_valid);
/**
* drm_edid_is_valid - sanity check EDID data
@@ -232,19 +250,21 @@ EXPORT_SYMBOL(drm_edid_block_valid);
*
* Sanity-check an entire EDID record (including extensions)
*/
-bool drm_edid_is_valid(struct edid *edid)
+unsigned drm_edid_is_valid(struct edid *edid, unsigned last_error_flags)
{
int i;
+ unsigned result;
u8 *raw = (u8 *)edid;
if (!edid)
- return false;
+ return EDID_ERR_NO_DATA;
- for (i = 0; i <= edid->extensions; i++)
- if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true))
- re
[PATCH 0/2] drm: exynos: hdmi: sending AVI and AUI info frames
On Fri, Nov 09, 2012 at 09:51:04PM +0530, 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 Hi Rahul, I'm currently working on a patchset that tries to unify the infoframe code that is currently duplicated for each driver. The latest code that I have adds support for packing AVI, audio and vendor infoframes, so it should cover all of what you're trying to do here. Furthermore there's a helper that fill in a basic set of fields in the AVI infoframe from a given drm_display_mode, based on code very similar to Stephane's patch from Paulo Zanoni (Cc'ed). Maybe we can try and make this work for Exynos as well. The code that I have is tested on NVIDIA Tegra and should be easily portable to i915, radeon and nouveau as well. I plan to do that as part of the patchset, so I thought you may be interested in using the generic code as well. The patches still need some cleanup but I think I can send them out either later tonight or tomorrow. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20121122/d76da079/attachment.pgp>
[PATCH v4] DRM/KMS/EDID: Move EDID related Functions to drm_edid.h (v2)
v2: Adjusted to apply cleanly. Signed-off-by: Egbert Eich --- include/drm/drm_crtc.h |8 include/drm/drm_edid.h |9 + 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 7a3ccbf..7eed9bd 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -878,9 +878,6 @@ extern char *drm_get_tv_subconnector_name(int val); extern char *drm_get_tv_select_name(int val); extern void drm_fb_release(struct drm_file *file_priv); extern int drm_mode_group_init_legacy_group(struct drm_device *dev, struct drm_mode_group *group); -extern bool drm_probe_ddc(struct i2c_adapter *adapter); -extern struct edid *drm_get_edid(struct drm_connector *connector, -struct i2c_adapter *adapter); extern int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid); extern void drm_mode_probed_add(struct drm_connector *connector, struct drm_display_mode *mode); extern void drm_mode_remove(struct drm_connector *connector, struct drm_display_mode *mode); @@ -1036,9 +1033,6 @@ extern int drm_mode_gamma_get_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); extern int drm_mode_gamma_set_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); -extern u8 *drm_find_cea_extension(struct edid *edid); -extern bool drm_detect_hdmi_monitor(struct edid *edid); -extern bool drm_detect_monitor_audio(struct edid *edid); extern int drm_mode_page_flip_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); extern struct drm_display_mode *drm_cvt_mode(struct drm_device *dev, @@ -1054,8 +1048,6 @@ extern struct drm_display_mode *drm_gtf_mode_complex(struct drm_device *dev, extern int drm_add_modes_noedid(struct drm_connector *connector, int hdisplay, int vdisplay); -extern int drm_edid_header_is_valid(const u8 *raw_edid); -extern unsigned drm_edid_is_valid(struct edid *edid, unsigned last_error_flags); struct drm_display_mode *drm_mode_find_dmt(struct drm_device *dev, int hsize, int vsize, int fresh, bool rb); diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index 6bcaee5..2c0d39a 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -268,4 +268,13 @@ int drm_validate_edid_blob(struct drm_connector *connector, u8 **blockp, int len void drm_cache_edid(struct drm_connector *connector, struct edid *edid); unsigned drm_edid_block_check_error(u8 *raw_edid, int block, unsigned last_error_flags); +extern bool drm_probe_ddc(struct i2c_adapter *adapter); +extern struct edid *drm_get_edid(struct drm_connector *connector, +struct i2c_adapter *adapter); +extern u8 *drm_find_cea_extension(struct edid *edid); +extern bool drm_detect_hdmi_monitor(struct edid *edid); +extern bool drm_detect_monitor_audio(struct edid *edid); +extern int drm_edid_header_is_valid(const u8 *raw_edid); +extern unsigned drm_edid_is_valid(struct edid *edid, unsigned last_error_flags); + #endif /* __DRM_EDID_H__ */ -- 1.7.7
[PATCH] drm: tegra: Use framebuffer pitch as line stride
Instead of using the stride derived from the display mode, use the pitch associated with the currently active framebuffer. This fixes a bug where the LCD display content would be skewed when enabling HDMI with a video mode different from that of the LCD. Signed-off-by: Thierry Reding --- drivers/gpu/drm/tegra/dc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c index 94686e5..41cde76 100644 --- a/drivers/gpu/drm/tegra/dc.c +++ b/drivers/gpu/drm/tegra/dc.c @@ -218,7 +218,7 @@ static int tegra_crtc_mode_set(struct drm_crtc *crtc, } bpp = crtc->fb->bits_per_pixel / 8; - win.stride = win.outw * bpp; + win.stride = crtc->fb->pitches[0]; /* program window registers */ value = tegra_dc_readl(dc, DC_CMD_DISPLAY_WINDOW_HEADER); -- 1.8.0
[Bug 56405] Distorted graphics on Radeon HD 6620G
https://bugs.freedesktop.org/show_bug.cgi?id=56405 --- Comment #32 from Michael Dressel --- Created attachment 70453 --> https://bugs.freedesktop.org/attachment.cgi?id=70453&action=edit new bisect log I started bisecting from the second last bad commit. I could verify it's bad. The new bisect log output shows the current status. I'm now testing the r600_dri.so with both libgl libraries. I do not point to my built tree for loading the libraries or objects. What I do is replace the r600_dri.so in /usr/lib/xorg/modules/dri/r600_dri.so with the one I built. I now test it twice with libgl 8.0.4 and 9.0.1. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20121122/9b615afd/attachment.html>
[Bug 56405] Distorted graphics on Radeon HD 6620G
https://bugs.freedesktop.org/show_bug.cgi?id=56405 --- Comment #33 from Michael Dressel --- Created attachment 70454 --> https://bugs.freedesktop.org/attachment.cgi?id=70454&action=edit the patch I used lately This is a patch I need to apply in order to get at least r600_dri.so compiled. Could the bug be in there? I guess not. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20121122/5793c36e/attachment.html>
[PATCH v3] DRM/KMS/EDID: Consolidate EDID Error Handling (v3)
Ville Syrj?l? writes: > On Thu, Nov 22, 2012 at 07:28:44PM +0100, Egbert Eich wrote: > > > > Something similar should be done for drm_edid_is_valid() - even if the > > driver doesn't bother (for instance because this function is only called > > once when the device structures are initialized). > > > > The current code ignores the error state for extension blocks i guess it > > should not if we want to avoid having repreated logging of errors in the > > extension blocks. > > I'm not sure. The current code dump all failed extension block, doesn't > it? Maybe we actually do want that to happen, at least w/ debugs > enabled. Yes, it does. It drops them silently. I was a bit unclear - what I meant was: we ignore the result for the final error state we keep to be able to track which errors we still want to log. > > Then there are various retry loops in the code, which may or may not be > necessary. I have a feeling some of them may have been attempts at > papering over a bug that I fixed [1] in the i2c bitbanging code. But if > they are necessary, I'm not sure we really appreciate repeated dumps of > the same block. > > [1] > https://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commit;h=8ee161ce5e0cfc689eb677f227a6248191165fac Right, I know this patch :) I'm also not sure about the retries. I guess for the base block we want at least one retry in case the display is in an intermediate state where it will not send the base block. Apart from this I've seen many EDID failures however I don't recall seeing one which got fixed by repeated reads. However to see if those loops fix any issue we may want to keep all messages enabled for the time being. Cheers, Egbert.
Linux 3.7-rc6
On Thu, Nov 22, 2012 at 7:23 PM, Henrik Rydberg wrote:
>> >> My apologies for the long delay in answering, I've somehow mixed up
>> >> different bugreports and thought I've sent you a patch to test
>> >> already. Anyway, please test
>> >>
>> >> https://patchwork.kernel.org/patch/1728111/
>> >
>> > Tested-by: Henrik Rydberg
>>
>> Can you please boot with drm.debug=0xe added to your kernel cmdline
>> with that patch applied and attach the full dmesg?
>
> Are you looking for something in particular? The patch obviously works
> because edp_bpp is never set. The reason seems to be
>
> [1.634759] [drm:intel_parse_bios], VBT signature missing
>
> In the source, a few lines above that message, we have something that
> looks suspciciously like an off-by-one error:
>
> /* Scour memory looking for the VBT signature */
> for (i = 0; i + 4 < size; i++) {
>
> If that matters or not, I do not know. Any more tests would have to
> wait until tomorrow.
Yeah, the utter lack of a vbt fits very nicely, thanks for checking,
I've merged the patch into drm-intel-fixes and will forward it for
inclusion into 3.7 rsn.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 04/10] drm/ttm: change fence_lock to inner lock, v3
On 11/22/2012 04:51 PM, Maarten Lankhorst wrote: > Op 21-11-12 14:27, Thomas Hellstrom schreef: >> On 11/21/2012 02:12 PM, Maarten Lankhorst wrote: >>> Op 21-11-12 13:42, Thomas Hellstrom schreef: On 11/21/2012 12:38 PM, Maarten Lankhorst wrote: > Hey, > > Op 20-11-12 16:08, Thomas Hellstrom schreef: >> 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
Linux 3.7-rc6
> Yeah, the utter lack of a vbt fits very nicely, thanks for checking, > I've merged the patch into drm-intel-fixes and will forward it for > inclusion into 3.7 rsn. Great, thanks. One thing about that patch: if we would ever encounter a non-zero edp.bpp < 3, display_bpc would not be clamped. I suppose monochrome screens went out of fashion twenty years ago, but who knows... Thanks, Henrik
[PATCH v5] DRM/KMS/EDID: Consolidate EDID Error Handling (v5)
Consolidate the null_edid_counter and the bad_edid_counter
into EDID error state flags which for the last EDID read
are accessible from user.
Errors are looged it the same error has not been present
in the previous read of the EDID. This will reset the
EDID error status for example when the monitor is changed
but still prevents permanent EDID errors from piling up
the the kernel logs.
v2: Fixed conflits due to reordering of commits.
Set error state where missing.
v3: Don't update cache when returning block from cache.
v4: Inspired by a discussion with Ville Syrj?l?
Fixed drm_edid_is_valid() to return the actual error
state.
Included results of extension block checking in the
overall error state.
Removed drm_edid_block_valid() as there is noone
using it.
v5: Added missing radeon/radeon_combios.c.
Signed-off-by: Egbert Eich
---
drivers/gpu/drm/drm_edid.c | 136
drivers/gpu/drm/radeon/radeon_atombios.c |2 +-
drivers/gpu/drm/radeon/radeon_combios.c|2 +-
drivers/gpu/drm/radeon/radeon_connectors.c |2 +-
include/drm/drm_crtc.h |6 +-
include/drm/drm_edid.h | 10 ++
6 files changed, 95 insertions(+), 63 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index dd0df60..6cedd46 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -157,6 +157,17 @@ int drm_edid_header_is_valid(const u8 *raw_edid)
}
EXPORT_SYMBOL(drm_edid_header_is_valid);
+static bool drm_edid_is_zero(u8 *in_edid, int length)
+{
+ int i;
+ u32 *raw_edid = (u32 *)in_edid;
+
+ for (i = 0; i < length / 4; i++)
+ if (*(raw_edid + i) != 0)
+ return false;
+ return true;
+}
+
static int edid_fixup __read_mostly = 6;
module_param_named(edid_fixup, edid_fixup, int, 0400);
MODULE_PARM_DESC(edid_fixup,
@@ -166,11 +177,13 @@ MODULE_PARM_DESC(edid_fixup,
* Sanity check the EDID block (base or extension). Return 0 if the block
* doesn't check out, or 1 if it's valid.
*/
-bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
+unsigned
+drm_edid_block_check_error(u8 *raw_edid, int block, unsigned last_error_flags)
{
int i;
u8 csum = 0;
struct edid *edid = (struct edid *)raw_edid;
+ unsigned result = 0;
if (edid_fixup > 8 || edid_fixup < 0)
edid_fixup = 6;
@@ -182,27 +195,33 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool
print_bad_edid)
DRM_DEBUG("Fixing EDID header, your hardware may be
failing\n");
memcpy(raw_edid, edid_header, sizeof(edid_header));
} else {
- goto bad;
+ result |= EDID_ERR_NO_BLOCK0;
+ if (drm_edid_is_zero(raw_edid, EDID_LENGTH)) {
+ result |= EDID_ERR_NULL;
+ goto bad;
+ }
}
}
for (i = 0; i < EDID_LENGTH; i++)
csum += raw_edid[i];
if (csum) {
- if (print_bad_edid) {
+ if ((last_error_flags & EDID_ERR_CSUM) == 0)
DRM_ERROR("EDID checksum is invalid, remainder is
%d\n", csum);
- }
/* allow CEA to slide through, switches mangle this */
if (raw_edid[0] != 0x02)
- goto bad;
+ result |= EDID_ERR_CSUM;
}
+ if (result)
+ goto bad;
/* per-block-type checks */
switch (raw_edid[0]) {
case 0: /* base */
if (edid->version != 1) {
DRM_ERROR("EDID has major version %d, instead of 1\n",
edid->version);
+ result |= EDID_ERR_UNSUPPORTED_VERSION;
goto bad;
}
@@ -214,17 +233,16 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool
print_bad_edid)
break;
}
- return 1;
+ return 0;
bad:
- if (raw_edid && print_bad_edid) {
+ if (raw_edid && last_error_flags != result) {
printk(KERN_ERR "Raw EDID:\n");
print_hex_dump(KERN_ERR, " \t", DUMP_PREFIX_NONE, 16, 1,
raw_edid, EDID_LENGTH, false);
}
- return 0;
+ return result;
}
-EXPORT_SYMBOL(drm_edid_block_valid);
/**
* drm_edid_is_valid - sanity check EDID data
@@ -232,19 +250,21 @@ EXPORT_SYMBOL(drm_edid_block_valid);
*
* Sanity-check an entire EDID record (including extensions)
*/
-bool drm_edid_is_valid(struct edid *edid)
+unsigned drm_edid_is_valid(struct edid *edid, unsigned last_error_flags)
{
int i;
+ unsigned result;
u8 *raw = (u8 *)edid;
if (!edid)
- return false;
+ return EDID_ERR_NO_DATA;
-
i915: black screen after blank when LID is closed on Linux >= 3.1
On Thu, Nov 22, 2012 at 10:35:22PM +0100, Krzysztof Mazur wrote:
> On Thu, Nov 22, 2012 at 09:17:54PM +0100, Daniel Vetter wrote:
> > Hi,
> >
> >
> > Since a dpms ioctl call tends to follow a modeset, this likely only
> > results in that dpms call enabling the hw again. Can you please add
> > drm.debug=0xe to your kernel cmdline and boot into a 3.6 with this
> > hack applied, reproduce the issue and the attach the complete dmesg?
> >
> > The below WARNs from 3.7 support that, we've simply improved the
> > code's ability to detect such problems. Can you please boot into a
> > kernel with the latest drm-intel-next-queued branch merged in, but no
> > other patches applied. Again please append drm.debug=0xe and then
> > attach the complete dmesg after you've reproduced the issue.
> >
> > Please also compile your kernels with CONFIG_PRINTK_TIME=y, the
> > timestamps in dmesg help a lot in figuring things out.
>
> In lastest drm-intel-next-queued (v3.7-rc4-323-g9352dce) I got
> following compile error:
>
> /home/krzysiek/src/linux-2.6/drivers/gpu/drm/i915/intel_lvds.c: In function
> 'intel_lvds_init':
> /home/krzysiek/src/linux-2.6/drivers/gpu/drm/i915/intel_lvds.c:1098:34:
> error: 'mode' undeclared (first use in this function)
> /home/krzysiek/src/linux-2.6/drivers/gpu/drm/i915/intel_lvds.c:1098:34: note:
> each undeclared identifier is reported only once for each function it appears
> in
>
> Without thinking I fixed it by:
>
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c
> b/drivers/gpu/drm/i915/intel_lvds.c
> index ced06f3..43f0874 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -1095,7 +1095,7 @@ bool intel_lvds_init(struct drm_device *dev)
> fixed_mode = intel_crtc_mode_get(dev, crtc);
> if (fixed_mode) {
> DRM_DEBUG_KMS("using current (BIOS) mode: ");
> - drm_mode_debug_printmodeline(&mode);
> + drm_mode_debug_printmodeline(fixed_mode);
> fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
> goto out;
> }
>
> Thanks,
Yeah, I've pushed before amending the conflict resolution. It's now fixed.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
[Bug 49121] Regression: ATI video card doesn't power down after elapsed time
https://bugzilla.kernel.org/show_bug.cgi?id=49121 --- Comment #3 from Joseph D. Wagner 2012-11-22 21:43:33 --- Issue appears to be resolved in 3.6.6-1.fc17.x86_64 (gcc version 4.7.2 20120921 (Red Hat 4.7.2-2) (GCC) ). -- Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are watching the assignee of the bug.
[RFC v2 0/5] Common Display Framework
From: Laurent Pinchart Hi everybody, Here's the second RFC of what was previously known as the Generic Panel Framework. I won't repeat all the background information from the first version here, you can read it at http://lwn.net/Articles/512363/. Many developers showed interest in the first RFC, and I've had the opportunity to discuss it with most of them. I would like to thank (in no particular order) Tomi Valkeinen for all the time he spend helping me to draft v2, Marcus Lorentzon for his useful input during Linaro Connect Q4 2012, and Linaro for inviting me to Connect and providing a venue to discuss this topic. After discussing the Generic Panel Framework at Linaro Connect we came to the conclusion that "panel" is too limiting a name. In addition to panel drivers we also want to share transmitter and bridge drivers between DRM and FBDEV. I have thus introduced the concept of a display entity in this version to represent any hardware block that sources, processes or sinks display-related video streams. This patch set implements the Common Display Framework (CDF). Display entities are connected to at least one video data bus, and optionally to a control bus. The video data busses carry display-related video data out of sources (such as a CRTC in a display controller) to sinks (such as a panel or a monitor), optionally going through transmitters, encoders, decoders, bridges or other similar devices. A CRTC or a panel will usually be connected to a single data bus, while an encoder or a transmitter will be connected to two data busses. While some display entities don't require any configuration (DPI panels are a good example), many of them are connected to a control bus accessible to the CPU. Control requests can be sent on a dedicated control bus (such as I2C or SPI) or multiplexed on a mixed control and data bus (such as DBI or DSI). To support both options the CDF display entity model separates the control and data busses in different APIs. Display entities are abstract object that must be implemented by a real device. The device sits on its control bus and is registered with the Linux device core and matched with his driver using the control bus specific API. The CDF doesn't create a display entity class or bus, display entity drivers thus standard Linux kernel drivers using existing busses. When a display entity driver probes a device it must create an instance of the display_entity structure, initialize it and register it with the CDF core. The display entity exposes abstract operations through function pointers, and the entity driver must implement those operations. They are divided in two groups, control operations and video operations. Control operations are called by upper-level drivers, usually in response to a request originating from userspace. They control the display entity state and operation. Currently defined control operations are - set_state(), to control the state of the entity (off, standby or on) - update(), to trigger a display update (for entities that implement manual update, such as manual-update panels that store frames in their internal frame buffer) - get_modes(), to retrieve the video modes supported by the entity - get_params(), to retrive the data bus parameters at the entity input (sink) - get_size(), to retrive the entity physical size (applicable to panels only) Video operations are called by downstream entities on upstream entities (from a video data bus point of view) to control the video operation. The only currently defined video operation is - set_stream(), to start (in continuous or single-shot mode) the video stream http://www.ideasonboard.org/media/cdf/cdf.pdf#1 describes how a panel driver implemented using the CDF interacts with the other components in the system. The first page shows the panel driver receiving control request from the display controller driver at its top side, usually in response to a DRM or FBDEV API call. It then issues requests on its control bus (several possible control busses are shown on the diagram, the panel driver uses one of them only) and calls video operations of the display controller on its left side to control the video stream. The second page shows a slightly more complex use case, with a display controller that includes an LVDS transceiver (shown as two separate entities on the left hand side), connected to an LVDS to DSI converter that is itself connected to a DSI panel module. The panel module contains a DSI panel controller that drives the LCD panel. While this particular example is probably too theoretical to be found in real devices, it illustrates the concept of display entities chains. The CDF models this using a Russian doll's model. From the display controller point of view only the first external entity (LVDS to DSI converter) is visible. The display controller thus calls the control operations implemented by the LVDS to DSI transmitter driver (left-most green arrow). The driver is aware of the
[RFC v2 1/5] video: Add generic display entity core
From: Laurent Pinchart
Signed-off-by: Laurent Pinchart
---
drivers/video/Kconfig|1 +
drivers/video/Makefile |1 +
drivers/video/display/Kconfig|4 +
drivers/video/display/Makefile |1 +
drivers/video/display/display-core.c | 362 ++
include/video/display.h | 150 ++
6 files changed, 519 insertions(+), 0 deletions(-)
create mode 100644 drivers/video/display/Kconfig
create mode 100644 drivers/video/display/Makefile
create mode 100644 drivers/video/display/display-core.c
create mode 100644 include/video/display.h
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index c5b7bcf..e91f03e 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -2442,6 +2442,7 @@ source "drivers/video/omap/Kconfig"
source "drivers/video/omap2/Kconfig"
source "drivers/video/exynos/Kconfig"
source "drivers/video/backlight/Kconfig"
+source "drivers/video/display/Kconfig"
if VT
source "drivers/video/console/Kconfig"
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index b936b00..0a4cfea 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -14,6 +14,7 @@ fb-objs := $(fb-y)
obj-$(CONFIG_VT) += console/
obj-$(CONFIG_LOGO) += logo/
obj-y+= backlight/
+obj-y+= display/
obj-$(CONFIG_EXYNOS_VIDEO) += exynos/
diff --git a/drivers/video/display/Kconfig b/drivers/video/display/Kconfig
new file mode 100644
index 000..1d533e7
--- /dev/null
+++ b/drivers/video/display/Kconfig
@@ -0,0 +1,4 @@
+menuconfig DISPLAY_CORE
+ tristate "Display Core"
+ ---help---
+ Support common display framework for graphics devices.
diff --git a/drivers/video/display/Makefile b/drivers/video/display/Makefile
new file mode 100644
index 000..bd93496
--- /dev/null
+++ b/drivers/video/display/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_DISPLAY_CORE) += display-core.o
diff --git a/drivers/video/display/display-core.c
b/drivers/video/display/display-core.c
new file mode 100644
index 000..358c089
--- /dev/null
+++ b/drivers/video/display/display-core.c
@@ -0,0 +1,362 @@
+/*
+ * Display Core
+ *
+ * Copyright (C) 2012 Renesas Solutions Corp.
+ *
+ * Contacts: Laurent Pinchart
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+static LIST_HEAD(display_entity_list);
+static LIST_HEAD(display_entity_notifiers);
+static DEFINE_MUTEX(display_entity_mutex);
+
+/*
-
+ * Control operations
+ */
+
+/**
+ * display_entity_set_state - Set the display entity operation state
+ * @entity: The display entity
+ * @state: Display entity operation state
+ *
+ * See &enum display_entity_state for information regarding the entity states.
+ *
+ * Return 0 on success or a negative error code otherwise.
+ */
+int display_entity_set_state(struct display_entity *entity,
+enum display_entity_state state)
+{
+ int ret;
+
+ if (entity->state == state)
+ return 0;
+
+ if (!entity->ops.ctrl || !entity->ops.ctrl->set_state)
+ return 0;
+
+ ret = entity->ops.ctrl->set_state(entity, state);
+ if (ret < 0)
+ return ret;
+
+ entity->state = state;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(display_entity_set_state);
+
+/**
+ * display_entity_update - Update the display
+ * @entity: The display entity
+ *
+ * Make the display entity ready to receive pixel data and start frame
transfer.
+ * This operation can only be called if the display entity is in STANDBY or ON
+ * state.
+ *
+ * The display entity will call the upstream entity in the video chain to start
+ * the video stream.
+ *
+ * Return 0 on success or a negative error code otherwise.
+ */
+int display_entity_update(struct display_entity *entity)
+{
+ if (!entity->ops.ctrl || !entity->ops.ctrl->update)
+ return 0;
+
+ return entity->ops.ctrl->update(entity);
+}
+EXPORT_SYMBOL_GPL(display_entity_update);
+
+/**
+ * display_entity_get_modes - Get video modes supported by the display entity
+ * @entity The display entity
+ * @modes: Pointer to an array of modes
+ *
+ * Fill the modes argument with a pointer to an array of video modes. The array
+ * is owned by the display entity.
+ *
+ * Return the number of supported modes on success (including 0 if no mode is
+ * supported) or a negative error code otherwise.
+ */
+int display_entity_get_modes(struct display_entity *entity,
+const struct videomode **modes)
+{
+ if (!entity->ops.ctrl || !entity->ops.ctrl->get_mod
[RFC v2 2/5] video: panel: Add DPI panel support
From: Laurent Pinchart
Signed-off-by: Laurent Pinchart
---
drivers/video/display/Kconfig | 13 +++
drivers/video/display/Makefile|1 +
drivers/video/display/panel-dpi.c | 147 +
include/video/panel-dpi.h | 24 ++
4 files changed, 185 insertions(+), 0 deletions(-)
create mode 100644 drivers/video/display/panel-dpi.c
create mode 100644 include/video/panel-dpi.h
diff --git a/drivers/video/display/Kconfig b/drivers/video/display/Kconfig
index 1d533e7..0f9b990 100644
--- a/drivers/video/display/Kconfig
+++ b/drivers/video/display/Kconfig
@@ -2,3 +2,16 @@ menuconfig DISPLAY_CORE
tristate "Display Core"
---help---
Support common display framework for graphics devices.
+
+if DISPLAY_CORE
+
+config DISPLAY_PANEL_DPI
+ tristate "DPI (Parallel) Display Panels"
+ ---help---
+ Support for simple digital (parallel) pixel interface panels. Those
+ panels receive pixel data through a parallel bus and have no control
+ bus.
+
+ If you are in doubt, say N.
+
+endif # DISPLAY_CORE
diff --git a/drivers/video/display/Makefile b/drivers/video/display/Makefile
index bd93496..47978d4 100644
--- a/drivers/video/display/Makefile
+++ b/drivers/video/display/Makefile
@@ -1 +1,2 @@
obj-$(CONFIG_DISPLAY_CORE) += display-core.o
+obj-$(CONFIG_DISPLAY_PANEL_DPI) += panel-dpi.o
diff --git a/drivers/video/display/panel-dpi.c
b/drivers/video/display/panel-dpi.c
new file mode 100644
index 000..c56197a
--- /dev/null
+++ b/drivers/video/display/panel-dpi.c
@@ -0,0 +1,147 @@
+/*
+ * DPI Display Panel
+ *
+ * Copyright (C) 2012 Renesas Solutions Corp.
+ *
+ * Contacts: Laurent Pinchart
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+struct panel_dpi {
+ struct display_entity entity;
+ const struct panel_dpi_platform_data *pdata;
+};
+
+#define to_panel_dpi(p)container_of(p, struct panel_dpi,
entity)
+
+static const struct display_entity_interface_params panel_dpi_params = {
+ .type = DISPLAY_ENTITY_INTERFACE_DPI,
+};
+
+static int panel_dpi_set_state(struct display_entity *entity,
+ enum display_entity_state state)
+{
+ switch (state) {
+ case DISPLAY_ENTITY_STATE_OFF:
+ case DISPLAY_ENTITY_STATE_STANDBY:
+ display_entity_set_stream(entity->source,
+ DISPLAY_ENTITY_STREAM_STOPPED);
+ break;
+
+ case DISPLAY_ENTITY_STATE_ON:
+ display_entity_set_stream(entity->source,
+ DISPLAY_ENTITY_STREAM_CONTINUOUS);
+ break;
+ }
+
+ return 0;
+}
+
+static int panel_dpi_get_modes(struct display_entity *entity,
+ const struct videomode **modes)
+{
+ struct panel_dpi *panel = to_panel_dpi(entity);
+
+ *modes = panel->pdata->mode;
+ return 1;
+}
+
+static int panel_dpi_get_size(struct display_entity *entity,
+ unsigned int *width, unsigned int *height)
+{
+ struct panel_dpi *panel = to_panel_dpi(entity);
+
+ *width = panel->pdata->width;
+ *height = panel->pdata->height;
+ return 0;
+}
+
+static int panel_dpi_get_params(struct display_entity *entity,
+ struct display_entity_interface_params *params)
+{
+ *params = panel_dpi_params;
+ return 0;
+}
+
+static const struct display_entity_control_ops panel_dpi_control_ops = {
+ .set_state = panel_dpi_set_state,
+ .get_modes = panel_dpi_get_modes,
+ .get_size = panel_dpi_get_size,
+ .get_params = panel_dpi_get_params,
+};
+
+static void panel_dpi_release(struct display_entity *entity)
+{
+ struct panel_dpi *panel = to_panel_dpi(entity);
+
+ kfree(panel);
+}
+
+static int panel_dpi_remove(struct platform_device *pdev)
+{
+ struct panel_dpi *panel = platform_get_drvdata(pdev);
+
+ platform_set_drvdata(pdev, NULL);
+ display_entity_unregister(&panel->entity);
+
+ return 0;
+}
+
+static int __devinit panel_dpi_probe(struct platform_device *pdev)
+{
+ const struct panel_dpi_platform_data *pdata = pdev->dev.platform_data;
+ struct panel_dpi *panel;
+ int ret;
+
+ if (pdata == NULL)
+ return -ENODEV;
+
+ panel = kzalloc(sizeof(*panel), GFP_KERNEL);
+ if (panel == NULL)
+ return -ENOMEM;
+
+ panel->pdata = pdata;
+ panel->entity.dev = &pdev->dev;
+ panel->entity.release = panel_dpi_release;
+ panel->entity.ops.ctrl = &panel_dpi_control_ops;
+
+ ret = display_entity_register(&panel->entity);
+ if (ret < 0) {
+ kfree(panel
[RFC v2 3/5] video: display: Add MIPI DBI bus support
From: Laurent Pinchart
Signed-off-by: Laurent Pinchart
---
drivers/video/display/Kconfig|4 +
drivers/video/display/Makefile |1 +
drivers/video/display/mipi-dbi-bus.c | 228 ++
include/video/display.h |5 +
include/video/mipi-dbi-bus.h | 125 +++
5 files changed, 363 insertions(+), 0 deletions(-)
create mode 100644 drivers/video/display/mipi-dbi-bus.c
create mode 100644 include/video/mipi-dbi-bus.h
diff --git a/drivers/video/display/Kconfig b/drivers/video/display/Kconfig
index 0f9b990..b04c8be 100644
--- a/drivers/video/display/Kconfig
+++ b/drivers/video/display/Kconfig
@@ -5,6 +5,10 @@ menuconfig DISPLAY_CORE
if DISPLAY_CORE
+config DISPLAY_MIPI_DBI
+ tristate
+ default n
+
config DISPLAY_PANEL_DPI
tristate "DPI (Parallel) Display Panels"
---help---
diff --git a/drivers/video/display/Makefile b/drivers/video/display/Makefile
index 47978d4..00ef1c2 100644
--- a/drivers/video/display/Makefile
+++ b/drivers/video/display/Makefile
@@ -1,2 +1,3 @@
obj-$(CONFIG_DISPLAY_CORE) += display-core.o
+obj-$(CONFIG_DISPLAY_MIPI_DBI) += mipi-dbi-bus.o
obj-$(CONFIG_DISPLAY_PANEL_DPI) += panel-dpi.o
diff --git a/drivers/video/display/mipi-dbi-bus.c
b/drivers/video/display/mipi-dbi-bus.c
new file mode 100644
index 000..bd39a97
--- /dev/null
+++ b/drivers/video/display/mipi-dbi-bus.c
@@ -0,0 +1,228 @@
+/*
+ * MIPI DBI Bus
+ *
+ * Copyright (C) 2012 Renesas Solutions Corp.
+ *
+ * Contacts: Laurent Pinchart
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+/*
-
+ * Bus operations
+ */
+
+int mipi_dbi_set_data_width(struct mipi_dbi_device *dev, unsigned int width)
+{
+ if (width != 8 && width != 16)
+ return -EINVAL;
+
+ dev->data_width = width;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(mipi_dbi_set_data_width);
+
+int mipi_dbi_write_command(struct mipi_dbi_device *dev, u16 cmd)
+{
+ return dev->bus->ops->write_command(dev->bus, dev, cmd);
+}
+EXPORT_SYMBOL_GPL(mipi_dbi_write_command);
+
+int mipi_dbi_write_data(struct mipi_dbi_device *dev, const u8 *data,
+ size_t len)
+{
+ return dev->bus->ops->write_data(dev->bus, dev, data, len);
+}
+EXPORT_SYMBOL_GPL(mipi_dbi_write_data);
+
+int mipi_dbi_read_data(struct mipi_dbi_device *dev, u8 *data, size_t len)
+{
+ return dev->bus->ops->read_data(dev->bus, dev, data, len);
+}
+EXPORT_SYMBOL_GPL(mipi_dbi_read_data);
+
+/*
-
+ * Bus type
+ */
+
+static const struct mipi_dbi_device_id *
+mipi_dbi_match_id(const struct mipi_dbi_device_id *id,
+ struct mipi_dbi_device *dev)
+{
+ while (id->name[0]) {
+ if (strcmp(dev->name, id->name) == 0) {
+ dev->id_entry = id;
+ return id;
+ }
+ id++;
+ }
+ return NULL;
+}
+
+static int mipi_dbi_match(struct device *_dev, struct device_driver *_drv)
+{
+ struct mipi_dbi_device *dev = to_mipi_dbi_device(_dev);
+ struct mipi_dbi_driver *drv = to_mipi_dbi_driver(_drv);
+
+ if (drv->id_table)
+ return mipi_dbi_match_id(drv->id_table, dev) != NULL;
+
+ return (strcmp(dev->name, _drv->name) == 0);
+}
+
+static ssize_t modalias_show(struct device *_dev, struct device_attribute *a,
+char *buf)
+{
+ struct mipi_dbi_device *dev = to_mipi_dbi_device(_dev);
+ int len = snprintf(buf, PAGE_SIZE, MIPI_DBI_MODULE_PREFIX "%s\n",
+ dev->name);
+
+ return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len;
+}
+
+static struct device_attribute mipi_dbi_dev_attrs[] = {
+ __ATTR_RO(modalias),
+ __ATTR_NULL,
+};
+
+static int mipi_dbi_uevent(struct device *_dev, struct kobj_uevent_env *env)
+{
+ struct mipi_dbi_device *dev = to_mipi_dbi_device(_dev);
+
+ add_uevent_var(env, "MODALIAS=%s%s", MIPI_DBI_MODULE_PREFIX,
+ dev->name);
+ return 0;
+}
+
+static const struct dev_pm_ops mipi_dbi_dev_pm_ops = {
+ .runtime_suspend = pm_generic_runtime_suspend,
+ .runtime_resume = pm_generic_runtime_resume,
+ .runtime_idle = pm_generic_runtime_idle,
+ .suspend = pm_generic_suspend,
+ .resume = pm_generic_resume,
+ .freeze = pm_generic_freeze,
+ .thaw = pm_generic_thaw,
+ .poweroff = pm_generic_poweroff,
+ .restore = pm_generic_restore,
+};
+
+static struct bus_type mipi_dbi_bus_type = {
+ .name = "mipi-dbi",
+ .dev_attrs = mipi_dbi_dev
[RFC v2 4/5] video: panel: Add R61505 panel support
From: Laurent Pinchart The R61505 is a SYS-80 bus panel controller from Renesas. Signed-off-by: Laurent Pinchart --- drivers/video/display/Kconfig|9 + drivers/video/display/Makefile |1 + drivers/video/display/panel-r61505.c | 554 ++ include/video/panel-r61505.h | 27 ++ 4 files changed, 591 insertions(+), 0 deletions(-) create mode 100644 drivers/video/display/panel-r61505.c create mode 100644 include/video/panel-r61505.h diff --git a/drivers/video/display/Kconfig b/drivers/video/display/Kconfig index b04c8be..c88999c 100644 --- a/drivers/video/display/Kconfig +++ b/drivers/video/display/Kconfig @@ -18,4 +18,13 @@ config DISPLAY_PANEL_DPI If you are in doubt, say N. +config DISPLAY_PANEL_R61505 + tristate "Renesas R61505-based Display Panel" + select DISPLAY_MIPI_DBI + ---help--- + Support panels based on the Renesas R61505 panel controller. + Those panels are controlled through a MIPI DBI interface. + + If you are in doubt, say N. + endif # DISPLAY_CORE diff --git a/drivers/video/display/Makefile b/drivers/video/display/Makefile index 00ef1c2..4c68465 100644 --- a/drivers/video/display/Makefile +++ b/drivers/video/display/Makefile @@ -1,3 +1,4 @@ obj-$(CONFIG_DISPLAY_CORE) += display-core.o obj-$(CONFIG_DISPLAY_MIPI_DBI) += mipi-dbi-bus.o obj-$(CONFIG_DISPLAY_PANEL_DPI) += panel-dpi.o +obj-$(CONFIG_DISPLAY_PANEL_R61505) += panel-r61505.o diff --git a/drivers/video/display/panel-r61505.c b/drivers/video/display/panel-r61505.c new file mode 100644 index 000..d72d324 --- /dev/null +++ b/drivers/video/display/panel-r61505.c @@ -0,0 +1,554 @@ +/* + * Renesas R61505-based Display Panels + * + * Copyright (C) 2012 Renesas Solutions Corp. + * Based on SuperH MigoR Quarter VGA LCD Panel + * Copyright (C) 2008 Magnus Damm + * Based on lcd_powertip.c from Kenati Technologies Pvt Ltd. + * Copyright (c) 2007 Ujjwal Pande + * + * Contacts: Laurent Pinchart + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include + +#define R61505_DEVICE_CODE 0x +#define R61505_DEVICE_CODE_VALUE 0x1505 +#define R61505_DRIVER_OUTPUT_CONTROL 0x0001 +#define R61505_DRIVER_OUTPUT_CONTROL_SM(1 << 10) +#define R61505_DRIVER_OUTPUT_CONTROL_SS(1 << 8) +#define R61505_LCD_WAVEFORM0x0002 +#define R61505_LCD_WAVEFORM_BC0(1 << 9) +#define R61505_LCD_WAVEFORM_EOR(1 << 8) +#define R61505_ENTRY_MODE 0x0003 +#define R61505_ENTRY_MODE_TRIREG (1 << 15) +#define R61505_ENTRY_MODE_DFM (1 << 14) +#define R61505_ENTRY_MODE_BGR (1 << 12) +#define R61505_ENTRY_MODE_HWM (1 << 9) +#define R61505_ENTRY_MODE_ORG (1 << 7) +#define R61505_ENTRY_MODE_ID1 (1 << 5) +#define R61505_ENTRY_MODE_ID0 (1 << 4) +#define R61505_ENTRY_MODE_AM (1 << 3) +#define R61505_RESIZE_CONTROL 0x0004 +#define R61505_RESIZE_CONTROL_RCV(n) (((n) & 3) << 8) +#define R61505_RESIZE_CONTROL_RCH(n) (((n) & 3) << 4) +#define R61505_RESIZE_CONTROL_RSZ_4(3 << 0) +#define R61505_RESIZE_CONTROL_RSZ_2(1 << 0) +#define R61505_RESIZE_CONTROL_RSZ_1(0 << 0) +#define R61505_DISPLAY_CONTROL10x0007 +#define R61505_DISPLAY_CONTROL1_PTDE1 (1 << 13) +#define R61505_DISPLAY_CONTROL1_PTDE0 (1 << 12) +#define R61505_DISPLAY_CONTROL1_BASEE (1 << 8) +#define R61505_DISPLAY_CONTROL1_VON(1 << 6) +#define R61505_DISPLAY_CONTROL1_GON(1 << 5) +#define R61505_DISPLAY_CONTROL1_DTE(1 << 4) +#define R61505_DISPLAY_CONTROL1_COL(1 << 3) +#define R61505_DISPLAY_CONTROL1_D1 (1 << 1) +#define R61505_DISPLAY_CONTROL1_D0 (1 << 0) +#define R61505_DISPLAY_CONTROL20x0008 +#define R61505_DISPLAY_CONTROL2_FP(n) (((n) & 0xf) << 8) +#define R61505_DISPLAY_CONTROL2_BP(n) (((n) & 0xf) << 0) +#define R61505_DISPLAY_CONTROL30x0009 +#define R61505_DISPLAY_CONTROL3_PTS(n) (((n) & 7) << 8) +#define R61505_DISPLAY_CONTROL3_PTG(n) (((n) & 3) << 3) +#define R61505_DISPLAY_CONTROL3_ICS(n) (((n) & 0xf) << 0) +#define R61505_DISPLAY_CONTROL40x000a +#define R61505_DISPLAY_CONTROL4_FMARKOE(1 << 3) +#define R61505_DISPLAY_CONTROL4_FMI_6 (5 << 0) +#define R61505_DISPLAY_CONTROL4_FMI_4 (3 << 0) +#define R61505_DISPLAY_CONTROL4_FMI_2
[RFC v2 5/5] video: panel: Add R61517 panel support
From: Laurent Pinchart
The R61517 is a MIPI DBI panel controller from Renesas.
Signed-off-by: Laurent Pinchart
---
drivers/video/display/Kconfig|9 +
drivers/video/display/Makefile |1 +
drivers/video/display/panel-r61517.c | 447 ++
include/video/panel-r61517.h | 28 ++
4 files changed, 485 insertions(+), 0 deletions(-)
create mode 100644 drivers/video/display/panel-r61517.c
create mode 100644 include/video/panel-r61517.h
diff --git a/drivers/video/display/Kconfig b/drivers/video/display/Kconfig
index c88999c..13b6aaf 100644
--- a/drivers/video/display/Kconfig
+++ b/drivers/video/display/Kconfig
@@ -27,4 +27,13 @@ config DISPLAY_PANEL_R61505
If you are in doubt, say N.
+config DISPLAY_PANEL_R61517
+ tristate "Renesas R61517-based Display Panel"
+ select DISPLAY_MIPI_DBI
+ ---help---
+ Support panels based on the Renesas R61517 panel controller.
+ Those panels are controlled through a MIPI DBI interface.
+
+ If you are in doubt, say N.
+
endif # DISPLAY_CORE
diff --git a/drivers/video/display/Makefile b/drivers/video/display/Makefile
index 4c68465..482bec7 100644
--- a/drivers/video/display/Makefile
+++ b/drivers/video/display/Makefile
@@ -2,3 +2,4 @@ obj-$(CONFIG_DISPLAY_CORE) += display-core.o
obj-$(CONFIG_DISPLAY_MIPI_DBI) += mipi-dbi-bus.o
obj-$(CONFIG_DISPLAY_PANEL_DPI) += panel-dpi.o
obj-$(CONFIG_DISPLAY_PANEL_R61505) += panel-r61505.o
+obj-$(CONFIG_DISPLAY_PANEL_R61517) += panel-r61517.o
diff --git a/drivers/video/display/panel-r61517.c
b/drivers/video/display/panel-r61517.c
new file mode 100644
index 000..b4dced4
--- /dev/null
+++ b/drivers/video/display/panel-r61517.c
@@ -0,0 +1,447 @@
+/*
+ * Renesas R61517-based Display Panels
+ *
+ * Copyright (C) 2012 Renesas Solutions Corp.
+ * Based on KFR2R09 LCD panel support
+ * Copyright (C) 2009 Magnus Damm
+ * Register settings based on the out-of-tree t33fb.c driver
+ * Copyright (C) 2008 Lineo Solutions, Inc.
+ *
+ * Contacts: Laurent Pinchart
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+
+struct r61517 {
+ struct display_entity entity;
+ struct mipi_dbi_device *dbi;
+ const struct panel_r61517_platform_data *pdata;
+};
+
+#define to_panel(p)container_of(p, struct r61517, entity)
+
+/*
-
+ * Read, write and reset
+ */
+
+static void r61517_write_command(struct r61517 *panel, u16 cmd)
+{
+ mipi_dbi_write_command(panel->dbi, cmd);
+}
+
+static void r61517_write_data(struct r61517 *panel, u8 data)
+{
+ mipi_dbi_write_data(panel->dbi, &data, 1);
+}
+
+static int r61517_read_data(struct r61517 *panel)
+{
+ u8 data;
+ int ret;
+
+ ret = mipi_dbi_read_data(panel->dbi, &data, 1);
+ if (ret < 0)
+ return ret;
+
+ return data;
+}
+
+static void r61517_write(struct r61517 *panel, u8 reg, const u8 *data,
+size_t len)
+{
+ mipi_dbi_write_command(panel->dbi, reg);
+ mipi_dbi_write_data(panel->dbi, data, len);
+}
+
+static void r61517_write8(struct r61517 *panel, u8 reg, u8 data)
+{
+ r61517_write(panel, reg, &data, 1);
+}
+
+static void r61517_write16(struct r61517 *panel, u8 reg, u16 data)
+{
+ u8 buffer[2] = { (data >> 8) & 0xff, (data >> 0) & 0xff };
+
+ r61517_write(panel, reg, buffer, 2);
+}
+
+static void r61517_write32(struct r61517 *panel, u8 reg, u32 data)
+{
+ u8 buffer[4] = { (data >> 24) & 0xff, (data >> 16) & 0xff,
+(data >> 8) & 0xff, (data >> 0) & 0xff };
+
+ r61517_write(panel, reg, buffer, 4);
+}
+
+#define r61517_write_array(p, c, a) \
+ r61517_write((p), (c), (a), ARRAY_SIZE(a))
+
+static void r61517_reset(struct r61517 *panel)
+{
+ gpio_set_value(panel->pdata->protect, 0); /* PROTECT/ -> L */
+ gpio_set_value(panel->pdata->reset, 0); /* LCD_RST/ -> L */
+ gpio_set_value(panel->pdata->protect, 1); /* PROTECT/ -> H */
+ usleep_range(1100, 1200);
+ gpio_set_value(panel->pdata->reset, 1); /* LCD_RST/ -> H */
+ usleep_range(10, 100);
+ gpio_set_value(panel->pdata->protect, 0); /* PROTECT/ -> L */
+ msleep(20);
+}
+
+/*
-
+ * Configuration
+ */
+
+static const u8 data_frame_if[] = {
+ 0x02, /* WEMODE: 1=cont, 0=one-shot */
+ 0x00, 0x00,
+ 0x00, /* EPF, DFM */
+ 0x02, /* RIM[1] : 1 (18bpp) */
+};
+
+static const u8 data_panel[] = {
+ 0x0b,
+ 0x63, /* 400 lines */
+ 0x04, 0x00, 0x00, 0x04, 0x11, 0x
[PATCH 1/1] drm/exynos: Fix potential NULL pointer dereference in exynos_drm_encoder.c
[snip] >> >> And NULL pointer checking was already done above like below, >> >> if (overlay_ops && overlay_ops->disable) >> >> overlay_ops->disable(manager->dev, zpos); >> > Correct. But that check is applicable only for that one statement >> > (overlay_ops->disable(manager->dev, zpos);). >> > >> > Similar check needs to be added to below 'if' code too. >> >> What are your comments about this? >> > > Left condition first is checked so as I mentioned before, it doesn't need > overlay_ops checking because that was checked already. why do you think > overlay_ops should be checked again? > Consider the case when overlay_ops is NULL. if (overlay_ops && overlay_ops->disable) overlay_ops->disable(manager->dev, zpos); It does not enter this condition as overlay_ops is NULL and moves to the next statement, if (overlay_ops->wait_for_vblank) where it gets dereferenced. Please note we are not returning back from the first condition if overlay_ops is NULL. Hence we need to check the condition in second case too. -- With warm regards, Sachin
Linux 3.7-rc6
Hi Daniel, > My apologies for the long delay in answering, I've somehow mixed up > different bugreports and thought I've sent you a patch to test > already. Anyway, please test > > https://patchwork.kernel.org/patch/1728111/ Tested-by: Henrik Rydberg Thanks, Henrik
[PATCH] drm: exynos: hdmi: sending AVI and AUI info frames
This patch adds code for composing AVI and AUI info frames
and send them every VSYNC.
This patch is important for hdmi certification.
Based on exynos-drm-fixes branch of
git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git
Signed-off-by: Rahul Sharma
Signed-off-by: Fahad Kunnathadi
Signed-off-by: Shirish S
---
drivers/gpu/drm/exynos/exynos_hdmi.c | 142 +-
drivers/gpu/drm/exynos/exynos_hdmi.h | 23 ++
drivers/gpu/drm/exynos/regs-hdmi.h | 17 -
3 files changed, 161 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index ee110c9..5ffedc3 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -185,6 +185,7 @@ struct hdmi_v13_conf {
int height;
int vrefresh;
bool interlace;
+ int cea_video_id;
const u8 *hdmiphy_data;
const struct hdmi_v13_preset_conf *conf;
};
@@ -356,15 +357,20 @@ static const struct hdmi_v13_preset_conf
hdmi_v13_conf_1080p60 = {
};
static const struct hdmi_v13_conf hdmi_v13_confs[] = {
- { 1280, 720, 60, false, hdmiphy_v13_conf74_25, &hdmi_v13_conf_720p60 },
- { 1280, 720, 50, false, hdmiphy_v13_conf74_25, &hdmi_v13_conf_720p60 },
- { 720, 480, 60, false, hdmiphy_v13_conf27_027, &hdmi_v13_conf_480p },
- { 1920, 1080, 50, true, hdmiphy_v13_conf74_25, &hdmi_v13_conf_1080i50 },
- { 1920, 1080, 50, false, hdmiphy_v13_conf148_5,
-&hdmi_v13_conf_1080p50 },
- { 1920, 1080, 60, true, hdmiphy_v13_conf74_25, &hdmi_v13_conf_1080i60 },
- { 1920, 1080, 60, false, hdmiphy_v13_conf148_5,
-&hdmi_v13_conf_1080p60 },
+ { 1280, 720, 60, false, 4, hdmiphy_v13_conf74_25,
+ &hdmi_v13_conf_720p60 },
+ { 1280, 720, 50, false, 19, hdmiphy_v13_conf74_25,
+ &hdmi_v13_conf_720p60 },
+ { 720, 480, 60, false, 3, hdmiphy_v13_conf27_027,
+ &hdmi_v13_conf_480p },
+ { 1920, 1080, 50, true, 20, hdmiphy_v13_conf74_25,
+ &hdmi_v13_conf_1080i50 },
+ { 1920, 1080, 50, false, 31, hdmiphy_v13_conf148_5,
+ &hdmi_v13_conf_1080p50 },
+ { 1920, 1080, 60, true, 5, hdmiphy_v13_conf74_25,
+ &hdmi_v13_conf_1080i60 },
+ { 1920, 1080, 60, false, 16, hdmiphy_v13_conf148_5,
+ &hdmi_v13_conf_1080p60 },
};
/* HDMI Version 1.4 */
@@ -482,6 +488,7 @@ struct hdmi_conf {
int height;
int vrefresh;
bool interlace;
+ int cea_video_id;
const u8 *hdmiphy_data;
const struct hdmi_preset_conf *conf;
};
@@ -937,16 +944,21 @@ static const struct hdmi_preset_conf hdmi_conf_1080p60 = {
};
static const struct hdmi_conf hdmi_confs[] = {
- { 720, 480, 60, false, hdmiphy_conf27_027, &hdmi_conf_480p60 },
- { 1280, 720, 50, false, hdmiphy_conf74_25, &hdmi_conf_720p50 },
- { 1280, 720, 60, false, hdmiphy_conf74_25, &hdmi_conf_720p60 },
- { 1920, 1080, 50, true, hdmiphy_conf74_25, &hdmi_conf_1080i50 },
- { 1920, 1080, 60, true, hdmiphy_conf74_25, &hdmi_conf_1080i60 },
- { 1920, 1080, 30, false, hdmiphy_conf74_176, &hdmi_conf_1080p30 },
- { 1920, 1080, 50, false, hdmiphy_conf148_5, &hdmi_conf_1080p50 },
- { 1920, 1080, 60, false, hdmiphy_conf148_5, &hdmi_conf_1080p60 },
+ { 720, 480, 60, false, 3, hdmiphy_conf27_027, &hdmi_conf_480p60 },
+ { 1280, 720, 50, false, 19, hdmiphy_conf74_25, &hdmi_conf_720p50 },
+ { 1280, 720, 60, false, 4, hdmiphy_conf74_25, &hdmi_conf_720p60 },
+ { 1920, 1080, 50, true, 20, hdmiphy_conf74_25, &hdmi_conf_1080i50 },
+ { 1920, 1080, 60, true, 5, hdmiphy_conf74_25, &hdmi_conf_1080i60 },
+ { 1920, 1080, 30, false, 34, hdmiphy_conf74_176, &hdmi_conf_1080p30 },
+ { 1920, 1080, 50, false, 31, hdmiphy_conf148_5, &hdmi_conf_1080p50 },
+ { 1920, 1080, 60, false, 16, hdmiphy_conf148_5, &hdmi_conf_1080p60 },
};
+struct hdmi_infoframe {
+ enum HDMI_PACKET_TYPE type;
+ u8 ver;
+ u8 len;
+};
static inline u32 hdmi_reg_read(struct hdmi_context *hdata, u32 reg_id)
{
@@ -1270,6 +1282,88 @@ static int hdmi_conf_index(struct hdmi_context *hdata,
return hdmi_v14_conf_index(mode);
}
+static u8 hdmi_chksum(struct hdmi_context *hdata,
+ u32 start, u8 len, u32 hdr_sum)
+{
+ int i;
+ /* hdr_sum : header0 + header1 + header2
+ * start : start address of packet byte1
+ * len : packet bytes - 1 */
+ for (i = 0; i < len; ++i)
+ hdr_sum += 0xff & hdmi_reg_read(hdata, start + i * 4);
+
+ return (u8)(0x100 - (hdr_sum & 0xff));
+}
+
+void hdmi_reg_infoframe(struct hdmi_context *hdata,
+ struct hdmi_infoframe *infoframe)
+{
+ u32 hdr_sum;
+ u8 chksum;
+ u32 aspe
i915: black screen after blank when LID is closed on Linux >= 3.1
Hi,
since Linux 3.1 I'm having some problems with i915 driver on HP nc6120
with 915GM chipset. The display goes black after the kernel tries to
blank screen while LID is closed (see steps to reproduce to more detailed
description).
Currently I'm using Linux 3.7-rc6 with KMS enabled and disabled ACPI
video by something equivalent to (without it the LID, and the whole
ACPI, does not work):
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 0230cb6..40e060b 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -1782,6 +1797,10 @@ static int __init intel_opregion_present(void)
int acpi_video_register(void)
{
int result = 0;
+ return -ENODEV;
if (register_count) {
/*
* if the function of acpi_video_register is already called,
I also tested this kernel with merged
git://people.freedesktop.org/~danvet/drm-intel drm-intel-next.
Steps to reproduce:
1. $ setterm -blank 1
2. close LID
3. sleep 90
4. open LID
5. press key
6. close LID
nothing happens here, the screen is still turned on, should be turned off
7. open LID
backlight is on, but screen is black, but some artifacts are visible so
LCD is probably turned off, and the display is not usable anymore
I have bisected this problem and it's introduced by commit
120eced9efe7fdb5123db4ea47e9adee9b66284e
(drm/i915: Set crtc DPMS mode to ON in intel_crtc_mode_set).
In Linux 3.1 to 3.6 I've been using following workaround:
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 2163818..51ebb77 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6000,10 +6000,12 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
x, y, old_fb);
drm_vblank_post_modeset(dev, pipe);
+#if 0
if (ret)
intel_crtc->dpms_mode = DRM_MODE_DPMS_OFF;
else
intel_crtc->dpms_mode = DRM_MODE_DPMS_ON;
+#endif
return ret;
}
Thanks,
Krzysiek
---
# lspci -v
...
00:02.0 VGA compatible controller: Intel Corporation Mobile 915GM/GMS/910GML
Express Graphics Controller (rev 03) (prog-if 00 [VGA controller])
Subsystem: Hewlett-Packard Company NX6110/NC6120
Flags: bus master, fast devsel, latency 0, IRQ 16
Memory at d040 (32-bit, non-prefetchable) [size=512K]
I/O ports at 7000 [size=8]
Memory at c000 (32-bit, prefetchable) [size=256M]
Memory at d048 (32-bit, non-prefetchable) [size=256K]
Expansion ROM at [disabled]
Capabilities: [d0] Power Management version 2
Kernel driver in use: i915
00:02.1 Display controller: Intel Corporation Mobile 915GM/GMS/910GML Express
Graphics Controller (rev 03)
Subsystem: Hewlett-Packard Company NX6110/NC6120
Flags: bus master, fast devsel, latency 0
Memory at d050 (32-bit, non-prefetchable) [size=512K]
Capabilities: [d0] Power Management version 2
$ dmesg
...
[ cut here ]
WARNING: at
/home/krzysiek/src/linux-2.6/drivers/gpu/drm/i915/intel_display.c:3588
intel_modeset_check_state+0x612/0x640()
Hardware name: NC6120(EK094ES)
active connector not linked to encoder
Pid: 420, comm: kworker/0:2 Not tainted 3.7.0-rc6-3-gddba8b4 #83
Call Trace:
[<80131173>] ? warn_slowpath_common+0x73/0xb0
[<80394cc2>] ? intel_modeset_check_state+0x612/0x640
[<80394cc2>] ? intel_modeset_check_state+0x612/0x640
[<80131203>] ? warn_slowpath_fmt+0x33/0x40
[<80394cc2>] ? intel_modeset_check_state+0x612/0x640
[<8032336f>] ? acpi_lid_open+0x25/0x3e
[<8039962d>] ? intel_lid_notify+0x8d/0xb0
[<80151fb2>] ? notifier_call_chain+0x42/0x60
[<801522b4>] ? __blocking_notifier_call_chain+0x44/0x60
[<801522e7>] ? blocking_notifier_call_chain+0x17/0x20
[<80322e2a>] ? acpi_lid_send_state+0x7e/0xa2
[<80322e92>] ? acpi_button_notify+0x2f/0xae
[<8030511b>] ? acpi_device_notify+0xf/0x11
[<8030f5eb>] ? acpi_ev_notify_dispatch+0x2b/0x40
[<80302f08>] ? acpi_os_execute_deferred+0x18/0x21
[<80147224>] ? process_one_work+0x104/0x380
[<8013d963>] ? internal_add_timer+0x13/0x40
[<80302ef0>] ? acpi_os_wait_events_complete+0x14/0x14
[<80147797>] ? worker_thread+0x107/0x3a0
[<80147690>] ? rescuer_thread+0x1c0/0x1c0
[<8014bd7c>] ? kthread+0x8c/0xa0
[<806097b7>] ? ret_from_kernel_thread+0x1b/0x28
[<8014bcf0>] ? __kthread_parkme+0x60/0x60
---[ end trace 2fa112e1a3f687cc ]---
[ cut here ]
WARNING: at
/home/krzysiek/src/linux-2.6/drivers/gpu/drm/i915/intel_display.c:3590
intel_modeset_check_state+0x5ef/0x640()
Hardware name: NC6120(EK094ES)
encoder->connectors_active not set
Pid: 420, comm: kworker/0:2 Tainted: GW3.7.0-rc6-3-gddba8b4 #83
Call Trace:
[<80131173>] ? warn_slowpath_common+0x73/0xb0
[<80394c9f>] ? intel_modeset_check_state+0x5ef/0x640
[<80394c9f>] ? intel_modeset_check_state+0x5ef/0x640
[<80131203>] ? warn_slowpath_fmt+0x33
i915: black screen after blank when LID is closed on Linux >= 3.1
On Thu, Nov 22, 2012 at 09:17:54PM +0100, Daniel Vetter wrote:
> Hi,
>
> Thanks for the report. Now this smells like something which could take
> a bit longer to track down, so can you please file this on
> bugs.freedesktop.org against DRM -> DRI/Intel to ensure that we dont'
> loose track of it?
Yes, I will add a bug report there.
>
> A few questions and things to test below.
>
> On Thu, Nov 22, 2012 at 7:54 PM, Krzysztof Mazur
> wrote:
> > since Linux 3.1 I'm having some problems with i915 driver on HP nc6120
> > with 915GM chipset. The display goes black after the kernel tries to
> > blank screen while LID is closed (see steps to reproduce to more detailed
> > description).
> >
> > Currently I'm using Linux 3.7-rc6 with KMS enabled and disabled ACPI
> > video by something equivalent to (without it the LID, and the whole
> > ACPI, does not work):
>
> Can you be more specific about what does not work? Do the lid events
> no longer function, or is something else broken?
It's a separate issue, probably not related to i915 driver. However
it might be important that I have disabled ACPI video, and it might
cause later problems so I added that information. However in < 3.1
kernel I also had disabled ACPI video and everything worked without
problems.
This ACPI problem started around 2.6.33 (the first workaround I used
is on top of 2.6.33.7:
commit 1dcdd646523a63a9e6476fd5ef46d82c67bb9a99
Author: Krzysztof Mazur
Date: Wed Aug 11 09:35:35 2010 +0200
disable ACPI video
ACPI video causes problem on HP Compaq nc6120. After starting of X
lid stops working.
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index b765790..1427cbb 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -2478,7 +2478,8 @@ static int __init acpi_video_init(void)
{
dmi_check_system(video_dmi_table);
- if (intel_opregion_present())
+ /* KM: test for nc6120 */
+ if (intel_opregion_present() || 1)
return 0;
return acpi_video_register();
)
I found similar bug reported on
https://bugzilla.redhat.com/show_bug.cgi?id=512958 and I never reported
that. I also tried:
static int acpi_video_bus_start_devices(struct acpi_video_bus *video)
{
+ return acpi_video_bus_DOS(video, 3, 0);
return acpi_video_bus_DOS(video, 0, 0);
}
but without success.
Without ACPI video driver loaded everything works, LID button sends ACPI
events, the display is tuned off when LID is pressed. After loading ACPI
video driver the LID stops sending ACPI events, and some other really
strange issues starts - for instance reading
/sys/class/thermal/thermal_zone%d/tempcauses causes displaying "^@" in
console.
> >
> > In Linux 3.1 to 3.6 I've been using following workaround:
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 2163818..51ebb77 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -6000,10 +6000,12 @@ static int intel_crtc_mode_set(struct drm_crtc
> > *crtc,
> > x, y, old_fb);
> > drm_vblank_post_modeset(dev, pipe);
> >
> > +#if 0
> > if (ret)
> > intel_crtc->dpms_mode = DRM_MODE_DPMS_OFF;
> > else
> > intel_crtc->dpms_mode = DRM_MODE_DPMS_ON;
> > +#endif
> >
> > return ret;
> > }
>
> Since a dpms ioctl call tends to follow a modeset, this likely only
> results in that dpms call enabling the hw again. Can you please add
> drm.debug=0xe to your kernel cmdline and boot into a 3.6 with this
> hack applied, reproduce the issue and the attach the complete dmesg?
>
> The below WARNs from 3.7 support that, we've simply improved the
> code's ability to detect such problems. Can you please boot into a
> kernel with the latest drm-intel-next-queued branch merged in, but no
> other patches applied. Again please append drm.debug=0xe and then
> attach the complete dmesg after you've reproduced the issue.
Thanks, I will check that, but probably I will need to disable ACPI
video, because without it the LID does not work so the problem cannot
be probably triggered.
>
> Please also compile your kernels with CONFIG_PRINTK_TIME=y, the
> timestamps in dmesg help a lot in figuring things out.
>
> Yours, Daniel
>
Thanks,
Krzysiek
i915: black screen after blank when LID is closed on Linux >= 3.1
On Thu, Nov 22, 2012 at 09:17:54PM +0100, Daniel Vetter wrote:
> Hi,
>
>
> Since a dpms ioctl call tends to follow a modeset, this likely only
> results in that dpms call enabling the hw again. Can you please add
> drm.debug=0xe to your kernel cmdline and boot into a 3.6 with this
> hack applied, reproduce the issue and the attach the complete dmesg?
>
> The below WARNs from 3.7 support that, we've simply improved the
> code's ability to detect such problems. Can you please boot into a
> kernel with the latest drm-intel-next-queued branch merged in, but no
> other patches applied. Again please append drm.debug=0xe and then
> attach the complete dmesg after you've reproduced the issue.
>
> Please also compile your kernels with CONFIG_PRINTK_TIME=y, the
> timestamps in dmesg help a lot in figuring things out.
In lastest drm-intel-next-queued (v3.7-rc4-323-g9352dce) I got
following compile error:
/home/krzysiek/src/linux-2.6/drivers/gpu/drm/i915/intel_lvds.c: In function
'intel_lvds_init':
/home/krzysiek/src/linux-2.6/drivers/gpu/drm/i915/intel_lvds.c:1098:34: error:
'mode' undeclared (first use in this function)
/home/krzysiek/src/linux-2.6/drivers/gpu/drm/i915/intel_lvds.c:1098:34: note:
each undeclared identifier is reported only once for each function it appears in
Without thinking I fixed it by:
diff --git a/drivers/gpu/drm/i915/intel_lvds.c
b/drivers/gpu/drm/i915/intel_lvds.c
index ced06f3..43f0874 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -1095,7 +1095,7 @@ bool intel_lvds_init(struct drm_device *dev)
fixed_mode = intel_crtc_mode_get(dev, crtc);
if (fixed_mode) {
DRM_DEBUG_KMS("using current (BIOS) mode: ");
- drm_mode_debug_printmodeline(&mode);
+ drm_mode_debug_printmodeline(fixed_mode);
fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
goto out;
}
Thanks,
Krzysiek
Re: [PATCH] drm/i915: set the AVI VIC of the HDMI mode
2012/11/22 Thierry Reding : > On Wed, Nov 21, 2012 at 05:08:12PM +0100, Daniel Vetter wrote: >> On Wed, Nov 21, 2012 at 4:47 PM, Thierry Reding >> wrote: >> > Oh great, so I copied that table for nothing. Thanks for Cc'ing, I can >> > reuse that in the HDMI infoframe series. >> >> Wrt the infoframe series, I think it'd be awesome if you could convert >> i915 and radeon (iirc the existing drivers with the "best" avi >> infoframe support) over to the new code. This gives some nice >> validation, both by testing on actual hw and that the interface is >> sane, since it'll be used by 2-3 different drivers then. > > I'll have to rely on somebody else to do the testing since I don't have > an HDMI capable hardware except Tegra to run this on. But yes, I had > planned to convert Tegra and at least one other driver for reference. > But I guess while at it I could just as well convert all of them. I'll take a look at radeon (I'll try to convert it to the new functions) over weekend. -- Rafał ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/i915: set the AVI VIC of the HDMI mode
On Thu, Nov 22, 2012 at 09:00:24AM +0100, Rafał Miłecki wrote: > 2012/11/22 Thierry Reding : > > On Wed, Nov 21, 2012 at 05:08:12PM +0100, Daniel Vetter wrote: > >> On Wed, Nov 21, 2012 at 4:47 PM, Thierry Reding > >> wrote: > >> > Oh great, so I copied that table for nothing. Thanks for Cc'ing, I can > >> > reuse that in the HDMI infoframe series. > >> > >> Wrt the infoframe series, I think it'd be awesome if you could convert > >> i915 and radeon (iirc the existing drivers with the "best" avi > >> infoframe support) over to the new code. This gives some nice > >> validation, both by testing on actual hw and that the interface is > >> sane, since it'll be used by 2-3 different drivers then. > > > > I'll have to rely on somebody else to do the testing since I don't have > > an HDMI capable hardware except Tegra to run this on. But yes, I had > > planned to convert Tegra and at least one other driver for reference. > > But I guess while at it I could just as well convert all of them. > > I'll take a look at radeon (I'll try to convert it to the new > functions) over weekend. Okay, great! I think for radeon things should be the easiest since it doesn't currently fill in anything but the colorspace field. Judging by the bug report that Paulo mentioned this will probably not be enough for some hardware, but should be enough according to the specification. Having more data in the AVI infoframe shouldn't hurt, though. Thierry pgpBjcmNWlpQZ.pgp Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Trace on HD6320 after resume on v3.7-rc6
> > [ 245.003823] handlers: > > [ 245.003838] [] azx_interrupt [snd_hda_intel] > > [ 245.003841] Disabling IRQ #16 > > Does /proc/interrupts show IRQ 16 being shared between snd_hda_intel > and radeon? If not, this looks like a snd_hda_intel (or another driver > sharing the IRQ) issue. /proc/interrupts shows no sharing between snd_hda_intel and radeon. I just noticed, that radeon was stated in the trace, therefore I send it to this mailing list. Thank you for the response and sorry for the noise. With best regards, Julian Wollrath ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
