[Mesa-dev] [PATCH 07/25] dri/nouveau: add GLX_MESA_query_renderer support
Ilia Mirkin
imirkin at alum.mit.edu
Sat Feb 22 11:49:22 PST 2014
On Sat, Feb 22, 2014 at 7:54 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> 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.
OK. It implies that value is an array, and you're setting the first
element. If that's the way things really are, totally agreed.
>
>>> + 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.
It links against a lot of things. That has nothing to do with it. No
code in mesa's nouveau dri/gallium drivers makes any drmCommand* calls
directly. It's all hidden behind helpers in libdrm_nouveau (like
nouveau_getparam, and many others). That's what I meant by abstraction
violation.
>
>>> + 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