On Thu, Apr 5, 2012 at 4:21 PM, Ville Syrjälä <syrj...@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ä
> syrj...@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-de...@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
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

Reply via email to