On 2018-03-26 10:00 PM, sunpeng...@amd.com wrote:
> From: "Leo (Sunpeng) Li" <sunpeng...@amd.com>
> 
> This change adds a few functions in preparation of enabling CRTC color
> managment via the randr interface.
> 
> The driver-private CRTC object now contains a list of properties,
> mirroring the driver-private output object. The lifecycle of the CRTC
> properties will also mirror the output.
> 
> Since color managment properties are all DRM blobs, we'll expose the
> ability to change the blob ID. The user can create blobs via libdrm
> (which can be done without ownership of DRM master), then set the ID via
> xrandr. The user will then have to ensure proper cleanup by subsequently
> releasing the blob.

That sounds a bit clunky. :)

When changing a blob ID, the change only takes effect on the next atomic
commit, doesn't it? How does the client trigger the atomic commit?


> @@ -1604,6 +1623,18 @@ static void drmmode_output_dpms(xf86OutputPtr output, 
> int mode)
>       }
>  }
>  
> +static Bool drmmode_crtc_property_ignore(drmModePropertyPtr prop)
> +{
> +     if (!prop)
> +             return TRUE;
> +     /* Ignore CRTC gamma lut sizes */
> +     if (!strcmp(prop->name, "GAMMA_LUT_SIZE") ||
> +         !strcmp(prop->name, "DEGAMMA_LUT_SIZE"))
> +             return TRUE;

Without these properties, how can a client know the LUT sizes?


> @@ -1618,6 +1649,163 @@ static Bool 
> drmmode_property_ignore(drmModePropertyPtr prop)
>       return FALSE;
>  }
>  
> +/**
> +* Configure and change the given output property through randr. Currently

"RandR"

> +* ignores DRM_MODE_PROP_ENU property types. Used as part of create_resources.

DRM_MODE_PROP_ENUM is missing the final M.

> +*
> +* Return: 0 on success, X-defined error codes on failure.
> +*/
> +static int __rr_configure_and_change_property(xf86OutputPtr output,
> +                                           drmmode_prop_ptr pmode_prop)

No leading underscores in function names please.


> +     }
> +     else if (mode_prop->flags & DRM_MODE_PROP_RANGE) {

The else should be on the same line as }.


> +static void drmmode_crtc_create_resources(xf86CrtcPtr crtc,
> +                                       xf86OutputPtr output)
> +{
> +     AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(crtc->scrn);
> +     int i, j;
> +
> +     /* 'p' prefix for driver private objects */
> +     drmmode_crtc_private_ptr pmode_crtc = crtc->driver_private;

Existing code refers to this as drmmode_crtc, please stick to that.


> +     drmModeCrtcPtr mode_crtc = pmode_crtc->mode_crtc;
> +
> +     drmmode_prop_ptr pmode_prop;
> +     drmModePropertyPtr mode_prop;
> +
> +     /* Get list of DRM CRTC properties, and their values */
> +     drmModeObjectPropertiesPtr mode_props;

All local variable declarations should be in a single block, with no
blank lines between them, and generally sorted from longer lines to
shorter ones.


> +     mode_props = drmModeObjectGetProperties(pAMDGPUEnt->fd,
> +                                             mode_crtc->crtc_id,
> +                                             DRM_MODE_OBJECT_CRTC);
> +     if (!mode_props)
> +             goto err_allocs;
> +
> +     /* Allocate, then populate the driver-private CRTC property list */
> +     pmode_crtc->props = calloc(mode_props->count_props + 1,
> +                                  sizeof(drmmode_prop_rec));

Continuation lines should be aligned to opening parens. Any editor which
supports EditorConfig should do this automagically.


> +     if (!pmode_crtc->props)
> +             goto err_allocs;
> +
> +     pmode_crtc->num_props = 0;
> +
> +     /* Filter through drm crtc properties for relevant ones, and save
> +      * them */

        /* Multi-line comments should be formatted like this.
         * Comment end marker on a separate line:
         */


> +err_allocs:
> +     xf86DrvMsg(output->scrn->scrnIndex, X_ERROR,
> +                "Memory error creating resources for CRTC %d\n",
> +                mode_crtc->crtc_id);
> +     drmModeFreeObjectProperties(mode_props);
> +     return;
> +}

Remove the superfluous return statement at the end of a function
returning void.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to