Re: [PATCH v3 1/6] drm/fourcc: Add char_per_block, block_w and block_h in drm_format_info

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Oct 11, 2018 at 2:53 PM Liviu Dudau <Liviu.Dudau@xxxxxxx> wrote:
> On Thu, Oct 11, 2018 at 02:40:10PM +0200, Daniel Vetter wrote:
> > On Thu, Oct 11, 2018 at 12:11 PM Liviu Dudau <Liviu.Dudau@xxxxxxx> wrote:
> > > On Thu, Oct 11, 2018 at 10:58:30AM +0100, Alexandru-Cosmin Gheorghe wrote:
> > > > On Thu, Oct 11, 2018 at 10:29:42AM +0200, Daniel Vetter wrote:
> > > > > On Mon, Oct 08, 2018 at 09:52:04AM +0000, Alexandru-Cosmin Gheorghe wrote:
> > > > > > > One question I have is validating this. I think a bunch of unit tests,
> > > > > > > integrated into the existing kms selftests we already (so that
> > > > > > > intel-gfx-ci and other CI can run it) would be great. Both for the small
> > > > > > > helper functions (block width/height, but especially drm_pitch), but also
> > > > > > > for the drm_framebuffer_create functions, so that we can exercise the
> > > > > > > metric pile of validation corner cases.
> > > > > > > -Daniel
> > > > > >
> > > > > > So, you are thinking of adding tests here drivers/gpu/drm/selftests/ ?
> > > > > > Looking inside the folder, it seems more like a stub than proper
> > > > > > test suite for the drm framework, or am I missing something ?
> > > > >
> > > > > It's indeed very incomplete still.
> > > > >
> > > > > > I did run tests for all supported formats by the mali-dp, with our
> > > > > > internal testsuite and everything was OK for all the fourcc enabled
> > > > > > in mali-dp
> > > > >
> > > > > Your internal test suite is to no value to the overall community :-) Can
> > > > > we get those upstreamed somewhere, ideally as part of igt?
> > >
> > > On balance of things, how many more drivers are we going to cover when
> > > going from internal test suite to igt? +1 (intel)? +3 (finger in the
> > > air)? By looking at the commit log is hard to judge the traction of igt in
> > > the "overall community".
> >
> > With my intel hat on I don't particularly care how you test mali, and
> > I don't think you folks care how we test intel. (Ok maybe there's a
> > shared interest in having high quality upstream drivers so that OS
> > people can upgrade without fear of regressions across any of the hw
> > vendors, but that's a long story and somewhat an aside).
> >
> > But there's piles of shared infrastructure, and that's really where I
> > think a common test suite, ideally run as part of drm core CI (I'm
> > working on that, doesn't exist yet). That's where the shared CI
> > efforts really pay off imo. There's also some value in trying to make
> > sure all drivers work the same across all drivers. Both of these
> > pieces need to be there to be able to refactor the drm subsystem
> > aggressively, which we'll need to do because hw is changing all the
> > time. If you don't care about any of this, then pushing your driver to
> > upstream doesn't make a hole lot of sense imo :-)
>
> Nah, I wasn't looking to start a holy war, it was a genuine question. I
> don't remember exactly the timeline but I do admit that I was a bit
> puzzled (confused?) by the appearance of drivers/gpu/drm/selftests given
> the existence of igt, so I though one of them is the "newer" thing to
> support. I was on the fence on which one is worth backing, hence the
> quantitative question (how many more drivers actively get tested with
> igt?).

They're two sides of the same coin really:
- igt does (mostly) blackbox testing from userspace, with very minimal
ties into the driver through debugfs.
- selftests run within the kernel, have deep knowledge of the
implementation (not just the uapi), and are mostly unit tests of
individual bits&pieces.

Currently we need some minimal wrappers in igt to be able to run the
kernel selftests using igt (so that it's all under one overall testing
umbrella). But the ksefltests can also be run without igt.

Doing the selftests in igt would unnecessarily tie igt to the kernel,
a massive maintenance headaches. And we want to be able to run igt
uapi tests on older/newer kernels, so that we can make sure we don't
break back/forward-compatibility. So merging igt into the kernel
(which some othe tests-suites have done to make maintenanace easier)
doesn't make much sense imo.

Depending upon what you're doing it's better to pick igt or selftests
(or both, for different aspects) as the best way to create
regression-tests for your new feature. For entirely internal work
(like the new block_* description for pixel formats) I think in-kernel
unit-tests are the most effective way to regression test them, hence
my recommendation to look into selftests.

For new uapi you generally want some igts, but perhaps augmented with
self-tests for tricky internal implementation details. That was my
recommendation for the dirty rectangle, where the validation code is
an igt testcase and a bunch of self-tests.

> I know that I have used igt to test mali-dp before submission, we also
> had some patches that need refreshing to add writeback support (sorry
> for being slow on those), but I haven't seen people contributing in
> earnest to igt support for their drivers (vc4 is the newest, amdgpu from
> a few months ago, I thought there was some adreno stuff but I can't find
> right now). So for the passer-bys (aka managers) it is hard (sometimes)
> to see the benefit of investing effort in something that doesn't look
> like a clear winner.

Igt is supposed to run on any kms driver without changes. There's no
need to add special code in there. The vc4/amdgpu/freedreno code is
for the gem side, where you do need special code to test-drive the
driver-private ioctl interfaces.

Now of course it's not perfect, and occasionally we need to remove
some intel-ism. But e.g. just recently vmwgfx people made a small
patch series to be able to run igt on vmwgfx, and it was very minimal.
And vmwgfx is probably the most "special" kms driver we have in-tree,
so I think we're doing a fairly good job overall.

> Maybe igt could use some "Supported drivers" section in the README so we
> can point it out to our decision makers?

All of them, or at least all the kms ones? Or are you more looking for
"actively supported", as in people running them in some CI regularly?
The answer for the later I frankly don't even know ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux