From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH 2/2] drm/i915/dp: check eDP display control capability registers Date: Mon, 18 Nov 2013 15:27:56 +0100 Message-ID: <20131118142755.GF26046@ulmo.nvidia.com> References: <1384520511-24267-1-git-send-email-jani.nikula@intel.com> <1384520511-24267-2-git-send-email-jani.nikula@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0379615923==" Return-path: In-Reply-To: <1384520511-24267-2-git-send-email-jani.nikula@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Jani Nikula Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============0379615923== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="cpvLTH7QU4gwfq3S" Content-Disposition: inline --cpvLTH7QU4gwfq3S Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Nov 15, 2013 at 03:01:51PM +0200, Jani Nikula wrote: > Debug print the capabilities, and flag an error if the panel does not > support adjusting backlight through the BL_PWM_DIM pin, requiring > backlight control through DPCD. >=20 > I haven't seen such panels yet, but it's a matter of time. Give > ourselves a reminder when we need to fix this for real. >=20 > Signed-off-by: Jani Nikula > --- > drivers/gpu/drm/i915/intel_dp.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) I have a few general comments below, but this patch itself look fine, so: Reviewed-by: Thierry Reding > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel= _dp.c > index cbf33be..ea4f3d1 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -2816,6 +2816,20 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > dev_priv->psr.sink_support =3D true; > DRM_DEBUG_KMS("Detected EDP PSR Panel.\n"); > } > + > + if (intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP] & > + DP_DPCD_DISPLAY_CONTROL_CAP) { > + u8 ctrl[4] =3D { 0 }; > + > + intel_dp_aux_native_read(intel_dp, DP_EDP_REV, > + ctrl, sizeof(ctrl)); > + DRM_DEBUG_KMS("eDP DPCD CTRL %02x %02x %02x %02x\n", > + ctrl[0], ctrl[1], ctrl[2], ctrl[3]); > + > + /* We don't support DPCD backlight control yet. */ > + if (ctrl[0] && (ctrl[1] & 1) && !(ctrl[2] & 1)) > + DRM_ERROR("eDP AUX backlight control only\n"); > + } > } > =20 > if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] & I think a lot of eDP utility code could be made reusable across drivers. We could probably do that by having each driver expose a drm_edp object of some sort. Actually, the same would be true of DP in general. Accessing the DPCD is something that's driver specific, but once you know how to do that a lot of code can be made generic. I think a struct drm_dp could look like this: struct drm_dp; struct drm_dpcd_ops { ssize_t (*read)(struct drm_dp *dp, unsigned int offset, void *buffer, size_t size); ssize_t (*write)(struct drm_dp *dp, unsigned int offset, const void *buffer, size_t size); }; struct drm_dp { const struct drm_dpcd_ops *dpcd; }; Perhaps that could even be extended with functionality to implement link training in a generic way. There are already quite a few helpers to help with that in drivers/gpu/drm/drm_dp_helper.c, but they assume that the DPCD will be handed to them as a large buffer and therefore cannot write DPCD registers. I suppose one could argue that it would be introducing a mid-layer, but that layer would be really thin in my opinion. And it would allow a lot of the algorithms to be written only once instead of multiple times. Thierry --cpvLTH7QU4gwfq3S Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJSiiPrAAoJEN0jrNd/PrOhd6YP/jiVhw6Bb+fwDJAFFUSBfCZ5 VW4/wDK2DdRCbEwShRW0ccQC2llP6FkL1LgqUaqPakWQ0VsQ2TS1kgC+VN3jA+2q 5ee0tFFZLEJeV//HSZz8KuF9zamxmhhlKj5mf0tZTxYXbPXnjO6ZZiVL58dcwzIX MGX6nHwxgDd8SAEqJ1JZBCoHk1XqNgCCDevqJKeHxgQcHrDD+VKBBzNj4wDFWC8d btBzfMdSMLGOjcVeM5FnMuTHKwqEQ2SKWFj01AERYU4XW2k57ZTqhFIAP9rFNYo5 bnrBaFYYiMr+myJSUmw8cmXsyioancXTRB2LFU/CNRhfKwzfBJmgPtJqlKHeFda2 Qz8yFnBPXNw7mF4BWaIeC+XMXa0cNWQOGdkgikzYs96jxcplvYESVG9MkV3iKw6v DvISc9O0ahbInOwGliOyN+LwYb41rhnFU/CMAXwnTGDkfB+h+kt4+0Lx8EEZRx8O Ksn6H9cjjUdTMdbU2dkxQ6SyB4HLusEzrnfrnDc4mpHdqultM+UU5Iq3854YbaJt ardqrB7WibY9BRGE5sCx4gOI7W7Kn4ZZOEXxy6ZSmTgZqrUkZKlqmy6cbDxwJcD+ +523H46Z3NF40PiReg7wAguE/75uEajEIgGqq9eDTYNfNqpZUFLYgyz488+t0WYF RADzqjS2LoyxJaofaVms =UsBV -----END PGP SIGNATURE----- --cpvLTH7QU4gwfq3S-- --===============0379615923== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx --===============0379615923==--