On 2018-03-26 10:00 PM, sunpeng.li at amd.com wrote:
> From: "Leo (Sunpeng) Li" <sunpeng.li at 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
