[PATCH] drm/i915: customize DPCD brightness control for specific panel

Adam Jackson ajax at redhat.com
Fri Oct 4 15:19:20 UTC 2019

On Sat, 2019-10-05 at 05:58 +0800, Lee Shawn C wrote:
> This panel (manufacturer is SDC, product ID is 0x4141)
> used manufacturer defined DPCD register to control brightness
> that not defined in eDP spec so far. This change follow panel
> vendor's instruction to support brightness adjustment.

I'm sure this works, but this smells a little funny to me.

> +	/* Samsung eDP panel */

It feels a bit like a layering violation to identify eDP behavior
changes based on EDID. But I'm not sure there's any obvious way to
identify this device by its DPCD. The Sink OUI (from the linked
bugzilla) seems to be 0011F8, which doesn't match up to anything in my

> @@ -1953,6 +1956,7 @@ static u32 edid_get_quirks(const struct edid *edid)
>  	return 0;
>  }
> +EXPORT_SYMBOL(edid_get_quirks);

If we're going to export this it should probably get a drm_ prefix.

> +#define DPCD_EDP_BRIGHTNESS_NITS		0x354
> +

This also seems a bit weird, the 0x300-0x3FF registers belong to the
_source_ DP device. But then later...

> +	/* write source OUI */
> +	write_val[0] = 0x00;
> +	write_val[1] = 0xaa;
> +	write_val[2] = 0x01;

Oh hey, you're writing (an) Intel OUI to the Source OUI, so now it
makes sense that you're writing to registers whose behavior the source
defines. But this does raise the question: is this just a convention
between Intel and this particular panel? Would we expect this to work
with other similar panels? Is there any way to know to expect this
convention from DPCD instead?

- ajax

More information about the dri-devel mailing list