Hi,
On Wed, Nov 8, 2023 at 7:45 AM Maxime Ripard mripard@kernel.org wrote:
@@ -575,9 +599,18 @@ static int panel_edp_get_modes(struct drm_panel *panel,
if (!p->edid) p->edid = drm_get_edid(connector, p->ddc);
if (p->edid)num += drm_add_edid_modes(connector, p->edid);
if (p->edid) {if (has_override_edid_mode) {It's not clear to me why the override mechanism is only there when there's a ddc bus?
I think you're confusing the two different (but related) issues addressed by this series. One is when you're using the generic "edp-panel" compatible string. In that case the mode comes from the EDID and only the EDID since there's no hardcoded mode. We need a mode override there since some EDIDs shipped with a bad mode. That's the subject of ${SUBJECT} patch.
The second issue is what to do with a hardcoded mode. That's the subject of the next patch in the series (patch #5). Previously we merged the hardcoded and EDID modes. Now (in the next patch) we use only the hardcoded mode. There's no need for a fixup because the mode is hardcoded in the kernel.
You mentioned before that you were following panel-simple,
As of the newest version of the patch, it's no longer following panel-simple in response to your feedback on previous versions.
but that's a clear deviation from what I can see. If there's a reason for that deviation, that's fine by me, but it should at least be documented in the commit log.
I think the commit log is OK. I suspect the confusion is only because you've reviewed previous versions of the series. Please shout if things still look confusing.
@@ -950,6 +983,19 @@ static const struct panel_desc auo_b101ean01 = { }, };
+static const struct drm_display_mode auo_b116xa3_mode = {
.clock = 70589,.hdisplay = 1366,.hsync_start = 1366 + 40,.hsync_end = 1366 + 40 + 40,.htotal = 1366 + 40 + 40 + 32,.vdisplay = 768,.vsync_start = 768 + 10,.vsync_end = 768 + 10 + 12,.vtotal = 768 + 10 + 12 + 6,.flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,+};
That should be a separate patch
That's fair. I didn't think it was a huge deal, but I agree that it's slightly cleaner.
-Doug