Re: [PATCH v1 4/4] drm/panel: ili9806e: Break some CMDS into helper functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/07/2024 21:36, Doug Anderson wrote:
Hi,

On Wed, Jul 10, 2024 at 6:09 PM cong yang
<yangcong5@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> wrote:

Hi,

Michael Walle <mwalle@xxxxxxxxxx> 于2024年7月11日周四 03:38写道:

On Wed Jul 10, 2024 at 9:12 PM CEST, Doug Anderson wrote:
Hi,

On Wed, Jul 10, 2024 at 2:02 AM Michael Walle <mwalle@xxxxxxxxxx> wrote:

On Wed Jul 10, 2024 at 10:47 AM CEST, Cong Yang wrote:
Break select page cmds into helper function.

Why though? I don't find that anything easier to read. In fact, I
deliberately chose not to factor that out into a function. It's just
a sequence of magic commands, taken straight from the datasheet. So,
I'd like to keep it that way.

The consensus of previous discussion on the lists was that folks
agreed that we should, where possible, make it more obvious what these
magic sequences of commands were doing. IMO separating out the page
switch command helps. Certainly I'm always happy to hear other
opinions, though.

Fair enough, but in that case, one should take the datasheet (which
you can find online) and replace all the magic numbers with the
correct command names from it. E.g. 0xff is the ENEXTC register. To
be clear, I'm not just talking about the "switch page command".

As patch stands, I don't see much value, TBH. On the contrary, you
make it harder to compare it with the Ortustech panel datasheet.

just my 2c,
-michael

If all drivers replace all the magic numbers with the correct command names,
it will be a huge amount of work (assuming that the datasheet can be found).
  I am afraid I don't have enough time to complete it.  Thanks.

Makes sense. I'd be interested in hearing the opinion of others in the
DRM community about whether they'd prefer to land something long this
patch as-is or drop it.

I don't have a strong opinion, but I think only changing the switch
page operations doesn't make a lot of sense by itself.

Having a much larger register coverage would be preferred.

Neil


-Doug




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux