On Tue, 10 Sep 2019 12:48:42 +0300, Ville Syrjälä wrote:
On Tue, Sep 10, 2019 at 11:46:20AM +0200, Jean Delvare wrote:
Hi Ville,
On Mon, 2 Sep 2019 16:15:46 +0300, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Let's make cea_db_offsets() a bit more convenient to use by setting the start/end offsets to zero whenever the data block collection isn't present. This makes it safe for the caller to blindly iterate the data blocks even if there are none.
Cc: Jean Delvare jdelvare@suse.de Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/drm_edid.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 7b3072fc550b..e5905dc764c1 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3690,6 +3690,9 @@ cea_revision(const u8 *cea) static int cea_db_offsets(const u8 *cea, int *start, int *end) {
- *start = 0;
- *end = 0;
- if (cea_revision(cea) < 3) return -ENOTSUPP;
@@ -4116,10 +4119,7 @@ static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) if (cea_revision(cea) >= 3) { int i, start, end;
if (cea_db_offsets(cea, &start, &end)) {
start = 0;
end = 0;
}
cea_db_offsets(cea, &start, &end);
for_each_cea_db(cea, i, start, end) { db = &cea[i];
Not sure if that's really needed. As it stands there's only one function which wants to continue after cea_db_offsets() fails, all others just bail out at that point. Now that cea_db_offsets() checks for revision >= 3, the construct above could simply become:
int i, start, end;
if (cea_db_offsets(cea, &start, &end) == 0) { for_each_cea_db(cea, i, start, end) { db = &cea[i];
which is IMHO more elegant and does not require zeroing start and end in cea_db_offsets().
I dislike indentation.
It would be the same as currently, so it's not that bad. But well I'm not maintaining that piece of code so it's not my call anyway.
Also could perhaps even move the cea_db_offsets() into the for_each_cea_db() macro so that the caller doesn't have to care about these mundane details at all.
If the macro stays readable, why not.