Re: [PATCH v2 3/3] modetest: Add a command line parameter to select the driver
On Mon, 11 Feb 2013, Laurent Pinchart wrote:
> On Monday 11 February 2013 21:13:45 Laurent Pinchart wrote:
>> If the -M parameter is specific, modetest will use the requested device
>> name instead of trying its builtin list of device names.
>>
>> Signed-off-by: Laurent Pinchart
>> ---
>> tests/modetest/modetest.c | 41 -
>> 1 file changed, 28 insertions(+), 13 deletions(-)
>>
>> diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
>> index 34457e2..9a2d1f8 100644
>> --- a/tests/modetest/modetest.c
>> +++ b/tests/modetest/modetest.c
>
> [snip]
>
>> @@ -989,14 +996,27 @@ int main(int argc, char **argv)
>> if (argc == 1)
>> encoders = connectors = crtcs = planes = modes = framebuffers =
>> 1;
>>
>> -for (i = 0; i < ARRAY_SIZE(modules); i++) {
>> -printf("trying to load module %s...", modules[i]);
>> -fd = drmOpen(modules[i], NULL);
>> +if (module) {
>> +fd = drmOpen(module, NULL);
>> if (fd < 0) {
>> -printf("failed.\n");
>> -} else {
>> -printf("success.\n");
>> -break;
>> +fprintf(stderr, "failed to open device '%s'.\n",
>> module);
>> +return 1;
>> +}
>> +} else {
>> +for (i = 0; i < ARRAY_SIZE(modules); i++) {
>> +printf("trying to open device '%s'...", modules[i]);
>> +fd = drmOpen(modules[i], NULL);
>> +if (fd < 0) {
>> +printf("failed.\n");
>> +} else {
>> +printf("success.\n");
>> +break;
>> +}
>> +}
>> +
>> +if (fd < 0) {
>> +fprintf(stderr, "no device found.\n", module);
>
> I should sleep before sending patches... Sorry for the noise, v3 will fix
> that.
If you fix that bit, you can slam my
Reviewed-by: Jani Nikula
on the series.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[GMA500] Valid VCO frequency range on GMA500 platform?
On Mon, Feb 11, 2013 at 2:15 PM, "David M?ller (ELSOFT AG)" wrote: > Hi Patrik > > Patrik Jakobsson wrote: >> The value of VCO_MIN comes from the Intel PRM for similar GPUs. >> >> Instead of changing VCO_MIN, could you try setting N_MIN=1, N_MAX=6 and >> M1_MAX=22? I'll test it on my own hardware tomorrow. > > Thanks for your suggestion. > > With "N_MIN=1, N_MAX=6 and M1_MAX=22", i get a refresh rate of 59.9Hz. > > > Dave > Those values come from the i915 driver and should work fine for us as well. I had no problems at my end so I'll submit a patch for this. -Patrik
[PATCH 1/5] radeon: Remove redundant NULL check before radeon_i2c_destroy().
radeon_i2c_destroy on a NULL pointer is a no-op.
Signed-off-by: Cyril Roelandt
---
drivers/gpu/drm/radeon/radeon_i2c.c |6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_i2c.c
b/drivers/gpu/drm/radeon/radeon_i2c.c
index fc60b74..850df44 100644
--- a/drivers/gpu/drm/radeon/radeon_i2c.c
+++ b/drivers/gpu/drm/radeon/radeon_i2c.c
@@ -1032,10 +1032,8 @@ void radeon_i2c_fini(struct radeon_device *rdev)
int i;
for (i = 0; i < RADEON_MAX_I2C_BUS; i++) {
- if (rdev->i2c_bus[i]) {
- radeon_i2c_destroy(rdev->i2c_bus[i]);
- rdev->i2c_bus[i] = NULL;
- }
+ radeon_i2c_destroy(rdev->i2c_bus[i]);
+ rdev->i2c_bus[i] = NULL;
}
}
--
1.7.10.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH linux-next] drm/radeon: Avoid NULL pointer dereference from atom_index_iio() allocation failure
Smatch anlysis:
drivers/gpu/drm/radeon/atom.c:1242 atom_index_iio() error: potential null
dereference 'ctx->iio'. (kzalloc returns null)
Also cleaned up some checks before calls to kfree(). kfree(NULL) is OK.
Cc: David Airlie
Cc: Alex Deucher
Cc: "Michel Dänzer"
Cc: Dave Airlie
Cc: "Christian König"
Cc: Jerome Glisse
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Tim Gardner
---
drivers/gpu/drm/radeon/atom.c |9 +++--
drivers/gpu/drm/radeon/radeon_device.c |9 -
2 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/radeon/atom.c b/drivers/gpu/drm/radeon/atom.c
index 5ce9bf5..46a9c37 100644
--- a/drivers/gpu/drm/radeon/atom.c
+++ b/drivers/gpu/drm/radeon/atom.c
@@ -1238,6 +1238,8 @@ static int atom_iio_len[] = { 1, 2, 3, 3, 3, 3, 4, 4, 4,
3 };
static void atom_index_iio(struct atom_context *ctx, int base)
{
ctx->iio = kzalloc(2 * 256, GFP_KERNEL);
+ if (!ctx->iio)
+ return;
while (CU8(base) == ATOM_IIO_START) {
ctx->iio[CU8(base + 1)] = base + 2;
base += 2;
@@ -1287,6 +1289,10 @@ struct atom_context *atom_parse(struct card_info *card,
void *bios)
ctx->cmd_table = CU16(base + ATOM_ROM_CMD_PTR);
ctx->data_table = CU16(base + ATOM_ROM_DATA_PTR);
atom_index_iio(ctx, CU16(ctx->data_table + ATOM_DATA_IIO_PTR) + 4);
+ if (!ctx->iio) {
+ atom_destroy(ctx);
+ return NULL;
+ }
str = CSTR(CU16(base + ATOM_ROM_MSG_PTR));
while (*str && ((*str == '\n') || (*str == '\r')))
@@ -1335,8 +1341,7 @@ int atom_asic_init(struct atom_context *ctx)
void atom_destroy(struct atom_context *ctx)
{
- if (ctx->iio)
- kfree(ctx->iio);
+ kfree(ctx->iio);
kfree(ctx);
}
diff --git a/drivers/gpu/drm/radeon/radeon_device.c
b/drivers/gpu/drm/radeon/radeon_device.c
index 8794de1..44b8034 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -759,6 +759,11 @@ int radeon_atombios_init(struct radeon_device *rdev)
atom_card_info->pll_write = cail_pll_write;
rdev->mode_info.atom_context = atom_parse(atom_card_info, rdev->bios);
+ if (!rdev->mode_info.atom_context) {
+ radeon_atombios_fini(rdev);
+ return -ENOMEM;
+ }
+
mutex_init(&rdev->mode_info.atom_context->mutex);
radeon_atom_initialize_bios_scratch_regs(rdev->ddev);
atom_allocate_fb_scratch(rdev->mode_info.atom_context);
@@ -778,9 +783,11 @@ void radeon_atombios_fini(struct radeon_device *rdev)
{
if (rdev->mode_info.atom_context) {
kfree(rdev->mode_info.atom_context->scratch);
- kfree(rdev->mode_info.atom_context);
}
+ kfree(rdev->mode_info.atom_context);
+ rdev->mode_info.atom_context = NULL;
kfree(rdev->mode_info.atom_card_info);
+ rdev->mode_info.atom_card_info = NULL;
}
/* COMBIOS */
--
1.7.9.5
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] modetest: Add a command line parameter to select the driver
Hi Jani,
On Friday 08 February 2013 15:15:50 Jani Nikula wrote:
> On Fri, 08 Feb 2013, Laurent Pinchart wrote:
> > If the -M parameter is specific, modetest will use the requested device
> > name instead of trying its builtin list of device names.
> >
> > Signed-off-by: Laurent Pinchart
> > ---
> >
> > tests/modetest/modetest.c | 34 +++---
> > 1 file changed, 27 insertions(+), 7 deletions(-)
> >
> > diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> > index 34457e2..b6298fc 100644
> > --- a/tests/modetest/modetest.c
> > +++ b/tests/modetest/modetest.c
[snip]
> > @@ -989,14 +996,27 @@ int main(int argc, char **argv)
> > if (argc == 1)
> > encoders = connectors = crtcs = planes = modes = framebuffers =
> > 1;
> >
> > - for (i = 0; i < ARRAY_SIZE(modules); i++) {
> > - printf("trying to load module %s...", modules[i]);
> > + if (module) {
> >
> > fd = drmOpen(modules[i], NULL);
>
> If this worked for you, I presume you have some uncommitted changes in
> your tree. ;) The compiler should cry about uninitialized use of i too.
>
> fd = drmOpen(module, NULL); ?
Oops :-)
modetest is compiled without any -W flag. I'll send a v4 that enables warnings
and fixes them (please ignore the v3, sorry for the noise).
> > if (fd < 0) {
> >
> > - printf("failed.\n");
> > - } else {
> > - printf("success.\n");
> > - break;
> > + printf("failed to open device '%s'.\n", module);
> > + return 1;
> > + }
> > + } else {
> > + for (i = 0; i < ARRAY_SIZE(modules); i++) {
> > + printf("trying to open device '%s'...", modules[i]);
> > + fd = drmOpen(modules[i], NULL);
> > + if (fd < 0) {
> > + printf("failed.\n");
> > + } else {
> > + printf("success.\n");
> > + break;
> > + }
> > + }
> > +
> > + if (fd < 0) {
> > + printf("no device found.\n", module);
>
> Extra param to printf.
Will be fixed in v4.
--
Regards,
Laurent Pinchart
[Bug 53391] nouveau: wrong display output order
https://bugzilla.kernel.org/show_bug.cgi?id=53391 --- Comment #9 from Marcin Slusarz 2013-02-11 22:12:35 --- Yes, please do it. I think it will fix all issues (both VGA and DVI setups will work and output numbering will be correct). -- Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are watching the assignee of the bug.
drm/nv50/pm: use hwsq for engine reclocking too
Hello Ben Skeggs,
The patch 496a73bbecb8: "drm/nv50/pm: use hwsq for engine reclocking
too" from Jan 24, 2012, leads to the following Smatch warning:
"drivers/gpu/drm/nouveau/nv50_pm.c:638 nv50_pm_clocks_pre()
warn: 'info->mmast' might be uninitialized"
[ This Smatch check isn't ready for release yet ].
drivers/gpu/drm/nouveau/nv50_pm.c
621
622 info = kmalloc(sizeof(*info), GFP_KERNEL);
623 if (!info)
624 return ERR_PTR(-ENOMEM);
625 info->perflvl = perflvl;
626
627 /* memory: build hwsq ucode which we'll use to reclock memory.
628 * use pcie refclock if possible, otherwise use mpll */
629 info->mclk_hwsq.len = 0;
630 if (perflvl->memory) {
631 ret = calc_mclk(dev, perflvl, info);
632 if (ret)
633 goto error;
634 info->mscript = perflvl->memscript;
635 }
636
637 divs = read_div(dev);
638 mast = info->mmast;
^^^
My reading is that "info" is setup inside calc_mclk() so if
perflvl->memory is false then info->mmast isn't set.
regards,
dan carpenter
[PATCH v2 3/3] modetest: Add a command line parameter to select the driver
On Monday 11 February 2013 21:13:45 Laurent Pinchart wrote:
> If the -M parameter is specific, modetest will use the requested device
> name instead of trying its builtin list of device names.
>
> Signed-off-by: Laurent Pinchart
> ---
> tests/modetest/modetest.c | 41 -
> 1 file changed, 28 insertions(+), 13 deletions(-)
>
> diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> index 34457e2..9a2d1f8 100644
> --- a/tests/modetest/modetest.c
> +++ b/tests/modetest/modetest.c
[snip]
> @@ -989,14 +996,27 @@ int main(int argc, char **argv)
> if (argc == 1)
> encoders = connectors = crtcs = planes = modes = framebuffers =
> 1;
>
> - for (i = 0; i < ARRAY_SIZE(modules); i++) {
> - printf("trying to load module %s...", modules[i]);
> - fd = drmOpen(modules[i], NULL);
> + if (module) {
> + fd = drmOpen(module, NULL);
> if (fd < 0) {
> - printf("failed.\n");
> - } else {
> - printf("success.\n");
> - break;
> + fprintf(stderr, "failed to open device '%s'.\n",
> module);
> + return 1;
> + }
> + } else {
> + for (i = 0; i < ARRAY_SIZE(modules); i++) {
> + printf("trying to open device '%s'...", modules[i]);
> + fd = drmOpen(modules[i], NULL);
> + if (fd < 0) {
> + printf("failed.\n");
> + } else {
> + printf("success.\n");
> + break;
> + }
> + }
> +
> + if (fd < 0) {
> + fprintf(stderr, "no device found.\n", module);
I should sleep before sending patches... Sorry for the noise, v3 will fix
that.
> + return 1;
> }
> }
--
Regards,
Laurent Pinchart
[PATCH v2 3/3] modetest: Add a command line parameter to select the driver
If the -M parameter is specific, modetest will use the requested device
name instead of trying its builtin list of device names.
Signed-off-by: Laurent Pinchart
---
tests/modetest/modetest.c | 41 -
1 file changed, 28 insertions(+), 13 deletions(-)
diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 34457e2..9a2d1f8 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -908,6 +908,9 @@ void usage(char *name)
fprintf(stderr, "\t-s [@]:[@]\tset
a mode\n");
fprintf(stderr, "\t-v\ttest vsynced page flipping\n");
+ fprintf(stderr, "\n Generic options:\n\n");
+ fprintf(stderr, "\t-M module\tuse the given driver\n");
+
fprintf(stderr, "\n\tDefault is to dump all info.\n");
exit(0);
}
@@ -935,7 +938,7 @@ static int page_flipping_supported(void)
#endif
}
-static char optstr[] = "cefmP:ps:v";
+static char optstr[] = "cefM:mP:ps:v";
int main(int argc, char **argv)
{
@@ -943,6 +946,7 @@ int main(int argc, char **argv)
int encoders = 0, connectors = 0, crtcs = 0, planes = 0, framebuffers =
0;
int test_vsync = 0;
char *modules[] = { "i915", "radeon", "nouveau", "vmwgfx", "omapdrm",
"exynos" };
+ char *module = NULL;
unsigned int i;
int count = 0, plane_count = 0;
struct connector con_args[2];
@@ -960,6 +964,9 @@ int main(int argc, char **argv)
case 'f':
framebuffers = 1;
break;
+ case 'M':
+ module = optarg;
+ break;
case 'm':
modes = 1;
break;
@@ -989,14 +996,27 @@ int main(int argc, char **argv)
if (argc == 1)
encoders = connectors = crtcs = planes = modes = framebuffers =
1;
- for (i = 0; i < ARRAY_SIZE(modules); i++) {
- printf("trying to load module %s...", modules[i]);
- fd = drmOpen(modules[i], NULL);
+ if (module) {
+ fd = drmOpen(module, NULL);
if (fd < 0) {
- printf("failed.\n");
- } else {
- printf("success.\n");
- break;
+ fprintf(stderr, "failed to open device '%s'.\n",
module);
+ return 1;
+ }
+ } else {
+ for (i = 0; i < ARRAY_SIZE(modules); i++) {
+ printf("trying to open device '%s'...", modules[i]);
+ fd = drmOpen(modules[i], NULL);
+ if (fd < 0) {
+ printf("failed.\n");
+ } else {
+ printf("success.\n");
+ break;
+ }
+ }
+
+ if (fd < 0) {
+ fprintf(stderr, "no device found.\n", module);
+ return 1;
}
}
@@ -1005,11 +1025,6 @@ int main(int argc, char **argv)
return -1;
}
- if (i == ARRAY_SIZE(modules)) {
- fprintf(stderr, "failed to load any modules, aborting.\n");
- return -1;
- }
-
resources = drmModeGetResources(fd);
if (!resources) {
fprintf(stderr, "drmModeGetResources failed: %s\n",
--
1.7.12.4
[PATCH v2 2/3] modetest: Sort command line arguments
The current mostly random sort order hinders code readability. Sort the
options alphabetically in the code, and by group in the help message.
Signed-off-by: Laurent Pinchart
---
tests/modetest/modetest.c | 49 ++-
1 file changed, 27 insertions(+), 22 deletions(-)
diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 489918e..34457e2 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -835,8 +835,6 @@ set_mode(struct connector *c, int count, struct plane *p,
int plane_count,
kms_destroy(&kms);
}
-static char optstr[] = "ecpmfs:P:v";
-
#define min(a, b) ((a) < (b) ? (a) : (b))
static int parse_connector(struct connector *c, const char *arg)
@@ -896,15 +894,20 @@ static int parse_plane(struct plane *p, const char *arg)
void usage(char *name)
{
- fprintf(stderr, "usage: %s [-ecpmf]\n", name);
- fprintf(stderr, "\t-e\tlist encoders\n");
+ fprintf(stderr, "usage: %s [-cefmPpsv]\n", name);
+
+ fprintf(stderr, "\n Query options:\n\n");
fprintf(stderr, "\t-c\tlist connectors\n");
- fprintf(stderr, "\t-p\tlist CRTCs and planes (pipes)\n");
- fprintf(stderr, "\t-m\tlist modes\n");
+ fprintf(stderr, "\t-e\tlist encoders\n");
fprintf(stderr, "\t-f\tlist framebuffers\n");
- fprintf(stderr, "\t-v\ttest vsynced page flipping\n");
- fprintf(stderr, "\t-s [@]:[@]\tset
a mode\n");
+ fprintf(stderr, "\t-m\tlist modes\n");
+
+ fprintf(stderr, "\n Test options:\n\n");
+ fprintf(stderr, "\t-p\tlist CRTCs and planes (pipes)\n");
fprintf(stderr, "\t-P :x[@]\tset a
plane\n");
+ fprintf(stderr, "\t-s [@]:[@]\tset
a mode\n");
+ fprintf(stderr, "\t-v\ttest vsynced page flipping\n");
+
fprintf(stderr, "\n\tDefault is to dump all info.\n");
exit(0);
}
@@ -932,6 +935,8 @@ static int page_flipping_supported(void)
#endif
}
+static char optstr[] = "cefmP:ps:v";
+
int main(int argc, char **argv)
{
int c;
@@ -946,34 +951,34 @@ int main(int argc, char **argv)
opterr = 0;
while ((c = getopt(argc, argv, optstr)) != -1) {
switch (c) {
- case 'e':
- encoders = 1;
- break;
case 'c':
connectors = 1;
break;
- case 'p':
- crtcs = 1;
- planes = 1;
+ case 'e':
+ encoders = 1;
+ break;
+ case 'f':
+ framebuffers = 1;
break;
case 'm':
modes = 1;
break;
- case 'f':
- framebuffers = 1;
+ case 'P':
+ if (parse_plane(&plane_args[plane_count], optarg) < 0)
+ usage(argv[0]);
+ plane_count++;
break;
- case 'v':
- test_vsync = 1;
+ case 'p':
+ crtcs = 1;
+ planes = 1;
break;
case 's':
if (parse_connector(&con_args[count], optarg) < 0)
usage(argv[0]);
count++;
break;
- case 'P':
- if (parse_plane(&plane_args[plane_count], optarg) < 0)
- usage(argv[0]);
- plane_count++;
+ case 'v':
+ test_vsync = 1;
break;
default:
usage(argv[0]);
--
1.7.12.4
[PATCH v2 1/3] modetest: Remove extern declarations of opt(arg|ind|err|opt)
Those variables are declared in unistd.h, there's no need to redeclare them here. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index c91bb9d..489918e 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -835,8 +835,6 @@ set_mode(struct connector *c, int count, struct plane *p, int plane_count, kms_destroy(&kms); } -extern char *optarg; -extern int optind, opterr, optopt; static char optstr[] = "ecpmfs:P:v"; #define min(a, b) ((a) < (b) ? (a) : (b)) -- 1.7.12.4
[PATCH v2 0/3] modetest: Allow selecting a driver manually
Hello, Here's the second version of a small patch set that adds an argument to the modetest command line to specify the driver name instead of using the builtin list of drivers. This is particularly handy during development of new DRM/KMS drivers. Changes from v1: - Use the module name specified on the command line instead of a random module name (I'll blame a bad copy & paste). Laurent Pinchart (3): modetest: Remove extern declarations of opt(arg|ind|err|opt) modetest: Sort command line arguments modetest: Add a command line parameter to select the driver tests/modetest/modetest.c | 88 --- 1 file changed, 53 insertions(+), 35 deletions(-) -- Regards, Laurent Pinchart
[PATCH] drm/fb-helper: don't disable everything in initial_config
On Sun, Jan 27, 2013 at 11:41 AM, Daniel Vetter
wrote:
> This should be done in the drivers for two reasons:
> - it gets in the way of fastboot efforts
> - it links the fb helpers with the crtc helpers instead of going
> through the real interface vfuncs, forcing i915 to fake all the
> ->disable callbacks used by the crtc helper to avoid ugly Oopsen
>
> v2: Resolve conflicts since drivers still call
> drm_fb_helper_single_add_all_connectors.
>
Reviewed-by: Rob Clark
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/ast/ast_fb.c |5 +
> drivers/gpu/drm/cirrus/cirrus_fbdev.c |4
> drivers/gpu/drm/drm_fb_cma_helper.c |3 +++
> drivers/gpu/drm/drm_fb_helper.c |3 ---
> drivers/gpu/drm/exynos/exynos_drm_fbdev.c |3 +++
> drivers/gpu/drm/gma500/framebuffer.c |4
> drivers/gpu/drm/i915/intel_fb.c |3 +++
> drivers/gpu/drm/mgag200/mgag200_fb.c |5 +
> drivers/gpu/drm/nouveau/nouveau_fbcon.c |3 +++
> drivers/gpu/drm/radeon/radeon_fb.c|4
> drivers/gpu/drm/udl/udl_fb.c |4
> drivers/staging/omapdrm/omap_fbdev.c |4
> 12 files changed, 42 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c
> index 3e6584b..81763ca 100644
> --- a/drivers/gpu/drm/ast/ast_fb.c
> +++ b/drivers/gpu/drm/ast/ast_fb.c
> @@ -40,6 +40,7 @@
> #include
> #include
> #include
> +#include
> #include "ast_drv.h"
>
> static void ast_dirty_update(struct ast_fbdev *afbdev,
> @@ -314,6 +315,10 @@ int ast_fbdev_init(struct drm_device *dev)
> }
>
> drm_fb_helper_single_add_all_connectors(&afbdev->helper);
> +
> + /* disable all the possible outputs/crtcs before entering KMS mode */
> + drm_helper_disable_unused_functions(dev);
> +
> drm_fb_helper_initial_config(&afbdev->helper, 32);
> return 0;
> }
> diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> index 3daea0f..b96605c 100644
> --- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> +++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> @@ -11,6 +11,7 @@
> #include
> #include
> #include
> +#include
>
> #include
>
> @@ -291,6 +292,9 @@ int cirrus_fbdev_init(struct cirrus_device *cdev)
> return ret;
> }
> drm_fb_helper_single_add_all_connectors(&gfbdev->helper);
> +
> + /* disable all the possible outputs/crtcs before entering KMS mode */
> + drm_helper_disable_unused_functions(cdev->dev);
> drm_fb_helper_initial_config(&gfbdev->helper, bpp_sel);
>
> return 0;
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> b/drivers/gpu/drm/drm_fb_cma_helper.c
> index 1b6ba2d..ef3d33a 100644
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -333,6 +333,9 @@ struct drm_fbdev_cma *drm_fbdev_cma_init(struct
> drm_device *dev,
>
> }
>
> + /* disable all the possible outputs/crtcs before entering KMS mode */
> + drm_helper_disable_unused_functions(dev);
> +
> ret = drm_fb_helper_initial_config(helper, preferred_bpp);
> if (ret < 0) {
> dev_err(dev->dev, "Failed to set inital hw configuration.\n");
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 3132d8a..f188344 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1357,9 +1357,6 @@ bool drm_fb_helper_initial_config(struct drm_fb_helper
> *fb_helper, int bpp_sel)
> struct drm_device *dev = fb_helper->dev;
> int count = 0;
>
> - /* disable all the possible outputs/crtcs before entering KMS mode */
> - drm_helper_disable_unused_functions(fb_helper->dev);
> -
> drm_fb_helper_parse_command_line(fb_helper);
>
> count = drm_fb_helper_probe_connector_modes(fb_helper,
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> index 086d0f7..fe2a0f0 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> @@ -295,6 +295,9 @@ int exynos_drm_fbdev_init(struct drm_device *dev)
>
> }
>
> + /* disable all the possible outputs/crtcs before entering KMS mode */
> + drm_helper_disable_unused_functions(dev);
> +
> ret = drm_fb_helper_initial_config(helper, PREFERRED_BPP);
> if (ret < 0) {
> DRM_ERROR("failed to set up hw configuration.\n");
> diff --git a/drivers/gpu/drm/gma500/framebuffer.c
> b/drivers/gpu/drm/gma500/framebuffer.c
> index c1ef37e..fee3bf8 100644
> --- a/drivers/gpu/drm/gma500/framebuffer.c
> +++ b/drivers/gpu/drm/gma500/framebuffer.c
> @@ -616,6 +616,10 @@ int psb_fbdev_init(struct drm_device *dev)
> INTELFB_CONN_LIMIT);
>
> drm_fb_helper_single_add_all_connectors(
[PATCH 13/16] drm/fb-helper: streamline drm_fb_helper_single_fb_probe
On Mon, Feb 11, 2013 at 7:33 PM, Daniel Vetter wrote: > On Tue, Feb 12, 2013 at 1:24 AM, Rob Clark wrote: >> On Thu, Jan 24, 2013 at 10:20 AM, Daniel Vetter >> wrote: >>> No need to check whether we've allocated a new fb since we're not >>> always doing that. Also, we always need to register the fbdev and add >>> it to the panic notifier. >> >> hmm, how is this not leading to a register_framebuffer() on every hotplug >> event? > > drm_fb_helper_hotplug_event calls drm_fb_helper_set_par directly, so > bypasses all the initial setup which I'm streamlining here. See > "drm/fb-helper: directly call set_par from the hotplug handler". doh, that's the problem with reviewing long patchset without applying them as I go.. ok, then looks good Reviewed-by: Rob Clark > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH] drm/fb-helper: fixup set_config semantics
On Mon, Feb 11, 2013 at 7:25 PM, Daniel Vetter
wrote:
> While doing the modeset rework for drm/i915 I've noticed that the fb
> helper is very liberal with the semantics of the ->set_config
> interface:
> - It doesn't bother clearing stale modes (e.g. when unplugging a
> screen).
> - It unconditionally sets the fb, even if no mode will be set on a
> given crtc.
> - The initial setup is a bit fun since we need to pick crtcs to decide
> the desired fb size, but also should set the modeset->fb pointer.
> Explain what's going on in the fixup code after the fb is allocated.
>
> The crtc helper didn't really care, but the new i915 modeset
> infrastructure did, so I've had to add a bunch of special-cases to
> catch this.
>
> Fix this all up and enforce the interface by converting the checks in
> drm/i915/intel_display.c to BUG_ONs.
>
> v2: Fix commit message spell fail spotted by Rob Clark.
Reviewed-by: Rob Clark
>
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_fb_helper.c | 27 +++
> drivers/gpu/drm/i915/intel_display.c | 11 +++
> 2 files changed, 26 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index d841b68..809ef99 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -689,7 +689,6 @@ int drm_fb_helper_set_par(struct fb_info *info)
> struct drm_fb_helper *fb_helper = info->par;
> struct drm_device *dev = fb_helper->dev;
> struct fb_var_screeninfo *var = &info->var;
> - struct drm_crtc *crtc;
> int ret;
> int i;
>
> @@ -700,7 +699,6 @@ int drm_fb_helper_set_par(struct fb_info *info)
>
> drm_modeset_lock_all(dev);
> for (i = 0; i < fb_helper->crtc_count; i++) {
> - crtc = fb_helper->crtc_info[i].mode_set.crtc;
> ret =
> drm_mode_set_config_internal(&fb_helper->crtc_info[i].mode_set);
> if (ret) {
> drm_modeset_unlock_all(dev);
> @@ -841,9 +839,17 @@ static int drm_fb_helper_single_fb_probe(struct
> drm_fb_helper *fb_helper,
>
> info = fb_helper->fbdev;
>
> - /* set the fb pointer */
> + /*
> +* Set the fb pointer - usually drm_setup_crtcs does this for hotplug
> +* events, but at init time drm_setup_crtcs needs to be called before
> +* the fb is allocated (since we need to figure out the desired size
> of
> +* the fb before we can allocate it ...). Hence we need to fix things
> up
> +* here again.
> +*/
> for (i = 0; i < fb_helper->crtc_count; i++)
> - fb_helper->crtc_info[i].mode_set.fb = fb_helper->fb;
> + if (fb_helper->crtc_info[i].mode_set.num_connectors)
> + fb_helper->crtc_info[i].mode_set.fb = fb_helper->fb;
> +
>
> if (new_fb) {
> info->var.pixclock = 0;
> @@ -1314,6 +1320,7 @@ static void drm_setup_crtcs(struct drm_fb_helper
> *fb_helper)
> for (i = 0; i < fb_helper->crtc_count; i++) {
> modeset = &fb_helper->crtc_info[i].mode_set;
> modeset->num_connectors = 0;
> + modeset->fb = NULL;
> }
>
> for (i = 0; i < fb_helper->connector_count; i++) {
> @@ -1330,9 +1337,21 @@ static void drm_setup_crtcs(struct drm_fb_helper
> *fb_helper)
> modeset->mode = drm_mode_duplicate(dev,
>
> fb_crtc->desired_mode);
> modeset->connectors[modeset->num_connectors++] =
> fb_helper->connector_info[i]->connector;
> + modeset->fb = fb_helper->fb;
> }
> }
>
> + /* Clear out any old modes if there are no more connected outputs. */
> + for (i = 0; i < fb_helper->crtc_count; i++) {
> + modeset = &fb_helper->crtc_info[i].mode_set;
> + if (modeset->num_connectors == 0) {
> + BUG_ON(modeset->fb);
> + BUG_ON(modeset->num_connectors);
> + if (modeset->mode)
> + drm_mode_destroy(dev, modeset->mode);
> + modeset->mode = NULL;
> + }
> + }
> out:
> kfree(crtcs);
> kfree(modes);
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 24f2654..ca8d592 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7978,14 +7978,9 @@ static int intel_crtc_set_config(struct drm_mode_set
> *set)
> BUG_ON(!set->crtc);
> BUG_ON(!set->crtc->helper_private);
>
> - if (!set->mode)
> - set->fb = NULL;
> -
> - /* The fb helper likes to play gross jokes with ->mode_set_config.
> -* Unfortunately the crtc helper doesn't do much at all for this case,
>
[PATCH 5/5] drm/tegra: Implement page-flipping support
On Sat, Feb 2, 2013 at 12:05 AM, Laurent Pinchart wrote: >> Back to the original topic: should it not work to queue a page flip and >> immediately exit(0) after that to test the cleanup code? I suppose if >> that can be tested on all drivers we should have pretty good coverage. > > Maybe we need some kind of compliance test suite tool ? kms_flip from our intel-specific testsuite is probably the most paranoid thing for testing page flips out there: http://cgit.freedesktop.org/xorg/app/intel-gpu-tools/tree/tests/kms_flip.c Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 13/16] drm/fb-helper: streamline drm_fb_helper_single_fb_probe
On Thu, Jan 24, 2013 at 10:20 AM, Daniel Vetter
wrote:
> No need to check whether we've allocated a new fb since we're not
> always doing that. Also, we always need to register the fbdev and add
> it to the panic notifier.
hmm, how is this not leading to a register_framebuffer() on every hotplug event?
BR,
-R
>
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_fb_helper.c | 29 +++--
> 1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 90c1117..edbfcde 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -752,10 +752,14 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo
> *var,
> }
> EXPORT_SYMBOL(drm_fb_helper_pan_display);
>
> +/*
> + * Allocates the backing storage through the ->fb_probe callback and then
> + * registers the fbdev and sets up the panic notifier.
> + */
> static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
> int preferred_bpp)
> {
> - int new_fb = 0;
> + int ret = 0;
> int crtc_count = 0;
> int i;
> struct fb_info *info;
> @@ -833,9 +837,9 @@ static int drm_fb_helper_single_fb_probe(struct
> drm_fb_helper *fb_helper,
> }
>
> /* push down into drivers */
> - new_fb = (*fb_helper->funcs->fb_probe)(fb_helper, &sizes);
> - if (new_fb < 0)
> - return new_fb;
> + ret = (*fb_helper->funcs->fb_probe)(fb_helper, &sizes);
> + if (ret < 0)
> + return ret;
>
> info = fb_helper->fbdev;
>
> @@ -851,15 +855,12 @@ static int drm_fb_helper_single_fb_probe(struct
> drm_fb_helper *fb_helper,
> fb_helper->crtc_info[i].mode_set.fb = fb_helper->fb;
>
>
> - if (new_fb) {
> - info->var.pixclock = 0;
> - if (register_framebuffer(info) < 0)
> - return -EINVAL;
> -
> - dev_info(fb_helper->dev->dev, "fb%d: %s frame buffer
> device\n",
> - info->node, info->fix.id);
> + info->var.pixclock = 0;
> + if (register_framebuffer(info) < 0)
> + return -EINVAL;
>
> - }
> + dev_info(fb_helper->dev->dev, "fb%d: %s frame buffer device\n",
> + info->node, info->fix.id);
>
> /* Switch back to kernel console on panic */
> /* multi card linked list maybe */
> @@ -869,8 +870,8 @@ static int drm_fb_helper_single_fb_probe(struct
> drm_fb_helper *fb_helper,
>&paniced);
> register_sysrq_key('v', &sysrq_drm_fb_helper_restore_op);
> }
> - if (new_fb)
> - list_add(&fb_helper->kernel_fb_list, &kernel_fb_helper_list);
> +
> + list_add(&fb_helper->kernel_fb_list, &kernel_fb_helper_list);
>
> return 0;
> }
> --
> 1.7.10.4
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 12/16] drm/fb-helper: directly call set_par from the hotplug handler
On Thu, Jan 24, 2013 at 10:20 AM, Daniel Vetter
wrote:
> The idea behind calling down into the driver's ->fb_probe function on each
> hotplug seems to be able to reallocate the backing storage (if e.g. a screen
> with higher resolution gets added). But that requires quite a bit of work in
> the
> fb helper itself, since currently we limit new screens to the currently
> allocated fb. An no kms driver supports fbdev fb resizing.
does an non kms fbdev driver support resizing? That might be a better
argument ;-)
w/ framebuffer vaddr all over the place, teaching fbdev to resize
seems like it might be kinda fun.. I guess if we really wanted to
resize fbdev then we'd be better off beating all the console_lock vs
notifier vs whatever else deadlocks so that we could reliably
unregister and re-register a framebuffer on last-unplug / first-plug.
Reviewed-by: Rob Clark
> So don't bother and start to simplify the code by calling
> drm_fb_helper_set_par
> directly from the fbdev hotplug function, since that's the only thing left in
> drm_fb_helper_single_fb_probe which does not concern itself with fb allocation
> and initial setup. Follow-on patches will streamline the initial setup
> code.
>
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_fb_helper.c |6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 5c73a12..90c1117 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -859,8 +859,6 @@ static int drm_fb_helper_single_fb_probe(struct
> drm_fb_helper *fb_helper,
> dev_info(fb_helper->dev->dev, "fb%d: %s frame buffer
> device\n",
> info->node, info->fix.id);
>
> - } else {
> - drm_fb_helper_set_par(info);
> }
>
> /* Switch back to kernel console on panic */
> @@ -1436,7 +1434,9 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper
> *fb_helper)
> drm_setup_crtcs(fb_helper);
> drm_modeset_unlock_all(dev);
>
> - return drm_fb_helper_single_fb_probe(fb_helper, bpp_sel);
> + drm_fb_helper_set_par(fb_helper->fbdev);
> +
> + return 0;
> }
> EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
>
> --
> 1.7.10.4
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 44341] Radeon HD6990M: HDMI audio output works now! Kernel gives new warning
https://bugzilla.kernel.org/show_bug.cgi?id=44341 Christopher Frömmel changed: What|Removed |Added Kernel Version|3.7.5 |3.7.7 --- Comment #10 from Christopher Frömmel 2013-02-12 01:42:56 --- 3.7.7 still gives similar warning with radeon.audio=1, but works fine. -- Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 11/16] drm/fb-helper: fixup up set_config semantics
On Thu, Jan 24, 2013 at 10:20 AM, Daniel Vetter
wrote:
> While doing the modeset rework for drm/i915 I've noticed that the fb
> helper is very liberal with the semantics of the ->set_config
> interface:
> - It doesn't bother clearing stale modes (e.g. when unpluggint a
s/unpluggint/unplugging/
> screen).
> - It unconditionally sets and the fb, even if no mode will be set on a
s/and the fb/the fb/ ?
> given crtc.
> - The initial setup is a bit fun since we need to pick crtcs to decide
> the desired fb size, but also should set the modeset->fb pointer.
> Explain what's going on in the fixup code after the fb is allocated.
>
> The crtc helper didn't really care, but the new i915 modeset
> infrastructure did, so I've had to add a bunch of special-cases to
> catch this.
>
> Fix this all up and enforce the interface by converting the checks in
> drm/i915/intel_display.c to BUG_ONs.
>
maybe nitpicking, but how about a matching set of BUG_ONs in crtc
helper, so someone messing with this code with a driver that still
uses crtc helper would catch the same issues?
Anyways, basically seems reasonable.
But thanks for making me look at code that so far I've mostly tried to
avoid looking at :-P
It does seem like the initial-config code could somehow be simpler than it is..
Anyways, while looking at that, it occurs to me that we should remove
mode, and saved_fb from 'struct drm_fb_helper'
BR,
-R
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_fb_helper.c | 27 +++
> drivers/gpu/drm/i915/intel_display.c | 11 +++
> 2 files changed, 26 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index dbf0020..5c73a12 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -689,7 +689,6 @@ int drm_fb_helper_set_par(struct fb_info *info)
> struct drm_fb_helper *fb_helper = info->par;
> struct drm_device *dev = fb_helper->dev;
> struct fb_var_screeninfo *var = &info->var;
> - struct drm_crtc *crtc;
> int ret;
> int i;
>
> @@ -700,7 +699,6 @@ int drm_fb_helper_set_par(struct fb_info *info)
>
> drm_modeset_lock_all(dev);
> for (i = 0; i < fb_helper->crtc_count; i++) {
> - crtc = fb_helper->crtc_info[i].mode_set.crtc;
> ret =
> drm_mode_set_config_internal(&fb_helper->crtc_info[i].mode_set);
> if (ret) {
> drm_modeset_unlock_all(dev);
> @@ -841,9 +839,17 @@ static int drm_fb_helper_single_fb_probe(struct
> drm_fb_helper *fb_helper,
>
> info = fb_helper->fbdev;
>
> - /* set the fb pointer */
> + /*
> +* Set the fb pointer - usually drm_setup_crtcs does this for hotplug
> +* events, but at init time drm_setup_crtcs needs to be called before
> +* the fb is allocated (since we need to figure out the desired size
> of
> +* the fb before we can allocate it ...). Hence we need to fix things
> up
> +* here again.
> +*/
> for (i = 0; i < fb_helper->crtc_count; i++)
> - fb_helper->crtc_info[i].mode_set.fb = fb_helper->fb;
> + if (fb_helper->crtc_info[i].mode_set.num_connectors)
> + fb_helper->crtc_info[i].mode_set.fb = fb_helper->fb;
> +
>
> if (new_fb) {
> info->var.pixclock = 0;
> @@ -1314,6 +1320,7 @@ static void drm_setup_crtcs(struct drm_fb_helper
> *fb_helper)
> for (i = 0; i < fb_helper->crtc_count; i++) {
> modeset = &fb_helper->crtc_info[i].mode_set;
> modeset->num_connectors = 0;
> + modeset->fb = NULL;
> }
>
> for (i = 0; i < fb_helper->connector_count; i++) {
> @@ -1330,9 +1337,21 @@ static void drm_setup_crtcs(struct drm_fb_helper
> *fb_helper)
> modeset->mode = drm_mode_duplicate(dev,
>
> fb_crtc->desired_mode);
> modeset->connectors[modeset->num_connectors++] =
> fb_helper->connector_info[i]->connector;
> + modeset->fb = fb_helper->fb;
> }
> }
>
> + /* Clear out any old modes if there are no more connected outputs. */
> + for (i = 0; i < fb_helper->crtc_count; i++) {
> + modeset = &fb_helper->crtc_info[i].mode_set;
> + if (modeset->num_connectors == 0) {
> + BUG_ON(modeset->fb);
> + BUG_ON(modeset->num_connectors);
> + if (modeset->mode)
> + drm_mode_destroy(dev, modeset->mode);
> + modeset->mode = NULL;
> + }
> + }
> out:
> kfree(crtcs);
> kfree(modes);
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 055b24a..7e3
[Bug 60200] radeon_bo with virtual address referencing mismatch
https://bugs.freedesktop.org/show_bug.cgi?id=60200 Laurent carlier changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #3 from Laurent carlier --- http://cgit.freedesktop.org/mesa/mesa/commit/?id=a37835c8eda017f0c955e0927e7418e7f3ba3b73 -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/fb-helper: don't disable everything in initial_config
On Sun, Jan 27, 2013 at 11:41 AM, Daniel Vetter wrote:
> This should be done in the drivers for two reasons:
> - it gets in the way of fastboot efforts
> - it links the fb helpers with the crtc helpers instead of going
> through the real interface vfuncs, forcing i915 to fake all the
> ->disable callbacks used by the crtc helper to avoid ugly Oopsen
>
> v2: Resolve conflicts since drivers still call
> drm_fb_helper_single_add_all_connectors.
>
Reviewed-by: Rob Clark
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/ast/ast_fb.c |5 +
> drivers/gpu/drm/cirrus/cirrus_fbdev.c |4
> drivers/gpu/drm/drm_fb_cma_helper.c |3 +++
> drivers/gpu/drm/drm_fb_helper.c |3 ---
> drivers/gpu/drm/exynos/exynos_drm_fbdev.c |3 +++
> drivers/gpu/drm/gma500/framebuffer.c |4
> drivers/gpu/drm/i915/intel_fb.c |3 +++
> drivers/gpu/drm/mgag200/mgag200_fb.c |5 +
> drivers/gpu/drm/nouveau/nouveau_fbcon.c |3 +++
> drivers/gpu/drm/radeon/radeon_fb.c|4
> drivers/gpu/drm/udl/udl_fb.c |4
> drivers/staging/omapdrm/omap_fbdev.c |4
> 12 files changed, 42 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c
> index 3e6584b..81763ca 100644
> --- a/drivers/gpu/drm/ast/ast_fb.c
> +++ b/drivers/gpu/drm/ast/ast_fb.c
> @@ -40,6 +40,7 @@
> #include
> #include
> #include
> +#include
> #include "ast_drv.h"
>
> static void ast_dirty_update(struct ast_fbdev *afbdev,
> @@ -314,6 +315,10 @@ int ast_fbdev_init(struct drm_device *dev)
> }
>
> drm_fb_helper_single_add_all_connectors(&afbdev->helper);
> +
> + /* disable all the possible outputs/crtcs before entering KMS mode */
> + drm_helper_disable_unused_functions(dev);
> +
> drm_fb_helper_initial_config(&afbdev->helper, 32);
> return 0;
> }
> diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> index 3daea0f..b96605c 100644
> --- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> +++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> @@ -11,6 +11,7 @@
> #include
> #include
> #include
> +#include
>
> #include
>
> @@ -291,6 +292,9 @@ int cirrus_fbdev_init(struct cirrus_device *cdev)
> return ret;
> }
> drm_fb_helper_single_add_all_connectors(&gfbdev->helper);
> +
> + /* disable all the possible outputs/crtcs before entering KMS mode */
> + drm_helper_disable_unused_functions(cdev->dev);
> drm_fb_helper_initial_config(&gfbdev->helper, bpp_sel);
>
> return 0;
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> b/drivers/gpu/drm/drm_fb_cma_helper.c
> index 1b6ba2d..ef3d33a 100644
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -333,6 +333,9 @@ struct drm_fbdev_cma *drm_fbdev_cma_init(struct
> drm_device *dev,
>
> }
>
> + /* disable all the possible outputs/crtcs before entering KMS mode */
> + drm_helper_disable_unused_functions(dev);
> +
> ret = drm_fb_helper_initial_config(helper, preferred_bpp);
> if (ret < 0) {
> dev_err(dev->dev, "Failed to set inital hw configuration.\n");
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 3132d8a..f188344 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1357,9 +1357,6 @@ bool drm_fb_helper_initial_config(struct drm_fb_helper
> *fb_helper, int bpp_sel)
> struct drm_device *dev = fb_helper->dev;
> int count = 0;
>
> - /* disable all the possible outputs/crtcs before entering KMS mode */
> - drm_helper_disable_unused_functions(fb_helper->dev);
> -
> drm_fb_helper_parse_command_line(fb_helper);
>
> count = drm_fb_helper_probe_connector_modes(fb_helper,
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> index 086d0f7..fe2a0f0 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> @@ -295,6 +295,9 @@ int exynos_drm_fbdev_init(struct drm_device *dev)
>
> }
>
> + /* disable all the possible outputs/crtcs before entering KMS mode */
> + drm_helper_disable_unused_functions(dev);
> +
> ret = drm_fb_helper_initial_config(helper, PREFERRED_BPP);
> if (ret < 0) {
> DRM_ERROR("failed to set up hw configuration.\n");
> diff --git a/drivers/gpu/drm/gma500/framebuffer.c
> b/drivers/gpu/drm/gma500/framebuffer.c
> index c1ef37e..fee3bf8 100644
> --- a/drivers/gpu/drm/gma500/framebuffer.c
> +++ b/drivers/gpu/drm/gma500/framebuffer.c
> @@ -616,6 +616,10 @@ int psb_fbdev_init(struct drm_device *dev)
> INTELFB_CONN_LIMIT);
>
> drm_fb_helper_single_add_all_connectors(&
Re: [PATCH 13/16] drm/fb-helper: streamline drm_fb_helper_single_fb_probe
On Mon, Feb 11, 2013 at 7:33 PM, Daniel Vetter wrote: > On Tue, Feb 12, 2013 at 1:24 AM, Rob Clark wrote: >> On Thu, Jan 24, 2013 at 10:20 AM, Daniel Vetter >> wrote: >>> No need to check whether we've allocated a new fb since we're not >>> always doing that. Also, we always need to register the fbdev and add >>> it to the panic notifier. >> >> hmm, how is this not leading to a register_framebuffer() on every hotplug >> event? > > drm_fb_helper_hotplug_event calls drm_fb_helper_set_par directly, so > bypasses all the initial setup which I'm streamlining here. See > "drm/fb-helper: directly call set_par from the hotplug handler". doh, that's the problem with reviewing long patchset without applying them as I go.. ok, then looks good Reviewed-by: Rob Clark > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/fb-helper: fixup set_config semantics
On Mon, Feb 11, 2013 at 7:25 PM, Daniel Vetter wrote:
> While doing the modeset rework for drm/i915 I've noticed that the fb
> helper is very liberal with the semantics of the ->set_config
> interface:
> - It doesn't bother clearing stale modes (e.g. when unplugging a
> screen).
> - It unconditionally sets the fb, even if no mode will be set on a
> given crtc.
> - The initial setup is a bit fun since we need to pick crtcs to decide
> the desired fb size, but also should set the modeset->fb pointer.
> Explain what's going on in the fixup code after the fb is allocated.
>
> The crtc helper didn't really care, but the new i915 modeset
> infrastructure did, so I've had to add a bunch of special-cases to
> catch this.
>
> Fix this all up and enforce the interface by converting the checks in
> drm/i915/intel_display.c to BUG_ONs.
>
> v2: Fix commit message spell fail spotted by Rob Clark.
Reviewed-by: Rob Clark
>
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_fb_helper.c | 27 +++
> drivers/gpu/drm/i915/intel_display.c | 11 +++
> 2 files changed, 26 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index d841b68..809ef99 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -689,7 +689,6 @@ int drm_fb_helper_set_par(struct fb_info *info)
> struct drm_fb_helper *fb_helper = info->par;
> struct drm_device *dev = fb_helper->dev;
> struct fb_var_screeninfo *var = &info->var;
> - struct drm_crtc *crtc;
> int ret;
> int i;
>
> @@ -700,7 +699,6 @@ int drm_fb_helper_set_par(struct fb_info *info)
>
> drm_modeset_lock_all(dev);
> for (i = 0; i < fb_helper->crtc_count; i++) {
> - crtc = fb_helper->crtc_info[i].mode_set.crtc;
> ret =
> drm_mode_set_config_internal(&fb_helper->crtc_info[i].mode_set);
> if (ret) {
> drm_modeset_unlock_all(dev);
> @@ -841,9 +839,17 @@ static int drm_fb_helper_single_fb_probe(struct
> drm_fb_helper *fb_helper,
>
> info = fb_helper->fbdev;
>
> - /* set the fb pointer */
> + /*
> +* Set the fb pointer - usually drm_setup_crtcs does this for hotplug
> +* events, but at init time drm_setup_crtcs needs to be called before
> +* the fb is allocated (since we need to figure out the desired size
> of
> +* the fb before we can allocate it ...). Hence we need to fix things
> up
> +* here again.
> +*/
> for (i = 0; i < fb_helper->crtc_count; i++)
> - fb_helper->crtc_info[i].mode_set.fb = fb_helper->fb;
> + if (fb_helper->crtc_info[i].mode_set.num_connectors)
> + fb_helper->crtc_info[i].mode_set.fb = fb_helper->fb;
> +
>
> if (new_fb) {
> info->var.pixclock = 0;
> @@ -1314,6 +1320,7 @@ static void drm_setup_crtcs(struct drm_fb_helper
> *fb_helper)
> for (i = 0; i < fb_helper->crtc_count; i++) {
> modeset = &fb_helper->crtc_info[i].mode_set;
> modeset->num_connectors = 0;
> + modeset->fb = NULL;
> }
>
> for (i = 0; i < fb_helper->connector_count; i++) {
> @@ -1330,9 +1337,21 @@ static void drm_setup_crtcs(struct drm_fb_helper
> *fb_helper)
> modeset->mode = drm_mode_duplicate(dev,
>
> fb_crtc->desired_mode);
> modeset->connectors[modeset->num_connectors++] =
> fb_helper->connector_info[i]->connector;
> + modeset->fb = fb_helper->fb;
> }
> }
>
> + /* Clear out any old modes if there are no more connected outputs. */
> + for (i = 0; i < fb_helper->crtc_count; i++) {
> + modeset = &fb_helper->crtc_info[i].mode_set;
> + if (modeset->num_connectors == 0) {
> + BUG_ON(modeset->fb);
> + BUG_ON(modeset->num_connectors);
> + if (modeset->mode)
> + drm_mode_destroy(dev, modeset->mode);
> + modeset->mode = NULL;
> + }
> + }
> out:
> kfree(crtcs);
> kfree(modes);
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 24f2654..ca8d592 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7978,14 +7978,9 @@ static int intel_crtc_set_config(struct drm_mode_set
> *set)
> BUG_ON(!set->crtc);
> BUG_ON(!set->crtc->helper_private);
>
> - if (!set->mode)
> - set->fb = NULL;
> -
> - /* The fb helper likes to play gross jokes with ->mode_set_config.
> -* Unfortunately the crtc helper doesn't do much at all for this case,
>
Re: [PATCH 13/16] drm/fb-helper: streamline drm_fb_helper_single_fb_probe
On Tue, Feb 12, 2013 at 1:24 AM, Rob Clark wrote: > On Thu, Jan 24, 2013 at 10:20 AM, Daniel Vetter > wrote: >> No need to check whether we've allocated a new fb since we're not >> always doing that. Also, we always need to register the fbdev and add >> it to the panic notifier. > > hmm, how is this not leading to a register_framebuffer() on every hotplug > event? drm_fb_helper_hotplug_event calls drm_fb_helper_set_par directly, so bypasses all the initial setup which I'm streamlining here. See "drm/fb-helper: directly call set_par from the hotplug handler". -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/fb-helper: fixup set_config semantics
While doing the modeset rework for drm/i915 I've noticed that the fb
helper is very liberal with the semantics of the ->set_config
interface:
- It doesn't bother clearing stale modes (e.g. when unplugging a
screen).
- It unconditionally sets the fb, even if no mode will be set on a
given crtc.
- The initial setup is a bit fun since we need to pick crtcs to decide
the desired fb size, but also should set the modeset->fb pointer.
Explain what's going on in the fixup code after the fb is allocated.
The crtc helper didn't really care, but the new i915 modeset
infrastructure did, so I've had to add a bunch of special-cases to
catch this.
Fix this all up and enforce the interface by converting the checks in
drm/i915/intel_display.c to BUG_ONs.
v2: Fix commit message spell fail spotted by Rob Clark.
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_fb_helper.c | 27 +++
drivers/gpu/drm/i915/intel_display.c | 11 +++
2 files changed, 26 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index d841b68..809ef99 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -689,7 +689,6 @@ int drm_fb_helper_set_par(struct fb_info *info)
struct drm_fb_helper *fb_helper = info->par;
struct drm_device *dev = fb_helper->dev;
struct fb_var_screeninfo *var = &info->var;
- struct drm_crtc *crtc;
int ret;
int i;
@@ -700,7 +699,6 @@ int drm_fb_helper_set_par(struct fb_info *info)
drm_modeset_lock_all(dev);
for (i = 0; i < fb_helper->crtc_count; i++) {
- crtc = fb_helper->crtc_info[i].mode_set.crtc;
ret =
drm_mode_set_config_internal(&fb_helper->crtc_info[i].mode_set);
if (ret) {
drm_modeset_unlock_all(dev);
@@ -841,9 +839,17 @@ static int drm_fb_helper_single_fb_probe(struct
drm_fb_helper *fb_helper,
info = fb_helper->fbdev;
- /* set the fb pointer */
+ /*
+* Set the fb pointer - usually drm_setup_crtcs does this for hotplug
+* events, but at init time drm_setup_crtcs needs to be called before
+* the fb is allocated (since we need to figure out the desired size of
+* the fb before we can allocate it ...). Hence we need to fix things up
+* here again.
+*/
for (i = 0; i < fb_helper->crtc_count; i++)
- fb_helper->crtc_info[i].mode_set.fb = fb_helper->fb;
+ if (fb_helper->crtc_info[i].mode_set.num_connectors)
+ fb_helper->crtc_info[i].mode_set.fb = fb_helper->fb;
+
if (new_fb) {
info->var.pixclock = 0;
@@ -1314,6 +1320,7 @@ static void drm_setup_crtcs(struct drm_fb_helper
*fb_helper)
for (i = 0; i < fb_helper->crtc_count; i++) {
modeset = &fb_helper->crtc_info[i].mode_set;
modeset->num_connectors = 0;
+ modeset->fb = NULL;
}
for (i = 0; i < fb_helper->connector_count; i++) {
@@ -1330,9 +1337,21 @@ static void drm_setup_crtcs(struct drm_fb_helper
*fb_helper)
modeset->mode = drm_mode_duplicate(dev,
fb_crtc->desired_mode);
modeset->connectors[modeset->num_connectors++] =
fb_helper->connector_info[i]->connector;
+ modeset->fb = fb_helper->fb;
}
}
+ /* Clear out any old modes if there are no more connected outputs. */
+ for (i = 0; i < fb_helper->crtc_count; i++) {
+ modeset = &fb_helper->crtc_info[i].mode_set;
+ if (modeset->num_connectors == 0) {
+ BUG_ON(modeset->fb);
+ BUG_ON(modeset->num_connectors);
+ if (modeset->mode)
+ drm_mode_destroy(dev, modeset->mode);
+ modeset->mode = NULL;
+ }
+ }
out:
kfree(crtcs);
kfree(modes);
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 24f2654..ca8d592 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7978,14 +7978,9 @@ static int intel_crtc_set_config(struct drm_mode_set
*set)
BUG_ON(!set->crtc);
BUG_ON(!set->crtc->helper_private);
- if (!set->mode)
- set->fb = NULL;
-
- /* The fb helper likes to play gross jokes with ->mode_set_config.
-* Unfortunately the crtc helper doesn't do much at all for this case,
-* so we have to cope with this madness until the fb helper is fixed
up. */
- if (set->fb && set->num_connectors == 0)
- return 0;
+ /* Enforce sane interface api - has been abused by the fb helper. */
+ BUG_ON(!set->mode && set->fb);
+ BUG_ON(set->fb &&
[PATCH] drm/fb-helper: remove unused members of struct drm_fb_helper
Spotted by Rob Clark.
Signed-off-by: Daniel Vetter
---
include/drm/drm_fb_helper.h |2 --
1 file changed, 2 deletions(-)
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index a60311c..8ef4c63 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -77,9 +77,7 @@ struct drm_fb_helper_connector {
struct drm_fb_helper {
struct drm_framebuffer *fb;
- struct drm_framebuffer *saved_fb;
struct drm_device *dev;
- struct drm_display_mode *mode;
int crtc_count;
struct drm_fb_helper_crtc *crtc_info;
int connector_count;
--
1.7.10.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 13/16] drm/fb-helper: streamline drm_fb_helper_single_fb_probe
On Thu, Jan 24, 2013 at 10:20 AM, Daniel Vetter wrote:
> No need to check whether we've allocated a new fb since we're not
> always doing that. Also, we always need to register the fbdev and add
> it to the panic notifier.
hmm, how is this not leading to a register_framebuffer() on every hotplug event?
BR,
-R
>
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_fb_helper.c | 29 +++--
> 1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 90c1117..edbfcde 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -752,10 +752,14 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo
> *var,
> }
> EXPORT_SYMBOL(drm_fb_helper_pan_display);
>
> +/*
> + * Allocates the backing storage through the ->fb_probe callback and then
> + * registers the fbdev and sets up the panic notifier.
> + */
> static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
> int preferred_bpp)
> {
> - int new_fb = 0;
> + int ret = 0;
> int crtc_count = 0;
> int i;
> struct fb_info *info;
> @@ -833,9 +837,9 @@ static int drm_fb_helper_single_fb_probe(struct
> drm_fb_helper *fb_helper,
> }
>
> /* push down into drivers */
> - new_fb = (*fb_helper->funcs->fb_probe)(fb_helper, &sizes);
> - if (new_fb < 0)
> - return new_fb;
> + ret = (*fb_helper->funcs->fb_probe)(fb_helper, &sizes);
> + if (ret < 0)
> + return ret;
>
> info = fb_helper->fbdev;
>
> @@ -851,15 +855,12 @@ static int drm_fb_helper_single_fb_probe(struct
> drm_fb_helper *fb_helper,
> fb_helper->crtc_info[i].mode_set.fb = fb_helper->fb;
>
>
> - if (new_fb) {
> - info->var.pixclock = 0;
> - if (register_framebuffer(info) < 0)
> - return -EINVAL;
> -
> - dev_info(fb_helper->dev->dev, "fb%d: %s frame buffer
> device\n",
> - info->node, info->fix.id);
> + info->var.pixclock = 0;
> + if (register_framebuffer(info) < 0)
> + return -EINVAL;
>
> - }
> + dev_info(fb_helper->dev->dev, "fb%d: %s frame buffer device\n",
> + info->node, info->fix.id);
>
> /* Switch back to kernel console on panic */
> /* multi card linked list maybe */
> @@ -869,8 +870,8 @@ static int drm_fb_helper_single_fb_probe(struct
> drm_fb_helper *fb_helper,
>&paniced);
> register_sysrq_key('v', &sysrq_drm_fb_helper_restore_op);
> }
> - if (new_fb)
> - list_add(&fb_helper->kernel_fb_list, &kernel_fb_helper_list);
> +
> + list_add(&fb_helper->kernel_fb_list, &kernel_fb_helper_list);
>
> return 0;
> }
> --
> 1.7.10.4
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/fb-helper: fixup set_config semantics
While doing the modeset rework for drm/i915 I've noticed that the fb
helper is very liberal with the semantics of the ->set_config
interface:
- It doesn't bother clearing stale modes (e.g. when unplugging a
screen).
- It unconditionally sets the fb, even if no mode will be set on a
given crtc.
- The initial setup is a bit fun since we need to pick crtcs to decide
the desired fb size, but also should set the modeset->fb pointer.
Explain what's going on in the fixup code after the fb is allocated.
The crtc helper didn't really care, but the new i915 modeset
infrastructure did, so I've had to add a bunch of special-cases to
catch this.
Fix this all up and enforce the interface by converting the checks in
drm/i915/intel_display.c to BUG_ONs.
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_fb_helper.c | 27 +++
drivers/gpu/drm/i915/intel_display.c | 11 +++
2 files changed, 26 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index d841b68..809ef99 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -689,7 +689,6 @@ int drm_fb_helper_set_par(struct fb_info *info)
struct drm_fb_helper *fb_helper = info->par;
struct drm_device *dev = fb_helper->dev;
struct fb_var_screeninfo *var = &info->var;
- struct drm_crtc *crtc;
int ret;
int i;
@@ -700,7 +699,6 @@ int drm_fb_helper_set_par(struct fb_info *info)
drm_modeset_lock_all(dev);
for (i = 0; i < fb_helper->crtc_count; i++) {
- crtc = fb_helper->crtc_info[i].mode_set.crtc;
ret =
drm_mode_set_config_internal(&fb_helper->crtc_info[i].mode_set);
if (ret) {
drm_modeset_unlock_all(dev);
@@ -841,9 +839,17 @@ static int drm_fb_helper_single_fb_probe(struct
drm_fb_helper *fb_helper,
info = fb_helper->fbdev;
- /* set the fb pointer */
+ /*
+* Set the fb pointer - usually drm_setup_crtcs does this for hotplug
+* events, but at init time drm_setup_crtcs needs to be called before
+* the fb is allocated (since we need to figure out the desired size of
+* the fb before we can allocate it ...). Hence we need to fix things up
+* here again.
+*/
for (i = 0; i < fb_helper->crtc_count; i++)
- fb_helper->crtc_info[i].mode_set.fb = fb_helper->fb;
+ if (fb_helper->crtc_info[i].mode_set.num_connectors)
+ fb_helper->crtc_info[i].mode_set.fb = fb_helper->fb;
+
if (new_fb) {
info->var.pixclock = 0;
@@ -1314,6 +1320,7 @@ static void drm_setup_crtcs(struct drm_fb_helper
*fb_helper)
for (i = 0; i < fb_helper->crtc_count; i++) {
modeset = &fb_helper->crtc_info[i].mode_set;
modeset->num_connectors = 0;
+ modeset->fb = NULL;
}
for (i = 0; i < fb_helper->connector_count; i++) {
@@ -1330,9 +1337,21 @@ static void drm_setup_crtcs(struct drm_fb_helper
*fb_helper)
modeset->mode = drm_mode_duplicate(dev,
fb_crtc->desired_mode);
modeset->connectors[modeset->num_connectors++] =
fb_helper->connector_info[i]->connector;
+ modeset->fb = fb_helper->fb;
}
}
+ /* Clear out any old modes if there are no more connected outputs. */
+ for (i = 0; i < fb_helper->crtc_count; i++) {
+ modeset = &fb_helper->crtc_info[i].mode_set;
+ if (modeset->num_connectors == 0) {
+ BUG_ON(modeset->fb);
+ BUG_ON(modeset->num_connectors);
+ if (modeset->mode)
+ drm_mode_destroy(dev, modeset->mode);
+ modeset->mode = NULL;
+ }
+ }
out:
kfree(crtcs);
kfree(modes);
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 24f2654..ca8d592 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7978,14 +7978,9 @@ static int intel_crtc_set_config(struct drm_mode_set
*set)
BUG_ON(!set->crtc);
BUG_ON(!set->crtc->helper_private);
- if (!set->mode)
- set->fb = NULL;
-
- /* The fb helper likes to play gross jokes with ->mode_set_config.
-* Unfortunately the crtc helper doesn't do much at all for this case,
-* so we have to cope with this madness until the fb helper is fixed
up. */
- if (set->fb && set->num_connectors == 0)
- return 0;
+ /* Enforce sane interface api - has been abused by the fb helper. */
+ BUG_ON(!set->mode && set->fb);
+ BUG_ON(set->fb && set->num_connectors == 0);
if (set->fb) {
[PATCH] drm/radeon: enforce use of radeon_get_ib_value when reading user cmd
On Mon, Feb 11, 2013 at 8:57 AM, wrote:
> From: Jerome Glisse
>
> When ever parsing cmd buffer supplied by userspace we need to use
> radeon_get_ib_value rather than directly accessing the ib as the user
> cmd might not yet be copied into the ib thus the parser might read
> value that does not correspond to what user is sending and possibly
> allowing user to send malicious command undected.
>
> Signed-off-by: Jerome Glisse
Reviewed-by: Alex Deucher
> ---
> drivers/gpu/drm/radeon/evergreen_cs.c | 86
> +--
> drivers/gpu/drm/radeon/r600_cs.c | 38
> 2 files changed, 62 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/evergreen_cs.c
> b/drivers/gpu/drm/radeon/evergreen_cs.c
> index 7a44566..ee4cff5 100644
> --- a/drivers/gpu/drm/radeon/evergreen_cs.c
> +++ b/drivers/gpu/drm/radeon/evergreen_cs.c
> @@ -2909,14 +2909,14 @@ int evergreen_dma_cs_parse(struct radeon_cs_parser *p)
> return -EINVAL;
> }
> if (tiled) {
> - dst_offset = ib[idx+1];
> + dst_offset = radeon_get_ib_value(p, idx+1);
> dst_offset <<= 8;
>
> ib[idx+1] += (u32)(dst_reloc->lobj.gpu_offset
> >> 8);
> p->idx += count + 7;
> } else {
> - dst_offset = ib[idx+1];
> - dst_offset |= ((u64)(ib[idx+2] & 0xff)) << 32;
> + dst_offset = radeon_get_ib_value(p, idx+1);
> + dst_offset |= ((u64)(radeon_get_ib_value(p,
> idx+2) & 0xff)) << 32;
>
> ib[idx+1] += (u32)(dst_reloc->lobj.gpu_offset
> & 0xfffc);
> ib[idx+2] +=
> upper_32_bits(dst_reloc->lobj.gpu_offset) & 0xff;
> @@ -2954,12 +2954,12 @@ int evergreen_dma_cs_parse(struct radeon_cs_parser *p)
> DRM_ERROR("bad L2T,
> frame to fields DMA_PACKET_COPY\n");
> return -EINVAL;
> }
> - dst_offset = ib[idx+1];
> + dst_offset =
> radeon_get_ib_value(p, idx+1);
> dst_offset <<= 8;
> - dst2_offset = ib[idx+2];
> + dst2_offset =
> radeon_get_ib_value(p, idx+2);
> dst2_offset <<= 8;
> - src_offset = ib[idx+8];
> - src_offset |=
> ((u64)(ib[idx+9] & 0xff)) << 32;
> + src_offset =
> radeon_get_ib_value(p, idx+8);
> + src_offset |=
> ((u64)(radeon_get_ib_value(p, idx+9) & 0xff)) << 32;
> if ((src_offset + (count *
> 4)) > radeon_bo_size(src_reloc->robj)) {
> dev_warn(p->dev, "DMA
> L2T, frame to fields src buffer too small (%llu %lu)\n",
> src_offset +
> (count * 4), radeon_bo_size(src_reloc->robj));
> @@ -3014,12 +3014,12 @@ int evergreen_dma_cs_parse(struct radeon_cs_parser *p)
> DRM_ERROR("bad L2T,
> broadcast DMA_PACKET_COPY\n");
> return -EINVAL;
> }
> - dst_offset = ib[idx+1];
> + dst_offset =
> radeon_get_ib_value(p, idx+1);
> dst_offset <<= 8;
> - dst2_offset = ib[idx+2];
> + dst2_offset =
> radeon_get_ib_value(p, idx+2);
> dst2_offset <<= 8;
> - src_offset = ib[idx+8];
> - src_offset |=
> ((u64)(ib[idx+9] & 0xff)) << 32;
> + src_offset =
> radeon_get_ib_value(p, idx+8);
> + src_offset |=
> ((u64)(radeon_get_ib_value(p, idx+9) & 0xff)) << 32;
> if ((src_offset + (count *
> 4)) > radeon_bo_size(src_reloc->robj)) {
> dev_warn(p->dev, "DMA
> L2T, broadcast src buffer too small
Re: [PATCH 12/16] drm/fb-helper: directly call set_par from the hotplug handler
On Thu, Jan 24, 2013 at 10:20 AM, Daniel Vetter wrote:
> The idea behind calling down into the driver's ->fb_probe function on each
> hotplug seems to be able to reallocate the backing storage (if e.g. a screen
> with higher resolution gets added). But that requires quite a bit of work in
> the
> fb helper itself, since currently we limit new screens to the currently
> allocated fb. An no kms driver supports fbdev fb resizing.
does an non kms fbdev driver support resizing? That might be a better
argument ;-)
w/ framebuffer vaddr all over the place, teaching fbdev to resize
seems like it might be kinda fun.. I guess if we really wanted to
resize fbdev then we'd be better off beating all the console_lock vs
notifier vs whatever else deadlocks so that we could reliably
unregister and re-register a framebuffer on last-unplug / first-plug.
Reviewed-by: Rob Clark
> So don't bother and start to simplify the code by calling
> drm_fb_helper_set_par
> directly from the fbdev hotplug function, since that's the only thing left in
> drm_fb_helper_single_fb_probe which does not concern itself with fb allocation
> and initial setup. Follow-on patches will streamline the initial setup
> code.
>
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_fb_helper.c |6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 5c73a12..90c1117 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -859,8 +859,6 @@ static int drm_fb_helper_single_fb_probe(struct
> drm_fb_helper *fb_helper,
> dev_info(fb_helper->dev->dev, "fb%d: %s frame buffer
> device\n",
> info->node, info->fix.id);
>
> - } else {
> - drm_fb_helper_set_par(info);
> }
>
> /* Switch back to kernel console on panic */
> @@ -1436,7 +1434,9 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper
> *fb_helper)
> drm_setup_crtcs(fb_helper);
> drm_modeset_unlock_all(dev);
>
> - return drm_fb_helper_single_fb_probe(fb_helper, bpp_sel);
> + drm_fb_helper_set_par(fb_helper->fbdev);
> +
> + return 0;
> }
> EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
>
> --
> 1.7.10.4
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 07/16] drm/fb-helper: unexport drm_fb_helper_single_fb_probe
On Thu, Jan 24, 2013 at 10:20 AM, Daniel Vetter
wrote:
> Not called by anyone, and really, shouldn't be. Drivers are supposed
> either drm_fb_helper_initial_config or drm_fb_helper_hotplug_event.
> Originally this was done differently, but is now consolidated in the
> helper functions and no longer done by drivers directly.
>
> Signed-off-by: Daniel Vetter
Reviewed-by: Rob Clark
> ---
> drivers/gpu/drm/drm_fb_helper.c |5 ++---
> include/drm/drm_fb_helper.h |3 ---
> 2 files changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 4549512..2377fef 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -754,8 +754,8 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo
> *var,
> }
> EXPORT_SYMBOL(drm_fb_helper_pan_display);
>
> -int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
> - int preferred_bpp)
> +static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
> +int preferred_bpp)
> {
> int new_fb = 0;
> int crtc_count = 0;
> @@ -870,7 +870,6 @@ int drm_fb_helper_single_fb_probe(struct drm_fb_helper
> *fb_helper,
>
> return 0;
> }
> -EXPORT_SYMBOL(drm_fb_helper_single_fb_probe);
>
> void drm_fb_helper_fill_fix(struct fb_info *info, uint32_t pitch,
> uint32_t depth)
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 4e989dc..62f8848 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -82,9 +82,6 @@ struct drm_fb_helper {
> bool delayed_hotplug;
> };
>
> -int drm_fb_helper_single_fb_probe(struct drm_fb_helper *helper,
> - int preferred_bpp);
> -
> int drm_fb_helper_init(struct drm_device *dev,
>struct drm_fb_helper *helper, int crtc_count,
>int max_conn);
> --
> 1.7.10.4
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 05/16] drm/fb-helper: unexport drm_fb_helper_panic
On Thu, Jan 24, 2013 at 10:20 AM, Daniel Vetter
wrote:
> It doesn't even show up in any header files and only used iternally.
> Originally it was (ab)used to restore the fbcon on lastclose, but that
> died with
>
> commit e8e7a2b8ccfdae0d4cb6bd25824bbedcd42da316
> Author: Dave Airlie
> Date: Thu Apr 21 22:18:32 2011 +0100
>
> drm/i915: restore only the mode of this driver on lastclose (v2)
>
> Signed-off-by: Daniel Vetter
Reviewed-by: Rob Clark
> ---
> drivers/gpu/drm/drm_fb_helper.c |3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 6d689f2..ce816a5 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -284,7 +284,7 @@ static bool drm_fb_helper_force_kernel_mode(void)
> return error;
> }
>
> -int drm_fb_helper_panic(struct notifier_block *n, unsigned long ununsed,
> +static int drm_fb_helper_panic(struct notifier_block *n, unsigned long
> ununsed,
> void *panic_str)
> {
> /*
> @@ -297,7 +297,6 @@ int drm_fb_helper_panic(struct notifier_block *n,
> unsigned long ununsed,
> pr_err("panic occurred, switching back to text console\n");
> return drm_fb_helper_force_kernel_mode();
> }
> -EXPORT_SYMBOL(drm_fb_helper_panic);
>
> static struct notifier_block paniced = {
> .notifier_call = drm_fb_helper_panic,
> --
> 1.7.10.4
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 03/16] drm: review locking for drm_fb_helper_restore_fbdev_mode
On Thu, Jan 24, 2013 at 10:20 AM, Daniel Vetter
wrote:
> ... it's required. Fix up exynos and the cma helper, and add a
> corresponding WARN_ON to drm_fb_helper_restore_fbdev_mode.
>
> Note that tegra calls the fbdev cma helper restore function also from
> it's driver-load callback. Which is a bit against current practice,
> since usually the call is only from ->lastclose, and initial setup is
> done by drm_fb_helper_initial_config.
>
> Also add the relevant drm DocBook entry.
>
> Signed-off-by: Daniel Vetter
With the actual addition of the promised WARN_ON,
Reviewed-by: Rob Clark
> ---
> drivers/gpu/drm/drm_fb_cma_helper.c |2 ++
> drivers/gpu/drm/drm_fb_helper.c |8
> drivers/gpu/drm/exynos/exynos_drm_fbdev.c |2 ++
> 3 files changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> b/drivers/gpu/drm/drm_fb_cma_helper.c
> index 3742bc9..1b6ba2d 100644
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -389,8 +389,10 @@ EXPORT_SYMBOL_GPL(drm_fbdev_cma_fini);
> */
> void drm_fbdev_cma_restore_mode(struct drm_fbdev_cma *fbdev_cma)
> {
> + drm_modeset_lock_all(dev);
> if (fbdev_cma)
> drm_fb_helper_restore_fbdev_mode(&fbdev_cma->fb_helper);
> + drm_modeset_unlock_all(dev);
> }
> EXPORT_SYMBOL_GPL(drm_fbdev_cma_restore_mode);
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 0c6e25e..0439cb0 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -239,6 +239,14 @@ int drm_fb_helper_debug_leave(struct fb_info *info)
> }
> EXPORT_SYMBOL(drm_fb_helper_debug_leave);
>
> +/**
> + * drm_fb_helper_restore_fbdev_mode - restore fbdev configuration
> + * @fb_helper: fbcon to restore
> + *
> + * This should be called from driver's drm->lastclose callback when
> implementing
> + * an fbcon on top of kms using this helper. This ensures that the user isn't
> + * greeted with a black screen when e.g. X dies.
> + */
> bool drm_fb_helper_restore_fbdev_mode(struct drm_fb_helper *fb_helper)
> {
> bool error = false;
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> index 90d335c..086d0f7 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> @@ -376,5 +376,7 @@ void exynos_drm_fbdev_restore_mode(struct drm_device *dev)
> if (!private || !private->fb_helper)
> return;
>
> + drm_modeset_lock_all(dev);
> drm_fb_helper_restore_fbdev_mode(private->fb_helper);
> + drm_modeset_unlock_all(dev);
> }
> --
> 1.7.10.4
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 04/16] drm/fb-helper: kill drm_fb_helper_restore
On Thu, Jan 24, 2013 at 10:20 AM, Daniel Vetter
wrote:
> It's only used internally for the sysrq and panic handlers provided by
> the drm fb helper implementation. Hence just inline it, kill the
> export and remove the confusing kerneldoc. Driver's are supposed to
> call drm_fb_helper_restore_fbdev_mode on lastclose.
>
> Note that locking is totally fubar - the sysrq case doesn't take any
> locks at all. The panic handler probably shouldn't take any locks
> since it'll only make things worse. Otoh it's probably better to
> switch things over to the atomic modeset callbacks (and disable the
> panic handler for those drivers which don't implement it).
>
> But that's both better done in separate patches.
>
> Signed-off-by: Daniel Vetter
Reviewed-by: Rob Clark
> ---
> drivers/gpu/drm/drm_fb_helper.c | 23 ---
> include/drm/drm_fb_helper.h |1 -
> 2 files changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 0439cb0..6d689f2 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -261,6 +261,10 @@ bool drm_fb_helper_restore_fbdev_mode(struct
> drm_fb_helper *fb_helper)
> }
> EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode);
>
> +/*
> + * restore fbcon display for all kms driver's using this helper, used for
> sysrq
> + * and panic handling.
> + */
> static bool drm_fb_helper_force_kernel_mode(void)
> {
> bool ret, error = false;
> @@ -299,20 +303,6 @@ static struct notifier_block paniced = {
> .notifier_call = drm_fb_helper_panic,
> };
>
> -/**
> - * drm_fb_helper_restore - restore the framebuffer console (kernel) config
> - *
> - * Restore's the kernel's fbcon mode, used for lastclose & panic paths.
> - */
> -void drm_fb_helper_restore(void)
> -{
> - bool ret;
> - ret = drm_fb_helper_force_kernel_mode();
> - if (ret == true)
> - DRM_ERROR("Failed to restore crtc configuration\n");
> -}
> -EXPORT_SYMBOL(drm_fb_helper_restore);
> -
> static bool drm_fb_helper_is_bound(struct drm_fb_helper *fb_helper)
> {
> struct drm_device *dev = fb_helper->dev;
> @@ -334,7 +324,10 @@ static bool drm_fb_helper_is_bound(struct drm_fb_helper
> *fb_helper)
> #ifdef CONFIG_MAGIC_SYSRQ
> static void drm_fb_helper_restore_work_fn(struct work_struct *ignored)
> {
> - drm_fb_helper_restore();
> + bool ret;
> + ret = drm_fb_helper_force_kernel_mode();
> + if (ret == true)
> + DRM_ERROR("Failed to restore crtc configuration\n");
> }
> static DECLARE_WORK(drm_fb_helper_restore_work,
> drm_fb_helper_restore_work_fn);
>
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 5120b01..ba32505 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -103,7 +103,6 @@ int drm_fb_helper_setcolreg(unsigned regno,
> struct fb_info *info);
>
> bool drm_fb_helper_restore_fbdev_mode(struct drm_fb_helper *fb_helper);
> -void drm_fb_helper_restore(void);
> void drm_fb_helper_fill_var(struct fb_info *info, struct drm_fb_helper
> *fb_helper,
> uint32_t fb_width, uint32_t fb_height);
> void drm_fb_helper_fill_fix(struct fb_info *info, uint32_t pitch,
> --
> 1.7.10.4
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 11/16] drm/fb-helper: fixup up set_config semantics
On Thu, Jan 24, 2013 at 10:20 AM, Daniel Vetter wrote:
> While doing the modeset rework for drm/i915 I've noticed that the fb
> helper is very liberal with the semantics of the ->set_config
> interface:
> - It doesn't bother clearing stale modes (e.g. when unpluggint a
s/unpluggint/unplugging/
> screen).
> - It unconditionally sets and the fb, even if no mode will be set on a
s/and the fb/the fb/ ?
> given crtc.
> - The initial setup is a bit fun since we need to pick crtcs to decide
> the desired fb size, but also should set the modeset->fb pointer.
> Explain what's going on in the fixup code after the fb is allocated.
>
> The crtc helper didn't really care, but the new i915 modeset
> infrastructure did, so I've had to add a bunch of special-cases to
> catch this.
>
> Fix this all up and enforce the interface by converting the checks in
> drm/i915/intel_display.c to BUG_ONs.
>
maybe nitpicking, but how about a matching set of BUG_ONs in crtc
helper, so someone messing with this code with a driver that still
uses crtc helper would catch the same issues?
Anyways, basically seems reasonable.
But thanks for making me look at code that so far I've mostly tried to
avoid looking at :-P
It does seem like the initial-config code could somehow be simpler than it is..
Anyways, while looking at that, it occurs to me that we should remove
mode, and saved_fb from 'struct drm_fb_helper'
BR,
-R
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_fb_helper.c | 27 +++
> drivers/gpu/drm/i915/intel_display.c | 11 +++
> 2 files changed, 26 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index dbf0020..5c73a12 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -689,7 +689,6 @@ int drm_fb_helper_set_par(struct fb_info *info)
> struct drm_fb_helper *fb_helper = info->par;
> struct drm_device *dev = fb_helper->dev;
> struct fb_var_screeninfo *var = &info->var;
> - struct drm_crtc *crtc;
> int ret;
> int i;
>
> @@ -700,7 +699,6 @@ int drm_fb_helper_set_par(struct fb_info *info)
>
> drm_modeset_lock_all(dev);
> for (i = 0; i < fb_helper->crtc_count; i++) {
> - crtc = fb_helper->crtc_info[i].mode_set.crtc;
> ret =
> drm_mode_set_config_internal(&fb_helper->crtc_info[i].mode_set);
> if (ret) {
> drm_modeset_unlock_all(dev);
> @@ -841,9 +839,17 @@ static int drm_fb_helper_single_fb_probe(struct
> drm_fb_helper *fb_helper,
>
> info = fb_helper->fbdev;
>
> - /* set the fb pointer */
> + /*
> +* Set the fb pointer - usually drm_setup_crtcs does this for hotplug
> +* events, but at init time drm_setup_crtcs needs to be called before
> +* the fb is allocated (since we need to figure out the desired size
> of
> +* the fb before we can allocate it ...). Hence we need to fix things
> up
> +* here again.
> +*/
> for (i = 0; i < fb_helper->crtc_count; i++)
> - fb_helper->crtc_info[i].mode_set.fb = fb_helper->fb;
> + if (fb_helper->crtc_info[i].mode_set.num_connectors)
> + fb_helper->crtc_info[i].mode_set.fb = fb_helper->fb;
> +
>
> if (new_fb) {
> info->var.pixclock = 0;
> @@ -1314,6 +1320,7 @@ static void drm_setup_crtcs(struct drm_fb_helper
> *fb_helper)
> for (i = 0; i < fb_helper->crtc_count; i++) {
> modeset = &fb_helper->crtc_info[i].mode_set;
> modeset->num_connectors = 0;
> + modeset->fb = NULL;
> }
>
> for (i = 0; i < fb_helper->connector_count; i++) {
> @@ -1330,9 +1337,21 @@ static void drm_setup_crtcs(struct drm_fb_helper
> *fb_helper)
> modeset->mode = drm_mode_duplicate(dev,
>
> fb_crtc->desired_mode);
> modeset->connectors[modeset->num_connectors++] =
> fb_helper->connector_info[i]->connector;
> + modeset->fb = fb_helper->fb;
> }
> }
>
> + /* Clear out any old modes if there are no more connected outputs. */
> + for (i = 0; i < fb_helper->crtc_count; i++) {
> + modeset = &fb_helper->crtc_info[i].mode_set;
> + if (modeset->num_connectors == 0) {
> + BUG_ON(modeset->fb);
> + BUG_ON(modeset->num_connectors);
> + if (modeset->mode)
> + drm_mode_destroy(dev, modeset->mode);
> + modeset->mode = NULL;
> + }
> + }
> out:
> kfree(crtcs);
> kfree(modes);
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 055b24a..7e3d
[PATCH linux-next] drm/radeon: Avoid NULL pointer dereference from atom_index_iio() allocation failure
Smatch anlysis:
drivers/gpu/drm/radeon/atom.c:1242 atom_index_iio() error: potential null
dereference 'ctx->iio'. (kzalloc returns null)
Also cleaned up some checks before calls to kfree(). kfree(NULL) is OK.
Cc: David Airlie
Cc: Alex Deucher
Cc: "Michel D?nzer"
Cc: Dave Airlie
Cc: "Christian K?nig"
Cc: Jerome Glisse
Cc: dri-devel at lists.freedesktop.org
Signed-off-by: Tim Gardner
---
drivers/gpu/drm/radeon/atom.c |9 +++--
drivers/gpu/drm/radeon/radeon_device.c |9 -
2 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/radeon/atom.c b/drivers/gpu/drm/radeon/atom.c
index 5ce9bf5..46a9c37 100644
--- a/drivers/gpu/drm/radeon/atom.c
+++ b/drivers/gpu/drm/radeon/atom.c
@@ -1238,6 +1238,8 @@ static int atom_iio_len[] = { 1, 2, 3, 3, 3, 3, 4, 4, 4,
3 };
static void atom_index_iio(struct atom_context *ctx, int base)
{
ctx->iio = kzalloc(2 * 256, GFP_KERNEL);
+ if (!ctx->iio)
+ return;
while (CU8(base) == ATOM_IIO_START) {
ctx->iio[CU8(base + 1)] = base + 2;
base += 2;
@@ -1287,6 +1289,10 @@ struct atom_context *atom_parse(struct card_info *card,
void *bios)
ctx->cmd_table = CU16(base + ATOM_ROM_CMD_PTR);
ctx->data_table = CU16(base + ATOM_ROM_DATA_PTR);
atom_index_iio(ctx, CU16(ctx->data_table + ATOM_DATA_IIO_PTR) + 4);
+ if (!ctx->iio) {
+ atom_destroy(ctx);
+ return NULL;
+ }
str = CSTR(CU16(base + ATOM_ROM_MSG_PTR));
while (*str && ((*str == '\n') || (*str == '\r')))
@@ -1335,8 +1341,7 @@ int atom_asic_init(struct atom_context *ctx)
void atom_destroy(struct atom_context *ctx)
{
- if (ctx->iio)
- kfree(ctx->iio);
+ kfree(ctx->iio);
kfree(ctx);
}
diff --git a/drivers/gpu/drm/radeon/radeon_device.c
b/drivers/gpu/drm/radeon/radeon_device.c
index 8794de1..44b8034 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -759,6 +759,11 @@ int radeon_atombios_init(struct radeon_device *rdev)
atom_card_info->pll_write = cail_pll_write;
rdev->mode_info.atom_context = atom_parse(atom_card_info, rdev->bios);
+ if (!rdev->mode_info.atom_context) {
+ radeon_atombios_fini(rdev);
+ return -ENOMEM;
+ }
+
mutex_init(&rdev->mode_info.atom_context->mutex);
radeon_atom_initialize_bios_scratch_regs(rdev->ddev);
atom_allocate_fb_scratch(rdev->mode_info.atom_context);
@@ -778,9 +783,11 @@ void radeon_atombios_fini(struct radeon_device *rdev)
{
if (rdev->mode_info.atom_context) {
kfree(rdev->mode_info.atom_context->scratch);
- kfree(rdev->mode_info.atom_context);
}
+ kfree(rdev->mode_info.atom_context);
+ rdev->mode_info.atom_context = NULL;
kfree(rdev->mode_info.atom_card_info);
+ rdev->mode_info.atom_card_info = NULL;
}
/* COMBIOS */
--
1.7.9.5
Re: [GMA500] Valid VCO frequency range on GMA500 platform?
On Mon, Feb 11, 2013 at 2:15 PM, "David Müller (ELSOFT AG)" wrote: > Hi Patrik > > Patrik Jakobsson wrote: >> The value of VCO_MIN comes from the Intel PRM for similar GPUs. >> >> Instead of changing VCO_MIN, could you try setting N_MIN=1, N_MAX=6 and >> M1_MAX=22? I'll test it on my own hardware tomorrow. > > Thanks for your suggestion. > > With "N_MIN=1, N_MAX=6 and M1_MAX=22", i get a refresh rate of 59.9Hz. > > > Dave > Those values come from the i915 driver and should work fine for us as well. I had no problems at my end so I'll submit a patch for this. -Patrik ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[GMA500] Valid VCO frequency range on GMA500 platform?
Hi Patrik Patrik Jakobsson wrote: > The value of VCO_MIN comes from the Intel PRM for similar GPUs. > > Instead of changing VCO_MIN, could you try setting N_MIN=1, N_MAX=6 and > M1_MAX=22? I'll test it on my own hardware tomorrow. Thanks for your suggestion. With "N_MIN=1, N_MAX=6 and M1_MAX=22", i get a refresh rate of 59.9Hz. Dave
Re: [PATCH 07/16] drm/fb-helper: unexport drm_fb_helper_single_fb_probe
On Thu, Jan 24, 2013 at 10:20 AM, Daniel Vetter wrote:
> Not called by anyone, and really, shouldn't be. Drivers are supposed
> either drm_fb_helper_initial_config or drm_fb_helper_hotplug_event.
> Originally this was done differently, but is now consolidated in the
> helper functions and no longer done by drivers directly.
>
> Signed-off-by: Daniel Vetter
Reviewed-by: Rob Clark
> ---
> drivers/gpu/drm/drm_fb_helper.c |5 ++---
> include/drm/drm_fb_helper.h |3 ---
> 2 files changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 4549512..2377fef 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -754,8 +754,8 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo
> *var,
> }
> EXPORT_SYMBOL(drm_fb_helper_pan_display);
>
> -int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
> - int preferred_bpp)
> +static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
> +int preferred_bpp)
> {
> int new_fb = 0;
> int crtc_count = 0;
> @@ -870,7 +870,6 @@ int drm_fb_helper_single_fb_probe(struct drm_fb_helper
> *fb_helper,
>
> return 0;
> }
> -EXPORT_SYMBOL(drm_fb_helper_single_fb_probe);
>
> void drm_fb_helper_fill_fix(struct fb_info *info, uint32_t pitch,
> uint32_t depth)
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 4e989dc..62f8848 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -82,9 +82,6 @@ struct drm_fb_helper {
> bool delayed_hotplug;
> };
>
> -int drm_fb_helper_single_fb_probe(struct drm_fb_helper *helper,
> - int preferred_bpp);
> -
> int drm_fb_helper_init(struct drm_device *dev,
>struct drm_fb_helper *helper, int crtc_count,
>int max_conn);
> --
> 1.7.10.4
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 53391] nouveau: wrong display output order
https://bugzilla.kernel.org/show_bug.cgi?id=53391 --- Comment #9 from Marcin Slusarz 2013-02-11 22:12:35 --- Yes, please do it. I think it will fix all issues (both VGA and DVI setups will work and output numbering will be correct). -- Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 05/16] drm/fb-helper: unexport drm_fb_helper_panic
On Thu, Jan 24, 2013 at 10:20 AM, Daniel Vetter wrote:
> It doesn't even show up in any header files and only used iternally.
> Originally it was (ab)used to restore the fbcon on lastclose, but that
> died with
>
> commit e8e7a2b8ccfdae0d4cb6bd25824bbedcd42da316
> Author: Dave Airlie
> Date: Thu Apr 21 22:18:32 2011 +0100
>
> drm/i915: restore only the mode of this driver on lastclose (v2)
>
> Signed-off-by: Daniel Vetter
Reviewed-by: Rob Clark
> ---
> drivers/gpu/drm/drm_fb_helper.c |3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 6d689f2..ce816a5 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -284,7 +284,7 @@ static bool drm_fb_helper_force_kernel_mode(void)
> return error;
> }
>
> -int drm_fb_helper_panic(struct notifier_block *n, unsigned long ununsed,
> +static int drm_fb_helper_panic(struct notifier_block *n, unsigned long
> ununsed,
> void *panic_str)
> {
> /*
> @@ -297,7 +297,6 @@ int drm_fb_helper_panic(struct notifier_block *n,
> unsigned long ununsed,
> pr_err("panic occurred, switching back to text console\n");
> return drm_fb_helper_force_kernel_mode();
> }
> -EXPORT_SYMBOL(drm_fb_helper_panic);
>
> static struct notifier_block paniced = {
> .notifier_call = drm_fb_helper_panic,
> --
> 1.7.10.4
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 03/16] drm: review locking for drm_fb_helper_restore_fbdev_mode
On Thu, Jan 24, 2013 at 10:20 AM, Daniel Vetter wrote:
> ... it's required. Fix up exynos and the cma helper, and add a
> corresponding WARN_ON to drm_fb_helper_restore_fbdev_mode.
>
> Note that tegra calls the fbdev cma helper restore function also from
> it's driver-load callback. Which is a bit against current practice,
> since usually the call is only from ->lastclose, and initial setup is
> done by drm_fb_helper_initial_config.
>
> Also add the relevant drm DocBook entry.
>
> Signed-off-by: Daniel Vetter
With the actual addition of the promised WARN_ON,
Reviewed-by: Rob Clark
> ---
> drivers/gpu/drm/drm_fb_cma_helper.c |2 ++
> drivers/gpu/drm/drm_fb_helper.c |8
> drivers/gpu/drm/exynos/exynos_drm_fbdev.c |2 ++
> 3 files changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> b/drivers/gpu/drm/drm_fb_cma_helper.c
> index 3742bc9..1b6ba2d 100644
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -389,8 +389,10 @@ EXPORT_SYMBOL_GPL(drm_fbdev_cma_fini);
> */
> void drm_fbdev_cma_restore_mode(struct drm_fbdev_cma *fbdev_cma)
> {
> + drm_modeset_lock_all(dev);
> if (fbdev_cma)
> drm_fb_helper_restore_fbdev_mode(&fbdev_cma->fb_helper);
> + drm_modeset_unlock_all(dev);
> }
> EXPORT_SYMBOL_GPL(drm_fbdev_cma_restore_mode);
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 0c6e25e..0439cb0 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -239,6 +239,14 @@ int drm_fb_helper_debug_leave(struct fb_info *info)
> }
> EXPORT_SYMBOL(drm_fb_helper_debug_leave);
>
> +/**
> + * drm_fb_helper_restore_fbdev_mode - restore fbdev configuration
> + * @fb_helper: fbcon to restore
> + *
> + * This should be called from driver's drm->lastclose callback when
> implementing
> + * an fbcon on top of kms using this helper. This ensures that the user isn't
> + * greeted with a black screen when e.g. X dies.
> + */
> bool drm_fb_helper_restore_fbdev_mode(struct drm_fb_helper *fb_helper)
> {
> bool error = false;
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> index 90d335c..086d0f7 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> @@ -376,5 +376,7 @@ void exynos_drm_fbdev_restore_mode(struct drm_device *dev)
> if (!private || !private->fb_helper)
> return;
>
> + drm_modeset_lock_all(dev);
> drm_fb_helper_restore_fbdev_mode(private->fb_helper);
> + drm_modeset_unlock_all(dev);
> }
> --
> 1.7.10.4
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/16] drm/fb-helper: kill drm_fb_helper_restore
On Thu, Jan 24, 2013 at 10:20 AM, Daniel Vetter wrote:
> It's only used internally for the sysrq and panic handlers provided by
> the drm fb helper implementation. Hence just inline it, kill the
> export and remove the confusing kerneldoc. Driver's are supposed to
> call drm_fb_helper_restore_fbdev_mode on lastclose.
>
> Note that locking is totally fubar - the sysrq case doesn't take any
> locks at all. The panic handler probably shouldn't take any locks
> since it'll only make things worse. Otoh it's probably better to
> switch things over to the atomic modeset callbacks (and disable the
> panic handler for those drivers which don't implement it).
>
> But that's both better done in separate patches.
>
> Signed-off-by: Daniel Vetter
Reviewed-by: Rob Clark
> ---
> drivers/gpu/drm/drm_fb_helper.c | 23 ---
> include/drm/drm_fb_helper.h |1 -
> 2 files changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 0439cb0..6d689f2 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -261,6 +261,10 @@ bool drm_fb_helper_restore_fbdev_mode(struct
> drm_fb_helper *fb_helper)
> }
> EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode);
>
> +/*
> + * restore fbcon display for all kms driver's using this helper, used for
> sysrq
> + * and panic handling.
> + */
> static bool drm_fb_helper_force_kernel_mode(void)
> {
> bool ret, error = false;
> @@ -299,20 +303,6 @@ static struct notifier_block paniced = {
> .notifier_call = drm_fb_helper_panic,
> };
>
> -/**
> - * drm_fb_helper_restore - restore the framebuffer console (kernel) config
> - *
> - * Restore's the kernel's fbcon mode, used for lastclose & panic paths.
> - */
> -void drm_fb_helper_restore(void)
> -{
> - bool ret;
> - ret = drm_fb_helper_force_kernel_mode();
> - if (ret == true)
> - DRM_ERROR("Failed to restore crtc configuration\n");
> -}
> -EXPORT_SYMBOL(drm_fb_helper_restore);
> -
> static bool drm_fb_helper_is_bound(struct drm_fb_helper *fb_helper)
> {
> struct drm_device *dev = fb_helper->dev;
> @@ -334,7 +324,10 @@ static bool drm_fb_helper_is_bound(struct drm_fb_helper
> *fb_helper)
> #ifdef CONFIG_MAGIC_SYSRQ
> static void drm_fb_helper_restore_work_fn(struct work_struct *ignored)
> {
> - drm_fb_helper_restore();
> + bool ret;
> + ret = drm_fb_helper_force_kernel_mode();
> + if (ret == true)
> + DRM_ERROR("Failed to restore crtc configuration\n");
> }
> static DECLARE_WORK(drm_fb_helper_restore_work,
> drm_fb_helper_restore_work_fn);
>
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 5120b01..ba32505 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -103,7 +103,6 @@ int drm_fb_helper_setcolreg(unsigned regno,
> struct fb_info *info);
>
> bool drm_fb_helper_restore_fbdev_mode(struct drm_fb_helper *fb_helper);
> -void drm_fb_helper_restore(void);
> void drm_fb_helper_fill_var(struct fb_info *info, struct drm_fb_helper
> *fb_helper,
> uint32_t fb_width, uint32_t fb_height);
> void drm_fb_helper_fill_fix(struct fb_info *info, uint32_t pitch,
> --
> 1.7.10.4
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 49981] On HD6850, Power Profile doesn't change if 2 screen is attached.
https://bugs.freedesktop.org/show_bug.cgi?id=49981 --- Comment #28 from Alex Deucher --- Created attachment 74606 --> https://bugs.freedesktop.org/attachment.cgi?id=74606&action=edit fix multi-head stability This should fix stability problems with profiles and multi-head. -- 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/20130211/612831be/attachment.html>
Re: [PATCH] drm/radeon: enforce use of radeon_get_ib_value when reading user cmd
On Mon, Feb 11, 2013 at 8:57 AM, wrote:
> From: Jerome Glisse
>
> When ever parsing cmd buffer supplied by userspace we need to use
> radeon_get_ib_value rather than directly accessing the ib as the user
> cmd might not yet be copied into the ib thus the parser might read
> value that does not correspond to what user is sending and possibly
> allowing user to send malicious command undected.
>
> Signed-off-by: Jerome Glisse
Reviewed-by: Alex Deucher
> ---
> drivers/gpu/drm/radeon/evergreen_cs.c | 86
> +--
> drivers/gpu/drm/radeon/r600_cs.c | 38
> 2 files changed, 62 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/evergreen_cs.c
> b/drivers/gpu/drm/radeon/evergreen_cs.c
> index 7a44566..ee4cff5 100644
> --- a/drivers/gpu/drm/radeon/evergreen_cs.c
> +++ b/drivers/gpu/drm/radeon/evergreen_cs.c
> @@ -2909,14 +2909,14 @@ int evergreen_dma_cs_parse(struct radeon_cs_parser *p)
> return -EINVAL;
> }
> if (tiled) {
> - dst_offset = ib[idx+1];
> + dst_offset = radeon_get_ib_value(p, idx+1);
> dst_offset <<= 8;
>
> ib[idx+1] += (u32)(dst_reloc->lobj.gpu_offset
> >> 8);
> p->idx += count + 7;
> } else {
> - dst_offset = ib[idx+1];
> - dst_offset |= ((u64)(ib[idx+2] & 0xff)) << 32;
> + dst_offset = radeon_get_ib_value(p, idx+1);
> + dst_offset |= ((u64)(radeon_get_ib_value(p,
> idx+2) & 0xff)) << 32;
>
> ib[idx+1] += (u32)(dst_reloc->lobj.gpu_offset
> & 0xfffc);
> ib[idx+2] +=
> upper_32_bits(dst_reloc->lobj.gpu_offset) & 0xff;
> @@ -2954,12 +2954,12 @@ int evergreen_dma_cs_parse(struct radeon_cs_parser *p)
> DRM_ERROR("bad L2T,
> frame to fields DMA_PACKET_COPY\n");
> return -EINVAL;
> }
> - dst_offset = ib[idx+1];
> + dst_offset =
> radeon_get_ib_value(p, idx+1);
> dst_offset <<= 8;
> - dst2_offset = ib[idx+2];
> + dst2_offset =
> radeon_get_ib_value(p, idx+2);
> dst2_offset <<= 8;
> - src_offset = ib[idx+8];
> - src_offset |=
> ((u64)(ib[idx+9] & 0xff)) << 32;
> + src_offset =
> radeon_get_ib_value(p, idx+8);
> + src_offset |=
> ((u64)(radeon_get_ib_value(p, idx+9) & 0xff)) << 32;
> if ((src_offset + (count *
> 4)) > radeon_bo_size(src_reloc->robj)) {
> dev_warn(p->dev, "DMA
> L2T, frame to fields src buffer too small (%llu %lu)\n",
> src_offset +
> (count * 4), radeon_bo_size(src_reloc->robj));
> @@ -3014,12 +3014,12 @@ int evergreen_dma_cs_parse(struct radeon_cs_parser *p)
> DRM_ERROR("bad L2T,
> broadcast DMA_PACKET_COPY\n");
> return -EINVAL;
> }
> - dst_offset = ib[idx+1];
> + dst_offset =
> radeon_get_ib_value(p, idx+1);
> dst_offset <<= 8;
> - dst2_offset = ib[idx+2];
> + dst2_offset =
> radeon_get_ib_value(p, idx+2);
> dst2_offset <<= 8;
> - src_offset = ib[idx+8];
> - src_offset |=
> ((u64)(ib[idx+9] & 0xff)) << 32;
> + src_offset =
> radeon_get_ib_value(p, idx+8);
> + src_offset |=
> ((u64)(radeon_get_ib_value(p, idx+9) & 0xff)) << 32;
> if ((src_offset + (count *
> 4)) > radeon_bo_size(src_reloc->robj)) {
> dev_warn(p->dev, "DMA
> L2T, broadcast src buffer too small
Re: [PATCH 3/3] modetest: Add a command line parameter to select the driver
Hi Jani,
On Friday 08 February 2013 15:15:50 Jani Nikula wrote:
> On Fri, 08 Feb 2013, Laurent Pinchart wrote:
> > If the -M parameter is specific, modetest will use the requested device
> > name instead of trying its builtin list of device names.
> >
> > Signed-off-by: Laurent Pinchart
> > ---
> >
> > tests/modetest/modetest.c | 34 +++---
> > 1 file changed, 27 insertions(+), 7 deletions(-)
> >
> > diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> > index 34457e2..b6298fc 100644
> > --- a/tests/modetest/modetest.c
> > +++ b/tests/modetest/modetest.c
[snip]
> > @@ -989,14 +996,27 @@ int main(int argc, char **argv)
> > if (argc == 1)
> > encoders = connectors = crtcs = planes = modes = framebuffers =
> > 1;
> >
> > - for (i = 0; i < ARRAY_SIZE(modules); i++) {
> > - printf("trying to load module %s...", modules[i]);
> > + if (module) {
> >
> > fd = drmOpen(modules[i], NULL);
>
> If this worked for you, I presume you have some uncommitted changes in
> your tree. ;) The compiler should cry about uninitialized use of i too.
>
> fd = drmOpen(module, NULL); ?
Oops :-)
modetest is compiled without any -W flag. I'll send a v4 that enables warnings
and fixes them (please ignore the v3, sorry for the noise).
> > if (fd < 0) {
> >
> > - printf("failed.\n");
> > - } else {
> > - printf("success.\n");
> > - break;
> > + printf("failed to open device '%s'.\n", module);
> > + return 1;
> > + }
> > + } else {
> > + for (i = 0; i < ARRAY_SIZE(modules); i++) {
> > + printf("trying to open device '%s'...", modules[i]);
> > + fd = drmOpen(modules[i], NULL);
> > + if (fd < 0) {
> > + printf("failed.\n");
> > + } else {
> > + printf("success.\n");
> > + break;
> > + }
> > + }
> > +
> > + if (fd < 0) {
> > + printf("no device found.\n", module);
>
> Extra param to printf.
Will be fixed in v4.
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: Use names of ioctls in debug traces
The intention here is to make the output of dmesg with full verbosity a
bit easier for a human to parse. This commit transforms:
[drm:drm_ioctl], pid=699, cmd=0x6458, nr=0x58, dev 0xe200, auth=1
[drm:drm_ioctl], pid=699, cmd=0xc010645b, nr=0x5b, dev 0xe200, auth=1
[drm:drm_ioctl], pid=699, cmd=0xc0106461, nr=0x61, dev 0xe200, auth=1
[drm:drm_ioctl], pid=699, cmd=0xc01c64ae, nr=0xae, dev 0xe200, auth=1
[drm:drm_mode_addfb], [FB:32]
[drm:drm_ioctl], pid=699, cmd=0xc0106464, nr=0x64, dev 0xe200, auth=1
[drm:drm_vm_open_locked], 0x7fd9302fe000,0x00a0
[drm:drm_ioctl], pid=699, cmd=0x400c645f, nr=0x5f, dev 0xe200, auth=1
[drm:drm_ioctl], pid=699, cmd=0xc00464af, nr=0xaf, dev 0xe200, auth=1
[drm:intel_crtc_set_config], [CRTC:3] [NOFB]
into:
[drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_THROTTLE
[drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_CREATE
[drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_SET_TILING
[drm:drm_ioctl], pid=699, dev=0xe200, auth=1, IOCTL_MODE_ADDFB
[drm:drm_mode_addfb], [FB:32]
[drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_MMAP_GTT
[drm:drm_vm_open_locked], 0x7fd9302fe000,0x00a0
[drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_SET_DOMAIN
[drm:drm_ioctl], pid=699, dev=0xe200, auth=1, DRM_IOCTL_MODE_RMFB
[drm:intel_crtc_set_config], [CRTC:3] [NOFB]
Signed-off-by: Chris Cummins
---
drivers/gpu/drm/drm_drv.c | 20 +---
include/drm/drmP.h| 3 ++-
2 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index be174ca..6480d92 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -57,7 +57,7 @@ static int drm_version(struct drm_device *dev, void *data,
struct drm_file *file_priv);
#define DRM_IOCTL_DEF(ioctl, _func, _flags) \
- [DRM_IOCTL_NR(ioctl)] = {.cmd = ioctl, .func = _func, .flags = _flags,
.cmd_drv = 0}
+ [DRM_IOCTL_NR(ioctl)] = {.cmd = ioctl, .func = _func, .flags = _flags,
.cmd_drv = 0, .name = #ioctl}
/** Ioctl table */
static struct drm_ioctl_desc drm_ioctls[] = {
@@ -376,7 +376,7 @@ long drm_ioctl(struct file *filp,
{
struct drm_file *file_priv = filp->private_data;
struct drm_device *dev;
- struct drm_ioctl_desc *ioctl;
+ struct drm_ioctl_desc *ioctl = NULL;
drm_ioctl_t *func;
unsigned int nr = DRM_IOCTL_NR(cmd);
int retcode = -EINVAL;
@@ -393,11 +393,6 @@ long drm_ioctl(struct file *filp,
atomic_inc(&dev->counts[_DRM_STAT_IOCTLS]);
++file_priv->ioctl_count;
- DRM_DEBUG("pid=%d, cmd=0x%02x, nr=0x%02x, dev 0x%lx, auth=%d\n",
- task_pid_nr(current), cmd, nr,
- (long)old_encode_dev(file_priv->minor->device),
- file_priv->authenticated);
-
if ((nr >= DRM_CORE_IOCTL_COUNT) &&
((nr < DRM_COMMAND_BASE) || (nr >= DRM_COMMAND_END)))
goto err_i1;
@@ -417,6 +412,11 @@ long drm_ioctl(struct file *filp,
} else
goto err_i1;
+ DRM_DEBUG("pid=%d, dev=0x%lx, auth=%d, %s\n",
+ task_pid_nr(current),
+ (long)old_encode_dev(file_priv->minor->device),
+ file_priv->authenticated, ioctl->name);
+
/* Do not trust userspace, use our own definition */
func = ioctl->func;
/* is there a local override? */
@@ -471,6 +471,12 @@ long drm_ioctl(struct file *filp,
}
err_i1:
+ if (!ioctl)
+ DRM_DEBUG("invalid iotcl: pid=%d, dev=0x%lx, auth=%d,
cmd=0x%02x, nr=0x%02x\n",
+ task_pid_nr(current),
+ (long)old_encode_dev(file_priv->minor->device),
+ file_priv->authenticated, cmd, nr);
+
if (kdata != stack_kdata)
kfree(kdata);
atomic_dec(&dev->ioctl_count);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index fad21c9..b55ba15 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -313,6 +313,7 @@ struct drm_ioctl_desc {
int flags;
drm_ioctl_t *func;
unsigned int cmd_drv;
+ const char *name;
};
/**
@@ -321,7 +322,7 @@ struct drm_ioctl_desc {
*/
#define DRM_IOCTL_DEF_DRV(ioctl, _func, _flags)\
- [DRM_IOCTL_NR(DRM_##ioctl)] = {.cmd = DRM_##ioctl, .func = _func,
.flags = _flags, .cmd_drv = DRM_IOCTL_##ioctl}
+ [DRM_IOCTL_NR(DRM_##ioctl)] = {.cmd = DRM_##ioctl, .func = _func,
.flags = _flags, .cmd_drv = DRM_IOCTL_##ioctl, .name = #ioctl}
struct drm_magic_entry {
struct list_head head;
--
1.8.1.2
-
Intel Corporation (UK) Limited
Registered No. 1134945 (England)
Registered Office: Pipers Way, Swindon SN3 1RJ
VAT No: 860 2173 47
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is
Re: [PATCH v2 3/3] modetest: Add a command line parameter to select the driver
On Monday 11 February 2013 21:13:45 Laurent Pinchart wrote:
> If the -M parameter is specific, modetest will use the requested device
> name instead of trying its builtin list of device names.
>
> Signed-off-by: Laurent Pinchart
> ---
> tests/modetest/modetest.c | 41 -
> 1 file changed, 28 insertions(+), 13 deletions(-)
>
> diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> index 34457e2..9a2d1f8 100644
> --- a/tests/modetest/modetest.c
> +++ b/tests/modetest/modetest.c
[snip]
> @@ -989,14 +996,27 @@ int main(int argc, char **argv)
> if (argc == 1)
> encoders = connectors = crtcs = planes = modes = framebuffers =
> 1;
>
> - for (i = 0; i < ARRAY_SIZE(modules); i++) {
> - printf("trying to load module %s...", modules[i]);
> - fd = drmOpen(modules[i], NULL);
> + if (module) {
> + fd = drmOpen(module, NULL);
> if (fd < 0) {
> - printf("failed.\n");
> - } else {
> - printf("success.\n");
> - break;
> + fprintf(stderr, "failed to open device '%s'.\n",
> module);
> + return 1;
> + }
> + } else {
> + for (i = 0; i < ARRAY_SIZE(modules); i++) {
> + printf("trying to open device '%s'...", modules[i]);
> + fd = drmOpen(modules[i], NULL);
> + if (fd < 0) {
> + printf("failed.\n");
> + } else {
> + printf("success.\n");
> + break;
> + }
> + }
> +
> + if (fd < 0) {
> + fprintf(stderr, "no device found.\n", module);
I should sleep before sending patches... Sorry for the noise, v3 will fix
that.
> + return 1;
> }
> }
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 3/3] modetest: Add a command line parameter to select the driver
If the -M parameter is specific, modetest will use the requested device
name instead of trying its builtin list of device names.
Signed-off-by: Laurent Pinchart
---
tests/modetest/modetest.c | 41 -
1 file changed, 28 insertions(+), 13 deletions(-)
diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 34457e2..9a2d1f8 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -908,6 +908,9 @@ void usage(char *name)
fprintf(stderr, "\t-s [@]:[@]\tset
a mode\n");
fprintf(stderr, "\t-v\ttest vsynced page flipping\n");
+ fprintf(stderr, "\n Generic options:\n\n");
+ fprintf(stderr, "\t-M module\tuse the given driver\n");
+
fprintf(stderr, "\n\tDefault is to dump all info.\n");
exit(0);
}
@@ -935,7 +938,7 @@ static int page_flipping_supported(void)
#endif
}
-static char optstr[] = "cefmP:ps:v";
+static char optstr[] = "cefM:mP:ps:v";
int main(int argc, char **argv)
{
@@ -943,6 +946,7 @@ int main(int argc, char **argv)
int encoders = 0, connectors = 0, crtcs = 0, planes = 0, framebuffers =
0;
int test_vsync = 0;
char *modules[] = { "i915", "radeon", "nouveau", "vmwgfx", "omapdrm",
"exynos" };
+ char *module = NULL;
unsigned int i;
int count = 0, plane_count = 0;
struct connector con_args[2];
@@ -960,6 +964,9 @@ int main(int argc, char **argv)
case 'f':
framebuffers = 1;
break;
+ case 'M':
+ module = optarg;
+ break;
case 'm':
modes = 1;
break;
@@ -989,14 +996,27 @@ int main(int argc, char **argv)
if (argc == 1)
encoders = connectors = crtcs = planes = modes = framebuffers =
1;
- for (i = 0; i < ARRAY_SIZE(modules); i++) {
- printf("trying to load module %s...", modules[i]);
- fd = drmOpen(modules[i], NULL);
+ if (module) {
+ fd = drmOpen(module, NULL);
if (fd < 0) {
- printf("failed.\n");
- } else {
- printf("success.\n");
- break;
+ fprintf(stderr, "failed to open device '%s'.\n",
module);
+ return 1;
+ }
+ } else {
+ for (i = 0; i < ARRAY_SIZE(modules); i++) {
+ printf("trying to open device '%s'...", modules[i]);
+ fd = drmOpen(modules[i], NULL);
+ if (fd < 0) {
+ printf("failed.\n");
+ } else {
+ printf("success.\n");
+ break;
+ }
+ }
+
+ if (fd < 0) {
+ fprintf(stderr, "no device found.\n", module);
+ return 1;
}
}
@@ -1005,11 +1025,6 @@ int main(int argc, char **argv)
return -1;
}
- if (i == ARRAY_SIZE(modules)) {
- fprintf(stderr, "failed to load any modules, aborting.\n");
- return -1;
- }
-
resources = drmModeGetResources(fd);
if (!resources) {
fprintf(stderr, "drmModeGetResources failed: %s\n",
--
1.7.12.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/3] modetest: Sort command line arguments
The current mostly random sort order hinders code readability. Sort the
options alphabetically in the code, and by group in the help message.
Signed-off-by: Laurent Pinchart
---
tests/modetest/modetest.c | 49 ++-
1 file changed, 27 insertions(+), 22 deletions(-)
diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 489918e..34457e2 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -835,8 +835,6 @@ set_mode(struct connector *c, int count, struct plane *p,
int plane_count,
kms_destroy(&kms);
}
-static char optstr[] = "ecpmfs:P:v";
-
#define min(a, b) ((a) < (b) ? (a) : (b))
static int parse_connector(struct connector *c, const char *arg)
@@ -896,15 +894,20 @@ static int parse_plane(struct plane *p, const char *arg)
void usage(char *name)
{
- fprintf(stderr, "usage: %s [-ecpmf]\n", name);
- fprintf(stderr, "\t-e\tlist encoders\n");
+ fprintf(stderr, "usage: %s [-cefmPpsv]\n", name);
+
+ fprintf(stderr, "\n Query options:\n\n");
fprintf(stderr, "\t-c\tlist connectors\n");
- fprintf(stderr, "\t-p\tlist CRTCs and planes (pipes)\n");
- fprintf(stderr, "\t-m\tlist modes\n");
+ fprintf(stderr, "\t-e\tlist encoders\n");
fprintf(stderr, "\t-f\tlist framebuffers\n");
- fprintf(stderr, "\t-v\ttest vsynced page flipping\n");
- fprintf(stderr, "\t-s [@]:[@]\tset
a mode\n");
+ fprintf(stderr, "\t-m\tlist modes\n");
+
+ fprintf(stderr, "\n Test options:\n\n");
+ fprintf(stderr, "\t-p\tlist CRTCs and planes (pipes)\n");
fprintf(stderr, "\t-P :x[@]\tset a
plane\n");
+ fprintf(stderr, "\t-s [@]:[@]\tset
a mode\n");
+ fprintf(stderr, "\t-v\ttest vsynced page flipping\n");
+
fprintf(stderr, "\n\tDefault is to dump all info.\n");
exit(0);
}
@@ -932,6 +935,8 @@ static int page_flipping_supported(void)
#endif
}
+static char optstr[] = "cefmP:ps:v";
+
int main(int argc, char **argv)
{
int c;
@@ -946,34 +951,34 @@ int main(int argc, char **argv)
opterr = 0;
while ((c = getopt(argc, argv, optstr)) != -1) {
switch (c) {
- case 'e':
- encoders = 1;
- break;
case 'c':
connectors = 1;
break;
- case 'p':
- crtcs = 1;
- planes = 1;
+ case 'e':
+ encoders = 1;
+ break;
+ case 'f':
+ framebuffers = 1;
break;
case 'm':
modes = 1;
break;
- case 'f':
- framebuffers = 1;
+ case 'P':
+ if (parse_plane(&plane_args[plane_count], optarg) < 0)
+ usage(argv[0]);
+ plane_count++;
break;
- case 'v':
- test_vsync = 1;
+ case 'p':
+ crtcs = 1;
+ planes = 1;
break;
case 's':
if (parse_connector(&con_args[count], optarg) < 0)
usage(argv[0]);
count++;
break;
- case 'P':
- if (parse_plane(&plane_args[plane_count], optarg) < 0)
- usage(argv[0]);
- plane_count++;
+ case 'v':
+ test_vsync = 1;
break;
default:
usage(argv[0]);
--
1.7.12.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/3] modetest: Remove extern declarations of opt(arg|ind|err|opt)
Those variables are declared in unistd.h, there's no need to redeclare them here. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index c91bb9d..489918e 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -835,8 +835,6 @@ set_mode(struct connector *c, int count, struct plane *p, int plane_count, kms_destroy(&kms); } -extern char *optarg; -extern int optind, opterr, optopt; static char optstr[] = "ecpmfs:P:v"; #define min(a, b) ((a) < (b) ? (a) : (b)) -- 1.7.12.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/3] modetest: Allow selecting a driver manually
Hello, Here's the second version of a small patch set that adds an argument to the modetest command line to specify the driver name instead of using the builtin list of drivers. This is particularly handy during development of new DRM/KMS drivers. Changes from v1: - Use the module name specified on the command line instead of a random module name (I'll blame a bad copy & paste). Laurent Pinchart (3): modetest: Remove extern declarations of opt(arg|ind|err|opt) modetest: Sort command line arguments modetest: Add a command line parameter to select the driver tests/modetest/modetest.c | 88 --- 1 file changed, 53 insertions(+), 35 deletions(-) -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH 0/4] Common Display Framework-TF
.. Name: signature.asc Type: application/pgp-signature Size: 899 bytes Desc: OpenPGP digital signature URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130211/94bbf725/attachment.pgp>
re: drm/nv50/pm: use hwsq for engine reclocking too
Hello Ben Skeggs,
The patch 496a73bbecb8: "drm/nv50/pm: use hwsq for engine reclocking
too" from Jan 24, 2012, leads to the following Smatch warning:
"drivers/gpu/drm/nouveau/nv50_pm.c:638 nv50_pm_clocks_pre()
warn: 'info->mmast' might be uninitialized"
[ This Smatch check isn't ready for release yet ].
drivers/gpu/drm/nouveau/nv50_pm.c
621
622 info = kmalloc(sizeof(*info), GFP_KERNEL);
623 if (!info)
624 return ERR_PTR(-ENOMEM);
625 info->perflvl = perflvl;
626
627 /* memory: build hwsq ucode which we'll use to reclock memory.
628 * use pcie refclock if possible, otherwise use mpll */
629 info->mclk_hwsq.len = 0;
630 if (perflvl->memory) {
631 ret = calc_mclk(dev, perflvl, info);
632 if (ret)
633 goto error;
634 info->mscript = perflvl->memscript;
635 }
636
637 divs = read_div(dev);
638 mast = info->mmast;
^^^
My reading is that "info" is setup inside calc_mclk() so if
perflvl->memory is false then info->mmast isn't set.
regards,
dan carpenter
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/radeon: enforce use of radeon_get_ib_value when reading user cmd
From: Jerome Glisse
When ever parsing cmd buffer supplied by userspace we need to use
radeon_get_ib_value rather than directly accessing the ib as the user
cmd might not yet be copied into the ib thus the parser might read
value that does not correspond to what user is sending and possibly
allowing user to send malicious command undected.
Signed-off-by: Jerome Glisse
---
drivers/gpu/drm/radeon/evergreen_cs.c | 86 +--
drivers/gpu/drm/radeon/r600_cs.c | 38
2 files changed, 62 insertions(+), 62 deletions(-)
diff --git a/drivers/gpu/drm/radeon/evergreen_cs.c
b/drivers/gpu/drm/radeon/evergreen_cs.c
index 7a44566..ee4cff5 100644
--- a/drivers/gpu/drm/radeon/evergreen_cs.c
+++ b/drivers/gpu/drm/radeon/evergreen_cs.c
@@ -2909,14 +2909,14 @@ int evergreen_dma_cs_parse(struct radeon_cs_parser *p)
return -EINVAL;
}
if (tiled) {
- dst_offset = ib[idx+1];
+ dst_offset = radeon_get_ib_value(p, idx+1);
dst_offset <<= 8;
ib[idx+1] += (u32)(dst_reloc->lobj.gpu_offset
>> 8);
p->idx += count + 7;
} else {
- dst_offset = ib[idx+1];
- dst_offset |= ((u64)(ib[idx+2] & 0xff)) << 32;
+ dst_offset = radeon_get_ib_value(p, idx+1);
+ dst_offset |= ((u64)(radeon_get_ib_value(p,
idx+2) & 0xff)) << 32;
ib[idx+1] += (u32)(dst_reloc->lobj.gpu_offset &
0xfffc);
ib[idx+2] +=
upper_32_bits(dst_reloc->lobj.gpu_offset) & 0xff;
@@ -2954,12 +2954,12 @@ int evergreen_dma_cs_parse(struct radeon_cs_parser *p)
DRM_ERROR("bad L2T,
frame to fields DMA_PACKET_COPY\n");
return -EINVAL;
}
- dst_offset = ib[idx+1];
+ dst_offset =
radeon_get_ib_value(p, idx+1);
dst_offset <<= 8;
- dst2_offset = ib[idx+2];
+ dst2_offset =
radeon_get_ib_value(p, idx+2);
dst2_offset <<= 8;
- src_offset = ib[idx+8];
- src_offset |= ((u64)(ib[idx+9]
& 0xff)) << 32;
+ src_offset =
radeon_get_ib_value(p, idx+8);
+ src_offset |=
((u64)(radeon_get_ib_value(p, idx+9) & 0xff)) << 32;
if ((src_offset + (count * 4))
> radeon_bo_size(src_reloc->robj)) {
dev_warn(p->dev, "DMA
L2T, frame to fields src buffer too small (%llu %lu)\n",
src_offset +
(count * 4), radeon_bo_size(src_reloc->robj));
@@ -3014,12 +3014,12 @@ int evergreen_dma_cs_parse(struct radeon_cs_parser *p)
DRM_ERROR("bad L2T,
broadcast DMA_PACKET_COPY\n");
return -EINVAL;
}
- dst_offset = ib[idx+1];
+ dst_offset =
radeon_get_ib_value(p, idx+1);
dst_offset <<= 8;
- dst2_offset = ib[idx+2];
+ dst2_offset =
radeon_get_ib_value(p, idx+2);
dst2_offset <<= 8;
- src_offset = ib[idx+8];
- src_offset |= ((u64)(ib[idx+9]
& 0xff)) << 32;
+ src_offset =
radeon_get_ib_value(p, idx+8);
+ src_offset |=
((u64)(radeon_get_ib_value(p, idx+9) & 0xff)) << 32;
if ((src_offset + (count * 4))
> radeon_bo_size(src_reloc->robj)) {
dev_warn(p->dev, "DMA
L2T, broadcast src buffer too small (%llu %lu)\n",
src_offset +
(count * 4), radeon_bo_size(src_reloc->robj));
@@ -3046,22 +3046,22 @@ int evergreen_dma_cs_parse(struct radeon_cs_parser *p)
[RFC PATCH 0/4] Common Display Framework-TF
On 02/11/2013 09:21 AM, Tomi Valkeinen wrote: > On 2013-02-08 16:54, Marcus Lorentzon wrote: >> On 02/08/2013 03:02 PM, Tomi Valkeinen wrote: >>> On 2013-02-08 15:28, Marcus Lorentzon wrote: >>> When we do that we stop->setup->start during blanking. So our "DSS" is optimized to be able to do that without getting blocked. All DSI video mode panels (and DPI) products we have done so far have not had any issue with that (as long as DSI HS clock is set to continuous). I think this approach is less platform dependant, as long as there is no SoC that take more than a blanking period to reconfigure. >>> So do you stop, setup and start the link with CPU, and this has to be >>> happen during blanking? Isn't that prone to errors? Or did you mean that >>> the hardware handles that automatically? >>> >>> In OMAP DSS there are so called shadow registers, that can be programmed >>> at any time. The you set a bit (GO bit), which tells the hardware to >>> take the new settings into use at the next vblank. >>> >>> From DSI driver's perspective the link is never stopped when >>> reconfiguring the video timings. However, many other settings have to be >>> configured when the link is disabled. >> Yeah, you lucky guys with the GO bit ;). No, we actually do CPU >> stop,setup,start. But since it is video mode, master is driving the sync >> so it is not a hard deadline. It is enough to restart before pixels >> start to degrade. On an LCD that is not so much time, but on an OLED it >> could be 10 secs :). Anyway, we have had several mass products with this >> soft solution and it has worked well. > Ah, ok. But in that case what you said in an earlier mail is not quite > correct: "I think this approach is less platform dependant, as long as > there is no SoC that take more than a blanking period to reconfigure.". > So in your approach the reconfiguration doesn't have to be done inside > the blanking period, but before the panel's picture starts to fade? > > I don't know... It's early Monday morning, and not enough coffee yet, > but I get a bit uneasy feeling if I think that your method of > reconfiguring would be the only think allowed by the CDF API. > > Some SoCs do support reconfiguring on the fly, without disabling the > link. It would not be nice if we didn't allow this to happen. And > actually, we're not only talking about SoCs here, but also any display > devices, like external buffer chips etc. They may also offer means to > change configs on the fly. > > Well, I don't have any hard point about this at the moment, but I think > we should think different approaches how the configuration can be done. Ok, so what about a compromise which I think would work for "both" HWs ... we allow the "configure" operation during video on, then each HW driver could decide if this means you have to stop or not to do the changes required. But then it is also important that we keep all config in one struct and not split it up. Otherwise HW like ours that has so be stopped could need to stop once for each setting/operation called. So in short, allow configure(bus_params) during video on, keep all bus_params in the struct. It is in kernel struct so it can easily be changed/refactored. > >> I understand, but removing the omap prefix doesn't mean it has to go in >> the panel slave port/bus settings. I still can't see why this should be >> configuration on the panel driver and not the DSI master driver. Number >> of pins might be useful since you might start with one lane and then >> activate the rest. But partial muxing (pre pinmux) doesn't seem to be >> something the panel should control or know anything about. Sounds like >> normal platform/DT data per product/board. > I think one case where this kind of pin configuration is needed, and > which also dictates that all panel related configuration has to be in > the panel's data, not in the DSI master's data, is hotplug. > > If you have a board that has two panels connected to the same video > output, probably via some kind of mux, at least for the sensitive pins > like DSI, only one of the panels can be enabled at a time. The panels > can have different wiring, and thus the panel driver would need to > configure everything related to the bus when it's starting up. > > The same also happens if you have a true hotplug, i.e. you can remove > the panel totally and plug in a new one. Again the wiring can be > different, and needs to be set up. > > And, as I said, this means that all relevant data about the video bus > has to be in the panel's data, so that each panel can have its own set > of, say, pin configuration. > > Hotplug is not a use case we should aim to support for the CDF v1, but I > think we should strive not to prevent hotplug either. So if we can > design the API so that hotplug is possible, I say let's do that. > Again, this probing and bus muxing is platform/bus specific and not panel specific. Any panel of that type will only ever work on Omap
[RFC PATCH 0/4] Common Display Framework-TF
On 2013-02-08 16:54, Marcus Lorentzon wrote: > On 02/08/2013 03:02 PM, Tomi Valkeinen wrote: >> On 2013-02-08 15:28, Marcus Lorentzon wrote: >> >>> When we do that we stop->setup->start during blanking. So our "DSS" is >>> optimized to be able to do that without getting blocked. All DSI video >>> mode panels (and DPI) products we have done so far have not had any >>> issue with that (as long as DSI HS clock is set to continuous). I think >>> this approach is less platform dependant, as long as there is no SoC >>> that take more than a blanking period to reconfigure. >> So do you stop, setup and start the link with CPU, and this has to be >> happen during blanking? Isn't that prone to errors? Or did you mean that >> the hardware handles that automatically? >> >> In OMAP DSS there are so called shadow registers, that can be programmed >> at any time. The you set a bit (GO bit), which tells the hardware to >> take the new settings into use at the next vblank. >> >> From DSI driver's perspective the link is never stopped when >> reconfiguring the video timings. However, many other settings have to be >> configured when the link is disabled. > > Yeah, you lucky guys with the GO bit ;). No, we actually do CPU > stop,setup,start. But since it is video mode, master is driving the sync > so it is not a hard deadline. It is enough to restart before pixels > start to degrade. On an LCD that is not so much time, but on an OLED it > could be 10 secs :). Anyway, we have had several mass products with this > soft solution and it has worked well. Ah, ok. But in that case what you said in an earlier mail is not quite correct: "I think this approach is less platform dependant, as long as there is no SoC that take more than a blanking period to reconfigure.". So in your approach the reconfiguration doesn't have to be done inside the blanking period, but before the panel's picture starts to fade? I don't know... It's early Monday morning, and not enough coffee yet, but I get a bit uneasy feeling if I think that your method of reconfiguring would be the only think allowed by the CDF API. Some SoCs do support reconfiguring on the fly, without disabling the link. It would not be nice if we didn't allow this to happen. And actually, we're not only talking about SoCs here, but also any display devices, like external buffer chips etc. They may also offer means to change configs on the fly. Well, I don't have any hard point about this at the moment, but I think we should think different approaches how the configuration can be done. > I understand, but removing the omap prefix doesn't mean it has to go in > the panel slave port/bus settings. I still can't see why this should be > configuration on the panel driver and not the DSI master driver. Number > of pins might be useful since you might start with one lane and then > activate the rest. But partial muxing (pre pinmux) doesn't seem to be > something the panel should control or know anything about. Sounds like > normal platform/DT data per product/board. I think one case where this kind of pin configuration is needed, and which also dictates that all panel related configuration has to be in the panel's data, not in the DSI master's data, is hotplug. If you have a board that has two panels connected to the same video output, probably via some kind of mux, at least for the sensitive pins like DSI, only one of the panels can be enabled at a time. The panels can have different wiring, and thus the panel driver would need to configure everything related to the bus when it's starting up. The same also happens if you have a true hotplug, i.e. you can remove the panel totally and plug in a new one. Again the wiring can be different, and needs to be set up. And, as I said, this means that all relevant data about the video bus has to be in the panel's data, so that each panel can have its own set of, say, pin configuration. Hotplug is not a use case we should aim to support for the CDF v1, but I think we should strive not to prevent hotplug either. So if we can design the API so that hotplug is possible, I say let's do that. Tomi -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 899 bytes Desc: OpenPGP digital signature URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130211/d79bdc97/attachment.pgp>
[PATCH v2 4/5] drm/tegra: Implement VBLANK support
On Tue, Jan 22, 2013 at 06:37:39PM +0100, Mario Kleiner wrote:
> On 14.01.13 17:05, Thierry Reding wrote:
> >Implement support for the VBLANK IOCTL. Note that Tegra is somewhat
> >special in this case because it doesn't use the generic IRQ support
> >provided by the DRM core (DRIVER_HAVE_IRQ) but rather registers one
> >interrupt handler for each display controller.
> >
> >While at it, clean up the way that interrupts are enabled to ensure
> >that the VBLANK interrupt only gets enabled when required.
> >
> >Signed-off-by: Thierry Reding
>
> ... snip ...
>
> > struct drm_driver tegra_drm_driver = {
> > .driver_features = DRIVER_BUS_PLATFORM | DRIVER_MODESET | DRIVER_GEM,
> > .load = tegra_drm_load,
> >@@ -96,6 +136,10 @@ struct drm_driver tegra_drm_driver = {
> > .open = tegra_drm_open,
> > .lastclose = tegra_drm_lastclose,
> >
> >+.get_vblank_counter = drm_vblank_count,
>
> -> .get_vblank_counter = drm_vblank_count is a no-op.
>
> .get_vblank_counter() is supposed to return some real hardware
> vblank counter value to reinitialize the software vblank counter at
> vbl irq enable time. That software counter gets queried via
> drm_vblank_count(). If you hook this up to drm_vblank_count() it
> essentially returns a constant, frozen vblank count, it has the same
> effect as returning zero or any other constant value -- You lose all
> vblank counter increments during vblank irq off time. The same
> problem is present in nouveau-kms.
>
> I think it would be better to either implement a real hw counter
> query, or some function with a /* TODO: Implement me properly */
> comment which returns zero, so it is clear something is missing
> here.
I've finally managed to find some time to look into this some more. The
comment atop the drm_driver.get_vblank_counter() field actually suggests
that drivers should set this to drm_vblank_count if no real hardware
counter is supported.
Now it seems like we may get functionality to obtain the real VBLANK
counter once the syncpoint support is merged, so maybe we can leave this
as-is until then and replace it with a proper implementation at that
point in time?
Alternatively I could use a small wrapper with an explicit comment that
this should be implemented using the upcoming syncpoint support.
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/20130211/957289c3/attachment.pgp>
[PATCH v2 4/5] drm/tegra: Implement VBLANK support
On Wed, Jan 23, 2013 at 10:02:20AM +0200, Terje Bergstr?m wrote: > On 22.01.2013 21:59, Mario Kleiner wrote: > > The current swap scheduling is based on having an accurate software > > vblank counter. Atm. that counter is maintained in software, incremented > > during vblank irq. At irq off -> on transition we need a hw counter to > > reinitialize. And there is a timeout between dropping the last reference > > to a counter via drm_vblank_put() and actually disabling the vblank irq. > > If the timeout is long enough and a timing sensitive app is aware that > > vblank counters may be inaccurate/unreliable over long time periods, it > > can work around the problem. > > We have a hardware counter that can be used as VBLANK counter - host1x > sync point register numbers 26 and 27. tegradrm already sets them up in > dc init (DC_CMD_CONT_SYNCPT_VSYNC). Basic syncpt support (read, wait) is > part of my patch set to implement 2D acceleration. Are the syncpoints incremented even if the VBLANK interrupts are turned off? If that's the case they could indeed be used as a hardware counter rather than the fake approach used right now. Maybe we should leave the code as it is right now and fix that up once the syncpoint support has been merged. 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/20130211/67b8d42d/attachment-0001.pgp>
Re: [PATCH 5/5] drm/tegra: Implement page-flipping support
On Sat, Feb 2, 2013 at 12:05 AM, Laurent Pinchart wrote: >> Back to the original topic: should it not work to queue a page flip and >> immediately exit(0) after that to test the cleanup code? I suppose if >> that can be tested on all drivers we should have pretty good coverage. > > Maybe we need some kind of compliance test suite tool ? kms_flip from our intel-specific testsuite is probably the most paranoid thing for testing page flips out there: http://cgit.freedesktop.org/xorg/app/intel-gpu-tools/tree/tests/kms_flip.c Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 5/5] drm/tegra: Implement page-flipping support
On Tue, Jan 22, 2013 at 06:27:24PM +0100, Mario Kleiner wrote:
> On 22.01.13 09:31, Terje Bergstr?m wrote:
> >On 14.01.2013 18:06, Thierry Reding wrote:
> >>+static int tegra_dc_page_flip(struct drm_crtc *crtc, struct
> >>drm_framebuffer *fb,
> >>+ struct drm_pending_vblank_event *event)
> >>+{
> >>+ struct tegra_framebuffer *newfb = to_tegra_fb(fb);
> >>+ struct tegra_dc *dc = to_tegra_dc(crtc);
> >>+ struct drm_device *drm = crtc->dev;
> >>+ unsigned long flags;
> >>+
> >>+ if (dc->event)
> >>+ return -EBUSY;
>
> Hi
>
> I haven't read the Tegra programming manual or played with this, so
> maybe i'm wrong for some reason, but i think there is a race here
> between actual pageflip completion - latching newfb into the scanout
> base register and the completion routine that gets called from the
> vblank irq handler.
>
> If this code gets called close to vblank or inside vblank, couldn't
> it happen that tegra_dc_set_base() programs a pageflip that gets
> executed immediately - the new fb gets latched into the crtc, but
> the corresponding vblank irq handler for the vblank of flip
> completion runs before the "if (event)" block can set dc->event =
> event;? Or vblank irq's are off because drm_vblank_get() is only
> called at the end of the routine? In both cases the completion
> routine would miss the correct vblank and only timestamp and emit
> the completion event 1 vblank after actual flip completion.
>
> In that case it would be better to place tegra_dc_set_base() after
> the "if (event)" block and have some check inside the flip
> completion routine to make sure the pageflip completion event is
> only emitted if the scanout is really already latched with the
> newfb.
An easier solution might be to expand the scope of the lock a bit to
encompass the call to tegra_dc_set_base() and extend until the end of
tegra_dc_page_flip(). That should properly keep the page-flip completion
from interfering, right?
spin_lock_irqsave(&drm->event_lock, flags);
tegra_dc_set_base(dc, 0, 0, newfb);
if (event) {
event->pipe = dc->pipe;
dc->event = event;
drm_vblank_get(drm, dc->pipe);
}
spin_unlock_irqrestore(&drm->event_lock, flags);
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/20130211/92bb6f44/attachment.pgp>
[PATCH] drm/radeon: enforce use of radeon_get_ib_value when reading user cmd
From: Jerome Glisse
When ever parsing cmd buffer supplied by userspace we need to use
radeon_get_ib_value rather than directly accessing the ib as the user
cmd might not yet be copied into the ib thus the parser might read
value that does not correspond to what user is sending and possibly
allowing user to send malicious command undected.
Signed-off-by: Jerome Glisse
---
drivers/gpu/drm/radeon/evergreen_cs.c | 86 +--
drivers/gpu/drm/radeon/r600_cs.c | 38
2 files changed, 62 insertions(+), 62 deletions(-)
diff --git a/drivers/gpu/drm/radeon/evergreen_cs.c
b/drivers/gpu/drm/radeon/evergreen_cs.c
index 7a44566..ee4cff5 100644
--- a/drivers/gpu/drm/radeon/evergreen_cs.c
+++ b/drivers/gpu/drm/radeon/evergreen_cs.c
@@ -2909,14 +2909,14 @@ int evergreen_dma_cs_parse(struct radeon_cs_parser *p)
return -EINVAL;
}
if (tiled) {
- dst_offset = ib[idx+1];
+ dst_offset = radeon_get_ib_value(p, idx+1);
dst_offset <<= 8;
ib[idx+1] += (u32)(dst_reloc->lobj.gpu_offset
>> 8);
p->idx += count + 7;
} else {
- dst_offset = ib[idx+1];
- dst_offset |= ((u64)(ib[idx+2] & 0xff)) << 32;
+ dst_offset = radeon_get_ib_value(p, idx+1);
+ dst_offset |= ((u64)(radeon_get_ib_value(p,
idx+2) & 0xff)) << 32;
ib[idx+1] += (u32)(dst_reloc->lobj.gpu_offset &
0xfffc);
ib[idx+2] +=
upper_32_bits(dst_reloc->lobj.gpu_offset) & 0xff;
@@ -2954,12 +2954,12 @@ int evergreen_dma_cs_parse(struct radeon_cs_parser *p)
DRM_ERROR("bad L2T,
frame to fields DMA_PACKET_COPY\n");
return -EINVAL;
}
- dst_offset = ib[idx+1];
+ dst_offset =
radeon_get_ib_value(p, idx+1);
dst_offset <<= 8;
- dst2_offset = ib[idx+2];
+ dst2_offset =
radeon_get_ib_value(p, idx+2);
dst2_offset <<= 8;
- src_offset = ib[idx+8];
- src_offset |= ((u64)(ib[idx+9]
& 0xff)) << 32;
+ src_offset =
radeon_get_ib_value(p, idx+8);
+ src_offset |=
((u64)(radeon_get_ib_value(p, idx+9) & 0xff)) << 32;
if ((src_offset + (count * 4))
> radeon_bo_size(src_reloc->robj)) {
dev_warn(p->dev, "DMA
L2T, frame to fields src buffer too small (%llu %lu)\n",
src_offset +
(count * 4), radeon_bo_size(src_reloc->robj));
@@ -3014,12 +3014,12 @@ int evergreen_dma_cs_parse(struct radeon_cs_parser *p)
DRM_ERROR("bad L2T,
broadcast DMA_PACKET_COPY\n");
return -EINVAL;
}
- dst_offset = ib[idx+1];
+ dst_offset =
radeon_get_ib_value(p, idx+1);
dst_offset <<= 8;
- dst2_offset = ib[idx+2];
+ dst2_offset =
radeon_get_ib_value(p, idx+2);
dst2_offset <<= 8;
- src_offset = ib[idx+8];
- src_offset |= ((u64)(ib[idx+9]
& 0xff)) << 32;
+ src_offset =
radeon_get_ib_value(p, idx+8);
+ src_offset |=
((u64)(radeon_get_ib_value(p, idx+9) & 0xff)) << 32;
if ((src_offset + (count * 4))
> radeon_bo_size(src_reloc->robj)) {
dev_warn(p->dev, "DMA
L2T, broadcast src buffer too small (%llu %lu)\n",
src_offset +
(count * 4), radeon_bo_size(src_reloc->robj));
@@ -3046,22 +3046,22 @@ int evergreen_dma_cs_parse(struct radeon_cs_parser *p)
[PATCHv5,RESEND 8/8] drm: tegra: Add gr2d device
On Sun, Feb 10, 2013 at 04:42:53PM -0800, Terje Bergstr?m wrote: > On 07.02.2013 23:07, Thierry Reding wrote: > > On Wed, Feb 06, 2013 at 01:23:17PM -0800, Terje Bergstr?m wrote: > >>>> That's the security firewall. It walks through each submit, and ensures > >>>> that each register write that writes an address, goes through the host1x > >>>> reloc mechanism. This way user space cannot ask 2D to write to arbitrary > >>>> memory locations. > >>> I see. Can this be made more generic? Perhaps adding a table of valid > >>> registers to the device and use a generic function to iterate over that > >>> instead of having to provide the same function for each client. > >> For which one does gcc generate more efficient code? I've thought a > >> switch-case statement might get compiled into something more efficient > >> than a table lookup. > >> But the rest of the code is generic - just the one function which > >> compares against known address registers is specific to 2D. > > Table lookup should be pretty fast. I wouldn't worry too much about > > performance at this stage, though. Readability is more important in my > > opinion. A lookup table is a lot more readable and reusable I think. If > > it turns out that using a function is actually faster we can always > > optimize later. > > You're right about performance. We already saw quite a bad performance > hit with the current firewall, so we'll need to worry about performance > later. I guess the additional overhead of looking up in a table vs. an actual function being run will be rather small compared to the total overhead incurred by having the firewall in the first place. > I'll take a look at converting the register list to a table. Instead of > always doing a linear search of a table, a bitfield might be more > appropriate. I don't know. Just a plain table with register offsets seems a lot more straightforward than a bitfield. In my opinion an array of offsets is a lot more readable than a field of bits. Especially since you can't just setup a bitfield easily with initialized values. 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/20130211/adcd2a5f/attachment.pgp>
Re: [PATCH v2 4/5] drm/tegra: Implement VBLANK support
On 11.02.2013 01:08, Thierry Reding wrote: > Are the syncpoints incremented even if the VBLANK interrupts are turned > off? If that's the case they could indeed be used as a hardware counter > rather than the fake approach used right now. > > Maybe we should leave the code as it is right now and fix that up once > the syncpoint support has been merged. Yes, the sync point increment signal to host1x is independent of VBLANK interrupt. Definitely not needed yet, so that was a comment for future improvement. Terje ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 4/5] drm/tegra: Implement VBLANK support
On 11.02.2013 01:08, Thierry Reding wrote: > Are the syncpoints incremented even if the VBLANK interrupts are turned > off? If that's the case they could indeed be used as a hardware counter > rather than the fake approach used right now. > > Maybe we should leave the code as it is right now and fix that up once > the syncpoint support has been merged. Yes, the sync point increment signal to host1x is independent of VBLANK interrupt. Definitely not needed yet, so that was a comment for future improvement. Terje
Re: [PATCHv5,RESEND 8/8] drm: tegra: Add gr2d device
On 10.02.2013 22:44, Thierry Reding wrote: > On Sun, Feb 10, 2013 at 04:42:53PM -0800, Terje Bergström wrote: >> You're right about performance. We already saw quite a bad performance >> hit with the current firewall, so we'll need to worry about performance >> later. > > I guess the additional overhead of looking up in a table vs. an actual > function being run will be rather small compared to the total overhead > incurred by having the firewall in the first place. Yeah, I'll just implement a simple linear table lookup and let's see what happens. I'll optimize with bitfield if needed. Terje ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCHv5,RESEND 8/8] drm: tegra: Add gr2d device
On 10.02.2013 22:44, Thierry Reding wrote: > On Sun, Feb 10, 2013 at 04:42:53PM -0800, Terje Bergstr?m wrote: >> You're right about performance. We already saw quite a bad performance >> hit with the current firewall, so we'll need to worry about performance >> later. > > I guess the additional overhead of looking up in a table vs. an actual > function being run will be rather small compared to the total overhead > incurred by having the firewall in the first place. Yeah, I'll just implement a simple linear table lookup and let's see what happens. I'll optimize with bitfield if needed. Terje
Re: [RFC PATCH 0/4] Common Display Framework-TF
On 2013-02-11 11:31, Marcus Lorentzon wrote: > Ok, so what about a compromise which I think would work for "both" HWs > ... we allow the "configure" operation during video on, then each HW > driver could decide if this means you have to stop or not to do the > changes required. But then it is also important that we keep all config > in one struct and not split it up. Otherwise HW like ours that has so be > stopped could need to stop once for each setting/operation called. > So in short, allow configure(bus_params) during video on, keep all > bus_params in the struct. It is in kernel struct so it can easily be > changed/refactored. Right, we need some way to group the configuration parameters. Either one big struct, or something like start_config() and end_config(). I think we could also think what parameters make no sense to be configured on the fly, and possibly have those separately. Although it may be a bit difficult to think of all the (weird) use cases. > Again, this probing and bus muxing is platform/bus specific and not > panel specific. Any panel of that type will only ever work on Omap (or The parameters used for configuration itself is platform specific, and that's why it needs to be defined in the DT data. But the API itself is not platform specific. The parameters are information about how the panel is connected, just like, say, number of data lines is for DPI. Which is also something I think should be configured by the panel. > someone else implementing the same muxing features) as I see it. So why No, it works for everyone. Well, at least I still don't see anything omap or platform specific in the API. Of course, if the platform/device doesn't support modifying the pin functions, then the function does nothing. > not just put that config on the bus master, dispc? I still can't see how > this is panel config. You are only pushing CDF API and meta data to > describe something that is only needed by one bus master. I have never The information about how the panel is connected (the wiring) has to be somewhere in the DT data. We could have the info in the DSI master or in the DSI slave. Or, in some platforms where the DSS is not involved in the muxing/config, the info could be totally separate, in the boards pinmuxing data. I think all those work ok for normal cases without hotplug. But if you have hotplug, then you need separate pinmuxing/config data for each panel. You could possibly have a list of panels in your DSI master node, containing the muxing data, but that sounds rather hacky, and also very hardcoded, i.e. you need to know each panel that is ever going to be connected. If, on the other hand, you have the info in the panel node, it "just works". When a new panel is connected, the relevant panel DT data comes from somewhere (it's not relevant where), and it tells the DSI master how it's connected. Something like this is probably needed for the BeagleBone capes, if you have happened to follow the discussion. Although it could be argued that the capes may perhaps be not runtime hotswappable, and thus the configuration can be done only once during boot after the cape has been probed. But I'd rather not design the API so that we prevent hot swapping. > seen any DSI slave that can change their pin config. And since there is Well, if omap is the only SoC/device out there that supports configuring the pin functions, and everybody is against the API, I'm not going to press it. But then again, I think similar configuration support may be needed even for the normal pinmuxing, even in the case where you can't reconfigure the DSI pin functions. You still need to mux the pins (perhaps, say, between DSI and a GPIO), depending on how many lanes the panel uses. In fact, speaking about all pins in general, I'm not very fond of having a static pinmuxing in the board DT data, handled by the board setup code. I think generally the pins should be muxed to safe-mode by the board setup code, and then configured to their proper function by the driver when it is initializing. > no generic hot plug detect of DSI panels, at least not before DSI bus is > available, I have to assume this probing it very platform specific. We > have some products that provide 1-2 gpios to specify what panel is > available, some use I2C sensor probing to then assume the panel plugged. Yes, the hotplug mechanism is platform/board specific. But that's not relevant here. > At least in the first step I don't think this type of hot plug should be > in the API. Then once the base panel driver is in we could discuss > different solutions for you hot plug scenario. Yes, as I said, I also think we shouldn't aim for hotplug in the v1. But we also shouldn't prevent it. Tomi signature.asc Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 49981] On HD6850, Power Profile doesn't change if 2 screen is attached.
https://bugs.freedesktop.org/show_bug.cgi?id=49981 --- Comment #28 from Alex Deucher --- Created attachment 74606 --> https://bugs.freedesktop.org/attachment.cgi?id=74606&action=edit fix multi-head stability This should fix stability problems with profiles and multi-head. -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [GMA500] Valid VCO frequency range on GMA500 platform?
Hi Patrik Patrik Jakobsson wrote: > The value of VCO_MIN comes from the Intel PRM for similar GPUs. > > Instead of changing VCO_MIN, could you try setting N_MIN=1, N_MAX=6 and > M1_MAX=22? I'll test it on my own hardware tomorrow. Thanks for your suggestion. With "N_MIN=1, N_MAX=6 and M1_MAX=22", i get a refresh rate of 59.9Hz. Dave ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: Use names of ioctls in debug traces
The intention here is to make the output of dmesg with full verbosity a
bit easier for a human to parse. This commit transforms:
[drm:drm_ioctl], pid=699, cmd=0x6458, nr=0x58, dev 0xe200, auth=1
[drm:drm_ioctl], pid=699, cmd=0xc010645b, nr=0x5b, dev 0xe200, auth=1
[drm:drm_ioctl], pid=699, cmd=0xc0106461, nr=0x61, dev 0xe200, auth=1
[drm:drm_ioctl], pid=699, cmd=0xc01c64ae, nr=0xae, dev 0xe200, auth=1
[drm:drm_mode_addfb], [FB:32]
[drm:drm_ioctl], pid=699, cmd=0xc0106464, nr=0x64, dev 0xe200, auth=1
[drm:drm_vm_open_locked], 0x7fd9302fe000,0x00a0
[drm:drm_ioctl], pid=699, cmd=0x400c645f, nr=0x5f, dev 0xe200, auth=1
[drm:drm_ioctl], pid=699, cmd=0xc00464af, nr=0xaf, dev 0xe200, auth=1
[drm:intel_crtc_set_config], [CRTC:3] [NOFB]
into:
[drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_THROTTLE
[drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_CREATE
[drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_SET_TILING
[drm:drm_ioctl], pid=699, dev=0xe200, auth=1, IOCTL_MODE_ADDFB
[drm:drm_mode_addfb], [FB:32]
[drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_MMAP_GTT
[drm:drm_vm_open_locked], 0x7fd9302fe000,0x00a0
[drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_SET_DOMAIN
[drm:drm_ioctl], pid=699, dev=0xe200, auth=1, DRM_IOCTL_MODE_RMFB
[drm:intel_crtc_set_config], [CRTC:3] [NOFB]
Signed-off-by: Chris Cummins
---
drivers/gpu/drm/drm_drv.c | 20 +---
include/drm/drmP.h| 3 ++-
2 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index be174ca..6480d92 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -57,7 +57,7 @@ static int drm_version(struct drm_device *dev, void *data,
struct drm_file *file_priv);
#define DRM_IOCTL_DEF(ioctl, _func, _flags) \
- [DRM_IOCTL_NR(ioctl)] = {.cmd = ioctl, .func = _func, .flags = _flags,
.cmd_drv = 0}
+ [DRM_IOCTL_NR(ioctl)] = {.cmd = ioctl, .func = _func, .flags = _flags,
.cmd_drv = 0, .name = #ioctl}
/** Ioctl table */
static struct drm_ioctl_desc drm_ioctls[] = {
@@ -376,7 +376,7 @@ long drm_ioctl(struct file *filp,
{
struct drm_file *file_priv = filp->private_data;
struct drm_device *dev;
- struct drm_ioctl_desc *ioctl;
+ struct drm_ioctl_desc *ioctl = NULL;
drm_ioctl_t *func;
unsigned int nr = DRM_IOCTL_NR(cmd);
int retcode = -EINVAL;
@@ -393,11 +393,6 @@ long drm_ioctl(struct file *filp,
atomic_inc(&dev->counts[_DRM_STAT_IOCTLS]);
++file_priv->ioctl_count;
- DRM_DEBUG("pid=%d, cmd=0x%02x, nr=0x%02x, dev 0x%lx, auth=%d\n",
- task_pid_nr(current), cmd, nr,
- (long)old_encode_dev(file_priv->minor->device),
- file_priv->authenticated);
-
if ((nr >= DRM_CORE_IOCTL_COUNT) &&
((nr < DRM_COMMAND_BASE) || (nr >= DRM_COMMAND_END)))
goto err_i1;
@@ -417,6 +412,11 @@ long drm_ioctl(struct file *filp,
} else
goto err_i1;
+ DRM_DEBUG("pid=%d, dev=0x%lx, auth=%d, %s\n",
+ task_pid_nr(current),
+ (long)old_encode_dev(file_priv->minor->device),
+ file_priv->authenticated, ioctl->name);
+
/* Do not trust userspace, use our own definition */
func = ioctl->func;
/* is there a local override? */
@@ -471,6 +471,12 @@ long drm_ioctl(struct file *filp,
}
err_i1:
+ if (!ioctl)
+ DRM_DEBUG("invalid iotcl: pid=%d, dev=0x%lx, auth=%d,
cmd=0x%02x, nr=0x%02x\n",
+ task_pid_nr(current),
+ (long)old_encode_dev(file_priv->minor->device),
+ file_priv->authenticated, cmd, nr);
+
if (kdata != stack_kdata)
kfree(kdata);
atomic_dec(&dev->ioctl_count);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index fad21c9..b55ba15 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -313,6 +313,7 @@ struct drm_ioctl_desc {
int flags;
drm_ioctl_t *func;
unsigned int cmd_drv;
+ const char *name;
};
/**
@@ -321,7 +322,7 @@ struct drm_ioctl_desc {
*/
#define DRM_IOCTL_DEF_DRV(ioctl, _func, _flags)\
- [DRM_IOCTL_NR(DRM_##ioctl)] = {.cmd = DRM_##ioctl, .func = _func,
.flags = _flags, .cmd_drv = DRM_IOCTL_##ioctl}
+ [DRM_IOCTL_NR(DRM_##ioctl)] = {.cmd = DRM_##ioctl, .func = _func,
.flags = _flags, .cmd_drv = DRM_IOCTL_##ioctl, .name = #ioctl}
struct drm_magic_entry {
struct list_head head;
--
1.8.1.2
-
Intel Corporation (UK) Limited
Registered No. 1134945 (England)
Registered Office: Pipers Way, Swindon SN3 1RJ
VAT No: 860 2173 47
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by ot
Re: [RFC PATCH 0/4] Common Display Framework-TF
On 02/11/2013 09:21 AM, Tomi Valkeinen wrote: On 2013-02-08 16:54, Marcus Lorentzon wrote: On 02/08/2013 03:02 PM, Tomi Valkeinen wrote: On 2013-02-08 15:28, Marcus Lorentzon wrote: When we do that we stop->setup->start during blanking. So our "DSS" is optimized to be able to do that without getting blocked. All DSI video mode panels (and DPI) products we have done so far have not had any issue with that (as long as DSI HS clock is set to continuous). I think this approach is less platform dependant, as long as there is no SoC that take more than a blanking period to reconfigure. So do you stop, setup and start the link with CPU, and this has to be happen during blanking? Isn't that prone to errors? Or did you mean that the hardware handles that automatically? In OMAP DSS there are so called shadow registers, that can be programmed at any time. The you set a bit (GO bit), which tells the hardware to take the new settings into use at the next vblank. From DSI driver's perspective the link is never stopped when reconfiguring the video timings. However, many other settings have to be configured when the link is disabled. Yeah, you lucky guys with the GO bit ;). No, we actually do CPU stop,setup,start. But since it is video mode, master is driving the sync so it is not a hard deadline. It is enough to restart before pixels start to degrade. On an LCD that is not so much time, but on an OLED it could be 10 secs :). Anyway, we have had several mass products with this soft solution and it has worked well. Ah, ok. But in that case what you said in an earlier mail is not quite correct: "I think this approach is less platform dependant, as long as there is no SoC that take more than a blanking period to reconfigure.". So in your approach the reconfiguration doesn't have to be done inside the blanking period, but before the panel's picture starts to fade? I don't know... It's early Monday morning, and not enough coffee yet, but I get a bit uneasy feeling if I think that your method of reconfiguring would be the only think allowed by the CDF API. Some SoCs do support reconfiguring on the fly, without disabling the link. It would not be nice if we didn't allow this to happen. And actually, we're not only talking about SoCs here, but also any display devices, like external buffer chips etc. They may also offer means to change configs on the fly. Well, I don't have any hard point about this at the moment, but I think we should think different approaches how the configuration can be done. Ok, so what about a compromise which I think would work for "both" HWs ... we allow the "configure" operation during video on, then each HW driver could decide if this means you have to stop or not to do the changes required. But then it is also important that we keep all config in one struct and not split it up. Otherwise HW like ours that has so be stopped could need to stop once for each setting/operation called. So in short, allow configure(bus_params) during video on, keep all bus_params in the struct. It is in kernel struct so it can easily be changed/refactored. I understand, but removing the omap prefix doesn't mean it has to go in the panel slave port/bus settings. I still can't see why this should be configuration on the panel driver and not the DSI master driver. Number of pins might be useful since you might start with one lane and then activate the rest. But partial muxing (pre pinmux) doesn't seem to be something the panel should control or know anything about. Sounds like normal platform/DT data per product/board. I think one case where this kind of pin configuration is needed, and which also dictates that all panel related configuration has to be in the panel's data, not in the DSI master's data, is hotplug. If you have a board that has two panels connected to the same video output, probably via some kind of mux, at least for the sensitive pins like DSI, only one of the panels can be enabled at a time. The panels can have different wiring, and thus the panel driver would need to configure everything related to the bus when it's starting up. The same also happens if you have a true hotplug, i.e. you can remove the panel totally and plug in a new one. Again the wiring can be different, and needs to be set up. And, as I said, this means that all relevant data about the video bus has to be in the panel's data, so that each panel can have its own set of, say, pin configuration. Hotplug is not a use case we should aim to support for the CDF v1, but I think we should strive not to prevent hotplug either. So if we can design the API so that hotplug is possible, I say let's do that. Again, this probing and bus muxing is platform/bus specific and not panel specific. Any panel of that type will only ever work on Omap (or someone else implementing the same muxing features) as I see it. So why not just put that config on the bus master, dispc? I still can't see how this is panel config. You are on
Re: [RFC PATCH 0/4] Common Display Framework-TF
On 2013-02-08 16:54, Marcus Lorentzon wrote: > On 02/08/2013 03:02 PM, Tomi Valkeinen wrote: >> On 2013-02-08 15:28, Marcus Lorentzon wrote: >> >>> When we do that we stop->setup->start during blanking. So our "DSS" is >>> optimized to be able to do that without getting blocked. All DSI video >>> mode panels (and DPI) products we have done so far have not had any >>> issue with that (as long as DSI HS clock is set to continuous). I think >>> this approach is less platform dependant, as long as there is no SoC >>> that take more than a blanking period to reconfigure. >> So do you stop, setup and start the link with CPU, and this has to be >> happen during blanking? Isn't that prone to errors? Or did you mean that >> the hardware handles that automatically? >> >> In OMAP DSS there are so called shadow registers, that can be programmed >> at any time. The you set a bit (GO bit), which tells the hardware to >> take the new settings into use at the next vblank. >> >> From DSI driver's perspective the link is never stopped when >> reconfiguring the video timings. However, many other settings have to be >> configured when the link is disabled. > > Yeah, you lucky guys with the GO bit ;). No, we actually do CPU > stop,setup,start. But since it is video mode, master is driving the sync > so it is not a hard deadline. It is enough to restart before pixels > start to degrade. On an LCD that is not so much time, but on an OLED it > could be 10 secs :). Anyway, we have had several mass products with this > soft solution and it has worked well. Ah, ok. But in that case what you said in an earlier mail is not quite correct: "I think this approach is less platform dependant, as long as there is no SoC that take more than a blanking period to reconfigure.". So in your approach the reconfiguration doesn't have to be done inside the blanking period, but before the panel's picture starts to fade? I don't know... It's early Monday morning, and not enough coffee yet, but I get a bit uneasy feeling if I think that your method of reconfiguring would be the only think allowed by the CDF API. Some SoCs do support reconfiguring on the fly, without disabling the link. It would not be nice if we didn't allow this to happen. And actually, we're not only talking about SoCs here, but also any display devices, like external buffer chips etc. They may also offer means to change configs on the fly. Well, I don't have any hard point about this at the moment, but I think we should think different approaches how the configuration can be done. > I understand, but removing the omap prefix doesn't mean it has to go in > the panel slave port/bus settings. I still can't see why this should be > configuration on the panel driver and not the DSI master driver. Number > of pins might be useful since you might start with one lane and then > activate the rest. But partial muxing (pre pinmux) doesn't seem to be > something the panel should control or know anything about. Sounds like > normal platform/DT data per product/board. I think one case where this kind of pin configuration is needed, and which also dictates that all panel related configuration has to be in the panel's data, not in the DSI master's data, is hotplug. If you have a board that has two panels connected to the same video output, probably via some kind of mux, at least for the sensitive pins like DSI, only one of the panels can be enabled at a time. The panels can have different wiring, and thus the panel driver would need to configure everything related to the bus when it's starting up. The same also happens if you have a true hotplug, i.e. you can remove the panel totally and plug in a new one. Again the wiring can be different, and needs to be set up. And, as I said, this means that all relevant data about the video bus has to be in the panel's data, so that each panel can have its own set of, say, pin configuration. Hotplug is not a use case we should aim to support for the CDF v1, but I think we should strive not to prevent hotplug either. So if we can design the API so that hotplug is possible, I say let's do that. Tomi signature.asc Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 4/5] drm/tegra: Implement VBLANK support
On Tue, Jan 22, 2013 at 06:37:39PM +0100, Mario Kleiner wrote:
> On 14.01.13 17:05, Thierry Reding wrote:
> >Implement support for the VBLANK IOCTL. Note that Tegra is somewhat
> >special in this case because it doesn't use the generic IRQ support
> >provided by the DRM core (DRIVER_HAVE_IRQ) but rather registers one
> >interrupt handler for each display controller.
> >
> >While at it, clean up the way that interrupts are enabled to ensure
> >that the VBLANK interrupt only gets enabled when required.
> >
> >Signed-off-by: Thierry Reding
>
> ... snip ...
>
> > struct drm_driver tegra_drm_driver = {
> > .driver_features = DRIVER_BUS_PLATFORM | DRIVER_MODESET | DRIVER_GEM,
> > .load = tegra_drm_load,
> >@@ -96,6 +136,10 @@ struct drm_driver tegra_drm_driver = {
> > .open = tegra_drm_open,
> > .lastclose = tegra_drm_lastclose,
> >
> >+.get_vblank_counter = drm_vblank_count,
>
> -> .get_vblank_counter = drm_vblank_count is a no-op.
>
> .get_vblank_counter() is supposed to return some real hardware
> vblank counter value to reinitialize the software vblank counter at
> vbl irq enable time. That software counter gets queried via
> drm_vblank_count(). If you hook this up to drm_vblank_count() it
> essentially returns a constant, frozen vblank count, it has the same
> effect as returning zero or any other constant value -- You lose all
> vblank counter increments during vblank irq off time. The same
> problem is present in nouveau-kms.
>
> I think it would be better to either implement a real hw counter
> query, or some function with a /* TODO: Implement me properly */
> comment which returns zero, so it is clear something is missing
> here.
I've finally managed to find some time to look into this some more. The
comment atop the drm_driver.get_vblank_counter() field actually suggests
that drivers should set this to drm_vblank_count if no real hardware
counter is supported.
Now it seems like we may get functionality to obtain the real VBLANK
counter once the syncpoint support is merged, so maybe we can leave this
as-is until then and replace it with a proper implementation at that
point in time?
Alternatively I could use a small wrapper with an explicit comment that
this should be implemented using the upcoming syncpoint support.
Thierry
pgpIljOxVCLqi.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 4/5] drm/tegra: Implement VBLANK support
On Wed, Jan 23, 2013 at 10:02:20AM +0200, Terje Bergström wrote: > On 22.01.2013 21:59, Mario Kleiner wrote: > > The current swap scheduling is based on having an accurate software > > vblank counter. Atm. that counter is maintained in software, incremented > > during vblank irq. At irq off -> on transition we need a hw counter to > > reinitialize. And there is a timeout between dropping the last reference > > to a counter via drm_vblank_put() and actually disabling the vblank irq. > > If the timeout is long enough and a timing sensitive app is aware that > > vblank counters may be inaccurate/unreliable over long time periods, it > > can work around the problem. > > We have a hardware counter that can be used as VBLANK counter - host1x > sync point register numbers 26 and 27. tegradrm already sets them up in > dc init (DC_CMD_CONT_SYNCPT_VSYNC). Basic syncpt support (read, wait) is > part of my patch set to implement 2D acceleration. Are the syncpoints incremented even if the VBLANK interrupts are turned off? If that's the case they could indeed be used as a hardware counter rather than the fake approach used right now. Maybe we should leave the code as it is right now and fix that up once the syncpoint support has been merged. Thierry pgpPLP3mU2EF1.pgp Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 5/5] drm/tegra: Implement page-flipping support
On Tue, Jan 22, 2013 at 06:27:24PM +0100, Mario Kleiner wrote:
> On 22.01.13 09:31, Terje Bergström wrote:
> >On 14.01.2013 18:06, Thierry Reding wrote:
> >>+static int tegra_dc_page_flip(struct drm_crtc *crtc, struct
> >>drm_framebuffer *fb,
> >>+ struct drm_pending_vblank_event *event)
> >>+{
> >>+ struct tegra_framebuffer *newfb = to_tegra_fb(fb);
> >>+ struct tegra_dc *dc = to_tegra_dc(crtc);
> >>+ struct drm_device *drm = crtc->dev;
> >>+ unsigned long flags;
> >>+
> >>+ if (dc->event)
> >>+ return -EBUSY;
>
> Hi
>
> I haven't read the Tegra programming manual or played with this, so
> maybe i'm wrong for some reason, but i think there is a race here
> between actual pageflip completion - latching newfb into the scanout
> base register and the completion routine that gets called from the
> vblank irq handler.
>
> If this code gets called close to vblank or inside vblank, couldn't
> it happen that tegra_dc_set_base() programs a pageflip that gets
> executed immediately - the new fb gets latched into the crtc, but
> the corresponding vblank irq handler for the vblank of flip
> completion runs before the "if (event)" block can set dc->event =
> event;? Or vblank irq's are off because drm_vblank_get() is only
> called at the end of the routine? In both cases the completion
> routine would miss the correct vblank and only timestamp and emit
> the completion event 1 vblank after actual flip completion.
>
> In that case it would be better to place tegra_dc_set_base() after
> the "if (event)" block and have some check inside the flip
> completion routine to make sure the pageflip completion event is
> only emitted if the scanout is really already latched with the
> newfb.
An easier solution might be to expand the scope of the lock a bit to
encompass the call to tegra_dc_set_base() and extend until the end of
tegra_dc_page_flip(). That should properly keep the page-flip completion
from interfering, right?
spin_lock_irqsave(&drm->event_lock, flags);
tegra_dc_set_base(dc, 0, 0, newfb);
if (event) {
event->pipe = dc->pipe;
dc->event = event;
drm_vblank_get(drm, dc->pipe);
}
spin_unlock_irqrestore(&drm->event_lock, flags);
Thierry
pgppxPLd1ehvS.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
