Re: [PATCH 4/4] radeon: fall back to ACPI EDID retrieval

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 7/28/20 1:50 AM, Christian König wrote:

Am 27.07.20 um 22:53 schrieb Daniel Dadap:
Fall back to retrieving the EDID via the ACPI _DDC method, when present
for notebook internal panels, when retrieving BIOS-embedded EDIDs.

Signed-off-by: Daniel Dadap <ddadap@xxxxxxxxxx>
---
  drivers/gpu/drm/radeon/radeon_combios.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_combios.c b/drivers/gpu/drm/radeon/radeon_combios.c
index c3e49c973812..de801d9fca54 100644
--- a/drivers/gpu/drm/radeon/radeon_combios.c
+++ b/drivers/gpu/drm/radeon/radeon_combios.c
@@ -401,9 +401,8 @@ bool radeon_combios_check_hardcoded_edid(struct radeon_device *rdev)
  struct edid *
  radeon_bios_get_hardcoded_edid(struct radeon_device *rdev)
  {
-     struct edid *edid;
-
      if (rdev->mode_info.bios_hardcoded_edid) {
+             struct edid *edid;

That's an unrelated an incorrect style change. You need a blank line
after declaration.


Ah, yes, that doesn't really need to be changed. I'll remove it from this patch. Would a separate patch to change the scope of that declaration (with a blank line after) be welcome, or should I just leave it alone?



              edid = kmalloc(rdev->mode_info.bios_hardcoded_edid_size, GFP_KERNEL);
              if (edid) {
                      memcpy((unsigned char *)edid,
@@ -412,7 +411,8 @@ radeon_bios_get_hardcoded_edid(struct radeon_device *rdev)
                      return edid;
              }
      }
-     return NULL;
+
+     return drm_get_edid_acpi();

In general a good idea, but I'm wondering if we should really do this so
unconditionally here.


I'm not personally aware of any AMD notebook designs that require the ACPI _DDC EDID retrieval. I've only seen it on NVIDIA+Intel hybrid systems and on a small number of NVIDIA discrete-only systems. I just figured I'd update the radeon DRM-KMS driver while updating i915 and Nouveau, for completeness, as it could be helpful should such a design exist. As for whether there should be some condition around this, I suppose that's reasonable, but I'm not really sure what would make sense as a condition. As it stands, drm_edid_acpi() only returns a value if at least one of the VGA or 3D controllers on the system provides an ACPI _DDC method, and if that ACPI method successfully returns an EDID.

On the caller's end, it's currently part of the path where the radeon driver is already trying to fall back to a hardcoded EDID provided by the system. Perhaps instead if we call it within the LVDS || eDP condition here, instead?


        if (rdev->is_atom_bios) {
            /* some laptops provide a hardcoded edid in rom for LCDs */
            if (((connector->connector_type == DRM_MODE_CONNECTOR_LVDS) ||
                 (connector->connector_type == DRM_MODE_CONNECTOR_eDP)))
                radeon_connector->edid = radeon_bios_get_hardcoded_edid(rdev);
        } else {
            /* some servers provide a hardcoded edid in rom for KVMs */
            radeon_connector->edid = radeon_bios_get_hardcoded_edid(rdev);
}

That would be more in line with the changes in this patchset for i915 and nouveau.


Regards,
Christian.

  }

  static struct radeon_i2c_bus_rec combios_setup_i2c_bus(struct radeon_device *rdev,

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux