Re: [PATCH v4] drm: add kms driver for loongson display controller

From: suijingfeng
Date: Thu Feb 23 2023 - 21:15:59 EST



On 2023/2/23 22:07, Thomas Zimmermann wrote:

It's the same story all over: you rather want to remove all these lookup helpers and do a single test before you create the modesetting pipeline:

 if (chip == LS7A2000 && index == 0)
    lscd_create_output_ls7a2000_0();
 else if (chip == LS7A2000 && index == 1)
    lscd_create_output_ls7a2000_1();
 else if (chip == LS7A1000 && index == 0)
    lscd_create_output_ls7a1000_0();
 else if (chip == LS7A1000 && index == 1)
    lscd_create_output_ls7a1000_1();
 else if (...)
    ...

There you create the data structures for each pair of chip and index. If you have functions that can be used by multiple pairs, sharing those is fine. You might end up with duplicated code, but the overall design is sound.

Your current design will become unmaintainable pretty soon. We already have several of such drivers.


Yes, your are right.

I'm fear of the duplicated code in the past.

I did not put my attention to the maintainable of the software.

a quick skim through radeon, I find that there also seems have "duplicated" code, this is  a design.

I didn't get it before, now I get it.