On 2018-05-03 08:31 PM, sunpeng.li at amd.com wrote:
> From: "Leo (Sunpeng) Li" <sunpeng.li at amd.com>
>
> Push staged values on the driver-private CRTC, to kernel DRM when it's
> initialized. This is to flush out any previous state that hardware was
> in, and set them to their default values.
>
> Signed-off-by: Leo (Sunpeng) Li <sunpeng.li at amd.com>
> ---
> src/drmmode_display.c | 136 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 136 insertions(+)
>
> diff --git a/src/drmmode_display.c b/src/drmmode_display.c
> index 0ffc6ad..85de01e 100644
> --- a/src/drmmode_display.c
> +++ b/src/drmmode_display.c
> @@ -824,6 +824,133 @@ err_allocs:
> return 0;
> }
>
> +/**
> + * Query DRM for the property ID - as recognized by DRM - for the specified
> + * color management property, on the specified CRTC.
> + *
> + * @crtc: The CRTC to query DRM properties on.
> + * @prop_id: Color management property enum.
> + *
> + * Return the DRM property ID, if the property exists. 0 otherwise.
> + */
> +static uint32_t get_drm_cm_prop_id(xf86CrtcPtr crtc,
> + enum drmmode_cm_prop prop_id)
> +{
> + AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(crtc->scrn);
> + drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
> + drmModeObjectPropertiesPtr drm_props;
> + int i;
> +
> + drm_props = drmModeObjectGetProperties(pAMDGPUEnt->fd,
> + drmmode_crtc->mode_crtc->crtc_id,
> + DRM_MODE_OBJECT_CRTC);
> + if (!drm_props)
> + goto err_allocs;
> +
> + for (i = 0; i < drm_props->count_props; i++) {
> + drmModePropertyPtr drm_prop;
> +
> + drm_prop = drmModeGetProperty(pAMDGPUEnt->fd,
> + drm_props->props[i]);
> + if (!drm_prop)
> + goto err_allocs;
> +
> + if (get_cm_enum_from_str(drm_prop->name) == prop_id){
> + drmModeFreeProperty(drm_prop);
> + return drm_props->props[i];
> + }
> +
> + drmModeFreeProperty(drm_prop);
> + }
> +
> +err_allocs:
> + drmModeFreeObjectProperties(drm_props);
> + return 0;
> +}
It seems a bit heavy to call drmModeObjectGetProperties and
drmModeGetProperty (which both in turn call into the kernel) every time
here. Assuming the property IDs don't change at runtime, they could be
cached.
> +/**
> + * Push staged color management properties on the CRTC to DRM.
> + *
> + * @crtc: The CRTC containing staged properties
> + * @cm_prop_index: The color property to push
> + *
> + * Return 0 on success, X-defined error codes on failure.
> + */
> +static int drmmode_crtc_push_cm_prop(xf86CrtcPtr crtc,
> + enum drmmode_cm_prop cm_prop_index)
> +{
> + AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(crtc->scrn);
> + drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
> + size_t expected_bytes = 0;
> + uint32_t created_blob_id = 0;
> + void *blob_data = NULL;
> + uint32_t drm_prop_id;
> + Bool free_blob_data = FALSE;
free_blob_data is always FALSE. If that's intended, it shouldn't be
added (in this patch yet).
> @@ -1458,6 +1586,14 @@ drmmode_crtc_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, drmModeResPtr mode_res
> drmmode_crtc->ctm->matrix[0] = drmmode_crtc->ctm->matrix[4] =
> drmmode_crtc->ctm->matrix[8] = (uint64_t)1 << 32;
>
> + /* Push properties to initialize them */
> + for (i = 0; i < CM_NUM_PROPS; i++) {
> + if (i == CM_DEGAMMA_LUT_SIZE || i == CM_GAMMA_LUT_SIZE)
> + continue;
> + if (drmmode_crtc_push_cm_prop(crtc, i))
> + return 0;
Per my follow-up to the cover letter, I don't think this should be
necessary here anyway, but FWIW:
Returning 0 early here breaks the pAMDGPUEnt->assigned_crtcs related
logic in this function and drmmode_pre_init.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
