From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marius Vlad Subject: Re: [PATCH i-g-t 2/7] lib/igt_kms: refactor atomic properties load/commit Date: Wed, 23 Mar 2016 14:13:50 +0200 Message-ID: <20160323121350.GA31168@mcvlad-wk.rb.intel.com> References: <1458733090-25768-1-git-send-email-lionel.g.landwerlin@intel.com> <1458733090-25768-3-git-send-email-lionel.g.landwerlin@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0745086339==" Return-path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by gabe.freedesktop.org (Postfix) with ESMTP id 0AB3D6E7E3 for ; Wed, 23 Mar 2016 12:12:44 +0000 (UTC) In-Reply-To: <1458733090-25768-3-git-send-email-lionel.g.landwerlin@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Lionel Landwerlin Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============0745086339== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="LQksG6bCIzRHxTLp" Content-Disposition: inline --LQksG6bCIzRHxTLp Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Mar 23, 2016 at 11:38:05AM +0000, Lionel Landwerlin wrote: > All properties can be listed regardless of whether we're dealing with > an atomic enabled driver, so load all of the properties ids regardless > at init time. >=20 > Also, we can have only one function loading the property ids and reuse > that for different DRM objects. >=20 > Signed-off-by: Lionel Landwerlin > --- > lib/igt_kms.c | 185 +++++++++++++++++++++-------------------------------= ------ > lib/igt_kms.h | 21 +++---- > 2 files changed, 76 insertions(+), 130 deletions(-) >=20 > diff --git a/lib/igt_kms.c b/lib/igt_kms.c > index 5ef89cf..3321738 100644 > --- a/lib/igt_kms.c > +++ b/lib/igt_kms.c > @@ -173,85 +173,31 @@ static const char *igt_connector_prop_names[IGT_NUM= _CONNECTOR_PROPS] =3D { > }; > =20 > /* > - * Retrieve all the properies specified in props_name and store them into > - * plane->atomic_props_plane. > + * Retrieve all the properies specified in props_name from object_id typo. > + * and store them into prop_ids. > */ > static void > -igt_atomic_fill_plane_props(igt_display_t *display, igt_plane_t *plane, > - int num_props, const char **prop_names) > +igt_atomic_fill_props(igt_display_t *display, > + uint32_t object_id, uint32_t object_type, > + uint32_t *prop_ids, const char **prop_names, > + int num_props) > { > drmModeObjectPropertiesPtr props; > - int i, j, fd; > - > - fd =3D display->drm_fd; > + int i, j; > =20 > - props =3D drmModeObjectGetProperties(fd, plane->drm_plane->plane_id, DR= M_MODE_OBJECT_PLANE); > + props =3D drmModeObjectGetProperties(display->drm_fd, > + object_id, object_type); > igt_assert(props); > =20 > for (i =3D 0; i < props->count_props; i++) { > drmModePropertyPtr prop =3D > - drmModeGetProperty(fd, props->props[i]); > + drmModeGetProperty(display->drm_fd, props->props[i]); > =20 > for (j =3D 0; j < num_props; j++) { > if (strcmp(prop->name, prop_names[j]) !=3D 0) > continue; > =20 > - plane->atomic_props_plane[j] =3D props->props[i]; > - break; > - } > - > - drmModeFreeProperty(prop); > - } > - > - drmModeFreeObjectProperties(props); > -} > - > -/* > - * Retrieve all the properies specified in props_name and store them into > - * config->atomic_props_crtc and config->atomic_props_connector. > - */ > -static void > -igt_atomic_fill_props(igt_display_t *display, igt_output_t *output, > - int num_crtc_props, const char **crtc_prop_names, > - int num_connector_props, const char **conn_prop_names) > -{ > - drmModeObjectPropertiesPtr props; > - int i, j, fd; > - > - fd =3D display->drm_fd; > - > - props =3D drmModeObjectGetProperties(fd, output->config.crtc->crtc_id, = DRM_MODE_OBJECT_CRTC); > - igt_assert(props); > - > - for (i =3D 0; i < props->count_props; i++) { > - drmModePropertyPtr prop =3D > - drmModeGetProperty(fd, props->props[i]); > - > - for (j =3D 0; j < num_crtc_props; j++) { > - if (strcmp(prop->name, crtc_prop_names[j]) !=3D 0) > - continue; > - > - output->config.atomic_props_crtc[j] =3D props->props[i]; > - break; > - } > - > - drmModeFreeProperty(prop); > - } > - > - drmModeFreeObjectProperties(props); > - props =3D NULL; > - props =3D drmModeObjectGetProperties(fd, output->config.connector->conn= ector_id, DRM_MODE_OBJECT_CONNECTOR); > - igt_assert(props); > - > - for (i =3D 0; i < props->count_props; i++) { > - drmModePropertyPtr prop =3D > - drmModeGetProperty(fd, props->props[i]); > - > - for (j =3D 0; j < num_connector_props; j++) { > - if (strcmp(prop->name, conn_prop_names[j]) !=3D 0) > - continue; > - > - output->config.atomic_props_connector[j] =3D props->props[i]; > + prop_ids[j] =3D props->props[i]; > break; > } > =20 > @@ -259,7 +205,6 @@ igt_atomic_fill_props(igt_display_t *display, igt_out= put_t *output, > } > =20 > drmModeFreeObjectProperties(props); > - > } > =20 > const unsigned char* igt_kms_get_alt_edid(void) > @@ -1118,8 +1063,9 @@ static void igt_output_refresh(igt_output_t *output) > kmstest_pipe_name(output->config.pipe)); > =20 > display->pipes_in_use |=3D 1 << output->config.pipe; > - igt_atomic_fill_props(display, output, IGT_NUM_CRTC_PROPS, igt_crtc_pro= p_names, > - IGT_NUM_CONNECTOR_PROPS, igt_connector_prop_names); > + igt_atomic_fill_props(display, output->id, DRM_MODE_OBJECT_CONNECTOR, > + output->properties, igt_connector_prop_names, > + IGT_NUM_CONNECTOR_PROPS); Does it make sense to call it if !display->atomic? > } > =20 > static bool > @@ -1189,7 +1135,6 @@ void igt_display_init(igt_display_t *display, int d= rm_fd) > drmModeRes *resources; > drmModePlaneRes *plane_resources; > int i; > - int is_atomic =3D 0; > =20 > memset(display, 0, sizeof(igt_display_t)); > =20 > @@ -1207,7 +1152,7 @@ void igt_display_init(igt_display_t *display, int d= rm_fd) > display->n_pipes =3D resources->count_crtcs; > =20 > drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1); > - is_atomic =3D drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1); > + display->is_atomic =3D drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1= ) =3D=3D 0; > plane_resources =3D drmModeGetPlaneResources(display->drm_fd); > igt_assert(plane_resources); > =20 > @@ -1265,10 +1210,12 @@ void igt_display_init(igt_display_t *display, int= drm_fd) > =20 > plane->pipe =3D pipe; > plane->drm_plane =3D drm_plane; > - if (is_atomic =3D=3D 0) { > - display->is_atomic =3D 1; > - igt_atomic_fill_plane_props(display, plane, IGT_NUM_PLANE_PROPS, igt= _plane_prop_names); > - } > + igt_atomic_fill_props(display, > + plane->drm_plane->plane_id, > + DRM_MODE_OBJECT_PLANE, > + plane->properties, > + igt_plane_prop_names, > + IGT_NUM_PLANE_PROPS); > =20 > get_plane_property(display->drm_fd, drm_plane->plane_id, > "rotation", > @@ -1316,6 +1263,36 @@ void igt_display_init(igt_display_t *display, int = drm_fd) > /* planes =3D 1 primary, (p-1) sprites, 1 cursor */ > pipe->n_planes =3D p + 1; > =20 > + igt_atomic_fill_props(display, pipe->crtc_id, DRM_MODE_OBJECT_CRTC, > + pipe->properties, igt_crtc_prop_names, > + IGT_NUM_CRTC_PROPS); > + if (pipe->crtc_id) { > + uint64_t prop_value; > + > + get_crtc_property(display->drm_fd, pipe->crtc_id, > + "background_color", > + &pipe->background_property, > + &prop_value, > + NULL); > + pipe->background =3D (uint32_t)prop_value; > + get_crtc_property(display->drm_fd, pipe->crtc_id, > + "DEGAMMA_LUT", > + &pipe->degamma_property, > + NULL, > + NULL); > + get_crtc_property(display->drm_fd, pipe->crtc_id, > + "CTM", > + &pipe->ctm_property, > + NULL, > + NULL); > + get_crtc_property(display->drm_fd, pipe->crtc_id, > + "GAMMA_LUT", > + &pipe->gamma_property, > + NULL, > + NULL); > + } > + > + > /* make sure we don't overflow the plane array */ > igt_assert(pipe->n_planes <=3D IGT_MAX_PLANES); > } > @@ -1330,7 +1307,6 @@ void igt_display_init(igt_display_t *display, int d= rm_fd) > igt_assert(display->outputs); > =20 > for (i =3D 0; i < display->n_outputs; i++) { > - int j; > igt_output_t *output =3D &display->outputs[i]; > =20 > /* > @@ -1342,35 +1318,6 @@ void igt_display_init(igt_display_t *display, int = drm_fd) > output->display =3D display; > =20 > igt_output_refresh(output); > - > - for (j =3D 0; j < display->n_pipes; j++) { > - uint64_t prop_value; > - igt_pipe_t *pipe =3D &display->pipes[j]; > - > - if (output->config.crtc) { > - get_crtc_property(display->drm_fd, output->config.crtc->crtc_id, > - "background_color", > - &pipe->background_property, > - &prop_value, > - NULL); > - pipe->background =3D (uint32_t)prop_value; > - get_crtc_property(display->drm_fd, output->config.crtc->crtc_id, > - "DEGAMMA_LUT", > - &pipe->degamma_property, > - NULL, > - NULL); > - get_crtc_property(display->drm_fd, output->config.crtc->crtc_id, > - "CTM", > - &pipe->ctm_property, > - NULL, > - NULL); > - get_crtc_property(display->drm_fd, output->config.crtc->crtc_id, > - "GAMMA_LUT", > - &pipe->gamma_property, > - NULL, > - NULL); > - } > - } > } > =20 > drmModeFreePlaneResources(plane_resources); > @@ -1945,22 +1892,19 @@ static int igt_output_commit(igt_output_t *output, > /* > * Add crtc property changes to the atomic property set > */ > -static void igt_atomic_prepare_crtc_commit(igt_output_t *output, drmMode= AtomicReq *req) > +static void igt_atomic_prepare_pipe_commit(igt_pipe_t *pipe, drmModeAtom= icReq *req) > { > + if (pipe->background_changed) > + igt_atomic_populate_pipe_req(req, pipe, IGT_CRTC_BACKGROUND, pipe->bac= kground); > =20 > - igt_pipe_t *pipe_obj =3D igt_output_get_driving_pipe(output); > - > - if (pipe_obj->background_changed) > - igt_atomic_populate_crtc_req(req, output, IGT_CRTC_BACKGROUND, pipe_ob= j->background); > - > - if (pipe_obj->color_mgmt_changed) { > - igt_atomic_populate_crtc_req(req, output, IGT_CRTC_DEGAMMA_LUT, pipe_o= bj->degamma_blob); > - igt_atomic_populate_crtc_req(req, output, IGT_CRTC_CTM, pipe_obj->ctm_= blob); > - igt_atomic_populate_crtc_req(req, output, IGT_CRTC_GAMMA_LUT, pipe_obj= ->gamma_blob); > + if (pipe->color_mgmt_changed) { > + igt_atomic_populate_pipe_req(req, pipe, IGT_CRTC_DEGAMMA_LUT, pipe->de= gamma_blob); > + igt_atomic_populate_pipe_req(req, pipe, IGT_CRTC_CTM, pipe->ctm_blob); > + igt_atomic_populate_pipe_req(req, pipe, IGT_CRTC_GAMMA_LUT, pipe->gamm= a_blob); > } > =20 > - pipe_obj->background_changed =3D false; > - pipe_obj->color_mgmt_changed =3D false; > + pipe->background_changed =3D false; > + pipe->color_mgmt_changed =3D false; > } > =20 > /* > @@ -2001,10 +1945,14 @@ static int igt_atomic_commit(igt_display_t *displ= ay) > igt_plane_t *plane; > enum pipe pipe; > =20 > + pipe_obj =3D igt_output_get_driving_pipe(output); > + > + pipe =3D pipe_obj->pipe; > + > /* > * Add CRTC Properties to the property set > */ > - igt_atomic_prepare_crtc_commit(output, req); > + igt_atomic_prepare_pipe_commit(pipe_obj, req); > =20 > /* > * Add Connector Properties to the property set > @@ -2012,9 +1960,6 @@ static int igt_atomic_commit(igt_display_t *display) > igt_atomic_prepare_connector_commit(output, req); > =20 > =20 > - pipe_obj =3D igt_output_get_driving_pipe(output); > - > - pipe =3D pipe_obj->pipe; > =20 > for_each_plane_on_pipe(display, pipe, plane) { > igt_atomic_prepare_plane_commit(plane, output, req); > diff --git a/lib/igt_kms.h b/lib/igt_kms.h > index 8ae1192..9f04f72 100644 > --- a/lib/igt_kms.h > +++ b/lib/igt_kms.h > @@ -129,8 +129,6 @@ struct kmstest_connector_config { > bool connector_scaling_mode_changed; > uint64_t connector_dpms; > bool connector_dpms_changed; > - uint32_t atomic_props_crtc[IGT_NUM_CRTC_PROPS]; > - uint32_t atomic_props_connector[IGT_NUM_CONNECTOR_PROPS]; > int crtc_idx; > int pipe; > }; > @@ -246,7 +244,7 @@ typedef struct { > /* panning offset within the fb */ > unsigned int pan_x, pan_y; > igt_rotation_t rotation; > - uint32_t atomic_props_plane[IGT_NUM_PLANE_PROPS]; > + uint32_t properties[IGT_NUM_PLANE_PROPS]; > } igt_plane_t; > =20 > struct igt_pipe { > @@ -269,6 +267,8 @@ struct igt_pipe { > uint32_t color_mgmt_changed : 1; > =20 > uint32_t crtc_id; > + > + uint32_t properties[IGT_NUM_CRTC_PROPS]; > }; So the macro remain as CRTCs? > =20 > typedef struct { > @@ -281,6 +281,7 @@ typedef struct { > unsigned long pending_crtc_idx_mask; > bool use_override_mode; > drmModeModeInfo override_mode; > + uint32_t properties[IGT_NUM_CONNECTOR_PROPS]; > } igt_output_t; > =20 > struct igt_display { > @@ -355,18 +356,18 @@ void igt_wait_for_vblank(int drm_fd, enum pipe pipe= ); > */ > #define igt_atomic_populate_plane_req(req, plane, prop, value) \ > igt_assert_lt(0, drmModeAtomicAddProperty(req, plane->drm_plane->plane_= id,\ > - plane->atomic_props_plane[prop], value)) > + plane->properties[prop], value)) > =20 > /** > - * igt_atomic_populate_crtc_req: > + * igt_atomic_populate_pipe_req: > * @req: A pointer to drmModeAtomicReq > - * @output: A pointer igt_output_t > + * @pipe: A pointer igt_pipe_t > * @prop: one of igt_atomic_crtc_properties > * @value: the value to add > */ > -#define igt_atomic_populate_crtc_req(req, output, prop, value) \ > - igt_assert_lt(0, drmModeAtomicAddProperty(req, output->config.crtc->crt= c_id,\ > - output->config.atomic_props_crtc[prop], value)) > +#define igt_atomic_populate_pipe_req(req, pipe, prop, value) \ > + igt_assert_lt(0, drmModeAtomicAddProperty(req, pipe->crtc_id,\ > + pipe->properties[prop], value)) > /** > * igt_atomic_populate_connector_req: > * @req: A pointer to drmModeAtomicReq > @@ -376,7 +377,7 @@ void igt_wait_for_vblank(int drm_fd, enum pipe pipe); > */ > #define igt_atomic_populate_connector_req(req, output, prop, value) \ > igt_assert_lt(0, drmModeAtomicAddProperty(req, output->config.connector= ->connector_id,\ > - output->config.atomic_props_connector[prop], value)) > + output->properties[prop], value)) > =20 > void igt_enable_connectors(void); > void igt_reset_connectors(void); > --=20 > 2.8.0.rc3 >=20 > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx --LQksG6bCIzRHxTLp Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJW8oh+AAoJELmLWIAQzyE+5M0H/1JOWOg+MX7QagVGkder8grT /v1lv5GBbR12j6M8MUXBP97x84E9LCmWoY8movNFU+FZ06wB/FXcPCMvTYc8pTya QXfEFjVGdrfxnbJLzhrmbMj1IXrwvRRgOC7tEASMIBE5/ABU/5lg7ExQpV9qJbcP bSFaWK5pPVhkRNK4vB9xrefmIfg6bfKbJLCBXtgJM5KNtqK35ZlW+IQoTgp1UFCL HCtzVSfbKsQ2s9ZiLInnrX7ORrlGNW78jRkrqmGg3QyNPn0bn9PCjZOZzDAAOp28 q9JJaBNChstg+GsmlCoOmPlMHHIzenE6+G0CQY7l4hP9UI5CpHWheuVbqoZxp2o= =+jOS -----END PGP SIGNATURE----- --LQksG6bCIzRHxTLp-- --===============0745086339== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vaW50ZWwtZ2Z4Cg== --===============0745086339==--