From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Jani Nikula <jani.nikula@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH i-g-t 5/8] tools/intel_vbt_decode: unify child device printing across versions
Date: Fri, 20 Oct 2017 14:33:52 +0300 [thread overview]
Message-ID: <20171020113352.GF10981@intel.com> (raw)
In-Reply-To: <87a80mjdf8.fsf@intel.com>
On Fri, Oct 20, 2017 at 01:56:11PM +0300, Jani Nikula wrote:
> On Thu, 19 Oct 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Thu, Oct 19, 2017 at 06:22:56PM +0300, Jani Nikula wrote:
> >> Make it easier to compare dumping against the struct definition.
> >>
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> ---
> >> tools/intel_vbt_decode.c | 128 ++++++++++++++++++++++++++---------------------
> >> 1 file changed, 71 insertions(+), 57 deletions(-)
> >>
> >> diff --git a/tools/intel_vbt_decode.c b/tools/intel_vbt_decode.c
> >> index 69cf1e4c6cc6..bc5f38112619 100644
> >> --- a/tools/intel_vbt_decode.c
> >> +++ b/tools/intel_vbt_decode.c
> >> @@ -401,74 +401,88 @@ static const char *mipi_bridge_type(uint8_t type)
> >> static void dump_child_device(struct context *context,
> >> const struct child_device_config *child)
> >> {
> >> - const struct child_device_config *efp = child;
> >> -
> >> if (!child->device_type)
> >> return;
> >>
> >> + printf("\tChild device info:\n");
> >> + printf("\t\tDevice handle: 0x%04x (%s)\n", child->handle,
> >> + child_device_handle(child->handle));
> >> + printf("\t\tDevice type: 0x%04x (%s)\n", child->device_type,
> >> + child_device_type(child->device_type));
> >> + dump_child_device_type_bits(child->device_type);
> >> +
> >> if (context->bdb->version < 152) {
> >> - printf("\tChild device info:\n");
> >> - printf("\t\tDevice type: %04x (%s)\n", child->device_type,
> >> - child_device_type(child->device_type));
> >> printf("\t\tSignature: %.*s\n", (int)sizeof(child->device_id), child->device_id);
> >> - printf("\t\tAIM offset: %d\n", child->addin_offset);
> >> - printf("\t\tDVO port: 0x%02x\n", child->dvo_port);
> >> - } else { /* 152+ have EFP blocks here */
> >> - printf("\tEFP device info:\n");
> >> - printf("\t\tDevice handle: 0x%04x (%s)\n", efp->handle,
> >> - child_device_handle(efp->handle));
> >> - printf("\t\tDevice type: 0x%04x (%s)\n", efp->device_type,
> >> - child_device_type(efp->device_type));
> >> - dump_child_device_type_bits(efp->device_type);
> >> - printf("\t\tI2C speed: 0x%02x\n", efp->i2c_speed);
> >> - printf("\t\tDP onboard redriver: 0x%02x\n", efp->dp_onboard_redriver);
> >> - printf("\t\tDP ondock redriver: 0x%02x\n", efp->dp_ondock_redriver);
> >> - printf("\t\tHDMI max data rate: 0x%02x\n", efp->hdmi_max_data_rate);
> >> - printf("\t\tHDMI level shifter value: 0x%02x\n", efp->hdmi_level_shifter_value);
> >> - printf("\t\tOffset to DTD buffer for edidless EFP: 0x%02x\n", efp->dtd_buf_ptr);
> >> - printf("\t\tDual pipe ganged eDP: %s\n", YESNO(efp->ganged_edp));
> >> - printf("\t\tCompression method CPS: %s\n", YESNO(efp->compression_method));
> >> - printf("\t\tCompression enable: %s\n", YESNO(efp->compression_enable));
> >> - printf("\t\tEdidless EFP: %s\n", YESNO(efp->edidless_efp));
> >> - printf("\t\tCompression structure index: 0x%02x)\n", efp->compression_structure_index);
> >> - printf("\t\tSlave DDI port: 0x%02x (%s)\n", efp->slave_port, dvo_port(efp->slave_port));
> >> - printf("\t\tAIM offset: %d\n", child->addin_offset);
> >> - printf("\t\tPort: 0x%02x (%s)\n", efp->dvo_port, dvo_port(efp->dvo_port));
> >> - printf("\t\tAIM I2C pin: 0x%02x\n", efp->i2c_pin);
> >> - printf("\t\tAIM Slave address: 0x%02x\n", efp->slave_addr);
> >> - printf("\t\tDDC pin: 0x%02x\n", efp->ddc_pin);
> >> - printf("\t\tEDID buffer ptr: 0x%02x\n", efp->edid_ptr);
> >> - printf("\t\tDVO config: 0x%02x\n", efp->dvo_cfg);
> >> - printf("\t\tHPD sense invert: %s\n", YESNO(efp->hpd_invert));
> >> - printf("\t\tIboost enable: %s\n", YESNO(efp->iboost));
> >> - printf("\t\tOnboard LSPCON: %s\n", YESNO(efp->lspcon));
> >> - printf("\t\tLane reversal: %s\n", YESNO(efp->lane_reversal));
> >> - printf("\t\tEFP routed through dock: %s\n", YESNO(efp->efp_routed));
> >> - printf("\t\tHDMI compatible? %s\n", YESNO(efp->hdmi_support));
> >> - printf("\t\tDP compatible? %s\n", YESNO(efp->dp_support));
> >> - printf("\t\tTMDS compatible? %s\n", YESNO(efp->tmds_support));
> >> - printf("\t\tAux channel: 0x%02x\n", efp->aux_channel);
> >> - printf("\t\tDongle detect: 0x%02x\n", efp->dongle_detect);
> >> - printf("\t\tIntegrated encoder instead of SDVO: %s\n", YESNO(efp->integrated_encoder));
> >> - printf("\t\tHotplu connect status: 0x%02x\n", efp->hpd_status);
> >> - printf("\t\tSDVO stall signal available: %s\n", YESNO(efp->sdvo_stall));
> >> - printf("\t\tPipe capabilities: 0x%02x\n", efp->pipe_cap);
> >> - printf("\t\tDVO wiring: 0x%02x\n", efp->dvo_wiring);
> >> - printf("\t\tMIPI bridge type: %02x (%s)\n", efp->mipi_bridge_type,
> >> - mipi_bridge_type(efp->mipi_bridge_type));
> >> - printf("\t\tDevice class extendsion: 0x%02x\n", efp->extended_type);
> >> - printf("\t\tDVO function: 0x%02x\n", efp->dvo_function);
> >> + } else {
> >> + printf("\t\tI2C speed: 0x%02x\n", child->i2c_speed);
> >> + printf("\t\tDP onboard redriver: 0x%02x\n", child->dp_onboard_redriver);
> >> + printf("\t\tDP ondock redriver: 0x%02x\n", child->dp_ondock_redriver);
> >> + printf("\t\tHDMI level shifter value: 0x%02x\n", child->hdmi_level_shifter_value);
> >> + printf("\t\tHDMI max data rate: 0x%02x\n", child->hdmi_max_data_rate);
> >> + printf("\t\tOffset to DTD buffer for edidless CHILD: 0x%02x\n", child->dtd_buf_ptr);
> >> + printf("\t\tEdidless EFP: %s\n", YESNO(child->edidless_efp));
> >> + printf("\t\tCompression enable: %s\n", YESNO(child->compression_enable));
> >> + printf("\t\tCompression method CPS: %s\n", YESNO(child->compression_method));
> >> + printf("\t\tDual pipe ganged eDP: %s\n", YESNO(child->ganged_edp));
> >> + printf("\t\tCompression structure index: 0x%02x)\n", child->compression_structure_index);
> >> + printf("\t\tSlave DDI port: 0x%02x (%s)\n", child->slave_port, dvo_port(child->slave_port));
> >> + }
> >> +
> >> + printf("\t\tAIM offset: %d\n", child->addin_offset);
> >> + printf("\t\tDVO Port: 0x%02x (%s)\n", child->dvo_port, dvo_port(child->dvo_port));
> >> +
> >> + if (context->bdb->version < 152)
> >> + return;
> >> +
> >> + printf("\t\tAIM I2C pin: 0x%02x\n", child->i2c_pin);
> >> + printf("\t\tAIM Slave address: 0x%02x\n", child->slave_addr);
> >> + printf("\t\tDDC pin: 0x%02x\n", child->ddc_pin);
> >> + printf("\t\tEDID buffer ptr: 0x%02x\n", child->edid_ptr);
> >> + printf("\t\tDVO config: 0x%02x\n", child->dvo_cfg);
> >> +
> >> + if (context->bdb->version < 158) {
> >> + printf("\t\tDVO2 Port: 0x%02x (%s)\n", child->dvo2_port, dvo_port(child->dvo2_port));
> >> + printf("\t\tI2C2 pin: 0x%02x\n", child->i2c2_pin);
> >> + printf("\t\tSlave2 address: 0x%02x\n", child->slave2_addr);
> >> + printf("\t\tDDC2 pin: 0x%02x\n", child->ddc2_pin);
> >> + } else {
> >> + printf("\t\tEFP routed through dock: %s\n", YESNO(child->efp_routed));
> >> + printf("\t\tLane reversal: %s\n", YESNO(child->lane_reversal));
> >> + printf("\t\tOnboard LSPCON: %s\n", YESNO(child->lspcon));
> >> + printf("\t\tIboost enable: %s\n", YESNO(child->iboost));
> >> + printf("\t\tHPD sense invert: %s\n", YESNO(child->hpd_invert));
> >> + printf("\t\tHDMI compatible? %s\n", YESNO(child->hdmi_support));
> >> + printf("\t\tDP compatible? %s\n", YESNO(child->dp_support));
> >> + printf("\t\tTMDS compatible? %s\n", YESNO(child->tmds_support));
> >> + printf("\t\tAux channel: 0x%02x\n", child->aux_channel);
> >> + printf("\t\tDongle detect: 0x%02x\n", child->dongle_detect);
> >
> > The version of the spec I have at hand says aux and dongle are
> > already part of version 155.
>
> Seems like dvo2/i2c2/slave2/ddc2 were converted to flags at 155. Okay if
> I just change the condition above from < 158 to < 155?
Aye.
I didn't trawl through all the gritty details of the entire series, but
what I do see I like, so for the series
Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
I wonder if we should add more version checks so that we wouldn't dump
things that aren't actually specified? One crazy idea I had is that we
could define a parallel structure which would have a matching u8 member
to everything in the child_device_config, and we would initialize
each member with the correct version number. Then we could have some
kind of nice dump macro/function that checks the current VBT version
against the number from said parallel structure.
>
> BR,
> Jani.
>
>
> >
> >> + }
> >> +
> >> + printf("\t\tPipe capabilities: 0x%02x\n", child->pipe_cap);
> >> + printf("\t\tSDVO stall signal available: %s\n", YESNO(child->sdvo_stall));
> >> + printf("\t\tHotplu connect status: 0x%02x\n", child->hpd_status);
> > ^
> > Pre-existing typo
> >
> >> + printf("\t\tIntegrated encoder instead of SDVO: %s\n", YESNO(child->integrated_encoder));
> >> + printf("\t\tDVO wiring: 0x%02x\n", child->dvo_wiring);
> >> +
> >> + if (context->bdb->version < 171) {
> >> + printf("\t\tDVO2 wiring: 0x%02x\n", child->dvo2_wiring);
> >> + } else {
> >> + printf("\t\tMIPI bridge type: %02x (%s)\n", child->mipi_bridge_type,
> >> + mipi_bridge_type(child->mipi_bridge_type));
> >> }
> >>
> >> + printf("\t\tDevice class extendsion: 0x%02x\n", child->extended_type);
> > ^
> > ditto
> >
> >> + printf("\t\tDVO function: 0x%02x\n", child->dvo_function);
> >> +
> >> if (context->bdb->version >= 195) {
> >> - printf("\t\tDP USB type C support: %s\n", YESNO(efp->dp_usb_type_c));
> >> - printf("\t\t2X DP GPIO index: 0x%02x\n", efp->dp_gpio_index);
> >> - printf("\t\t2X DP GPIO pin number: 0x%02x\n", efp->dp_gpio_pin_num);
> >> + printf("\t\tDP USB type C support: %s\n", YESNO(child->dp_usb_type_c));
> >> + printf("\t\t2X DP GPIO index: 0x%02x\n", child->dp_gpio_index);
> >> + printf("\t\t2X DP GPIO pin number: 0x%02x\n", child->dp_gpio_pin_num);
> >> }
> >>
> >> if (context->bdb->version >= 196) {
> >> - printf("\t\tIBoost level for HDMI: 0x%02x\n", efp->hdmi_iboost_level);
> >> - printf("\t\tIBoost level for DP/eDP: 0x%02x\n", efp->dp_iboost_level);
> >> + printf("\t\tIBoost level for HDMI: 0x%02x\n", child->hdmi_iboost_level);
> >> + printf("\t\tIBoost level for DP/eDP: 0x%02x\n", child->dp_iboost_level);
> >> }
> >> }
> >>
> >> --
> >> 2.11.0
>
> --
> Jani Nikula, Intel Open Source Technology Center
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2017-10-20 11:33 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-19 15:22 [PATCH i-g-t 0/8] tools/intel_vbt_decode: refactoring and cleanups Jani Nikula
2017-10-19 15:22 ` [PATCH i-g-t 1/8] tools/intel_vbt_decode: make a copy of child devices before dumping Jani Nikula
2017-10-19 15:22 ` [PATCH i-g-t 2/8] tools/intel_vbt_decode: update dvo port name dumping Jani Nikula
2017-10-19 15:22 ` [PATCH i-g-t 3/8] tools/intel_vbt_decode: use %.*s instead of duplicating a string Jani Nikula
2017-10-19 15:22 ` [PATCH i-g-t 4/8] tools/intel_vbt_decode: abstract DSI bridge type dump Jani Nikula
2017-10-19 15:22 ` [PATCH i-g-t 5/8] tools/intel_vbt_decode: unify child device printing across versions Jani Nikula
2017-10-19 17:24 ` Ville Syrjälä
2017-10-20 10:56 ` Jani Nikula
2017-10-20 11:33 ` Ville Syrjälä [this message]
2017-10-20 13:17 ` Jani Nikula
2017-10-20 13:49 ` Ville Syrjälä
2017-10-19 15:22 ` [PATCH i-g-t 6/8] tools/intel_vbt_decode: unify legacy child device block dumping Jani Nikula
2017-10-19 15:22 ` [PATCH i-g-t 7/8] tools/intel_vbt_decode: dump more child device data for version < 152 Jani Nikula
2017-10-19 15:22 ` [PATCH i-g-t 8/8] tools/intel_vbt_decode: abstract child devices printing more Jani Nikula
2017-10-19 16:52 ` ✗ Fi.CI.BAT: failure for tools/intel_vbt_decode: refactoring and cleanups Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171020113352.GF10981@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jani.nikula@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.