Eric Engestrom <eric.engestrom@xxxxxxxxxx> writes:
> copy/paste error: s/drm/display/
That's actually intentional -- any system which supports 'drm' can
support the display backend as it's based on that. Maybe it should use a
different test? (note that I haven't been using the autotools backend
recently, so it may not be great at this point. At least my current tree
builds?)
>> +if with_platform_display
>> + radv_flags += [
>> + '-DVK_USE_PLATFORM_DISPLAY_KHR',
>> + ]
>
> Nit: this can be a simple
> radv_flags += '-DVK_USE_PLATFORM_DISPLAY_KHR'
>
>> +if with_platform_display
>> + vulkan_wsi_args += [
>> + '-DVK_USE_PLATFORM_DISPLAY_KHR',
>> + ]
>
> Ditto:
> vulkan_wsi_args += '-DVK_USE_PLATFORM_DISPLAY_KHR'
Oh, good point -- I'd split out the RANDR and DISPLAY bits but didn't
simplify the meson stuff.
> Nit: src/amd/vulkan/ uses tabs for indent, you used spaces.
Thanks; I'll reconfigure my environment so that it uses tabs in that
directory, and re-indent the changes.
>> +#define MM_PER_PIXEL (1.0/96.0 * 25.4)
>
> unused
Good catch; those got left in both anv and radv directories after some
refactoring.
>> +#if 0
>
> `#if DEBUG` maybe?
Could do; I could also just strip out the printf debugging, but when
you're doing timing-sensitive stuff at that level, printf debugging is
pretty useful.
>> +#define wsi_display_debug(...) fprintf(stderr, __VA_ARGS__)
>> +#define wsi_display_debug_code(...) __VA_ARGS__
>
> that 2nd one is unused
It gets used in a later patch.
>> + if (wsi) {
>
> if (!wsi) return;
> and carry on without the extra indent
Yeah, would probably look cleaner.
> I don't know enough for this to be an actual review though, but the rest
> of this file looks reasonable to me :)
Thanks for reviewing the basic formatting and structure though; I
totally missed the tabs/spaces issue.
--
-keith
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
