Hi, [snip] >> I am assuming that you are referring to 'DISPC_IRQ_MASK_ERROR' and the >> registering/unregistering of irq masks in manager.c > > Not only the irqs, but also, for example, code like this: > > > if (channel == OMAP_DSS_CHANNEL_LCD || > channel == OMAP_DSS_CHANNEL_LCD2) > bit = 5; /* GOLCD */ > else > bit = 6; /* GODIGIT */ > > if (channel == OMAP_DSS_CHANNEL_LCD2) > return REG_GET(DISPC_CONTROL2, bit, bit) == 1; else > return REG_GET(DISPC_CONTROL, bit, bit) == 1; > Things would be bad only for DISPC_CONTROL and DISPC_CONFIG since we couldn't parametrize them based on channel. Others look scalable, I don't know if there will be more such registers on OMAP5, but we can ask HW Team to keep them specific to the channel. > So lots of the functions contain ifs based on the channel. I > don't know right now what would be the best way to make the > code cleaner (probably some kinds of look-up tables, but they > incur some overhead), and as I said, it's cosmetical and can > be cleaned up later (presuming we come up with a good way). > >> I guess we could have a dss_features function which could return the >> mask based on what omap it is. But this way the mask values/contents >> would be totally invisble in manager.c and dispc.c, one > would need to check it in dss_features.c, which also isn't readable. > > Right. I agree that that solution isn't perhaps the best one either. > >> One thing which I would like to add is that this series > doesn't need to touch any board file for now. >> The present dss_recheck_connections() doesn't try to differentiate >> between LCD and DIGIT channels, it just uses 'omap_display_type' to >> differentiate between them. Only a panel which needs to connect to >> LCD2 has to do this, This method wouldn't have worked if we didn't >> switch to uniform use of dssdev->manager->id instead of >> dssdev->channel. We will need to change > dss_recheck_connections() in the future to make it more uniform. > > Ok. > > If you can split this patch into the two parts I suggested > (if that's ok for you, you didn't comment on that one), and > check if there's anything to add to the commit descriptions, > I think we can go and apply this patch set. If I split this into 2 parts, the first patch will be just a line addition in display.h, I was beaten up by the community because of this in the previous version. One line patches in a series aren't taken very kindly :|. What I can do is clearly mention the 2 sets of things which this commit does, if you think that is good enough. > > Btw, on what platforms have you tested this (or generally any > patches you send?). I only have 3430SDP currently that I can > easily use to test, so my testing is a bit limited. > This one I had tested throughly on 3430SDP, bootup on zoom2 and zoom3. I couldn't test on omap2, we have a n800 board but we don't know what panel driver we need to use, board-n8x0.c doesn't tell anything about. If you can tell me what panel driver to use etc I can try out. Archit-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
