* Re: drm: Branch 'master' - 5 commits
[not found] <20120403215001.C45FA1004B@kemper.freedesktop.org>
@ 2012-04-05 21:21 ` Ville Syrjälä
2012-04-05 23:00 ` Rob Clark
2012-04-05 23:08 ` Rob Clark
0 siblings, 2 replies; 3+ messages in thread
From: Ville Syrjälä @ 2012-04-05 21:21 UTC (permalink / raw)
To: dri-devel; +Cc: rob.clark
On Tue, Apr 03, 2012 at 02:50:01PM -0700, Rob Clark wrote:
> + /* just testing a limited # of formats to test single
> + * and multi-planar path.. would be nice to add more..
> + */
> + if (!strcmp(p->format_str, "YUYV")) {
> + pitches[0] = p->w * 2;
> + offsets[0] = 0;
> + kms_bo_get_prop(plane_bo, KMS_HANDLE, &handles[0]);
> +
> + fill422(virtual, 0, p->w, p->h, pitches[0]);
> +
> + format = DRM_FORMAT_YUYV;
> + } else if (!strcmp(p->format_str, "NV12")) {
> + pitches[0] = p->w;
> + offsets[0] = 0;
> + kms_bo_get_prop(plane_bo, KMS_HANDLE, &handles[0]);
> + pitches[1] = p->w;
> + offsets[1] = p->w * p->h;
> + kms_bo_get_prop(plane_bo, KMS_HANDLE, &handles[1]);
> +
> + fill420(virtual, virtual+offsets[1], virtual+offsets[1]+1,
> + 2, 0, p->w, p->h, pitches[0]);
> +
> + format = DRM_FORMAT_NV12;
> + } else if (!strcmp(p->format_str, "YV12")) {
> + pitches[0] = p->w;
> + offsets[0] = 0;
> + kms_bo_get_prop(plane_bo, KMS_HANDLE, &handles[0]);
> + pitches[1] = p->w / 2;
> + offsets[1] = p->w * p->h;
> + kms_bo_get_prop(plane_bo, KMS_HANDLE, &handles[1]);
> + pitches[2] = p->w / 2;
> + offsets[2] = offsets[1] + (p->w * p->h) / 4;
> + kms_bo_get_prop(plane_bo, KMS_HANDLE, &handles[1]);
^
Should be '2'. The kernel patch I just posted might have caught this.
OTOH it might not have in case handles[2] contains uninitialized data.
We should add a test that would make sure that passing an invalid bo
handle for any plane would return with an error. The problem is knowing
what exactly is an invalid handle since it's all driver specific.
Perhaps 0xffffffff would be a reasonably safe choice. Hmm, now I wonder
if 0 might actually be a valid handle for some of the current drivers...
There should obviously also be a test that does use separate bos
for reach plane. That should either succeed and produce the correct
result, or the driver should respond with an error at least to the
setplane ioctl. Whether it would allow addfb2 to succeed is a slightly
more complicated matter. I suppose it could be possible that on some
odd hardware some planes would support multiple bos and some would
not. In which case the driver would need to allow the addfb2.
--
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/
------------------------------------------------------------------------------
Better than sec? Nothing is better than sec when it comes to
monitoring Big Data applications. Try Boundary one-second
resolution app monitoring today. Free.
http://p.sf.net/sfu/Boundary-dev2dev
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: drm: Branch 'master' - 5 commits
2012-04-05 21:21 ` drm: Branch 'master' - 5 commits Ville Syrjälä
@ 2012-04-05 23:00 ` Rob Clark
2012-04-05 23:08 ` Rob Clark
1 sibling, 0 replies; 3+ messages in thread
From: Rob Clark @ 2012-04-05 23:00 UTC (permalink / raw)
To: dri-devel, rob.clark
On Thu, Apr 5, 2012 at 4:21 PM, Ville Syrjälä <syrjala@sci.fi> wrote:
> On Tue, Apr 03, 2012 at 02:50:01PM -0700, Rob Clark wrote:
>> + /* just testing a limited # of formats to test single
>> + * and multi-planar path.. would be nice to add more..
>> + */
>> + if (!strcmp(p->format_str, "YUYV")) {
>> + pitches[0] = p->w * 2;
>> + offsets[0] = 0;
>> + kms_bo_get_prop(plane_bo, KMS_HANDLE, &handles[0]);
>> +
>> + fill422(virtual, 0, p->w, p->h, pitches[0]);
>> +
>> + format = DRM_FORMAT_YUYV;
>> + } else if (!strcmp(p->format_str, "NV12")) {
>> + pitches[0] = p->w;
>> + offsets[0] = 0;
>> + kms_bo_get_prop(plane_bo, KMS_HANDLE, &handles[0]);
>> + pitches[1] = p->w;
>> + offsets[1] = p->w * p->h;
>> + kms_bo_get_prop(plane_bo, KMS_HANDLE, &handles[1]);
>> +
>> + fill420(virtual, virtual+offsets[1], virtual+offsets[1]+1,
>> + 2, 0, p->w, p->h, pitches[0]);
>> +
>> + format = DRM_FORMAT_NV12;
>> + } else if (!strcmp(p->format_str, "YV12")) {
>> + pitches[0] = p->w;
>> + offsets[0] = 0;
>> + kms_bo_get_prop(plane_bo, KMS_HANDLE, &handles[0]);
>> + pitches[1] = p->w / 2;
>> + offsets[1] = p->w * p->h;
>> + kms_bo_get_prop(plane_bo, KMS_HANDLE, &handles[1]);
>> + pitches[2] = p->w / 2;
>> + offsets[2] = offsets[1] + (p->w * p->h) / 4;
>> + kms_bo_get_prop(plane_bo, KMS_HANDLE, &handles[1]);
> ^
> Should be '2'. The kernel patch I just posted might have caught this.
> OTOH it might not have in case handles[2] contains uninitialized data.
>
Good catch, I can push a fix. If you already have a patch, please
send, otherwise I'll just go ahead and fix it myself.
fwiw, on omap4xxx we only support 1 and 2 plane formats, so 3 plane
was untested.. but I wanted to push the test at least so we could have
some common test code between the different platforms and catch any
potential mismatches in understanding (for ex. the src coords in 16.16
format vs integer)
> We should add a test that would make sure that passing an invalid bo
> handle for any plane would return with an error. The problem is knowing
> what exactly is an invalid handle since it's all driver specific.
> Perhaps 0xffffffff would be a reasonably safe choice. Hmm, now I wonder
> if 0 might actually be a valid handle for some of the current drivers...
I guess *nearly* everyone uses GEM? I suppose we could have a
validate_handle fxn ptr w/ a default drm_gem_validate_handle() helper
that any of the drivers using GEM could use. I'm not sure if that is
just overkill (since the drm_gem_object_lookup() would anyways fail in
the driver part).
On the other hand moving something like the objects_lookup() helper in
omapdrm into core gem code might not be a bad idea. This way each
driver isn't having to handle the cleanup when it gets only part way
thru object lookup when it encounters an invalid handle.
> There should obviously also be a test that does use separate bos
> for reach plane. That should either succeed and produce the correct
> result, or the driver should respond with an error at least to the
> setplane ioctl. Whether it would allow addfb2 to succeed is a slightly
> more complicated matter. I suppose it could be possible that on some
> odd hardware some planes would support multiple bos and some would
> not. In which case the driver would need to allow the addfb2.
Yeah, there certainly is room for more tests, especially because there
are so many possible permutations. Possibly we should split out into
'planetest' before modetest gets too big? Anyways, I figured at least
one basic test was better than zero ;-)
BR,
-R
> --
> Ville Syrjälä
> syrjala@sci.fi
> http://www.sci.fi/~syrjala/
>
> ------------------------------------------------------------------------------
> Better than sec? Nothing is better than sec when it comes to
> monitoring Big Data applications. Try Boundary one-second
> resolution app monitoring today. Free.
> http://p.sf.net/sfu/Boundary-dev2dev
> --
> _______________________________________________
> Dri-devel mailing list
> Dri-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
------------------------------------------------------------------------------
For Developers, A Lot Can Happen In A Second.
Boundary is the first to Know...and Tell You.
Monitor Your Applications in Ultra-Fine Resolution. Try it FREE!
http://p.sf.net/sfu/Boundary-d2dvs2
--
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: drm: Branch 'master' - 5 commits
2012-04-05 21:21 ` drm: Branch 'master' - 5 commits Ville Syrjälä
2012-04-05 23:00 ` Rob Clark
@ 2012-04-05 23:08 ` Rob Clark
1 sibling, 0 replies; 3+ messages in thread
From: Rob Clark @ 2012-04-05 23:08 UTC (permalink / raw)
To: dri-devel, Ville Syrjälä
hmm, let's try that again with correct list addr this time..
On Thu, Apr 5, 2012 at 4:21 PM, Ville Syrjälä <syrjala@sci.fi> wrote:
> On Tue, Apr 03, 2012 at 02:50:01PM -0700, Rob Clark wrote:
>> + /* just testing a limited # of formats to test single
>> + * and multi-planar path.. would be nice to add more..
>> + */
>> + if (!strcmp(p->format_str, "YUYV")) {
>> + pitches[0] = p->w * 2;
>> + offsets[0] = 0;
>> + kms_bo_get_prop(plane_bo, KMS_HANDLE, &handles[0]);
>> +
>> + fill422(virtual, 0, p->w, p->h, pitches[0]);
>> +
>> + format = DRM_FORMAT_YUYV;
>> + } else if (!strcmp(p->format_str, "NV12")) {
>> + pitches[0] = p->w;
>> + offsets[0] = 0;
>> + kms_bo_get_prop(plane_bo, KMS_HANDLE, &handles[0]);
>> + pitches[1] = p->w;
>> + offsets[1] = p->w * p->h;
>> + kms_bo_get_prop(plane_bo, KMS_HANDLE, &handles[1]);
>> +
>> + fill420(virtual, virtual+offsets[1], virtual+offsets[1]+1,
>> + 2, 0, p->w, p->h, pitches[0]);
>> +
>> + format = DRM_FORMAT_NV12;
>> + } else if (!strcmp(p->format_str, "YV12")) {
>> + pitches[0] = p->w;
>> + offsets[0] = 0;
>> + kms_bo_get_prop(plane_bo, KMS_HANDLE, &handles[0]);
>> + pitches[1] = p->w / 2;
>> + offsets[1] = p->w * p->h;
>> + kms_bo_get_prop(plane_bo, KMS_HANDLE, &handles[1]);
>> + pitches[2] = p->w / 2;
>> + offsets[2] = offsets[1] + (p->w * p->h) / 4;
>> + kms_bo_get_prop(plane_bo, KMS_HANDLE, &handles[1]);
> ^
> Should be '2'. The kernel patch I just posted might have caught this.
> OTOH it might not have in case handles[2] contains uninitialized data.
Good catch, I can push a fix. If you already have a patch, please
send, otherwise I'll just go ahead and fix it myself.
fwiw, on omap4xxx we only support 1 and 2 plane formats, so 3 plane
was untested.. but I wanted to push the test at least so we could have
some common test code between the different platforms and catch any
potential mismatches in understanding (for ex. the src coords in 16.16
format vs integer)
> We should add a test that would make sure that passing an invalid bo
> handle for any plane would return with an error. The problem is knowing
> what exactly is an invalid handle since it's all driver specific.
> Perhaps 0xffffffff would be a reasonably safe choice. Hmm, now I wonder
> if 0 might actually be a valid handle for some of the current drivers...
I guess *nearly* everyone uses GEM? I suppose we could have a
validate_handle fxn ptr w/ a default drm_gem_validate_handle() helper
that any of the drivers using GEM could use. I'm not sure if that is
just overkill (since the drm_gem_object_lookup() would anyways fail in
the driver part).
On the other hand moving something like the objects_lookup() helper in
omapdrm into core gem code might not be a bad idea. This way each
driver isn't having to handle the cleanup when it gets only part way
thru object lookup when it encounters an invalid handle.
> There should obviously also be a test that does use separate bos
> for reach plane. That should either succeed and produce the correct
> result, or the driver should respond with an error at least to the
> setplane ioctl. Whether it would allow addfb2 to succeed is a slightly
> more complicated matter. I suppose it could be possible that on some
> odd hardware some planes would support multiple bos and some would
> not. In which case the driver would need to allow the addfb2.
Yeah, there certainly is room for more tests, especially because there
are so many possible permutations. Possibly we should split out into
'planetest' before modetest gets too big? Anyways, I figured at least
one basic test was better than zero ;-)
BR,
-R
> --
> Ville Syrjälä
> syrjala@sci.fi
> http://www.sci.fi/~syrjala/
>
> ------------------------------------------------------------------------------
> Better than sec? Nothing is better than sec when it comes to
> monitoring Big Data applications. Try Boundary one-second
> resolution app monitoring today. Free.
> http://p.sf.net/sfu/Boundary-dev2dev
> --
> _______________________________________________
> Dri-devel mailing list
> Dri-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-04-05 23:09 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20120403215001.C45FA1004B@kemper.freedesktop.org>
2012-04-05 21:21 ` drm: Branch 'master' - 5 commits Ville Syrjälä
2012-04-05 23:00 ` Rob Clark
2012-04-05 23:08 ` Rob Clark
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.