On Thu, 16 Feb 2017, Ville Syrjälä <ville.syrj...@linux.intel.com> wrote:
> On Thu, Feb 16, 2017 at 07:54:00PM +0200, Jani Nikula wrote:
>> On Thu, 16 Feb 2017, Ville Syrjälä <ville.syrj...@linux.intel.com> wrote:
>> > On Thu, Feb 16, 2017 at 12:36:45PM +0200, Jani Nikula wrote:
>> >> Handle debugfs override edid and firmware edid at the low level to
>> >> transparently and completely replace the real edid. Previously, we
>> >> practically only used the modes from the override EDID, and none of the
>> >> other data. This also prevents actual EDID reads when the EDID is to be
>> >> overridden, but retains the DDC probe.
>> >
>> > Hmm. Isn't that a bad thing? If someone has broken DDC on their cable or
>> > somethign then the override EDID wouldn't be returned by drm_get_edid().
>> 
>> Isn't this in line with how this currently works?
>
> Not really. If connector->force is used and an override EDID is
> provided, then drm_get_edid() likely never gets called, and the modes
> come directly from the overide EDID. With your change forcing the
> connector state wouldn't because we still wouldn't get any modes
> from the override EDID due to the DDC check.

Right.

> I guess one way around this would be to skip the DDC check if the
> connector state has been forced.

I guess I'm asking if you think the whole approach here is
workable. What we currently have is so flaky.

BR,
Jani.

>
>> 
>> BR,
>> Jani.
>> 
>> 
>> >
>> >> 
>> >> FIXME: validate override edid, deduplicate firmware edid validation.
>> >> 
>> >> Signed-off-by: Jani Nikula <jani.nik...@intel.com>
>> >> ---
>> >>  drivers/gpu/drm/drm_edid.c         | 15 +++++++++++++++
>> >>  drivers/gpu/drm/drm_probe_helper.c | 19 +------------------
>> >>  2 files changed, 16 insertions(+), 18 deletions(-)
>> >> 
>> >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> >> index 362036360724..054e2d74eafc 100644
>> >> --- a/drivers/gpu/drm/drm_edid.c
>> >> +++ b/drivers/gpu/drm/drm_edid.c
>> >> @@ -1309,6 +1309,10 @@ static void connector_bad_edid(struct 
>> >> drm_connector *connector,
>> >>   * level, drivers must make all reasonable efforts to expose it as an I2C
>> >>   * adapter and use drm_get_edid() instead of abusing this function.
>> >>   *
>> >> + * The EDID may be overridden using debugfs override_edid or firmare EDID
>> >> + * (drm_load_edid_firmware()), in this priority order. Having either of 
>> >> them
>> >> + * bypasses actual EDID reads.
>> >> + *
>> >>   * Return: Pointer to valid EDID or NULL if we couldn't find any.
>> >>   */
>> >>  struct edid *drm_do_get_edid(struct drm_connector *connector,
>> >> @@ -1318,6 +1322,17 @@ struct edid *drm_do_get_edid(struct drm_connector 
>> >> *connector,
>> >>  {
>> >>   int i, j = 0, valid_extensions = 0;
>> >>   u8 *edid, *new;
>> >> + struct edid *override = NULL;
>> >> +
>> >> + if (connector->override_edid)
>> >> +         override = drm_edid_duplicate((const struct edid *)
>> >> +                                       connector->edid_blob_ptr->data);
>> >> +
>> >> + if (!override)
>> >> +         override = drm_load_edid_firmware(connector);
>> >> +
>> >> + if (!IS_ERR_OR_NULL(override))
>> >> +         return override;
>> >>  
>> >>   if ((edid = kmalloc(EDID_LENGTH, GFP_KERNEL)) == NULL)
>> >>           return NULL;
>> >> diff --git a/drivers/gpu/drm/drm_probe_helper.c 
>> >> b/drivers/gpu/drm/drm_probe_helper.c
>> >> index 358957118ca9..871326cbc465 100644
>> >> --- a/drivers/gpu/drm/drm_probe_helper.c
>> >> +++ b/drivers/gpu/drm/drm_probe_helper.c
>> >> @@ -199,8 +199,6 @@ drm_connector_detect(struct drm_connector *connector, 
>> >> bool force)
>> >>   *    drm_mode_probed_add(). New modes start their life with status as 
>> >> OK.
>> >>   *    Modes are added from a single source using the following priority 
>> >> order.
>> >>   *
>> >> - *    - debugfs 'override_edid' (used for testing only)
>> >> - *    - firmware EDID (drm_load_edid_firmware())
>> >>   *    - &drm_connector_helper_funcs.get_modes vfunc
>> >>   *    - if the connector status is connector_status_connected, standard
>> >>   *      VESA DMT modes up to 1024x768 are automatically added
>> >> @@ -305,22 +303,7 @@ int drm_helper_probe_single_connector_modes(struct 
>> >> drm_connector *connector,
>> >>           goto prune;
>> >>   }
>> >>  
>> >> - if (connector->override_edid) {
>> >> -         struct edid *edid = (struct edid *) 
>> >> connector->edid_blob_ptr->data;
>> >> -
>> >> -         count = drm_add_edid_modes(connector, edid);
>> >> -         drm_edid_to_eld(connector, edid);
>> >> - } else {
>> >> -         struct edid *edid = drm_load_edid_firmware(connector);
>> >> -         if (!IS_ERR_OR_NULL(edid)) {
>> >> -                 drm_mode_connector_update_edid_property(connector, 
>> >> edid);
>> >> -                 count = drm_add_edid_modes(connector, edid);
>> >> -                 drm_edid_to_eld(connector, edid);
>> >> -                 kfree(edid);
>> >> -         }
>> >> -         if (count == 0)
>> >> -                 count = (*connector_funcs->get_modes)(connector);
>> >> - }
>> >> + count = (*connector_funcs->get_modes)(connector);
>> >>  
>> >>   if (count == 0 && connector->status == connector_status_connected)
>> >>           count = drm_add_modes_noedid(connector, 1024, 768);
>> >> -- 
>> >> 2.1.4
>> 
>> -- 
>> Jani Nikula, Intel Open Source Technology Center

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to