Re: [PATCH 4/5] drm/connector: Split out orientation quirk detection

From: Hans de Goede
Date: Wed Jun 12 2019 - 08:37:46 EST


On 12-06-19 02:16, dbasehore . wrote:
On Tue, Jun 11, 2019 at 1:54 AM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:


On 11-06-19 10:08, Jani Nikula wrote:
On Mon, 10 Jun 2019, Derek Basehore <dbasehore@xxxxxxxxxxxx> wrote:
This removes the orientation quirk detection from the code to add
an orientation property to a panel. This is used only for legacy x86
systems, yet we'd like to start using this on devicetree systems where
quirk detection like this is not needed.

Not needed, but no harm done either, right?

I guess I'll defer judgement on this to Hans and Ville (Cc'd).

Hmm, I'm not big fan of this change. It adds code duplication and as
other models with the same issue using a different driver or panel-type
show up we will get more code duplication.

Also I'm not convinced that devicetree based platforms will not need
this. The whole devicetree as an ABI thing, which means that all
devicetree bindings need to be set in stone before things are merged
into the mainline, is done solely so that we can get vendors to ship
hardware with the dtb files included in the firmware.

We've posted fixes to the devicetree well after the initial merge into
mainline before, so I don't see what you mean about the bindings being
set in stone.

That was just me repeating the official party line about devicetree.

I also don't really see the point. The devicetree is in
the kernel. If there's some setting in the devicetree that we want to
change, it's effectively the same to make the change in the devicetree
versus some quirk setting. The only difference seems to be that making
the change in the devicetree is cleaner.

I agree with you that devicetree in practice is easy to update after
shipping. But at least whenever I tried to get new bindings reviewed
I was always told that I was not allowed to count on that.

I'm 100% sure that there is e.g. ARM hardware out there which uses
non upright mounted LCD panels (I used to have a few Allwinner
tablets which did this). And given my experience with the quality
of firmware bundled tables like ACPI tables I'm quite sure that
if we ever move to firmware included dtb files that we will need
quirks for those too.

Is there a timeline to start using firmware bundled tables?

Nope, as I said "if we ever move to ...".

Since the
quirk code only uses DMI, it will need to be changed anyways for
firmware bundled devicetree files anyways.

We could consolidate the duplicated code into another function that
calls drm_get_panel_orientation_quirks too. The only reason it's like
it is is because I initially only had the call to
drm_get_panel_orientation_quirk once in the code.

Yes if you can add a new helper for the current callers, then
I'm fine with dropping the quirk handling from