[Mesa-dev] [PATCH 07/25] dri/nouveau: add GLX_MESA_query_renderer support
Emil Velikov
emil.l.velikov at gmail.com
Sat Feb 22 04:54:49 PST 2014
On 22/02/14 05:46, Ilia Mirkin wrote:
> On Fri, Feb 21, 2014 at 10:03 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> v2: nv04-2x cards support upto opengl 1.3.
>
> nv1x only has 1.2, iirc. i'll re-check. why does it matter though?
> version is computed based on extension availability...
>
Indeed, nv2x are capable of 1.3 whereas older cards are 1.2.
Version computation is possible whenever context is created. Whereas
GLX_MESA_query_renderer dictates that one should return the version of
the API they support even without ever creating one. So unless I'm
missing something going that road will require a major rework of both
classic mesa and gallium.
>> v3: Include correct headers.
>>
>> Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
>> ---
>> src/mesa/drivers/dri/nouveau/nouveau_screen.c | 83 ++++++++++++++++++++++++++-
>> 1 file changed, 82 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/drivers/dri/nouveau/nouveau_screen.c b/src/mesa/drivers/dri/nouveau/nouveau_screen.c
>> index a381064..fb92161 100644
>> --- a/src/mesa/drivers/dri/nouveau/nouveau_screen.c
>> +++ b/src/mesa/drivers/dri/nouveau/nouveau_screen.c
>> @@ -24,6 +24,8 @@
>> *
>> */
>>
>> +#include <xf86drm.h>
>> +#include <nouveau_drm.h>
>> #include "nouveau_driver.h"
>> #include "nouveau_context.h"
>> #include "nouveau_fbo.h"
>> @@ -118,13 +120,17 @@ nouveau_init_screen2(__DRIscreen *dri_screen)
>> /* Compat version validation will occur at context init after
>> * _mesa_compute_version().
>> */
>> - dri_screen->max_gl_compat_version = 15;
>> + dri_screen->max_gl_compat_version = 13;
>> + dri_screen->max_gl_core_version = 0;
>>
>> /* NV10 and NV20 can support OpenGL ES 1.0 only. Older chips
>> * cannot do even that.
>> */
>> if ((screen->device->chipset & 0xf0) != 0x00)
>> dri_screen->max_gl_es1_version = 10;
>> + else
>> + dri_screen->max_gl_es1_version = 0;
>> + dri_screen->max_gl_es2_version = 0;
>>
>> dri_screen->driverPrivate = screen;
>> dri_screen->extensions = nouveau_screen_extensions;
>> @@ -141,6 +147,80 @@ fail:
>>
>> }
>>
>> +static int
>> +nouveau_query_renderer_integer(__DRIscreen *psp, int param,
>> + unsigned int *value)
>> +{
>> + const struct nouveau_screen *const screen =
>> + (struct nouveau_screen *) psp->driverPrivate;
>> +
>> + switch (param) {
>> + case __DRI2_RENDERER_VENDOR_ID:
>> + value[0] = 0x10de;
>
> IMHO *value = 0x10de would read nicer (and similarly below). But feel
> free to keep it as is.
>
I'd rather keep this one as is, it's more explicit.
>> + return 0;
>> + case __DRI2_RENDERER_DEVICE_ID: {
>> + struct drm_nouveau_getparam gp;
>> + int *chip_id = 0, ret;
>
> aka int *chip_id = NULL. Of course that's probably not what you meant
> given how you use it below.
>
>> +
>> + memset(&gp, 0, sizeof(gp));
>> + gp.param = NOUVEAU_GETPARAM_PCI_DEVICE;
>> + gp.value = (unsigned long) chip_id;
>> +
>> + ret = drmCommandWriteRead(psp->fd, DRM_NOUVEAU_GETPARAM, &gp, sizeof(gp));
>> + if (ret) {
>> + nouveau_error("Error retrieving NOUVEAU_GETPARAM_PCI_DEVICE.\n");
>> + *chip_id = -1;
>> + }
>> + value[0] = *chip_id;
>
> Maybe use nouveau_getparam() from libdrm_nouveau for that? It's more
> concise and less of an abstraction violation, IMO. I think you
> shouldn't have to add any #include's then. Would also save you from
> the chip_id issue above.
>
Currently nouveau_vieux uses and links against both libdrm and
libdrm_nouveau, so I do not see how this will be a abstraction
violation. Although using nouveau_getparam() is cleaner and shorter way
of doing it, so I'll opt for it.
>> + return 0;
>> + }
>> + case __DRI2_RENDERER_ACCELERATED:
>> + value[0] = 1;
>> + return 0;
>> + case __DRI2_RENDERER_VIDEO_MEMORY:
>> + /* XXX: return vram_size, vram_limit, gart_size or gart_limit ? */
>> + value[0] = screen->device->vram_size >> 20;
>> + return 0;
>> + case __DRI2_RENDERER_UNIFIED_MEMORY_ARCHITECTURE:
>> + value[0] = 0;
>> + return 0;
>> + case __DRI2_RENDERER_PREFERRED_PROFILE:
>> + value[0] = (1U << __DRI_API_OPENGL);
>
> Unnecessary parens. Unnecessary U too, actually, but whatever.
>
Actually one can move this to driQueryRendererIntegerCommon, considering
that every driver opts for opengl core if available and then falls back
to opengl compat(legacy) as their preferred profile.
>> + return 0;
>> + default:
>> + return driQueryRendererIntegerCommon(psp, param, value);
>> + }
>> +
>> + return -1;
>
> Can't happen, right? I'd just as soon remove it -- the compiler
> shouldn't complain.
>
Fair enough.
Thanks for having a look.
Emil
>> +}
>> +
>> +static int
>> +nouveau_query_renderer_string(__DRIscreen *psp, int param, const char **value)
>> +{
>> + const struct nouveau_screen *const screen =
>> + (struct nouveau_screen *) psp->driverPrivate;
>> +
>> + switch (param) {
>> + case __DRI2_RENDERER_VENDOR_ID:
>> + value[0] = nouveau_vendor_string;
>> + return 0;
>> + case __DRI2_RENDERER_DEVICE_ID:
>> + value[0] = nouveau_get_renderer_string(screen->device->chipset);
>> + return 0;
>> + default:
>> + break;
>> + }
>> +
>> + return -1;
>> +}
>> +
>> +static const __DRI2rendererQueryExtension nouveau_renderer_query_extension = {
>> + .base = { __DRI2_RENDERER_QUERY, 1 },
>> +
>> + .queryInteger = nouveau_query_renderer_integer,
>> + .queryString = nouveau_query_renderer_string
>> +};
>> +
>> static void
>> nouveau_destroy_screen(__DRIscreen *dri_screen)
>> {
>> @@ -241,6 +321,7 @@ static const struct __DRItexBufferExtensionRec nouveau_texbuffer_extension = {
>> static const __DRIextension *nouveau_screen_extensions[] = {
>> &nouveau_flush_extension.base,
>> &nouveau_texbuffer_extension.base,
>> + &nouveau_renderer_query_extension.base,
>> &dri2ConfigQueryExtension.base,
>> NULL
>> };
>> --
>> 1.9.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list