Reduce the image size.

Yes, this is unrelated to the previous patch, but for the coding it is important to have same style for range limit EIDD block. I mean the following: for calculation if mode is in range one style ,but for ranges retrieval another style. I am proposing to follow the style which looks to me closer to the spec.

I have attached the screenshot demonstrates those magic numbers. Vitaly

-----Original Message----- From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter Sent: Tuesday, May 03, 2016 4:26 PM To: Prosyak, Vitaly Cc: dri-devel@lists.freedesktop.org Subject: Re: [PATCH] drm/edid : calculate vsync and hsync from range limits block according to the EDID 1.4

On Tue, May 03, 2016 at 11:05:25AM -0400, Vitaly Prosyak wrote:

Do calculation of vsync and hsync from range limits EDID block according to the spec. EDID 1.4.

Signed-off-by: Vitaly Prosyak vitaly.prosyak@amd.com

Seems unrelated to the previous patch, please submit in a separate series.

---

drivers/gpu/drm/drm_edid.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 7e49962..601152b 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1977,11 +1977,11 @@ mode_in_hsync_range(const struct drm_display_mode *mode, int hsync, hmin, hmax;

hmin = t[7];

• if (edid->revision >= 4)

•    hmin += ((t[4] & 0x04) ? 255 : 0);
  

• if (edid->revision >= 4 && ((t[4] & 0x0c) == 0x0c))

•    hmin += 255 ;
  

  hmax = t[8];

• if (edid->revision >= 4)

•    hmax += ((t[4] & 0x08) ? 255 : 0);
  

• if (edid->revision >= 4 && (t[4] & 0x08))

•    hmax += 255;
  

Please don't reshuffle code without functional changes. Same above, for such small bugfixes it's best to only change what you need changed.

It might also be useful to introduce symbolic #defines for all these magic constants. -Daniel

hsync = drm_mode_hsync(mode);

return (hsync <= hmax && hsync >= hmin); @@ -1994,11 +1994,11 @@ mode_in_vsync_range(const struct drm_display_mode *mode, int vsync, vmin, vmax;

vmin = t[5];

• if (edid->revision >= 4)

•    vmin += ((t[4] & 0x01) ? 255 : 0);
  

• if (edid->revision >= 4 && ((t[4] & 0x03) == 0x03))

•    vmin += 255;
  

  vmax = t[6];

• if (edid->revision >= 4)

•    vmax += ((t[4] & 0x02) ? 255 : 0);
  

• if (edid->revision >= 4 && (t[4] & 0x02))

•    vmax += 255;
  

  vsync = drm_mode_vrefresh(mode);

  return (vsync <= vmax && vsync >= vmin);

-- 1.9.1

---

dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch

On Tue, 03 May 2016, Vitaly Prosyak vitaly.prosyak@amd.com wrote:

Do calculation of vsync and hsync from range limits EDID block according to the spec. EDID 1.4.

Signed-off-by: Vitaly Prosyak vitaly.prosyak@amd.com

drivers/gpu/drm/drm_edid.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 7e49962..601152b 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1977,11 +1977,11 @@ mode_in_hsync_range(const struct drm_display_mode *mode, int hsync, hmin, hmax;

hmin = t[7];

• if (edid->revision >= 4)

•    hmin += ((t[4] & 0x04) ? 255 : 0);
  

• if (edid->revision >= 4 && ((t[4] & 0x0c) == 0x0c))

•    hmin += 255 ;
  

  hmax = t[8];

• if (edid->revision >= 4)

•    hmax += ((t[4] & 0x08) ? 255 : 0);
  

• if (edid->revision >= 4 && (t[4] & 0x08))

•    hmax += 255;
  

  hsync = drm_mode_hsync(mode);

  return (hsync <= hmax && hsync >= hmin);

@@ -1994,11 +1994,11 @@ mode_in_vsync_range(const struct drm_display_mode *mode, int vsync, vmin, vmax;

vmin = t[5];

• if (edid->revision >= 4)

•    vmin += ((t[4] & 0x01) ? 255 : 0);
  

• if (edid->revision >= 4 && ((t[4] & 0x03) == 0x03))

•    vmin += 255;
  

  vmax = t[6];

• if (edid->revision >= 4)

•    vmax += ((t[4] & 0x02) ? 255 : 0);
  

• if (edid->revision >= 4 && (t[4] & 0x02))

•    vmax += 255;
  

Please fix the indentation to use tabs on the lines you change while you're at it.

BR, Jani.

vsync = drm_mode_vrefresh(mode);

return (vsync <= vmax && vsync >= vmin);

We are going to use min/max vertical refresh rate when enter/exit to the dynamic refresh rate mode ,it is known as 'freesync', enter/exit to full screen games. Right , we may have a helper function which extracts these properties and store wherever need it. But this an alternative solution with additional helper adds some code duplication into drm_edid.c since the case already present in the code: case 0x01: /* just the ranges, no formula */ Also these values are parsed during each mode enumeration.

Why I put into connector: Drm connector have already similar items from EDID: drm_tile_group ,etc...

Vitaly -----Original Message-----.

From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter Sent: Tuesday, May 03, 2016 4:24 PM To: Prosyak, Vitaly Cc: dri-devel@lists.freedesktop.org Subject: Re: [PATCH] drm/edid : cache edid range limits in drm connector

On Tue, May 03, 2016 at 11:05:24AM -0400, Vitaly Prosyak wrote:

Cache in drm connector the edid range limits properties: min/max vertical refresh rates and max pixel clock. It would be used when enter to drr mode.

Signed-off-by: Vitaly Prosyak vitaly.prosyak@amd.com

Where's the patches that will use this? It might make sense to instead have some helper functions to compute these values, so that drivers can store them wherever they want/need. But impossible to tell without the users of this stuff. -Daniel

---

drivers/gpu/drm/drm_edid.c | 11 +++++++++++ include/drm/drm_crtc.h | 5 +++++ 2 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 04cb487..7e49962 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2190,6 +2190,17 @@ do_inferred_modes(struct detailed_timing *timing, void *c) timing); break; case 0x01: /* just the ranges, no formula */

• closure->connector->min_vfreq = range->min_vfreq;
  

• closure->connector->max_vfreq = range->max_vfreq;
  

• if (closure->edid->revision >= 4) {
  

• 	if ((range->flags & 0x03) == 0x3)
  

• 		closure->connector->min_vfreq += 255;
  

• 	if (range->flags & 0x02)
  

• 		closure->connector->max_vfreq += 255;
  

• }
  

• closure->connector->max_pixel_clock_khz =
  

• 	range_pixel_clock(closure->edid, (u8 *)timing);
  

• break;
  

  default: break; }

diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index c5b4b81..85fc554 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1230,6 +1230,11 @@ struct drm_connector { uint8_t num_h_tile, num_v_tile; uint8_t tile_h_loc, tile_v_loc; uint16_t tile_h_size, tile_v_size;

• /*EDID range limits for drr*/
• int min_vfreq ;
• int max_vfreq ;
• int max_pixel_clock_khz;

};

/**

1.9.1

---

dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch

On Wed, 04 May 2016, "Prosyak, Vitaly" Vitaly.Prosyak@amd.com wrote:

We are going to use min/max vertical refresh rate when enter/exit to the dynamic refresh rate mode ,it is known as 'freesync', enter/exit to full screen games.

As Daniel said, we should see the patch using them too. It's hard to say whether this is the right thing to do otherwise.

BR, Jani.

Right , we may have a helper function which extracts these properties and store wherever need it. But this an alternative solution with additional helper adds some code duplication into drm_edid.c since the case already present in the code: case 0x01: /* just the ranges, no formula */ Also these values are parsed during each mode enumeration.

Why I put into connector: Drm connector have already similar items from EDID: drm_tile_group ,etc...

Vitaly -----Original Message-----.

From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter Sent: Tuesday, May 03, 2016 4:24 PM To: Prosyak, Vitaly Cc: dri-devel@lists.freedesktop.org Subject: Re: [PATCH] drm/edid : cache edid range limits in drm connector

On Tue, May 03, 2016 at 11:05:24AM -0400, Vitaly Prosyak wrote:

...

Cache in drm connector the edid range limits properties: min/max vertical refresh rates and max pixel clock. It would be used when enter to drr mode.

Signed-off-by: Vitaly Prosyak vitaly.prosyak@amd.com

Where's the patches that will use this? It might make sense to instead have some helper functions to compute these values, so that drivers can store them wherever they want/need. But impossible to tell without the users of this stuff. -Daniel

...

---

drivers/gpu/drm/drm_edid.c | 11 +++++++++++ include/drm/drm_crtc.h | 5 +++++ 2 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 04cb487..7e49962 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2190,6 +2190,17 @@ do_inferred_modes(struct detailed_timing *timing, void *c) timing); break; case 0x01: /* just the ranges, no formula */

• closure->connector->min_vfreq = range->min_vfreq;
  

• closure->connector->max_vfreq = range->max_vfreq;
  

• if (closure->edid->revision >= 4) {
  

• 	if ((range->flags & 0x03) == 0x3)
  

• 		closure->connector->min_vfreq += 255;
  

• 	if (range->flags & 0x02)
  

• 		closure->connector->max_vfreq += 255;
  

• }
  

• closure->connector->max_pixel_clock_khz =
  

• 	range_pixel_clock(closure->edid, (u8 *)timing);
  

• break;
  

  default: break; }

diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index c5b4b81..85fc554 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1230,6 +1230,11 @@ struct drm_connector { uint8_t num_h_tile, num_v_tile; uint8_t tile_h_loc, tile_v_loc; uint16_t tile_h_size, tile_v_size;

• /*EDID range limits for drr*/
• int min_vfreq ;
• int max_vfreq ;
• int max_pixel_clock_khz;

};

/**

1.9.1

---

dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel